[FFmpeg-devel] [PATCH 2/2] avfilter/f_interleave: reimplement on top of framesync

[Stefano Sabatini]

On date Wednesday 2013-10-09 18:23:19 +0000, Paul B Mahol encoded:
> On 10/9/13, Nicolas George <george at nsup.org> wrote:
> > Le quartidi 14 vendemiaire, an CCXXII, Paul B Mahol a ecrit :
> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> ---
> >>  libavfilter/Makefile       |   4 +-
> >>  libavfilter/f_interleave.c | 109
> >> +++++++++++++++++----------------------------
> >>  2 files changed, 44 insertions(+), 69 deletions(-)
> >>
> >> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> >> index b2d3587..5a84c81 100644
> >> --- a/libavfilter/Makefile
> >> +++ b/libavfilter/Makefile
> >> @@ -56,7 +56,7 @@ OBJS-$(CONFIG_ADELAY_FILTER)                 +=
> >> af_adelay.o
> >>  OBJS-$(CONFIG_AECHO_FILTER)                  += af_aecho.o
> >>  OBJS-$(CONFIG_AFADE_FILTER)                  += af_afade.o
> >>  OBJS-$(CONFIG_AFORMAT_FILTER)                += af_aformat.o
> >> -OBJS-$(CONFIG_AINTERLEAVE_FILTER)            += f_interleave.o
> >> +OBJS-$(CONFIG_AINTERLEAVE_FILTER)            += f_interleave.o
> >> framesync.o
> >>  OBJS-$(CONFIG_ALLPASS_FILTER)                += af_biquads.o
> >>  OBJS-$(CONFIG_AMERGE_FILTER)                 += af_amerge.o
> >>  OBJS-$(CONFIG_AMIX_FILTER)                   += af_amix.o
> >> @@ -150,7 +150,7 @@ OBJS-$(CONFIG_HUE_FILTER)                    +=
> >> vf_hue.o
> >>  OBJS-$(CONFIG_IDET_FILTER)                   += vf_idet.o
> >>  OBJS-$(CONFIG_IL_FILTER)                     += vf_il.o
> >>  OBJS-$(CONFIG_INTERLACE_FILTER)              += vf_interlace.o
> >> -OBJS-$(CONFIG_INTERLEAVE_FILTER)             += f_interleave.o
> >> +OBJS-$(CONFIG_INTERLEAVE_FILTER)             += f_interleave.o
> >> framesync.o
> >>  OBJS-$(CONFIG_KERNDEINT_FILTER)              += vf_kerndeint.o
> >>  OBJS-$(CONFIG_LUT3D_FILTER)                  += vf_lut3d.o
> >>  OBJS-$(CONFIG_LUT_FILTER)                    += vf_lut.o
> >> diff --git a/libavfilter/f_interleave.c b/libavfilter/f_interleave.c
> >> index 72050db..a107802 100644
> >> --- a/libavfilter/f_interleave.c
> >> +++ b/libavfilter/f_interleave.c
> >> @@ -27,8 +27,8 @@
> >>  #include "libavutil/avstring.h"
> >>  #include "libavutil/opt.h"
> >>  #include "avfilter.h"
> >> -#include "bufferqueue.h"
> >>  #include "formats.h"
> >> +#include "framesync.h"
> >>  #include "internal.h"
> >>  #include "audio.h"
> >>  #include "video.h"
> >> @@ -36,7 +36,8 @@
> >>  typedef struct {
> >>      const AVClass *class;
> >>      int nb_inputs;
> >> -    struct FFBufQueue *queues;
> >> +
> >> +    FFFrameSync fs;
> >>  } InterleaveContext;
> >>
> >>  #define OFFSET(x) offsetof(InterleaveContext, x)
> >> @@ -48,58 +49,30 @@ static const AVOption filt_name##_options[] = {
> >>              \
> >>     { NULL }                                                         \
> >>  }
> >>
> >> -inline static int push_frame(AVFilterContext *ctx)
> >
> >> +inline static int push_frame(FFFrameSync *fs)
> >
> > I suspect the "inline" should go away.
> >
> >>  {
> >> +    AVFilterContext *ctx = fs->parent;
> >> +    AVFilterLink *outlink = ctx->outputs[0];
> >>      InterleaveContext *s = ctx->priv;
> >>      AVFrame *frame;
> >> -    int i, queue_idx = -1;
> >> -    int64_t pts_min = INT64_MAX;
> >> +    int i, ret;
> >>
> >> -    /* look for oldest frame */
> >> -    for (i = 0; i < ctx->nb_inputs; i++) {
> >> -        struct FFBufQueue *q = &s->queues[i];
> >> -
> >> -        if (!q->available && !ctx->inputs[i]->closed)
> >> -            return 0;
> >> -        if (q->available) {
> >> -            frame = ff_bufqueue_peek(q, 0);
> >> -            if (frame->pts < pts_min) {
> >> -                pts_min = frame->pts;
> >> -                queue_idx = i;
> >> -            }
> >> -        }
> >> -    }
> >
> >> +    for (i = 0; i < s->nb_inputs; i++) {
> >> +        if ((ret = ff_framesync_get_frame(fs, i, &frame, 1)) < 0)
> >> +            return ret;
> >>
> >> -    /* all inputs are closed */
> >> -    if (queue_idx < 0)
> >> -        return AVERROR_EOF;
> >> +        frame->pts = av_rescale_q(fs->pts, fs->time_base,
> >> outlink->time_base);
> >> +        if ((ret = ff_filter_frame(outlink, frame)) < 0)
> >> +            return ret;
> >> +    }
> >

> > I wonder about the logic here: it seems to me you my send each frame
> > multiple times for each sync event. Did you test this with inputs at
> > different frame rates?
> 

> I can not use current state of filter to do anything useful. So it
> is really hard to find if something is broken.

Please elaborate on this. The issue with stereo3d looks to me like a
stereo3d issue (assuming every input has a defined framerate,
misbehaving otherwise), or ffmpeg not liking multiple frames with the
same timestamps.

The other limitations are due to buffering problems, there is IMHO no
way to workaround them without duplicating/dropping frames.

So I still have no evidence that there is something broken in
interleave (apart design issues which can't be really fixed), and I'm
not sure what this patch is about.
-- 
FFmpeg = Formidable and Faithful Mastodontic Programmable Everlasting Gospel