From: Thilo Borgmann <thilo.borgmann@mail.de>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 1/2] ffmpeg: add display_{rotation, hflip, vflip} options
Date: Tue, 2 Aug 2022 18:48:13 +0200
Message-ID: <efff060f-aa3f-5be4-7a53-31e1a44b8f19@mail.de> (raw)
In-Reply-To: <64edef9e-5052-d335-5dc3-97aa9e23f989@gyani.pro>
[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]
Hi,
>>>>> On 2022-05-18 11:34 pm, Jan Ekström wrote:
>>>>>> This enables overriding the rotation as well as horizontal/vertical
>>>>>> flip state of a specific video stream on either the input or output
>>>>>> side.
>>>>>
>>>>> Since rotation, flip and scale are stored in a single data structure,
>>>>> how about a single option i.e. -display "rotate=90,flip=hv,scale=2"
>>>>
>>>>
>>>> I did think about that, and I even implemented AVDict based options for ffmpeg.c in a branch. But then pretty much got tired of lack of AVOption automation (f.ex. for the flip flagging etc), and decided that since these are relatively basic and simple, the non-generic option for this could be just three options in the initial submission.
>>>
>>> In the end I did implement this with a single dict-based thing in a
>>> branch but never got to posting it to this thread:
>>> https://github.com/jeeb/ffmpeg/commits/ffmpeg_c_display_matrix_with_dicts
>>>
>>> So for the positive: Now it's all in a single option so it's clear
>>> that it's defining a single structure.
>>> And the negative: Needs a struct definition, struct, AVOption list,
>>> AVClass and actually if you already made a dict out of the options
>>> before, as the functions will free the original after usage and
>>> generate a new one, it destroys the life time expectations of the dict
>>> and thus it is just simpler to copy the dict when entering the
>>> function (I think there is an arguments string based API which might
>>> or might not actually be simpler for this case).
>>>
>>> So yea, not sure if I prefer this version.
>>
>> Ping.
>>
>> Did this stall for a reason? How can we iterate on it?
>
> Since the rotation/flip/scale hints are stored in a single data structure, I prefer a single user option to set those values.
attached patch allows for printing the arguments of the new -display_rotation (or any) option.
I wrote this in jeeb's github branch [1] where it applies on-top. Didn't test with FFmpeg/HEAD as this still needs cleanup anyways.
Never touched this whole options thing before, I guess there's lot to complain about...
-Thilo
[1] https://github.com/jeeb/ffmpeg/commits/ffmpeg_c_display_matrix_with_dicts
[-- Attachment #2: 0001-ffmpeg-Allow-printing-of-option-arguments-in-help-ou.patch --]
[-- Type: text/plain, Size: 14616 bytes --]
From 4fae59de38c93a4cdd079535cc3631f9ccadced1 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Tue, 2 Aug 2022 18:39:18 +0200
Subject: [PATCH] ffmpeg: Allow printing of option arguments in help output
---
fftools/cmdutils.c | 5 ++
fftools/cmdutils.h | 1 +
fftools/ffmpeg_opt.c | 56 ++++++------
libavutil/opt.c | 205 +++++++++++++++++++++++++++++++++++++++++++
libavutil/opt.h | 7 ++
5 files changed, 247 insertions(+), 27 deletions(-)
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 9c475acf7f..73362e5af2 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -169,6 +169,11 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags,
av_strlcat(buf, po->argname, sizeof(buf));
}
printf("-%-17s %s\n", buf, po->help);
+
+ if (po->args) {
+ const AVClass *p = po->args;
+ av_arg_show(&p, NULL);
+ }
}
printf("\n");
}
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 6a519c6546..df90cc6958 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -166,6 +166,7 @@ typedef struct OptionDef {
} u;
const char *help;
const char *argname;
+ const AVClass *args;
} OptionDef;
/**
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 8dffe7c225..d8db0f0681 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -114,6 +114,32 @@ static const char *const opt_name_time_bases[] = {"time_base", NU
static const char *const opt_name_enc_time_bases[] = {"enc_time_base", NULL};
static const char *const opt_name_bits_per_raw_sample[] = {"bits_per_raw_sample", NULL};
+// XXX this should probably go into a seperate file <name>_args.c and #included here
+ struct display_matrix_s {
+ const AVClass *class;
+ double rotation;
+ int hflip;
+ int vflip;
+ };
+#define OFFSET(x) offsetof(struct display_matrix_s, x)
+ static const AVOption display_matrix_args[] = {
+ { "rotation", "set rotation", OFFSET(rotation), AV_OPT_TYPE_DOUBLE,
+ { .dbl = DBL_MAX }, -(DBL_MAX), DBL_MAX - 1.0f, AV_OPT_FLAG_EXPORT},
+ { "hflip", "set hflip", OFFSET(hflip), AV_OPT_TYPE_BOOL,
+ { .i64 = -1 }, 0, 1, AV_OPT_FLAG_EXPORT},
+ { "vflip", "set vflip", OFFSET(vflip), AV_OPT_TYPE_BOOL,
+ { .i64 = -1 }, 0, 1, AV_OPT_FLAG_EXPORT},
+ { NULL },
+ };
+ static const AVClass class_display_matrix_args = {
+ .class_name = "display_matrix_args",
+ .item_name = av_default_item_name,
+ .option = display_matrix_args,
+ .version = LIBAVUTIL_VERSION_INT,
+ };
+#undef OFFSET
+// XXX
+
#define WARN_MULTIPLE_OPT_USAGE(name, type, so, st)\
{\
char namestr[128] = "";\
@@ -755,39 +781,15 @@ static int opt_recording_timestamp(void *optctx, const char *opt, const char *ar
static void add_display_matrix_to_stream(OptionsContext *o,
AVFormatContext *ctx, AVStream *st)
{
- struct DisplayMatrixOpts {
- const AVClass *class;
- double rotation;
- int hflip;
- int vflip;
- };
int hflip_set = 0, vflip_set = 0, display_rotation_set = 0;
uint8_t *buf = NULL;
-#define OFFSET(x) offsetof(struct DisplayMatrixOpts, x)
- static const AVOption opts[] = {
- { "rotation", NULL, OFFSET(rotation), AV_OPT_TYPE_DOUBLE,
- { .dbl = DBL_MAX }, -(DBL_MAX), DBL_MAX - 1.0f},
- { "hflip", NULL, OFFSET(hflip), AV_OPT_TYPE_BOOL,
- { .i64 = -1 }, 0, 1},
- { "vflip", NULL, OFFSET(vflip), AV_OPT_TYPE_BOOL,
- { .i64 = -1 }, 0, 1},
- { NULL },
- };
- static const AVClass class = {
- .class_name = "ffmpeg",
- .item_name = av_default_item_name,
- .option = opts,
- .version = LIBAVUTIL_VERSION_INT,
- };
- const AVClass *pclass = &class;
- static struct DisplayMatrixOpts test_args = {
- .class = &class,
+ static struct display_matrix_s test_args = {
+ .class = &class_display_matrix_args,
.rotation = DBL_MAX,
.hflip = -1,
.vflip = -1,
};
-#undef OFFSET
AVDictionary *global_args = NULL;
AVDictionary *local_args = NULL;
@@ -3854,7 +3856,7 @@ const OptionDef options[] = {
OPT_OUTPUT, { .off = OFFSET(display_matrixes) },
"define a display matrix with rotation and/or horizontal/vertical "
"flip for stream(s)",
- "arguments" },
+ "arguments", &class_display_matrix_args },
{ "display_rotation", OPT_VIDEO | HAS_ARG | OPT_DOUBLE | OPT_SPEC | OPT_INPUT |
OPT_OUTPUT, { .off = OFFSET(display_rotations) },
"set pure counter-clockwise rotation in degrees for stream(s)",
diff --git a/libavutil/opt.c b/libavutil/opt.c
index 8ffb10449b..428da2319f 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -1443,6 +1443,201 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
}
+static void arg_list(void *obj, void *av_log_obj, const char *unit, enum AVOptionType parent_type)
+{
+ const AVOption *opt = NULL;
+ AVOptionRanges *r;
+ int i;
+
+ while ((opt = av_opt_next(obj, opt))) {
+ /* Don't print CONST's on level one.
+ * Don't print anything but CONST's on level two.
+ * Only print items from the requested unit.
+ */
+ if (!unit && opt->type == AV_OPT_TYPE_CONST)
+ continue;
+ else if (unit && opt->type != AV_OPT_TYPE_CONST)
+ continue;
+ else if (unit && opt->type == AV_OPT_TYPE_CONST && strcmp(unit, opt->unit))
+ continue;
+ else if (unit && opt->type == AV_OPT_TYPE_CONST)
+ av_log(av_log_obj, AV_LOG_INFO, " %-15s ", opt->name);
+ else
+ av_log(av_log_obj, AV_LOG_INFO, " %-17s ", opt->name);
+
+ switch (opt->type) {
+ case AV_OPT_TYPE_FLAGS:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<flags>");
+ break;
+ case AV_OPT_TYPE_INT:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<int>");
+ break;
+ case AV_OPT_TYPE_INT64:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<int64>");
+ break;
+ case AV_OPT_TYPE_UINT64:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<uint64>");
+ break;
+ case AV_OPT_TYPE_DOUBLE:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<double>");
+ break;
+ case AV_OPT_TYPE_FLOAT:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<float>");
+ break;
+ case AV_OPT_TYPE_STRING:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<string>");
+ break;
+ case AV_OPT_TYPE_RATIONAL:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<rational>");
+ break;
+ case AV_OPT_TYPE_BINARY:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<binary>");
+ break;
+ case AV_OPT_TYPE_DICT:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<dictionary>");
+ break;
+ case AV_OPT_TYPE_IMAGE_SIZE:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<image_size>");
+ break;
+ case AV_OPT_TYPE_VIDEO_RATE:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<video_rate>");
+ break;
+ case AV_OPT_TYPE_PIXEL_FMT:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<pix_fmt>");
+ break;
+ case AV_OPT_TYPE_SAMPLE_FMT:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<sample_fmt>");
+ break;
+ case AV_OPT_TYPE_DURATION:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<duration>");
+ break;
+ case AV_OPT_TYPE_COLOR:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<color>");
+ break;
+ case AV_OPT_TYPE_CHLAYOUT:
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+ case AV_OPT_TYPE_CHANNEL_LAYOUT:
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<channel_layout>");
+ break;
+ case AV_OPT_TYPE_BOOL:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "<boolean>");
+ break;
+ case AV_OPT_TYPE_CONST:
+ if (parent_type == AV_OPT_TYPE_INT)
+ av_log(av_log_obj, AV_LOG_INFO, "%-12"PRId64" ", opt->default_val.i64);
+ else
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
+ break;
+ default:
+ av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
+ break;
+ }
+
+ if (opt->help)
+ av_log(av_log_obj, AV_LOG_INFO, " %s", opt->help);
+
+ if (av_opt_query_ranges(&r, obj, opt->name, AV_OPT_SEARCH_FAKE_OBJ) >= 0) {
+ switch (opt->type) {
+ case AV_OPT_TYPE_INT:
+ case AV_OPT_TYPE_INT64:
+ case AV_OPT_TYPE_UINT64:
+ case AV_OPT_TYPE_DOUBLE:
+ case AV_OPT_TYPE_FLOAT:
+ case AV_OPT_TYPE_RATIONAL:
+ for (i = 0; i < r->nb_ranges; i++) {
+ av_log(av_log_obj, AV_LOG_INFO, " (from ");
+ log_value(av_log_obj, AV_LOG_INFO, r->range[i]->value_min);
+ av_log(av_log_obj, AV_LOG_INFO, " to ");
+ log_value(av_log_obj, AV_LOG_INFO, r->range[i]->value_max);
+ av_log(av_log_obj, AV_LOG_INFO, ")");
+ }
+ break;
+ }
+ av_opt_freep_ranges(&r);
+ }
+
+ if (opt->type != AV_OPT_TYPE_CONST &&
+ opt->type != AV_OPT_TYPE_BINARY &&
+ !((opt->type == AV_OPT_TYPE_COLOR ||
+ opt->type == AV_OPT_TYPE_IMAGE_SIZE ||
+ opt->type == AV_OPT_TYPE_STRING ||
+ opt->type == AV_OPT_TYPE_DICT ||
+ opt->type == AV_OPT_TYPE_CHLAYOUT ||
+ opt->type == AV_OPT_TYPE_VIDEO_RATE) &&
+ !opt->default_val.str)) {
+ av_log(av_log_obj, AV_LOG_INFO, " (default ");
+ switch (opt->type) {
+ case AV_OPT_TYPE_BOOL:
+ av_log(av_log_obj, AV_LOG_INFO, "%s", (char *)av_x_if_null(get_bool_name(opt->default_val.i64), "invalid"));
+ break;
+ case AV_OPT_TYPE_FLAGS: {
+ char *def_flags = get_opt_flags_string(obj, opt->unit, opt->default_val.i64);
+ if (def_flags) {
+ av_log(av_log_obj, AV_LOG_INFO, "%s", def_flags);
+ av_freep(&def_flags);
+ } else {
+ av_log(av_log_obj, AV_LOG_INFO, "%"PRIX64, opt->default_val.i64);
+ }
+ break;
+ }
+ case AV_OPT_TYPE_DURATION: {
+ char buf[25];
+ format_duration(buf, sizeof(buf), opt->default_val.i64);
+ av_log(av_log_obj, AV_LOG_INFO, "%s", buf);
+ break;
+ }
+ case AV_OPT_TYPE_INT:
+ case AV_OPT_TYPE_UINT64:
+ case AV_OPT_TYPE_INT64: {
+ const char *def_const = get_opt_const_name(obj, opt->unit, opt->default_val.i64);
+ if (def_const)
+ av_log(av_log_obj, AV_LOG_INFO, "%s", def_const);
+ else
+ log_int_value(av_log_obj, AV_LOG_INFO, opt->default_val.i64);
+ break;
+ }
+ case AV_OPT_TYPE_DOUBLE:
+ case AV_OPT_TYPE_FLOAT:
+ log_value(av_log_obj, AV_LOG_INFO, opt->default_val.dbl);
+ break;
+ case AV_OPT_TYPE_RATIONAL: {
+ AVRational q = av_d2q(opt->default_val.dbl, INT_MAX);
+ av_log(av_log_obj, AV_LOG_INFO, "%d/%d", q.num, q.den); }
+ break;
+ case AV_OPT_TYPE_PIXEL_FMT:
+ av_log(av_log_obj, AV_LOG_INFO, "%s", (char *)av_x_if_null(av_get_pix_fmt_name(opt->default_val.i64), "none"));
+ break;
+ case AV_OPT_TYPE_SAMPLE_FMT:
+ av_log(av_log_obj, AV_LOG_INFO, "%s", (char *)av_x_if_null(av_get_sample_fmt_name(opt->default_val.i64), "none"));
+ break;
+ case AV_OPT_TYPE_COLOR:
+ case AV_OPT_TYPE_IMAGE_SIZE:
+ case AV_OPT_TYPE_STRING:
+ case AV_OPT_TYPE_DICT:
+ case AV_OPT_TYPE_VIDEO_RATE:
+ case AV_OPT_TYPE_CHLAYOUT:
+ av_log(av_log_obj, AV_LOG_INFO, "\"%s\"", opt->default_val.str);
+ break;
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+ case AV_OPT_TYPE_CHANNEL_LAYOUT:
+ av_log(av_log_obj, AV_LOG_INFO, "0x%"PRIx64, opt->default_val.i64);
+ break;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ }
+ av_log(av_log_obj, AV_LOG_INFO, ")");
+ }
+
+ av_log(av_log_obj, AV_LOG_INFO, "\n");
+ if (opt->unit && opt->type != AV_OPT_TYPE_CONST)
+ arg_list(obj, av_log_obj, opt->unit, opt->type);
+ }
+}
+
int av_opt_show2(void *obj, void *av_log_obj, int req_flags, int rej_flags)
{
if (!obj)
@@ -1455,6 +1650,16 @@ int av_opt_show2(void *obj, void *av_log_obj, int req_flags, int rej_flags)
return 0;
}
+int av_arg_show(void *obj, void *av_log_obj)
+{
+ if (!obj)
+ return -1;
+
+ arg_list(obj, av_log_obj, NULL, -1);
+
+ return 0;
+}
+
void av_opt_set_defaults(void *s)
{
av_opt_set_defaults2(s, 0, 0);
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 461b5d3b6b..7bd560782f 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -386,6 +386,13 @@ typedef struct AVOptionRanges {
*/
int av_opt_show2(void *obj, void *av_log_obj, int req_flags, int rej_flags);
+/**
+ * Show the obj arguments.
+ *
+ * @param av_log_obj log context to use for showing the options
+ */
+int av_arg_show(void *obj, void *av_log_obj);
+
/**
* Set the values of all AVOption fields to their default values.
*
--
2.20.1 (Apple Git-117)
[-- Attachment #3: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2022-08-02 16:48 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-18 18:04 Jan Ekström
2022-05-18 18:04 ` [FFmpeg-devel] [PATCH 2/2] ffmpeg: deprecate display rotation override with a metadata key Jan Ekström
2022-05-19 4:21 ` [FFmpeg-devel] [PATCH 1/2] ffmpeg: add display_{rotation, hflip, vflip} options Gyan Doshi
2022-05-19 6:27 ` Jan Ekström
2022-05-26 11:31 ` Jan Ekström
2022-07-17 15:19 ` Thilo Borgmann
2022-07-17 16:26 ` Gyan Doshi
2022-08-02 16:48 ` Thilo Borgmann [this message]
2022-08-02 16:49 ` Thilo Borgmann
2022-08-02 17:10 ` Andreas Rheinhardt
2022-08-02 17:21 ` Thilo Borgmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=efff060f-aa3f-5be4-7a53-31e1a44b8f19@mail.de \
--to=thilo.borgmann@mail.de \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git