PR #24880 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24880 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24880.patch
Threaded AArch64 NV12-to-BGRA conversion can divide an even-height frame into odd-height slices. The NEON converter requires even slice boundaries: older implementations advance chroma incorrectly, and current two-row implementations additionally write past odd slice ends. When selecting the NEON converter, raise `dst_slice_align` to at least `1 << chrSrcVSubSample`. This preserves larger existing alignment and also covers planar 4:2:0 paths when dithering options bypass the generic selector. Update the assembly precondition comment accordingly. The fix preserves Kevin Watters's authorship of the correction proposed in #24266. Add `fate-sws-unscaled-threaded`, comparing single-threaded output against 2, 3, 7 and 16 threads across five heights and 75 format pairs. Exercise both dynamic and initialized legacy contexts, and print mismatch counts for comparison against `tests/ref/fate/sws-unscaled-threaded`. Validation: - The issue's reproducer fails on AArch64 master 7366b2f552c92 and passes with the fix; both thread counts then produce MD5 `3281306bf84b90bd3d4d2a18cb676e0d`. - The new FATE test fails without the fix and passes with it on native AArch64. - The new FATE test passes on native x86-64. - The configured `make fate-libswscale` target passes on AArch64. - The issue reproduces on release/5.1, 6.1, 7.1, 8.0, 8.1 and 9.0. An equivalent local correction also fixes the original reproducer on release/5.1. The library regression was verified at d6fdc78e9164 against its parent. The CLI reproducer starts failing at 60515a6d61049, which forwards the filter thread count to swscale. Fixes #24266. >From 4e3daa2f85d072eeaf6c1deb9faa58d5347b0440 Mon Sep 17 00:00:00 2001 From: Kevin Watters <[email protected]> Date: Sun, 4 Oct 2026 15:41:59 +0000 Subject: [PATCH 1/2] swscale/aarch64: align unscaled YUV to RGB slices to source chroma The NV12 and NV21 NEON converters need even slice boundaries, but the RGB destination only requests one-row alignment. Threaded conversion can therefore start on an odd luma row and use the wrong chroma phase. The current two-row converters additionally write past odd slice ends. Raise dst_slice_align when selecting a NEON converter, preserving any larger existing alignment. This also covers planar 4:2:0 when dithering options bypass the generic yuv2rgb selector's alignment assignment. Update the assembly precondition comment to include graph dispatch. Fixes #24266. Fixes: d6fdc78e9164 ("sws: implement slice threading") Assisted-by: Fairy --- libswscale/aarch64/swscale_unscaled.c | 4 +++- libswscale/aarch64/yuv2rgb_neon.S | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libswscale/aarch64/swscale_unscaled.c b/libswscale/aarch64/swscale_unscaled.c index c9041db99f..d11625dbbb 100644 --- a/libswscale/aarch64/swscale_unscaled.c +++ b/libswscale/aarch64/swscale_unscaled.c @@ -146,8 +146,10 @@ static int nv24_to_yuv420p_neon_wrapper(SwsInternal *c, const uint8_t *const src && c->opts.dst_format == AV_PIX_FMT_##OFMT \ && !(c->opts.src_h & 1) \ && !(c->opts.src_w & 15) \ - && !accurate_rnd) \ + && !accurate_rnd) { \ c->convert_unscaled = ifmt##_to_##ofmt##_neon_wrapper; \ + c->dst_slice_align = FFMAX(c->dst_slice_align, 1 << c->chrSrcVSubSample); \ + } \ } while (0) #define SET_FF_YUVX_TO_ALL_RGBX_FUNC(yuvx, YUVX, accurate_rnd) do { \ diff --git a/libswscale/aarch64/yuv2rgb_neon.S b/libswscale/aarch64/yuv2rgb_neon.S index 734b6dd174..de5c5ce70a 100644 --- a/libswscale/aarch64/yuv2rgb_neon.S +++ b/libswscale/aarch64/yuv2rgb_neon.S @@ -941,10 +941,9 @@ endfunc // share one chroma row, so the chroma -> RGB offsets (v20-v25) are // computed once and applied to both luma rows. // -// Precondition: slice height is even. SET_FF_YUVX_TO_RGBX_FUNC gates -// on !(src_h & 1); scale_internal()'s macro_height_src check in -// libswscale/swscale.c rejects any odd srcSliceH for vertically- -// subsampled sources (chrSrcVSubSample > 0). +// Precondition: slice height is even. SET_FF_YUVX_TO_RGBX_FUNC checks +// the frame height and sets dst_slice_align for threaded conversion. +// scale_internal() also rejects odd input slices for subsampled sources. .macro declare_2l_packed ifmt ofmt function ff_\ifmt\()_to_\ofmt\()_neon, export=1 uxtw widthx, width -- 2.52.0 >From 25447598a2ab0e6ab71c57621a2c9c9cae4c3140 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Sun, 4 Oct 2026 15:41:59 +0000 Subject: [PATCH 2/2] tests/swscale: compare unscaled conversion across thread counts Compare one-thread conversion with 2, 3, 7 and 16 threads for five even frame heights, including cases that otherwise produce odd slices. Cover NV12, NV21, YUV420P, YUVA420P and YUV422P to packed and planar RGB, including both byte orders of the 16-bit formats. Exercise both dynamic and initialized legacy swscale contexts. Disable dithering so the AArch64 selector must supply its own slice alignment for planar inputs as well. Compare within each implementation to avoid requiring identical color conversion rounding across architectures. Print mismatching row counts for FATE to compare with its reference. The test fails before the preceding fix on native AArch64 and passes afterwards. It also passes on native x86-64. Assisted-by: Fairy --- libswscale/Makefile | 1 + libswscale/tests/.gitignore | 1 + libswscale/tests/unscaled_threaded.c | 148 +++++++++++++++++++++++++++ tests/fate/libswscale.mak | 4 + tests/ref/fate/sws-unscaled-threaded | 75 ++++++++++++++ 5 files changed, 229 insertions(+) create mode 100644 libswscale/tests/unscaled_threaded.c create mode 100644 tests/ref/fate/sws-unscaled-threaded diff --git a/libswscale/Makefile b/libswscale/Makefile index f6dbed3c08..4c12c888c6 100644 --- a/libswscale/Makefile +++ b/libswscale/Makefile @@ -53,6 +53,7 @@ TESTPROGS = colorspace \ floatimg_cmp \ pixdesc_query \ swscale \ + unscaled_threaded \ TESTPROGS-$(CONFIG_UNSTABLE) = sws_ops \ sws_ops_aarch64 \ diff --git a/libswscale/tests/.gitignore b/libswscale/tests/.gitignore index f27fc891f5..1fd518eae1 100644 --- a/libswscale/tests/.gitignore +++ b/libswscale/tests/.gitignore @@ -4,3 +4,4 @@ /swscale /sws_ops /sws_ops_aarch64 +/unscaled_threaded diff --git a/libswscale/tests/unscaled_threaded.c b/libswscale/tests/unscaled_threaded.c new file mode 100644 index 0000000000..78f1350b6e --- /dev/null +++ b/libswscale/tests/unscaled_threaded.c @@ -0,0 +1,148 @@ +/* + * 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 + */ + +#include <stdio.h> +#include <string.h> + +#include "libavutil/frame.h" +#include "libavutil/imgutils.h" +#include "libavutil/log.h" +#include "libavutil/opt.h" +#include "libavutil/pixdesc.h" +#include "libswscale/swscale.h" + +static AVFrame *alloc_frame(enum AVPixelFormat format, int height) +{ + AVFrame *frame = av_frame_alloc(); + + if (!frame) + return NULL; + frame->format = format; + frame->width = 64; + frame->height = height; + if (av_frame_get_buffer(frame, 32) < 0) + av_frame_free(&frame); + return frame; +} + +static int convert(AVFrame *dst, const AVFrame *src, int threads, int legacy) +{ + struct SwsContext *sws = sws_alloc_context(); + int ret; + + if (!sws) + return AVERROR(ENOMEM); + av_opt_set_int(sws, "threads", threads, 0); + av_opt_set_int(sws, "sws_flags", SWS_BILINEAR, 0); + av_opt_set(sws, "sws_dither", "none", 0); + if (legacy) { + av_opt_set_int(sws, "srcw", src->width, 0); + av_opt_set_int(sws, "srch", src->height, 0); + av_opt_set_int(sws, "src_format", src->format, 0); + av_opt_set_int(sws, "dstw", dst->width, 0); + av_opt_set_int(sws, "dsth", dst->height, 0); + av_opt_set_int(sws, "dst_format", dst->format, 0); + ret = sws_init_context(sws, NULL, NULL); + if (ret < 0) + goto end; + } + ret = sws_scale_frame(sws, dst, src); +end: + sws_freeContext(sws); + return ret; +} + +static int compare(enum AVPixelFormat src_fmt, enum AVPixelFormat dst_fmt, + int height, int legacy, unsigned *mismatches) +{ + static const int threads[] = { 2, 3, 7, 16 }; + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(src_fmt); + AVFrame *src = alloc_frame(src_fmt, height); + AVFrame *ref = alloc_frame(dst_fmt, height); + AVFrame *dst = alloc_frame(dst_fmt, height); + int ret = AVERROR(ENOMEM); + + if (!src || !ref || !dst) + goto end; + for (int p = 0; p < 4 && src->data[p]; p++) { + const int h = AV_CEIL_RSHIFT(height, p == 1 || p == 2 ? desc->log2_chroma_h : 0); + const int w = av_image_get_linesize(src_fmt, src->width, p); + for (int y = 0; y < h; y++) + for (int x = 0; x < w; x++) + src->data[p][y * src->linesize[p] + x] = + (x * 37 + y * 73 + x * y * 11 + p * 59) & 255; + } + ret = convert(ref, src, 1, legacy); + if (ret < 0) + goto end; + for (int t = 0; t < FF_ARRAY_ELEMS(threads); t++) { + ret = convert(dst, src, threads[t], legacy); + if (ret < 0) + goto end; + for (int p = 0; p < 4 && dst->data[p]; p++) { + const int w = av_image_get_linesize(dst_fmt, dst->width, p); + for (int y = 0; y < height; y++) + *mismatches += !!memcmp(ref->data[p] + y * ref->linesize[p], + dst->data[p] + y * dst->linesize[p], w); + } + } +end: + av_frame_free(&src); + av_frame_free(&ref); + av_frame_free(&dst); + return ret; +} + +int main(void) +{ + static const enum AVPixelFormat inputs[] = { + AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV420P, + AV_PIX_FMT_YUVA420P, AV_PIX_FMT_YUV422P, + }; + static const enum AVPixelFormat outputs[] = { + AV_PIX_FMT_ARGB, AV_PIX_FMT_RGBA, AV_PIX_FMT_ABGR, AV_PIX_FMT_BGRA, + AV_PIX_FMT_RGB24, AV_PIX_FMT_BGR24, AV_PIX_FMT_GBRP, + AV_PIX_FMT_RGB565LE, AV_PIX_FMT_BGR565LE, + AV_PIX_FMT_RGB555LE, AV_PIX_FMT_BGR555LE, + AV_PIX_FMT_RGB565BE, AV_PIX_FMT_BGR565BE, + AV_PIX_FMT_RGB555BE, AV_PIX_FMT_BGR555BE, + }; + static const int heights[] = { 2, 6, 14, 48, 50 }; + + av_log_set_level(AV_LOG_ERROR); + + /* Compare against the same implementation with one thread, since different + * architectures need not produce identical rounding. Include both the + * dynamic graph and the initialized legacy context APIs. */ + for (int s = 0; s < FF_ARRAY_ELEMS(inputs); s++) { + for (int d = 0; d < FF_ARRAY_ELEMS(outputs); d++) { + unsigned mismatches = 0; + for (int legacy = 0; legacy <= 1; legacy++) { + for (int h = 0; h < FF_ARRAY_ELEMS(heights); h++) { + int ret = compare(inputs[s], outputs[d], heights[h], legacy, &mismatches); + if (ret < 0) + return 1; + } + } + printf("%s -> %s: %u mismatching rows\n", + av_get_pix_fmt_name(inputs[s]), av_get_pix_fmt_name(outputs[d]), + mismatches); + } + } + return 0; +} diff --git a/tests/fate/libswscale.mak b/tests/fate/libswscale.mak index 673c28eae9..1ad8e96f8d 100644 --- a/tests/fate/libswscale.mak +++ b/tests/fate/libswscale.mak @@ -1,3 +1,7 @@ +FATE_LIBSWSCALE += fate-sws-unscaled-threaded +fate-sws-unscaled-threaded: libswscale/tests/unscaled_threaded$(EXESUF) +fate-sws-unscaled-threaded: CMD = run libswscale/tests/unscaled_threaded$(EXESUF) + FATE_LIBSWSCALE += fate-sws-pixdesc-query fate-sws-pixdesc-query: libswscale/tests/pixdesc_query$(EXESUF) fate-sws-pixdesc-query: CMD = run libswscale/tests/pixdesc_query$(EXESUF) diff --git a/tests/ref/fate/sws-unscaled-threaded b/tests/ref/fate/sws-unscaled-threaded new file mode 100644 index 0000000000..e412147a99 --- /dev/null +++ b/tests/ref/fate/sws-unscaled-threaded @@ -0,0 +1,75 @@ +nv12 -> argb: 0 mismatching rows +nv12 -> rgba: 0 mismatching rows +nv12 -> abgr: 0 mismatching rows +nv12 -> bgra: 0 mismatching rows +nv12 -> rgb24: 0 mismatching rows +nv12 -> bgr24: 0 mismatching rows +nv12 -> gbrp: 0 mismatching rows +nv12 -> rgb565le: 0 mismatching rows +nv12 -> bgr565le: 0 mismatching rows +nv12 -> rgb555le: 0 mismatching rows +nv12 -> bgr555le: 0 mismatching rows +nv12 -> rgb565be: 0 mismatching rows +nv12 -> bgr565be: 0 mismatching rows +nv12 -> rgb555be: 0 mismatching rows +nv12 -> bgr555be: 0 mismatching rows +nv21 -> argb: 0 mismatching rows +nv21 -> rgba: 0 mismatching rows +nv21 -> abgr: 0 mismatching rows +nv21 -> bgra: 0 mismatching rows +nv21 -> rgb24: 0 mismatching rows +nv21 -> bgr24: 0 mismatching rows +nv21 -> gbrp: 0 mismatching rows +nv21 -> rgb565le: 0 mismatching rows +nv21 -> bgr565le: 0 mismatching rows +nv21 -> rgb555le: 0 mismatching rows +nv21 -> bgr555le: 0 mismatching rows +nv21 -> rgb565be: 0 mismatching rows +nv21 -> bgr565be: 0 mismatching rows +nv21 -> rgb555be: 0 mismatching rows +nv21 -> bgr555be: 0 mismatching rows +yuv420p -> argb: 0 mismatching rows +yuv420p -> rgba: 0 mismatching rows +yuv420p -> abgr: 0 mismatching rows +yuv420p -> bgra: 0 mismatching rows +yuv420p -> rgb24: 0 mismatching rows +yuv420p -> bgr24: 0 mismatching rows +yuv420p -> gbrp: 0 mismatching rows +yuv420p -> rgb565le: 0 mismatching rows +yuv420p -> bgr565le: 0 mismatching rows +yuv420p -> rgb555le: 0 mismatching rows +yuv420p -> bgr555le: 0 mismatching rows +yuv420p -> rgb565be: 0 mismatching rows +yuv420p -> bgr565be: 0 mismatching rows +yuv420p -> rgb555be: 0 mismatching rows +yuv420p -> bgr555be: 0 mismatching rows +yuva420p -> argb: 0 mismatching rows +yuva420p -> rgba: 0 mismatching rows +yuva420p -> abgr: 0 mismatching rows +yuva420p -> bgra: 0 mismatching rows +yuva420p -> rgb24: 0 mismatching rows +yuva420p -> bgr24: 0 mismatching rows +yuva420p -> gbrp: 0 mismatching rows +yuva420p -> rgb565le: 0 mismatching rows +yuva420p -> bgr565le: 0 mismatching rows +yuva420p -> rgb555le: 0 mismatching rows +yuva420p -> bgr555le: 0 mismatching rows +yuva420p -> rgb565be: 0 mismatching rows +yuva420p -> bgr565be: 0 mismatching rows +yuva420p -> rgb555be: 0 mismatching rows +yuva420p -> bgr555be: 0 mismatching rows +yuv422p -> argb: 0 mismatching rows +yuv422p -> rgba: 0 mismatching rows +yuv422p -> abgr: 0 mismatching rows +yuv422p -> bgra: 0 mismatching rows +yuv422p -> rgb24: 0 mismatching rows +yuv422p -> bgr24: 0 mismatching rows +yuv422p -> gbrp: 0 mismatching rows +yuv422p -> rgb565le: 0 mismatching rows +yuv422p -> bgr565le: 0 mismatching rows +yuv422p -> rgb555le: 0 mismatching rows +yuv422p -> bgr555le: 0 mismatching rows +yuv422p -> rgb565be: 0 mismatching rows +yuv422p -> bgr565be: 0 mismatching rows +yuv422p -> rgb555be: 0 mismatching rows +yuv422p -> bgr555be: 0 mismatching rows -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
