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
From 4fae59de38c93a4cdd079535cc3631f9ccadced1 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgm...@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)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to