[FFmpeg-devel] [DEVEL][PATCH v3] ffmpeg: fix channel_layout bug on non-default layout
Michael Niedermayer
michael at niedermayer.cc
Tue Nov 14 14:13:55 EET 2017
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 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
> >
> >[...]
> >
>
> 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 at 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 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..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)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171114/87d76a6f/attachment.sig>
More information about the ffmpeg-devel
mailing list