(replyall fail)

On Sat, Jun 9, 2012 at 12:14 AM, Alex Converse <[email protected]> wrote:
> On Fri, Jun 8, 2012 at 11:45 PM, Anton Khirnov <[email protected]> wrote:
>>
>>
>> On Fri,  8 Jun 2012 18:46:42 -0700, Alex Converse
>> <[email protected]> wrote:
>> > Inspired by MPlayer's af_channels filter and SoX's remix effect.
>> > ---
>> >  Changelog                 |    1 +
>> >  libavfilter/Makefile      |    1 +
>> >  libavfilter/af_channels.c |  218
>> > +++++++++++++++++++++++++++++++++++++++++++++
>> >  libavfilter/allfilters.c  |    1 +
>> >  4 files changed, 221 insertions(+), 0 deletions(-)
>> >  create mode 100644 libavfilter/af_channels.c
>> >
>> > diff --git a/Changelog b/Changelog
>> > index 898a247..2ee532a 100644
>> > --- a/Changelog
>> > +++ b/Changelog
>> > @@ -24,6 +24,7 @@ version <next>:
>> >  - avprobe output is now standard INI or JSON. The old format can still
>> >    be used with -of old.
>> >  - Indeo Audio decoder
>> > +- audio channel mapping filter
>> >
>> >
>> >  version 0.8:
>> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> > index 914f0c6..a3a1f27 100644
>> > --- a/libavfilter/Makefile
>> > +++ b/libavfilter/Makefile
>> > @@ -29,6 +29,7 @@ OBJS-$(CONFIG_AMIX_FILTER)                   +=
>> > af_amix.o
>> >  OBJS-$(CONFIG_ANULL_FILTER)                  += af_anull.o
>> >  OBJS-$(CONFIG_ASPLIT_FILTER)                 += split.o
>> >  OBJS-$(CONFIG_ASYNCTS_FILTER)                += af_asyncts.o
>> > +OBJS-$(CONFIG_CHANNELS_FILTER)               += af_channels.o
>> >  OBJS-$(CONFIG_RESAMPLE_FILTER)               += af_resample.o
>> >
>> >  OBJS-$(CONFIG_ANULLSRC_FILTER)               += asrc_anullsrc.o
>> > diff --git a/libavfilter/af_channels.c b/libavfilter/af_channels.c
>> > new file mode 100644
>> > index 0000000..afbe8f1
>> > --- /dev/null
>> > +++ b/libavfilter/af_channels.c
>> > @@ -0,0 +1,218 @@
>> > +/*
>> > + * Copyright (c) 2012 Google, Inc.
>> > + *
>> > + * This file is part of Libav.
>> > + *
>> > + * Libav 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.
>> > + *
>> > + * Libav 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
>> > + * Lesser General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU Lesser General Public
>> > + * License along with Libav; if not, write to the Free Software
>> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> > 02110-1301 USA
>> > + */
>> > +
>> > +/**
>> > + * @file
>> > + * audio channel mapping filter
>> > + */
>> > +
>> > +#include "libavutil/audioconvert.h"
>> > +#include "libavutil/avstring.h"
>> > +#include "libavutil/mathematics.h"
>> > +#include "libavutil/opt.h"
>> > +#include "libavutil/samplefmt.h"
>> > +
>> > +#include "audio.h"
>> > +#include "avfilter.h"
>> > +#include "formats.h"
>> > +#include "internal.h"
>> > +
>> > +#define MAX_CH 8
>> > +typedef struct ChannelsContext {
>> > +    const AVClass   *class;
>> > +
>> > +    AVFilterFormats *formats;
>> > +    AVFilterFormats *sample_rates;
>>
>> Those two are unused.
>>
>> > +    AVFilterChannelLayouts *channel_layouts;
>> > +
>> > +    char *mapping_str;
>> > +    char *channel_layout_str;
>> > +    int map[MAX_CH];
>> > +    int nch;
>> > +    int output_layout;
>> > +} ChannelsContext;
>> > +
>> > +#define OFFSET(x) offsetof(ChannelsContext, x)
>> > +#define A AV_OPT_FLAG_AUDIO_PARAM
>> > +static const AVOption options[] = {
>> > +    { "mapping",        "A comma-separated list of input channel
>> > numbers in the order to be outputted.",
>> > +          OFFSET(mapping_str),        AV_OPT_TYPE_STRING, .flags = A },
>> > +    { "channel_layout", "Output channel layout.",
>> > +          OFFSET(channel_layout_str), AV_OPT_TYPE_STRING, .flags = A },
>> > +    { NULL },
>> > +};
>> > +
>> > +static const AVClass aformat_class = {
>> > +    .class_name = "channels filter",
>> > +    .item_name  = av_default_item_name,
>> > +    .option     = options,
>> > +    .version    = LIBAVUTIL_VERSION_INT,
>> > +};
>> > +
>> > +static av_cold int init(AVFilterContext *ctx, const char *args, void
>> > *opaque)
>> > +{
>> > +    ChannelsContext *s = ctx->priv;
>> > +    int ret;
>> > +    const char *mapping;
>> > +    int nch = 0;
>> > +    int res;
>> > +
>> > +    if (!args) {
>> > +        av_log(ctx, AV_LOG_ERROR, "No parameters supplied.\n");
>> > +        return AVERROR(EINVAL);
>> > +    }
>> > +
>> > +    s->class = &aformat_class;
>> > +    av_opt_set_defaults(s);
>> > +
>> > +    if ((ret = av_set_options_string(s, args, "=", ":")) < 0) {
>> > +        av_log(ctx, AV_LOG_ERROR, "Error parsing options string
>> > '%s'.\n", args);
>> > +        return ret;
>> > +    }
>> > +
>> > +    mapping = s->mapping_str;
>> > +    if (mapping) do {
>> > +        int ch = 0, n = 0;
>> > +        res = sscanf(mapping, "%d%n", &ch, &n);
>> > +        if (res > 0) {
>> > +            mapping += n;
>> > +            if (*mapping == ',')
>> > +                mapping++;
>
> Should I use a different delimiter here? escaping the commas on the
> command line is a pita but comma seems to be the defacto standard.
>
>> > +            s->map[nch++] = ch;
>> > +        }
>> > +    } while (res > 0 && *mapping && nch < MAX_CH);
>> > +    s->nch = nch;
>> > +    s->output_layout = av_get_default_channel_layout(nch);
>> > +
>> > +    if (s->channel_layout_str) {
>> > +        uint64_t fmt;
>> > +        if ((fmt = av_get_channel_layout(s->channel_layout_str)) == 0)
>> > {
>> > +            av_log(ctx, AV_LOG_ERROR, "Error parsing channel layout:
>> > %s.\n",
>> > +                   s->channel_layout_str);
>> > +            ret = AVERROR(EINVAL);
>> > +            goto fail;
>> > +        }
>> > +        if (!s->nch) {
>> > +            int i;
>> > +            s->nch = av_get_channel_layout_nb_channels(fmt);
>> > +            for (i = 0; i < MAX_CH && i < s->nch; i++)
>> > +                s->map[i] = i;
>> > +        } else if (s->nch != av_get_channel_layout_nb_channels(fmt)) {
>> > +            av_log(ctx, AV_LOG_ERROR, "Output channel layout %s does
>> > not match the number of channels mapped %d.\n",
>> > +                   s->channel_layout_str, s->nch);
>> > +            ret = AVERROR(EINVAL);
>> > +            goto fail;
>> > +        }
>> > +        s->output_layout = fmt;
>> > +    }
>> > +    ff_add_channel_layout(&s->channel_layouts, s->output_layout);
>> > +
>> > +fail:
>> > +    av_opt_free(s);
>> > +    return ret;
>> > +}
>> > +
>> > +static int query_formats(AVFilterContext *ctx)
>> > +{
>> > +    ChannelsContext *s = ctx->priv;
>> > +
>> > +    ff_set_common_formats(ctx, ff_all_formats(AVMEDIA_TYPE_AUDIO));
>>
>> Won't it be cleaner/simpler/better to support only planar formats and
>> leave it to libavresample to handle (de)interleaving?
>>
>
> If I can handle the planar case without a copy then I agree. Also does
> the interleaved case allow more than 8 channels? (ignore the fact that
> this filter is currently limited to 8)
>
>> > +    ff_set_common_samplerates(ctx, ff_all_samplerates());
>> > +    ff_channel_layouts_ref(ff_all_channel_layouts(),
>> > &ctx->inputs[0]->out_channel_layouts);
>> > +    ff_channel_layouts_ref(s->channel_layouts,
>> > &ctx->outputs[0]->in_channel_layouts);
>> > +
>> > +    return 0;
>> > +}
>> > +
>> > +#define COPY_INTERLEAVED(type) \
>> > +do { \
>> > +    const type *restrict in = (const type*)buf->extended_data[0]; \
>> > +    type *restrict out = (type*)buf_out->extended_data[0]; \
>> > +    for (ch = 0; ch < nch_out; ch++) { \
>> > +        for (i = 0; i < nb_samples; i++) { \
>> > +            out[i * nch_out + ch] = in[i * nch_in + map[ch]]; \
>> > +        }\
>> > +    }\
>> > +} while (0)
>> > +
>> > +static void filter_samples(AVFilterLink *inlink, AVFilterBufferRef
>> > *buf)
>> > +{
>> > +    AVFilterContext  *ctx = inlink->dst;
>> > +    AVFilterLink *outlink = ctx->outputs[0];
>> > +    AVFilterBufferRef *buf_out;
>> > +    int ch, i;
>> > +    const ChannelsContext *s = ctx->priv;
>> > +    const int nch_in =
>> > av_get_channel_layout_nb_channels(inlink->channel_layout);
>> > +    const int nch_out = s->nch;
>> > +    const int nb_samples = buf->audio->nb_samples;
>> > +    const int *restrict map = s->map;
>> > +
>> > +    buf_out = ff_get_audio_buffer(outlink, AV_PERM_WRITE,
>> > buf->audio->nb_samples);
>> > +
>> > +    if (buf_out->audio->planar) {
>> > +        for (ch = 0; ch < nch_out; ch++) {
>> > +            memcpy(buf_out->extended_data[ch],
>> > buf->extended_data[map[ch]], buf->linesize[0]);
>>
>> For planar formats you don't need to memcpy, just shuffle the pointers
>> around in your input buffer.
>
> Can you elaborate on this a little more? In this case would I not call
> ff_get_audio_buffer() and then ff_filter_samples(outlink, buf) when
> I'm done?
>
>>
>> > +        }
>> > +    } else {
>> > +        switch (inlink->format) {
>> > +        case AV_SAMPLE_FMT_U8:  COPY_INTERLEAVED( uint8_t); break;
>> > +        case AV_SAMPLE_FMT_S16: COPY_INTERLEAVED(uint16_t); break;
>> > +        case AV_SAMPLE_FMT_FLT:
>> > +        case AV_SAMPLE_FMT_S32: COPY_INTERLEAVED(uint32_t); break;
>> > +        case AV_SAMPLE_FMT_DBL: COPY_INTERLEAVED(uint64_t); break;
>> > +        }
>> > +    }
>> > +
>> > +    if (buf->pts != AV_NOPTS_VALUE)
>> > +        buf_out->pts = av_rescale_q(buf->pts, inlink->time_base,
>> > +                                    outlink->time_base);
>> > +    else
>> > +        buf_out->pts = AV_NOPTS_VALUE;
>>
>> You're not setting output timebase, so it's equal to input.
>> buf_out->pts = buf->pts is enough
>>
>> > +
>> > +    ff_filter_samples(outlink, buf_out);
>> > +    avfilter_unref_buffer(buf);
>> > +}
>> > +
>> > +static int config_output(AVFilterLink *outlink)
>> > +{
>> > +    AVFilterContext *ctx = outlink->src;
>> > +    ChannelsContext *s   = ctx->priv;
>> > +
>> > +    outlink->channel_layout = s->output_layout;
>> > +
>> > +    return 0;
>> > +}
>> > +
>> > +AVFilter avfilter_af_channels = {
>> > +    .name          = "channels",
>>
>> Not sure just "channels" is a good name, it's a bit too generic.
>> channelmap might be better. Or does anyone have any other suggestions?
>>
>
> I'm open to suggestions. I'm slightly worried about confusion in
> overloading the term map. "remix" is misleading in my opinion.
>
>> Also some documentation in doc/filters.texi is needed.
>>
>> --
>> Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to