PR #23656 opened by mkver URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23656 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23656.patch
>From 5b2ed6703339a8ca4743c3a1781b83ef7b639b2c Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Tue, 30 Jun 2026 14:28:01 +0200 Subject: [PATCH 1/4] avcodec/x86/me_cmp: Improve median_sad functions The median_abs functions involve computing a predictor for a pixel difference; deviations from this predicted pixel difference are summed to the result. For the leftmost element of a row, the predictor is simply the leftmost element of the last row (if any; otherwise zero). For the other elements, it is the median of top, left and (top + left - topleft). The current approach to deal with this is to treat the first element specially (by using a special accumulator just for it and mask at the end). But there is a better way: If the left and topleft predictors are zero, the median prediction is top and therefore yields the correct result for the leftmost element, obviating the need for the special case. Creating the registers with zeroed left predictors is easy: Just shift in the other direction to how it is done now. Old benchmarks: median_sad_0_c: 429.9 median_sad_0_ssse3: 43.7 ( 9.83x) median_sad_1_c: 189.9 median_sad_1_ssse3: 24.5 ( 7.75x) New benchmarks: median_sad_0_c: 431.1 median_sad_0_ssse3: 39.7 (10.84x) median_sad_1_c: 190.6 median_sad_1_ssse3: 20.4 ( 9.36x) Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavcodec/x86/me_cmp.asm | 82 +++++++++++++++------------------------ 1 file changed, 31 insertions(+), 51 deletions(-) diff --git a/libavcodec/x86/me_cmp.asm b/libavcodec/x86/me_cmp.asm index 17e8baeba0..5408946245 100644 --- a/libavcodec/x86/me_cmp.asm +++ b/libavcodec/x86/me_cmp.asm @@ -822,7 +822,7 @@ VSAD_APPROX 16, u ; the subtraction. The shifted columns are derived from the unshifted word ; vectors, so no out-of-bounds loads are made. ; %1: V columns 0-7, %2: V columns 8-15 -; %3: V columns 1-8, %4: V columns 9-16 (column 16 is zero) +; %3: 0w followed by V columns 0-6, %4: V columns 7-15 ; %5: scratch register, its contents are irrelevant %macro LOAD_V16 5 movu %1, [pix1q] @@ -833,19 +833,19 @@ VSAD_APPROX 16, u punpcklbw %3, %5 psubw %1, %3 ; V columns 0-7 psubw %2, %4 ; V columns 8-15 - palignr %3, %2, %1, 2 ; V columns 1-8 - psrldq %4, %2, 2 ; V columns 9-16 + pslldq %3, %1, 2 ; 0w followed by V columns 0-6 + palignr %4, %2, %1, 14 ; V columns 7-14 %endmacro ; Same as LOAD_V16 for one row of 8 pixels. -; %1: V columns 0-7, %2: V columns 1-8 (column 8 is zero), %3: scratch register +; %1: V columns 0-7, %2: 0w followed by V columns 0-6, %3: scratch register %macro LOAD_V8 3 movq %1, [pix1q] movq %2, [pix2q] punpcklbw %1, %3 punpcklbw %2, %3 psubw %1, %2 ; V columns 0-7 - psrldq %2, %1, 2 ; V columns 1-8 + pslldq %2, %1, 2 ; 0w, V columns 0-6 %endmacro ; Accumulate abs(%5 - mid_pred(%2, %3, %2 + %3 - %4)) into %1, using @@ -866,30 +866,27 @@ VSAD_APPROX 16, u %endmacro ; Accumulate one row's cost from the previous and current row vectors. -; %1-%4: previous row V (columns 0-7, 8-15, 1-8, 9-16) -; %5-%8: current row V (columns 0-7, 8-15, 1-8, 9-16), loaded here -; m0-m2 are the accumulators, m11/m12 temporaries, m14 scratch. The top +; %1-%4: previous row V (columns 0-7, 8-15, 0-6, 7-14) +; %5-%8: current row V (columns 0-7, 8-15, 0-6, 7-14), loaded here +; m0 is the accumulator, m11/m12 temporaries, m14 scratch. The top ; predictors %3/%4 are consumed by MEDIAN_ABS_ACC, but they belong to the ; previous row and are reloaded before being needed again. %macro PROCESS_ROW16 8 LOAD_V16 %5, %6, %7, %8, m14 add pix1q, strideq add pix2q, strideq - ; column 0: abs(V(0) - V(-stride)) - psubw m11, %5, %1 - pabsw m11, m11 - paddw m2, m11 - ; columns 1-8 and 9-16 - MEDIAN_ABS_ACC m0, %3, %5, %1, %7, m11, m12 - MEDIAN_ABS_ACC m1, %4, %6, %2, %8, m11, m12 + ; columns 0-7; no special case for the first element lacking + ; left and top-left predictors is needed here: The left vectors + ; have 0 as first element which leads to the desired result. + MEDIAN_ABS_ACC m0, %1, %7, %3, %5, m11, m12 + ; columns 8-15 + MEDIAN_ABS_ACC m0, %2, %8, %4, %6, m11, m12 %endmacro ; Register layout: -; m0 accumulator for columns 1-8 -; m1 accumulator for columns 9-16 (the last word is discarded at the end) -; m2 accumulator for column 0 (only the first word is used) -; m3-m6 one row's V (columns 0-7, 8-15, 1-8, 9-16) -; m7-m10 the other row's V (columns 0-7, 8-15, 1-8, 9-16) +; m0 accumulator +; m3-m6 one row's V (columns 0-7, 8-15, 0-6, 7-14) +; m7-m10 the other row's V (columns 0-7, 8-15, 0-6, 7-14) ; m11, m12 temporaries ; m14 scratch register for LOAD_V16 ; The loop is unrolled by two so the two register sets alternate the roles of @@ -901,11 +898,11 @@ cglobal median_sad16, 5, 5, 15, v, pix1, pix2, stride, h add pix2q, strideq ; first row: abs(V(0)) + sum of abs(V(j) - V(j-1)) - pabsw m2, m3 psubw m0, m5, m3 pabsw m0, m0 psubw m1, m6, m4 pabsw m1, m1 + paddw m0, m1 sub hd, 1 jle .end @@ -917,15 +914,8 @@ cglobal median_sad16, 5, 5, 15, v, pix1, pix2, stride, h sub hd, 1 jg .loop .end: - ; column 16 lies outside of the block and column 0 only contributes its - ; first word; the kept columns may end up in any lane since the final sum - ; is horizontal anyway - pslldq m1, 2 - pslldq m2, 14 - paddw m0, m1 - paddw m0, m2 - ; the per-word sums are at most 16 * 510, but their total needs more than - ; 16 bits: widen to dwords before the horizontal sum + ; the per-word sums are at most 2 * 16 * 510, but their total may need + ; more than 16 bits: widen to dwords before the horizontal sum pxor m1, m1 punpckhwd m12, m0, m1 punpcklwd m0, m1 @@ -939,26 +929,23 @@ INIT_XMM ssse3 MEDIAN_SAD16 ; Accumulate one row's cost from the previous and current row vectors. -; %1: previous row V columns 0-7, %2: previous row V columns 1-8 -; %3: current row V columns 0-7, %4: current row V columns 1-8 (loaded here) -; m0/m1 are the accumulators, m7/m8 temporaries, m9 scratch. +; %1: previous row V columns 0-7, %2: previous row V columns 0-6 +; %3: current row V columns 0-7, %4: current row V columns 0-6 (loaded here) +; m0 is the accumulator, m7/m8 temporaries, m9 scratch. %macro PROCESS_ROW8 4 LOAD_V8 %3, %4, m9 add pix1q, strideq add pix2q, strideq - ; column 0: abs(V(0) - V(-stride)) - psubw m7, %3, %1 - pabsw m7, m7 - paddw m1, m7 - ; columns 1-8 - MEDIAN_ABS_ACC m0, %2, %3, %1, %4, m7, m8 + ; No special case for the first element lacking left and top-left + ; predictors is needed here: The left vectors have 0 as first element + ; which leads to the desired result. + MEDIAN_ABS_ACC m0, %1, %4, %2, %3, m7, m8 %endmacro ; Register layout: -; m0 accumulator for columns 1-8 (the last word is discarded at the end) -; m1 accumulator for column 0 (only the first word is used) -; m2, m3 one row's V (columns 0-7, 1-8) -; m5, m6 the other row's V (columns 0-7, 1-8) +; m0 accumulator for columns 0-7 +; m2, m3 one row's V (columns 0-7, 0-6) +; m5, m6 the other row's V (columns 0-7, 0-6) ; m7, m8 temporaries ; m9 scratch register for LOAD_V8 ; As in median_sad16 the loop is unrolled by two so the two register sets @@ -970,8 +957,7 @@ cglobal median_sad8, 5, 5, 10, v, pix1, pix2, stride, h add pix2q, strideq ; first row: abs(V(0)) + sum of abs(V(j) - V(j-1)) - pabsw m1, m2 - psubw m0, m3, m2 + psubw m0, m2, m3 pabsw m0, m0 sub hd, 1 @@ -984,12 +970,6 @@ cglobal median_sad8, 5, 5, 10, v, pix1, pix2, stride, h sub hd, 1 jg .loop .end: - ; column 8 lies outside of the block and column 0 only contributes its - ; first word; the kept columns may end up in any lane since the final sum - ; is horizontal anyway - pslldq m0, 2 - pslldq m1, 14 - paddw m0, m1 pxor m4, m4 punpckhwd m7, m0, m4 punpcklwd m0, m4 -- 2.52.0 >From 8b52ea628dc918938c93fc0efee3184c91e59de3 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Tue, 30 Jun 2026 15:08:34 +0200 Subject: [PATCH 2/4] avcodec/x86/me_cmp: Reduce amount of registers used One can avoid one scratch register in MEDIAN_ABS_ACC (by clobbering topleft which isn't needed lateron). Furthermore, the earlier code requested more registers in cglobal than it actually used: median_sad8 only needs seven registers, median_sad16 only 11. One of these registers is actually unclobbered (read-only), so median_sad8 does not need to save and restore and non-volatile registers on Win64. Furthermore, the restriction of median_sad8 to x64 can now be lifted. Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavcodec/x86/me_cmp.asm | 90 ++++++++++++++++++------------------ libavcodec/x86/me_cmp_init.c | 2 +- 2 files changed, 45 insertions(+), 47 deletions(-) diff --git a/libavcodec/x86/me_cmp.asm b/libavcodec/x86/me_cmp.asm index 5408946245..f06d703020 100644 --- a/libavcodec/x86/me_cmp.asm +++ b/libavcodec/x86/me_cmp.asm @@ -814,7 +814,6 @@ VSAD_APPROX 16, u ;int ff_median_sad_<opt>(MPVEncContext *v, const uint8_t *pix1, const uint8_t *pix2, ; ptrdiff_t stride, int h); ;--------------------------------------------------------------------- -%if ARCH_X86_64 ; Load one row of 16 pixels from pix1/pix2 and compute V = pix1 - pix2 as ; int16 words. No zero register is needed: both byte vectors are unpacked @@ -849,22 +848,22 @@ VSAD_APPROX 16, u %endmacro ; Accumulate abs(%5 - mid_pred(%2, %3, %2 + %3 - %4)) into %1, using -; mid_pred(a, b, c) == max(min(a, b), min(max(a, b), c)). The top predictor -; %2 is not needed afterwards and is clobbered. -; %1: accumulator, %2: top, %3: left, %4: topleft, %5: values being predicted -; %6, %7: temporaries -%macro MEDIAN_ABS_ACC 7 +; mid_pred(a, b, c) == max(min(a, b), min(max(a, b), c)). +; %1: accumulator, %2: top (clobbered), %3: left, %4: topleft (clobbered), +; %5: values being predicted, %6 scratch register +%macro MEDIAN_ABS_ACC 6 paddw %6, %2, %3 ; top + left psubw %6, %4 ; top + left - topleft - pminsw %7, %2, %3 ; min(top, left) + pminsw %4, %2, %3 ; min(top, left) pmaxsw %2, %3 ; max(top, left) pminsw %2, %6 - pmaxsw %7, %2 ; mid_pred(top, left, top + left - topleft) - psubw %6, %5, %7 - pabsw %6, %6 - paddw %1, %6 + pmaxsw %4, %2 ; mid_pred(top, left, top + left - topleft) + psubw %4, %5 + pabsw %4, %4 + paddw %1, %4 %endmacro +%if ARCH_X86_64 ; Accumulate one row's cost from the previous and current row vectors. ; %1-%4: previous row V (columns 0-7, 8-15, 0-6, 7-14) ; %5-%8: current row V (columns 0-7, 8-15, 0-6, 7-14), loaded here @@ -872,114 +871,113 @@ VSAD_APPROX 16, u ; predictors %3/%4 are consumed by MEDIAN_ABS_ACC, but they belong to the ; previous row and are reloaded before being needed again. %macro PROCESS_ROW16 8 - LOAD_V16 %5, %6, %7, %8, m14 + LOAD_V16 %5, %6, %7, %8, m10 add pix1q, strideq add pix2q, strideq ; columns 0-7; no special case for the first element lacking ; left and top-left predictors is needed here: The left vectors ; have 0 as first element which leads to the desired result. - MEDIAN_ABS_ACC m0, %1, %7, %3, %5, m11, m12 + MEDIAN_ABS_ACC m0, %1, %7, %3, %5, m9 ; columns 8-15 - MEDIAN_ABS_ACC m0, %2, %8, %4, %6, m11, m12 + MEDIAN_ABS_ACC m0, %2, %8, %4, %6, m9 %endmacro ; Register layout: ; m0 accumulator -; m3-m6 one row's V (columns 0-7, 8-15, 0-6, 7-14) -; m7-m10 the other row's V (columns 0-7, 8-15, 0-6, 7-14) -; m11, m12 temporaries -; m14 scratch register for LOAD_V16 +; m1-m4 one row's V (columns 0-7, 8-15, 0-6, 7-14) +; m5-m8 the other row's V (columns 0-7, 8-15, 0-6, 7-14) +; m9 scratch register +; m10 dummy register (unclobbered) ; The loop is unrolled by two so the two register sets alternate the roles of ; previous and current row, which removes the per-row register copies. %macro MEDIAN_SAD16 0 -cglobal median_sad16, 5, 5, 15, v, pix1, pix2, stride, h - LOAD_V16 m3, m4, m5, m6, m14 +cglobal median_sad16, 5, 5, 10, v, pix1, pix2, stride, h + LOAD_V16 m1, m2, m3, m4, m10 add pix1q, strideq add pix2q, strideq ; first row: abs(V(0)) + sum of abs(V(j) - V(j-1)) - psubw m0, m5, m3 + psubw m0, m3, m1 + psubw m5, m4, m2 pabsw m0, m0 - psubw m1, m6, m4 - pabsw m1, m1 - paddw m0, m1 + pabsw m5, m5 + paddw m0, m5 sub hd, 1 jle .end .loop: - PROCESS_ROW16 m3, m4, m5, m6, m7, m8, m9, m10 + PROCESS_ROW16 m1, m2, m3, m4, m5, m6, m7, m8 sub hd, 1 jle .end - PROCESS_ROW16 m7, m8, m9, m10, m3, m4, m5, m6 + PROCESS_ROW16 m5, m6, m7, m8, m1, m2, m3, m4 sub hd, 1 jg .loop .end: ; the per-word sums are at most 2 * 16 * 510, but their total may need ; more than 16 bits: widen to dwords before the horizontal sum pxor m1, m1 - punpckhwd m12, m0, m1 + punpckhwd m2, m0, m1 punpcklwd m0, m1 - paddd m0, m12 - HADDD m0, m12 + paddd m0, m2 + HADDD m0, m2 movd eax, m0 RET %endmacro INIT_XMM ssse3 MEDIAN_SAD16 +%endif ; ARCH_X86_64 ; Accumulate one row's cost from the previous and current row vectors. ; %1: previous row V columns 0-7, %2: previous row V columns 0-6 ; %3: current row V columns 0-7, %4: current row V columns 0-6 (loaded here) -; m0 is the accumulator, m7/m8 temporaries, m9 scratch. +; m0 is the accumulator, m5 scratch register, m6 unclobbered dummy. %macro PROCESS_ROW8 4 - LOAD_V8 %3, %4, m9 + LOAD_V8 %3, %4, m7 add pix1q, strideq add pix2q, strideq ; No special case for the first element lacking left and top-left ; predictors is needed here: The left vectors have 0 as first element ; which leads to the desired result. - MEDIAN_ABS_ACC m0, %1, %4, %2, %3, m7, m8 + MEDIAN_ABS_ACC m0, %1, %4, %2, %3, m5 %endmacro ; Register layout: ; m0 accumulator for columns 0-7 -; m2, m3 one row's V (columns 0-7, 0-6) -; m5, m6 the other row's V (columns 0-7, 0-6) -; m7, m8 temporaries -; m9 scratch register for LOAD_V8 +; m1, m2 one row's V (columns 0-7, 0-6) +; m3, m4 the other row's V (columns 0-7, 0-6) +; m5 scratch register +; m7 dummy register, unclobbered ; As in median_sad16 the loop is unrolled by two so the two register sets ; alternate the roles of previous and current row. %macro MEDIAN_SAD8 0 -cglobal median_sad8, 5, 5, 10, v, pix1, pix2, stride, h - LOAD_V8 m2, m3, m9 +cglobal median_sad8, 5, 5, 6, v, pix1, pix2, stride, h + LOAD_V8 m1, m2, m7 add pix1q, strideq add pix2q, strideq ; first row: abs(V(0)) + sum of abs(V(j) - V(j-1)) - psubw m0, m2, m3 + psubw m0, m1, m2 pabsw m0, m0 sub hd, 1 jle .end .loop: - PROCESS_ROW8 m2, m3, m5, m6 + PROCESS_ROW8 m1, m2, m3, m4 sub hd, 1 jle .end - PROCESS_ROW8 m5, m6, m2, m3 + PROCESS_ROW8 m3, m4, m1, m2 sub hd, 1 jg .loop .end: pxor m4, m4 - punpckhwd m7, m0, m4 + punpckhwd m1, m0, m4 punpcklwd m0, m4 - paddd m0, m7 - HADDD m0, m7 + paddd m0, m1 + HADDD m0, m1 movd eax, m0 RET %endmacro INIT_XMM ssse3 MEDIAN_SAD8 - -%endif ; ARCH_X86_64 diff --git a/libavcodec/x86/me_cmp_init.c b/libavcodec/x86/me_cmp_init.c index 3d41f56874..1ba4003923 100644 --- a/libavcodec/x86/me_cmp_init.c +++ b/libavcodec/x86/me_cmp_init.c @@ -177,7 +177,7 @@ av_cold void ff_me_cmp_init_x86(MECmpContext *c, AVCodecContext *avctx) #if ARCH_X86_64 c->median_sad[0] = ff_median_sad16_ssse3; - c->median_sad[1] = ff_median_sad8_ssse3; #endif + c->median_sad[1] = ff_median_sad8_ssse3; } } -- 2.52.0 >From bd5d8653f29d121795611d65dd02a128250a3816 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Tue, 30 Jun 2026 15:22:09 +0200 Subject: [PATCH 3/4] tests/checkasm/motion: Improve reporting Signed-off-by: Andreas Rheinhardt <[email protected]> --- tests/checkasm/motion.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/checkasm/motion.c b/tests/checkasm/motion.c index 960a41a8ed..e3d5acc5df 100644 --- a/tests/checkasm/motion.c +++ b/tests/checkasm/motion.c @@ -124,11 +124,13 @@ static void check_motion(void) test_motion(buf, me_ctx.pix_abs[i][j]); } } + report("pix_abs"); #define XX(me_cmp_array) \ for (int i = 0; i < FF_ARRAY_ELEMS(me_ctx.me_cmp_array); i++) { \ snprintf(buf, sizeof(buf), #me_cmp_array "_%d", i); \ test_motion(buf, me_ctx.me_cmp_array[i]); \ + report(#me_cmp_array); \ } ME_CMP_1D_ARRAYS(XX) #undef XX @@ -137,5 +139,4 @@ static void check_motion(void) void checkasm_check_motion(void) { check_motion(); - report("motion"); } -- 2.52.0 >From 4982798e9adaacbde56b0ec46502903c063f31dd Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Tue, 30 Jun 2026 15:37:56 +0200 Subject: [PATCH 4/4] tests/checkasm/motion: Improve randomizing buffers Randomize four bytes at a time and only do it if needed. Signed-off-by: Andreas Rheinhardt <[email protected]> --- tests/checkasm/motion.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/checkasm/motion.c b/tests/checkasm/motion.c index e3d5acc5df..bbe61998ad 100644 --- a/tests/checkasm/motion.c +++ b/tests/checkasm/motion.c @@ -27,12 +27,12 @@ #include "checkasm.h" -static void fill_random(uint8_t *tab, int size) +static void fill_random(uint8_t *tab, size_t size) { - int i; - for (i = 0; i < size; i++) { - tab[i] = rnd() % 256; - } + for (size_t i = 0; i < (size & ~3); i += 4) + AV_WN32A(tab + i, rnd()); + for (size_t i = size & ~3; i < size; ++i) + tab[i] = rnd(); } static void test_motion(const char *name, me_cmp_func test_func) @@ -61,10 +61,10 @@ static void test_motion(const char *name, me_cmp_func test_func) } /* test correctness */ - fill_random(img1, WIDTH * HEIGHT); - fill_random(img2, WIDTH * HEIGHT); - if (check_func(test_func, "%s", name)) { + fill_random(img1, WIDTH * HEIGHT); + fill_random(img2, WIDTH * HEIGHT); + for (i = 0; i < ITERATIONS; i++) { x = rnd() % (WIDTH - look_ahead); y = rnd() % (HEIGHT - look_ahead); -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
