[FFmpeg-devel] [PATCH] avfilter/vf_cropdetect: add ability to change limit/reset at runtime

[Paul B Mahol]

On 1/17/23, Jeffrey Chapuis <ashyni1987 at gmail.com> wrote:
>>Le 10/01/2023 à 16:45, Paul B Mahol a écrit :
>>> On 1/10/23, Jeffrey CHAPUIS <ashyni1987 at gmail.com> wrote:
>>>> Hello,
>>>> I decided to continue on a simpler path without 'reset/reset_count', it
>>>> was
>>>> only to experiment anyway, 'limit' is the main goal.
>>>> 'limit' is added to the metadata to control that the result is
>>>> associated to
>>>> a change at runtime, it's after scaling with bitdetph but that's not
>>>> really
>>>> a problem (at least for me, we can always store the parameter before
>>>> any
>>>> alteration).
>>>>
>>>>> +    if (!strcmp(cmd, "limit")) {
>>>>> +        if (s->limit < 1.0)
>>>>> +            s->limit *= (1 << s->bitdepth) - 1;
>>>>> +        s->frame_nb = s->reset_count;
>>>>> +    }
>>>> Should i remove the if statement here ? or keep it for future change
>>>> eventually.
>>>
>>> Split variables, keep one variable settable by user and unchanged by
>>> filter.
>>>
>>>>
>>>> Notes I didn't think about?
>>>>
>>>> Thunderbird altered the patch somehow (remove empty new lines), it's
>>>> edited
>>>> manually.
>>>
>>> Attach patch instead.
>>>
>>>
>>> Avoid using strcmp to check for this variable change, instead check
>>> with previous and new value in process function.
>>
>>>Here is part of the updated patch, 'limit' exposed in metadata/log is now
>>> coherent with init().
>>>Like 'limit/limit_user' is of type float, i've used what's done in
>>> av_dict_set_int() to print it as float.
>>>Compare 's->limit_user' and 's->limit' to check for a change instead of
>>> 'strcmp'.
>>>Is there anything to adjust ?
>>
>>Forgot to update ref file for fate (full patch attached).
>
> Is the update code good?
>
>> +    char limit_str[22];
>> +        snprintf(limit_str, sizeof(limit_str), "%f", s->limit_user);
>> +        av_dict_set(metadata, "lavfi.cropdetect.limit", limit_str, 0);
>
> Should i create a function av_dict_set_float() in libavutil/dict.c and
> libavutil/dict.h?

Nope.

Shouldnt limit variable be changed if < 1.0 before being used in
process_command()  ?