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

[Michael Niedermayer]

On Thu, Sep 01, 2016 at 06:17:11PM -0700, Brett Harrison wrote:
> Most recent patch.  I was evaluating fontsize too early before when using
> the 'n' variable in equations.
> 
> On Thu, Sep 1, 2016 at 3:44 PM, Brett Harrison <brett.harrison at zyamusic.com>
> wrote:
> 
> > Any feedback on this newest patch?
> >
> > On Tue, Aug 30, 2016 at 2:17 PM, Brett Harrison <
> > brett.harrison at zyamusic.com> wrote:
> >
> >> Since there are differing opinions on how the default fontsize should be
> >> established this patch adds my changes while preserving the current
> >> behavior when fontsize is not specified.
> >>
> >> On Tue, Aug 30, 2016 at 2:43 AM, Nicolas George <george at nsup.org> wrote:
> >>
> >>> Le tridi 13 fructidor, an CCXXIV, Brett Harrison a écrit :
> >>> > 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/fo
> >>> nts/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?
> >>>
> >>> I think the current behaviour is correct.
> >>>
> >>> I start with the following principle: when users want something precise
> >>> about aesthetic or other arbitrary settings, they have to say it.
> >>>
> >>> Default values are for the lazy or the careless ones: quick profiling and
> >>> testing when the exact result does not matter much. But as soon as the
> >>> result matters, explicit values must be given.
> >>>
> >>> Do not take me wrong, the default values should be, as much as possible,
> >>> sensible. But for a font size, 12 is as sensible as 16.
> >>>
> >>> Most importantly, backward compatibility should not be an hindrance to
> >>> choosing better default values. We should not change them lightly, but
> >>> not
> >>> feel forbidden to do so either.
> >>>
> >>> Fontconfig is not just a path search library for finding font files. It
> >>> is a
> >>> complete mechanism for choosing the right font according to several
> >>> conditions set by the user. Including the font size.
> >>>
> >>> There are no less than four levels for choosing the font size:
> >>>
> >>> (1) lavfi's default, 16;
> >>>
> >>> (2) fontconfig's default;
> >>>
> >>> (3) fontconfig's explicit value, as in "Times-12:bold";
> >>>
> >>> (4) lavfi's explicit value, as in "fontsize=16".
> >>>
> >>> I think the order of precedence should be just that, for the following
> >>> reasons:
> >>>
> >>> - First, of course, (3), (4) > (1), (2), because explicit values are
> >>> always
> >>>   more important.
> >>>
> >>> - Second, conflicting explicit values are the users' problem. We can
> >>> produce
> >>>   warnings to help diagnose, but in the end it is their choice. (4) >
> >>> (3) is
> >>>   slightly easier to implement (distinguishing (3) from (2) requires a
> >>> bit
> >>>   of work), and (4) is more supple, especially when your patch gets
> >>> applied.
> >>>
> >>> - Last, (2) > (1), because fontconfig is more cross-applications than
> >>> lavfi,
> >>>   and also because it includes a mechanism for explicit configuration.
> >>>
> >>> Still, the documentation should be clarified, possibly something like
> >>> that:
> >>> "The default value of fontsize is provided by fontconfig if in use or 16
> >>> when using a font file directly."
> >>>
> >>> Regards,
> >>>
> >>> --
> >>>   Nicolas George
> >>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel at ffmpeg.org
> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>>
> >>
> >

>  vf_drawtext.c |  121 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 109 insertions(+), 12 deletions(-)
> e87e1b5f3fa57f9c0dee2d30e9889ba0503c0a0c  0001-added-expr-evaluation-to-drawtext-fontsize.patch
> From 1913b998490df8ae2bca4a5b3034b19f5e44928f 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

libavfilter/vf_drawtext.c: In function ‘update_fontsize’:
libavfilter/vf_drawtext.c:422:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]

also patch breaks:
./ffmpeg -i m.mpg  -vf drawtext=fontfile=/usr/share/fonts/truetype/msttcorefonts/arial.ttf:text=a -f null -

[AVFilterGraph @ 0x37a6960] Error initializing filter 'drawtext' with args 'fontfile=/usr/share/fonts/truetype/msttcorefonts/arial.ttf:text=a'

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- 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/20160902/54a0b9cd/attachment.sig>