On Fri, May 15, 2020 at 11:27:00AM +0200, Paul B Mahol wrote:
> On 5/15/20, lance.lmw...@gmail.com <lance.lmw...@gmail.com> wrote:
> > On Fri, May 15, 2020 at 10:27:35AM +0200, Paul B Mahol wrote:
> >> On 5/15/20, lance.lmw...@gmail.com <lance.lmw...@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@ffmpeg.org
> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >>
> >> >> To unsubscribe, visit link above, or email
> >> >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> >> >
> >> > --
> >> > Thanks,
> >> > Limin Wang
> >> > _______________________________________________
> >> > 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".
> >
> > --
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > 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".
--
Thanks,
Limin Wang
_______________________________________________
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".
