[FFmpeg-devel] [PATCH v2] avfilter: fix YUV colorspace negotiation for YUVJ

[Niklas Haas]

On Sun, 08 Sep 2024 15:04:29 +0200 Niklas Haas <ffmpeg at haasn.xyz> wrote:
> From: Niklas Haas <git at haasn.dev>
>
> Ironically, despite being introduced to make YUVJ unnecessary, the new
> YUV negotiation logic failed to actually negotiate YUVJ formats
> themselves correctly, leading to errors when passing YUVJ frames into
> a filter graph. (They were effectively treated like RGB or Grayscale
> formats, rather than as forced-full-range YUV, and hence did not have
> their colorspace matrix correctly negotiated)
>
> Fix this by splitting off the YUVJ check from ff_fmt_is_regular_yuv().
> Obviously, we can trivially undo this change again once YUVJ is actually
> deleted from the codebase.

Need to fix FATE regression before merging this.

> ---
>  libavfilter/avfiltergraph.c | 31 +++++++++++++++++++------------
>  libavfilter/buffersrc.c     | 15 ++++++++++-----
>  libavfilter/video.h         |  5 +++++
>  3 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index c84dd3cf25..806e2a93f4 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -654,19 +654,22 @@ int ff_fmt_is_regular_yuv(enum AVPixelFormat fmt)
>      if (desc->nb_components < 3)
>          return 0; /* Grayscale is explicitly full-range in swscale */
>      av_assert1(!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL));
> -    if (desc->flags & (AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PAL |
> -                       AV_PIX_FMT_FLAG_XYZ | AV_PIX_FMT_FLAG_FLOAT))
> -        return 0;
> +    return !(desc->flags & (AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PAL |
> +                            AV_PIX_FMT_FLAG_XYZ | AV_PIX_FMT_FLAG_FLOAT));
> +}
>
> +
> +int ff_fmt_is_forced_full_range(enum AVPixelFormat fmt)
> +{
>      switch (fmt) {
>      case AV_PIX_FMT_YUVJ420P:
>      case AV_PIX_FMT_YUVJ422P:
>      case AV_PIX_FMT_YUVJ444P:
>      case AV_PIX_FMT_YUVJ440P:
>      case AV_PIX_FMT_YUVJ411P:
> -        return 0;
> -    default:
>          return 1;
> +    default:
> +        return 0;
>      }
>  }
>
> @@ -742,14 +745,18 @@ static int pick_format(AVFilterLink *link, AVFilterLink *ref)
>              link->incfg.color_spaces->nb_formats = 1;
>              link->colorspace = link->incfg.color_spaces->formats[0];
>
> -            if (!link->incfg.color_ranges->nb_formats) {
> -                av_log(link->src, AV_LOG_ERROR, "Cannot select color range for"
> -                       " the link between filters %s and %s.\n", link->src->name,
> -                       link->dst->name);
> -                return AVERROR(EINVAL);
> +            if (ff_fmt_is_forced_full_range(swfmt)) {
> +                link->color_range = AVCOL_RANGE_JPEG;
> +            } else {
> +                if (!link->incfg.color_ranges->nb_formats) {
> +                    av_log(link->src, AV_LOG_ERROR, "Cannot select color range for"
> +                           " the link between filters %s and %s.\n", link->src->name,
> +                           link->dst->name);
> +                    return AVERROR(EINVAL);
> +                }
> +                link->incfg.color_ranges->nb_formats = 1;
> +                link->color_range = link->incfg.color_ranges->formats[0];
>              }
> -            link->incfg.color_ranges->nb_formats = 1;
> -            link->color_range = link->incfg.color_ranges->formats[0];
>          }
>      } else if (link->type == AVMEDIA_TYPE_AUDIO) {
>          int ret;
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index a3113f5c13..b5682006f0 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -461,12 +461,17 @@ static int query_formats(AVFilterContext *ctx)
>              if ((ret = ff_add_format(&color_spaces, c->color_space)) < 0 ||
>                  (ret = ff_set_common_color_spaces(ctx, color_spaces)) < 0)
>                  return ret;
> -            if ((ret = ff_add_format(&color_ranges, c->color_range)) < 0)
> -                return ret;
> -            if (c->color_range == AVCOL_RANGE_UNSPECIFIED) {
> -                /* allow implicitly promoting unspecified to mpeg */
> -                if ((ret = ff_add_format(&color_ranges, AVCOL_RANGE_MPEG)) < 0)
> +            if (ff_fmt_is_forced_full_range(swfmt)) {
> +                if ((ret = ff_add_format(&color_ranges, AVCOL_RANGE_JPEG)) < 0)
> +                    return ret;
> +            } else {
> +                if ((ret = ff_add_format(&color_ranges, c->color_range)) < 0)
>                      return ret;
> +                if (c->color_range == AVCOL_RANGE_UNSPECIFIED) {
> +                    /* allow implicitly promoting unspecified to mpeg */
> +                    if ((ret = ff_add_format(&color_ranges, AVCOL_RANGE_MPEG)) < 0)
> +                        return ret;
> +                }
>              }
>              if ((ret = ff_set_common_color_ranges(ctx, color_ranges)) < 0)
>                  return ret;
> diff --git a/libavfilter/video.h b/libavfilter/video.h
> index 77f27fdf7c..f44d3445dc 100644
> --- a/libavfilter/video.h
> +++ b/libavfilter/video.h
> @@ -51,4 +51,9 @@ AVFrame *ff_get_video_buffer(AVFilterLink *link, int w, int h);
>   */
>  int ff_fmt_is_regular_yuv(enum AVPixelFormat fmt);
>
> +/**
> + * Returns true if a YUV pixel format is forced full range (i.e. YUVJ).
> + */
> +int ff_fmt_is_forced_full_range(enum AVPixelFormat fmt);
> +
>  #endif /* AVFILTER_VIDEO_H */
> --
> 2.46.0
>