From bf907c4d841410481a5bed848ba5ae2e1e8fe68f Mon Sep 17 00:00:00 2001 From: lusoris Date: Sat, 27 Jun 2026 19:54:41 +0200 Subject: [PATCH] fix(denoise): meta=1 residual accumulators overflow uint32 at 4K/8K The opt-in meta=1 residual-energy accumulators (abs_sum/sq_sum, uint32) summed |in-out|*GS with GS=1e6 over ~W*H/16 pixels per slice. At 4K a slice is 518K pixels, so a realistic residual of 0.02 sums to 1.04e10 -- 2.4x over UINT32_MAX -- and wraps silently, corrupting residual_energy/sigma_estimate/psnr in the PEL_SEC_DENOISE telemetry (overflow threshold: residual >=0.008 at 4K, >=0.002 at 8K). Lowered GS to 1e3 (worst-case 8K sum 2.07e9 < UINT32_MAX) and mirrored the host divisor. residual_energy (the mean) stays accurate; sigma_estimate is now coarse-but-bounded (a precise sigma would need a 64-bit atomic accumulator, noted as a follow-up). Opt-in telemetry only -- no effect on denoised pixel output. Found by an adversarial shader bug-hunt; the same hunt's false positives (a var/varr naming nit, an intentional lockstep barrier) were adversarially refuted and not changed. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + .../0003-denoise-meta-uint32-overflow.md | 1 + .../0003-add-vf_pelorus_denoise_vulkan.patch | 28 ++++++++++++------- .../files/vf_pelorus_denoise_vulkan.c | 18 ++++++++---- 4 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 changelog.d/fixed/0003-denoise-meta-uint32-overflow.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 540b70c..4b1e8db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ All notable changes to Pelorus are documented here. The format is ### Fixed +- **Fixed**: the `vf_pelorus_denoise_vulkan` `meta=1` residual-energy accumulators (`abs_sum`/`sq_sum`, `uint32`) silently overflowed at 4K/8K — the per-slice sum `Σ |in−out|·GS` with `GS=1e6` over ~`W·H/16` pixels exceeds `UINT32_MAX` at realistic residuals (≥0.008 at 4K, ≥0.002 at 8K), wrapping mod 2³² and corrupting the `residual_energy`/`sigma_estimate`/`psnr` telemetry in `PEL_SEC_DENOISE`. Lowered `GS` to `1e3` (worst-case 8K sum 2.07e9 < UINT32_MAX) and mirrored the host divisor; `residual_energy` (the mean) stays accurate, `sigma_estimate` is now a coarse-but-bounded estimate (a precise sigma would need a 64-bit atomic accumulator — a noted follow-up). Opt-in telemetry only; **no effect on denoised pixel output**. Found by an adversarial shader bug-hunt. - **FFmpeg link of `libpelorus`**: the patch stack ran `require_pkg_config` (which adds cflags but not link libs), so a fully-linked `ffmpeg` failed with `undefined reference to pel_blob_pack` — a `.o`-only CI build never caught it. `generate.sh` now also emits `add_extralibs $libpelorus_extralibs`, and CI builds + links the `ffmpeg` binary and smoke-tests that the filters register. ADR-0104. - **`bd_rate.py` sign**: it computed `integ(baseline) − integ(pelorus)`, sign-flipping both BD-rate and BD-VMAF (a 50% saving read as ~+75%). Corrected to `integ(pelorus) − integ(baseline)`, with a self-test (half-bitrate ⇒ −50%, identical ⇒ 0%, double ⇒ +100%). ADR-0111. - **`block-unsafe-bash` hook**: the root-wipe guard matched any `rm -rf /…` path, blocking legitimate `rm -rf /tmp/…`. Narrowed to the filesystem root only. diff --git a/changelog.d/fixed/0003-denoise-meta-uint32-overflow.md b/changelog.d/fixed/0003-denoise-meta-uint32-overflow.md new file mode 100644 index 0000000..290ccce --- /dev/null +++ b/changelog.d/fixed/0003-denoise-meta-uint32-overflow.md @@ -0,0 +1 @@ +- **Fixed**: the `vf_pelorus_denoise_vulkan` `meta=1` residual-energy accumulators (`abs_sum`/`sq_sum`, `uint32`) silently overflowed at 4K/8K — the per-slice sum `Σ |in−out|·GS` with `GS=1e6` over ~`W·H/16` pixels exceeds `UINT32_MAX` at realistic residuals (≥0.008 at 4K, ≥0.002 at 8K), wrapping mod 2³² and corrupting the `residual_energy`/`sigma_estimate`/`psnr` telemetry in `PEL_SEC_DENOISE`. Lowered `GS` to `1e3` (worst-case 8K sum 2.07e9 < UINT32_MAX) and mirrored the host divisor; `residual_energy` (the mean) stays accurate, `sigma_estimate` is now a coarse-but-bounded estimate (a precise sigma would need a 64-bit atomic accumulator — a noted follow-up). Opt-in telemetry only; **no effect on denoised pixel output**. Found by an adversarial shader bug-hunt. diff --git a/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch b/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch index 611ce41..0a5a4f3 100644 --- a/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch +++ b/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch @@ -1,6 +1,6 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Lusoris -Date: Sat, 27 Jun 2026 16:51:25 +0200 +Date: Sat, 27 Jun 2026 19:53:01 +0200 Subject: [PATCH 03/18] feat(denoise): add vf_pelorus_denoise_vulkan (temporal denoise) MIME-Version: 1.0 @@ -40,8 +40,8 @@ ADR: docs/adr/0112-temporal-denoise.md configure | 2 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + - libavfilter/vf_pelorus_denoise_vulkan.c | 1258 +++++++++++++++++++++++ - 4 files changed, 1262 insertions(+) + libavfilter/vf_pelorus_denoise_vulkan.c | 1266 +++++++++++++++++++++++ + 4 files changed, 1270 insertions(+) create mode 100644 libavfilter/vf_pelorus_denoise_vulkan.c diff --git a/configure b/configure @@ -90,10 +90,10 @@ index 41761c6146..8f49f54808 100644 extern const FFFilter ff_vf_phase; diff --git a/libavfilter/vf_pelorus_denoise_vulkan.c b/libavfilter/vf_pelorus_denoise_vulkan.c new file mode 100644 -index 0000000000..0294cc6275 +index 0000000000..4de0147268 --- /dev/null +++ b/libavfilter/vf_pelorus_denoise_vulkan.c -@@ -0,0 +1,1258 @@ +@@ -0,0 +1,1266 @@ +/* + * Copyright 2026 Lusoris + * @@ -647,7 +647,15 @@ index 0000000000..0294cc6275 + GLSLD(denoise_glsl); + GLSLC(0, void main() ); + GLSLC(0, { ); -+ GLSLC(1, const float GS = 1000000.0; ); ++ /* Fixed-point scale for the uint32 residual accumulators. Kept at 1e3 (not ++ * 1e6) so the per-slice sum cannot overflow uint32: a slice covers ~W*H/16 ++ * pixels (2.07M at 8K) and the worst-case sum is 2.07M*1.0*1e3 = 2.07e9 < ++ * UINT32_MAX. GS=1e6 silently wrapped at >=8K (and at 4K for residuals ++ * >=0.008). residual_energy (the mean) stays accurate; sq_sum/sigma_est are ++ * a COARSE estimate at 1e3 (a precise sigma would need a 64-bit atomic ++ * accumulator — a documented follow-up). meta=1 telemetry only; no pixel ++ * effect. Mirror the divisor in attach_interop(). */ ++ GLSLC(1, const float GS = 1000.0; ); + GLSLC(1, ivec2 size; ); + GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); + /* slice = wg_index & 15 — equivalent to % 16u (PEL_SLICES is a power of @@ -742,15 +750,15 @@ index 0000000000..0294cc6275 + } + + if (cnt_y > 0) { -+ res_y = (float)(abs_y / 1e6 / (double)cnt_y); -+ msq = (float)(sq_y / 1e6 / (double)cnt_y); ++ res_y = (float)(abs_y / 1e3 / (double)cnt_y); ++ msq = (float)(sq_y / 1e3 / (double)cnt_y); + sigma_est = sqrtf(msq); + /* denoised-vs-input PSNR on a [0,1] domain: peak^2 / mean-square = 1/msq */ + psnr = msq > 0.0f ? (float)(10.0 * log10(1.0 / (double)msq)) : 0.0f; + } + if (cnt_c > 0) { -+ res_u = (float)(abs_u / 1e6 / (double)cnt_c); -+ res_v = (float)(abs_v / 1e6 / (double)cnt_c); ++ res_u = (float)(abs_u / 1e3 / (double)cnt_c); ++ res_v = (float)(abs_v / 1e3 / (double)cnt_c); + } + + memset(&meta, 0, sizeof(meta)); diff --git a/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c b/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c index 0294cc6..4de0147 100644 --- a/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c +++ b/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c @@ -551,7 +551,15 @@ static av_cold int init_filter(AVFilterContext *ctx) GLSLD(denoise_glsl); GLSLC(0, void main() ); GLSLC(0, { ); - GLSLC(1, const float GS = 1000000.0; ); + /* Fixed-point scale for the uint32 residual accumulators. Kept at 1e3 (not + * 1e6) so the per-slice sum cannot overflow uint32: a slice covers ~W*H/16 + * pixels (2.07M at 8K) and the worst-case sum is 2.07M*1.0*1e3 = 2.07e9 < + * UINT32_MAX. GS=1e6 silently wrapped at >=8K (and at 4K for residuals + * >=0.008). residual_energy (the mean) stays accurate; sq_sum/sigma_est are + * a COARSE estimate at 1e3 (a precise sigma would need a 64-bit atomic + * accumulator — a documented follow-up). meta=1 telemetry only; no pixel + * effect. Mirror the divisor in attach_interop(). */ + GLSLC(1, const float GS = 1000.0; ); GLSLC(1, ivec2 size; ); GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); /* slice = wg_index & 15 — equivalent to % 16u (PEL_SLICES is a power of @@ -646,15 +654,15 @@ static int attach_interop(PelorusDenoiseVulkanContext *s, AVFrame *out, } if (cnt_y > 0) { - res_y = (float)(abs_y / 1e6 / (double)cnt_y); - msq = (float)(sq_y / 1e6 / (double)cnt_y); + res_y = (float)(abs_y / 1e3 / (double)cnt_y); + msq = (float)(sq_y / 1e3 / (double)cnt_y); sigma_est = sqrtf(msq); /* denoised-vs-input PSNR on a [0,1] domain: peak^2 / mean-square = 1/msq */ psnr = msq > 0.0f ? (float)(10.0 * log10(1.0 / (double)msq)) : 0.0f; } if (cnt_c > 0) { - res_u = (float)(abs_u / 1e6 / (double)cnt_c); - res_v = (float)(abs_v / 1e6 / (double)cnt_c); + res_u = (float)(abs_u / 1e3 / (double)cnt_c); + res_v = (float)(abs_v / 1e3 / (double)cnt_c); } memset(&meta, 0, sizeof(meta));