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

[Segall, Andrew]

On 9/20/24, 12:15 AM, "ffmpeg-devel on behalf of Lynne via ffmpeg-devel" <ffmpeg-devel-bounces at ffmpeg.org <mailto:ffmpeg-devel-bounces at ffmpeg.org> on behalf of ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>> wrote:


CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.






On 20/09/2024 02:44, Segall, Andrew via ffmpeg-devel wrote:
> Details: Add support for the apply_grain_flag. This allows the film grain process to be enabled/disabled for different display properties.
>
> On 9/8/24, 12:06 AM, "Andrew Segall" <asegall at amazon.com <mailto:asegall at amazon.com> <mailto:asegall at amazon.com <mailto:asegall at amazon.com>>> wrote:
>
>
> Signed-off-by: Andrew Segall <asegall at amazon.com <mailto:asegall at amazon.com> <mailto:asegall at amazon.com <mailto:asegall at amazon.com>>>
> ---
> libavcodec/aom_film_grain.c | 6 ++++--
> libavcodec/hevc/hevcdec.c | 5 ++++-
> libavfilter/vf_showinfo.c | 1 +
> libavutil/film_grain_params.h | 5 +++++
> 4 files changed, 14 insertions(+), 3 deletions(-)
>
>
> diff --git a/libavcodec/aom_film_grain.c b/libavcodec/aom_film_grain.c
> index fdfa3dbf77..251a2793ac 100644
> --- a/libavcodec/aom_film_grain.c
> +++ b/libavcodec/aom_film_grain.c
> @@ -149,8 +149,10 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s,
> fgp = &s->sets[set_idx];
> aom = &fgp->codec.aom;
>
>
> - fgp->type = get_bits1(gb) ? AV_FILM_GRAIN_PARAMS_AV1 : AV_FILM_GRAIN_PARAMS_NONE;
> - if (!fgp->type)
> + fgp->type = AV_FILM_GRAIN_PARAMS_AV1;
> +
> + fgp->apply_grain = get_bits1(gb);
> + if ( !fgp->apply_grain)
> {
> payload_bits = get_bits_count(gb) - start_position;
> if (payload_bits > payload_size * 8)
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index d915d74d22..8dd4fb10c5 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -3155,7 +3155,10 @@ static int hevc_frame_end(HEVCContext *s)
> &s->h274db, fgp);
> break;
> 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;
> }
> av_assert1(ret >= 0);
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index f81df9d1bf..08e144ca46 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -455,6 +455,7 @@ static void dump_sei_film_grain_params_metadata(AVFilterContext *ctx, const AVFr
> }
>
>
> av_log(ctx, AV_LOG_INFO, "type %s; ", film_grain_type_names[fgp->type]);
> + av_log(ctx, AV_LOG_INFO, "apply_grain=%d; ", fgp->apply_grain);
> av_log(ctx, AV_LOG_INFO, "seed=%"PRIu64"; ", fgp->seed);
> av_log(ctx, AV_LOG_INFO, "width=%d; ", fgp->width);
> av_log(ctx, AV_LOG_INFO, "height=%d; ", fgp->height);
> diff --git a/libavutil/film_grain_params.h b/libavutil/film_grain_params.h
> index ccacab88fe..f3275923e1 100644
> --- a/libavutil/film_grain_params.h
> +++ b/libavutil/film_grain_params.h
> @@ -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.
> *


> This is an ABI break.

> Furthermore, what exactly does this improve over simply stripping the side data?

Mainly addressing conformance issues.

Example is that we have two parameters in the AFGS1 message.  The first is associated with resolution A and apply_grain=1.  Second is associated with resolution B and apply_grain=0.

If the synthesis Is performed at resolution B, then the message says that we shouldn't perform film grain synthesis since apply_grain=0.  However, if we strip, the information is lost - and synthesis may use the parameters associated with resolution A instead.