On Wed, 28 Jun 2017 18:10:45 -0400
Vittorio Giovara <vittorio.giov...@gmail.com> wrote:

> From: Anton Khirnov <an...@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.giov...@gmail.com>.
> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
> ---

I guess this is essentially the final version, so I'm bikeshedding a
little bit harder. We'll probably have to live forever with this API,
anyway.

>  /**
> + * 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.

You should specify how it has to be handled. You can:
- default initialize it with {0} or by setting all used fields correctly
- using a predefined layout as initializer (AV_CHANNEL_LAYOUT_STEREO
  etc.)
- initialize it with a constructor function
- must be uninitialized with av_channel_layout_uninit() (at least in
  some situations, which is weird)
- copy via assigning is forbidden (probably?), av_channel_layout_copy()
  must be used instead

> + * No new fields may be added to it without a major version bump.

I think it's still intended that you can add new fields to the union?
So you will add new fields. You just won't change the size, or add
new fields which are mandatory for already defined layout types.

> + *
> + * An AVChannelLayout can be constructed using the convenience function
> + * av_channel_layout_from_mask() / av_channel_layout_from_string(), or it 
> can be
> + * built manually by the caller.
> + */
> +typedef struct AVChannelLayout {
> +    /**
> +     * Channel order used in this layout.
> +     */

("Mandatory field.")

> +    enum AVChannelOrder order;
> +
> +    /**
> +     * Number of channels in this layout. Mandatory field.
> +     */
> +    int nb_channels;
> +
> +    /**
> +     * 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 presend of the AVChannel with the
> +         * corresponding value.
> +         *
> +         * I.e. when map[i] is equal to AV_CHAN_FOO, then AV_CH_FOO is the 
> i-th
> +         * channel in the audio data.
> +         */
> +        enum AVChannel *map;

Even if the channel map identifier is AV_CHAN_SILENCE? What does the
data contain then, actual silence or undefined contents?

> +    } u;
> +} 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 }}
> +
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +/**
>   * Return a channel layout id that matches name, or 0 if no match is found.
>   *
>   * name can be one or several of the following notations,
> @@ -134,7 +313,10 @@ enum AVMatrixEncoding {
>   *   AV_CH_* macros).
>   *
>   * Example: "stereo+FC" = "2+FC" = "2c+1c" = "0x7"
> + *
> + * @deprecated use av_channel_layout_from_string()
>   */
> +attribute_deprecated
>  uint64_t av_get_channel_layout(const char *name);
>  
>  /**
> @@ -143,17 +325,24 @@ uint64_t av_get_channel_layout(const char *name);
>   *
>   * @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);
>  
>  /**
>   * 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
>  uint64_t av_get_default_channel_layout(int nb_channels);
>  
>  /**
> @@ -164,21 +353,142 @@ uint64_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_channel_index()
>   */
> +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_get_channel()
>   */
> +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);
> +#endif
> +
> +/**
> + * @return a string describing a given channel.
> + */
> +const char *av_channel_name(enum AVChannel channel);

What does it return for invalid channels?

> +
> +/**
> + * @return a channel described by the given string.
> + */
> +int av_channel_from_string(const char *name);

Return what exactly? I guess AVChannel or negative error code. Could
also say it's the inverse of av_channel_name().

> +
> +/**
> + * Initialize a native channel layout from a bitmask indicating which 
> channels
> + * are present.
> + */
> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout, uint64_t 
> mask);

What does it do if *channel_layout is not set to all 0 bytes?

> +
> +/**
> + * 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()
> + *    or concatenated with "|")
> + *  - a hexadecimal value of a channel layout (eg. "0x4")
> + *  - the number of channels with default layout (eg. "5")
> + *  - the number of unordered channels (eg. "4 channels")
> + *
> + * @param channel_layout input channel layout
> + * @param str string describing the channel layout
> + * @return 0 channel layout was detected, AVERROR_INVALIDATATA otherwise
> + */
> +int av_channel_layout_from_string(AVChannelLayout *channel_layout,
> +                                  const char *str);

(Same question.)

> +
> +/**
> + * Get the default channel layout for a given number of channels.
> + */
> +void av_channel_layout_default(AVChannelLayout *ch_layout, int nb_channels);

(Same question.)

> +
> +/**
> + * Free any allocated data in the channel layout and reset the channel
> + * count to 0.
> + */
> +void av_channel_layout_uninit(AVChannelLayout *channel_layout);

Can the user assume that for defined channel orders, which do not use
allocated memory (like channel mask), this function can be skipped?

> +
> +/**
> + * 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.
> + *
> + * @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);

Same question.

> +/**
> + * @return a string describing channel_layout or NULL on failure in the same
> + *         format that is accepted by av_channel_layout_from_string(). The
> + *         returned string allocated with av_malloc() and must be freed by 
> the
> + *         caller with av_free().
> + */
> +char *av_channel_layout_describe(const AVChannelLayout *channel_layout);
> +
> +/**
> + * Get the channel with the given index in a channel layout.
> + *
> + * @return channel with the index idx in channel_layout on success or a 
> negative
> + *                 AVERROR on failure (if idx is not valid or the channel 
> order
> + *                 is unspecified)
> + */
> +int av_channel_layout_get_channel(const AVChannelLayout *channel_layout, int 
> idx);
> +
> +/**
> + * Get the index of a given channel in a 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_channel_index(const AVChannelLayout *channel_layout,
> +                                    enum AVChannel channel);

Returns the first index if there are multiple channels like this, I
suppose.

> +
> +/**
> + * Find out what channels from a given set are present in a channel layout,
> + * without regard for their positions.
> + *
> + * @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);

No idea what's this for or what happens to AVChannel values that are
grater than 63.

> +/**
> + * Check whether a channel layout is valid, i.e. can possibly describe audio
> + * data.
> + *
> + * @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.
> + *
> + * @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);
>  
>  /**
>   * @}
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 2c85120b64..a882a68f6c 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -96,6 +96,9 @@
>  #ifndef FF_API_CRYPTO_SIZE_T
>  #define FF_API_CRYPTO_SIZE_T            (LIBAVUTIL_VERSION_MAJOR < 57)
>  #endif
> +#ifndef FF_API_OLD_CHANNEL_LAYOUT
> +#define FF_API_OLD_CHANNEL_LAYOUT       (LIBAVUTIL_VERSION_MAJOR < 57)
> +#endif
>  
>  
>  /**

_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to