| Message ID | 20220908224303.3729-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 9903ba28c28ab18dc7b7b6fb8571cc8b5caae1a6 |
| Headers | show |
| Series | [FFmpeg-devel] swsresample/swresample: abort on invalid layouts | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 9/8/2022 7:47 PM, Andreas Rheinhardt wrote: > James Almer: >> If it's unsupported or invalid, then there's no point trying to rebuild it >> using a value that may have been derived from the same layout to begin with. >> >> Move the checks before the attempts at copying the layout while at it. >> >> Fixes ticket #9908. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libswresample/swresample.c | 48 +++++++++++++++++++++++++------------- >> 1 file changed, 32 insertions(+), 16 deletions(-) >> >> diff --git a/libswresample/swresample.c b/libswresample/swresample.c >> index 6f04d130d3..5884f8d533 100644 >> --- a/libswresample/swresample.c >> +++ b/libswresample/swresample.c >> @@ -227,7 +227,7 @@ av_cold int swr_init(struct SwrContext *s){ >> s->in_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >> s->in_ch_layout.nb_channels = s->user_in_ch_count; >> } >> - } else >> + } else if (av_channel_layout_check(&s->user_in_chlayout)) >> av_channel_layout_copy(&s->in_ch_layout, &s->user_in_chlayout); >> >> if ((s->user_out_ch_count && s->user_out_ch_count != s->user_out_chlayout.nb_channels) || >> @@ -240,17 +240,45 @@ av_cold int swr_init(struct SwrContext *s){ >> s->out_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >> s->out_ch_layout.nb_channels = s->user_out_ch_count; >> } >> - } else >> + } else if (av_channel_layout_check(&s->user_out_chlayout)) >> av_channel_layout_copy(&s->out_ch_layout, &s->user_out_chlayout); >> >> if (!s->out.ch_count && !s->user_out_ch_layout) >> s->out.ch_count = s->out_ch_layout.nb_channels; >> if (!s-> in.ch_count && !s-> user_in_ch_layout) >> s-> in.ch_count = s->in_ch_layout.nb_channels; >> + >> + if (!(ret = av_channel_layout_check(&s->in_ch_layout)) || s->in_ch_layout.nb_channels > SWR_CH_MAX) { >> + if (ret) >> + av_channel_layout_describe(&s->in_ch_layout, l1, sizeof(l1)); >> + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", ret ? l1 : ""); >> + return AVERROR(EINVAL); >> + } >> + >> + if (!(ret = av_channel_layout_check(&s->out_ch_layout)) || s->out_ch_layout.nb_channels > SWR_CH_MAX) { >> + if (ret) >> + av_channel_layout_describe(&s->out_ch_layout, l2, sizeof(l2)); >> + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", ret ? l2 : ""); >> + return AVERROR(EINVAL); >> + } >> #else >> s->out.ch_count = s-> user_out_chlayout.nb_channels; >> s-> in.ch_count = s-> user_in_chlayout.nb_channels; >> >> + if (!(ret = av_channel_layout_check(&s->user_in_chlayout)) || s->user_in_chlayout.nb_channels > SWR_CH_MAX) { >> + if (ret) >> + av_channel_layout_describe(&s->user_in_chlayout, l1, sizeof(l1)); >> + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", ret ? l1 : ""); >> + return AVERROR(EINVAL); >> + } >> + >> + if (!(ret = av_channel_layout_check(&s->user_out_chlayout)) || s->user_out_chlayout.nb_channels > SWR_CH_MAX) { >> + if (ret) >> + av_channel_layout_describe(&s->user_out_chlayout, l2, sizeof(l2)); >> + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", ret ? l2 : ""); > > Why are you using AV_LOG_WARNING when you are erroring out? > >> + return AVERROR(EINVAL); >> + } >> + >> ret = av_channel_layout_copy(&s->in_ch_layout, &s->user_in_chlayout); >> ret |= av_channel_layout_copy(&s->out_ch_layout, &s->user_out_chlayout); >> if (ret < 0) >> @@ -261,18 +289,6 @@ av_cold int swr_init(struct SwrContext *s){ >> >> s->dither.method = s->user_dither_method; >> >> - if (!av_channel_layout_check(&s->in_ch_layout) || s->in_ch_layout.nb_channels > SWR_CH_MAX) { >> - av_channel_layout_describe(&s->in_ch_layout, l1, sizeof(l1)); >> - av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", l1); >> - av_channel_layout_uninit(&s->in_ch_layout); >> - } >> - >> - if (!av_channel_layout_check(&s->out_ch_layout) || s->out_ch_layout.nb_channels > SWR_CH_MAX) { >> - av_channel_layout_describe(&s->out_ch_layout, l2, sizeof(l2)); >> - av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", l2); >> - av_channel_layout_uninit(&s->out_ch_layout); >> - } >> - >> switch(s->engine){ >> #if CONFIG_LIBSOXR >> case SWR_ENGINE_SOXR: s->resampler = &swri_soxr_resampler; break; >> @@ -291,9 +307,9 @@ av_cold int swr_init(struct SwrContext *s){ >> av_channel_layout_uninit(&s->in_ch_layout); >> } >> >> - if (!s->in_ch_layout.nb_channels || s->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >> + if (s->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >> av_channel_layout_default(&s->in_ch_layout, s->used_ch_count); >> - if (!s->out_ch_layout.nb_channels || s->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >> + if (s->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >> av_channel_layout_default(&s->out_ch_layout, s->out.ch_count); >> >> s->rematrix = av_channel_layout_compare(&s->out_ch_layout, &s->in_ch_layout) || > > It seems that you are not aborting; you are instead erroring out. I'll amend the commit message. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 9/8/2022 8:00 PM, James Almer wrote: > On 9/8/2022 7:47 PM, Andreas Rheinhardt wrote: >> James Almer: >>> If it's unsupported or invalid, then there's no point trying to >>> rebuild it >>> using a value that may have been derived from the same layout to >>> begin with. >>> >>> Move the checks before the attempts at copying the layout while at it. >>> >>> Fixes ticket #9908. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libswresample/swresample.c | 48 +++++++++++++++++++++++++------------- >>> 1 file changed, 32 insertions(+), 16 deletions(-) >>> >>> diff --git a/libswresample/swresample.c b/libswresample/swresample.c >>> index 6f04d130d3..5884f8d533 100644 >>> --- a/libswresample/swresample.c >>> +++ b/libswresample/swresample.c >>> @@ -227,7 +227,7 @@ av_cold int swr_init(struct SwrContext *s){ >>> s->in_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >>> s->in_ch_layout.nb_channels = s->user_in_ch_count; >>> } >>> - } else >>> + } else if (av_channel_layout_check(&s->user_in_chlayout)) >>> av_channel_layout_copy(&s->in_ch_layout, >>> &s->user_in_chlayout); >>> if ((s->user_out_ch_count && s->user_out_ch_count != >>> s->user_out_chlayout.nb_channels) || >>> @@ -240,17 +240,45 @@ av_cold int swr_init(struct SwrContext *s){ >>> s->out_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >>> s->out_ch_layout.nb_channels = s->user_out_ch_count; >>> } >>> - } else >>> + } else if (av_channel_layout_check(&s->user_out_chlayout)) >>> av_channel_layout_copy(&s->out_ch_layout, >>> &s->user_out_chlayout); >>> if (!s->out.ch_count && !s->user_out_ch_layout) >>> s->out.ch_count = s->out_ch_layout.nb_channels; >>> if (!s-> in.ch_count && !s-> user_in_ch_layout) >>> s-> in.ch_count = s->in_ch_layout.nb_channels; >>> + >>> + if (!(ret = av_channel_layout_check(&s->in_ch_layout)) || >>> s->in_ch_layout.nb_channels > SWR_CH_MAX) { >>> + if (ret) >>> + av_channel_layout_describe(&s->in_ch_layout, l1, >>> sizeof(l1)); >>> + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is >>> invalid or unsupported.\n", ret ? l1 : ""); >>> + return AVERROR(EINVAL); >>> + } >>> + >>> + if (!(ret = av_channel_layout_check(&s->out_ch_layout)) || >>> s->out_ch_layout.nb_channels > SWR_CH_MAX) { >>> + if (ret) >>> + av_channel_layout_describe(&s->out_ch_layout, l2, >>> sizeof(l2)); >>> + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is >>> invalid or unsupported.\n", ret ? l2 : ""); >>> + return AVERROR(EINVAL); >>> + } >>> #else >>> s->out.ch_count = s-> user_out_chlayout.nb_channels; >>> s-> in.ch_count = s-> user_in_chlayout.nb_channels; >>> + if (!(ret = av_channel_layout_check(&s->user_in_chlayout)) || >>> s->user_in_chlayout.nb_channels > SWR_CH_MAX) { >>> + if (ret) >>> + av_channel_layout_describe(&s->user_in_chlayout, l1, >>> sizeof(l1)); >>> + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is >>> invalid or unsupported.\n", ret ? l1 : ""); >>> + return AVERROR(EINVAL); >>> + } >>> + >>> + if (!(ret = av_channel_layout_check(&s->user_out_chlayout)) || >>> s->user_out_chlayout.nb_channels > SWR_CH_MAX) { >>> + if (ret) >>> + av_channel_layout_describe(&s->user_out_chlayout, l2, >>> sizeof(l2)); >>> + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is >>> invalid or unsupported.\n", ret ? l2 : ""); >> >> Why are you using AV_LOG_WARNING when you are erroring out? >> >>> + return AVERROR(EINVAL); >>> + } >>> + >>> ret = av_channel_layout_copy(&s->in_ch_layout, >>> &s->user_in_chlayout); >>> ret |= av_channel_layout_copy(&s->out_ch_layout, >>> &s->user_out_chlayout); >>> if (ret < 0) >>> @@ -261,18 +289,6 @@ av_cold int swr_init(struct SwrContext *s){ >>> s->dither.method = s->user_dither_method; >>> - if (!av_channel_layout_check(&s->in_ch_layout) || >>> s->in_ch_layout.nb_channels > SWR_CH_MAX) { >>> - av_channel_layout_describe(&s->in_ch_layout, l1, sizeof(l1)); >>> - av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is >>> invalid or unsupported.\n", l1); >>> - av_channel_layout_uninit(&s->in_ch_layout); >>> - } >>> - >>> - if (!av_channel_layout_check(&s->out_ch_layout) || >>> s->out_ch_layout.nb_channels > SWR_CH_MAX) { >>> - av_channel_layout_describe(&s->out_ch_layout, l2, sizeof(l2)); >>> - av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is >>> invalid or unsupported.\n", l2); >>> - av_channel_layout_uninit(&s->out_ch_layout); >>> - } >>> - >>> switch(s->engine){ >>> #if CONFIG_LIBSOXR >>> case SWR_ENGINE_SOXR: s->resampler = &swri_soxr_resampler; >>> break; >>> @@ -291,9 +307,9 @@ av_cold int swr_init(struct SwrContext *s){ >>> av_channel_layout_uninit(&s->in_ch_layout); >>> } >>> - if (!s->in_ch_layout.nb_channels || s->in_ch_layout.order == >>> AV_CHANNEL_ORDER_UNSPEC) >>> + if (s->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >>> av_channel_layout_default(&s->in_ch_layout, s->used_ch_count); >>> - if (!s->out_ch_layout.nb_channels || s->out_ch_layout.order == >>> AV_CHANNEL_ORDER_UNSPEC) >>> + if (s->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) >>> av_channel_layout_default(&s->out_ch_layout, s->out.ch_count); >>> s->rematrix = av_channel_layout_compare(&s->out_ch_layout, >>> &s->in_ch_layout) || >> >> It seems that you are not aborting; you are instead erroring out. > > I'll amend the commit message. Will apply.
diff --git a/libswresample/swresample.c b/libswresample/swresample.c index 6f04d130d3..5884f8d533 100644 --- a/libswresample/swresample.c +++ b/libswresample/swresample.c @@ -227,7 +227,7 @@ av_cold int swr_init(struct SwrContext *s){ s->in_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; s->in_ch_layout.nb_channels = s->user_in_ch_count; } - } else + } else if (av_channel_layout_check(&s->user_in_chlayout)) av_channel_layout_copy(&s->in_ch_layout, &s->user_in_chlayout); if ((s->user_out_ch_count && s->user_out_ch_count != s->user_out_chlayout.nb_channels) || @@ -240,17 +240,45 @@ av_cold int swr_init(struct SwrContext *s){ s->out_ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; s->out_ch_layout.nb_channels = s->user_out_ch_count; } - } else + } else if (av_channel_layout_check(&s->user_out_chlayout)) av_channel_layout_copy(&s->out_ch_layout, &s->user_out_chlayout); if (!s->out.ch_count && !s->user_out_ch_layout) s->out.ch_count = s->out_ch_layout.nb_channels; if (!s-> in.ch_count && !s-> user_in_ch_layout) s-> in.ch_count = s->in_ch_layout.nb_channels; + + if (!(ret = av_channel_layout_check(&s->in_ch_layout)) || s->in_ch_layout.nb_channels > SWR_CH_MAX) { + if (ret) + av_channel_layout_describe(&s->in_ch_layout, l1, sizeof(l1)); + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", ret ? l1 : ""); + return AVERROR(EINVAL); + } + + if (!(ret = av_channel_layout_check(&s->out_ch_layout)) || s->out_ch_layout.nb_channels > SWR_CH_MAX) { + if (ret) + av_channel_layout_describe(&s->out_ch_layout, l2, sizeof(l2)); + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", ret ? l2 : ""); + return AVERROR(EINVAL); + } #else s->out.ch_count = s-> user_out_chlayout.nb_channels; s-> in.ch_count = s-> user_in_chlayout.nb_channels; + if (!(ret = av_channel_layout_check(&s->user_in_chlayout)) || s->user_in_chlayout.nb_channels > SWR_CH_MAX) { + if (ret) + av_channel_layout_describe(&s->user_in_chlayout, l1, sizeof(l1)); + av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", ret ? l1 : ""); + return AVERROR(EINVAL); + } + + if (!(ret = av_channel_layout_check(&s->user_out_chlayout)) || s->user_out_chlayout.nb_channels > SWR_CH_MAX) { + if (ret) + av_channel_layout_describe(&s->user_out_chlayout, l2, sizeof(l2)); + av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", ret ? l2 : ""); + return AVERROR(EINVAL); + } + ret = av_channel_layout_copy(&s->in_ch_layout, &s->user_in_chlayout); ret |= av_channel_layout_copy(&s->out_ch_layout, &s->user_out_chlayout); if (ret < 0) @@ -261,18 +289,6 @@ av_cold int swr_init(struct SwrContext *s){ s->dither.method = s->user_dither_method; - if (!av_channel_layout_check(&s->in_ch_layout) || s->in_ch_layout.nb_channels > SWR_CH_MAX) { - av_channel_layout_describe(&s->in_ch_layout, l1, sizeof(l1)); - av_log(s, AV_LOG_WARNING, "Input channel layout \"%s\" is invalid or unsupported.\n", l1); - av_channel_layout_uninit(&s->in_ch_layout); - } - - if (!av_channel_layout_check(&s->out_ch_layout) || s->out_ch_layout.nb_channels > SWR_CH_MAX) { - av_channel_layout_describe(&s->out_ch_layout, l2, sizeof(l2)); - av_log(s, AV_LOG_WARNING, "Output channel layout \"%s\" is invalid or unsupported.\n", l2); - av_channel_layout_uninit(&s->out_ch_layout); - } - switch(s->engine){ #if CONFIG_LIBSOXR case SWR_ENGINE_SOXR: s->resampler = &swri_soxr_resampler; break; @@ -291,9 +307,9 @@ av_cold int swr_init(struct SwrContext *s){ av_channel_layout_uninit(&s->in_ch_layout); } - if (!s->in_ch_layout.nb_channels || s->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) + if (s->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) av_channel_layout_default(&s->in_ch_layout, s->used_ch_count); - if (!s->out_ch_layout.nb_channels || s->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) + if (s->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) av_channel_layout_default(&s->out_ch_layout, s->out.ch_count); s->rematrix = av_channel_layout_compare(&s->out_ch_layout, &s->in_ch_layout) ||
If it's unsupported or invalid, then there's no point trying to rebuild it using a value that may have been derived from the same layout to begin with. Move the checks before the attempts at copying the layout while at it. Fixes ticket #9908. Signed-off-by: James Almer <jamrial@gmail.com> --- libswresample/swresample.c | 48 +++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 16 deletions(-)