On 1/17/23, Jeffrey Chapuis <ashyni1...@gmail.com> wrote: > On 17/01/2023 13:34, Paul B Mahol wrote: >> On 1/17/23, Jeffrey Chapuis <ashyni1...@gmail.com> wrote: >>> On 17/01/2023 12:52, Paul B Mahol wrote: >>>> On 1/17/23, Jeffrey Chapuis <ashyni1...@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".