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

Michael Niedermayer michael at niedermayer.cc
Tue Apr 18 03:48:16 EEST 2017


On Sun, Apr 16, 2017 at 10:01:01PM -0700, Brett Harrison wrote:
> Any comments on this patch?
> 
> ---------- Forwarded message ----------
> From: Brett Harrison <brett.harrison at zyamusic.com>
> Date: Tue, Apr 11, 2017 at 1:37 PM
> Subject: Re: [FFmpeg-devel] [PATCH] added expr evaluation to drawtext -
> fontsize
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> 
> 
> Pinging for comments / review...
> 
> On Tue, Apr 4, 2017 at 3:45 PM, Brett Harrison <brett.harrison at zyamusic.com>
> wrote:
> 
> > Resurrecting this patch.
> >
> > On Thu, Sep 15, 2016 at 3:20 AM, Michael Niedermayer <
> > michael at niedermayer.cc> wrote:
> >
> >> On Fri, Sep 09, 2016 at 05:26:03PM -0700, Brett Harrison wrote:
> >> > Here are the changes requested
> >> [...]
> >> > +static av_cold int parse_fontsize(AVFilterContext *ctx)
> >> > +{
> >> > +    DrawTextContext *s = ctx->priv;
> >> > +    int err;
> >> > +
> >> > +    if (s->fontsize_expr == NULL)
> >> > +        return AVERROR(EINVAL);
> >> > +
> >> > +    av_expr_free(s->fontsize_pexpr);
> >> > +    s->fontsize_pexpr = NULL;
> >> > +
> >> > +    if ((err = av_expr_parse(&s->fontsize_pexpr, s->fontsize_expr,
> >> var_names,
> >> > +                             NULL, NULL, fun2_names, fun2, 0, ctx)) <
> >> 0)
> >> > +        return err;
> >> > +
> >> > +    return 0;
> >> > +}
> >>
> >> why is av_expr_parse() not executed where the other av_expr_parse()
> >> are ?
> >>
> >
> > I needed to perform av_expr_parse() during init() to resolve the default
> > fontsize.  init() is called before config_input() where the other
> > av_expr_parse() calls are.
> >
> >

>  vf_drawtext.c |  125 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 108 insertions(+), 17 deletions(-)
> 085506596906b7f89f46edf6d21d34374e92d994  0001-added-expr-evaluation-to-drawtext-fontsize.patch
> From 8647e01f8ac2cd622e0ff5c1257773cfffa01ed9 Mon Sep 17 00:00:00 2001
> From: Brett Harrison <brett.harrison at musicmastermind.com>
> Date: Tue, 4 Apr 2017 15:39:06 -0700
> Subject: [PATCH] added expr evaluation to drawtext - fontsize
> 
> ---
>  libavfilter/vf_drawtext.c | 125 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 108 insertions(+), 17 deletions(-)
> 
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 8b24f50..77209f3 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -156,7 +156,10 @@ 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
> +    unsigned int default_fontsize;  ///< default font size to use
>  
>      int line_spacing;               ///< lines spacing in pixels
>      short int draw_box;             ///< draw box around text - true or false
> @@ -211,7 +214,7 @@ static const AVOption drawtext_options[]= {
>      {"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},
>      {"line_spacing",  "set line spacing in pixels", OFFSET(line_spacing),   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=NULL},  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},
> @@ -281,6 +284,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;
> @@ -293,7 +297,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);
>  }
>  
>  /**
> @@ -317,6 +325,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);
> @@ -366,6 +375,68 @@ error:
>      return ret;
>  }
>  
> +static av_cold int set_fontsize(AVFilterContext *ctx)
> +{
> +    int err;
> +    DrawTextContext *s = ctx->priv;
> +
> +    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 parse_fontsize(AVFilterContext *ctx)
> +{
> +    DrawTextContext *s = ctx->priv;
> +    int err;
> +
> +    if (s->fontsize_pexpr)
> +      return 0;

indention is inconsistent


> +
> +    if (s->fontsize_expr == NULL)
> +        return AVERROR(EINVAL);
> +
> +    if ((err = av_expr_parse(&s->fontsize_pexpr, s->fontsize_expr, var_names,
> +                             NULL, NULL, fun2_names, fun2, 0, ctx)) < 0)
> +        return err;
> +
> +    return 0;
> +}
> +
> +static av_cold int update_fontsize(AVFilterContext *ctx)
> +{
> +    DrawTextContext *s = ctx->priv;
> +    unsigned int fontsize = s->default_fontsize;
> +    int err;
> +    double size;
> +
> +    // if no fontsize specified use the default
> +    if (s->fontsize_expr != NULL) {
> +        if ((err = parse_fontsize(ctx)) < 0)
> +           return err;
> +
> +        size = av_expr_eval(s->fontsize_pexpr, s->var_values, &s->prng);
> +

> +        if (!isnan(size))
> +            fontsize = round(size);

round() returns a double
fontsize is unsigend int.
the cast can overflow


> +    }
> +
> +    if (fontsize == 0)
> +        fontsize = 1;

> +
> +    // no change
> +    if (fontsize == s->fontsize)
> +        return 0;
> +
> +    s->fontsize = fontsize;
> +
> +    return set_fontsize(ctx);

set_fontsize(ctx, fontsize);
seems cleaner to me

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170418/66f2d08a/attachment.sig>


More information about the ffmpeg-devel mailing list