[FFmpeg-devel] [PATCH 001/293 v8] avutil/channel_layout: Add a new channel layout API

Nicolas George george at nsup.org
Thu Jan 27 17:08:10 EET 2022


Looking only at the changes in channel_layout.h:

James Almer (12022-01-24):
> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> index d39ae1177a..51e4ea3804 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 = 0x200,
> +
> +    /** Channel contains data, but its position is unknown. */
> +    AV_CHAN_UNKWNOWN = 0x300,
> +};
> +
> +enum AVChannelOrder {
> +    /**
> +     * Only the channel count is specified, without any further information
> +     * about the channel order.
> +     */
> +    AV_CHANNEL_ORDER_UNSPEC,
> +    /**
> +     * The native channel order, i.e. the channels are in the same order in
> +     * which they are defined in the AVChannel enum. This supports up to 63
> +     * different channels.
> +     */
> +    AV_CHANNEL_ORDER_NATIVE,
> +    /**
> +     * The channel order does not correspond to any other predefined order and
> +     * is stored as an explicit map. For example, this could be used to support
> +     * layouts with 64 or more channels, or with empty/skipped (AV_CHAN_SILENCE)
> +     * channels at arbitrary positions.
> +     */
> +    AV_CHANNEL_ORDER_CUSTOM,
> +};
> +
> +
>  /**
>   * @defgroup channel_masks Audio channel masks
>   *
> @@ -46,41 +115,46 @@
>   *
>   * @{
>   */
> -#define AV_CH_FRONT_LEFT             0x00000001
> -#define AV_CH_FRONT_RIGHT            0x00000002
> -#define AV_CH_FRONT_CENTER           0x00000004
> -#define AV_CH_LOW_FREQUENCY          0x00000008
> -#define AV_CH_BACK_LEFT              0x00000010
> -#define AV_CH_BACK_RIGHT             0x00000020
> -#define AV_CH_FRONT_LEFT_OF_CENTER   0x00000040
> -#define AV_CH_FRONT_RIGHT_OF_CENTER  0x00000080
> -#define AV_CH_BACK_CENTER            0x00000100
> -#define AV_CH_SIDE_LEFT              0x00000200
> -#define AV_CH_SIDE_RIGHT             0x00000400
> -#define AV_CH_TOP_CENTER             0x00000800
> -#define AV_CH_TOP_FRONT_LEFT         0x00001000
> -#define AV_CH_TOP_FRONT_CENTER       0x00002000
> -#define AV_CH_TOP_FRONT_RIGHT        0x00004000
> -#define AV_CH_TOP_BACK_LEFT          0x00008000
> -#define AV_CH_TOP_BACK_CENTER        0x00010000
> -#define AV_CH_TOP_BACK_RIGHT         0x00020000
> -#define AV_CH_STEREO_LEFT            0x20000000  ///< Stereo downmix.
> -#define AV_CH_STEREO_RIGHT           0x40000000  ///< See AV_CH_STEREO_LEFT.
> -#define AV_CH_WIDE_LEFT              0x0000000080000000ULL
> -#define AV_CH_WIDE_RIGHT             0x0000000100000000ULL
> -#define AV_CH_SURROUND_DIRECT_LEFT   0x0000000200000000ULL
> -#define AV_CH_SURROUND_DIRECT_RIGHT  0x0000000400000000ULL
> -#define AV_CH_LOW_FREQUENCY_2        0x0000000800000000ULL
> -#define AV_CH_TOP_SIDE_LEFT          0x0000001000000000ULL
> -#define AV_CH_TOP_SIDE_RIGHT         0x0000002000000000ULL
> -#define AV_CH_BOTTOM_FRONT_CENTER    0x0000004000000000ULL
> -#define AV_CH_BOTTOM_FRONT_LEFT      0x0000008000000000ULL
> -#define AV_CH_BOTTOM_FRONT_RIGHT     0x0000010000000000ULL
> +#define AV_CH_FRONT_LEFT             (1ULL << AV_CHAN_FRONT_LEFT           )
> +#define AV_CH_FRONT_RIGHT            (1ULL << AV_CHAN_FRONT_RIGHT          )
> +#define AV_CH_FRONT_CENTER           (1ULL << AV_CHAN_FRONT_CENTER         )
> +#define AV_CH_LOW_FREQUENCY          (1ULL << AV_CHAN_LOW_FREQUENCY        )
> +#define AV_CH_BACK_LEFT              (1ULL << AV_CHAN_BACK_LEFT            )
> +#define AV_CH_BACK_RIGHT             (1ULL << AV_CHAN_BACK_RIGHT           )
> +#define AV_CH_FRONT_LEFT_OF_CENTER   (1ULL << AV_CHAN_FRONT_LEFT_OF_CENTER )
> +#define AV_CH_FRONT_RIGHT_OF_CENTER  (1ULL << AV_CHAN_FRONT_RIGHT_OF_CENTER)
> +#define AV_CH_BACK_CENTER            (1ULL << AV_CHAN_BACK_CENTER          )
> +#define AV_CH_SIDE_LEFT              (1ULL << AV_CHAN_SIDE_LEFT            )
> +#define AV_CH_SIDE_RIGHT             (1ULL << AV_CHAN_SIDE_RIGHT           )
> +#define AV_CH_TOP_CENTER             (1ULL << AV_CHAN_TOP_CENTER           )
> +#define AV_CH_TOP_FRONT_LEFT         (1ULL << AV_CHAN_TOP_FRONT_LEFT       )
> +#define AV_CH_TOP_FRONT_CENTER       (1ULL << AV_CHAN_TOP_FRONT_CENTER     )
> +#define AV_CH_TOP_FRONT_RIGHT        (1ULL << AV_CHAN_TOP_FRONT_RIGHT      )
> +#define AV_CH_TOP_BACK_LEFT          (1ULL << AV_CHAN_TOP_BACK_LEFT        )
> +#define AV_CH_TOP_BACK_CENTER        (1ULL << AV_CHAN_TOP_BACK_CENTER      )
> +#define AV_CH_TOP_BACK_RIGHT         (1ULL << AV_CHAN_TOP_BACK_RIGHT       )
> +#define AV_CH_STEREO_LEFT            (1ULL << AV_CHAN_STEREO_LEFT          )
> +#define AV_CH_STEREO_RIGHT           (1ULL << AV_CHAN_STEREO_RIGHT         )
> +#define AV_CH_WIDE_LEFT              (1ULL << AV_CHAN_WIDE_LEFT            )
> +#define AV_CH_WIDE_RIGHT             (1ULL << AV_CHAN_WIDE_RIGHT           )
> +#define AV_CH_SURROUND_DIRECT_LEFT   (1ULL << AV_CHAN_SURROUND_DIRECT_LEFT )
> +#define AV_CH_SURROUND_DIRECT_RIGHT  (1ULL << AV_CHAN_SURROUND_DIRECT_RIGHT)
> +#define AV_CH_LOW_FREQUENCY_2        (1ULL << AV_CHAN_LOW_FREQUENCY_2      )
> +#define AV_CH_TOP_SIDE_LEFT          (1ULL << AV_CHAN_TOP_SIDE_LEFT        )
> +#define AV_CH_TOP_SIDE_RIGHT         (1ULL << AV_CHAN_TOP_SIDE_RIGHT       )
> +#define AV_CH_BOTTOM_FRONT_CENTER    (1ULL << AV_CHAN_BOTTOM_FRONT_CENTER  )
> +#define AV_CH_BOTTOM_FRONT_LEFT      (1ULL << AV_CHAN_BOTTOM_FRONT_LEFT    )
> +#define AV_CH_BOTTOM_FRONT_RIGHT     (1ULL << AV_CHAN_BOTTOM_FRONT_RIGHT   )
>  
> +#if FF_API_OLD_CHANNEL_LAYOUT
>  /** Channel mask value used for AVCodecContext.request_channel_layout
>      to indicate that the user requests the channel order of the decoder output
> -    to be the native codec channel order. */
> +    to be the native codec channel order.
> +    @deprecated channel order is now indicated in a special field in
> +                AVChannelLayout
> +    */
>  #define AV_CH_LAYOUT_NATIVE          0x8000000000000000ULL
> +#endif
>  
>  /**
>   * @}
> @@ -128,6 +202,167 @@ enum AVMatrixEncoding {
>      AV_MATRIX_ENCODING_NB
>  };
>  
> +/**
> + * @}
> + */
> +
> +/**
> + * An AVChannelCustom defines a single channel within a custom order layout
> + *
> + * Unlike most structures in FFmpeg, sizeof(AVChannelCustom) is a part of the
> + * public ABI.
> + *
> + * No new fields may be added to it without a major version bump.
> + */
> +typedef struct AVChannelCustom {
> +    enum AVChannel id;
> +    char name[16];
> +    void *opaque;
> +} AVChannelCustom;
> +
> +/**
> + * An AVChannelLayout holds information about the channel layout of audio data.
> + *
> + * A channel layout here is defined as a set of channels ordered in a specific
> + * way (unless the channel order is AV_CHANNEL_ORDER_UNSPEC, in which case an
> + * AVChannelLayout carries only the channel count).
> + *
> + * Unlike most structures in Libav, sizeof(AVChannelLayout) is a part of the
> + * public ABI and may be used by the caller. E.g. it may be allocated on stack
> + * or embedded in caller-defined structs.
> + *
> + * AVChannelLayout can be initialized as follows:
> + * - default initialization with {0}, followed by setting all used fields
> + *   correctly;
> + * - by assigning one of the predefined AV_CHANNEL_LAYOUT_* initializers;
> + * - with a constructor function, such as av_channel_layout_default(),
> + *   av_channel_layout_from_mask() or av_channel_layout_from_string().
> + *
> + * The channel layout must be unitialized with av_channel_layout_uninit()
> + *
> + * Copying an AVChannelLayout via assigning is forbidden,

> + * av_channel_layout_copy() must be used. instead (and its return value should

Spurious stop.

> + * be checked)
> + *
> + * No new fields may be added to it without a major version bump, except for
> + * new elements of the union fitting in sizeof(uint64_t).
> + */
> +typedef struct AVChannelLayout {
> +    /**
> +     * Channel order used in this layout.
> +     * This is a mandatory field.
> +     */
> +    enum AVChannelOrder order;
> +
> +    /**
> +     * Number of channels in this layout. Mandatory field.
> +     */

> +    int nb_channels;

If it does not make sense to have a negative value, then unsigned is
preferable.

> +
> +    /**
> +     * Details about which channels are present in this layout.
> +     * For AV_CHANNEL_ORDER_UNSPEC, this field is undefined and must not be
> +     * used.
> +     */
> +    union {
> +        /**
> +         * This member must be used for AV_CHANNEL_ORDER_NATIVE.
> +         * It is a bitmask, where the position of each set bit means that the
> +         * AVChannel with the corresponding value is present.
> +         *
> +         * I.e. when (mask & (1 << AV_CHAN_FOO)) is non-zero, then AV_CHAN_FOO
> +         * is present in the layout. Otherwise it is not present.
> +         *
> +         * @note when a channel layout using a bitmask is constructed or
> +         * modified manually (i.e.  not using any of the av_channel_layout_*
> +         * functions), the code doing it must ensure that the number of set bits
> +         * is equal to nb_channels.
> +         */
> +        uint64_t mask;
> +        /**
> +         * This member must be used when the channel order is
> +         * AV_CHANNEL_ORDER_CUSTOM. It is a nb_channels-sized array, with each
> +         * element signalling the presence of the AVChannel with the
> +         * corresponding value in map[i].id.
> +         *
> +         * I.e. when map[i].id is equal to AV_CHAN_FOO, then AV_CH_FOO is the
> +         * i-th channel in the audio data.
> +         *
> +         * map[i].name may be filled with a 0-terminated string, in which case
> +         * it will be used for the purpose of identifying the channel with the
> +         * convenience functions below. Otherise it must be zeroed.
> +         */
> +        AVChannelCustom *map;
> +    } u;
> +
> +    /**
> +     * For some private data of the user.
> +     */
> +    void *opaque;
> +} AVChannelLayout;
> +

> +#define AV_CHANNEL_LAYOUT_MONO \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 1,  .u = { .mask = AV_CH_LAYOUT_MONO }}
> +#define AV_CHANNEL_LAYOUT_STEREO \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 2,  .u = { .mask = AV_CH_LAYOUT_STEREO }}
> +#define AV_CHANNEL_LAYOUT_2POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = AV_CH_LAYOUT_2POINT1 }}
> +#define AV_CHANNEL_LAYOUT_2_1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = AV_CH_LAYOUT_2_1 }}
> +#define AV_CHANNEL_LAYOUT_SURROUND \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 3,  .u = { .mask = AV_CH_LAYOUT_SURROUND }}
> +#define AV_CHANNEL_LAYOUT_3POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = AV_CH_LAYOUT_3POINT1 }}
> +#define AV_CHANNEL_LAYOUT_4POINT0 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = AV_CH_LAYOUT_4POINT0 }}
> +#define AV_CHANNEL_LAYOUT_4POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 5,  .u = { .mask = AV_CH_LAYOUT_4POINT1 }}
> +#define AV_CHANNEL_LAYOUT_2_2 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = AV_CH_LAYOUT_2_2 }}
> +#define AV_CHANNEL_LAYOUT_QUAD \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 4,  .u = { .mask = AV_CH_LAYOUT_QUAD }}
> +#define AV_CHANNEL_LAYOUT_5POINT0 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 5,  .u = { .mask = AV_CH_LAYOUT_5POINT0 }}
> +#define AV_CHANNEL_LAYOUT_5POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 6,  .u = { .mask = AV_CH_LAYOUT_5POINT1 }}
> +#define AV_CHANNEL_LAYOUT_5POINT0_BACK \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 5,  .u = { .mask = AV_CH_LAYOUT_5POINT0_BACK }}
> +#define AV_CHANNEL_LAYOUT_5POINT1_BACK \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 6,  .u = { .mask = AV_CH_LAYOUT_5POINT1_BACK }}
> +#define AV_CHANNEL_LAYOUT_6POINT0 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 6,  .u = { .mask = AV_CH_LAYOUT_6POINT0 }}
> +#define AV_CHANNEL_LAYOUT_6POINT0_FRONT \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 6,  .u = { .mask = AV_CH_LAYOUT_6POINT0_FRONT }}
> +#define AV_CHANNEL_LAYOUT_HEXAGONAL \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 6,  .u = { .mask = AV_CH_LAYOUT_HEXAGONAL }}
> +#define AV_CHANNEL_LAYOUT_6POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 7,  .u = { .mask = AV_CH_LAYOUT_6POINT1 }}
> +#define AV_CHANNEL_LAYOUT_6POINT1_BACK \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 7,  .u = { .mask = AV_CH_LAYOUT_6POINT1_BACK }}
> +#define AV_CHANNEL_LAYOUT_6POINT1_FRONT \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 7,  .u = { .mask = AV_CH_LAYOUT_6POINT1_FRONT }}
> +#define AV_CHANNEL_LAYOUT_7POINT0 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 7,  .u = { .mask = AV_CH_LAYOUT_7POINT0 }}
> +#define AV_CHANNEL_LAYOUT_7POINT0_FRONT \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 7,  .u = { .mask = AV_CH_LAYOUT_7POINT0_FRONT }}
> +#define AV_CHANNEL_LAYOUT_7POINT1 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 8,  .u = { .mask = AV_CH_LAYOUT_7POINT1 }}
> +#define AV_CHANNEL_LAYOUT_7POINT1_WIDE \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 8,  .u = { .mask = AV_CH_LAYOUT_7POINT1_WIDE }}
> +#define AV_CHANNEL_LAYOUT_7POINT1_WIDE_BACK \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 8,  .u = { .mask = AV_CH_LAYOUT_7POINT1_WIDE_BACK }}
> +#define AV_CHANNEL_LAYOUT_OCTAGONAL \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 8,  .u = { .mask = AV_CH_LAYOUT_OCTAGONAL }}
> +#define AV_CHANNEL_LAYOUT_HEXADECAGONAL \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 16, .u = { .mask = AV_CH_LAYOUT_HEXAGONAL }}
> +#define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 2,  .u = { .mask = AV_CH_LAYOUT_STEREO_DOWNMIX }}
> +#define AV_CHANNEL_LAYOUT_22POINT2 \
> +    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = 24, .u = { .mask = AV_CH_LAYOUT_22POINT2 }}

#define AV_CHANNEL_LAYOUT_MASK(nb, m) \
    { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { .mask = (m) }}

and use it to define all other macros will make this much more readable.

> +
> +struct AVBPrint;
> +
> +#if FF_API_OLD_CHANNEL_LAYOUT
>  /**
>   * Return a channel layout id that matches name, or 0 if no match is found.
>   *
> @@ -144,7 +379,10 @@ enum AVMatrixEncoding {
>   *   AV_CH_* macros).
>   *
>   * Example: "stereo+FC" = "2c+FC" = "2c+1c" = "0x7"
> + *
> + * @deprecated use av_channel_layout_from_string()
>   */
> +attribute_deprecated
>  uint64_t av_get_channel_layout(const char *name);
>  
>  /**
> @@ -158,7 +396,9 @@ uint64_t av_get_channel_layout(const char *name);
>   * @param[out] nb_channels      number of channels
>   *
>   * @return 0 on success, AVERROR(EINVAL) if the parsing fails.
> + * @deprecated use av_channel_layout_from_string()
>   */
> +attribute_deprecated
>  int av_get_extended_channel_layout(const char *name, uint64_t* channel_layout, int* nb_channels);
>  
>  /**
> @@ -167,23 +407,31 @@ int av_get_extended_channel_layout(const char *name, uint64_t* channel_layout, i
>   *
>   * @param buf put here the string containing the channel layout
>   * @param buf_size size in bytes of the buffer
> + * @deprecated use av_channel_layout_describe()
>   */
> +attribute_deprecated
>  void av_get_channel_layout_string(char *buf, int buf_size, int nb_channels, uint64_t channel_layout);
>  
> -struct AVBPrint;
>  /**
>   * Append a description of a channel layout to a bprint buffer.
> + * @deprecated use av_channel_layout_describe()
>   */
> +attribute_deprecated
>  void av_bprint_channel_layout(struct AVBPrint *bp, int nb_channels, uint64_t channel_layout);
>  
>  /**
>   * Return the number of channels in the channel layout.
> + * @deprecated use AVChannelLayout.nb_channels
>   */
> +attribute_deprecated
>  int av_get_channel_layout_nb_channels(uint64_t channel_layout);
>  
>  /**
>   * Return default channel layout for a given number of channels.
> + *
> + * @deprecated use av_channel_layout_default()
>   */
> +attribute_deprecated
>  int64_t av_get_default_channel_layout(int nb_channels);
>  
>  /**
> @@ -194,20 +442,28 @@ int64_t av_get_default_channel_layout(int nb_channels);
>   *
>   * @return index of channel in channel_layout on success, a negative AVERROR
>   *         on error.
> + *
> + * @deprecated use av_channel_layout_index_from_channel()
>   */
> +attribute_deprecated
>  int av_get_channel_layout_channel_index(uint64_t channel_layout,
>                                          uint64_t channel);
>  
>  /**
>   * Get the channel with the given index in channel_layout.
> + * @deprecated use av_channel_layout_channel_from_index()
>   */
> +attribute_deprecated
>  uint64_t av_channel_layout_extract_channel(uint64_t channel_layout, int index);
>  
>  /**
>   * Get the name of a given channel.
>   *
>   * @return channel name on success, NULL on error.
> + *
> + * @deprecated use av_channel_name()
>   */
> +attribute_deprecated
>  const char *av_get_channel_name(uint64_t channel);
>  
>  /**
> @@ -215,7 +471,9 @@ const char *av_get_channel_name(uint64_t channel);
>   *
>   * @param channel  a channel layout with a single channel
>   * @return  channel description on success, NULL on error
> + * @deprecated use av_channel_description()
>   */
> +attribute_deprecated
>  const char *av_get_channel_description(uint64_t channel);
>  
>  /**
> @@ -226,9 +484,238 @@ const char *av_get_channel_description(uint64_t channel);
>   * @param[out] name    name of the layout
>   * @return  0  if the layout exists,
>   *          <0 if index is beyond the limits
> + * @deprecated use av_channel_layout_standard()
>   */
> +attribute_deprecated
>  int av_get_standard_channel_layout(unsigned index, uint64_t *layout,
>                                     const char **name);
> +#endif
> +
> +/**
> + * Get a human readable string in an abbreviated form describing a given channel,
> + * or "?" if not found.
> + * This is the inverse function of @ref av_channel_from_string().
> + *
> + * @param buf pre-allocated buffer where to put the generated string
> + * @param buf_size size in bytes of the buffer.
> + * @return amount of bytes needed to hold the output string, or a negative AVERROR
> + *         on failure. If the returned value is bigger than buf_size, then the
> + *         string was truncated.
> + */
> +int av_channel_name(char *buf, size_t buf_size, enum AVChannel channel);
> +
> +/**
> + * bprint variant of av_channel_name().
> + *
> + * @note the string will be appended to the bprint buffer.
> + */
> +void av_channel_name_bprint(struct AVBPrint *bp, enum AVChannel channel_id);
> +
> +/**
> + * Get a human readable string describing a given channel, or "?" if not found.
> + *

Add:

The resulting string can be passed to av_channel_from_string() to
rebuild the same channel layouts, except for opaque pointers.

> + * @param buf pre-allocated buffer where to put the generated string
> + * @param buf_size size in bytes of the buffer.
> + * @return amount of bytes needed to hold the output string, or a negative AVERROR
> + *         on failure. If the returned value is bigger than buf_size, then the
> + *         string was truncated.
> + */
> +int av_channel_description(char *buf, size_t buf_size, enum AVChannel channel);
> +
> +/**
> + * bprint variant of av_channel_description().
> + *
> + * @note the string will be appended to the bprint buffer.
> + */
> +void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel channel_id);
> +
> +/**
> + * This is the inverse function of @ref av_channel_name().
> + *
> + * @return the channel with the given name
> + *         AV_CHAN_NONE when name does not identify a known channel
> + */
> +enum AVChannel av_channel_from_string(const char *name);
> +
> +/**
> + * Initialize a native channel layout from a bitmask indicating which channels
> + * are present.
> + *
> + * @param channel_layout the layout structure to be initialized
> + * @param mask bitmask describing the channel layout
> + *
> + * @return 0 on success
> + *         AVERROR(EINVAL) for invalid mask values
> + */
> +int av_channel_layout_from_mask(AVChannelLayout *channel_layout, uint64_t mask);
> +
> +/**
> + * Initialize a channel layout from a given string description.
> + * The input string can be represented by:
> + *  - the formal channel layout name (returned by av_channel_layout_describe())
> + *  - single or multiple channel names (returned by av_channel_name(), eg. "FL",
> + *    or concatenated with "+", each optionally containing a custom name after
> + *    a "@", eg. "FL at Left+FR@Right+LFE")
> + *  - a decimal or hexadecimal value of a native channel layout (eg. "4" or "0x4")
> + *  - the number of channels with default layout (eg. "4c")
> + *  - the number of unordered channels (eg. "4C" or "4 channels")

Add:

The string can be the output of av_channel_description().

Also, the documentation of the syntax should go into the user
documentation, not just the API documentation.

> + *
> + * @param channel_layout input channel layout
> + * @param str string describing the channel layout

> + * @return 0 channel layout was detected, AVERROR_INVALIDATATA otherwise

Nit: detected?

> + */
> +int av_channel_layout_from_string(AVChannelLayout *channel_layout,
> +                                  const char *str);
> +
> +/**
> + * Get the default channel layout for a given number of channels.
> + *
> + * @param channel_layout the layout structure to be initialized
> + * @param nb_channels number of channels
> + */
> +void av_channel_layout_default(AVChannelLayout *ch_layout, int nb_channels);
> +
> +/**
> + * Iterate over all standard channel layouts.
> + *
> + * @param opaque a pointer where libavutil will store the iteration state. Must
> + *               point to NULL to start the iteration.
> + *
> + * @return the standard channel layout or NULL when the iteration is
> + *         finished
> + */
> +const AVChannelLayout *av_channel_layout_standard(void **opaque);
> +
> +/**
> + * Free any allocated data in the channel layout and reset the channel
> + * count to 0.
> + *
> + * @param channel_layout the layout structure to be uninitialized
> + */
> +void av_channel_layout_uninit(AVChannelLayout *channel_layout);
> +

> +/**
> + * Make a copy of a channel layout. This differs from just assigning src to dst
> + * in that it allocates and copies the map for AV_CHANNEL_ORDER_CUSTOM.
> + *
> + * @note the destination channel_layout will be always uninitialized before copy.
> + *
> + * @param dst destination channel layout
> + * @param src source channel layout
> + * @return 0 on success, a negative AVERROR on error.
> + */
> +int av_channel_layout_copy(AVChannelLayout *dst, const AVChannelLayout *src);

I find this defeats a lot of the benefits of making
sizeof(AVChannelLayout) part of the API.

Have you seriously considered making AVChannelLayout refcounted instead?

> +
> +/**
> + * Get a human-readable string describing the channel layout properties.
> + * The string will be in the same format that is accepted by
> + * @ref av_channel_layout_from_string().
> + *
> + * @param channel_layout channel layout to be described
> + * @param buf pre-allocated buffer where to put the generated string
> + * @param buf_size size in bytes of the buffer.
> + * @return amount of bytes needed to hold the output string, or a negative AVERROR
> + *         on failure. If the returned value is bigger than buf_size, then the
> + *         string was truncated.
> + */
> +int av_channel_layout_describe(const AVChannelLayout *channel_layout,
> +                               char *buf, size_t buf_size);
> +
> +/**
> + * bprint variant of av_channel_layout_describe().
> + *
> + * @note the string will be appended to the bprint buffer.
> + * @return 0 on success, or a negative AVERROR value on failure.
> + */
> +int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
> +                                      struct AVBPrint *bp);
> +
> +/**
> + * Get the channel with the given index in a channel layout.
> + *
> + * @param channel_layout input channel layout
> + * @return channel with the index idx in channel_layout on success or
> + *         AV_CHAN_NONE on failure (if idx is not valid or the channel order is
> + *         unspecified)
> + */
> +enum AVChannel
> +av_channel_layout_channel_from_index(const AVChannelLayout *channel_layout, unsigned int idx);
> +
> +/**
> + * Get the index of a given channel in a channel layout. In case multiple
> + * channels are found, only the first match will be returned.
> + *
> + * @param channel_layout input channel layout
> + * @return index of channel in channel_layout on success or a negative number if
> + *         channel is not present in channel_layout.
> + */
> +int av_channel_layout_index_from_channel(const AVChannelLayout *channel_layout,
> +                                         enum AVChannel channel);
> +
> +/**
> + * Get the index in a channel layout of a channel described by the given string.
> + * In case multiple channels are found, only the first match will be returned.
> + *
> + * This function accepts channel names in the same format as
> + * @ref av_channel_from_string().
> + *
> + * @param channel_layout input channel layout
> + * @return a channel index described by the given string, or a negative AVERROR
> + *         value.
> + */
> +int av_channel_layout_index_from_string(const AVChannelLayout *channel_layout,
> +                                        const char *name);
> +
> +/**
> + * Get a channel described by the given string.
> + *
> + * This function accepts channel names in the same format as
> + * @ref av_channel_from_string().
> + *
> + * @param channel_layout input channel layout
> + * @return a channel described by the given string in channel_layout on success
> + *         or AV_CHAN_NONE on failure (if the string is not valid or the channel
> + *         order is unspecified)
> + */
> +enum AVChannel
> +av_channel_layout_channel_from_string(const AVChannelLayout *channel_layout,
> +                                      const char *name);
> +
> +/**
> + * Find out what channels from a given set are present in a channel layout,
> + * without regard for their positions.
> + *
> + * @param channel_layout input channel layout
> + * @param mask a combination of AV_CH_* representing a set of channels
> + * @return a bitfield representing all the channels from mask that are present
> + *         in channel_layout
> + */
> +uint64_t av_channel_layout_subset(const AVChannelLayout *channel_layout,
> +                                  uint64_t mask);
> +
> +/**
> + * Check whether a channel layout is valid, i.e. can possibly describe audio
> + * data.
> + *
> + * @param channel_layout input channel layout
> + * @return 1 if channel_layout is valid, 0 otherwise.
> + */
> +int av_channel_layout_check(const AVChannelLayout *channel_layout);
> +
> +/**
> + * Check whether two channel layouts are semantically the same, i.e. the same
> + * channels are present on the same positions in both.
> + *
> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the other is
> + * not, they are considered to be unequal. If both are AV_CHANNEL_ORDER_UNSPEC,
> + * they are considered equal iff the channel counts are the same in both.
> + *
> + * @param chl input channel layout
> + * @param chl1 input channel layout
> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A negative
> + *         AVERROR code if one or both are invalid.
> + */
> +int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1);
>  
>  /**
>   * @}

I would really like you to consider the option of refcounting. I think
it would be more convenient and more robust for the future.

But apart from that and the few nits above, I have no more objections to
the API.

Thanks for your efforts.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20220127/38cce920/attachment.sig>


More information about the ffmpeg-devel mailing list