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

[Paul B Mahol <onemda@gmail.com>]

On 1/17/23, Jeffrey Chapuis <ashyni1987@gmail.com> wrote:
> On 17/01/2023 12:52, Paul B Mahol wrote:
>> On 1/17/23, Jeffrey Chapuis <ashyni1987@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() ?

Inside that function.

> I thought ff_filter_process_command() was only checking the command flag and
> input value.

Call to ff_filter_process_command() does update to new values set by user.

So if limit is lower than 1.0 have special meaning it needs to be
handled properly.

The ideal solution is thus to keep user supplied value always constant
after its changed by user, and to do operations with it into new
variables.

>
> 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.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".