On 17/01/2023 14:45, Paul B Mahol wrote: > 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.
> static int process_command(AVFilterContext *ctx, const char *cmd, const char > *args, > char *res, int res_len, int flags) > { > CropDetectContext *s = ctx->priv; > int ret; > > + if (s->limit_user == s->limit) > + return AVERROR(ENOSYS); > + > if ((ret = ff_filter_process_command(ctx, cmd, args, res, res_len, > flags)) < 0) > return ret; > > if (s->limit_user != s->limit) { > s->limit_user = s->limit; > if (s->limit < 1.0) > s->limit *= (1 << s->bitdepth) - 1; > s->frame_nb = s->reset_count; > } > > return 0; > } I did not check if the limit was identical to the old one before ff_filter_process_command() set it, and it wasn't upscale for bitdepth in that case, so we avoid touching limit all together now. Is this correct? _______________________________________________ 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".