Re: [FFmpeg-devel] [PATCH 001/293 v7] Add a new channel layout API

[Marton Balint <cus@passwd.hu>]

On Thu, 20 Jan 2022, James Almer wrote:

> From: Anton Khirnov <anton@khirnov.net>
>
> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
>
> Deprecate the old API working with just uint64_t bitmasks.
>
> Expanded and completed by Vittorio Giovara <vittorio.giovara@gmail.com>
> and James Almer <jamrial@gmail.com>.
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Changes since last version:
>
> *Implemented Marton's suggestion for av_channel_layout_index_from_string()
> and av_channel_layout_channel_from_string(), to not mix designation and
> custom name namespaces.

Thanks.

[...]

> +enum AVChannel
> +av_channel_layout_channel_from_string(const AVChannelLayout *channel_layout,
> +                                      const char *str)
> +{

As far as I see this function equvivalent to

     int index = av_channel_layout_index_from_string(channel_layout, str);
     if (index < 0)
         return AV_CHAN_NONE;
     return av_channel_layout_channel_from_index(channel_layout, index);

This would avoid code duplication.

[...]

> +int av_channel_layout_index_from_string(const AVChannelLayout *channel_layout,
> +                                        const char *str)
> +{
> +    char *chname, buf[16];
> +    enum AVChannel ch = AV_CHAN_NONE;
> +
> +    switch (channel_layout->order) {
> +    case AV_CHANNEL_ORDER_CUSTOM:
> +        chname = strstr(str, "@");
> +        if (chname) {
> +            char buf[16];
> +            chname++;
> +            av_strlcpy(buf, str, FFMIN(sizeof(buf), chname - str));
> +            if (!*chname)
> +                chname = NULL;
> +            ch = av_channel_from_string(buf);

If designation is present but invalid we should return error, e.g:

                if (ch == AV_CHAN_NONE && *buf)
                    return AVERROR(EINVAL);

[...]

> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> index d39ae1177a..2efac9046a 100644
> --- a/libavutil/channel_layout.h
> +++ b/libavutil/channel_layout.h
> @@ -23,6 +23,10 @@
> #define AVUTIL_CHANNEL_LAYOUT_H
>
> #include <stdint.h>
> +#include <stdlib.h>
> +
> +#include "version.h"
> +#include "attributes.h"
>
> /**
>  * @file
> @@ -34,6 +38,71 @@
>  * @{
>  */
>
> +enum AVChannel {
> +    ///< Invalid channel index
> +    AV_CHAN_NONE = -1,
> +    AV_CHAN_FRONT_LEFT,
> +    AV_CHAN_FRONT_RIGHT,
> +    AV_CHAN_FRONT_CENTER,
> +    AV_CHAN_LOW_FREQUENCY,
> +    AV_CHAN_BACK_LEFT,
> +    AV_CHAN_BACK_RIGHT,
> +    AV_CHAN_FRONT_LEFT_OF_CENTER,
> +    AV_CHAN_FRONT_RIGHT_OF_CENTER,
> +    AV_CHAN_BACK_CENTER,
> +    AV_CHAN_SIDE_LEFT,
> +    AV_CHAN_SIDE_RIGHT,
> +    AV_CHAN_TOP_CENTER,
> +    AV_CHAN_TOP_FRONT_LEFT,
> +    AV_CHAN_TOP_FRONT_CENTER,
> +    AV_CHAN_TOP_FRONT_RIGHT,
> +    AV_CHAN_TOP_BACK_LEFT,
> +    AV_CHAN_TOP_BACK_CENTER,
> +    AV_CHAN_TOP_BACK_RIGHT,
> +    /** Stereo downmix. */
> +    AV_CHAN_STEREO_LEFT = 29,
> +    /** See above. */
> +    AV_CHAN_STEREO_RIGHT,
> +    AV_CHAN_WIDE_LEFT,
> +    AV_CHAN_WIDE_RIGHT,
> +    AV_CHAN_SURROUND_DIRECT_LEFT,
> +    AV_CHAN_SURROUND_DIRECT_RIGHT,
> +    AV_CHAN_LOW_FREQUENCY_2,
> +    AV_CHAN_TOP_SIDE_LEFT,
> +    AV_CHAN_TOP_SIDE_RIGHT,
> +    AV_CHAN_BOTTOM_FRONT_CENTER,
> +    AV_CHAN_BOTTOM_FRONT_LEFT,
> +    AV_CHAN_BOTTOM_FRONT_RIGHT,
> +
> +    /** Channel is empty can be safely skipped. */
> +    AV_CHAN_UNUSED = 64,
> +
> +    /** Channel contains data, but its position is unknown. */
> +    AV_CHAN_UNKWNOWN = 128,

typo: UNKNOWN

No more comments for me, thanks.

Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".