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

From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavf: allow setting AVStream.discard as an AVOption
Date: Wed, 20 Dec 2023 00:02:37 -0300
Message-ID: <840608eb-8284-45da-95b3-eb997fee0194@gmail.com> (raw)
In-Reply-To: <170298785392.8914.2659869163650906462@lain.khirnov.net>

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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".