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

[Limin Wang]

On Sat, Oct 12, 2019 at 11:41:53PM +0200, Michael Niedermayer wrote:
> On Sat, Oct 12, 2019 at 09:39:14AM +0800, Limin Wang wrote:
> > On Fri, Oct 11, 2019 at 08:13:02PM +0200, Michael Niedermayer wrote:
> > > On Thu, Oct 10, 2019 at 10:11:10PM +0800, Limin Wang wrote:
> > > > On Thu, Oct 10, 2019 at 01:52:56PM +0200, Michael Niedermayer wrote:
> > > > > On Tue, Oct 08, 2019 at 11:28:37PM +0800, lance.lmwang at gmail.com wrote:
> > > > > > From: Limin Wang <lance.lmwang at gmail.com>
> > > > > > 
> > > > > > Reviewed-by: Paul B Mahol <onemda at gmail.com>
> > > > > > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > > > > ---
> > > > > >  libavfilter/vf_framerate.c | 15 +++++++++++----
> > > > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c
> > > > > > index 6c8d01c..8d16998 100644
> > > > > > --- a/libavfilter/vf_framerate.c
> > > > > > +++ b/libavfilter/vf_framerate.c
> > > > > > @@ -71,13 +71,20 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next
> > > > > >  
> > > > > >      if (crnt->height == next->height &&
> > > > > >          crnt->width  == next->width) {
> > > > > > +        AVDictionaryEntry *e_mafd = NULL;
> > > > > >          uint64_t sad;
> > > > > > -        double mafd, diff;
> > > > > > +        double mafd = HUGE_VAL, diff;
> > > > > > +        char *tail = NULL;
> > > > > >  
> > > > > >          ff_dlog(ctx, "get_scene_score() process\n");
> > > > > > -        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);
> > > > > > +        e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL, AV_DICT_MATCH_CASE);
> > > > > > +        if (e_mafd)
> > > > > > +            mafd = strtod(e_mafd->value, &tail);
> > > > > > +        if (*tail || mafd == HUGE_VAL) {
> > > > > > +            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);
> > > > > > +        }
> > > > > 
> > > > > while this is not introduced by the patch, i would suggest to seperate
> > > > > emms_c() from functions using floating point, This combination does have
> > > > > some potential for emms and the compilers optimizer hitting each other
> > > > 
> > > > Michael, I haven't clear about what to seperate the emms_c? any reference usage in 
> > > > existing codebase? I can help to submit patch to fix it, there're at least five filters
> > > > are using the same way.
> > > 
> > > emms clears the float/MMX state of the cpu
> > > if the compiler uses the FPU at the same time and is unaware of that
> > > clearing it could result in problems.
> > > so i just always tried to keep emms away from floating point usage to minimize 
> > > the chances of this.
> > > 
> > > also it was rather slow on some CPUs to mix mmx and float code 
> > 
> > OK, how about to scale mafd with 1000000 to avoid the float point use? 
> 
> For the use case of whole frame compare / scene detection, i dont see why
> the function leaves the FPU state in need of a seperate EMMS
> If this is never used with finer granularity then its probably best to
> move the emms into the x86 specific code. 

For I'm not familar with this related assemable code, maybe other expert developer 
can help change it later.

> 
> Thanks
>  
> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Observe your enemies, for they first find out your faults. -- Antisthenes



> _______________________________________________
> 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".