[FFmpeg-devel] [DEVEL][PATCH v2] ffmpeg: fix channel_layout bug on non-default layout

[pkv.stream]

Le 11/11/2017 à 1:07 AM, Michael Niedermayer a écrit :
> On Fri, Nov 10, 2017 at 10:27:48PM +0100, pkv.stream wrote:
>> Le 10/11/2017 à 1:12 AM, Michael Niedermayer a écrit :
>>> On Thu, Nov 09, 2017 at 09:37:33PM +0100, pkv.stream wrote:
>>>> Hi Michael,
>>>>
>>>>>>   ffmpeg_opt.c |   11 ++++++++++-
>>>>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>> 2af07f4366efdfaf1018bb2ea29be672befe0823  0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch
>>>>>>  From 4ec55dc88923108132307b41300a1abddf32e6f7 Mon Sep 17 00:00:00 2001
>>>>>> From: pkviet <pkv.stream at gmail.com>
>>>>>> Date: Mon, 2 Oct 2017 11:14:31 +0200
>>>>>> Subject: [PATCH] ffmpeg: fix channel_layout bug on non-default layout
>>>>>>
>>>>>> Fix for ticket 6706.
>>>>>> The -channel_layout option was not working when the channel layout was not
>>>>>> a default one (ex: for 4 channels, quad was interpreted as 4.0 which is
>>>>>> the default layout for 4 channels; or octagonal interpreted as 7.1).
>>>>>> This led to the spurious auto-insertion of an auto-resampler filter
>>>>>> remapping the channels even if input and output had identical channel
>>>>>> layouts.
>>>>>> The fix operates by directly calling the channel layout if defined in
>>>>>> options. If the layout is undefined, the default layout is selected as
>>>>>> before the fix.
>>>>>> ---
>>>>>>   ffmpeg_opt.c | 11 ++++++++++-
>>>>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
>>>>>> index 100fa76..cf5a63c 100644
>>>>>> --- a/ffmpeg_opt.c
>>>>>> +++ b/ffmpeg_opt.c
>>>>>> @@ -1804,6 +1804,12 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in
>>>>>>           MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st);
>>>>>> +        AVDictionaryEntry *output_layout = av_dict_get(o->g->codec_opts,
>>>>>> +                                                       "channel_layout",
>>>>>> +                                                       NULL, AV_DICT_MATCH_CASE);
>>>>> This doesnt look right
>>>>>
>>>>> not an issue of the patch as such but
>>>>> why is the channel_layout option per file and not per stream?
>>>> just my ignorance; do you mean this is not the right way to retrieve
>>>> the channel_layout option from an audio stream ?
>>> I think there is more buggy with how the channel_layout is handled
>>>
>>> try this:
>>> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -channel_layout 5 test.wav
>>> and this:
>>> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -channel_layout:a 5 test.wav
>>>
>>> while it may appear that the are both working this is deceiving.
>>> I think only the channel number gets actually used in the 2nd case
>>>
>>> Look at what your code with av_dict_get() reads.
>>> It does not obtain the 5 in the 2nd case
>> ok I fixed that by using the flag AV_DICT_IGNORE_SUFFIX ; now
>> -channel_layout:a works as expected.
>> I fixed also all the styling issues you spotted (mixed declarations,
>> {} for if etc).
>>
>> Still not understanding your initial comment though :
>> 'why is the channel_layout option per file and not per stream?'
>>
>> do you mean o->g->codec_opts is not where I should retrieve the
>> channel_layout ? if not where from ?
>>
>> Thanks
>>
>>> [...]
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>   ffmpeg_opt.c |   12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>> 849898d28e989ffa5cc598c6fe4d43847b636132  0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch
>>  From 6d4f1c14135f9473b77e1c649d0e7bbaeba00a50 Mon Sep 17 00:00:00 2001
>> From: pkviet <pkv.stream at gmail.com>
>> Date: Mon, 2 Oct 2017 11:14:31 +0200
>> Subject: [PATCH] ffmpeg: fix channel_layout bug on non-default layout
>>
>> Fix for ticket 6706.
>> The -channel_layout option was not working when the channel layout was not
>> a default one (ex: for 4 channels, quad was interpreted as 4.0 which is
>> the default layout for 4 channels; or octagonal interpreted as 7.1).
>> This led to the spurious auto-insertion of an auto-resampler filter
>> remapping the channels even if input and output had identical channel
>> layouts.
>> The fix operates by directly calling the channel layout if defined in
>> options. If the layout is undefined, the default layout is selected as
>> before the fix.
>> ---
>>   fftools/ffmpeg_opt.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index ca6f10d..cb25d7b 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -1785,6 +1785,7 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in
>>       AVStream *st;
>>       OutputStream *ost;
>>       AVCodecContext *audio_enc;
>> +    AVDictionaryEntry *output_layout;
>>   
>>       ost = new_output_stream(o, oc, AVMEDIA_TYPE_AUDIO, source_index);
>>       st  = ost->st;
>> @@ -1799,7 +1800,10 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in
>>           char *sample_fmt = NULL;
>>   
>>           MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st);
>> -
>> +        output_layout = av_dict_get(o->g->codec_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX);
> i didnt try but i assume
> this would fail if there are 2 audio streams each with different
> layout

ah ok, I understand now; thanks for elaborating.
i tested with two output streams, it's not failing and working correctly.

ex: ffmpeg -channel_layout octagonal -i source.wav -c:a pcm_s16le 
-channel_layout octagonal out1.wav -c:a pcm_s16le -channel_layout quad 
out2.wav

(if I pick layout 0x5 as in your examples it fails because the rematrix 
is not supported, so this is expected behavior although failing; but 
with pan filter it eventually works)

Tested also with two source streams used for two ouputs. Working too.