[FFmpeg-devel] [PATCH 229/281] libopus: convert to new channel layout API

[Anton Khirnov]

Quoting James Almer (2022-01-13 03:05:08)
> From: Anton Khirnov <anton at khirnov.net>
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/libopusdec.c  | 39 +++++++++++++-----------
>  libavcodec/libopusenc.c  | 65 ++++++++++++++++++++++------------------
>  libavcodec/vorbis.h      |  3 ++
>  libavcodec/vorbis_data.c | 18 +++++++++++
>  4 files changed, 79 insertions(+), 46 deletions(-)
> 
> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> index 86ef715205..abffd5f463 100644
> --- a/libavcodec/libopusdec.c
> +++ b/libavcodec/libopusdec.c
> @@ -50,55 +50,60 @@ struct libopus_context {
>  static av_cold int libopus_decode_init(AVCodecContext *avc)
>  {
>      struct libopus_context *opus = avc->priv_data;
> -    int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled;
> +    int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled, channels;
>      uint8_t mapping_arr[8] = { 0, 1 }, *mapping;
>  
> -    avc->channels = avc->extradata_size >= 10 ? avc->extradata[9] : (avc->channels == 1) ? 1 : 2;
> -    if (avc->channels <= 0) {
> +    channels = avc->extradata_size >= 10 ? avc->extradata[9] : (channels == 1) ? 1 : 2;
                                                                   ^^^^^^^^
uninitialized read
this is meant to use the user-provided value

> diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
> index 45b23fcbb5..401a2c17c8 100644
> --- a/libavcodec/libopusenc.c
> +++ b/libavcodec/libopusenc.c
> @@ -91,7 +91,7 @@ static void libopus_write_header(AVCodecContext *avctx, int stream_count,
>                                   const uint8_t *channel_mapping)
>  {
>      uint8_t *p   = avctx->extradata;
> -    int channels = avctx->channels;
> +    int channels = avctx->ch_layout.nb_channels;
>  
>      bytestream_put_buffer(&p, "OpusHead", 8);
>      bytestream_put_byte(&p, 1); /* Version */
> @@ -180,9 +180,9 @@ static int libopus_configure_encoder(AVCodecContext *avctx, OpusMSEncoder *enc,
>  
>  static int libopus_check_max_channels(AVCodecContext *avctx,
>                                        int max_channels) {
> -    if (avctx->channels > max_channels) {
> +    if (avctx->ch_layout.nb_channels > max_channels) {
>          av_log(avctx, AV_LOG_ERROR, "Opus mapping family undefined for %d channels.\n",
> -               avctx->channels);
> +               avctx->ch_layout.nb_channels);
>          return AVERROR(EINVAL);
>      }
>  
> @@ -190,21 +190,26 @@ static int libopus_check_max_channels(AVCodecContext *avctx,
>  }
>  
>  static int libopus_check_vorbis_layout(AVCodecContext *avctx, int mapping_family) {
> -    av_assert2(avctx->channels < FF_ARRAY_ELEMS(ff_vorbis_channel_layouts));
> +    av_assert2(avctx->ch_layout.nb_channels < FF_ARRAY_ELEMS(ff_vorbis_ch_layouts));
>  
> -    if (!avctx->channel_layout) {
> +    if (avctx->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
>          av_log(avctx, AV_LOG_WARNING,
>                 "No channel layout specified. Opus encoder will use Vorbis "
> -               "channel layout for %d channels.\n", avctx->channels);
> -    } else if (avctx->channel_layout != ff_vorbis_channel_layouts[avctx->channels - 1]) {
> -        char name[32];
> -        av_get_channel_layout_string(name, sizeof(name), avctx->channels,
> -                                     avctx->channel_layout);
> -        av_log(avctx, AV_LOG_ERROR,
> -               "Invalid channel layout %s for specified mapping family %d.\n",
> -               name, mapping_family);
> +               "channel layout for %d channels.\n", avctx->ch_layout.nb_channels);
> +    } else {
> +        AVChannelLayout chl;
> +        av_channel_layout_copy(&chl, &ff_vorbis_ch_layouts[avctx->ch_layout.nb_channels - 1]);

chl is garbage, while av_channel_layout_copy() uninits it

is the copy even necessary? Why not pass ff_vorbis_ch_layouts to compare
directly?

-- 
Anton Khirnov