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