[FFmpeg-devel] [PATCH v2] drawtext: add fontcolor_expr

[Andrey Utkin]

2014-07-03 20:59 GMT+03:00 Stefano Sabatini <stefasab at gmail.com>:
> On date Thursday 2014-07-03 16:36:12 +0300, Andrey Utkin encoded:
>> + at item fontcolor_expr
>> +String which is expanded the same way as @var{text} to obtain dynamic
>> + at var{fontcolor} value. By default this option has empty value and is not
>> +processed.
>
> It's not very clear the interaction with fontcolor.

When fontcolor_expr is set, it overrides fontcolor option.
I can add this phrase into doc.

> Alternatively, what about something like fontcolor_r/g/b/a_expr with
> RGBA values expressed as floats in the 0-1 range?
>
> This should be easier on the user and probably more robust.

I'm not absolutely convinced about that, although if i haven't already
done that, maybe i could agree and do it this way.
Currently fontcolor_expr evaluates to the same value that "fontcolor"
gets, and adding per-component options would actually add complexity.
Also if later we add same functions for box, border and shadow, that
will be a big number of these options.
Also the way it is implemented now, fontcolor_expr can evaluate not
only to something like "ff0000ff", but also to something like
"red at 1.0", which fontcolor also percepts. With this second form, you
just want to use expr() and not eif() for dynamic opacity.

>> + at item eif
>> +Evaluate the expression's value and output as formatted integer.
>> +
>> +First argument is expression to be evaluated, same as for @var{expr} function.
>> +Second argument specifies output format. Allowed values are 'x', 'X', 'd' and
>> +'u', they are treated exactly as in printf function.
>> +Third parameter is optional and sets the number of positions taken by output.
>> +Effectively this allows to add padding with zeros from the left.
>
> Not a strong objection, but probably better sent as a separate patch
> (IIRC this should also fix an open ticket).

Ok, will update.

>> +#!/bin/bash
>
> #!/bin/sh is preferred for compatibility reasons.

Ok, will update.

-- 
Andrey Utkin