Message ID | 19c44254-1279-b685-3f46-0f03b2fa7a07@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Sun, Nov 12, 2017 at 06:26:18PM +0100, pkv.stream wrote: > 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@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@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@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@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 > > > >[...] > > > > ok should be fixed with this new patch version. > Changed > > o->g->codec_opts with ost->encoder_opts > > Now it's working correctly with two streams. > Regards > > > > > >_______________________________________________ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > ffmpeg_opt.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > a7c71fb1ccd7d91b61033be70dfd324b4e3f3c34 0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch > From fb7f7f6e01cc242e13d0e640f583dffe6e7a8ada Mon Sep 17 00:00:00 2001 > From: pkviet <pkv.stream@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..8941d66 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(ost->encoder_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX); > + if (output_layout) { > + audio_enc->channel_layout = strtoull(output_layout->value, NULL, 10); > + } why is this handled different from audio_channels ? that is why is this not using MATCH_PER_STREAM_OPT() ? (yes this would require some changes but i wonder why this would be handled differently) [...]
Le 14/11/2017 à 1:13 PM, Michael Niedermayer a écrit : > On Sun, Nov 12, 2017 at 06:26:18PM +0100, pkv.stream wrote: >> 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@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@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@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@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 >>> >>> [...] >>> >> ok should be fixed with this new patch version. >> Changed >> >> o->g->codec_opts with ost->encoder_opts >> >> Now it's working correctly with two streams. >> Regards >> >> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> ffmpeg_opt.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> a7c71fb1ccd7d91b61033be70dfd324b4e3f3c34 0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch >> From fb7f7f6e01cc242e13d0e640f583dffe6e7a8ada Mon Sep 17 00:00:00 2001 >> From: pkviet<pkv.stream@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..8941d66 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(ost->encoder_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX); >> + if (output_layout) { >> + audio_enc->channel_layout = strtoull(output_layout->value, NULL, 10); >> + } > why is this handled different from audio_channels ? > that is why is this not using MATCH_PER_STREAM_OPT() ? > (yes this would require some changes but i wonder why this would be > handled differently) Hi I did try to use the MATCH_PER_STREAM_OPT() but didn't manage to have it working. Also I was a bit hesitant to modify the OptionsContext struct, and preferred something minimal. If you think this can definitely be made to work without too much coding and prefer such a solution, I'll retry working on a MATCH_PER_STREAM_OPT() solution. Thanks
On Tue, Nov 14, 2017 at 11:49:26PM +0100, pkv.stream wrote: > Le 14/11/2017 à 1:13 PM, Michael Niedermayer a écrit : > >On Sun, Nov 12, 2017 at 06:26:18PM +0100, pkv.stream wrote: > >>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@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@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@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@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 > >>> > >>>[...] > >>> > >>ok should be fixed with this new patch version. > >>Changed > >> > >>o->g->codec_opts with ost->encoder_opts > >> > >>Now it's working correctly with two streams. > >>Regards > >> > >> > >>>_______________________________________________ > >>>ffmpeg-devel mailing list > >>>ffmpeg-devel@ffmpeg.org > >>>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> ffmpeg_opt.c | 12 ++++++++++-- > >> 1 file changed, 10 insertions(+), 2 deletions(-) > >>a7c71fb1ccd7d91b61033be70dfd324b4e3f3c34 0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch > >> From fb7f7f6e01cc242e13d0e640f583dffe6e7a8ada Mon Sep 17 00:00:00 2001 > >>From: pkviet<pkv.stream@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..8941d66 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(ost->encoder_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX); > >>+ if (output_layout) { > >>+ audio_enc->channel_layout = strtoull(output_layout->value, NULL, 10); > >>+ } > >why is this handled different from audio_channels ? > >that is why is this not using MATCH_PER_STREAM_OPT() ? > >(yes this would require some changes but i wonder why this would be > > handled differently) > Hi > I did try to use the MATCH_PER_STREAM_OPT() but didn't manage to > have it working. Also I was a bit hesitant to modify the > OptionsContext struct, and preferred something minimal. > If you think this can definitely be made to work without too much > coding and prefer such a solution, I'll retry working on a > MATCH_PER_STREAM_OPT() solution. i dont really know if it "can definitely be made to work without too much coding", it just seemed odd how this is handled differently [...]
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index ca6f10d..8941d66 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(ost->encoder_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX); + if (output_layout) { + audio_enc->channel_layout = strtoull(output_layout->value, NULL, 10); + } MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st); if (sample_fmt && (audio_enc->sample_fmt = av_get_sample_fmt(sample_fmt)) == AV_SAMPLE_FMT_NONE) { @@ -2523,7 +2527,11 @@ loop_end: (count + 1) * sizeof(*f->sample_rates)); } if (ost->enc_ctx->channels) { - f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels); + if (ost->enc_ctx->channel_layout) { + f->channel_layout = ost->enc_ctx->channel_layout; + } else { + f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels); + } } else if (ost->enc->channel_layouts) { count = 0; while (ost->enc->channel_layouts[count])