PR #24348 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24348 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24348.patch
In the process, replace the av_assume() by just directly declaring intermediates as the correct type, which seems to help codegen here: checkasm: - CPU: AMD Ryzen 9 9950X3D 16-Core Processor (00B40F40) - Timing source: x86 (rdtsc) - Bench duration: 100000 µs per function (442374016 cycles) - Random seed: 2809724650 Benchmark results: (old) name cycles (vs ref) detect_alpha_8_full_c: 7582.7 detect_alpha_8_full_off_c: 7587.4 detect_alpha_8_limited_c: 13306.7 detect_alpha_16_full_c: 3482.5 detect_alpha_16_full_off_c: 3480.4 detect_alpha_16_limited_c: 8055.1 detect_range_8_c: 1271.4 detect_range_16_c: 1213.0 Benchmark results: (new) name cycles (vs ref) detect_alpha_8_full_c: 4791.9 detect_alpha_8_full_off_c: 4792.6 detect_alpha_8_limited_c: 8109.4 detect_alpha_16_full_c: 2967.2 detect_alpha_16_full_off_c: 2964.9 detect_alpha_16_limited_c: 10060.0 detect_range_8_c: 1272.1 detect_range_16_c: 1212.1 The ~20% slowdown for detect_alpha_16_limited_c is because the previous version of the code short-circuited this condition, which is faster when the compiler is not able to vectorize the loop body and falls back to a pure scalar loop. However, I don't think optimizing around this is necessary as it's a pure compiler optimization question. In particular, it actually _prevents_ better compilers from being able to vectorize this in the future. (As an example, doing the same short-circuiting for 8-bit would be a 2x regression on my end) People running into performance issues should just use the SIMD versions. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> # Summary of changes Briefly describe what this PR does and why. <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> From 8fd7e54fea72e5a6228c43905f66fa8870ca88a1 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 2 Sep 2026 12:04:03 +0200 Subject: [PATCH] avfilter/vf_colordetectdsp: rewrite C implementations as macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the process, replace the av_assume() by just directly declaring intermediates as the correct type, which seems to help codegen here: checkasm: - CPU: AMD Ryzen 9 9950X3D 16-Core Processor (00B40F40) - Timing source: x86 (rdtsc) - Bench duration: 100000 µs per function (442374016 cycles) - Random seed: 2809724650 Benchmark results: (old) name cycles (vs ref) detect_alpha_8_full_c: 7582.7 detect_alpha_8_full_off_c: 7587.4 detect_alpha_8_limited_c: 13306.7 detect_alpha_16_full_c: 3482.5 detect_alpha_16_full_off_c: 3480.4 detect_alpha_16_limited_c: 8055.1 detect_range_8_c: 1271.4 detect_range_16_c: 1213.0 Benchmark results: (new) name cycles (vs ref) detect_alpha_8_full_c: 4791.9 detect_alpha_8_full_off_c: 4792.6 detect_alpha_8_limited_c: 8109.4 detect_alpha_16_full_c: 2967.2 detect_alpha_16_full_off_c: 2964.9 detect_alpha_16_limited_c: 10060.0 detect_range_8_c: 1272.1 detect_range_16_c: 1212.1 The ~20% slowdown for detect_alpha_16_limited_c is because the previous version of the code short-circuited this condition, which is faster when the compiler is not able to vectorize the loop body and falls back to a pure scalar loop. However, I don't think optimizing around this is necessary as it's a pure compiler optimization question. In particular, it actually _prevents_ better compilers from being able to vectorize this in the future. (As an example, doing the same short-circuiting for 8-bit would be a 2x regression on my end) People running into performance issues should just use the SIMD versions. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavfilter/vf_colordetectdsp.h | 211 ++++++++++++-------------------- 1 file changed, 78 insertions(+), 133 deletions(-) diff --git a/libavfilter/vf_colordetectdsp.h b/libavfilter/vf_colordetectdsp.h index ef81559a6d..45544b71b6 100644 --- a/libavfilter/vf_colordetectdsp.h +++ b/libavfilter/vf_colordetectdsp.h @@ -54,146 +54,91 @@ void ff_color_detect_dsp_init_aarch64(FFColorDetectDSPContext *dsp, int depth, void ff_color_detect_dsp_init_x86(FFColorDetectDSPContext *dsp, int depth, int offset, enum AVColorRange color_range); -static inline int ff_detect_range_impl_c(const uint8_t *data, ptrdiff_t stride, - ptrdiff_t width, ptrdiff_t height, - uint8_t mpeg_min, uint8_t mpeg_max) -{ - while (height--) { - uint8_t cond = 0; - for (int x = 0; x < width; x++) { - const uint8_t val = data[x]; - cond |= val < mpeg_min || val > mpeg_max; - } - if (cond) - return 1; - data += stride; - } - - return 0; +#define DECL_DETECT_RANGE_IMPL(TYPE, NAME) \ +static inline int NAME(const uint8_t* data, ptrdiff_t stride, \ + ptrdiff_t width, ptrdiff_t height, \ + int mpeg_min, int mpeg_max) \ +{ \ + const TYPE min = mpeg_min; \ + const TYPE max = mpeg_max; \ + \ + while (height--) { \ + const TYPE *row = (const TYPE *) data; \ + uint8_t cond = 0; \ + for (int x = 0; x < width; x++) { \ + const TYPE val = row[x]; \ + cond |= val < min || val > max; \ + } \ + if (cond) \ + return 1; \ + data += stride; \ + } \ + \ + return 0; \ } -static inline int ff_detect_range_c(const uint8_t *data, ptrdiff_t stride, - ptrdiff_t width, ptrdiff_t height, - int mpeg_min, int mpeg_max) -{ - av_assume(mpeg_min >= 0 && mpeg_min <= UINT8_MAX); - av_assume(mpeg_max >= 0 && mpeg_max <= UINT8_MAX); - return ff_detect_range_impl_c(data, stride, width, height, mpeg_min, mpeg_max); +DECL_DETECT_RANGE_IMPL(uint8_t, ff_detect_range_c) +DECL_DETECT_RANGE_IMPL(uint16_t, ff_detect_range16_c) + +#define DECL_DETECT_ALPHA_FULL_IMPL(TYPE, INTER, NAME) \ +static inline int NAME(const uint8_t* color, ptrdiff_t color_stride, \ + const uint8_t* alpha, ptrdiff_t alpha_stride, \ + ptrdiff_t width, ptrdiff_t height, int alpha_max, \ + int mpeg_range, int offset) \ +{ \ + const TYPE max = alpha_max; \ + const INTER off = offset; \ + (void) mpeg_range; \ + \ + uint8_t transparent = 0; \ + while (height--) { \ + const TYPE *col = (const TYPE *) color; \ + const TYPE *alp = (const TYPE *) alpha; \ + uint8_t straight = 0; \ + for (int x = 0; x < width; x++) { \ + straight |= col[x] > alp[x] + off; \ + transparent |= alp[x] != max; \ + } \ + if (straight) \ + return FF_ALPHA_STRAIGHT; \ + color += color_stride; \ + alpha += alpha_stride; \ + } \ + return transparent ? FF_ALPHA_TRANSPARENT : 0; \ } -static inline int ff_detect_range16_impl_c(const uint8_t *data, ptrdiff_t stride, - ptrdiff_t width, ptrdiff_t height, - uint16_t mpeg_min, uint16_t mpeg_max) -{ - while (height--) { - const uint16_t *data16 = (const uint16_t *) data; - uint8_t cond = 0; - for (int x = 0; x < width; x++) { - const uint16_t val = data16[x]; - cond |= val < mpeg_min || val > mpeg_max; - } - if (cond) - return 1; - data += stride; - } +DECL_DETECT_ALPHA_FULL_IMPL(uint8_t, uint16_t, ff_detect_alpha_full_c) +DECL_DETECT_ALPHA_FULL_IMPL(uint16_t, uint32_t, ff_detect_alpha16_full_c) - return 0; +#define DECL_DETECT_ALPHA_LIMITED_IMPL(TYPE, INTER, NAME) \ +static inline int NAME(const uint8_t* color, ptrdiff_t color_stride, \ + const uint8_t* alpha, ptrdiff_t alpha_stride, \ + ptrdiff_t width, ptrdiff_t height, int alpha_max, \ + int mpeg_range, int offset) \ +{ \ + const TYPE max = alpha_max; \ + const INTER off = offset; \ + const INTER range = mpeg_range; \ + \ + uint8_t transparent = 0; \ + while (height--) { \ + const TYPE *col = (const TYPE *) color; \ + const TYPE *alp = (const TYPE *) alpha; \ + uint8_t straight = 0; \ + for (int x = 0; x < width; x++) { \ + straight |= (INTER) max * col[x] - off > range * alp[x]; \ + transparent |= alp[x] != max; \ + } \ + if (straight) \ + return FF_ALPHA_STRAIGHT; \ + color += color_stride; \ + alpha += alpha_stride; \ + } \ + return transparent ? FF_ALPHA_TRANSPARENT : 0; \ } -static inline int ff_detect_range16_c(const uint8_t *data, ptrdiff_t stride, - ptrdiff_t width, ptrdiff_t height, - int mpeg_min, int mpeg_max) -{ - av_assume(mpeg_min >= 0 && mpeg_min <= UINT16_MAX); - av_assume(mpeg_max >= 0 && mpeg_max <= UINT16_MAX); - return ff_detect_range16_impl_c(data, stride, width, height, mpeg_min, mpeg_max); -} - -static inline int -ff_detect_alpha_full_c(const uint8_t *color, ptrdiff_t color_stride, - const uint8_t *alpha, ptrdiff_t alpha_stride, - ptrdiff_t width, ptrdiff_t height, - int alpha_max, int mpeg_range, int offset) -{ - uint8_t transparent = 0; - while (height--) { - uint8_t straight = 0; - for (int x = 0; x < width; x++) { - straight |= color[x] > alpha[x] + offset; - transparent |= alpha[x] != alpha_max; - } - if (straight) - return FF_ALPHA_STRAIGHT; - color += color_stride; - alpha += alpha_stride; - } - return transparent ? FF_ALPHA_TRANSPARENT : 0; -} - -static inline int -ff_detect_alpha_limited_c(const uint8_t *color, ptrdiff_t color_stride, - const uint8_t *alpha, ptrdiff_t alpha_stride, - ptrdiff_t width, ptrdiff_t height, - int alpha_max, int mpeg_range, int offset) -{ - uint8_t transparent = 0; - while (height--) { - uint8_t straight = 0; - for (int x = 0; x < width; x++) { - straight |= alpha_max * color[x] - offset > mpeg_range * alpha[x]; - transparent |= alpha[x] != alpha_max; - } - if (straight) - return FF_ALPHA_STRAIGHT; - color += color_stride; - alpha += alpha_stride; - } - return transparent ? FF_ALPHA_TRANSPARENT : 0; -} - -static inline int -ff_detect_alpha16_full_c(const uint8_t *color, ptrdiff_t color_stride, - const uint8_t *alpha, ptrdiff_t alpha_stride, - ptrdiff_t width, ptrdiff_t height, - int alpha_max, int mpeg_range, int offset) -{ - uint8_t transparent = 0; - while (height--) { - const uint16_t *color16 = (const uint16_t *) color; - const uint16_t *alpha16 = (const uint16_t *) alpha; - uint8_t straight = 0; - for (int x = 0; x < width; x++) { - straight |= color16[x] > alpha16[x] + offset; - transparent |= alpha16[x] != alpha_max; - } - if (straight) - return FF_ALPHA_STRAIGHT; - color += color_stride; - alpha += alpha_stride; - } - return transparent ? FF_ALPHA_TRANSPARENT : 0; -} - -static inline int -ff_detect_alpha16_limited_c(const uint8_t *color, ptrdiff_t color_stride, - const uint8_t *alpha, ptrdiff_t alpha_stride, - ptrdiff_t width, ptrdiff_t height, - int alpha_max, int mpeg_range, int offset) -{ - uint8_t transparent = 0; - while (height--) { - const uint16_t *color16 = (const uint16_t *) color; - const uint16_t *alpha16 = (const uint16_t *) alpha; - for (int x = 0; x < width; x++) { - if ((int64_t) alpha_max * color16[x] - offset > (int64_t) mpeg_range * alpha16[x]) - return FF_ALPHA_STRAIGHT; - transparent |= alpha16[x] != alpha_max; - } - color += color_stride; - alpha += alpha_stride; - } - return transparent ? FF_ALPHA_TRANSPARENT : 0; -} +DECL_DETECT_ALPHA_LIMITED_IMPL(uint8_t, int32_t, ff_detect_alpha_limited_c) +DECL_DETECT_ALPHA_LIMITED_IMPL(uint16_t, int64_t, ff_detect_alpha16_limited_c) static av_cold inline void ff_color_detect_dsp_init(FFColorDetectDSPContext *dsp, int depth, int offset, -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
