| Message ID | GV1P250MB07376D82B1F1C02E332FBBA38F489@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/decode: Check for more invalid channel counts | expand |
| 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 |
On 9/15/2022 10:10 PM, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Maybe use av_channel_layout_check? > > libavcodec/decode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 2961705c9d..e24005cc44 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -1595,7 +1595,7 @@ FF_DISABLE_DEPRECATION_WARNINGS > FF_ENABLE_DEPRECATION_WARNINGS > #endif > > - if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && avctx->ch_layout.nb_channels == 0 && > + 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); This is a AV_CODEC_CAP_CHANNEL_CONF specific check to see if a channel count is set. A general sanity check like < 0 should be in avcodec_open2() before the ff_decode_preinit() call, next to (or as part of) the similar FF_SANE_NB_CHANNELS check, IMO.
On 9/15/2022 10:16 PM, James Almer wrote: > On 9/15/2022 10:10 PM, Andreas Rheinhardt wrote: >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> Maybe use av_channel_layout_check? >> >> libavcodec/decode.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >> index 2961705c9d..e24005cc44 100644 >> --- a/libavcodec/decode.c >> +++ b/libavcodec/decode.c >> @@ -1595,7 +1595,7 @@ FF_DISABLE_DEPRECATION_WARNINGS >> FF_ENABLE_DEPRECATION_WARNINGS >> #endif >> - if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && >> avctx->ch_layout.nb_channels == 0 && >> + 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); > > This is a AV_CODEC_CAP_CHANNEL_CONF specific check to see if a channel > count is set. A general sanity check like < 0 should be in > avcodec_open2() before the ff_decode_preinit() call, next to (or as part > of) the similar FF_SANE_NB_CHANNELS check, IMO. On second thought, maybe the early FF_SANE_NB_CHANNELS check should be removed from avcodec_open2() and av_channel_layout_check() used here for non AV_CODEC_CAP_CHANNEL_CONF decoders like you suggested above. For the rest, whatever value is set is going to be ignored, overwritten, and then verified after the decoder was initialized.
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 2961705c9d..e24005cc44 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1595,7 +1595,7 @@ FF_DISABLE_DEPRECATION_WARNINGS FF_ENABLE_DEPRECATION_WARNINGS #endif - if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && avctx->ch_layout.nb_channels == 0 && + 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);
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Maybe use av_channel_layout_check? libavcodec/decode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)