[FFmpeg-devel] Fwd: [PATCH] added expr evaluation to drawtext - fontsize
Michael Niedermayer
michael at niedermayer.cc
Wed Apr 19 13:38:00 EEST 2017
On Tue, Apr 18, 2017 at 06:20:51PM -0700, Brett Harrison wrote:
> On Mon, Apr 17, 2017 at 5:48 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
>
> > 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
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> >> + if (s->fontsize_pexpr)
> >> + return 0;
>
> > indention is inconsistent
>
> fixed
>
> >> + if (!isnan(size))
> >> + fontsize = round(size);
>
> >round() returns a double
> >fontsize is unsigend int.
> >the cast can overflow
>
> added check for overflow and returned an error
>
> >> + return set_fontsize(ctx);
>
> >set_fontsize(ctx, fontsize);
> >seems cleaner to me
>
> added second parameter and set s->fontsize in the set_fontsize call
> vf_drawtext.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 116 insertions(+), 17 deletions(-)
> 3adf54dd36434e02e3b70e63ebd77147febf6133 0001-added-expr-evaluation-to-drawtext-fontsize.patch
> From c43567075c55defd30cf1717589a7c715a27de31 Mon Sep 17 00:00:00 2001
> From: Brett Harrison <brett.harrison at musicmastermind.com>
> Date: Tue, 18 Apr 2017 18:15:39 -0700
> Subject: [PATCH] added expr evaluation to drawtext fontsize
applied
can you add a fate test for this feature ?
(is probably not completely easy due to non bitexactness)
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20170419/2d6a014b/attachment.sig>
More information about the ffmpeg-devel
mailing list