diff mbox

[FFmpeg-devel,DEVEL,2/2] ffmpeg: fix ticket 6706

Message ID 425f2195-fc3d-9325-df02-796dae5289e3@gmail.com
State Superseded
Headers show

Commit Message

pkv.stream Nov. 19, 2017, 10:01 a.m. UTC 
From f94f2e8c8878d6dbda540b19d90c2b8f1ba00850 Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream@gmail.com>
Date: Sat, 18 Nov 2017 00:26:50 +0100
Subject: [PATCH 2/2] ffmpeg: fix 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     |  3 +++
 fftools/ffmpeg_opt.c | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 34 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Nov. 19, 2017, 7:28 p.m. UTC | #1 
On Sun, Nov 19, 2017 at 11:01:37AM +0100, pkv.stream wrote:
[...]

> @@ -3674,6 +3697,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" },
> +    { "channel_layout_uint64", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |

i mean as "channel_layout"
does it work with both using the same name so the option is routed
to both the field and callback or am i missing something why this is
not intended?

[...]
pkv.stream Nov. 19, 2017, 8:03 p.m. UTC | #2 
Le 19/11/2017 à 8:28 PM, Michael Niedermayer a écrit :
> On Sun, Nov 19, 2017 at 11:01:37AM +0100, pkv.stream wrote:
> [...]
>
>> @@ -3674,6 +3697,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" },
>> +    { "channel_layout_uint64", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |
> i mean as "channel_layout"
> does it work with both using the same name so the option is routed
> to both the field and callback or am i missing something why this is
> not intended?
I assumed options in the same OptionDef should have unique names (seems 
to be the case at the moment).
I've just tried what you suggest but there are  errors parsing and 
writing the option when the channel layout is specified as a string 
(quad, octagonal, hexadecagonal ...)
In opt_channel_layout, the function parse_option assumes the first 
channel_layout option (non Spec), while we need the second one. So this 
generates errors.
If the order of the options is reversed, the string is not recognized 
any more.
Maybe it can be done with some more work. I don't really know. Tell me 
if you want me to investigate this some more.
Thanks.

>
> [...]
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index e0977e1..5c45df4 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -121,6 +121,8 @@  typedef struct OptionsContext {
     int        nb_frame_sizes;
     SpecifierOpt *frame_pix_fmts;
     int        nb_frame_pix_fmts;
+    SpecifierOpt *channel_layouts;
+    int        nb_channel_layouts;
 
     /* input options */
     int64_t input_ts_offset;
@@ -624,6 +626,7 @@  extern int vstats_version;
 extern const AVIOInterruptCB int_cb;
 
 extern const OptionDef options[];
+extern const OptionDef channel_layout_option[];
 extern const HWAccel hwaccels[];
 extern AVBufferRef *hw_device_ctx;
 #if CONFIG_QSV
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47d3841..7b2c18c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1803,6 +1803,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 &&
@@ -2527,7 +2528,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])
@@ -3104,7 +3109,7 @@  static int opt_channel_layout(void *optctx, const char *opt, const char *arg)
     char layout_str[32];
     char *stream_str;
     char *ac_str;
-    int ret, channels, ac_str_size;
+    int ret, channels, ac_str_size, stream_str_size;
     uint64_t layout;
 
     layout = av_get_channel_layout(arg);
@@ -3116,12 +3121,30 @@  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 'channel_layout_uint64' option which stores channel_layout (as uint64 channel mask)
+     * in SpecifierOpt, enabling access to channel layout through MATCH_PER_STREAM_OPT
+     */
+    ac_str_size = 22 + stream_str_size;
+    ac_str = av_mallocz(ac_str_size);
+    if (!ac_str) {
+        return AVERROR(ENOMEM);
+    }
+    av_strlcpy(ac_str, "channel_layout_uint64", 22);
+    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);
@@ -3674,6 +3697,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" },
+    { "channel_layout_uint64", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |
+                               OPT_INPUT | OPT_OUTPUT,                             { .off = OFFSET(channel_layouts) },
+        "set channel layout with uint64"
+        "(for more syntax choices use instead the channel_layout option)", "layout_uint64" }, // allows storing of option in SpecifierOpt
     { "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) },