Re: [FFmpeg-devel] [PATCH v3 5/9] avutil/film_grain_params: add av_film_grain_params_select()

[Niklas Haas <ffmpeg@haasn.xyz>]

On Fri, 15 Mar 2024 12:58:58 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Common utility function that can be used by all codecs to select the
> right (any valid) film grain parameter set. In particular, this is
> useful for AFGS1, which has support for multiple parameters.
> 
> However, it also performs parameter validation for H274.
> ---
>  doc/APIchanges                |  3 ++
>  libavutil/film_grain_params.c | 57 +++++++++++++++++++++++++++++++++++
>  libavutil/film_grain_params.h |  7 +++++
>  libavutil/version.h           |  2 +-
>  4 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 5a192b600af..34245c8b708 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>  
>  API changes, most recent first:
>  
> +2024-03-08 - xxxxxxxxxx - lavu 59.3.100 - film_grain_params.h
> +  Add av_film_grain_params_select().
> +
>  2024-03-08 - xxxxxxxxxx - lavu 59.2.100 - film_grain_params.h
>    Add AVFilmGrainAOMParams.color_range, color_primaries, color_trc,
>    color_space, width, height, subx, suby and bit_depth.
> diff --git a/libavutil/film_grain_params.c b/libavutil/film_grain_params.c
> index 930d23c7fe9..8c80adc66a7 100644
> --- a/libavutil/film_grain_params.c
> +++ b/libavutil/film_grain_params.c
> @@ -17,6 +17,7 @@
>   */
>  
>  #include "film_grain_params.h"
> +#include "pixdesc.h"
>  
>  AVFilmGrainParams *av_film_grain_params_alloc(size_t *size)
>  {
> @@ -40,3 +41,59 @@ AVFilmGrainParams *av_film_grain_params_create_side_data(AVFrame *frame)
>  
>      return (AVFilmGrainParams *)side_data->data;
>  }
> +
> +const AVFilmGrainParams *av_film_grain_params_select(const AVFrame *frame)
> +{
> +    const AVFilmGrainParams *fgp, *best = NULL;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> +    const AVFilmGrainAOMParams *aom;
> +    const AVFilmGrainH274Params *h274;
> +    if (!desc)
> +        return NULL;
> +
> +#define CHECK(a, b, unspec)                                     \
> +    do {                                                        \
> +        if ((a) != (unspec) && (b) != (unspec) && (a) != (b))   \
> +            continue;                                           \
> +    } while (0)
> +
> +    for (int i = 0; i < frame->nb_side_data; i++) {
> +        if (frame->side_data[i]->type != AV_FRAME_DATA_FILM_GRAIN_PARAMS)
> +            continue;
> +        fgp = (const AVFilmGrainParams*)frame->side_data[i]->data;
> +        switch (fgp->type) {
> +        case AV_FILM_GRAIN_PARAMS_NONE:
> +            continue;
> +        case AV_FILM_GRAIN_PARAMS_AV1:
> +            aom = &fgp->codec.aom;
> +            if (aom->subx != desc->log2_chroma_w || aom->suby != desc->log2_chroma_h)
> +                continue;
> +            CHECK(aom->bit_depth,       desc->comp[0].depth, 0);
> +            CHECK(aom->width,           frame->width, 0);
> +            CHECK(aom->height,          frame->height, 0);

This condition is wrong, it should allow all aom->width <= frame->width;
not just an exact match.

> +            CHECK(aom->color_range,     frame->color_range,     AVCOL_RANGE_UNSPECIFIED);
> +            CHECK(aom->color_primaries, frame->color_primaries, AVCOL_PRI_UNSPECIFIED);
> +            CHECK(aom->color_trc,       frame->color_trc,       AVCOL_TRC_UNSPECIFIED);
> +            CHECK(aom->color_space,     frame->colorspace,      AVCOL_SPC_UNSPECIFIED);
> +
> +            if (!best ||
> +                best->codec.aom.width < aom->width ||
> +                best->codec.aom.height < aom->height)
> +                best = fgp;
> +            break;
> +        case AV_FILM_GRAIN_PARAMS_H274:
> +            h274 = &fgp->codec.h274;
> +            /* There are no YUV formats with different bit depth per component,
> +             * so just check both against the first component for simplicity */
> +            CHECK(h274->bit_depth_luma,   desc->comp[0].depth, 0);
> +            CHECK(h274->bit_depth_chroma, desc->comp[0].depth, 0);
> +            CHECK(h274->color_range,      frame->color_range,     AVCOL_RANGE_UNSPECIFIED);
> +            CHECK(h274->color_primaries,  frame->color_primaries, AVCOL_PRI_UNSPECIFIED);
> +            CHECK(h274->color_trc,        frame->color_trc,       AVCOL_TRC_UNSPECIFIED);
> +            CHECK(h274->color_space,      frame->colorspace,      AVCOL_SPC_UNSPECIFIED);
> +            return fgp; /* H274 can't have multiple resolutions */
> +        }
> +    }
> +
> +    return best;
> +}
> diff --git a/libavutil/film_grain_params.h b/libavutil/film_grain_params.h
> index 17fefeb06c3..75c020ceb92 100644
> --- a/libavutil/film_grain_params.h
> +++ b/libavutil/film_grain_params.h
> @@ -272,4 +272,11 @@ AVFilmGrainParams *av_film_grain_params_alloc(size_t *size);
>   */
>  AVFilmGrainParams *av_film_grain_params_create_side_data(AVFrame *frame);
>  
> +/**
> + * Select the most appropriate film grain parameters set for the frame,
> + * taking into account the frame's format, resolution and video signal
> + * characteristics.
> + */
> +const AVFilmGrainParams *av_film_grain_params_select(const AVFrame *frame);
> +
>  #endif /* AVUTIL_FILM_GRAIN_PARAMS_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 57cad02ec0a..5027b025be4 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  59
> -#define LIBAVUTIL_VERSION_MINOR   2
> +#define LIBAVUTIL_VERSION_MINOR   3
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> -- 
> 2.44.0
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".