On Fri, May 15, 2020 at 11:27:00AM +0200, Paul B Mahol wrote: > On 5/15/20, [email protected] <[email protected]> wrote: > > On Fri, May 15, 2020 at 10:27:35AM +0200, Paul B Mahol wrote: > >> On 5/15/20, [email protected] <[email protected]> wrote: > >> > On Thu, May 14, 2020 at 09:00:21PM +0200, Marton Balint wrote: > >> >> > >> >> > >> >> On Thu, 14 May 2020, Marton Balint wrote: > >> >> > >> >> > > >> >> > > >> >> > On Thu, 14 May 2020, Nicolas George wrote: > >> >> > > >> >> > > Marton Balint (12020-05-14): > >> >> > > > I am not a huge fan of this patch, mafd refers to a score > >> >> > > > between this > >> >> > frame > >> >> > > > and the previous frame, we cannot ensure that there were no > >> >> > > > additional > >> >> > frame > >> >> > > > processing between scdet and this filter which may have > >> >> > > > duplicated > >> >> > > > or > >> >> > > > removed frames. So I'd rather not add this feature. > >> >> > > > >> >> > > It can only happen if the user has put filters in the middle. I > >> >> > > move > >> >> > > we > >> >> > > trust users who insert such obscure filters to do what they want > >> >> > > to, > >> >> > > or > >> >> > > to fix their issues if they have some. > >> >> > > >> >> > Fine, I am not blocking this if null pointer deref issues are fixed. > >> >> > > >> >> > I think we can also assume that if the metadata exists, it will > >> >> > contain > >> >> > a valid number, so I suggest this code: > >> >> > > >> >> > e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL, > >> >> > AV_DICT_MATCH_CASE); > >> >> > if (e_mafd) { > >> >> > mafd = strtod(e_mafd->value, NULL); > >> >> > } else { > >> >> > s->sad(crnt->data[0], crnt->linesize[0], next->data[0], > >> >> > next->linesize[0], crnt->width, crnt->height, &sad); > >> >> > emms_c(); > >> >> > mafd = (double)sad * 100.0 / (crnt->width * crnt->height) > >> >> > / > >> >> > (1 << s->bitdepth); > >> >> > } > >> >> > >> >> Why this patch was committed without a recent review??? > >> > > >> > Sorry, I consider it's reviewed old patchset, but it seems it's > >> > incomplete. > >> > >> Also it uses scd for metadata filter name, which is bad. It should use > >> full filter name. > > > > Sorry, in fact I intentionally did not use the filter name. At that time, I > > considered > > that there may be different filters for scene detection, but for filters > > that use the metadata, > > the processing method should be the same. So I didn't use the filter name. > > That is really bad explanation, Different filters writting to same > metadata entry is invalid.
Maybe my thoughts isn't complete as I assume user should choose one scene detect filter. One example is facedetect, now we have opencv facedetect, in future, we may add dnn facedetect and even more, for example, the drawbox care for the face detect result instead of which filter detect I think. That's my consideration. > > > > > > >> > >> > > >> >> > >> >> Thanks, > >> >> Marton > >> >> _______________________________________________ > >> >> ffmpeg-devel mailing list > >> >> [email protected] > >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> >> > >> >> To unsubscribe, visit link above, or email > >> >> [email protected] with subject "unsubscribe". > >> > > >> > -- > >> > Thanks, > >> > Limin Wang > >> > _______________________________________________ > >> > ffmpeg-devel mailing list > >> > [email protected] > >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > >> > To unsubscribe, visit link above, or email > >> > [email protected] with subject "unsubscribe". > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> [email protected] > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> [email protected] with subject "unsubscribe". > > > > -- > > Thanks, > > Limin Wang > > _______________________________________________ > > ffmpeg-devel mailing list > > [email protected] > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > [email protected] with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". -- Thanks, Limin Wang _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
