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

[Paul B Mahol]

On 10/11/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> 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.

I can change stereo3d filter to ignore framerate, iirc i already did this
but I did not get expected order of frames from interleave filter.

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

You always mentions some 'design issues which can't be really fixed'
but never tried to explain them or fix them or open bug report or etc....

Do you want to confirm that current state of filter (or lavfi in general)
is useless, as you have no control which frame from which input goes to output?

> --
> FFmpeg = Formidable and Faithful Mastodontic Programmable Everlasting
> Gospel
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>