[FFmpeg-devel,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.

Patch attached.
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(-)

On 11/12/2020 2:42 PM, Lynne wrote:
> 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@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.
> + */
> +#define AV_CODEC_EXPORT_DATA_FILM_GRAIN (1 << 3)
>  
>  /**
>   * Pan Scan area.
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 66bda42663..77e1211ae0 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -83,6 +83,7 @@ static const AVOption avcodec_options[] = {
>  {"mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
>  {"prft", "export Producer Reference Time through packet side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX, A|V|S|E, "export_side_data"},
>  {"venc_params", "export video encoding parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
> +{"export_film_grain", "export film grain parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_FILM_GRAIN}, INT_MIN, INT_MAX, V|D, "export_side_data"},

Just "film_grain". It's a flag for an option called export_side_data, so 
no need to be redundant.

>  {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0, INT_MAX},
>  {"g", "set the group of picture (GOP) size", OFFSET(gop_size), AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
>  {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 1585d95777..37fa426f40 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,8 +28,8 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR  58
> -#define LIBAVCODEC_VERSION_MINOR 112
> -#define LIBAVCODEC_VERSION_MICRO 103
> +#define LIBAVCODEC_VERSION_MINOR 113
> +#define LIBAVCODEC_VERSION_MICRO 100
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> -- 
> 2.29.2
>

Nov 12, 2020, 22:39 by jamrial@gmail.com:

> On 11/12/2020 2:42 PM, Lynne wrote:
>
>> 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@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.
>> + */
>> +#define AV_CODEC_EXPORT_DATA_FILM_GRAIN (1 << 3)
>>  /**
>>  * Pan Scan area.
>> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
>> index 66bda42663..77e1211ae0 100644
>> --- a/libavcodec/options_table.h
>> +++ b/libavcodec/options_table.h
>> @@ -83,6 +83,7 @@ static const AVOption avcodec_options[] = {
>>  {"mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
>>  {"prft", "export Producer Reference Time through packet side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX, A|V|S|E, "export_side_data"},
>>  {"venc_params", "export video encoding parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
>> +{"export_film_grain", "export film grain parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_FILM_GRAIN}, INT_MIN, INT_MAX, V|D, "export_side_data"},
>>
>
> Just "film_grain". It's a flag for an option called export_side_data, so no need to be redundant.
>

Makes sense. Changed locally, thanks.

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@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?

Nov 14, 2020, 11:23 by anton@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@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.

Quoting Lynne (2020-11-14 15:32:35)
> Nov 14, 2020, 11:23 by anton@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@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.

What's the current status of film grain across the various sw/hw
decoders we have? IIUC some apply it transparently, some don't?
Should there be a way for the user to check?
And/or a common option for "do not apply/apply (if possible)/auto (apply
if export not requested)"?

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@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@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).

In terms of an API user needing this functionality, the way I see it
working is that presence of the side-data will imply that it still needs
to be applied, with the side data being removed the moment grain
actually is applied. So that means that decoders applying film grain
must never export this side data, and decoders not applying film grain
must always export this side data.

A decoder both applying grain and exporting the side data is
nonsensical, and would only result in double-grain-application. In terms
of the capability checking, I don't need you to tell me whether or not
the decoder can apply/export side data or not, rather, I need to tell
*you* whether or not I can apply film grain myself. This is what I
understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
actually exported when that flag is present is irrelevant to me, all I'm
trying to do is signaling that I'd prefer film grain to be exported
rather than applied.

On Mon, 16 Nov 2020 11:34:05 -0300, James Almer <jamrial@gmail.com> wrote:
> 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@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@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).
> _______________________________________________
> 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".

Nov 16, 2020, 15:56 by ffmpeg@haasn.xyz:

> In terms of an API user needing this functionality, the way I see it
> working is that presence of the side-data will imply that it still needs
> to be applied, with the side data being removed the moment grain
> actually is applied. So that means that decoders applying film grain
> must never export this side data, and decoders not applying film grain
> must always export this side data.
>
> A decoder both applying grain and exporting the side data is
> nonsensical, and would only result in double-grain-application. In terms
> of the capability checking, I don't need you to tell me whether or not
> the decoder can apply/export side data or not, rather, I need to tell
> *you* whether or not I can apply film grain myself. This is what I
> understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
> actually exported when that flag is present is irrelevant to me, all I'm
> trying to do is signaling that I'd prefer film grain to be exported
> rather than applied.
>

Yup, that's my idea of how this should work as well.
Maybe even analyzers should want to skip applying film grain in a majority
of cases or apply it independently. So I still think of having an export
flag control the decoder behavior is acceptable in this case.

Quoting Lynne (2020-11-17 00:21:43)
> Nov 16, 2020, 15:56 by ffmpeg@haasn.xyz:
> 
> > In terms of an API user needing this functionality, the way I see it
> > working is that presence of the side-data will imply that it still needs
> > to be applied, with the side data being removed the moment grain
> > actually is applied. So that means that decoders applying film grain
> > must never export this side data, and decoders not applying film grain
> > must always export this side data.
> >
> > A decoder both applying grain and exporting the side data is
> > nonsensical, and would only result in double-grain-application. In terms
> > of the capability checking, I don't need you to tell me whether or not
> > the decoder can apply/export side data or not, rather, I need to tell
> > *you* whether or not I can apply film grain myself. This is what I
> > understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
> > actually exported when that flag is present is irrelevant to me, all I'm
> > trying to do is signaling that I'd prefer film grain to be exported
> > rather than applied.
> >
> 
> Yup, that's my idea of how this should work as well.
> Maybe even analyzers should want to skip applying film grain in a majority
> of cases or apply it independently. So I still think of having an export
> flag control the decoder behavior is acceptable in this case.

It's not about majority vs. minority, the question is
"is there EVER a valid reason to apply AND export?"
If the answer is yes, then there needs to be a way to do that. The flag
you're proposing explicitly says you can do either one or the other, but
never both.

Nov 17, 2020, 10:58 by anton@khirnov.net:

> Quoting Lynne (2020-11-17 00:21:43)
>
>> Nov 16, 2020, 15:56 by ffmpeg@haasn.xyz:
>>
>> > In terms of an API user needing this functionality, the way I see it
>> > working is that presence of the side-data will imply that it still needs
>> > to be applied, with the side data being removed the moment grain
>> > actually is applied. So that means that decoders applying film grain
>> > must never export this side data, and decoders not applying film grain
>> > must always export this side data.
>> >
>> > A decoder both applying grain and exporting the side data is
>> > nonsensical, and would only result in double-grain-application. In terms
>> > of the capability checking, I don't need you to tell me whether or not
>> > the decoder can apply/export side data or not, rather, I need to tell
>> > *you* whether or not I can apply film grain myself. This is what I
>> > understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
>> > actually exported when that flag is present is irrelevant to me, all I'm
>> > trying to do is signaling that I'd prefer film grain to be exported
>> > rather than applied.
>> >
>>
>> Yup, that's my idea of how this should work as well.
>> Maybe even analyzers should want to skip applying film grain in a majority
>> of cases or apply it independently. So I still think of having an export
>> flag control the decoder behavior is acceptable in this case.
>>
>
> It's not about majority vs. minority, the question is
> "is there EVER a valid reason to apply AND export?"
> If the answer is yes, then there needs to be a way to do that. The flag
> you're proposing explicitly says you can do either one or the other, but
> never both.
>

That's a kinda black and white view. We have both APIs that don't allow very
common cases to be handled and are creating us problems and we have APIs
which allow for completely unreasonable things to happen and are creating
us problems as well.
I don't think there's a reasonable non-contrived case in which you'd want to
both export and apply film grain. But I also didn't think there's a reasonable
case to ever need to chain demuxers and decoders.

On 11/17/2020 8:15 AM, Lynne wrote:
> Nov 17, 2020, 10:58 by anton@khirnov.net:
> 
>> Quoting Lynne (2020-11-17 00:21:43)
>>
>>> Nov 16, 2020, 15:56 by ffmpeg@haasn.xyz:
>>>
>>>> In terms of an API user needing this functionality, the way I see it
>>>> working is that presence of the side-data will imply that it still needs
>>>> to be applied, with the side data being removed the moment grain
>>>> actually is applied. So that means that decoders applying film grain
>>>> must never export this side data, and decoders not applying film grain
>>>> must always export this side data.
>>>>
>>>> A decoder both applying grain and exporting the side data is
>>>> nonsensical, and would only result in double-grain-application. In terms
>>>> of the capability checking, I don't need you to tell me whether or not
>>>> the decoder can apply/export side data or not, rather, I need to tell
>>>> *you* whether or not I can apply film grain myself. This is what I
>>>> understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
>>>> actually exported when that flag is present is irrelevant to me, all I'm
>>>> trying to do is signaling that I'd prefer film grain to be exported
>>>> rather than applied.
>>>>
>>>
>>> Yup, that's my idea of how this should work as well.
>>> Maybe even analyzers should want to skip applying film grain in a majority
>>> of cases or apply it independently. So I still think of having an export
>>> flag control the decoder behavior is acceptable in this case.
>>>
>>
>> It's not about majority vs. minority, the question is
>> "is there EVER a valid reason to apply AND export?"
>> If the answer is yes, then there needs to be a way to do that. The flag
>> you're proposing explicitly says you can do either one or the other, but
>> never both.
>>
> 
> That's a kinda black and white view. We have both APIs that don't allow very
> common cases to be handled and are creating us problems and we have APIs
> which allow for completely unreasonable things to happen and are creating
> us problems as well.
> I don't think there's a reasonable non-contrived case in which you'd want to
> both export and apply film grain. But I also didn't think there's a reasonable
> case to ever need to chain demuxers and decoders.

Just to check, The existence of this flag as it was defined implies 
there should not be an "apply grain" avoption in decoders that can 
export such params as side data, to avoid conflicts, right? The flag 
alone is meant to control if film grain is applied, or exported.

Now, what about decoders that can't export film grain params as side 
data? Take for example QSV, which may not allow it. Should such decoders 
offer an avoption to apply film grain or not? For those, the export flag 
can't be used to disable applying film grain because it can't also 
fulfill the "export" part of its purpose.

Nov 26, 2020, 16:21 by jamrial@gmail.com:

> On 11/17/2020 8:15 AM, Lynne wrote:
>
>> Nov 17, 2020, 10:58 by anton@khirnov.net:
>>
>>> Quoting Lynne (2020-11-17 00:21:43)
>>>
>>>> Nov 16, 2020, 15:56 by ffmpeg@haasn.xyz:
>>>>
>>>>> In terms of an API user needing this functionality, the way I see it
>>>>> working is that presence of the side-data will imply that it still needs
>>>>> to be applied, with the side data being removed the moment grain
>>>>> actually is applied. So that means that decoders applying film grain
>>>>> must never export this side data, and decoders not applying film grain
>>>>> must always export this side data.
>>>>>
>>>>> A decoder both applying grain and exporting the side data is
>>>>> nonsensical, and would only result in double-grain-application. In terms
>>>>> of the capability checking, I don't need you to tell me whether or not
>>>>> the decoder can apply/export side data or not, rather, I need to tell
>>>>> *you* whether or not I can apply film grain myself. This is what I
>>>>> understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's
>>>>> actually exported when that flag is present is irrelevant to me, all I'm
>>>>> trying to do is signaling that I'd prefer film grain to be exported
>>>>> rather than applied.
>>>>>
>>>>
>>>> Yup, that's my idea of how this should work as well.
>>>> Maybe even analyzers should want to skip applying film grain in a majority
>>>> of cases or apply it independently. So I still think of having an export
>>>> flag control the decoder behavior is acceptable in this case.
>>>>
>>>
>>> It's not about majority vs. minority, the question is
>>> "is there EVER a valid reason to apply AND export?"
>>> If the answer is yes, then there needs to be a way to do that. The flag
>>> you're proposing explicitly says you can do either one or the other, but
>>> never both.
>>>
>>
>> That's a kinda black and white view. We have both APIs that don't allow very
>> common cases to be handled and are creating us problems and we have APIs
>> which allow for completely unreasonable things to happen and are creating
>> us problems as well.
>> I don't think there's a reasonable non-contrived case in which you'd want to
>> both export and apply film grain. But I also didn't think there's a reasonable
>> case to ever need to chain demuxers and decoders.
>>
>
> Just to check, The existence of this flag as it was defined implies there should not be an "apply grain" avoption in decoders that can export such params as side data, to avoid conflicts, right? The flag alone is meant to control if film grain is applied, or exported.
>
> Now, what about decoders that can't export film grain params as side data? Take for example QSV, which may not allow it. Should such decoders offer an avoption to apply film grain or not? For those, the export flag can't be used to disable applying film grain because it can't also fulfill the "export" part of its purpose.
>

I think if the decoder doesn't support exporting it, it should apply it under all circumstances,
as otherwise correct presentation would be affected.
I think this is a pretty fringe problem right now as it only affects decoders independent of
av1dec, and given its meant to be applied during presentation time, I think those decoders
should implement support for exporting it while its not too late rather than having debugging
options for disabling it, which aren't really useful outside of debugging.