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

2022-08-02 Thread Thilo Borgmann



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 „ 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 ", 
"");

+break;
+case AV_OPT_TYPE_INT:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
+break;
+case AV_OPT_TYPE_INT64:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_UINT64:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_DOUBLE:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_FLOAT:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_STRING:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_RATIONAL:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_BINARY:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_DICT:
+av_log(av_log_obj, AV_LOG_INFO, "%-12s ", 
"");

+break;
+case AV_OPT_TYPE_IMAGE_SIZE:
+

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

2022-08-02 Thread Andreas Rheinhardt
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 ", "");
> +break;
> +case AV_OPT_TYPE_INT:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_INT64:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_UINT64:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_DOUBLE:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_FLOAT:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_STRING:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_RATIONAL:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_BINARY:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_DICT:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case AV_OPT_TYPE_IMAGE_SIZE:
> +av_log(av_log_obj, AV_LOG_INFO, "%-12s ", "");
> +break;
> +case 

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

2022-08-02 Thread Thilo Borgmann

Am 02.08.22 um 18:48 schrieb 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...


Produced output looks like:

-display_matrix arguments  define a display matrix with rotation and/or 
horizontal/vertical flip for stream(s)
   rotationset rotation (from -DBL_MAX to DBL_MAX) 
(default DBL_MAX)
   hflip  set hflip (default auto)
   vflip  set vflip (default auto)



-Thilo

[1] https://github.com/jeeb/ffmpeg/commits/ffmpeg_c_display_matrix_with_dicts

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


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

2022-08-02 Thread 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...

-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 
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(, 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 _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 

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

2022-07-17 Thread Gyan Doshi



On 2022-07-17 08:49 pm, Thilo Borgmann wrote:

Hi,

Am 26.05.22 um 13:31 schrieb Jan Ekström:

On Thu, May 19, 2022 at 9:27 AM Jan Ekström  wrote:


On Thu, May 19, 2022, 07:21 Gyan Doshi  wrote:




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.


Regards,
Gyan
___
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".


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

2022-07-17 Thread Thilo Borgmann

Hi,

Am 26.05.22 um 13:31 schrieb Jan Ekström:

On Thu, May 19, 2022 at 9:27 AM Jan Ekström  wrote:


On Thu, May 19, 2022, 07:21 Gyan Doshi  wrote:




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?

-Thilo
___
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".


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

2022-05-26 Thread Jan Ekström
On Thu, May 19, 2022 at 9:27 AM Jan Ekström  wrote:
>
> On Thu, May 19, 2022, 07:21 Gyan Doshi  wrote:
>>
>>
>>
>> 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.

Jan
___
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".


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

2022-05-19 Thread Jan Ekström
On Thu, May 19, 2022, 07:21 Gyan Doshi  wrote:

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

Jan
___
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".


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

2022-05-18 Thread Gyan Doshi



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"


Regards,
Gyan



Additionally, switch the singular test that was utilizing the rotation
metadata to instead override the input display rotation, thus leading
to the same result.
---
  doc/ffmpeg.texi | 29 +
  fftools/ffmpeg.h|  6 +
  fftools/ffmpeg_opt.c| 52 +
  tests/fate/filter-video.mak |  2 +-
  4 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 0d7e1a479d..f44d863f02 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -897,6 +897,35 @@ If used together with @option{-vcodec copy}, it will 
affect the aspect ratio
  stored at container level, but not the aspect ratio stored in encoded
  frames, if it exists.
  
+@item -display_rotation[:@var{stream_specifier}] @var{rotation} (@emph{input/output,per-stream})

+Set the video display rotation in degrees specified by @var{rotation}.
+
+@var{rotation} is a floating point number that describes a pure
+counter-clockwise rotation in degrees.
+
+When setting this for input streams, @code{-autorotate} logic will be affected.
+For additional parameters affecting display matrix side data into which this
+information is saved, see @code{-display_hflip} and @code{-display_vflip}.
+
+These options work as a unit, so if only one of them is set, then the display
+matrix will be overridden to that specific value with the rest being set to
+default values.
+
+If unset, the default value if a display matrix is being defined is a rotation
+of zero degrees.
+
+@item -display_hflip[:@var{stream_specifier}] (@emph{input/output,per-stream})
+Set whether on display the image should be horizontally flipped.
+
+If unset, the default value if a display matrix is being defined is that there
+is no additional horizontal flip. See @code{-display_rotation}.
+
+@item -display_vflip[:@var{stream_specifier}] (@emph{input/output,per-stream})
+Set whether on display the image should be vertically flipped.
+
+If unset, the default value if a display matrix is being defined is that there
+is no additional vertical flip. See @code{-display_rotation}.
+
  @item -vn (@emph{input/output})
  As an input option, blocks all video streams of a file from being filtered or
  being automatically selected or mapped for any output. See @code{-discard}
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 9f0c093e34..0da01b0668 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -180,6 +180,12 @@ typedef struct OptionsContext {
  intnb_force_fps;
  SpecifierOpt *frame_aspect_ratios;
  intnb_frame_aspect_ratios;
+SpecifierOpt *display_rotations;
+intnb_display_rotations;
+SpecifierOpt *display_hflips;
+intnb_display_hflips;
+SpecifierOpt *display_vflips;
+intnb_display_vflips;
  SpecifierOpt *rc_overrides;
  intnb_rc_overrides;
  SpecifierOpt *intra_matrices;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47e8b9b7bd..0d1c84d6df 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -20,6 +20,7 @@
  
  #include "config.h"
  
+#include 

  #include 
  
  #if HAVE_SYS_RESOURCE_H

@@ -43,6 +44,7 @@
  #include "libavutil/avutil.h"
  #include "libavutil/bprint.h"
  #include "libavutil/channel_layout.h"
+#include "libavutil/display.h"
  #include "libavutil/intreadwrite.h"
  #include "libavutil/fifo.h"
  #include "libavutil/mathematics.h"
@@ -82,6 +84,9 @@ static const char *const opt_name_qscale[]= {"q", 
"qscale",
  static const char *const opt_name_forced_key_frames[] = 
{"forced_key_frames", NULL};
  static const char *const opt_name_force_fps[] = {"force_fps", 
NULL};
  static const char *const opt_name_frame_aspect_ratios[]   = {"aspect", 
NULL};
+static const char *const opt_name_display_rotations[] = 
{"display_rotation", NULL};
+static const char *const opt_name_display_hflips[]= 
{"display_hflip", NULL};
+static const char *const opt_name_display_vflips[]= 
{"display_vflip", NULL};
  static const char *const opt_name_rc_overrides[]  = 
{"rc_override", NULL};
  static const char *const opt_name_intra_matrices[]= 
{"intra_matrix", NULL};
  static const char *const opt_name_inter_matrices[]= 
{"inter_matrix", NULL};
@@ -739,6 +744,39 @@ static int opt_recording_timestamp(void *optctx, const 
char *opt, const char *ar
  return 0;
  }
  
+static void add_display_matrix_to_stream(OptionsContext *o,

+ AVFormatContext *ctx, AVStream *st)
+{
+double 

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

2022-05-18 Thread Jan Ekström
This enables overriding the rotation as well as horizontal/vertical
flip state of a specific video stream on either the input or output
side.

Additionally, switch the singular test that was utilizing the rotation
metadata to instead override the input display rotation, thus leading
to the same result.
---
 doc/ffmpeg.texi | 29 +
 fftools/ffmpeg.h|  6 +
 fftools/ffmpeg_opt.c| 52 +
 tests/fate/filter-video.mak |  2 +-
 4 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 0d7e1a479d..f44d863f02 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -897,6 +897,35 @@ If used together with @option{-vcodec copy}, it will 
affect the aspect ratio
 stored at container level, but not the aspect ratio stored in encoded
 frames, if it exists.
 
+@item -display_rotation[:@var{stream_specifier}] @var{rotation} 
(@emph{input/output,per-stream})
+Set the video display rotation in degrees specified by @var{rotation}.
+
+@var{rotation} is a floating point number that describes a pure
+counter-clockwise rotation in degrees.
+
+When setting this for input streams, @code{-autorotate} logic will be affected.
+For additional parameters affecting display matrix side data into which this
+information is saved, see @code{-display_hflip} and @code{-display_vflip}.
+
+These options work as a unit, so if only one of them is set, then the display
+matrix will be overridden to that specific value with the rest being set to
+default values.
+
+If unset, the default value if a display matrix is being defined is a rotation
+of zero degrees.
+
+@item -display_hflip[:@var{stream_specifier}] (@emph{input/output,per-stream})
+Set whether on display the image should be horizontally flipped.
+
+If unset, the default value if a display matrix is being defined is that there
+is no additional horizontal flip. See @code{-display_rotation}.
+
+@item -display_vflip[:@var{stream_specifier}] (@emph{input/output,per-stream})
+Set whether on display the image should be vertically flipped.
+
+If unset, the default value if a display matrix is being defined is that there
+is no additional vertical flip. See @code{-display_rotation}.
+
 @item -vn (@emph{input/output})
 As an input option, blocks all video streams of a file from being filtered or
 being automatically selected or mapped for any output. See @code{-discard}
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 9f0c093e34..0da01b0668 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -180,6 +180,12 @@ typedef struct OptionsContext {
 intnb_force_fps;
 SpecifierOpt *frame_aspect_ratios;
 intnb_frame_aspect_ratios;
+SpecifierOpt *display_rotations;
+intnb_display_rotations;
+SpecifierOpt *display_hflips;
+intnb_display_hflips;
+SpecifierOpt *display_vflips;
+intnb_display_vflips;
 SpecifierOpt *rc_overrides;
 intnb_rc_overrides;
 SpecifierOpt *intra_matrices;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47e8b9b7bd..0d1c84d6df 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -20,6 +20,7 @@
 
 #include "config.h"
 
+#include 
 #include 
 
 #if HAVE_SYS_RESOURCE_H
@@ -43,6 +44,7 @@
 #include "libavutil/avutil.h"
 #include "libavutil/bprint.h"
 #include "libavutil/channel_layout.h"
+#include "libavutil/display.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
@@ -82,6 +84,9 @@ static const char *const opt_name_qscale[]
= {"q", "qscale",
 static const char *const opt_name_forced_key_frames[] = 
{"forced_key_frames", NULL};
 static const char *const opt_name_force_fps[] = {"force_fps", 
NULL};
 static const char *const opt_name_frame_aspect_ratios[]   = {"aspect", 
NULL};
+static const char *const opt_name_display_rotations[] = 
{"display_rotation", NULL};
+static const char *const opt_name_display_hflips[]= 
{"display_hflip", NULL};
+static const char *const opt_name_display_vflips[]= 
{"display_vflip", NULL};
 static const char *const opt_name_rc_overrides[]  = 
{"rc_override", NULL};
 static const char *const opt_name_intra_matrices[]= 
{"intra_matrix", NULL};
 static const char *const opt_name_inter_matrices[]= 
{"inter_matrix", NULL};
@@ -739,6 +744,39 @@ static int opt_recording_timestamp(void *optctx, const 
char *opt, const char *ar
 return 0;
 }
 
+static void add_display_matrix_to_stream(OptionsContext *o,
+ AVFormatContext *ctx, AVStream *st)
+{
+double display_rotation = DBL_MAX;
+int hflip = -1, vflip = -1,
+hflip_set = 0, vflip_set = 0, display_rotation_set = 0;
+uint8_t *buf = NULL;
+
+MATCH_PER_STREAM_OPT(display_rotations, dbl, display_rotation, ctx, st);
+