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

[Roger Pau Monné]

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

>
>> > +
>> > +    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.
> --
> FFmpeg = Freak Funny Mastodontic Plastic Exxagerate Geisha
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>

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?