diff mbox series

[FFmpeg-devel,v3,2/9] avcodec/av1dec: initialize AFGS1 VSC metadata

Message ID 20240315120442.73754-3-ffmpeg@haasn.xyz
State New
Headers show
Series AFGS1 film grain support | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Niklas Haas March 15, 2024, 11:58 a.m. UTC
From: Niklas Haas <git@haasn.dev>

Unused by AV1, but should still be set properly.
---
 libavcodec/av1dec.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

James Almer March 15, 2024, 12:20 p.m. UTC | #1
On 3/15/2024 8:58 AM, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Unused by AV1, but should still be set properly.

The doxy for av_film_grain_params_alloc() says "Allocate an 
AVFilmGrainParams structure and set its fields to default values", so 
this should be done there.

> ---
>   libavcodec/av1dec.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index e6346b51dbe..8a5796c757f 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -1124,6 +1124,10 @@ static int export_film_grain(AVCodecContext *avctx, AVFrame *frame)
>       aom->uv_mult_luma[1] = film_grain->cr_luma_mult;
>       aom->uv_offset[0] = film_grain->cb_offset;
>       aom->uv_offset[1] = film_grain->cr_offset;
> +    aom->color_range = AVCOL_RANGE_UNSPECIFIED;
> +    aom->color_primaries = AVCOL_PRI_UNSPECIFIED;
> +    aom->color_trc = AVCOL_TRC_UNSPECIFIED;
> +    aom->color_space = AVCOL_SPC_UNSPECIFIED;
>   
>       return 0;
>   }
Niklas Haas March 15, 2024, 12:23 p.m. UTC | #2
On Fri, 15 Mar 2024 09:20:04 -0300 James Almer <jamrial@gmail.com> wrote:
> On 3/15/2024 8:58 AM, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > Unused by AV1, but should still be set properly.
> 
> The doxy for av_film_grain_params_alloc() says "Allocate an 
> AVFilmGrainParams structure and set its fields to default values", so 
> this should be done there.

This function doesn't know whether the film grain is of type AV1 or
H274, and so cannot set the correct codec-specific parameter defaults.

Unless, of course, we add these fields to the common struct, as
discussed above. (Or if we add the type to the function signature)
James Almer March 15, 2024, 12:25 p.m. UTC | #3
On 3/15/2024 9:23 AM, Niklas Haas wrote:
> On Fri, 15 Mar 2024 09:20:04 -0300 James Almer <jamrial@gmail.com> wrote:
>> On 3/15/2024 8:58 AM, Niklas Haas wrote:
>>> From: Niklas Haas <git@haasn.dev>
>>>
>>> Unused by AV1, but should still be set properly.
>>
>> The doxy for av_film_grain_params_alloc() says "Allocate an
>> AVFilmGrainParams structure and set its fields to default values", so
>> this should be done there.
> 
> This function doesn't know whether the film grain is of type AV1 or
> H274, and so cannot set the correct codec-specific parameter defaults.
> 
> Unless, of course, we add these fields to the common struct, as
> discussed above. (Or if we add the type to the function signature)

We can't change the function signature, but we can add the fields to the 
common struct.
Just leave a comment about reordering the fields in the next bump (Which 
hopefully will not be forgotten).
diff mbox series

Patch

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index e6346b51dbe..8a5796c757f 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1124,6 +1124,10 @@  static int export_film_grain(AVCodecContext *avctx, AVFrame *frame)
     aom->uv_mult_luma[1] = film_grain->cr_luma_mult;
     aom->uv_offset[0] = film_grain->cb_offset;
     aom->uv_offset[1] = film_grain->cr_offset;
+    aom->color_range = AVCOL_RANGE_UNSPECIFIED;
+    aom->color_primaries = AVCOL_PRI_UNSPECIFIED;
+    aom->color_trc = AVCOL_TRC_UNSPECIFIED;
+    aom->color_space = AVCOL_SPC_UNSPECIFIED;
 
     return 0;
 }