[FFmpeg-devel] [PATCH 2/3] - libavcodec/aom_film_grain: Add apply_grain flag

[Christophe Gisquet]

Hello.

Disclaimer: I'm not the owner/maintainer of any of that code, nor do I
know who is.

Le ven. 20 sept. 2024 à 02:44, Segall, Andrew via ffmpeg-devel
<ffmpeg-devel at ffmpeg.org> a écrit :
> case AV_FILM_GRAIN_PARAMS_AV1:
> - ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp);
> + if(fgp->apply_grain)
> + ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp);
> + else
> + out->needs_fg = 0;
> break;
> }

Let's imagine AFGS1 would be used for VVC-coded content. It then feels
like that logic should be in ff_aom_apply_film_grain, but then
apparently, the HEVC frame object and the process has to be at the
HEVC decoder level, and not more generic ones.
But see below why I'm really commenting this.

> @@ -241,6 +241,11 @@ typedef struct AVFilmGrainParams {
> */
> enum AVFilmGrainParamsType type;
>
>
> + /**
> + * Specifies if film grain parameters are enabled.
> + */
> + int apply_grain;
> +
> /**
> * Seed to use for the synthesis process, if the codec allows for it.

While it's not abnormal that a generic struct holds things only valid
in a few cases, this flag doesn't apply to H.274, and is really
processed by AFGS1-aware code, so I'm wondering if that apply_grain
should really be in AVFilmGrainAOMParams?
Lynne's comment still applies, though.

-- 
Christophe