[FFmpeg-devel] [DEVEL][PATCH] ffmpeg: Fix channel_layout bug on non-default layout
pkv.stream
pkv.stream at gmail.com
Mon Feb 26 04:53:31 EET 2018
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.
-------------- next part --------------
From ecd706479f033e91001da4ddb699baf4f3440caa Mon Feb 26 02:33:20 2018
From: pkviet <pkv.stream at 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 at gmail.com>
---
fftools/ffmpeg.h | 2 ++
fftools/ffmpeg_opt.c | 38 +++++++++++++++++++++++++++++++++-----
2 files changed, 35 insertions(+), 5 deletions(-)
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) },
--
2.16.2.windows.1
-------------- next part --------------
From 3b3b8b38f8c7c62cbc1144f5d845d96a17a121f5 Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream at gmail.com>
Date: Wed, 22 Nov 2017 13:44:37 +0100
Subject: [PATCH 1/2] ffmpeg: parse duplicate option
Allows parsing of a duplicate option in an OptionDef table, with same
name as main option.
This is useful for example for channel_layout which can be parsed with
func_arg (opt_channel_layout) and also stored as an offset in a
SpecifierOpt.
---
fftools/cmdutils.c | 18 ++++++++++++++++++
fftools/cmdutils.h | 13 +++++++++++++
2 files changed, 31 insertions(+)
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 6920ca0..00802cf 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -380,6 +380,24 @@ int parse_option(void *optctx, const char *opt, const char *arg,
return !!(po->flags & HAS_ARG);
}
+int parse_duplicate_option(void *optctx, const char *opt, const char *arg,
+ const OptionDef *options)
+{
+ const OptionDef *po;
+ int ret;
+
+ po = find_option(options, opt) + 1;
+ if (po->name != (po - 1)->name) {
+ av_log(NULL, AV_LOG_ERROR, "Option '%s' has no duplicate.\n", opt);
+ return AVERROR(EINVAL);
+ }
+ ret = write_option(optctx, po, opt, arg);
+ if (ret < 0)
+ return ret;
+
+ return !!(po->flags & HAS_ARG);
+}
+
void parse_options(void *optctx, int argc, char **argv, const OptionDef *options,
void (*parse_arg_function)(void *, const char*))
{
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 8724489..087df14 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -283,6 +283,19 @@ int parse_option(void *optctx, const char *opt, const char *arg,
const OptionDef *options);
/**
+* Parse the duplicate of one given option (carrying same name). Some options may
+* need to be dealt with func_arg as well as OPT_SPEC.
+* Ex: channel_layout is parsed with func_arg (allowing strings as well as numerals
+* as arguments); but it can be needed also as an OPT_SPEC.
+* The duplicate option should be defined in an OptionDef table right after the main
+* option and carries the same name.
+*
+* @return on success 1 if arg was consumed, 0 otherwise; negative number on error
+*/
+int parse_duplicate_option(void *optctx, const char *opt, const char *arg,
+ const OptionDef *options);
+
+/**
* An option extracted from the commandline.
* Cannot use AVDictionary because of options like -map which can be
* used multiple times.
--
2.10.1.windows.1
-------------- next part --------------
From 6b779420be60afca1acdd58208b084dc3a0e7fea Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream at gmail.com>
Date: Sat, 18 Nov 2017 00:26:50 +0100
Subject: [PATCH 2/2] ffmpeg: fix channel_layout option bug
Fixes 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.
---
fftools/ffmpeg.h | 3 +++
fftools/ffmpeg_opt.c | 34 ++++++++++++++++++++++++++++++----
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 4e73d59..4708010 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -116,6 +116,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;
@@ -619,6 +621,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 a6e36ac..e428e68 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1817,6 +1817,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 &&
@@ -2543,7 +2544,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])
@@ -3120,7 +3125,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);
@@ -3132,12 +3137,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 duplicate 'channel_layout' option 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", 22);
+ if (stream_str) {
+ av_strlcat(ac_str, stream_str, ac_str_size);
+ }
+ ret = parse_duplicate_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);
@@ -3690,6 +3713,9 @@ 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", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |
+ OPT_INPUT | OPT_OUTPUT, { .off = OFFSET(channel_layouts) },
+ "set channel layout with uint64", "layout_uint64" }, // allows storage 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) },
--
2.10.1.windows.1
More information about the ffmpeg-devel
mailing list