| Message ID | 20240910131816.1482534-2-ffmpeg@haasn.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3,1/2] tests/fate: force MPEG range for rawvideo tests | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, 10 Sep 2024 15:18:16 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote: > From: Niklas Haas <git@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. Forgot to mention that this fixes #11179. > --- > 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 3946f04b53..38077ff701 100644 > --- a/libavfilter/avfiltergraph.c > +++ b/libavfilter/avfiltergraph.c > @@ -656,19 +656,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; > } > } > > @@ -744,14 +747,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 >
diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c index 3946f04b53..38077ff701 100644 --- a/libavfilter/avfiltergraph.c +++ b/libavfilter/avfiltergraph.c @@ -656,19 +656,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; } } @@ -744,14 +747,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 */
From: Niklas Haas <git@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. --- libavfilter/avfiltergraph.c | 31 +++++++++++++++++++------------ libavfilter/buffersrc.c | 15 ++++++++++----- libavfilter/video.h | 5 +++++ 3 files changed, 34 insertions(+), 17 deletions(-)