[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. 

Michael, please give comments whether the sct filter can be merged. Your comment of the 
emms has exist in the current codebase already and not related to my patchset.


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