[FFmpeg-devel] [PATCH] vf_overlay: add logic for avoiding overlaying frames with PTS > main frame PTS

[Stefano Sabatini]

On date Wednesday 2012-02-01 00:43:01 +0000, Paul B Mahol encoded:
> On 1/10/12, Stefano Sabatini <stefasab at gmail.com> wrote:
> > On date Tuesday 2012-01-10 00:30:12 +0000, Paul B Mahol encoded:
> >> On 1/10/12, Stefano Sabatini <stefasab at gmail.com> wrote:
> >> > Also add debug logging messages for helping tracking down similar
> >> > issues.
> >> >
> >> > Fix trac ticket #467.
> >> > ---
> >> >  libavfilter/vf_overlay.c |   41
> >> > +++++++++++++++++++++++++++++------------
> >> >  1 files changed, 29 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
> >> > index 062783e..9600061 100644
> >> > --- a/libavfilter/vf_overlay.c
> >> > +++ b/libavfilter/vf_overlay.c
> >> > @@ -25,6 +25,8 @@
> >> >   * overlay one video on top of another
> >> >   */
> >> >
> >> > +/* #define DEBUG */
> >>
> >> Is this really needed?
> >
> > It's handy for me since I need just a few keystrokes for
> > enabling/disabling it. Also it shows that there is debug code in the
> > file.
> >
> >> > +
> >> >  #include "avfilter.h"
> >> >  #include "libavutil/eval.h"
> >> >  #include "libavutil/avstring.h"
> >> > @@ -75,7 +77,7 @@ typedef struct {
> >> >      uint8_t overlay_rgba_map[4];
> >> >      uint8_t overlay_has_alpha;
> >> >
> >> > -    AVFilterBufferRef *overpicref;
> >> > +    AVFilterBufferRef *overpicref, *overpicref_next;
> >> >
> >> >      int main_pix_step[4];       ///< steps per pixel for each plane of
> >> > the
> >> > main output
> >> >      int overlay_pix_step[4];    ///< steps per pixel for each plane of
> >> > the
> >> > overlay
> >> > @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx)
> >> >
> >> >      if (over->overpicref)
> >> >          avfilter_unref_buffer(over->overpicref);
> >> > +    if (over->overpicref_next)
> >> > +        avfilter_unref_buffer(over->overpicref_next);
> >> >  }
> >> >
> >> >  static int query_formats(AVFilterContext *ctx)
> >> > @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink,
> >> > AVFilterBufferRef *inpicref)
> >> >      AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0);
> >> >      AVFilterContext *ctx = inlink->dst;
> >> >      OverlayContext *over = ctx->priv;
> >> > +    av_unused AVFilterLink *outlink = ctx->outputs[0];
> >> >
> >> >      inlink->dst->outputs[0]->out_buf = outpicref;
> >> >      outpicref->pts = av_rescale_q(outpicref->pts,
> >> > ctx->inputs[MAIN]->time_base,
> >> >                                    ctx->outputs[0]->time_base);
> >> >
> >> >      if (!over->overpicref || over->overpicref->pts < outpicref->pts) {
> >> > -        AVFilterBufferRef *old = over->overpicref;
> >> > -        over->overpicref = NULL;
> >> > -        avfilter_request_frame(ctx->inputs[OVERLAY]);
> >> > -        if (over->overpicref) {
> >> > -            if (old)
> >> > -                avfilter_unref_buffer(old);
> >> > -        } else
> >> > -            over->overpicref = old;
> >> > +        if (!over->overpicref_next)
> >> > +            avfilter_request_frame(ctx->inputs[OVERLAY]);
> >> > +
> >> > +        if (over->overpicref && over->overpicref_next &&
> >> > +            over->overpicref_next->pts <= outpicref->pts) {
> >> > +            avfilter_unref_buffer(over->overpicref);
> >> > +            over->overpicref = over->overpicref_next;
> >> > +            over->overpicref_next = NULL;
> >> > +        }
> >> >      }
> >> >
> >> > +#ifdef DEBUG
> >>
> >> Use av_dlog() instead.
> >> > +    av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f",
> >> > +           outpicref->pts, outpicref ->pts *
> >> > av_q2d(outlink->time_base));
> >> > +    if (over->overpicref)
> >> > +        av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64"
> >> > over_pts_time:%f",
> >> > +               over->overpicref->pts, over->overpicref->pts *
> >> > av_q2d(outlink->time_base));
> >> > +    av_dlog(ctx, "\n");
> >> > +#endif
> >
> > Fixed.
> 
> Looks fine to me.

I was waiting for the timestamp utils patch, also I'm not sure this
change is correct in case of undefined (NOTPS) timestamps, so the
patch is temporarily suspended, thanks for the review anyway.
-- 
FFmpeg = Friendly and Fostering Mega Pitiful Ecstatic Guru