[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:
> On 17/01/2023 13:34, Paul B Mahol wrote:
>> On 1/17/23, Jeffrey Chapuis <ashyni1987 at gmail.com> wrote:
>>> 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() ?
>>
>> 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.
>
> I'm lost, limit_user already keep the user settings untouched before limit
> is modified if < 1.0

That is an issue, limit should not ever change except if user set it.

> Do I have to ask how you would code it?
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>