[FFmpeg-devel] [PATCH] libswresample: Use channel count if channel layout is undefined

Tobias Rapp t.rapp at noa-archive.com
Fri Jul 20 10:51:49 EEST 2018


On 19.07.2018 23:37, Michael Niedermayer wrote:
> On Thu, Jul 19, 2018 at 01:53:09PM +0200, Tobias Rapp wrote:
>> On 18.07.2018 19:31, Marcin Gorzel wrote:
>>> Rematrixing supports up to 64 channels. However, there is only a limited number of channel layouts defined. Since the in/out channel count is obtained from the channel layout, for undefined layouts (e.g. for 9, 10, 11 channels etc.) the rematrixing fails.
>>>
>>> In ticket #6790 the problem has been partially addressed by applying a patch to swr_set_matrix() in rematrix.c:72.
>>> However, a similar change must be also applied to swri_rematrix_init() in rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36.
>>> ---
>>>   libswresample/rematrix.c          | 6 ++++--
>>>   libswresample/x86/rematrix_init.c | 6 ++++--
>>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
>>> index 8227730056..d1a0cfe7f8 100644
>>> --- a/libswresample/rematrix.c
>>> +++ b/libswresample/rematrix.c
>>> @@ -384,8 +384,10 @@ av_cold static int auto_matrix(SwrContext *s)
>>>   av_cold int swri_rematrix_init(SwrContext *s){
>>>       int i, j;
>>> -    int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
>>> -    int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
>>> +    int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
>>> +        av_get_channel_layout_nb_channels(s->in_ch_layout);
>>> +    int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
>>> +        av_get_channel_layout_nb_channels(s->out_ch_layout);
>>>       s->mix_any_f = NULL;
>>> diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c
>>> index d71b41a73e..4f9c92e4ab 100644
>>> --- a/libswresample/x86/rematrix_init.c
>>> +++ b/libswresample/x86/rematrix_init.c
>>> @@ -33,8 +33,10 @@ D(int16, sse2)
>>>   av_cold int swri_rematrix_init_x86(struct SwrContext *s){
>>>   #if HAVE_X86ASM
>>>       int mm_flags = av_get_cpu_flags();
>>> -    int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
>>> -    int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
>>> +    int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
>>> +        av_get_channel_layout_nb_channels(s->in_ch_layout);
>>> +    int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
>>> +        av_get_channel_layout_nb_channels(s->out_ch_layout);
>>>       int num    = nb_in * nb_out;
>>>       int i,j;
>>>
>>
>> Patch looks good to me, except that the title should be updated to reflect
>> the new logic. I suggest something like "swresample: Use channel count for
>> rematrix initialization if defined".
> 
> The patch does not look good to me
> There should be a field that represents the count of channels.
> No conditional should be needed
> 
> please check that every field is wrong in at least some case.
> If true a new field must be added or a existing one needs to be set
> differently
> But there should be a field that represents the channel count.

If I understand correctly you say that the fall-back to 
av_get_channel_layout_nb_channels() is not needed here as both functions 
are internal and called only as part of swr_init() so we may safely 
assume that the in/out.ch_count fields have been initialized before this 
code is reached.

Fine with me. Marcin, would you update the patch once more? And there 
are some additional FATE tests for the pan filter that I can post once 
this patch is through, if you haven't made up your own tests.

Regards,
Tobias



More information about the ffmpeg-devel mailing list