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

[Jeffrey Chapuis]

On 17/01/2023 12:52, Paul B Mahol wrote:
> 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()  ?

You mean before ff_filter_process_command() ?
I thought ff_filter_process_command() was only checking the command flag and input value.

In that case, i need to store variable and restore them if ff_filter_process_command() fail ?


Sorry for all this stupid question, i'm not a dev, learning stuff on the way.