[FFmpeg-devel] avcodec/mpeg4videodec: add forgotten flags to mpeg4_options

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/mpeg4videodec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Tue, Mar 02, 2021 at 07:53:07PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavcodec/mpeg4videodec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> index fd985f0430..43c2cf2dc3 100644
> --- a/libavcodec/mpeg4videodec.c
> +++ b/libavcodec/mpeg4videodec.c
> @@ -3551,9 +3551,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
>      return 0;
>  }
>  
> +#define OFFSET(x) offsetof(MpegEncContext, x)
> +#define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
>  static const AVOption mpeg4_options[] = {
> -    {"quarter_sample", "1/4 subpel MC", offsetof(MpegEncContext, quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
> -    {"divx_packed", "divx style packed b frames", offsetof(MpegEncContext, divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
> +    {"quarter_sample", "1/4 subpel MC", OFFSET(quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
> +    {"divx_packed", "divx style packed b frames", OFFSET(divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
>      {NULL}
>  };

this looks odd, divx_packed is set by the decoder not by the user

[...]

Michael Niedermayer:
> On Tue, Mar 02, 2021 at 07:53:07PM +0100, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/mpeg4videodec.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
>> index fd985f0430..43c2cf2dc3 100644
>> --- a/libavcodec/mpeg4videodec.c
>> +++ b/libavcodec/mpeg4videodec.c
>> @@ -3551,9 +3551,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>      return 0;
>>  }
>>  
>> +#define OFFSET(x) offsetof(MpegEncContext, x)
>> +#define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
>>  static const AVOption mpeg4_options[] = {
>> -    {"quarter_sample", "1/4 subpel MC", offsetof(MpegEncContext, quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
>> -    {"divx_packed", "divx style packed b frames", offsetof(MpegEncContext, divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
>> +    {"quarter_sample", "1/4 subpel MC", OFFSET(quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
>> +    {"divx_packed", "divx style packed b frames", OFFSET(divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
>>      {NULL}
>>  };
> 
> this looks odd, divx_packed is set by the decoder not by the user
> 
You added it in f51e5015ad76e5fae57e34712f6a6e4f0f8b2204 and the commit
message says that it is to export said information to the user. Which is
indeed really odd.

- Andreas

On Wed, Mar 03, 2021 at 12:49:16PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Tue, Mar 02, 2021 at 07:53:07PM +0100, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> >> ---
> >>  libavcodec/mpeg4videodec.c | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> >> index fd985f0430..43c2cf2dc3 100644
> >> --- a/libavcodec/mpeg4videodec.c
> >> +++ b/libavcodec/mpeg4videodec.c
> >> @@ -3551,9 +3551,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
> >>      return 0;
> >>  }
> >>  
> >> +#define OFFSET(x) offsetof(MpegEncContext, x)
> >> +#define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> >>  static const AVOption mpeg4_options[] = {
> >> -    {"quarter_sample", "1/4 subpel MC", offsetof(MpegEncContext, quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
> >> -    {"divx_packed", "divx style packed b frames", offsetof(MpegEncContext, divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0},
> >> +    {"quarter_sample", "1/4 subpel MC", OFFSET(quarter_sample), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
> >> +    {"divx_packed", "divx style packed b frames", OFFSET(divx_packed), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD},
> >>      {NULL}
> >>  };
> > 
> > this looks odd, divx_packed is set by the decoder not by the user
> > 
> You added it in f51e5015ad76e5fae57e34712f6a6e4f0f8b2204 and the commit
> message says that it is to export said information to the user. Which is
> indeed really odd.

what i meant is its odd to set VD making it accessible to the user to set
(which it previously was not) instead of adding 
AV_OPT_FLAG_READONLY /AV_OPT_FLAG_EXPORT
The original commit is not that odd as it was in 2011 and 
AV_OPT_FLAG_READONLY /AV_OPT_FLAG_EXPORT where added in 2014
in 2011 a export flag in the API was possible by not marking it as for
any decoder/encoder/video/audio

Thanks

[...]