On 2 Aug 2022, at 19:10, Andreas Rheinhardt wrote:

Thilo Borgmann:
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...


Indeed. I hope you don't expect us to apply this clone of opt_list (the only difference I found were that you are not printing the AV_OPT_FLAG_*
values).

I guess not, more a proof of concept.
More differences are that it does not prefix the arguments with a „-“ and it does not print a header like „<name> AVContext arguments“ (or something like that)

Can either be complexified to have these controlled by more args or split into several sub functions…?

-Thilo



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);
+    }
+}
+


_______________________________________________
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".
_______________________________________________
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