PR #25289 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25289 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25289.patch
Fixes #24891. The scalar convolution kernels convert scaled float results to `int` before clipping. Accepted `rdiv` and `bias` settings can exceed the integer range. The 16-bit prewitt, roberts, scharr, and kirsch kernels have the same problem with large `scale`. Clip in float before conversion. Also clamp the x86 SSE4 3×3 vector and scalar-tail results before `cvttps2dq`, which otherwise turns oversized positive results into zero. Add generated FATE coverage for 8-bit column and square modes and 16-bit square mode. Verified with Clang’s `float-cast-overflow` sanitizer on AArch64 across all ten scalar convolution variants and four 16-bit edge filters. The three FATE cases pass. On x86-64, the 8-bit square SIMD case changes from an all-zero frame before the fix to the expected saturated frame; the vector and scalar-tail paths were checked. From 57fa5700ca804d8b012f5564bdf328c208403410 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Wed, 21 Oct 2026 05:32:05 +0000 Subject: [PATCH] avfilter/convolution: clip float results before integer conversion The scalar convolution kernels convert the scaled and biased float result to int before clipping it to the output depth. Large accepted rdiv or bias values make that conversion undefined. The 16-bit prewitt, roberts, scharr, and kirsch kernels have the same problem with a large scale. Clip in float first, then convert within the output range. The x86 SSE4 3x3 path also turns an oversized positive result into zero. Clamp its float vector and scalar-tail results before conversion. Add generated FATE cases for 8-bit column and square modes and 16-bit square mode. Fixes: #24891 Reported-by: ashamedbit Assisted-by: Fairy --- libavfilter/vf_convolution.c | 33 +++++++++++-------- libavfilter/x86/vf_convolution.asm | 8 +++++ tests/fate/filter-video.mak | 5 +++ tests/ref/fate/filter-convolution-clip16 | 6 ++++ tests/ref/fate/filter-convolution-clip8 | 6 ++++ .../ref/fate/filter-convolution-clip8-square | 6 ++++ 6 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 tests/ref/fate/filter-convolution-clip16 create mode 100644 tests/ref/fate/filter-convolution-clip8 create mode 100644 tests/ref/fate/filter-convolution-clip8-square diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c index cc55beb0be1..43bc666dc5c 100644 --- a/libavfilter/vf_convolution.c +++ b/libavfilter/vf_convolution.c @@ -105,6 +105,11 @@ typedef struct ThreadData { AVFrame *in, *out; } ThreadData; +static av_always_inline int clip_convolution(float value, int peak) +{ + return (int)av_clipf(value, 0, peak); +} + static void filter16_prewitt(uint8_t *dstp, int width, float scale, float delta, const int *const matrix, const uint8_t *c[], int peak, int radius, @@ -119,7 +124,7 @@ static void filter16_prewitt(uint8_t *dstp, int width, float sumb = AV_RN16A(&c[0][2 * x]) * -1 + AV_RN16A(&c[2][2 * x]) * 1 + AV_RN16A(&c[3][2 * x]) * -1 + AV_RN16A(&c[5][2 * x]) * 1 + AV_RN16A(&c[6][2 * x]) * -1 + AV_RN16A(&c[8][2 * x]) * 1; - dst[x] = av_clip(sqrtf(suma*suma + sumb*sumb) * scale + delta, 0, peak); + dst[x] = clip_convolution(sqrtf(suma*suma + sumb*sumb) * scale + delta, peak); } } @@ -135,7 +140,7 @@ static void filter16_roberts(uint8_t *dstp, int width, float suma = AV_RN16A(&c[0][2 * x]) * 1 + AV_RN16A(&c[1][2 * x]) * -1; float sumb = AV_RN16A(&c[4][2 * x]) * 1 + AV_RN16A(&c[3][2 * x]) * -1; - dst[x] = av_clip(sqrtf(suma*suma + sumb*sumb) * scale + delta, 0, peak); + dst[x] = clip_convolution(sqrtf(suma*suma + sumb*sumb) * scale + delta, peak); } } @@ -155,7 +160,7 @@ static void filter16_scharr(uint8_t *dstp, int width, suma /= 256.f; sumb /= 256.f; - dst[x] = av_clip(sqrtf(suma*suma + sumb*sumb) * scale + delta, 0, peak); + dst[x] = clip_convolution(sqrtf(suma*suma + sumb*sumb) * scale + delta, peak); } } @@ -204,7 +209,7 @@ static void filter16_kirsch(uint8_t *dstp, int width, sum4 = FFMAX(sum4, sum6); sum0 = FFMAX(sum0, sum4); - dst[x] = av_clip(FFABS(sum0) * scale + delta, 0, peak); + dst[x] = clip_convolution(FFABS(sum0) * scale + delta, peak); } } @@ -331,7 +336,7 @@ static void filter16_3x3(uint8_t *dstp, int width, (unsigned)AV_RN16A(&c[6][2 * x]) * matrix[6] + (unsigned)AV_RN16A(&c[7][2 * x]) * matrix[7] + (unsigned)AV_RN16A(&c[8][2 * x]) * matrix[8]; - dst[x] = av_clip((int)sum * rdiv + bias + 0.5f, 0, peak); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, peak); } } @@ -350,7 +355,7 @@ static void filter16_5x5(uint8_t *dstp, int width, for (i = 0; i < 25; i++) sum += (unsigned)AV_RN16A(&c[i][2 * x]) * matrix[i]; - dst[x] = av_clip((int)sum * rdiv + bias + 0.5f, 0, peak); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, peak); } } @@ -369,7 +374,7 @@ static void filter16_7x7(uint8_t *dstp, int width, for (i = 0; i < 49; i++) sum += (unsigned)AV_RN16A(&c[i][2 * x]) * matrix[i]; - dst[x] = av_clip((int)sum * rdiv + bias + 0.5f, 0, peak); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, peak); } } @@ -388,7 +393,7 @@ static void filter16_row(uint8_t *dstp, int width, for (i = 0; i < 2 * radius + 1; i++) sum += (unsigned)AV_RN16A(&c[i][2 * x]) * matrix[i]; - dst[x] = av_clip((int)sum * rdiv + bias + 0.5f, 0, peak); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, peak); } } @@ -410,7 +415,7 @@ static void filter16_column(uint8_t *dstp, int height, } for (int off16 = 0; off16 < width; off16++) { - dst[off16] = av_clip((int)sum[off16] * rdiv + bias + 0.5f, 0, peak); + dst[off16] = clip_convolution((int)sum[off16] * rdiv + bias + 0.5f, peak); } dst += dstride / 2; } @@ -430,7 +435,7 @@ static void filter_7x7(uint8_t *dst, int width, for (i = 0; i < 49; i++) sum += (unsigned)c[i][x] * matrix[i]; - dst[x] = av_clip_uint8((int)sum * rdiv + bias + 0.5f); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, 255); } } @@ -448,7 +453,7 @@ static void filter_5x5(uint8_t *dst, int width, for (i = 0; i < 25; i++) sum += (unsigned)c[i][x] * matrix[i]; - dst[x] = av_clip_uint8((int)sum * rdiv + bias + 0.5f); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, 255); } } @@ -466,7 +471,7 @@ static void filter_3x3(uint8_t *dst, int width, unsigned sum = (unsigned)c0[x] * matrix[0] + (unsigned)c1[x] * matrix[1] + (unsigned)c2[x] * matrix[2] + (unsigned)c3[x] * matrix[3] + (unsigned)c4[x] * matrix[4] + (unsigned)c5[x] * matrix[5] + (unsigned)c6[x] * matrix[6] + (unsigned)c7[x] * matrix[7] + (unsigned)c8[x] * matrix[8]; - dst[x] = av_clip_uint8((int)sum * rdiv + bias + 0.5f); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, 255); } } @@ -484,7 +489,7 @@ static void filter_row(uint8_t *dst, int width, for (i = 0; i < 2 * radius + 1; i++) sum += (unsigned)c[i][x] * matrix[i]; - dst[x] = av_clip_uint8((int)sum * rdiv + bias + 0.5f); + dst[x] = clip_convolution((int)sum * rdiv + bias + 0.5f, 255); } } @@ -505,7 +510,7 @@ static void filter_column(uint8_t *dst, int height, } for (int off16 = 0; off16 < width; off16++) { - dst[off16] = av_clip_uint8((int)sum[off16] * rdiv + bias + 0.5f); + dst[off16] = clip_convolution((int)sum[off16] * rdiv + bias + 0.5f, 255); } dst += dstride; } diff --git a/libavfilter/x86/vf_convolution.asm b/libavfilter/x86/vf_convolution.asm index 9ac9ef5d73e..55928f7394b 100644 --- a/libavfilter/x86/vf_convolution.asm +++ b/libavfilter/x86/vf_convolution.asm @@ -22,6 +22,7 @@ SECTION_RODATA half: dd 0.5 +max8: dd 255.0 data_p1: dd 1 data_n1: dd -1 data_p2: dd 2 @@ -114,6 +115,10 @@ cglobal filter_3x3, 4, 15, 7, dst, width, rdiv, bias, matrix, ptr, c0, c1, c2, c mulps m4, m0 ; sum *= rdiv addps m4, m1 ; sum += bias addps m4, m5 ; sum += 0.5 + maxps m4, m6 + movss m2, [max8] + VBROADCASTSS m2, m2 + minps m4, m2 cvttps2dq m4, m4 packssdw m4, m4 packuswb m4, m4 @@ -146,6 +151,9 @@ cglobal filter_3x3, 4, 15, 7, dst, width, rdiv, bias, matrix, ptr, c0, c1, c2, c mulss m4, m0 ; sum *= rdiv addss m4, m1 ; sum += bias addss m4, m5 ; sum += 0.5 + maxss m4, m6 + movss m2, [max8] + minss m4, m2 ; we don't have simple scalar instructions to convert ; from 32bit to 8bit with saturation, so here ; just use packed version SSE instructions for simplicity. diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index 4a065ec0320..50c960a74e4 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -1,6 +1,11 @@ # Contains the requirements added implicitly if CMD is video_filter. VIDEO_FILTER = $(call ALLYES, $(1:%=%_FILTER) $(2) FILE_PROTOCOL IMAGE2_DEMUXER PGMYUV_DECODER RAWVIDEO_ENCODER NUT_MUXER MD5_PROTOCOL) +FATE_FILTER-$(call FILTERFRAMECRC, COLOR FORMAT CONVOLUTION) += fate-filter-convolution-clip8 fate-filter-convolution-clip8-square fate-filter-convolution-clip16 +fate-filter-convolution-clip8: CMD = framecrc -lavfi "color=c=white:s=32x32:r=1:d=1,format=gray,convolution=0m=1|1|1:0mode=column:0bias=2147483647" -frames:v 1 +fate-filter-convolution-clip8-square: CMD = framecrc -lavfi "color=c=white:s=32x32:r=1:d=1,format=gray,convolution=0m=1|1|1|1|1|1|1|1|1:0bias=2147483647" -frames:v 1 +fate-filter-convolution-clip16: CMD = framecrc -lavfi "color=c=white:s=32x32:r=1:d=1,format=gray16le,convolution=0m=1|1|1|1|1|1|1|1|1:0rdiv=2147483647" -frames:v 1 + FATE_FILTER_SAMPLES-$(call FILTERDEMDECENCMUX, PERMS OWDENOISE TRIM SCALE, SMJPEG, MJPEG, RAWVIDEO, RAWVIDEO, PIPE_PROTOCOL) += fate-filter-owdenoise-sample fate-filter-owdenoise-sample: CMD = ffmpeg -auto_conversion_filters -idct simple -i $(TARGET_SAMPLES)/smjpeg/scenwin.mjpg -vf "trim=duration=0.5,perms=random,owdenoise=10:20:20:enable=not(between(t\,0.2\,1.2))" -an -f rawvideo - fate-filter-owdenoise-sample: REF = $(SAMPLES)/filter-reference/owdenoise-scenwin-jpeg.raw diff --git a/tests/ref/fate/filter-convolution-clip16 b/tests/ref/fate/filter-convolution-clip16 new file mode 100644 index 00000000000..8444e8a5c9a --- /dev/null +++ b/tests/ref/fate/filter-convolution-clip16 @@ -0,0 +1,6 @@ +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 32x32 +#sar 0: 1/1 +0, 0, 0, 1, 2048, 0xda6bf869 diff --git a/tests/ref/fate/filter-convolution-clip8 b/tests/ref/fate/filter-convolution-clip8 new file mode 100644 index 00000000000..546fb8096ab --- /dev/null +++ b/tests/ref/fate/filter-convolution-clip8 @@ -0,0 +1,6 @@ +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 32x32 +#sar 0: 1/1 +0, 0, 0, 1, 1024, 0x75a6fc2d diff --git a/tests/ref/fate/filter-convolution-clip8-square b/tests/ref/fate/filter-convolution-clip8-square new file mode 100644 index 00000000000..546fb8096ab --- /dev/null +++ b/tests/ref/fate/filter-convolution-clip8-square @@ -0,0 +1,6 @@ +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 32x32 +#sar 0: 1/1 +0, 0, 0, 1, 1024, 0x75a6fc2d -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
