[FFmpeg-devel] [PATCH] libavfilter: created a new filter that obtains the average peak signal-to-noise ratio (PSNR) of two input video files in YUV format.

Stefano Sabatini stefano.sabatini-lala at poste.it
Mon Jul 4 19:22:26 CEST 2011


On date Thursday 2011-06-23 14:57:59 +0200, Stefano Sabatini encoded:
> On date Thursday 2011-06-23 14:07:51 +0200, Roger Pau Monné encoded:
> > 2011/6/23 Stefano Sabatini <stefano.sabatini-lala at poste.it>:
> > > On date Tuesday 2011-06-21 16:01:36 +0200, Stefano Sabatini encoded:
> > >> On date Tuesday 2011-06-21 14:44:50 +0200, Roger Pau Monné encoded:
> > >> > Hello,
> > >> >
> > >> > Sorry for the delay, I'm really busy these weeks, I will like to
> > >> > implement the option to store the results in a qpsnr style, but right
> > >> > now I don't have time, let's see if I can do it in a week or two. I've
> > >> > applied the suggested changes, thanks Stefano.
> > > [...]
> > >> > +static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref)
> > >> > +{
> > >> > +    AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0);
> > >> > +    AVFilterContext *ctx = inlink->dst;
> > >> > +    PSNRContext *psnr = ctx->priv;
> > >> > +
> > >> > +    inlink->dst->outputs[0]->out_buf = outpicref;
> > >
> > >> > +    outpicref->pts = av_rescale_q(inpicref->pts,
> > >> > +                                  ctx->inputs [0]->time_base,
> > >> > +                                  ctx->outputs[0]->time_base);
> > >
> > > Is this required?
> > 
> > If I recall correctly, if this was not added, the streams
> > desynchronized, and you ended up comparing frames from different
> > positions. Anyway, I will try removing it, to see what happens (maybe
> > the desynchronization problem didn't have anything to do with this).
> 
> From what I can see, the PTS information is never read. Every time you
> get a frame from the main source, avfilter_request_frame() is called
> on the reference source, and the pushed frame is cached in psnr->picref.
> 
> This suppose that we have a frame-by-frame correspondence between the
> main and the reference sources, which is a reasonable assumption.
> 
> > >
> > >> > +
> > >> > +    if (psnr->picref) {
> > >> > +        avfilter_unref_buffer(psnr->picref);
> > >> > +        psnr->picref = NULL;
> > >> > +    }
> > >
> > >> > +    avfilter_request_frame(ctx->inputs[1]);
> > >> > +
> > >> > +    avfilter_start_frame(inlink->dst->outputs[0], outpicref);
> > >
> > > Note: here we should fail in case avfilter_request_frame returns an
> > > error. Not necessary for this patch since this would possibly require
> > > to change the start_frame signature, but at least we may add a fixme.
> > 
> > I will look into it.
> > >
> > >> > +}
> > >> > +
> > >> > +static void start_frame_ref(AVFilterLink *inlink, AVFilterBufferRef *inpicref)
> > >> > +{
> > >> > +    AVFilterContext *ctx = inlink->dst;
> > >> > +    PSNRContext *psnr = ctx->priv;
> > >> > +
> > >> > +    psnr->picref = inpicref;
> > >
> > >> > +    psnr->picref->pts = av_rescale_q(inpicref->pts,
> > >> > +                                     ctx->inputs [1]->time_base,
> > >> > +                                     ctx->outputs[0]->time_base);
> > >
> > > Is this required? AFAIK the reference PTS is just ignored.
> > >
> > > [...]
> > >> (or we could change avfilter_unref_buffer to accepts a ** and set the
> > >> pointer reference to NULL).
> > >>
> > >> Looks fine otherwise. I'll fix the nits myself if you don't have time
> > >> or you don't care, as for the syntax thing I'd like to think a bit
> > >> about it and get advices.
> > >
> > > I addressed some of the nits and did some more changes to the filter,
> > > please keep working on this copy.
> [...] 
> > We should agree on what output format to use, how the CVS look like:
> > (sorry for the bad formatting)
> > 
> > # Frame PSNR : Y(dB)  Cb(dB) Cr(dB) Frame Average(dB)
> > 0                        60        49       30     55
> > 
> > Use Tabs (\t) or ',' between numbers? Option to choose between 'plain'
> > or 'cvs'? How should we call it?
> 
> We already have the stats_file/f option.
> 
> We may add a stats_file_format/fmt/sff option to specify the format,
> which could be choosen between:
> plain,csv,qpsnr,xml,json,native
> 
> but then I have no preference for which formats to support.

Updated work in progress, I changed the output format, and added more
information in the log.

As for what regards the format: after some thought, I concluded that
it's fine to support only one formats, format conversion can be done
via scripting rather than in C.

Todo: maybe the final stats should be printed on the log file (and
print by default with av_log() when no output file is specified), the
docs should define clearly what's the content of the final line.
-- 
FFmpeg = Freak Foolish Most Portable Erratic Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-add-psnr-filter.patch
Type: text/x-diff
Size: 17340 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110704/25fb4db9/attachment.bin>


More information about the ffmpeg-devel mailing list