[FFmpeg-devel] [PATCH 1/2] lavf: allow setting AVStream.discard as an AVOption

[Anton Khirnov]

Quoting James Almer (2023-12-20 04:02:37)
> On 12/19/2023 9:10 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-12-19 13:09:05)
> >> On 12/19/2023 7:41 AM, Anton Khirnov wrote:
> >>> Quoting James Almer (2023-12-18 20:30:47)
> >>>> On 12/18/2023 4:19 PM, Anton Khirnov wrote:
> >>>>> ---
> >>>>>     libavformat/options.c | 10 ++++++++++
> >>>>>     1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/libavformat/options.c b/libavformat/options.c
> >>>>> index bf6113ca95..cc89dd6c72 100644
> >>>>> --- a/libavformat/options.c
> >>>>> +++ b/libavformat/options.c
> >>>>> @@ -229,6 +229,16 @@ static const AVOption stream_options[] = {
> >>>>>             { "metadata",           .type = AV_OPT_TYPE_CONST, { .i64 = AV_DISPOSITION_METADATA          },    .unit = "disposition" },
> >>>>>             { "dependent",          .type = AV_OPT_TYPE_CONST, { .i64 = AV_DISPOSITION_DEPENDENT         },    .unit = "disposition" },
> >>>>>             { "still_image",        .type = AV_OPT_TYPE_CONST, { .i64 = AV_DISPOSITION_STILL_IMAGE       },    .unit = "disposition" },
> >>>>> +
> >>>>> +    { "discard", NULL, offsetof(AVStream, discard), AV_OPT_TYPE_INT, { .i64 = AVDISCARD_DEFAULT }, INT_MIN, INT_MAX,
> >>>>> +        .flags = AV_OPT_FLAG_DECODING_PARAM, .unit = "avdiscard" },
> >>>>> +        { "none",               .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_NONE     }, .unit = "avdiscard" },
> >>>>> +        { "default",            .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_DEFAULT  }, .unit = "avdiscard" },
> >>>>> +        { "noref",              .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_NONREF   }, .unit = "avdiscard" },
> >>>>> +        { "bidir",              .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_BIDIR    }, .unit = "avdiscard" },
> >>>>> +        { "nointra",            .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_NONINTRA }, .unit = "avdiscard" },
> >>>>> +        { "nokey",              .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_NONKEY   }, .unit = "avdiscard" },
> >>>>> +        { "all",                .type = AV_OPT_TYPE_CONST, {.i64 = AVDISCARD_ALL      }, .unit = "avdiscard" },
> >>>>>         { NULL }
> >>>>>     };
> >>>>
> >>>> Should be ok.
> >>>>
> >>>> Maybe also add "id" like i did for AVStreamGroup while at it.
> >>>
> >>> The problem with id is how to flag it - it's read-only for demuxing and
> >>> user-settable for muxing.
> >>
> >> Wouldn't the AV_OPT_FLAG_ENCODING_PARAM flag be enough for the CLI to
> >> reject it for demuxing cases?
> > 
> > Those flags should be set for all callers, not just the CLI. And for
> 
> Of course, but by flagging it as AV_OPT_FLAG_ENCODING_PARAM any caller, 
> CLI included, can make sure to not attempt to set it on demuxing scenarios.
> 
> > demuxing the proper flag is AV_OPT_FLAG_EXPORT
> 
> Agree, that flag should be set too for id.
> 
> I don't think ENCODING_PARAM and EXPORT are incompatible. The doxy for 
> the former states "a generic parameter which can be set by the user for 
> muxing or encoding" and for the latter "The option is intended for 
> exporting values to the caller", which fulfills both of the requirements 
> you listed above: The caller can't set it on demuxing scenarios but can 
> read it to see what a demuxer wrote to it, and can both set it on muxing 
> scenarios and look at what a muxer may have written on it.

Setting both makes the flags useless. The entire point of
AV_OPT_FLAG_EXPORT is to allow the caller to generically identify
options whose values are meant to be read rather than set. If it cannot
be used for that purpose, then why have it at all?

-- 
Anton Khirnov