[FFmpeg-devel] [PATCH] avfilter: add metadata filters

[Tobias Rapp]

On 06.02.2016 23:30, Paul B Mahol wrote:
> On 2/6/16, Paul B Mahol <onemda at 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?

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.

Regards,
Tobias