diff mbox series

[FFmpeg-devel,4/4,v2] avcodec/av1: signal the presence of Film Grain in the decoder context

Message ID 20210816162424.9857-1-jamrial@gmail.com
State Accepted
Commit 9b05263ac166924f28a2f4a65883629219fa6689
Headers show
Series None | expand

Commit Message

James Almer Aug. 16, 2021, 4:24 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/av1dec.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Lynne Aug. 16, 2021, 4:35 p.m. UTC | #1
16 Aug 2021, 18:24 by jamrial@gmail.com:

> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/av1dec.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 1dda0f9160..a69808f7b6 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -575,6 +575,11 @@ static int set_context_with_sequence(AVCodecContext *avctx,
>  break;
>  }
>  
> +    if (seq->film_grain_params_present)
> +        avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
> +    else
> +        avctx->properties &= ~FF_CODEC_PROPERTY_FILM_GRAIN;
> +
>  if (avctx->width != width || avctx->height != height) {
>  int ret = ff_set_dimensions(avctx, width, height);
>  if (ret < 0)
>

Why do we need to signal whether decoders support it or not? Is it in case
we have decoders which don't support applying film grain yet can export it?
Or is it to perhaps to implicitly signal which types of film grain clients can get
in case they want to export film grain and apply it themselves, so they can
disable film grain exporting if they can't apply the type that the decoder outputs
yet?
James Almer Aug. 16, 2021, 4:45 p.m. UTC | #2
On 8/16/2021 1:35 PM, Lynne wrote:
> 16 Aug 2021, 18:24 by jamrial@gmail.com:
> 
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/av1dec.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index 1dda0f9160..a69808f7b6 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -575,6 +575,11 @@ static int set_context_with_sequence(AVCodecContext *avctx,
>>   break;
>>   }
>>   
>> +    if (seq->film_grain_params_present)
>> +        avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
>> +    else
>> +        avctx->properties &= ~FF_CODEC_PROPERTY_FILM_GRAIN;
>> +
>>   if (avctx->width != width || avctx->height != height) {
>>   int ret = ff_set_dimensions(avctx, width, height);
>>   if (ret < 0)
>>
> 
> Why do we need to signal whether decoders support it or not? Is it in case
> we have decoders which don't support applying film grain yet can export it?

This is signaling the presence of film grain regardless of how it's 
handled, not that the decoder supports it, in a way the library user can 
query.

If a decoder applies film grain instead of exporting it as side data, 
there's nothing currently letting the library user know it's there at all.

> Or is it to perhaps to implicitly signal which types of film grain clients can get
> in case they want to export film grain and apply it themselves, so they can
> disable film grain exporting if they can't apply the type that the decoder outputs
> yet?
James Almer Aug. 16, 2021, 4:55 p.m. UTC | #3
On 8/16/2021 1:45 PM, James Almer wrote:
> On 8/16/2021 1:35 PM, Lynne wrote:
>> 16 Aug 2021, 18:24 by jamrial@gmail.com:
>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavcodec/av1dec.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index 1dda0f9160..a69808f7b6 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -575,6 +575,11 @@ static int 
>>> set_context_with_sequence(AVCodecContext *avctx,
>>>   break;
>>>   }
>>> +    if (seq->film_grain_params_present)
>>> +        avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
>>> +    else
>>> +        avctx->properties &= ~FF_CODEC_PROPERTY_FILM_GRAIN;
>>> +
>>>   if (avctx->width != width || avctx->height != height) {
>>>   int ret = ff_set_dimensions(avctx, width, height);
>>>   if (ret < 0)
>>>
>>
>> Why do we need to signal whether decoders support it or not? Is it in 
>> case
>> we have decoders which don't support applying film grain yet can 
>> export it?
> 
> This is signaling the presence of film grain regardless of how it's 
> handled, not that the decoder supports it, in a way the library user can 
> query.

That being said, of course only decoders that support handling film 
grain in some form are going to set this flag to being with. The point 
is being able to do what i described below.

> 
> If a decoder applies film grain instead of exporting it as side data, 
> there's nothing currently letting the library user know it's there at all.
> 
>> Or is it to perhaps to implicitly signal which types of film grain 
>> clients can get
>> in case they want to export film grain and apply it themselves, so 
>> they can
>> disable film grain exporting if they can't apply the type that the 
>> decoder outputs
>> yet?
diff mbox series

Patch

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1dda0f9160..a69808f7b6 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -575,6 +575,11 @@  static int set_context_with_sequence(AVCodecContext *avctx,
         break;
     }
 
+    if (seq->film_grain_params_present)
+        avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
+    else
+        avctx->properties &= ~FF_CODEC_PROPERTY_FILM_GRAIN;
+
     if (avctx->width != width || avctx->height != height) {
         int ret = ff_set_dimensions(avctx, width, height);
         if (ret < 0)