[FFmpeg-devel] [PATCH 2/2] lavfi: add amerge audio filter.

Stefano Sabatini stefasab at gmail.com
Wed Dec 28 17:09:49 CET 2011


On date Saturday 2011-12-24 17:57:53 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  Changelog                |    1 +
>  doc/filters.texi         |   30 +++++
>  libavfilter/Makefile     |    1 +
>  libavfilter/af_amerge.c  |  273 ++++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/allfilters.c |    1 +
>  5 files changed, 306 insertions(+), 0 deletions(-)
>  create mode 100644 libavfilter/af_amerge.c
> 
> diff --git a/Changelog b/Changelog
> index 7044b4c..430048e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -8,6 +8,7 @@ version next:
>  - OpenMG Audio muxer
>  - SMJPEG demuxer
>  - astreamsync audio filter
> +- amerge audio filter
>  
>  
>  version 0.9:
> diff --git a/doc/filters.texi b/doc/filters.texi
> index ce4a74f..77e9c50 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -156,6 +156,36 @@ aformat=u8\\,s16:mono:packed
>  aformat=s16:mono\\,stereo:all
>  @end example
>  
> + at section amerge
> +
> +Merge two audio streams into a single multi-channel stream.
> +
> +This filter does not need any argument.
> +
> +If the channel layouts of the inputs are disjoint, and therefore compatible,
> +the channel layout of the output will be set accordingly and the channels
> +will be reordered as necessary. If the channel layouts of the inputs are not
> +disjoint, the output will have all the channels of the first input then all
> +the channels of the second input, in that order, and the channel layout of
> +the output will be the default value corresponding to the total number of
> +channels.
> +
> +For example, if the first input is in 2.1 (FL+FR+LF) and the second input
> +is FC+BL+BR, then the output will be in 5.1, with the channels in the
> +following order: a1, a2, b1, a3, b2, b3 (a1 is the first channel of the
> +first input, b1 is the first channel of the second input).
> +
> +On the other hand, if both input are in stereo, the output channels will be
> +in the default order: a1, a2, b1, b2, and the channel layout will be
> +arbitrarily set to 4.0, which may or may not be the expected value.
> +
> +Both inputs must have the same sample rate, format and packing.
> +
> +Example: merge two mono files into a stereo stream:
> + at example
> +amovie=left.wav [l] ; amovie=right.mp3 [r] ; [l] [r] amerge
> + at end example
> +
>  @section anull
>  
>  Pass the audio source unchanged to the output.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 555767b..445daa1 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -26,6 +26,7 @@ OBJS-$(CONFIG_AVCODEC)                       += avcodec.o
>  
>  OBJS-$(CONFIG_ACONVERT_FILTER)               += af_aconvert.o
>  OBJS-$(CONFIG_AFORMAT_FILTER)                += af_aformat.o
> +OBJS-$(CONFIG_AMERGE_FILTER)                 += af_amerge.o
>  OBJS-$(CONFIG_ANULL_FILTER)                  += af_anull.o
>  OBJS-$(CONFIG_ARESAMPLE_FILTER)              += af_aresample.o
>  OBJS-$(CONFIG_ASHOWINFO_FILTER)              += af_ashowinfo.o
> diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c
> new file mode 100644
> index 0000000..fa86d26
> --- /dev/null
> +++ b/libavfilter/af_amerge.c
> @@ -0,0 +1,273 @@
> +/*
> + * Copyright (c) 2011 Nicolas George <nicolas.george at normalesup.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Audio merging filter
> + */
> +

> +#include <stdlib.h>
> +#include "libavcodec/avcodec.h"

unnecessary?

> +#include "libavutil/avstring.h"
> +#include "libswresample/swresample.h" // only for SWR_CH_MAX
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +#define QUEUE_SIZE 16
> +
> +struct amerge_context {

> +    int route[SWR_CH_MAX];
> +    int nb_in_ch[2];

these may benefit from a doxy

> +    int bps;
> +    struct amerge_queue {
> +        AVFilterBufferRef *buf[QUEUE_SIZE];
> +        int nb_buf, nb_samples, pos;
> +    } queue[2];
> +};

consistency nit: struct amerge_context -> AMergeContext

> +
> +static av_cold int init(AVFilterContext *ctx, const char *args0, void *opaque)
> +{
> +    return 0;
> +}

unnecessary?

> +
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    struct amerge_context *am = ctx->priv;
> +    int i, j;
> +
> +    for (i = 0; i < 2; i++)
> +        for (j = 0; j < am->queue[i].nb_buf; j++)
> +            avfilter_unref_buffer(am->queue[i].buf[j]);
> +}

This reminds me that we still don't properly support flush.

> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    struct amerge_context *am = ctx->priv;
> +    int64_t inlayout[2], outlayout;
> +    const int packing_fmts[] = { AVFILTER_PACKED, -1 };
> +    AVFilterFormats *formats;
> +    int i;
> +
> +    for (i = 0; i < 2; i++) {
> +        if (!ctx->inputs[i]->in_chlayouts ||
> +            !ctx->inputs[i]->in_chlayouts->format_count) {
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "No channel layout for input %d\n", i + 1);
> +            return AVERROR(EINVAL);
> +        }
> +        inlayout[i] = ctx->inputs[i]->in_chlayouts->formats[0];
> +        if (ctx->inputs[i]->in_chlayouts->format_count > 1) {
> +            char buf[256];
> +            av_get_channel_layout_string(buf, sizeof(buf), 0, inlayout[i]);
> +            av_log(ctx, AV_LOG_INFO, "Using \"%s\" for input %d\n", buf, i + 1);
> +        }
> +        am->nb_in_ch[i] = av_get_channel_layout_nb_channels(inlayout[i]);
> +    }

> +    if (am->nb_in_ch[0] + am->nb_in_ch[1] > SWR_CH_MAX) {
> +        av_log(ctx, AV_LOG_ERROR, "Too many channels\n");
> +        return AVERROR(EINVAL);
> +    }

maybe mention the value of SWR_CH_MAX

> +    if (inlayout[0] & inlayout[1]) {
> +        av_log(ctx, AV_LOG_WARNING,
> +               "Inputs overlap: output layout will be meaningless\n");
> +        for (i = 0; i < am->nb_in_ch[0] + am->nb_in_ch[1]; i++)
> +            am->route[i] = i;
> +        outlayout = av_get_default_channel_layout(am->nb_in_ch[0] +
> +                                                  am->nb_in_ch[1]);
> +        if (!outlayout)
> +            outlayout = ((int64_t)1 << (am->nb_in_ch[0] + am->nb_in_ch[1])) - 1;
> +    } else {
> +        int *route[2] = { am->route, am->route + am->nb_in_ch[0] };

> +        int c, out_no = 0;

nit: out_no is confusing, indeed I can't even fid a better name, maybe you can

> +
> +        outlayout = inlayout[0] | inlayout[1];
> +        for (c = 0; c < 64; c++)
> +            for (i = 0; i < 2; i++)
> +                if ((inlayout[i] >> c) & 1)
> +                    *(route[i]++) = out_no++;
> +    }
> +    formats = avfilter_make_all_formats(AVMEDIA_TYPE_AUDIO);
> +    avfilter_set_common_sample_formats(ctx, formats);
> +    formats = avfilter_make_format_list(packing_fmts);
> +    avfilter_set_common_packing_formats(ctx, formats);
> +    for (i = 0; i < 2; i++) {
> +        formats = NULL;
> +        avfilter_add_format(&formats, inlayout[i]);
> +        avfilter_formats_ref(formats, &ctx->inputs[i]->out_chlayouts);
> +    }
> +    formats = NULL;
> +    avfilter_add_format(&formats, outlayout);
> +    avfilter_formats_ref(formats, &ctx->outputs[0]->in_chlayouts);
> +    return 0;
> +}
> +
> +static int config_output(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    struct amerge_context *am = ctx->priv;
> +
> +    if (ctx->inputs[0]->sample_rate != ctx->inputs[1]->sample_rate) {
> +        av_log(ctx, AV_LOG_ERROR,
> +               "Inputs must have the same sample rate "
> +               "(%"PRIi64" vs %"PRIi64")\n",
> +               ctx->inputs[0]->sample_rate, ctx->inputs[1]->sample_rate);
> +        return AVERROR(EINVAL);
> +    }
> +    am->bps = av_get_bytes_per_sample(ctx->outputs[0]->format);
> +    outlink->sample_rate = ctx->inputs[0]->sample_rate;
> +    outlink->time_base   = ctx->inputs[0]->time_base;

here it will be extremely helpful an INFO log of the kind:
in_chl1:%s in_chl2:%s -> out_chl:%s

> +    return 0;
> +}
> +
> +static int request_frame(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    struct amerge_context *am = ctx->priv;
> +    int i;
> +
> +    for (i = 0; i < 2; i++)
> +        if (!am->queue[i].nb_samples)
> +            avfilter_request_frame(ctx->inputs[i]);

What when one stream ends? I see two possible policies, the filter may
continue to stream and fill with zeros the finished channels, or
return EOF when the first one finishes (configurable?).

> +    return 0;
> +}
> +

> +static inline void copy_samples(int nb_in_ch[2], int *route0, uint8_t *ins[2],
> +                                uint8_t **outs, int ns, int bps)

please add some doxy

>From my understanding:

/**
 * Copy samples from the two input channels in ins[2] to outs.
 * Samples data is assumed to be in packed format.
 *
 * @param nb_in_ch the number of channels in the two input buffers.
 * @param route0 int arrays which provides a map from the input to the
 *               output channel, has the form:
 *               [outch1_1 outch1_2 ... outch1_N1 outch2_1 ... outch2_N2]
 * @param ns number of samples per channel to copy to output
 */
static inline void copy_samples(int nb_in_ch[2], int *route0, uint8_t *ins[2],
                                uint8_t **outs, int ns, int bps)

> +{
> +    int *route;
> +    int i, c;
> +
> +    while (ns--) {
> +        route = route0;
> +        for (i = 0; i < 2; i++) {
> +            for (c = 0; c < nb_in_ch[i]; c++) {
> +                memcpy((*outs) + bps * *(route++), ins[i], bps);
> +                ins[i] += bps;
> +            }
> +        }
> +        *outs += (nb_in_ch[0] + nb_in_ch[1]) * bps;
> +    }
> +}
> +
> +static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef *insamples)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    struct amerge_context *am = ctx->priv;

> +    int in_no = inlink == ctx->inputs[1];

suggestion: inlink_id (more readable to me)

> +    struct amerge_queue *iq = &am->queue[in_no];
> +    int nb_samples, ns, i;
> +    AVFilterBufferRef *outbuf, **ibuf[2];

ibuf -> inbuf?

> +    uint8_t *ins[2], *outs;
> +
> +    if (iq->nb_buf == QUEUE_SIZE) {
> +        av_log(ctx, AV_LOG_ERROR, "Packet queue overflow; dropped\n");
> +        avfilter_unref_buffer(insamples);
> +        return;
> +    }

> +    iq->buf[iq->nb_buf++] = avfilter_ref_buffer(insamples, AV_PERM_READ |
> +                                                           AV_PERM_PRESERVE);

Is AV_PERM_PRESERVE really necessary?

> +    iq->nb_samples += insamples->audio->nb_samples;
> +    avfilter_unref_buffer(insamples);
> +    if (!am->queue[!in_no].nb_samples)
> +        return;
> +
> +    nb_samples = FFMIN(am->queue[0].nb_samples,
> +                       am->queue[1].nb_samples);
> +    outbuf = avfilter_get_audio_buffer(ctx->outputs[0], AV_PERM_WRITE,
> +                                       nb_samples);
> +    outs = outbuf->data[0];
> +    for (i = 0; i < 2; i++) {
> +        ibuf[i] = am->queue[i].buf;
> +        ins[i] = (*ibuf[i])->data[0] +
> +                 am->queue[i].pos * am->nb_in_ch[i] * am->bps;
> +    }
> +    while (nb_samples) {
> +        ns = nb_samples;
> +        for (i = 0; i < 2; i++)
> +            ns = FFMIN(ns, (*ibuf[i])->audio->nb_samples - am->queue[i].pos);

> +        /* Unroll the most common sample formats: speed +~350% for the loop,
> +           +~13% overall (including two common decoders) */

That seems weird to me, but I'm possibly naive about what the compiler
is expected to do, I'd expect that a const int bps in copy_samples()
would have the same effect...

> +        switch (am->bps) {
> +            case 1:
> +                copy_samples(am->nb_in_ch, am->route, ins, &outs, ns, 1);
> +                break;
> +            case 2:
> +                copy_samples(am->nb_in_ch, am->route, ins, &outs, ns, 2);
> +                break;
> +            case 4:
> +                copy_samples(am->nb_in_ch, am->route, ins, &outs, ns, 4);
> +                break;
> +            default:
> +                copy_samples(am->nb_in_ch, am->route, ins, &outs, ns, am->bps);
> +                break;
> +        }
> +
> +        nb_samples -= ns;
> +        for (i = 0; i < 2; i++) {
> +            am->queue[i].nb_samples -= ns;
> +            am->queue[i].pos += ns;
> +            if (am->queue[i].pos == (*ibuf[i])->audio->nb_samples) {
> +                am->queue[i].pos = 0;
> +                avfilter_unref_buffer(*ibuf[i]);
> +                *ibuf[i] = NULL;
> +                ibuf[i]++;
> +                ins[i] = *ibuf[i] ? (*ibuf[i])->data[0] : NULL;
> +            }
> +        }
> +    }
> +    for (i = 0; i < 2; i++) {
> +        int nbufused = ibuf[i] - am->queue[i].buf;
> +        if (nbufused) {
> +            am->queue[i].nb_buf -= nbufused;
> +            memmove(am->queue[i].buf, ibuf[i],
> +                    am->queue[i].nb_buf * sizeof(**ibuf));
> +        }
> +    }
> +    avfilter_filter_samples(ctx->outputs[0], outbuf);
> +}
> +
> +AVFilter avfilter_af_amerge = {
> +    .name          = "amerge",

> +    .description   = NULL_IF_CONFIG_SMALL("Merge two audio streams into "
> +                                          "a single multi-channel stream"),

nit+: missing final dot

And thanks for taking care to write this very important filter.
-- 
FFmpeg = Fostering and Fanciful Magic Problematic Ecumenical God


More information about the ffmpeg-devel mailing list