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

[Michael Niedermayer]

On Thu, Oct 05, 2017 at 12:37:13AM +0200, pkv.stream wrote:
> Le 04/10/2017 à 11:18 PM, Moritz Barsnick a écrit :
> >On Mon, Oct 02, 2017 at 21:50:50 +0200, pkv.stream wrote:
> >>      if (!ost->stream_copy) {
> >>-        char *sample_fmt = NULL;
> >>+
> >>+		char *sample_fmt = NULL;
> >This is very obviously a patch which will not be accepted.
> >
> >
> >>-        MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
> >>+		AVDictionaryEntry *output_layout = av_dict_get(o->g->codec_opts, "channel_layout",NULL, AV_DICT_MATCH_CASE);
> >>+        if (output_layout)
> >>+            ost->enc_ctx->channel_layout = strtoull(output_layout->value, NULL, 10);
> >>+
> >>+		MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
> >Your indentation is totally wrong, and makes use of tabs. Please follow
> >the ffmpeg style.
> 
> styling fixed
> thanks for your input
> 
> 

>  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?

also currently mixed statemts and declarations arent allowed


> +        if (output_layout)
> +            ost->enc_ctx->channel_layout = strtoull(output_layout->value, NULL, 10);

audio_enc


> +
>          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) {
> @@ -2524,7 +2530,10 @@ 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);

if () {
} else

is better as tat way future patches do not need to change existing
lines maing patches more readable if ever a line is added

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20171109/ab604c48/attachment.sig>