[FFmpeg-devel] [PATCH 1/2] ffmpeg: add display_{rotation, hflip, vflip} options

Thilo Borgmann thilo.borgmann at mail.de
Tue Aug 2 19:48:13 EEST 2022


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
-------------- next part --------------
From 4fae59de38c93a4cdd079535cc3631f9ccadced1 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at 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)



More information about the ffmpeg-devel mailing list