| Message ID | 20200511193522.31206-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avutil/opt: add AV_OPT_FLAG_CHILD_CONSTS | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Mon, May 11, 2020 at 09:35:17PM +0200, Marton Balint wrote: > This will be used for AVCodecContext->profile. By specifying constants in the > encoders we won't have to use the common AVCodecContext options table and > different encoders can use the same profile name even with different values. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/APIchanges | 3 +++ > libavutil/opt.c | 3 ++- > libavutil/opt.h | 1 + > libavutil/version.h | 2 +- > 4 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 75cfdb08b0..235888c174 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h > + Add AV_OPT_FLAG_CHILD_CONSTS. > + > 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h > Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions > and num_enabled_dev_extensions fields to AVVulkanDeviceContext > diff --git a/libavutil/opt.c b/libavutil/opt.c > index b792dec01c..423313bce2 100644 > --- a/libavutil/opt.c > +++ b/libavutil/opt.c > @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con > } > > { > - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, 0); > int res; > int ci = 0; > double const_values[64]; > const char * const_names[64]; > + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? AV_OPT_SEARCH_CHILDREN : 0; > + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, search_flags); > if (o_named && o_named->type == AV_OPT_TYPE_CONST) > d = DEFAULT_NUMVAL(o_named); > else { > diff --git a/libavutil/opt.h b/libavutil/opt.h > index 1969c984dd..e46119572a 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -291,6 +291,7 @@ typedef struct AVOption { > #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which can be set by the user at runtime > #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering > #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information > +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option constants can also reside in child objects why is this needed and not default ? thx [...]
On Tue, 12 May 2020, Michael Niedermayer wrote: > On Mon, May 11, 2020 at 09:35:17PM +0200, Marton Balint wrote: >> This will be used for AVCodecContext->profile. By specifying constants in the >> encoders we won't have to use the common AVCodecContext options table and >> different encoders can use the same profile name even with different values. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> doc/APIchanges | 3 +++ >> libavutil/opt.c | 3 ++- >> libavutil/opt.h | 1 + >> libavutil/version.h | 2 +- >> 4 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/doc/APIchanges b/doc/APIchanges >> index 75cfdb08b0..235888c174 100644 >> --- a/doc/APIchanges >> +++ b/doc/APIchanges >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >> >> API changes, most recent first: >> >> +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h >> + Add AV_OPT_FLAG_CHILD_CONSTS. >> + >> 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h >> Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions >> and num_enabled_dev_extensions fields to AVVulkanDeviceContext >> diff --git a/libavutil/opt.c b/libavutil/opt.c >> index b792dec01c..423313bce2 100644 >> --- a/libavutil/opt.c >> +++ b/libavutil/opt.c >> @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con >> } >> >> { >> - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, 0); >> int res; >> int ci = 0; >> double const_values[64]; >> const char * const_names[64]; >> + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? AV_OPT_SEARCH_CHILDREN : 0; >> + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, search_flags); >> if (o_named && o_named->type == AV_OPT_TYPE_CONST) >> d = DEFAULT_NUMVAL(o_named); >> else { >> diff --git a/libavutil/opt.h b/libavutil/opt.h >> index 1969c984dd..e46119572a 100644 >> --- a/libavutil/opt.h >> +++ b/libavutil/opt.h >> @@ -291,6 +291,7 @@ typedef struct AVOption { >> #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which can be set by the user at runtime >> #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering >> #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information > >> +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option constants can also reside in child objects > > why is this needed and not default ? Because a child object can possibly use the same option name with different semantics or for a different purpose. In order for this to not cause problems, the unit name used have to be somewhat unique, and AV_OPT_FLAG_CHILD_CONSTS should only be set for cases where it already is. That is why I change the unit name of AVCodecContext->profile to "avctx.profile" in a later patch. Regards, Marton > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Modern terrorism, a quick summary: Need oil, start war with country that > has oil, kill hundread thousand in war. Let country fall into chaos, > be surprised about raise of fundamantalists. Drop more bombs, kill more > people, be surprised about them taking revenge and drop even more bombs > and strip your own citizens of their rights and freedoms. to be continued >
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Marton Balint > Sent: Tuesday, May 12, 2020 03:35 > To: ffmpeg-devel@ffmpeg.org > Cc: Marton Balint <cus@passwd.hu> > Subject: [FFmpeg-devel] [PATCH 1/6] avutil/opt: add > AV_OPT_FLAG_CHILD_CONSTS > > This will be used for AVCodecContext->profile. By specifying constants in the > encoders we won't have to use the common AVCodecContext options table > and > different encoders can use the same profile name even with different values. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/APIchanges | 3 +++ > libavutil/opt.c | 3 ++- > libavutil/opt.h | 1 + > libavutil/version.h | 2 +- > 4 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 75cfdb08b0..235888c174 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h > + Add AV_OPT_FLAG_CHILD_CONSTS. > + > 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h > Add enabled_inst_extensions, num_enabled_inst_extensions, > enabled_dev_extensions > and num_enabled_dev_extensions fields to AVVulkanDeviceContext > diff --git a/libavutil/opt.c b/libavutil/opt.c > index b792dec01c..423313bce2 100644 > --- a/libavutil/opt.c > +++ b/libavutil/opt.c > @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void > *target_obj, const AVOption *o, con > } > > { > - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o- > >unit, 0, 0); > int res; > int ci = 0; > double const_values[64]; > const char * const_names[64]; > + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? > AV_OPT_SEARCH_CHILDREN : 0; > + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o- > >unit, 0, search_flags); > if (o_named && o_named->type == AV_OPT_TYPE_CONST) > d = DEFAULT_NUMVAL(o_named); > else { > diff --git a/libavutil/opt.h b/libavutil/opt.h > index 1969c984dd..e46119572a 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -291,6 +291,7 @@ typedef struct AVOption { > #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic > parameter which can be set by the user at runtime > #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic > parameter which can be set by the user for filtering > #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is > deprecated, users should refer to AVOption.help text for more information > +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option > constants can also reside in child objects > //FIXME think about enc-audio, ... style flags > > /** > diff --git a/libavutil/version.h b/libavutil/version.h > index 48d8a38c42..c4946c1c7e 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -80,7 +80,7 @@ > > #define LIBAVUTIL_VERSION_MAJOR 56 > #define LIBAVUTIL_VERSION_MINOR 44 > -#define LIBAVUTIL_VERSION_MICRO 100 > +#define LIBAVUTIL_VERSION_MICRO 101 > > #define LIBAVUTIL_VERSION_INT > AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > LIBAVUTIL_VERSION_MINOR, \ > -- > 2.16.4 I'm with this idea to specify avctx->profile with candidate options in child encoder. Also verified that this would benefit encoders who share the standard profile options and use avtcx->profile directly. For others we may need to implement some corresponding changes to transfer from private->profile to avctx->profile. (But we may keep it for now, and change step by step) - Linjie
On Mon, 11 May 2020, Marton Balint wrote: > This will be used for AVCodecContext->profile. By specifying constants in the > encoders we won't have to use the common AVCodecContext options table and > different encoders can use the same profile name even with different values. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/APIchanges | 3 +++ > libavutil/opt.c | 3 ++- > libavutil/opt.h | 1 + > libavutil/version.h | 2 +- > 4 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 75cfdb08b0..235888c174 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h > + Add AV_OPT_FLAG_CHILD_CONSTS. > + > 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h > Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions > and num_enabled_dev_extensions fields to AVVulkanDeviceContext > diff --git a/libavutil/opt.c b/libavutil/opt.c > index b792dec01c..423313bce2 100644 > --- a/libavutil/opt.c > +++ b/libavutil/opt.c > @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con > } > > { > - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, 0); > int res; > int ci = 0; > double const_values[64]; > const char * const_names[64]; > + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? AV_OPT_SEARCH_CHILDREN : 0; > + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, search_flags); > if (o_named && o_named->type == AV_OPT_TYPE_CONST) > d = DEFAULT_NUMVAL(o_named); > else { > diff --git a/libavutil/opt.h b/libavutil/opt.h > index 1969c984dd..e46119572a 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -291,6 +291,7 @@ typedef struct AVOption { > #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which can be set by the user at runtime > #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering > #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information > +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option constants can also reside in child objects > //FIXME think about enc-audio, ... style flags > > /** > diff --git a/libavutil/version.h b/libavutil/version.h > index 48d8a38c42..c4946c1c7e 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -80,7 +80,7 @@ > > #define LIBAVUTIL_VERSION_MAJOR 56 > #define LIBAVUTIL_VERSION_MINOR 44 > -#define LIBAVUTIL_VERSION_MICRO 100 > +#define LIBAVUTIL_VERSION_MICRO 101 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > LIBAVUTIL_VERSION_MINOR, \ Ping for the series, is there anybody against the approach I took to move profile name constants to encoders? Thanks, Marton
Quoting Marton Balint (2020-05-18 20:43:45) > Ping for the series, is there anybody against the approach I took to move > profile name constants to encoders? I like it. Wondered if sharing options like that might cause problems but couldn't think of anything.
On Tue, May 19, 2020 at 2:43 AM Marton Balint <cus@passwd.hu> wrote: > > > > On Mon, 11 May 2020, Marton Balint wrote: > > > This will be used for AVCodecContext->profile. By specifying constants in the > > encoders we won't have to use the common AVCodecContext options table and > > different encoders can use the same profile name even with different values. > > > > Signed-off-by: Marton Balint <cus@passwd.hu> > > --- > > doc/APIchanges | 3 +++ > > libavutil/opt.c | 3 ++- > > libavutil/opt.h | 1 + > > libavutil/version.h | 2 +- > > 4 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > index 75cfdb08b0..235888c174 100644 > > --- a/doc/APIchanges > > +++ b/doc/APIchanges > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > > API changes, most recent first: > > > > +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h > > + Add AV_OPT_FLAG_CHILD_CONSTS. > > + > > 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h > > Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions > > and num_enabled_dev_extensions fields to AVVulkanDeviceContext > > diff --git a/libavutil/opt.c b/libavutil/opt.c > > index b792dec01c..423313bce2 100644 > > --- a/libavutil/opt.c > > +++ b/libavutil/opt.c > > @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con > > } > > > > { > > - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, 0); > > int res; > > int ci = 0; > > double const_values[64]; > > const char * const_names[64]; > > + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? AV_OPT_SEARCH_CHILDREN : 0; > > + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, search_flags); > > if (o_named && o_named->type == AV_OPT_TYPE_CONST) > > d = DEFAULT_NUMVAL(o_named); > > else { > > diff --git a/libavutil/opt.h b/libavutil/opt.h > > index 1969c984dd..e46119572a 100644 > > --- a/libavutil/opt.h > > +++ b/libavutil/opt.h > > @@ -291,6 +291,7 @@ typedef struct AVOption { > > #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which can be set by the user at runtime > > #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering > > #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information > > +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option constants can also reside in child objects > > //FIXME think about enc-audio, ... style flags > > > > /** > > diff --git a/libavutil/version.h b/libavutil/version.h > > index 48d8a38c42..c4946c1c7e 100644 > > --- a/libavutil/version.h > > +++ b/libavutil/version.h > > @@ -80,7 +80,7 @@ > > > > #define LIBAVUTIL_VERSION_MAJOR 56 > > #define LIBAVUTIL_VERSION_MINOR 44 > > -#define LIBAVUTIL_VERSION_MICRO 100 > > +#define LIBAVUTIL_VERSION_MICRO 101 > > > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > > LIBAVUTIL_VERSION_MINOR, \ > > > Ping for the series, is there anybody against the approach I took to move > profile name constants to encoders? I'm OK for this series
On Thu, 21 May 2020, mypopy@gmail.com wrote: > On Tue, May 19, 2020 at 2:43 AM Marton Balint <cus@passwd.hu> wrote: >> >> >> >> On Mon, 11 May 2020, Marton Balint wrote: >> >> > This will be used for AVCodecContext->profile. By specifying constants in the >> > encoders we won't have to use the common AVCodecContext options table and >> > different encoders can use the same profile name even with different values. >> > >> > Signed-off-by: Marton Balint <cus@passwd.hu> >> > --- >> > doc/APIchanges | 3 +++ >> > libavutil/opt.c | 3 ++- >> > libavutil/opt.h | 1 + >> > libavutil/version.h | 2 +- >> > 4 files changed, 7 insertions(+), 2 deletions(-) >> > [...] >> Ping for the series, is there anybody against the approach I took to move >> profile name constants to encoders? > I'm OK for this series Thanks for the feedback, pushed the series. Regards, Marton
diff --git a/doc/APIchanges b/doc/APIchanges index 75cfdb08b0..235888c174 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2020-05-xx - xxxxxxxxxx - lavu 56.44.101 - opt.h + Add AV_OPT_FLAG_CHILD_CONSTS. + 2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions and num_enabled_dev_extensions fields to AVVulkanDeviceContext diff --git a/libavutil/opt.c b/libavutil/opt.c index b792dec01c..423313bce2 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con } { - const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, 0); int res; int ci = 0; double const_values[64]; const char * const_names[64]; + int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? AV_OPT_SEARCH_CHILDREN : 0; + const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, o->unit, 0, search_flags); if (o_named && o_named->type == AV_OPT_TYPE_CONST) d = DEFAULT_NUMVAL(o_named); else { diff --git a/libavutil/opt.h b/libavutil/opt.h index 1969c984dd..e46119572a 100644 --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -291,6 +291,7 @@ typedef struct AVOption { #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which can be set by the user at runtime #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information +#define AV_OPT_FLAG_CHILD_CONSTS (1<<18) ///< set if option constants can also reside in child objects //FIXME think about enc-audio, ... style flags /** diff --git a/libavutil/version.h b/libavutil/version.h index 48d8a38c42..c4946c1c7e 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -80,7 +80,7 @@ #define LIBAVUTIL_VERSION_MAJOR 56 #define LIBAVUTIL_VERSION_MINOR 44 -#define LIBAVUTIL_VERSION_MICRO 100 +#define LIBAVUTIL_VERSION_MICRO 101 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ LIBAVUTIL_VERSION_MINOR, \
This will be used for AVCodecContext->profile. By specifying constants in the encoders we won't have to use the common AVCodecContext options table and different encoders can use the same profile name even with different values. Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/APIchanges | 3 +++ libavutil/opt.c | 3 ++- libavutil/opt.h | 1 + libavutil/version.h | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-)