On 2/10/16, Tobias Rapp <t.r...@noa-archive.com> wrote:
> On 10.02.2016 10:01, Paul B Mahol wrote:
>> On 2/10/16, Tobias Rapp <t.r...@noa-archive.com> wrote:
>>> On 06.02.2016 23:30, Paul B Mahol wrote:
>>>> On 2/6/16, Paul B Mahol <one...@gmail.com> wrote:
>>>>> Hi,
>>>>>
>>>>> patch attached.
>>>>>
>>>>
>>>> Improved version attached.
>>>>
>>>> [...]
>>>> +
>>>> +static int string(const char *value1, const char *value2, size_t
>>>> length)
>>>> +{
>>>> + return !strncmp(value1, value2, length);
>>>> +}
>>>
>>> If I understand correctly this function is used to compare if the start
>>> of value2 matches value1. Maybe this function should be called
>>> "starts_with" (also in the "function" option enum)?
>>>
>>>> +
>>>> +static int equal(const char *value1, const char *value2, size_t length)
>>>> +{
>>>> + float f1, f2;
>>>> +
>>>> + if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
>>>> + return 0;
>>>> +
>>>> + return f1 != f2;
>>>> +}
>>>> +
>>>> +static int less(const char *value1, const char *value2, size_t length)
>>>> +{
>>>> + float f1, f2;
>>>> +
>>>> + if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
>>>> + return 0;
>>>> +
>>>> + return f1 > f2;
>>>> +}
>>>> +
>>>> +static int greater(const char *value1, const char *value2, size_t
>>>> length)
>>>> +{
>>>> + float f1, f2;
>>>> +
>>>> + if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
>>>> + return 0;
>>>> +
>>>> + return f1 < f2;
>>>> +}
>>>> +
>>>> [...]
>>>
>>> I think it would be better to not compare float values directly with
>>> "==", "<" or ">". Instead use some code like "fabsf(f1 - f2) <= epsilon".
>>>
>>> BTW: Is the return value of "equal", "less" and "greater" inverse on
>>> purpose?
>>
>> Not for equal, but for other it is.
>
> OK, now I got it that the argument order is flipped but matches the
> documentation. In that case it is a bit counter-intuitive that one has
> to do something like
>
> mode=print:key=mykey:value=1.0:function=less
>
> to print all metadata values of "mykey" which are *bigger* than 1.0.
>
Nope, this works fine here. It prints values less than 1.0
>>>
>>> Another sidenote: I have seen that some filters use a common expression
>>> language (e.g. aeval, crop, scale). Not sure if this expression language
>>> supports string operators or if it is limited to numbers but in my
>>> opinion it would be great to re-use it here.
>>
>> Added and applied. Filter is easily extendable.
>
> Thanks for adding expression support.
>
> I know my response has come late and I'm only an occasional FFmpeg
> developer so my thoughts might not have big weight. Still I think it
> would have been more polite to first respond on the list to all my
> comments (see the "starts_with" suggestion above) before applying the patch.
You can post another patch anytime here and if it makes sense I will
gladly apply it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
