PR #22793 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22793 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22793.patch
This change ensures that each backend is tested separately against only implementation variants provided by that backend. Importantly, it ensures that the existence of higher-priority backends (e.g. `memcpy`) does not prevent testing lower-priority backends. >From 92fc6c69066a1ae7ef19d81f45406aae8b10161a Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Tue, 7 Apr 2026 17:14:10 +0200 Subject: [PATCH 1/5] tests/checkasm/sw_ops: remove some unnecessary checks These don't actually exist at runtime, and will soon be removed from the backends as well. This commit is intentionally a bit incomplete; as I will rewrite this based on the auto-generated macros in the upcoming ops_micro series. Signed-off-by: Niklas Haas <[email protected]> --- tests/checkasm/sw_ops.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/checkasm/sw_ops.c b/tests/checkasm/sw_ops.c index 39a053172c..47660b7610 100644 --- a/tests/checkasm/sw_ops.c +++ b/tests/checkasm/sw_ops.c @@ -714,27 +714,17 @@ static void check_linear(void) const char *name; uint32_t mask; } patterns[] = { - { "noop", 0 }, { "luma", SWS_MASK_LUMA }, { "alpha", SWS_MASK_ALPHA }, { "luma+alpha", SWS_MASK_LUMA | SWS_MASK_ALPHA }, { "dot3", 0x7 }, - { "dot4", 0xF }, - { "row0", SWS_MASK_ROW(0) }, - { "row0+alpha", SWS_MASK_ROW(0) | SWS_MASK_ALPHA }, - { "off3", SWS_MASK_OFF3 }, - { "off3+alpha", SWS_MASK_OFF3 | SWS_MASK_ALPHA }, + { "row0", SWS_MASK_ROW(0) ^ SWS_MASK(0, 3) }, { "diag3", SWS_MASK_DIAG3 }, { "diag4", SWS_MASK_DIAG4 }, { "diag3+alpha", SWS_MASK_DIAG3 | SWS_MASK_ALPHA }, { "diag3+off3", SWS_MASK_DIAG3 | SWS_MASK_OFF3 }, - { "diag3+off3+alpha", SWS_MASK_DIAG3 | SWS_MASK_OFF3 | SWS_MASK_ALPHA }, - { "diag4+off4", SWS_MASK_DIAG4 | SWS_MASK_OFF4 }, - { "matrix3", SWS_MASK_MAT3 }, { "matrix3+off3", SWS_MASK_MAT3 | SWS_MASK_OFF3 }, { "matrix3+off3+alpha", SWS_MASK_MAT3 | SWS_MASK_OFF3 | SWS_MASK_ALPHA }, - { "matrix4", SWS_MASK_MAT4 }, - { "matrix4+off4", SWS_MASK_MAT4 | SWS_MASK_OFF4 }, }; for (SwsPixelType t = F32; t < SWS_PIXEL_TYPE_NB; t++) { -- 2.52.0 >From 153d3100b1d7476150d3b363ad382ad3d7f7d8fb Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sat, 11 Apr 2026 18:15:57 +0200 Subject: [PATCH 2/5] tests/checkasm/sw_ops: set correct plane index order All four components were accidentally being read/written to/from the same plane. Signed-off-by: Niklas Haas <[email protected]> --- tests/checkasm/sw_ops.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/checkasm/sw_ops.c b/tests/checkasm/sw_ops.c index 47660b7610..1205721c3e 100644 --- a/tests/checkasm/sw_ops.c +++ b/tests/checkasm/sw_ops.c @@ -117,12 +117,17 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], SwsContext *ctx = sws_alloc_context(); SwsCompiledOp comp_ref = {0}, comp_new = {0}; const SwsOpBackend *backend_new = NULL; - SwsOpList oplist = { .ops = (SwsOp *) ops }; const SwsOp *read_op, *write_op; static const unsigned def_ranges[4] = {0}; if (!ranges) ranges = def_ranges; + SwsOpList oplist = { + .ops = (SwsOp *) ops, + .plane_src = {0, 1, 2, 3}, + .plane_dst = {0, 1, 2, 3}, + }; + declare_func(void, const SwsOpExec *, const void *, int bx, int y, int bx_end, int y_end); static DECLARE_ALIGNED_64(char, src0)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; -- 2.52.0 >From dbb54a6b96158653c19001f8edac555a84d7ba32 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sat, 11 Apr 2026 17:44:29 +0200 Subject: [PATCH 3/5] tests/checkasm/sw_ops: check all planes in CHECK_COMMON() This can help e.g. properly test that the masked/excluded components are left unmodified. Signed-off-by: Niklas Haas <[email protected]> --- tests/checkasm/sw_ops.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/checkasm/sw_ops.c b/tests/checkasm/sw_ops.c index 1205721c3e..0223bf897d 100644 --- a/tests/checkasm/sw_ops.c +++ b/tests/checkasm/sw_ops.c @@ -318,10 +318,10 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], CHECK_RANGES(NAME, MK_RANGES(RANGE), N_IN, N_OUT, IN, OUT, __VA_ARGS__) #define CHECK_COMMON_RANGE(NAME, RANGE, IN, OUT, ...) \ - CHECK_RANGE(FMT("%s_p1000", NAME), RANGE, 1, 1, IN, OUT, __VA_ARGS__); \ - CHECK_RANGE(FMT("%s_p1110", NAME), RANGE, 3, 3, IN, OUT, __VA_ARGS__); \ + CHECK_RANGE(FMT("%s_p1000", NAME), RANGE, 4, 4, IN, OUT, __VA_ARGS__); \ + CHECK_RANGE(FMT("%s_p1110", NAME), RANGE, 4, 4, IN, OUT, __VA_ARGS__); \ CHECK_RANGE(FMT("%s_p1111", NAME), RANGE, 4, 4, IN, OUT, __VA_ARGS__); \ - CHECK_RANGE(FMT("%s_p1001", NAME), RANGE, 4, 2, IN, OUT, __VA_ARGS__, { \ + CHECK_RANGE(FMT("%s_p1001", NAME), RANGE, 4, 4, IN, OUT, __VA_ARGS__, { \ .op = SWS_OP_SWIZZLE, \ .type = OUT, \ .swizzle = SWS_SWIZZLE(0, 3, 1, 2), \ -- 2.52.0 >From fd9c1075eabd65e9e8de79edb48a5726423ac6c6 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sun, 12 Apr 2026 02:00:21 +0200 Subject: [PATCH 4/5] tests/checkasm/sw_ops: separate op compilation from testing This commit is purely moving around code; there is no functional change. Signed-off-by: Niklas Haas <[email protected]> --- tests/checkasm/sw_ops.c | 160 +++++++++++++++++++++++----------------- 1 file changed, 91 insertions(+), 69 deletions(-) diff --git a/tests/checkasm/sw_ops.c b/tests/checkasm/sw_ops.c index 0223bf897d..b010dbdf13 100644 --- a/tests/checkasm/sw_ops.c +++ b/tests/checkasm/sw_ops.c @@ -111,23 +111,12 @@ static void set_range(AVRational *rangeq, unsigned range, unsigned range_def) *rangeq = (AVRational) { range, 1 }; } -static void check_ops(const char *report, const unsigned ranges[NB_PLANES], - const SwsOp *ops) +static void check_compiled(const char *report, const SwsOpBackend *backend_new, + const SwsOp *read_op, const SwsOp *write_op, + const int ranges[NB_PLANES], + const SwsCompiledOp *comp_ref, + const SwsCompiledOp *comp_new) { - SwsContext *ctx = sws_alloc_context(); - SwsCompiledOp comp_ref = {0}, comp_new = {0}; - const SwsOpBackend *backend_new = NULL; - const SwsOp *read_op, *write_op; - static const unsigned def_ranges[4] = {0}; - if (!ranges) - ranges = def_ranges; - - SwsOpList oplist = { - .ops = (SwsOp *) ops, - .plane_src = {0, 1, 2, 3}, - .plane_dst = {0, 1, 2, 3}, - }; - declare_func(void, const SwsOpExec *, const void *, int bx, int y, int bx_end, int y_end); static DECLARE_ALIGNED_64(char, src0)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; @@ -135,41 +124,20 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], static DECLARE_ALIGNED_64(char, dst0)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; static DECLARE_ALIGNED_64(char, dst1)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; - if (!ctx) - return; - ctx->flags = SWS_BITEXACT; - - read_op = &ops[0]; - for (oplist.num_ops = 0; ops[oplist.num_ops].op; oplist.num_ops++) - write_op = &ops[oplist.num_ops]; - - const int read_size = PIXELS * rw_pixel_bits(read_op) >> 3; - const int write_size = PIXELS * rw_pixel_bits(write_op) >> 3; - for (int p = 0; p < NB_PLANES; p++) { void *plane = src0[p]; switch (read_op->type) { case U8: fill8(plane, sizeof(src0[p]) / sizeof(uint8_t), ranges[p]); - set_range(&oplist.comps_src.max[p], ranges[p], UINT8_MAX); - oplist.comps_src.min[p] = (AVRational) { 0, 1 }; break; case U16: fill16(plane, sizeof(src0[p]) / sizeof(uint16_t), ranges[p]); - set_range(&oplist.comps_src.max[p], ranges[p], UINT16_MAX); - oplist.comps_src.min[p] = (AVRational) { 0, 1 }; break; case U32: fill32(plane, sizeof(src0[p]) / sizeof(uint32_t), ranges[p]); - set_range(&oplist.comps_src.max[p], ranges[p], UINT32_MAX); - oplist.comps_src.min[p] = (AVRational) { 0, 1 }; break; case F32: fill32f(plane, sizeof(src0[p]) / sizeof(uint32_t), ranges[p]); - if (ranges[p] && ranges[p] <= INT_MAX) { - oplist.comps_src.max[p] = (AVRational) { ranges[p], 1 }; - oplist.comps_src.min[p] = (AVRational) { 0, 1 }; - } break; } } @@ -178,30 +146,8 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], memset(dst0, 0, sizeof(dst0)); memset(dst1, 0, sizeof(dst1)); - /* Compile `ops` using both the asm and c backends */ - for (int n = 0; ff_sws_op_backends[n]; n++) { - const SwsOpBackend *backend = ff_sws_op_backends[n]; - const bool is_ref = !strcmp(backend->name, "c"); - if (is_ref || !comp_new.func) { - SwsCompiledOp comp; - int ret = ff_sws_ops_compile_backend(ctx, backend, &oplist, &comp); - if (ret == AVERROR(ENOTSUP)) - continue; - else if (ret < 0) - fail(); - else if (PIXELS % comp.block_size != 0) - fail(); - - if (is_ref) - comp_ref = comp; - if (!comp_new.func) { - comp_new = comp; - backend_new = backend; - } - } - } - - av_assert0(comp_ref.func && comp_new.func); + const int read_size = PIXELS * rw_pixel_bits(read_op) >> 3; + const int write_size = PIXELS * rw_pixel_bits(write_op) >> 3; SwsOpExec exec = {0}; exec.width = PIXELS; @@ -237,24 +183,24 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], * the backend pointer and the active CPU flags. */ uintptr_t id = (uintptr_t) backend_new; - id ^= (id << 6) + (id >> 2) + 0x9e3779b97f4a7c15 + comp_new.cpu_flags; + id ^= (id << 6) + (id >> 2) + 0x9e3779b97f4a7c15 + comp_new->cpu_flags; if (check_key((void*) id, "%s", report)) { - exec.block_size_in = comp_ref.block_size * rw_pixel_bits(read_op) >> 3; - exec.block_size_out = comp_ref.block_size * rw_pixel_bits(write_op) >> 3; + exec.block_size_in = comp_ref->block_size * rw_pixel_bits(read_op) >> 3; + exec.block_size_out = comp_ref->block_size * rw_pixel_bits(write_op) >> 3; for (int i = 0; i < NB_PLANES; i++) { exec.in[i] = (void *) src0[i]; exec.out[i] = (void *) dst0[i]; } - checkasm_call(comp_ref.func, &exec, comp_ref.priv, 0, 0, PIXELS / comp_ref.block_size, LINES); + checkasm_call(comp_ref->func, &exec, comp_ref->priv, 0, 0, PIXELS / comp_ref->block_size, LINES); - exec.block_size_in = comp_new.block_size * rw_pixel_bits(read_op) >> 3; - exec.block_size_out = comp_new.block_size * rw_pixel_bits(write_op) >> 3; + exec.block_size_in = comp_new->block_size * rw_pixel_bits(read_op) >> 3; + exec.block_size_out = comp_new->block_size * rw_pixel_bits(write_op) >> 3; for (int i = 0; i < NB_PLANES; i++) { exec.in[i] = (void *) src1[i]; exec.out[i] = (void *) dst1[i]; } - checkasm_call_checked(comp_new.func, &exec, comp_new.priv, 0, 0, PIXELS / comp_new.block_size, LINES); + checkasm_call_checked(comp_new->func, &exec, comp_new->priv, 0, 0, PIXELS / comp_new->block_size, LINES); for (int i = 0; i < NB_PLANES; i++) { const char *name = FMT("%s[%d]", report, i); @@ -287,8 +233,84 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], break; } - bench(comp_new.func, &exec, comp_new.priv, 0, 0, PIXELS / comp_new.block_size, LINES); + bench(comp_new->func, &exec, comp_new->priv, 0, 0, PIXELS / comp_new->block_size, LINES); } +} + +static void check_ops(const char *report, const unsigned ranges[NB_PLANES], + const SwsOp *ops) +{ + SwsContext *ctx = sws_alloc_context(); + if (!ctx) + return; + ctx->flags = SWS_BITEXACT; + + static const unsigned def_ranges[4] = {0}; + if (!ranges) + ranges = def_ranges; + + const SwsOp *read_op, *write_op; + SwsOpList oplist = { + .ops = (SwsOp *) ops, + .plane_src = {0, 1, 2, 3}, + .plane_dst = {0, 1, 2, 3}, + }; + + read_op = &ops[0]; + for (oplist.num_ops = 0; ops[oplist.num_ops].op; oplist.num_ops++) + write_op = &ops[oplist.num_ops]; + + for (int p = 0; p < NB_PLANES; p++) { + switch (read_op->type) { + case U8: + set_range(&oplist.comps_src.max[p], ranges[p], UINT8_MAX); + oplist.comps_src.min[p] = (AVRational) { 0, 1 }; + break; + case U16: + set_range(&oplist.comps_src.max[p], ranges[p], UINT16_MAX); + oplist.comps_src.min[p] = (AVRational) { 0, 1 }; + break; + case U32: + set_range(&oplist.comps_src.max[p], ranges[p], UINT32_MAX); + oplist.comps_src.min[p] = (AVRational) { 0, 1 }; + break; + case F32: + if (ranges[p] && ranges[p] <= INT_MAX) { + oplist.comps_src.max[p] = (AVRational) { ranges[p], 1 }; + oplist.comps_src.min[p] = (AVRational) { 0, 1 }; + } + break; + } + } + + /* Compile `ops` using both the asm and c backends */ + SwsCompiledOp comp_ref = {0}, comp_new = {0}; + const SwsOpBackend *backend_new = NULL; + + for (int n = 0; ff_sws_op_backends[n]; n++) { + const SwsOpBackend *backend = ff_sws_op_backends[n]; + const bool is_ref = !strcmp(backend->name, "c"); + if (is_ref || !comp_new.func) { + SwsCompiledOp comp; + int ret = ff_sws_ops_compile_backend(ctx, backend, &oplist, &comp); + if (ret == AVERROR(ENOTSUP)) + continue; + else if (ret < 0) + fail(); + else if (PIXELS % comp.block_size != 0) + fail(); + + if (is_ref) + comp_ref = comp; + if (!comp_new.func) { + comp_new = comp; + backend_new = backend; + } + } + } + + av_assert0(comp_ref.func && comp_new.func); + check_compiled(report, backend_new, read_op, write_op, ranges, &comp_ref, &comp_new); if (comp_new.func != comp_ref.func) ff_sws_compiled_op_unref(&comp_new); -- 2.52.0 >From 090b7ad90f3629b6666289411767809806b45fe1 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sun, 12 Apr 2026 02:26:33 +0200 Subject: [PATCH 5/5] tests/checkasm/sw_ops: explicitly test all backends The current code was a bit clumsy in that it always picked the first available backend when choosing the new function. This meant that some x86 paths were not being tested at all, whenever the memcpy backend (which has higher priority) could serve the request. This change makes it so that each backend is explicitly tested against only implementations provided by that same backend. checkasm: all 1575 tests passed (up from 1305) As an aside, it also lets us benchmark the memcpy backend directly against the C reference backend. Signed-off-by: Niklas Haas <[email protected]> --- tests/checkasm/sw_ops.c | 72 +++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/tests/checkasm/sw_ops.c b/tests/checkasm/sw_ops.c index b010dbdf13..6ec049a7af 100644 --- a/tests/checkasm/sw_ops.c +++ b/tests/checkasm/sw_ops.c @@ -111,7 +111,7 @@ static void set_range(AVRational *rangeq, unsigned range, unsigned range_def) *rangeq = (AVRational) { range, 1 }; } -static void check_compiled(const char *report, const SwsOpBackend *backend_new, +static void check_compiled(const char *name, const SwsOpBackend *backend, const SwsOp *read_op, const SwsOp *write_op, const int ranges[NB_PLANES], const SwsCompiledOp *comp_ref, @@ -124,6 +124,7 @@ static void check_compiled(const char *report, const SwsOpBackend *backend_new, static DECLARE_ALIGNED_64(char, dst0)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; static DECLARE_ALIGNED_64(char, dst1)[NB_PLANES][LINES][PIXELS * sizeof(uint32_t[4])]; + av_assert0(PIXELS % comp_new->block_size == 0); for (int p = 0; p < NB_PLANES; p++) { void *plane = src0[p]; switch (read_op->type) { @@ -178,14 +179,14 @@ static void check_compiled(const char *report, const SwsOpBackend *backend_new, } /** - * Don't use check_func() because the actual function pointer may be a - * wrapper shared by multiple implementations. Instead, take a hash of both - * the backend pointer and the active CPU flags. + * We can't use `check_func()` alone because the actual function pointer + * may be a wrapper or entry point shared by multiple implementations. + * Solve it by hashing in the active CPU flags as well. */ - uintptr_t id = (uintptr_t) backend_new; + uintptr_t id = (uintptr_t) comp_new->func; id ^= (id << 6) + (id >> 2) + 0x9e3779b97f4a7c15 + comp_new->cpu_flags; - if (check_key((void*) id, "%s", report)) { + if (check_key((void*) id, "%s.%s", backend->name, name)) { exec.block_size_in = comp_ref->block_size * rw_pixel_bits(read_op) >> 3; exec.block_size_out = comp_ref->block_size * rw_pixel_bits(write_op) >> 3; for (int i = 0; i < NB_PLANES; i++) { @@ -237,7 +238,7 @@ static void check_compiled(const char *report, const SwsOpBackend *backend_new, } } -static void check_ops(const char *report, const unsigned ranges[NB_PLANES], +static void check_ops(const char *name, const unsigned ranges[NB_PLANES], const SwsOp *ops) { SwsContext *ctx = sws_alloc_context(); @@ -283,37 +284,46 @@ static void check_ops(const char *report, const unsigned ranges[NB_PLANES], } } - /* Compile `ops` using both the asm and c backends */ - SwsCompiledOp comp_ref = {0}, comp_new = {0}; - const SwsOpBackend *backend_new = NULL; - + /* Always compile `ops` using the C backend as a reference */ + SwsCompiledOp comp_ref = {0}; for (int n = 0; ff_sws_op_backends[n]; n++) { const SwsOpBackend *backend = ff_sws_op_backends[n]; - const bool is_ref = !strcmp(backend->name, "c"); - if (is_ref || !comp_new.func) { - SwsCompiledOp comp; - int ret = ff_sws_ops_compile_backend(ctx, backend, &oplist, &comp); - if (ret == AVERROR(ENOTSUP)) - continue; - else if (ret < 0) - fail(); - else if (PIXELS % comp.block_size != 0) - fail(); - - if (is_ref) - comp_ref = comp; - if (!comp_new.func) { - comp_new = comp; - backend_new = backend; - } + if (strcmp(backend->name, "c")) + continue; + int ret = ff_sws_ops_compile_backend(ctx, backend, &oplist, &comp_ref); + if (ret < 0) { + av_assert0(ret != AVERROR(ENOTSUP)); + fail(); + goto done; } + break; } - av_assert0(comp_ref.func && comp_new.func); - check_compiled(report, backend_new, read_op, write_op, ranges, &comp_ref, &comp_new); + /* Iterate over every other backend, and test it against the C reference */ + for (int n = 0; ff_sws_op_backends[n]; n++) { + const SwsOpBackend *backend = ff_sws_op_backends[n]; + if (backend->hw_format != AV_PIX_FMT_NONE || !strcmp(backend->name, "c")) + continue; - if (comp_new.func != comp_ref.func) + if (!av_get_cpu_flags()) { + /* Also test once with the existing C reference to set the baseline */ + check_compiled(name, backend, read_op, write_op, ranges, &comp_ref, &comp_ref); + } + + SwsCompiledOp comp_new = {0}; + int ret = ff_sws_ops_compile_backend(ctx, backend, &oplist, &comp_new); + if (ret == AVERROR(ENOTSUP)) { + continue; + } else if (ret < 0) { + fail(); + goto done; + } + + check_compiled(name, backend, read_op, write_op, ranges, &comp_ref, &comp_new); ff_sws_compiled_op_unref(&comp_new); + } + +done: ff_sws_compiled_op_unref(&comp_ref); sws_free_context(&ctx); } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
