[FFmpeg-devel] [PATCH 05/19] swscale/utils: add SwsFormat abstraction and helpers

[Niklas Haas]

On Fri, 11 Oct 2024 00:26:52 +0200 Niklas Haas <ffmpeg at haasn.xyz> wrote:
> From: Niklas Haas <git at haasn.dev>
>
> Groups together all relevant color metadata from an AVFrame. While we could
> use AVFrame directly, keeping it a separate struct has three advantages:
>
> 1. Functions accepting an SwsFormat will definitely not care about the
>    data pointers.
> 2. It clearly separates sanitized and raw metadata, since the function to
>    construct an SwsFormat from an AVFrame will also sanitize.
> 3. It's slightly more lightweight to pass around.
>
> Move these into a new header file "utils.h" to avoid crowding
> swscale_internal.h even more, and also to solve a circular dependency issue
> down the line.
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Niklas Haas <git at haasn.dev>
> ---
>  libswscale/utils.c | 65 ++++++++++++++++++++++++++++++++++++++++++
>  libswscale/utils.h | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
>  create mode 100644 libswscale/utils.h
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index d80a3f0a80..074be65410 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1,4 +1,5 @@
>  /*
> + * Copyright (C) 2024 Niklas Haas
>   * Copyright (C) 2001-2003 Michael Niedermayer <michaelni at gmx.at>
>   *
>   * This file is part of FFmpeg.
> @@ -59,6 +60,7 @@
>  #include "rgb2rgb.h"
>  #include "swscale.h"
>  #include "swscale_internal.h"
> +#include "utils.h"
>
>  typedef struct FormatEntry {
>      uint8_t is_supported_in         :1;
> @@ -2647,3 +2649,66 @@ int ff_range_add(RangeList *rl, unsigned int start, unsigned int len)
>
>      return 0;
>  }
> +
> +/**
> + * This function also sanitizes and strips the input data, removing irrelevant
> + * fields for certain formats.
> + */
> +SwsFormat ff_fmt_from_frame(const AVFrame *frame)
> +{
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> +    SwsFormat fmt = {
> +        .width  = frame->width,
> +        .height = frame->height,
> +        .format = frame->format,
> +        .range  = frame->color_range,
> +        .prim   = frame->color_primaries,
> +        .trc    = frame->color_trc,
> +        .csp    = frame->colorspace,
> +        .loc    = frame->chroma_location,
> +        .desc   = desc,
> +    };
> +
> +    av_assert1(fmt.width > 0);
> +    av_assert1(fmt.height > 0);
> +    av_assert1(fmt.format != AV_PIX_FMT_NONE);
> +    av_assert0(desc);
> +    if (desc->flags & (AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PAL | AV_PIX_FMT_FLAG_BAYER)) {
> +        /* RGB-like family */
> +        fmt.csp   = AVCOL_SPC_RGB;
> +        fmt.range = AVCOL_RANGE_JPEG;
> +    } else if (desc->flags & AV_PIX_FMT_FLAG_XYZ) {
> +        fmt.csp   = AVCOL_SPC_UNSPECIFIED;
> +        fmt.prim  = AVCOL_PRI_SMPTE428;
> +        fmt.trc   = AVCOL_TRC_SMPTE428;
> +    } else if (desc->nb_components < 3) {
> +        /* Grayscale formats */
> +        fmt.prim  = AVCOL_PRI_UNSPECIFIED;
> +        fmt.csp   = AVCOL_SPC_UNSPECIFIED;
> +        if (desc->flags & AV_PIX_FMT_FLAG_FLOAT)
> +            fmt.range = AVCOL_RANGE_UNSPECIFIED;
> +        else
> +            fmt.range = AVCOL_RANGE_JPEG; // FIXME: this restriction should be lifted

I should point out that this recreates the swscale status quo behavior.

> +    }
> +
> +    switch (frame->format) {
> +    case AV_PIX_FMT_YUVJ420P:
> +    case AV_PIX_FMT_YUVJ411P:
> +    case AV_PIX_FMT_YUVJ422P:
> +    case AV_PIX_FMT_YUVJ444P:
> +    case AV_PIX_FMT_YUVJ440P:
> +        fmt.range = AVCOL_RANGE_JPEG;
> +        break;
> +    }
> +
> +    if (!desc->log2_chroma_w && !desc->log2_chroma_h)
> +        fmt.loc = AVCHROMA_LOC_UNSPECIFIED;
> +
> +    if (frame->flags & AV_FRAME_FLAG_INTERLACED) {
> +        av_assert1(!(fmt.height & 1));

I'm not sure about this assertion; I think it may need to go. The easiest way
to handle inherlaced odd sized frames would be to just discard the last line,
though with a little extra effort we could easily support them as two
differently sized fields.

Does anybody know how odd sized interlaced frames are handled in elsewhere?

> +        fmt.height >>= 1;
> +        fmt.interlaced = 1;
> +    }
> +
> +    return fmt;
> +}
> diff --git a/libswscale/utils.h b/libswscale/utils.h
> new file mode 100644
> index 0000000000..3d0b08ffe1
> --- /dev/null
> +++ b/libswscale/utils.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2024 Niklas Haas
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef SWSCALE_UTILS_H
> +#define SWSCALE_UTILS_H
> +
> +#include "libavutil/pixdesc.h"
> +
> +#include "swscale.h"
> +
> +/* Subset of AVFrame parameters that uniquely determine pixel representation */
> +typedef struct SwsFormat {
> +    int width, height;
> +    int interlaced;
> +    enum AVPixelFormat format;
> +    enum AVColorRange range;
> +    enum AVColorPrimaries prim;
> +    enum AVColorTransferCharacteristic trc;
> +    enum AVColorSpace csp;
> +    enum AVChromaLocation loc;
> +    const AVPixFmtDescriptor *desc; /* convenience */
> +} SwsFormat;
> +
> +/**
> + * This function also sanitizes and strips the input data, removing irrelevant
> + * fields for certain formats.
> + */
> +SwsFormat ff_fmt_from_frame(const AVFrame *frame);
> +
> +static inline int ff_fmt_equal(const SwsFormat *fmt1, const SwsFormat *fmt2)
> +{
> +    return fmt1->width      == fmt2->width      &&
> +           fmt1->height     == fmt2->height     &&
> +           fmt1->interlaced == fmt2->interlaced &&
> +           fmt1->format     == fmt2->format     &&
> +           fmt1->range      == fmt2->range      &&
> +           fmt1->prim       == fmt2->prim       &&
> +           fmt1->trc        == fmt2->trc        &&
> +           fmt1->csp        == fmt2->csp        &&
> +           fmt1->loc        == fmt2->loc;
> +}
> +
> +static inline int ff_fmt_align(enum AVPixelFormat fmt)
> +{
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
> +    if (desc->flags & AV_PIX_FMT_FLAG_BAYER) {
> +        return 2;
> +    } else {
> +        return 1 << desc->log2_chroma_h;
> +    }
> +}
> +
> +#endif /* SWSCALE_UTILS_H */
> --
> 2.46.1
>