[FFmpeg-devel] [PATCH v2 2/3] libavcodec: add a new AV_CODEC_EXPORT_DATA_FILM_GRAIN flag and option

[James Almer]

On 11/16/2020 3:16 AM, Anton Khirnov wrote:
> Quoting Lynne (2020-11-14 15:32:35)
>> Nov 14, 2020, 11:23 by anton at khirnov.net:
>>
>>> Quoting Lynne (2020-11-12 18:42:22)
>>>
>>>> This introduces a new field to allow decoders to export their film grain
>>>> parameters.
>>>> Will be used by the next patch.
>>>>
>>>> Patch attached.
>>>>  From d5d5e1e5f90938ac5cfa462efc13658ab411246b Mon Sep 17 00:00:00 2001
>>>> From: Lynne <dev at lynne.ee>
>>>> Date: Thu, 12 Nov 2020 17:46:09 +0100
>>>> Subject: [PATCH v2 2/3] libavcodec: add a new AV_CODEC_EXPORT_DATA_FILM_GRAIN
>>>>   flag and option
>>>>
>>>> This introduces a new field to allow decoders to export their film grain
>>>> parameters.
>>>> Will be used by the next patch.
>>>> ---
>>>>   doc/APIchanges             | 3 +++
>>>>   libavcodec/avcodec.h       | 5 +++++
>>>>   libavcodec/options_table.h | 1 +
>>>>   libavcodec/version.h       | 4 ++--
>>>>   4 files changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>>> index 41248724d9..9d0ddb4ff6 100644
>>>> --- a/doc/APIchanges
>>>> +++ b/doc/APIchanges
>>>> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>>>>   
>>>>   API changes, most recent first:
>>>>   
>>>> +2020-xx-xx - xxxxxxxxxx - lavc 58.113.100 - avcodec.h
>>>> +  Adds a new flag AV_CODEC_EXPORT_DATA_FILM_GRAIN for export_side_data.
>>>> +
>>>>   2020-xx-xx - xxxxxxxxxx - lavu 56.61.100 - film_grain_params.h
>>>>   Adds a new API for extracting codec film grain parameters as side data.
>>>>   Adds a new AVFrameSideDataType entry AV_FRAME_DATA_FILM_GRAIN_PARAMS for it.
>>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>>> index 20af3ef00d..5047da0f6a 100644
>>>> --- a/libavcodec/avcodec.h
>>>> +++ b/libavcodec/avcodec.h
>>>> @@ -410,6 +410,11 @@ typedef struct RcOverride{
>>>>   * Export the AVVideoEncParams structure through frame side data.
>>>>   */
>>>>   #define AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS (1 << 2)
>>>> +/**
>>>> + * Decoding only.
>>>> + * Do not apply film grain, export it instead.
>>>>
>>>
>>> Could someone want to both apply AND export it?
>>>
>>
>> Analyzers. In which case they could use a film grain filter to apply it. But that's a fringe
>> use-case for this flag, this mainly targets video players and transcoding, where the
>> bandwidth/frames savings can be significant.
>> There's a WIP Vulkan filter to apply it, and if there's popular demand, the libdav1d code
>> could be ported as a software filter.
> 
> Then the documentation should allow for applying and exporting.
> Also, this flag is supposed to just trigger exporting some metadata, I
> am not very comfortable with it changing decoder output.

There's a precedent with AV_CODEC_FLAG2_SKIP_MANUAL, but that's a global 
decoder flag. An "export_side_data" option should probably not affect 
decoding output, agree.

> 
> What's the current status of film grain across the various sw/hw
> decoders we have? IIUC some apply it transparently, some don't?

Right now, libdav1d has an option to toggle it, and it's applied to the 
frames before they are returned to libavcodec. libaom-av1 does not seem 
to have an option for it at all, so i assume it's unconditionally 
applied if present in the bitstream passed to it. And I have no idea 
what the qsv decoder does.

The "native" decoder, currently being only hwaccel hooks, passes FG 
fields if present to the underlying hwaccels, which i assume will apply 
it to the decoded frame.
A software implementation could easily be made to behave like dav1d, but 
for the hwaccels, if we want to make it optional, then i guess it's a 
matter of not setting the relevant FG fields in hwaccel->start_frame(). 
The hw decoders would be none the wiser.

> Should there be a way for the user to check?

You mean something like an AV_CODEC_CAP_?

> And/or a common option for "do not apply/apply (if possible)/auto (apply
> if export not requested)"?

A common option would be good, but as i mentioned above, not all 
decoders can be configured (or even have them export anything).