[FFmpeg-devel] [PATCH v6 2/4] avfilter/vf_framerate: if metadata lavfi.scd.mafd exists, we'll use it first

[lance.lmwang at gmail.com]

On Fri, May 15, 2020 at 11:27:00AM +0200, Paul B Mahol wrote:
> On 5/15/20, lance.lmwang at gmail.com <lance.lmwang at gmail.com> wrote:
> > On Fri, May 15, 2020 at 10:27:35AM +0200, Paul B Mahol wrote:
> >> On 5/15/20, lance.lmwang at gmail.com <lance.lmwang at gmail.com> 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
> >> >> 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".
> >> >
> >> > --
> >> > Thanks,
> >> > Limin Wang
> >> > _______________________________________________
> >> > 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".
> >> _______________________________________________
> >> 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".
> >
> > --
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > 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".
> _______________________________________________
> 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".

-- 
Thanks,
Limin Wang