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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- 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/20171111/4e159362/attachment.sig>