| Message ID | 20240315120442.73754-3-ffmpeg@haasn.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | AFGS1 film grain support | expand |
| 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 |
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; > }
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)
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 --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; }
From: Niklas Haas <git@haasn.dev> Unused by AV1, but should still be set properly. --- libavcodec/av1dec.c | 4 ++++ 1 file changed, 4 insertions(+)