[FFmpeg-devel] [DEVEL][PATCH v2] ffmpeg: fix channel_layout bug on non-default layout
Michael Niedermayer
michael at niedermayer.cc
Sun Nov 12 18:38:27 EET 2017
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
There will always be a question for which you do not know the correct answer.
-------------- 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/20171112/95cabc66/attachment.sig>
More information about the ffmpeg-devel
mailing list