[FFmpeg-devel] control line spacing and support for text borders on drawtext filter (patch attached)
Leandro Santiago
leandrosansilva at gmail.com
Wed Sep 12 13:30:34 CEST 2012
Thanks for the tips. I'll fix the issues you indicated and resend the patches.
2012/9/11 Nicolas George <nicolas.george at normalesup.org>:
> Le sextidi 26 fructidor, an CCXX, Leandro Santiago a écrit :
>> Hello to all. I've made some modifications on drawtext filter which I
>> currently use at work.
>>
>> The modifications are the support of controlling the spacing between
>> lines and a very primitive support for borders around text. It's
>> primitive because it works only with one or 2px (I basically copied
>> the shadow code to execute four times...). I know the correct is using
>> the freetype support for text borders (which is very powerful), but
>> for now this patch is working for me.
>>
>> I'd enjoy if you revised it.
>
> Thanks for the work. A few general remarks first:
>
> - You should use git format-patch or git send-email, as the resulting patch
> is easier to apply, in particular it includes the author name, the date,
> the commit message. git format-patch outputs one file per commit that you
> can attach to your mail; git sent-email does that and sends it directly.
>
> - (Minor) If you send your mail yourself and not with git send-email, please
> try to get the attachments to have content-type text/plain or
> text/something: it makes it slightly easier to read it directly in a MUA.
>
> - Your patches implements two completely unrelated features; it is the
> policy in ffmpeg to require two separates commits for that. Fortunately,
> git helps a lot for that.
>
> - You forgot to add the documentation to the options you added. It should go
> in doc/filters.texi, along with the rest of the drawtext documentation.
>
>> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
>> index 729f152..e398732 100644
>> --- a/libavfilter/vf_drawtext.c
>> +++ b/libavfilter/vf_drawtext.c
>> @@ -133,6 +133,11 @@ typedef struct {
>> char *boxcolor_string; ///< box color as string
>> char *shadowcolor_string; ///< shadow color as string
>>
>> + int bordersize; ///< border weight
>> + char *bordercolor_string; ///< border color as string
>> +
>> + int line_spacing; ///< lines spacing in pixels
>> +
>> short int draw_box; ///< draw box around text - true or false
>> int use_kerning; ///< font kerning is used - true/false
>> int tabsize; ///< tab size
>> @@ -142,6 +147,7 @@ typedef struct {
>> FFDrawColor fontcolor; ///< foreground color
>> FFDrawColor shadowcolor; ///< shadow color
>> FFDrawColor boxcolor; ///< background color
>> + FFDrawColor bordercolor; ///< border color
>>
>> FT_Library library; ///< freetype font library handle
>> FT_Face face; ///< freetype font face handle
>> @@ -188,6 +194,11 @@ static const AVOption drawtext_options[]= {
>> {"rate", "set rate (timecode only)", OFFSET(tc_rate), AV_OPT_TYPE_RATIONAL, {.dbl=0}, 0, INT_MAX, FLAGS},
>> {"fix_bounds", "if true, check and fix text coords to avoid clipping", OFFSET(fix_bounds), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, FLAGS},
>>
>
>> +{"bordercolor", "set border color", OFFSET(bordercolor_string), AV_OPT_TYPE_STRING, {.str="yellow"}, CHAR_MIN, CHAR_MAX,FLAGS },
>> +{"bordersize", "set border weight", OFFSET(bordersize), AV_OPT_TYPE_INT, {.dbl=0}, INT_MIN, INT_MAX,FLAGS },
>
> weight -> width?
>
>> +
>
>> +{"line_spacing", "set line spacing in pixels", OFFSET(line_spacing), AV_OPT_TYPE_INT, {.dbl=0}, INT_MIN, INT_MAX,FLAGS },
>
> It would be nicer if it was an expression that could use the variables, like
> x and y. I do not consider that kind of critics blocking, though, especially
> in that case since the font size should be an expression too and is not.
>
>> +
>> /* FT_LOAD_* flags */
>> {"ft_load_flags", "set font loading flags for libfreetype", OFFSET(ft_load_flags), AV_OPT_TYPE_FLAGS, {.i64=FT_LOAD_DEFAULT|FT_LOAD_RENDER}, 0, INT_MAX, FLAGS, "ft_load_flags"},
>> {"default", "set default", 0, AV_OPT_TYPE_CONST, {.i64=FT_LOAD_DEFAULT}, INT_MIN, INT_MAX, FLAGS, "ft_load_flags"},
>> @@ -455,6 +466,13 @@ static av_cold int init(AVFilterContext *ctx, const char *args)
>> return err;
>> }
>>
>> + if ((err = av_parse_color(dtext->bordercolor.rgba, dtext->bordercolor_string, -1, ctx))) {
>> + av_log(ctx, AV_LOG_ERROR,
>> + "Invalid border color '%s'\n", dtext->shadowcolor_string);
>> + return err;
>> + }
>> +
>> +
>
> Stray extra new line.
>
>> if ((err = FT_Init_FreeType(&(dtext->library)))) {
>> av_log(ctx, AV_LOG_ERROR,
>> "Could not load FreeType: %s\n", FT_ERRMSG(err));
>> @@ -538,6 +556,7 @@ static int config_input(AVFilterLink *inlink)
>> ff_draw_color(&dtext->dc, &dtext->fontcolor, dtext->fontcolor.rgba);
>> ff_draw_color(&dtext->dc, &dtext->shadowcolor, dtext->shadowcolor.rgba);
>> ff_draw_color(&dtext->dc, &dtext->boxcolor, dtext->boxcolor.rgba);
>> + ff_draw_color(&dtext->dc, &dtext->bordercolor, dtext->bordercolor.rgba);
>>
>> dtext->var_values[VAR_w] = dtext->var_values[VAR_W] = dtext->var_values[VAR_MAIN_W] = inlink->w;
>> dtext->var_values[VAR_h] = dtext->var_values[VAR_H] = dtext->var_values[VAR_MAIN_H] = inlink->h;
>> @@ -713,7 +732,7 @@ static int draw_text(AVFilterContext *ctx, AVFilterBufferRef *picref,
>> prev_code = code;
>> if (is_newline(code)) {
>> max_text_line_w = FFMAX(max_text_line_w, x);
>
>> - y += dtext->max_glyph_h;
>> + y += dtext->max_glyph_h + dtext->line_spacing;
>
> If I read this correctly, line_spacing is "extra" line spacing according to
> a few layout engines who call "line spacing" the distance between the base
> line of successive lines. This is not a problem, but please make sure this
> is clear in the docs.
>
>> x = 0;
>> continue;
>> }
>> @@ -772,6 +791,27 @@ static int draw_text(AVFilterContext *ctx, AVFilterBufferRef *picref,
>> return ret;
>> }
>>
>> + // draw text border.
>> + if (dtext->bordersize) {
>> + int offset = dtext->bordersize;
>> + int pos[8][2] = {
>> + {0,-offset},
>> + {-offset,-offset},
>> + {offset,0},
>> + {0,offset},
>> +
>> + {-offset,offset},
>> + {offset,offset},
>> + {offset,-offset},
>> + {-offset,-offset}
>> + };
>
> Please align the table to make it more readable.
>
>> +
>> + int i;
>> + for (i = 0; i < 4; i++) // using 8 also draws on "diagonals"
>
> The unused part of the table should be commented out too, I think.
>
>> + if ((ret = draw_glyphs(dtext, picref, width, height, dtext->bordercolor.rgba,&dtext->bordercolor,pos[i][0],pos[i][1])) < 0)
>> + return ret;
>> + }
>> +
>
> I had not realized what you meant by "border" until I read that part of the
> code. I believe "outline" is usually used for that.
>
> Also, note that the algorithm you are implementing gives ugly results when
> the offset is larger than 1. I just looked at the implementation in libass
> (stroke_outline in ass_render.c): it seems FreeType has builtin functions to
> do that correctly.
>
>> if ((ret = draw_glyphs(dtext, picref, width, height, dtext->fontcolor.rgba,
>> &dtext->fontcolor, 0, 0)) < 0)
>> return ret;
>
> Regards,
>
> --
> Nicolas George
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (GNU/Linux)
>
> iEYEARECAAYFAlBPi00ACgkQsGPZlzblTJN1jgCeKmHUOXcV1uKRdIlgV1kqdPTK
> PcYAn2QGnSYULtX39dm/4grjFgsoMcnP
> =FnnC
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
--
-----
Sent from my Atari
More information about the ffmpeg-devel
mailing list