[FFmpeg-devel] [PATCH] added expr evaluation to drawtext - fontsize

Brett Harrison brett.harrison at zyamusic.com
Tue Aug 30 00:23:44 EEST 2016


Before I fix the patch, can you clarify the intended functionality?

The docs say that 16 is the default fontsize, however if
CONFIG_LIBFONTCONFIG is configured and ffmpeg if called with:

-vf drawtext=text=abc:fontcolor=white

on my system the font used will be /opt/X11/share/fonts/TTF/Vera.ttf (the
default chosen by libfontconfig) and the fontsize will be set to 12.

However if ffmpeg is called with:

-vf
drawtext=text=abc:fontcolor=white:fontfile=/opt/X11/share/fonts/TTF/Vera.ttf

This is the same font that libfontconfig used, however this time fontsize
16 is used as stated in the docs.

The difference is this line of code in load_font_fontconfig
  if (!s->fontsize)
        s->fontsize = size + 0.5;

I didn't set the fontsize in either command, but the output was different.
Do we want to keep this as is?


On Sun, Aug 28, 2016 at 6:09 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Sat, Aug 27, 2016 at 04:30:05PM -0700, Brett Harrison wrote:
> > Fixed patch based on comments.
> >
> > This time attaching patch file.
> >
> > On Sat, Aug 27, 2016 at 6:21 AM, Moritz Barsnick <barsnick at gmx.net>
> wrote:
> >
> > > On Fri, Aug 26, 2016 at 14:37:42 -0700, Brett Harrison wrote:
> > >
> > > > +    if (diff != 0) {
> > > > +      return diff > 0 ? 1 : diff < 0 ? -1 : 0;
> > > > +    }
> > >
> > > If diff != 0, it can only be >0 or <0, nothing else:
> > >        if (diff != 0)
> > >          return diff > 0 ? 1 : -1;
> > > (And you can drop the curly brackets.)
> > >
> > > > +    else {
> > > > +      int64_t diff = (int64_t)a->fontsize - (int64_t)bb->fontsize;
> > > > +      return diff > 0 ? 1 : diff < 0 ? -1 : 0;
> > > > +    }
> > >
> > > There's a macro for this:
> > >        else
> > >            return FFDIFFSIGN((int64_t)a->fontsize,
> (int64_t)bb->fontsize);
> > >
> > > Moritz
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
>
> >  vf_drawtext.c |   86 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++++-----
> >  1 file changed, 79 insertions(+), 7 deletions(-)
> > dd219d9b4d4f02ca4299a0bfd6ea3d5c15545f2b  0001-added-expr-evaluation-to-
> drawtext-fontsize.patch
> > From 5c9d7d18a5d2f15f48605021d7f5a7890a318cc4 Mon Sep 17 00:00:00 2001
> > From: Brett Harrison <brett.harrison at musicmastermind.com>
> > Date: Fri, 26 Aug 2016 14:29:34 -0700
> > Subject: [PATCH] added expr evaluation to drawtext - fontsize
>
> this changes the output and fontsize when fontsize is not explicitly
> specified
>
> for example:
> -vf drawtext=text=abc:fontcolor=white
>
>
> >
> > ---
> >  libavfilter/vf_drawtext.c | 86 ++++++++++++++++++++++++++++++
> +++++++++++++----
> >  1 file changed, 79 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> > index 214aef0..5311e29 100644
> > --- a/libavfilter/vf_drawtext.c
> > +++ b/libavfilter/vf_drawtext.c
> > @@ -156,6 +156,8 @@ typedef struct DrawTextContext {
> >      int max_glyph_h;                ///< max glyph height
> >      int shadowx, shadowy;
> >      int borderw;                    ///< border width
> > +    char *fontsize_expr;            ///< expression for fontsize
> > +    AVExpr *fontsize_pexpr;         ///< parsed expressions for fontsize
> >      unsigned int fontsize;          ///< font size to use
> >
> >      short int draw_box;             ///< draw box around text - true or
> false
> > @@ -209,7 +211,7 @@ static const AVOption drawtext_options[]= {
> >      {"shadowcolor", "set shadow color",     OFFSET(shadowcolor.rgba),
>  AV_OPT_TYPE_COLOR,  {.str="black"}, CHAR_MIN, CHAR_MAX, FLAGS},
> >      {"box",         "set box",              OFFSET(draw_box),
>  AV_OPT_TYPE_BOOL,   {.i64=0},     0,        1       , FLAGS},
> >      {"boxborderw",  "set box border width", OFFSET(boxborderw),
>  AV_OPT_TYPE_INT,    {.i64=0},     INT_MIN,  INT_MAX , FLAGS},
> > -    {"fontsize",    "set font size",        OFFSET(fontsize),
>  AV_OPT_TYPE_INT,    {.i64=0},     0,        INT_MAX , FLAGS},
> > +    {"fontsize",    "set font size",        OFFSET(fontsize_expr),
> AV_OPT_TYPE_STRING, {.str="16"},  CHAR_MIN, CHAR_MAX , FLAGS},
> >      {"x",           "set x expression",     OFFSET(x_expr),
>  AV_OPT_TYPE_STRING, {.str="0"},   CHAR_MIN, CHAR_MAX, FLAGS},
> >      {"y",           "set y expression",     OFFSET(y_expr),
>  AV_OPT_TYPE_STRING, {.str="0"},   CHAR_MIN, CHAR_MAX, FLAGS},
> >      {"shadowx",     "set shadow x offset",  OFFSET(shadowx),
> AV_OPT_TYPE_INT,    {.i64=0},     INT_MIN,  INT_MAX , FLAGS},
> > @@ -280,6 +282,7 @@ typedef struct Glyph {
> >      FT_Glyph glyph;
> >      FT_Glyph border_glyph;
> >      uint32_t code;
> > +    unsigned int fontsize;
> >      FT_Bitmap bitmap; ///< array holding bitmaps of font
> >      FT_Bitmap border_bitmap; ///< array holding bitmaps of font border
> >      FT_BBox bbox;
> > @@ -292,7 +295,11 @@ static int glyph_cmp(const void *key, const void *b)
> >  {
> >      const Glyph *a = key, *bb = b;
> >      int64_t diff = (int64_t)a->code - (int64_t)bb->code;
> > -    return diff > 0 ? 1 : diff < 0 ? -1 : 0;
> > +
> > +    if (diff != 0)
> > +         return diff > 0 ? 1 : -1;
> > +    else
> > +         return FFDIFFSIGN((int64_t)a->fontsize,
> (int64_t)bb->fontsize);
> >  }
> >
> >  /**
> > @@ -316,6 +323,7 @@ static int load_glyph(AVFilterContext *ctx, Glyph
> **glyph_ptr, uint32_t code)
> >          goto error;
> >      }
> >      glyph->code  = code;
> > +    glyph->fontsize = s->fontsize;
> >
> >      if (FT_Get_Glyph(s->face->glyph, &glyph->glyph)) {
> >          ret = AVERROR(EINVAL);
> > @@ -591,12 +599,62 @@ out:
> >  }
> >  #endif
> >
> > +static av_cold int set_fontsize(AVFilterContext *ctx)
> > +{
> > +    int err;
> > +    DrawTextContext *s = ctx->priv;
> > +
> > +    if (s->face == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Font not open\n");
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> > +    if ((err = FT_Set_Pixel_Sizes(s->face, 0, s->fontsize))) {
> > +        av_log(ctx, AV_LOG_ERROR, "Could not set font size to %d
> pixels: %s\n",
> > +               s->fontsize, FT_ERRMSG(err));
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static av_cold int update_fontsize(AVFilterContext *ctx)
> > +{
> > +    DrawTextContext *s = ctx->priv;
> > +    unsigned int fontsize = 16;
> > +
> > +    double size = av_expr_eval(s->fontsize_pexpr, s->var_values,
> &s->prng);
> > +
> > +    if (isnan(size))
> > +        fontsize = 16;
>
> > +    else if (round(size) <= 0)
> > +        fontsize = 1;
> > +    else
> > +        fontsize = round(size);
>
> you should check fontsize not round(size) and then re-evaluate
> round(size), C doesnt gurantee that to be teh same
>
>
> > +
> > +    // no change
> > +    if (fontsize == s->fontsize) {
> > +      return 0;
> > +    }
>
> inconsistet indention and supeflous {}
>
>
> [...]
>
> > @@ -1084,6 +1148,7 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> >      for (i = 0, p = text; *p; i++) {
> >          FT_Bitmap bitmap;
> >          Glyph dummy = { 0 };
> > +
> >          GET_UTF8(code, *p++, continue;);
> >
> >          /* skip new line chars, just go to new line */
>
> stray change
>
> [...]
>
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I do not agree with what you have to say, but I'll defend to the death your
> right to say it. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


More information about the ffmpeg-devel mailing list