[FFmpeg-devel] [DEVEL][PATCH v2] ffmpeg: fix channel_layout bug on non-default layout
pkv.stream
pkv.stream at gmail.com
Sun Nov 12 18:52:37 EET 2017
Le 12/11/2017 à 5:38 PM, Michael Niedermayer a écrit :
> On Sun, Nov 12, 2017 at 05:07:04PM +0100, Kv Pham wrote:
>> Le 12 nov. 2017 5:01 PM, "Michael Niedermayer" <michael at niedermayer.cc> a
>> écrit :
>>
>> On Sat, Nov 11, 2017 at 02:15:25AM +0100, pkv.stream wrote:
>>> 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.
>> have you tried with different layouts for each stream ?
>>
>>
>> Yes.
>> Different layouts for input streams as well as output streams.
>>
>> Other tests to do ?
> just test with 2 streams:
>
> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -map 0:a -map 0:a -channel_layout:a:0 quad -channel_layout:a:1 5.1 -y test.nut
>
> av_dict_get() will likely return the same layout twice instead of the
> correct layout for each stream
you're right.
suggestions welcome.
thanks
>
> [...]
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list