On Wed, 17 May 2017 13:46:51 -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>
> ---
> +enum AVChannelOrder {
> + /**
> + * 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 channels that could be
> skipped.
> + */
> + AV_CHANNEL_ORDER_CUSTOM,
> + /**
> + * Only the channel count is specified, without any further information
> + * about the channel order.
> + */
> + AV_CHANNEL_ORDER_UNSPEC,
Wouldn't it be better to make the value 0 something special like maybe
AV_CHANNEL_ORDER_INVALID? Otherwise it could lead to awkward error
messages etc. if something doesn't set the channel layout at all (it
will look like an invalid channel mask).
> +};
> +
> + /**
> + * 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.
> + */
> + uint8_t *map;
I think I'd still like some better documentation on this one. For
example, what does it mean if a channel label appears twice in the same
map. Would it contain the same data? An alternative version of the data
(and different how)? What value does it even take?
Also, why is this only 1 byte per channel? Seems a little short-sighted.
Maybe it would be better not to include ORDER_CUSTOM for now? And
ambisonics could be a different AV_CHANNEL_ORDER.
What's the rationale for making sizeof(AVChannelLayout) part of the
ABI? It seems generally would need to carefully use special operations
for copying and uninitializing the struct anyway. Why not make it an
opaque object?
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
