[FFmpeg-devel] [PATCH 06/10] lavfi/buffersrc: accept unknown channel layouts.

[Stefano Sabatini]

On date Wednesday 2013-01-16 17:50:20 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  doc/filters.texi        |    5 +++++
>  libavfilter/buffersrc.c |   26 +++++++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> 
> Updated error message.
> 
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index da7e816..2332436 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -983,6 +983,11 @@ Either a channel layout name from channel_layout_map in
>  @file{libavutil/channel_layout.c} or its corresponding integer representation
>  from the AV_CH_LAYOUT_* macros in @file{libavutil/channel_layout.h}
>  
> + at item channels
> +The number of channels of the incoming audio buffers.
> +If both @var{channels} and @var{channel_layout} are specified, then they
> +must be consistent.
> +
>  @end table
>  
>  For example:
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index 5d0fa92..7644221 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -55,6 +55,7 @@ typedef struct {
>      int sample_rate;
>      enum AVSampleFormat sample_fmt;
>      char               *sample_fmt_str;
> +    int channels;
>      uint64_t channel_layout;
>      char    *channel_layout_str;
>  
> @@ -240,6 +241,7 @@ static const AVOption abuffer_options[] = {
>      { "time_base",      NULL, OFFSET(time_base),           AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, INT_MAX, FLAGS },
>      { "sample_rate",    NULL, OFFSET(sample_rate),         AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, FLAGS },
>      { "sample_fmt",     NULL, OFFSET(sample_fmt_str),      AV_OPT_TYPE_STRING, .flags = FLAGS },
> +    { "channels",       NULL, OFFSET(channels),            AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, FLAGS },
>      { "channel_layout", NULL, OFFSET(channel_layout_str),  AV_OPT_TYPE_STRING, .flags = FLAGS },
>      { NULL },
>  };
> @@ -265,6 +267,9 @@ static av_cold int init_audio(AVFilterContext *ctx, const char *args)
>          goto fail;
>      }
>  
> +    if (s->channel_layout_str) {
> +        int n;
> +        /* TODO reindent */
>      s->channel_layout = av_get_channel_layout(s->channel_layout_str);
>      if (!s->channel_layout) {
>          av_log(ctx, AV_LOG_ERROR, "Invalid channel layout '%s'\n",
> @@ -272,6 +277,23 @@ static av_cold int init_audio(AVFilterContext *ctx, const char *args)
>          ret = AVERROR(EINVAL);
>          goto fail;
>      }
> +        n = av_get_channel_layout_nb_channels(s->channel_layout);
> +        if (s->channels) {
> +            if (n != s->channels) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Mismatching channel count %d and layout '%s' (%d)\n",

(%d) -> (channels %d)
or
layout '%s' with channel count %d

> +                       s->channels, s->channel_layout_str, n);
> +                ret = AVERROR(EINVAL);
> +                goto fail;
> +            }
> +        }
> +        s->channels = n;
> +    } else if (!s->channels) {

> +        av_log(ctx, AV_LOG_ERROR, "Neither number of channels nor "
> +                                  "channel layout not specified\n");

The double negation sounds weird, I'd write:
Neither number of channels nor channel layout was specified
or:
number of channels or channel layout not specified

> +        ret = AVERROR(EINVAL);
> +        goto fail;
> +    }
>  
>      if (!(s->fifo = av_fifo_alloc(sizeof(AVFilterBufferRef*)))) {
>          ret = AVERROR(ENOMEM);
> @@ -324,7 +346,9 @@ static int query_formats(AVFilterContext *ctx)
>          ff_add_format(&samplerates,       c->sample_rate);
>          ff_set_common_samplerates(ctx, samplerates);
>  
> -        ff_add_channel_layout(&channel_layouts, c->channel_layout);
> +        ff_add_channel_layout(&channel_layouts,
> +                              c->channel_layout ? c->channel_layout :
> +                              FF_COUNT2LAYOUT(c->channels));
>          ff_set_common_channel_layouts(ctx, channel_layouts);
>          break;
>      default:

LGTM otherwise.
-- 
FFmpeg = Formidable and Fundamentalist Magnificient Power Extreme Governor