Re: [FFmpeg-devel] [PATCH] swsresample/swresample: abort on invalid layouts

[James Almer <jamrial@gmail.com>]

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".
_______________________________________________
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".