diff mbox

[FFmpeg-devel,DEVEL] ffmpeg: Fix channel_layout bug on non-default layout

Message ID 77677cb0-5615-6de6-da8b-069c24819e3b@gmail.com
State New
Headers show

Commit Message

pkv.stream Feb. 26, 2018, 2:53 a.m. UTC 
Hi Michael,

this is a ping. You had reviewed earlier versions of the patch but had 
left the latest version without comments (3 months ago).

This is a patch for ticket 6706 :

The -channel_layout option is not working when the channel layout is not
a default one (ex: for 4 channels, quad is interpreted as 4.0 which is
the default layout for 4 channels; or octagonal interpreted as 7.1).
This leads to the spurious auto-insertion of an auto-resampler filter
remapping the channels even if input and output have identical channel
layouts.

I had two solutions for solving the bug:

1) adding a new option (channelmask) which can store channel_layout as 
an offset in a SpecifierOpt ;

2) or having channel_layout option use both offset .off and .func.

You suggested the second solution. But I found out it requires to add a 
function in cmdutils in order to parse correctly the duplicated option.

IMO solution (1) is simpler.

I attach again the first solution  (one patch) as well as the second 
(two patches) for your advisement.

The patch passes fate.

thanks.
From ecd706479f033e91001da4ddb699baf4f3440caa Mon Feb 26 02:33:20 2018
From: pkviet <pkv.stream@gmail.com>
Date: Mon, 26 Feb 2018 02:33:20 +0100
Subject: [PATCH] ffmpeg: Fix regression for channel_layout option

Fix for Ticket 6706.
Fix regression with channel_layout option which is not passed
correctly from output streams to filters when the channel layout is not
a default one.

Signed-off-by: pkviet <pkv.stream@gmail.com>
---
 fftools/ffmpeg.h     |  2 ++
 fftools/ffmpeg_opt.c | 38 +++++++++++++++++++++++++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)
diff mbox

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 8195f73e8b..c2af200023 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -107,6 +107,8 @@  typedef struct OptionsContext {
     int        nb_audio_channels;
     SpecifierOpt *audio_sample_rate;
     int        nb_audio_sample_rate;
+    SpecifierOpt *channel_layouts;
+    int        nb_channel_layouts;
     SpecifierOpt *frame_rates;
     int        nb_frame_rates;
     SpecifierOpt *frame_sizes;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 1b591d9695..c9384242ca 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1816,6 +1816,7 @@  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);
+        MATCH_PER_STREAM_OPT(channel_layouts, ui64, audio_enc->channel_layout, oc, st);
 
         MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
         if (sample_fmt &&
@@ -2448,7 +2449,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])
@@ -3024,8 +3029,8 @@  static int opt_channel_layout(void *optctx, const char *opt, const char *arg)
     OptionsContext *o = optctx;
     char layout_str[32];
     char *stream_str;
-    char *ac_str;
-    int ret, channels, ac_str_size;
+    char *ac_str, *cm_str;
+    int ret, channels, ac_str_size, stream_str_size;
     uint64_t layout;
 
     layout = av_get_channel_layout(arg);
@@ -3037,12 +3042,31 @@  static int opt_channel_layout(void *optctx, const char *opt, const char *arg)
     ret = opt_default_new(o, opt, layout_str);
     if (ret < 0)
         return ret;
+    stream_str = strchr(opt, ':');
+    stream_str_size = (stream_str ? strlen(stream_str) : 0);
+    /* Set 'channelmask' option which stores channel_layout as bitmask
+     * (uint64) in SpecifierOpt, enabling access to channel_layout through
+     * MATCH_PER_STREAM_OPT.
+     */
+    ac_str_size = 12 + stream_str_size;
+    ac_str = av_mallocz(ac_str_size);
+    if (!ac_str) {
+        return AVERROR(ENOMEM);
+    }
+    av_strlcpy(ac_str, "channelmask", 12);
+    if (stream_str) {
+        av_strlcat(ac_str, stream_str, ac_str_size);
+    }
+    ret = parse_option(o, ac_str, layout_str, options);
+    av_free(ac_str);
+    if (ret < 0) {
+        return ret;
+    }
 
     /* set 'ac' option based on channel layout */
     channels = av_get_channel_layout_nb_channels(layout);
     snprintf(layout_str, sizeof(layout_str), "%d", channels);
-    stream_str = strchr(opt, ':');
-    ac_str_size = 3 + (stream_str ? strlen(stream_str) : 0);
+    ac_str_size = 3 + stream_str_size;
     ac_str = av_mallocz(ac_str_size);
     if (!ac_str)
         return AVERROR(ENOMEM);
@@ -3595,6 +3619,10 @@  const OptionDef options[] = {
     { "channel_layout", OPT_AUDIO | HAS_ARG  | OPT_EXPERT | OPT_PERFILE |
                         OPT_INPUT | OPT_OUTPUT,                                    { .func_arg = opt_channel_layout },
         "set channel layout", "layout" },
+    /* internal OptionDef used to enable storage of channel_layout option in a SpecifierOpt */
+    { "channelmask",    OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |
+                        OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(channel_layouts) },
+       "stores channel layout in SpecifierOpt ", "channelmask" },
     { "af",             OPT_AUDIO | HAS_ARG  | OPT_PERFILE | OPT_OUTPUT,           { .func_arg = opt_audio_filters },
         "set audio filters", "filter_graph" },
     { "guess_layout_max", OPT_AUDIO | HAS_ARG | OPT_INT | OPT_SPEC | OPT_EXPERT | OPT_INPUT, { .off = OFFSET(guess_layout_max) },