PR #25779 opened by mkver URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25779 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25779.patch
>From 51531ba0e6e9829d211ace6d13819eda5c50635e Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Thu, 5 Nov 2026 10:17:37 +0100 Subject: [PATCH 1/4] avfilter/vf_limiter_init: Fix placement of av_unused In C23 mode av_unused expands to [[maybe_unused]] and the C23 standard attributes have stricter constraints (which are not fulfilled here) on where they are allowed to appear. Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavfilter/vf_limiter_init.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_limiter_init.h b/libavfilter/vf_limiter_init.h index f8e0a3319dd..fad61dfb387 100644 --- a/libavfilter/vf_limiter_init.h +++ b/libavfilter/vf_limiter_init.h @@ -48,7 +48,7 @@ static void limiter##n(const uint8_t *ssrc, uint8_t *ddst, \ LIMITER(8, uint8_t) LIMITER(16, uint16_t) -static av_unused void ff_limiter_init(LimiterContext *s, int depth) +av_unused static void ff_limiter_init(LimiterContext *s, int depth) { if (depth == 8) { s->dsp.limiter = limiter8; -- 2.54.0 >From fb18c48d9e56dcd57e8ca3a0884ba64aa4e84bfb Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Thu, 5 Nov 2026 10:27:27 +0100 Subject: [PATCH 2/4] avfilter/limiter,vf_limiter_init: Only use DSP context for DSP init Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavfilter/limiter.h | 15 --------------- libavfilter/vf_limiter.c | 16 +++++++++++++++- libavfilter/vf_limiter_init.h | 8 ++++---- tests/checkasm/vf_limiter.c | 4 ++-- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/libavfilter/limiter.h b/libavfilter/limiter.h index 8740c51bcb1..54d423d1f4b 100644 --- a/libavfilter/limiter.h +++ b/libavfilter/limiter.h @@ -21,7 +21,6 @@ #include <stddef.h> #include <stdint.h> -#include "avfilter.h" typedef struct LimiterDSPContext { void (*limiter)(const uint8_t *src, uint8_t *dst, @@ -29,20 +28,6 @@ typedef struct LimiterDSPContext { int w, int h, int min, int max); } LimiterDSPContext; -typedef struct LimiterContext { - const AVClass *class; - int min, max; /* user-facing option */ - int planes; - int nb_planes; - int plane_min[4]; /* resolved per-plane limits */ - int plane_max[4]; - int linesize[4]; - int width[4]; - int height[4]; - - LimiterDSPContext dsp; -} LimiterContext; - void ff_limiter_init_x86(LimiterDSPContext *dsp, int bpp); #endif /* AVFILTER_LIMITER_H */ diff --git a/libavfilter/vf_limiter.c b/libavfilter/vf_limiter.c index d3be9024918..6edc1912eb3 100644 --- a/libavfilter/vf_limiter.c +++ b/libavfilter/vf_limiter.c @@ -33,6 +33,20 @@ typedef struct ThreadData { AVFrame *out; } ThreadData; +typedef struct LimiterContext { + const AVClass *class; + int min, max; /* user-facing option */ + int planes; + int nb_planes; + int plane_min[4]; /* resolved per-plane limits */ + int plane_max[4]; + int linesize[4]; + int width[4]; + int height[4]; + + LimiterDSPContext dsp; +} LimiterContext; + #define OFFSET(x) offsetof(LimiterContext, x) #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_RUNTIME_PARAM @@ -88,7 +102,7 @@ static int config_input(AVFilterLink *inlink) s->width[1] = s->width[2] = AV_CEIL_RSHIFT(inlink->w, hsub); s->width[0] = s->width[3] = inlink->w; - ff_limiter_init(s, depth); + ff_limiter_init(&s->dsp, depth); return 0; } diff --git a/libavfilter/vf_limiter_init.h b/libavfilter/vf_limiter_init.h index fad61dfb387..a33b4dcc653 100644 --- a/libavfilter/vf_limiter_init.h +++ b/libavfilter/vf_limiter_init.h @@ -48,16 +48,16 @@ static void limiter##n(const uint8_t *ssrc, uint8_t *ddst, \ LIMITER(8, uint8_t) LIMITER(16, uint16_t) -av_unused static void ff_limiter_init(LimiterContext *s, int depth) +av_unused static void ff_limiter_init(LimiterDSPContext *s, int depth) { if (depth == 8) { - s->dsp.limiter = limiter8; + s->limiter = limiter8; } else { - s->dsp.limiter = limiter16; + s->limiter = limiter16; } #if ARCH_X86 && HAVE_X86ASM - ff_limiter_init_x86(&s->dsp, depth); + ff_limiter_init_x86(s, depth); #endif } diff --git a/tests/checkasm/vf_limiter.c b/tests/checkasm/vf_limiter.c index 2cfc1ee647f..28730677fd8 100644 --- a/tests/checkasm/vf_limiter.c +++ b/tests/checkasm/vf_limiter.c @@ -51,10 +51,10 @@ static void check_limiter(int depth) if (depth == 16) w /= 2; - LimiterContext s; + LimiterDSPContext s; ff_limiter_init(&s, depth); - if (check_func(s.dsp.limiter, "limiter%d", depth)) { + if (check_func(s.limiter, "limiter%d", depth)) { call_ref(src, dst_ref, WIDTH_PADDED, WIDTH_PADDED, w, HEIGHT, min, max); call_new(src, dst_new, WIDTH_PADDED, WIDTH_PADDED, w, HEIGHT, min, max); -- 2.54.0 >From 932ab13efe74f5d821c5a25f69f95188e80d3e37 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Thu, 5 Nov 2026 10:37:47 +0100 Subject: [PATCH 3/4] avfilter/vf_limiter_init: Avoid unnecessary shift/mask The limiter DSP functions use the common pattern of passing byte-strides and uint8_t* pointers which then get immediately converted to the proper type (i.e. uint16_t* for bitdepth >8) with the stride being converted via /= sizeof(type). There are several issues with this pattern: a) Strides are signed and use ptrdiff_t, yet sizeof(type) is size_t, so that the division is (on most systems) performed in an unsigned type, leading to logical right shifts. This is wrong and UB for negative strides, but it happens to work in practice, because pointer arithmetic entails an implicit multiplication by sizeof(type). b) Due to this, compilers (like GCC-16 here) actually do not shift at all; but they nevertheless need to mask the lowest bit of the strides (which would be eliminated by the division). This is unnecessary and eliminated by this commit. c) On systems without alignment requirements, it would even be possible for the strides for uint16_t to be odd. In this case, the current pattern would just be wrong. So change the code to convert to the byte-pointer to the actually used types in the outer loop and increment the byte-pointers using the byte-strides. Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavfilter/vf_limiter_init.h | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/libavfilter/vf_limiter_init.h b/libavfilter/vf_limiter_init.h index a33b4dcc653..f257e706ad0 100644 --- a/libavfilter/vf_limiter_init.h +++ b/libavfilter/vf_limiter_init.h @@ -25,23 +25,19 @@ #include "limiter.h" #define LIMITER(n, type) \ -static void limiter##n(const uint8_t *ssrc, uint8_t *ddst, \ +static void limiter##n(const uint8_t *src8, uint8_t *dst8, \ ptrdiff_t slinesize, ptrdiff_t dlinesize,\ int w, int h, int min, int max) \ { \ - const type *src = (const type *)ssrc; \ - type *dst = (type *)ddst; \ - \ - dlinesize /= sizeof(type); \ - slinesize /= sizeof(type); \ - \ for (int y = 0; y < h; y++) { \ + const type *src = (const type *)src8; \ + type *dst = (type *)dst8; \ for (int x = 0; x < w; x++) { \ dst[x] = av_clip(src[x], min, max); \ } \ \ - dst += dlinesize; \ - src += slinesize; \ + dst8 += dlinesize; \ + src8 += slinesize; \ } \ } -- 2.54.0 >From 74932c811c256c5cb42f8f837e32694dc78f954f Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Thu, 5 Nov 2026 11:04:50 +0100 Subject: [PATCH 4/4] tests/checkasm/vf_limiter: Only initialize when testing function Also randomize the padding in between src's lines (the padding is not guaranteed to be zero in actual usage) and use checkasm_randomize for it instead of doing it one byte at a time. Signed-off-by: Andreas Rheinhardt <[email protected]> --- tests/checkasm/vf_limiter.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/checkasm/vf_limiter.c b/tests/checkasm/vf_limiter.c index 28730677fd8..dac8a320512 100644 --- a/tests/checkasm/vf_limiter.c +++ b/tests/checkasm/vf_limiter.c @@ -36,14 +36,6 @@ static void check_limiter(int depth) declare_func(void, const uint8_t *, uint8_t *, ptrdiff_t, ptrdiff_t, int, int, int, int); - memset(dst_ref, 0, HEIGHT * WIDTH_PADDED); - memset(dst_new, 0, HEIGHT * WIDTH_PADDED); - memset(src, 0, HEIGHT * WIDTH_PADDED); - for (int y = 0; y < HEIGHT; y++) { - for (int x = 0; x < WIDTH; x++) - src[y * WIDTH_PADDED + x] = rnd() & 0xFF; - } - const int min = 16 << (depth - 8); const int max = 235 << (depth - 8); @@ -55,6 +47,10 @@ static void check_limiter(int depth) ff_limiter_init(&s, depth); if (check_func(s.limiter, "limiter%d", depth)) { + memset(dst_ref, 0, HEIGHT * WIDTH_PADDED); + memset(dst_new, 0, HEIGHT * WIDTH_PADDED); + checkasm_randomize(src, HEIGHT * WIDTH_PADDED); + call_ref(src, dst_ref, WIDTH_PADDED, WIDTH_PADDED, w, HEIGHT, min, max); call_new(src, dst_new, WIDTH_PADDED, WIDTH_PADDED, w, HEIGHT, min, max); -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
