PR #25819 opened by Baptiste Coudurier (bcoudurier) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25819 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25819.patch
### checkasm/vf_idet: test the high bit depth function with real samples ff_idet_dsp_init() takes the bit depth since it selects the 16-bit function for depths above 8, but the test passed depth > 8, so it only ever checked the 8-bit function. The buffers were also too small for 16-bit samples. Pass the actual depth, size the buffers for 16-bit samples, fill them with values of that depth, and also check 10 and 15 bits. ### avfilter/aarch64: add NEON filter_line for idet 8-bit samples, and samples of up to 15 bits, where a + c and 2 * b still fit in 16-bit lanes. 16-bit samples would need 32-bit lanes, which ran no faster than the C loop, so they stay on it. Apple M3 Max, Apple clang 21, checkasm --bench: ``` idet8_c: 38.2 idet8_neon: 16.2 ( 2.36x) idet10_c: 27.2 idet10_neon: 20.3 ( 1.34x) idet15_c: 24.3 idet15_neon: 21.0 ( 1.16x) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) From b566ac61525b18c648a8983946bd3dc2e5f1e07b Mon Sep 17 00:00:00 2001 From: Baptiste Coudurier <[email protected]> Date: Fri, 6 Nov 2026 14:10:47 -0800 Subject: [PATCH 1/2] checkasm/vf_idet: test the high bit depth function with real samples ff_idet_dsp_init() takes the bit depth since it selects the 16-bit function for depths above 8, but the test passed depth > 8, so it only ever checked the 8-bit function. The buffers were also too small for 16-bit samples. Pass the actual depth, size the buffers for 16-bit samples, fill them with values of that depth, and also check 10 and 15 bits. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- tests/checkasm/vf_idet.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/checkasm/vf_idet.c b/tests/checkasm/vf_idet.c index 4bc0f5c846d..e63fa0363d1 100644 --- a/tests/checkasm/vf_idet.c +++ b/tests/checkasm/vf_idet.c @@ -27,19 +27,30 @@ static void check_idet(int depth) { IDETDSPContext dsp; - LOCAL_ALIGNED_32(uint8_t, in0, [WIDTH]); - LOCAL_ALIGNED_32(uint8_t, in1, [WIDTH]); - LOCAL_ALIGNED_32(uint8_t, in2, [WIDTH]); + LOCAL_ALIGNED_32(uint8_t, in0, [WIDTH * 2]); + LOCAL_ALIGNED_32(uint8_t, in1, [WIDTH * 2]); + LOCAL_ALIGNED_32(uint8_t, in2, [WIDTH * 2]); declare_func(int, const uint8_t *a, const uint8_t *b, const uint8_t *c, int w); - ff_idet_dsp_init(&dsp, depth > 8); + ff_idet_dsp_init(&dsp, depth); - for (int x = 0; x < WIDTH; x++) { - in0[x] = rnd() & 0xFF; - in1[x] = rnd() & 0xFF; - in2[x] = rnd() & 0xFF; + if (depth > 8) { + uint16_t *in0_16 = (uint16_t *)in0; + uint16_t *in1_16 = (uint16_t *)in1; + uint16_t *in2_16 = (uint16_t *)in2; + for (int x = 0; x < WIDTH; x++) { + in0_16[x] = rnd() & ((1 << depth) - 1); + in1_16[x] = rnd() & ((1 << depth) - 1); + in2_16[x] = rnd() & ((1 << depth) - 1); + } + } else { + for (int x = 0; x < WIDTH; x++) { + in0[x] = rnd() & 0xFF; + in1[x] = rnd() & 0xFF; + in2[x] = rnd() & 0xFF; + } } if (check_func(dsp.filter_line, "idet%d", depth)) { @@ -60,6 +71,12 @@ void checkasm_check_idet(void) check_idet(8); report("idet8"); + check_idet(10); + report("idet10"); + + check_idet(15); + report("idet15"); + check_idet(16); report("idet16"); } -- 2.54.0 From 83c9c15b0929a1937667f6a13635011865bdb1b5 Mon Sep 17 00:00:00 2001 From: Baptiste Coudurier <[email protected]> Date: Fri, 6 Nov 2026 14:10:47 -0800 Subject: [PATCH 2/2] avfilter/aarch64: add NEON filter_line for idet 8-bit samples, and samples of up to 15 bits, where a + c and 2 * b still fit in 16-bit lanes. 16-bit samples would need 32-bit lanes, which ran no faster than the C loop, so they stay on it. Apple M3 Max, Apple clang 21, checkasm --bench: idet8_c: 38.2 idet8_neon: 16.2 ( 2.36x) idet10_c: 27.2 idet10_neon: 20.3 ( 1.34x) idet15_c: 24.3 idet15_neon: 21.0 ( 1.16x) Co-Authored-By: Claude Opus 5.5 <[email protected]> --- libavfilter/aarch64/Makefile | 2 + libavfilter/aarch64/vf_idetdsp_init.c | 70 ++++++++++++++++++++ libavfilter/aarch64/vf_idetdsp_neon.S | 93 +++++++++++++++++++++++++++ libavfilter/vf_idetdsp.c | 4 +- libavfilter/vf_idetdsp.h | 1 + 5 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 libavfilter/aarch64/vf_idetdsp_init.c create mode 100644 libavfilter/aarch64/vf_idetdsp_neon.S diff --git a/libavfilter/aarch64/Makefile b/libavfilter/aarch64/Makefile index d09f7e40806..b2d72fe4b4c 100644 --- a/libavfilter/aarch64/Makefile +++ b/libavfilter/aarch64/Makefile @@ -1,9 +1,11 @@ OBJS-$(CONFIG_BWDIF_FILTER) += aarch64/vf_bwdif_init_aarch64.o OBJS-$(CONFIG_COLORDETECT_FILTER) += aarch64/vf_colordetect_init.o +OBJS-$(CONFIG_IDET_FILTER) += aarch64/vf_idetdsp_init.o OBJS-$(CONFIG_NLMEANS_FILTER) += aarch64/vf_nlmeans_init.o OBJS-$(CONFIG_THRESHOLD_FILTER) += aarch64/vf_threshold_init.o NEON-OBJS-$(CONFIG_BWDIF_FILTER) += aarch64/vf_bwdif_neon.o NEON-OBJS-$(CONFIG_COLORDETECT_FILTER) += aarch64/vf_colordetect_neon.o +NEON-OBJS-$(CONFIG_IDET_FILTER) += aarch64/vf_idetdsp_neon.o NEON-OBJS-$(CONFIG_NLMEANS_FILTER) += aarch64/vf_nlmeans_neon.o NEON-OBJS-$(CONFIG_THRESHOLD_FILTER) += aarch64/vf_threshold_neon.o diff --git a/libavfilter/aarch64/vf_idetdsp_init.c b/libavfilter/aarch64/vf_idetdsp_init.c new file mode 100644 index 00000000000..b13fad8cf39 --- /dev/null +++ b/libavfilter/aarch64/vf_idetdsp_init.c @@ -0,0 +1,70 @@ +/* + * 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 "libavutil/attributes.h" +#include "libavutil/aarch64/cpu.h" +#include "libavfilter/vf_idetdsp.h" + +int ff_idet_filter_line_neon(const uint8_t *a, const uint8_t *b, + const uint8_t *c, int w); +int ff_idet_filter_line_15bit_neon(const uint8_t *a, const uint8_t *b, + const uint8_t *c, int w); + +static int idet_filter_line_neon(const uint8_t *a, const uint8_t *b, + const uint8_t *c, int w) +{ + const int left_over = w & 31; + const int w_main = w - left_over; + int sum = 0; + + if (w_main > 0) + sum += ff_idet_filter_line_neon(a, b, c, w_main); + if (left_over > 0) + sum += ff_idet_filter_line_c(a + w_main, b + w_main, c + w_main, left_over); + return sum; +} + +static int idet_filter_line_15bit_neon(const uint8_t *a, const uint8_t *b, + const uint8_t *c, int w) +{ + const int left_over = w & 15; + const int w_main = w - left_over; + const int offset = w_main << 1; + int sum = 0; + + if (w_main > 0) + sum += ff_idet_filter_line_15bit_neon(a, b, c, w_main); + if (left_over > 0) + sum += ff_idet_filter_line_c_16bit(a + offset, b + offset, c + offset, left_over); + return sum; +} + +av_cold void ff_idet_dsp_init_aarch64(IDETDSPContext *dsp, int depth) +{ + int cpu_flags = av_get_cpu_flags(); + + if (!have_neon(cpu_flags)) + return; + + /* 16-bit samples need 32-bit lanes for a + c and 2 * b, which ran no + * faster than the C loop, so they stay on it. */ + if (depth <= 8) + dsp->filter_line = idet_filter_line_neon; + else if (depth <= 15) + dsp->filter_line = idet_filter_line_15bit_neon; +} diff --git a/libavfilter/aarch64/vf_idetdsp_neon.S b/libavfilter/aarch64/vf_idetdsp_neon.S new file mode 100644 index 00000000000..f7aa8aa8593 --- /dev/null +++ b/libavfilter/aarch64/vf_idetdsp_neon.S @@ -0,0 +1,93 @@ +/* + * 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 "libavutil/aarch64/asm.S" + +/* Sum of |a + c - 2 * b| over w pixels. + * x0: const uint8_t *a + * x1: const uint8_t *b + * x2: const uint8_t *c + * w3: int w, a non-zero multiple of 32 + */ +function ff_idet_filter_line_neon, export=1 + movi v16.4s, #0 + movi v17.4s, #0 + movi v18.4s, #0 + movi v19.4s, #0 +1: + ld1 {v0.16b, v1.16b}, [x0], #32 + ld1 {v2.16b, v3.16b}, [x1], #32 + ld1 {v4.16b, v5.16b}, [x2], #32 + subs w3, w3, #32 + uaddl v20.8h, v0.8b, v4.8b // a + c + uaddl2 v21.8h, v0.16b, v4.16b + uaddl v22.8h, v1.8b, v5.8b + uaddl2 v23.8h, v1.16b, v5.16b + ushll v24.8h, v2.8b, #1 // 2 * b + ushll2 v25.8h, v2.16b, #1 + ushll v26.8h, v3.8b, #1 + ushll2 v27.8h, v3.16b, #1 + uabd v20.8h, v20.8h, v24.8h + uabd v21.8h, v21.8h, v25.8h + uabd v22.8h, v22.8h, v26.8h + uabd v23.8h, v23.8h, v27.8h + uadalp v16.4s, v20.8h + uadalp v17.4s, v21.8h + uadalp v18.4s, v22.8h + uadalp v19.4s, v23.8h + b.gt 1b + + add v16.4s, v16.4s, v17.4s + add v18.4s, v18.4s, v19.4s + add v16.4s, v16.4s, v18.4s + addv s0, v16.4s + fmov w0, s0 + ret +endfunc + +/* Same as above for samples of up to 15 bits, where a + c and 2 * b + * still fit in 16-bit lanes. + * x0: const uint8_t *a + * x1: const uint8_t *b + * x2: const uint8_t *c + * w3: int w, in samples, a non-zero multiple of 16 + */ +function ff_idet_filter_line_15bit_neon, export=1 + movi v16.4s, #0 + movi v17.4s, #0 +1: + ld1 {v0.8h, v1.8h}, [x0], #32 + ld1 {v2.8h, v3.8h}, [x1], #32 + ld1 {v4.8h, v5.8h}, [x2], #32 + subs w3, w3, #16 + add v0.8h, v0.8h, v4.8h // a + c + add v1.8h, v1.8h, v5.8h + shl v2.8h, v2.8h, #1 // 2 * b + shl v3.8h, v3.8h, #1 + uabd v0.8h, v0.8h, v2.8h + uabd v1.8h, v1.8h, v3.8h + uadalp v16.4s, v0.8h + uadalp v17.4s, v1.8h + b.gt 1b + + add v16.4s, v16.4s, v17.4s + addv s0, v16.4s + fmov w0, s0 + ret +endfunc + diff --git a/libavfilter/vf_idetdsp.c b/libavfilter/vf_idetdsp.c index d7c271df20d..5676c056b19 100644 --- a/libavfilter/vf_idetdsp.c +++ b/libavfilter/vf_idetdsp.c @@ -56,7 +56,9 @@ int ff_idet_filter_line_c_16bit(const uint8_t *a, const uint8_t *b, const uint8_ void av_cold ff_idet_dsp_init(IDETDSPContext *dsp, int depth) { dsp->filter_line = depth > 8 ? ff_idet_filter_line_c_16bit : ff_idet_filter_line_c; -#if ARCH_X86 && HAVE_X86ASM +#if ARCH_AARCH64 + ff_idet_dsp_init_aarch64(dsp, depth); +#elif ARCH_X86 && HAVE_X86ASM ff_idet_dsp_init_x86(dsp, depth); #endif } diff --git a/libavfilter/vf_idetdsp.h b/libavfilter/vf_idetdsp.h index 174d77c171f..a6300986765 100644 --- a/libavfilter/vf_idetdsp.h +++ b/libavfilter/vf_idetdsp.h @@ -29,6 +29,7 @@ typedef struct IDETDSPContext { void ff_idet_dsp_init(IDETDSPContext *idet, int depth); +void ff_idet_dsp_init_aarch64(IDETDSPContext *idet, int depth); void ff_idet_dsp_init_x86(IDETDSPContext *idet, int depth); /* main fall-back for left-over */ -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
