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

[Brett Harrison]

Addressed the following concerns.

===

>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]

Fixed this.

>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'

This is fixed.

===

>>* +        av_log(ctx, AV_LOG_ERROR, "Font not open\n");
*
>I was wondering: Was does this message tell the user?

I changed this error to read "Unable to initialize font".  This error
should only be seen if set_fontsize() is called before the font is
loaded.  I don't think a user would be able to cause this because if
the font fails to load ffmpeg would error out before this.  It is a
sanity check.

===

For the concerns about multiple to many brackets on "if ((ret =
update_fontsize(ctx)))",

I removed the "ret =" part as I wasn't using the value anyway.


On Fri, Sep 2, 2016 at 6:13 AM, Nicolas George <george at nsup.org> wrote:

> Le septidi 17 fructidor, an CCXXIV, Moritz Barsnick a écrit :
> > *Assuming* he means "assign update_fontsize()'s return value to ret,
> > and check whether ret is != 0", which would correspond to the more
> > verbose
> >   if ((ret = update_fontsize(ctx)) != 0) {
> >
> > is it sufficient to say:
> >   if (ret = update_fontsize(ctx)) {
> >
> > or is it required, or is it more readable or even desired by "style
> > guide" to say:
> >   if ((ret = update_fontsize(ctx))) {
> > (to clarify it's a check of an assignment) - this is what Brett used,
>
> Ah. Parentheses over the whole expression are always optional, but in this
> particular case, there is a good reason:
>
> <stdin>:4:1: warning: suggest parentheses around assignment used as truth
> value [-Wparentheses]
>
> Forgetting to double the equal in a comparison is a common mistake,
> compilers detect it. But then you need a way of stating when it is
> intentional.
>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-added-expr-evaluation-to-drawtext-fontsize.patch
Type: application/octet-stream
Size: 9022 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160902/bde8c811/attachment.obj>