diff mbox series

[FFmpeg-devel,03/17] avcodec/avcodec: Check for more invalid channel layouts

Message ID GV1P250MB0737CD3DCCBE67BA3CB2177A8F4A9@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,01/17] avcodec/avcodec: Uninitialize AVChannelLayout before overwriting it | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 18, 2022, 8:27 p.m. UTC 
In particular, check the provided channel layout for encoders
without AVCodec.ch_layouts set. This fixes an infinite loop
in the WavPack encoder (and maybe other issues in other encoders
as well) in case the channel count is zero.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/avcodec.c | 11 +++++++++++
 libavcodec/decode.c  |  5 -----
 libavcodec/encode.c  |  5 -----
 3 files changed, 11 insertions(+), 10 deletions(-)

Comments

James Almer Sept. 20, 2022, 10:47 p.m. UTC | #1 
On 9/18/2022 5:27 PM, Andreas Rheinhardt wrote:
> In particular, check the provided channel layout for encoders
> without AVCodec.ch_layouts set. This fixes an infinite loop
> in the WavPack encoder (and maybe other issues in other encoders
> as well) in case the channel count is zero.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/avcodec.c | 11 +++++++++++
>   libavcodec/decode.c  |  5 -----
>   libavcodec/encode.c  |  5 -----
>   3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index a165cdea95..96b69e0a17 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -246,6 +246,17 @@ FF_DISABLE_DEPRECATION_WARNINGS
>   FF_ENABLE_DEPRECATION_WARNINGS
>   #endif
>   
> +    if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && !avctx->ch_layout.nb_channels
> +        && !(codec->capabilities & AV_CODEC_CAP_CHANNEL_CONF)) {

nit: Might be a good idea to add a comment that this decoder-only flag 
is being checked here even for encoders as it will be a no-op for those.

> +        av_log(avctx, AV_LOG_ERROR, "Codec requires channel layout to be set\n");

const char *codec_string = av_codec_is_encoder(codec) ? "Encoder" : 
"Decoder";
av_log(avctx, AV_LOG_ERROR, "%s requires channel layout to be set\n", 
codec_string);

> +        ret = AVERROR(EINVAL);
> +        goto free_and_end;
> +    }
> +    if (avctx->ch_layout.nb_channels && !av_channel_layout_check(&avctx->ch_layout)) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid channel layout\n");
> +        ret = AVERROR(EINVAL);
> +        goto free_and_end;
> +    }
>       if (avctx->ch_layout.nb_channels > FF_SANE_NB_CHANNELS) {
>           av_log(avctx, AV_LOG_ERROR, "Too many channels: %d\n", avctx->ch_layout.nb_channels);
>           ret = AVERROR(EINVAL);
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 2961705c9d..6be2d3d6ed 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1595,11 +1595,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
>   FF_ENABLE_DEPRECATION_WARNINGS
>   #endif
>   
> -    if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && avctx->ch_layout.nb_channels == 0 &&
> -        !(avctx->codec->capabilities & AV_CODEC_CAP_CHANNEL_CONF)) {
> -        av_log(avctx, AV_LOG_ERROR, "Decoder requires channel count but channels not set\n");
> -        return AVERROR(EINVAL);
> -    }
>       if (avctx->codec->max_lowres < avctx->lowres || avctx->lowres < 0) {
>           av_log(avctx, AV_LOG_WARNING, "The maximum value for lowres supported by the decoder is %d\n",
>                  avctx->codec->max_lowres);
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 2168b88ea8..92e8337227 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -600,11 +600,6 @@ static int encode_preinit_audio(AVCodecContext *avctx)
>           return AVERROR(EINVAL);
>       }
>       if (avctx->codec->ch_layouts) {
> -        if (!av_channel_layout_check(&avctx->ch_layout)) {
> -            av_log(avctx, AV_LOG_WARNING, "Channel layout not specified correctly\n");
> -            return AVERROR(EINVAL);
> -        }
> -
>           for (i = 0; avctx->codec->ch_layouts[i].nb_channels; i++) {
>               if (!av_channel_layout_compare(&avctx->ch_layout, &avctx->codec->ch_layouts[i]))
>                   break;
diff mbox series

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index a165cdea95..96b69e0a17 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -246,6 +246,17 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
+    if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && !avctx->ch_layout.nb_channels
+        && !(codec->capabilities & AV_CODEC_CAP_CHANNEL_CONF)) {
+        av_log(avctx, AV_LOG_ERROR, "Codec requires channel layout to be set\n");
+        ret = AVERROR(EINVAL);
+        goto free_and_end;
+    }
+    if (avctx->ch_layout.nb_channels && !av_channel_layout_check(&avctx->ch_layout)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid channel layout\n");
+        ret = AVERROR(EINVAL);
+        goto free_and_end;
+    }
     if (avctx->ch_layout.nb_channels > FF_SANE_NB_CHANNELS) {
         av_log(avctx, AV_LOG_ERROR, "Too many channels: %d\n", avctx->ch_layout.nb_channels);
         ret = AVERROR(EINVAL);
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 2961705c9d..6be2d3d6ed 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1595,11 +1595,6 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-    if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && avctx->ch_layout.nb_channels == 0 &&
-        !(avctx->codec->capabilities & AV_CODEC_CAP_CHANNEL_CONF)) {
-        av_log(avctx, AV_LOG_ERROR, "Decoder requires channel count but channels not set\n");
-        return AVERROR(EINVAL);
-    }
     if (avctx->codec->max_lowres < avctx->lowres || avctx->lowres < 0) {
         av_log(avctx, AV_LOG_WARNING, "The maximum value for lowres supported by the decoder is %d\n",
                avctx->codec->max_lowres);
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 2168b88ea8..92e8337227 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -600,11 +600,6 @@  static int encode_preinit_audio(AVCodecContext *avctx)
         return AVERROR(EINVAL);
     }
     if (avctx->codec->ch_layouts) {
-        if (!av_channel_layout_check(&avctx->ch_layout)) {
-            av_log(avctx, AV_LOG_WARNING, "Channel layout not specified correctly\n");
-            return AVERROR(EINVAL);
-        }
-
         for (i = 0; avctx->codec->ch_layouts[i].nb_channels; i++) {
             if (!av_channel_layout_compare(&avctx->ch_layout, &avctx->codec->ch_layouts[i]))
                 break;