[FFmpeg-devel] [PATCH 2/5] audioconvert: make av_get_channel_layout accept composite names.

[Stefano Sabatini]

On date Saturday 2011-11-05 21:21:48 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  libavutil/audioconvert.c |   48 +++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/libavutil/audioconvert.c b/libavutil/audioconvert.c
> index d89796d..99a5f02 100644
> --- a/libavutil/audioconvert.c
> +++ b/libavutil/audioconvert.c
> @@ -72,23 +72,53 @@ static const struct {
>      { "5.1",         6,  AV_CH_LAYOUT_5POINT1_BACK },
>      { "7.1",         8,  AV_CH_LAYOUT_7POINT1 },
>      { "7.1(wide)",   8,  AV_CH_LAYOUT_7POINT1_WIDE },
> -    { "5.1+downmix", 8,  AV_CH_LAYOUT_5POINT1|AV_CH_LAYOUT_STEREO_DOWNMIX, },
> -    { "7.1+downmix", 10, AV_CH_LAYOUT_7POINT1|AV_CH_LAYOUT_STEREO_DOWNMIX, },
> +    { "downmix",     2,  AV_CH_LAYOUT_STEREO_DOWNMIX, },
>      { 0 }
>  };
>  
> -int64_t av_get_channel_layout(const char *name)
> +static int64_t get_channel_layout_single(const char *name, int name_len)
>  {
> -    int i = 0;
> -    do {
> -        if (!strcmp(channel_layout_map[i].name, name))
> -            return channel_layout_map[i].layout;
> -        i++;
> -    } while (channel_layout_map[i].name);
> +    int i;
> +    char *end;
>  
> +    for (i = 0; i < FF_ARRAY_ELEMS(channel_layout_map) - 1; i++) {
> +        if (strlen(channel_layout_map[i].name) == name_len &&
> +            !memcmp(channel_layout_map[i].name, name, name_len))
> +            return channel_layout_map[i].layout;
> +    }
> +    for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++)
> +        if (channel_names[i] &&
> +            strlen(channel_names[i]) == name_len &&
> +            !memcmp(channel_names[i], name, name_len))
> +            return (int64_t)1 << i;

> +    i = strtol(name, &end, 0);
> +    if (end - name == name_len)
> +        return av_get_default_channel_layout(i);

I wonder if this should be something like "2channels", while a bare
"2" may be interpreted as the channel layout with code 2.

>      return 0;
>  }
>  
> +int64_t av_get_channel_layout(const char *name)
> +{
> +    const char *n, *e;
> +    const char *name_end = name + strlen(name);

> +    int64_t layout = 0, lc;

Nit: please meaningful names, e.g. cl and cl1 (cl-single will do)

> +    int inter = 0;
> +
> +    for (n = name; n < name_end; n = e + 1) {
> +        for (e = n; e < name_end && *e != '+' && *e != '|'; e++);
> +        lc = get_channel_layout_single(n, e - n);
> +        if (!lc)
> +            return 0;
> +        if (layout & lc)
> +            inter++;
> +        layout |= lc;
> +    }

> +    if (inter)
> +        av_log(NULL, AV_LOG_WARNING,
> +               "Channel layout \"%s\" is redundant.", name);

Note: we may want to extend the function to take a log context +
offset, and avoid spamming log with unwanted warnings.

> +    return layout;
> +}
> +
>  void av_get_channel_layout_string(char *buf, int buf_size,
>                                    int nb_channels, int64_t channel_layout)
>  {

Rest looks nice, I had the very same idea but never find the
motivation to implement it. Also please document the extension, and
remember to bump micro when committing (but leave some time for others
to comment on this public API extension).
-- 
FFmpeg = Frightening and Fabulous Magic Puristic Egregious Gigant