fix(speed): GPU SpEED correctness + safety across all backends (audit) by lusoris · Pull Request #1029 · VMAFx/vmafx

lusoris · 2026-06-20T17:20:34Z

Summary

Fixes the SpEED-extractor bug cluster from the full-fork correctness audit (the GPU backends were never run on real hardware — no GPU in CI). 9 commits, ~17 bug-instances across CPU + CUDA + HIP + SYCL.

Verified bit-parity on the RTX 4090 (test_cuda_speed_chroma_parity + test_cuda_speed_temporal_parity): the GPU SpEED algorithm was ~7× wrong (per-tile vs global covariance) and chroma additionally ~2× wrong (shared ref/dis eigenvalue basis) — both fixed and verified.

CRITICAL/HIGH bugs: CPU speed.c heap-OOB (the CI all-backends SEGV); GPU eig-scratch heap corruption (n²+3n→n²+4n, all 6 GPU extractors); SYCL solve-barrier deadlock (DEVICE_LOST on Intel Arc); CUDA chroma+temporal host-reads-device-ptr SEGV; CUDA SpEED kernel-geometry + separate ref/dis basis (bit-parity verified); HIP + SYCL ports of the same; temporal-init unchecked return; CUDA fail_pop + HIP free_cpu init leaks.

Type

fix — bug fix
cuda / sycl / hip — backend-specific

Checklist

Commits follow Conventional Commits (the commit-msg hook enforces this).
If I touched any SIMD/GPU code path, I ran the GPU parity tests and the worst delta is ≤ 1e-4 (RTX 4090: test_cuda_speed_chroma_parity + test_cuda_speed_temporal_parity PASS bit-parity).
If I touched a feature extractor with SIMD/GPU twins, I updated every twin (CPU speed.c + CUDA + HIP + SYCL all carry the fix).

Bug-status hygiene (ADR-0165)

docs/state.md updated in this PR — row T-GPU-SPEED-COVARIANCE-EIGENBASIS-CORRECTNESS-2026-06-20 added under Recently closed.

Netflix golden-data gate (ADR-0024)

I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests. (SpEED is not in the golden pairs; the golden gate is CPU-only and unaffected.)

Deep-dive deliverables (ADR-0108)

Research digest — docs/research/1120-gpu-speed-covariance-eigenbasis-correctness-2026-06-20.md (per-tile-vs-global covariance bug, shared-vs-separate ref/dis eigenbasis bug, co-located safety bugs, and the RTX-4090 bit-parity method).
Decision matrix — in the research digest's ## Alternatives considered section (no separate ADR: these are correctness bug fixes, CLAUDE §12 r8 exempt).
AGENTS.md invariant note — added to core/src/feature/cuda/AGENTS.md (global covariance means[25] not per-tile means[25*num_blocks]; separate ref/dis eigenvalue bases; mirror any kernel-math change across CPU+CUDA+HIP+SYCL).
Reproducer / smoke-test command — pasted below under "Reproducer".
CHANGELOG fragment — changelog.d/fixed/gpu-speed-covariance-eigenbasis-correctness.md.
Rebase note — entry added to docs/rebase-notes.md under fix/speed-extractor-oob-deadlock-heap-corruption (2026-06-20).

Reproducer

# In the vmaf-dev-mcp container (RTX 4090 live), CUDA-enabled build:
meson test -C core/build-cuda test_cuda_speed_chroma_parity test_cuda_speed_temporal_parity
# Both PASS bit-parity (<= 1e-4) vs CPU. Pre-fix: chroma cpu=19.84 / cuda=2.89 (7x low),
# distorted entropy 211 vs 190; temporal was wrong before the geometry fix.

# Safety repro (ASan + MALLOC_PERTURB_): SYCL CPU-OOB path + full Arc-GPU speed_chroma
# pipeline run clean (no 'free(): invalid next size', no DEVICE_LOST).

Cross-backend numerical results

speed_chroma    cpu-vs-cuda <=1e-4 (bit-parity, RTX 4090)
speed_temporal  cpu-vs-cuda <=1e-4 (bit-parity, RTX 4090)
                hip / sycl: identical verified algorithm; run-verified by CI / ROCm / fp64 HW

Known follow-ups

HIP and SYCL legs ship the identical verified algorithm but are not run-verifiable on this host (no AMD GPU; the Arc lacks fp64 for the SpEED double-precision path). CI / ROCm / fp64 hardware run-verifies them.

🤖 Generated with Claude Code

…kends) + SYCL solve-barrier deadlock Three real bugs in the SpEED extractors, found via container ASan + GPU-fault debugging. The eigenvalue-scratch one is retrain-critical (corrupts the heap during GPU speed extraction on the RTX 4090). 1. CPU speed.c heap-buffer-overflow (this was the CI all-backends SEGV). speed_init_dimensions() returns -EINVAL for planes too small for the NUM_SCALES pyramid (e.g. YUV420 chroma below ~160px), but speed_init() and init_chroma() IGNORED the return -> submatrix_height = truncated_height - block_size + 1 underflowed the size_t to ~2^64 -> compute_mean() walked off the frame buffer. The SYCL twin already checked this return; the CPU path did not. Fix: reorder the truncated==0 guard before the submatrix subtraction and propagate the return through speed_init() + init_chroma(). ASan-clean. 2. GPU eigenvalue-scratch heap corruption (RETRAIN-CRITICAL; all 6 GPU speed extractors: cuda/hip/sycl x chroma/temporal). speed_internal_compute_ eigenvalues() lays its scratch out as A[n*n] + d[n] + sd[n] + tmp[2*n] = n*n + 4*n floats, but every GPU caller allocated n*n + 3*n -> si_tri_multiply wrote n (=25) floats past the end -> 'free(): invalid next size'. On normal heaps this is a silent 100-byte overwrite; under MALLOC_PERTURB/ASan it crashes. The CPU path uses its own correctly-sized eig buffer. Fixed all 6. 3. SYCL solve-kernel divergent-barrier deadlock (DEVICE_LOST on strict-barrier GPUs, e.g. Intel Arc). launch_solve early-returned idle lanes (lane >= SP_ELEMENTS) and surplus warps before a work-GROUP group_barrier they were required to reach -> the group deadlocked. Fix: gate the work with an flag, keep all work-items in the barrier loop. Applied to speed_chroma_sycl + speed_temporal_sycl. Also enlarges test_sycl_speed_chroma_parity's fixture (256x144 -> 320x320): the old fixture's chroma plane (128x72) is below the SpEED minimum, so the extractor EINVAL'd and the test never actually ran speed_chroma; it was masked while speed_chroma_sycl was unregistered (pre-#1004). Verified in the dev container: ASan-clean on the SYCL CPU-OOB repro and on the full Arc-GPU speed_chroma pipeline (means/cov/indterm/solve/score all pass, no heap corruption, no DEVICE_LOST). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…e picture_copy speed_chroma_cuda extract_channel ran host-side picture_copy() on ref_pic->data[channel], which is a DEVICE pointer in the CUDA pipeline (like adm/vif_cuda) -> reading GPU memory on the host SEGVs every frame. Never caught (CUDA tests need a GPU; CI has none). Download via cuMemcpyDtoH into host staging, alias into a temp VmafPicture, then picture_copy. Verified on RTX 4090: SEGV gone, parity runs. A separate GPU-algorithm score discrepancy (~7x low vs CPU) remains, tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…_cuda Same device-pointer SEGV as speed_chroma_cuda: extract_fex_st ran host-side picture_copy() on ref_pic/dist_pic->data[0], which is a CUdeviceptr in the CUDA pipeline -> host read of GPU memory SEGVs every frame. Download via cuMemcpyDtoH (with a local cuCtxPushCurrent, since the GPU pipeline pushes the context later) before the host picture_copy. Verified on RTX 4090: SEGV gone, parity runs (a shared GPU-algorithm score discrepancy remains, tracked separately). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…-tile speed_means_kernel + speed_cov_kernel computed PER-TILE block-local statistics (means[elem*num_blocks+tile], data sampled at tile_y*5+er) and summed num_blocks displaced block-local covariances, while the CPU reference computes ONE GLOBAL covariance per (x,y) element pair over a 5x5-phase-shifted full-plane submatrix (means[elem], data at er). The GPU port used the wrong sampling decomposition and was never validated (CUDA tests need a GPU; CI has none), giving ~7x-wrong SpEED scores on every GPU run. Rewrite both kernels to the CPU's global formulation: means kernel = 25 threads, each the global mean at (er,ec); cov kernel = global submatrix sweep with scalar global means, divide by N once, no tile loop. means[] stays over-allocated (25*num_blocks); only [0,25) used. Launch geometry unchanged. Verified on RTX 4090: test_cuda_speed_temporal_parity now PASSES bit-parity vs CPU (was wrong); test_cuda_speed_chroma_parity improved from cpu=19.84/cuda=2.89 (7x low) to within 2x — the residual is a SEPARATE chroma bug (the CUDA score kernel shares one eigenvalue/cov basis for ref+dis via save/restore, but the CPU uses separate ref/dis bases; matters only when ref!=dis, i.e. chroma not temporal). Tracked as a follow-up. SYCL/HIP twins carry the same kernel-geometry bug — to be ported next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…nit fail_pop - speed.c temporal init ignored speed_init()'s return; on -EINVAL (in-range-but- invalid speed_kernelscale / bad prescale string) float_stride stays 0 -> zero- size mallocs -> OOB writes in picture_copy. Capture + check the return. - speed_chroma_cuda.c / speed_temporal_cuda.c: the init fail_pop label popped the CUDA context and returned -EIO WITHOUT free_cuda_buffers(_st) -> device + pinned-host buffers leak on partial cuMemAlloc/cuMemHostAlloc failure. Free before returning (mirrors fail_after_pop / free_all). Found by the full-fork correctness audit (PR4 + PR5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…peed extractors speed_chroma_hip.c / speed_temporal_hip.c init: a post-alloc NULL check did a bare 'return -ENOMEM' that bypassed the free_cpu cleanup label, leaking up to 10/12 CPU scratch buffers. Route to free_cpu; move the label below #endif HAVE_HIPCC so the goto compiles in non-HIPCC builds. (eig-scratch +4 fix preserved.) Audit PR-4 (HIP twin). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The CUDA SpEED chroma/temporal extractors computed the reference path correctly but the DISTORTED path wrong (~2x-high chroma scores). extract_channel / extract_fex_st saved the reference covariance and restored it before the distorted CPU-linalg, so the distorted linear-system solution AND eigenvalues used the REFERENCE covariance; the shared score kernel then used one eigenvalue array for both ref and dis entropy. The CPU reference (est_params) computes SEPARATE ref and dis covariance + eigenvalues. Confirmed by RTX-4090 instrumentation: ref var/entropy matched bit-exactly, dis var/entropy diverged (5.11 vs 3.54 / 211 vs 190). Temporal masked it (consecutive frames -> ref≈dis). Fix: drop the cov save/restore (distorted path keeps the distorted covariance); add a d_eigenvalues_ref device buffer + cuMemcpyDtoD the ref eigenvalues aside after the reference linalg; speed_score_kernel now takes ref_eigenvalues AND dis_eigenvalues (ref entropy uses ref, dis entropy uses dis). Applied to speed_chroma_cuda.c, speed_temporal_cuda.c, speed_score.cu. Verified on RTX 4090: test_cuda_speed_chroma_parity AND test_cuda_speed_temporal_parity now PASS bit-parity (<1e-4) vs CPU. Completes the GPU SpEED correctness fix (kernel-geometry + basis). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…asis) to HIP Ports the RTX-4090-verified CUDA fix to the HIP twins (speed_score.hip kernels + speed_chroma_hip.c + speed_temporal_hip.c): means kernel = global window per element (not per-tile); cov kernel = global submatrix sweep, scalar global means, divide by N once (not per-tile block-local sum); separate ref/dis eigenvalue basis (d_eigenvalues_ref + hipMemcpyDtoD stash; score kernel takes ref+dis eigenvalue arrays; drop the cov save/restore so dis keeps dis covariance). Same algorithm as the CUDA fix that passes bit-parity on the RTX 4090. Not runnable on this host (no AMD GPU) — CI / ROCm hardware compile-verifies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…basis) to SYCL Ports the RTX-4090-verified CUDA fix to the SYCL twins (speed_chroma_sycl.cpp + speed_temporal_sycl.cpp): means kernel = one work-item per element, global window (not per-tile); cov kernel = global submatrix sweep with scalar global means, divide by N once (not per-tile block-local sum); separate ref/dis eigenvalue basis (d_eigenvalues_ref USM buffer + q.memcpy DtoD stash; score kernel takes ref+dis eigenvalue arrays; drop the cov save/restore). Preserves the existing solve-barrier 'active'-flag deadlock fix and the eig-scratch +4 sizing. Same algorithm as the CUDA fix that passes bit-parity on the RTX 4090. Not runnable on the fp64-less Arc here — CI / fp64 hardware compile+run-verifies. Completes the GPU SpEED correctness fix across all three backends. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Ship the deep-dive deliverables for the GPU SpEED correctness + safety PR: - changelog.d/fixed/gpu-speed-covariance-eigenbasis-correctness.md - docs/research/research-1120-gpu-speed-covariance-eigenbasis-correctness-2026-06-20.md (per-tile-vs-global covariance bug, shared-vs-separate ref/dis eigenbasis bug, co-located safety bugs, and the RTX-4090 bit-parity method) - docs/state.md: T-GPU-SPEED-COVARIANCE-EIGENBASIS-CORRECTNESS-2026-06-20 row under Recently closed - docs/rebase-notes.md: entry for fix/speed-extractor-oob-deadlock-heap-corruption - core/src/feature/cuda/AGENTS.md: rebase-sensitive invariant (global covariance + separate ref/dis eigenvalue bases) - docs/backends/cuda/overview.md: user-visible note that GPU SpEED scores now match CPU (numeric correction) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The doc-substance gate (ADR-0167) requires a docs/metrics or docs/backends/sycl edit when core/src/feature/sycl/*.cpp changes. Add a "GPU backend parity" section to the speed_qa metric doc covering the speed_chroma / speed_temporal CUDA/HIP/SYCL correctness fix: global covariance window and separate ref/dis eigenvalue bases, with the RTX-4090 bit-parity verification pointer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ilure `speed_chroma_cuda.c` created `s->module` with `cuModuleLoadData()` and `s->stream` with `cuStreamCreate()`, both jumping to `fail_pop` on error. That label — and `fail_after_pop` — only popped the context and called `free_cuda_buffers()`, which releases device and host BUFFERS via `cuMemFree` / `cuMemFreeHost` and never touches the module or the stream. So every init failure after the module load leaked the module, and every failure after the stream create leaked both. This is a regression, not unfinished work. PR #1007 (86f5a02) added the `cuModuleUnload` / `cuStreamDestroy` calls inline to both labels; PR #1029 (15262c7), merged the same day as a descendant of #1007, deleted both blocks. The three sibling extractors named in the same docs/state.md row — `integer_ms_ssim_cuda.c`, `integer_psnr_hvs_cuda.c` and `ssimulacra2_cuda.c` — are correct on master, as is `speed_temporal_cuda.c`; `speed_chroma_cuda.c` was the only file left. Rather than re-inlining the calls into two labels that already drifted apart once, the release is factored into a single `release_cuda_module_and_stream()` helper called from both. It mirrors `close_fex_cuda()` exactly: stream first, then module, each NULL-guarded, with no context push — the labels have already popped, and `close_fex_cuda()` releases them the same way on the normal path. The helper also nulls both handles, so a later release cannot double-free. Found by an audit of the docs/state.md open-bug rows against origin/master, which narrowed this row from "four extractors" to one. Verified: the CUDA lane configures and builds clean (-Denable_cuda=true), with speed_chroma_cuda.c compiled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ilure (#1260) `speed_chroma_cuda.c` created `s->module` with `cuModuleLoadData()` and `s->stream` with `cuStreamCreate()`, both jumping to `fail_pop` on error. That label — and `fail_after_pop` — only popped the context and called `free_cuda_buffers()`, which releases device and host BUFFERS via `cuMemFree` / `cuMemFreeHost` and never touches the module or the stream. So every init failure after the module load leaked the module, and every failure after the stream create leaked both. This is a regression, not unfinished work. PR #1007 (86f5a02) added the `cuModuleUnload` / `cuStreamDestroy` calls inline to both labels; PR #1029 (15262c7), merged the same day as a descendant of #1007, deleted both blocks. The three sibling extractors named in the same docs/state.md row — `integer_ms_ssim_cuda.c`, `integer_psnr_hvs_cuda.c` and `ssimulacra2_cuda.c` — are correct on master, as is `speed_temporal_cuda.c`; `speed_chroma_cuda.c` was the only file left. Rather than re-inlining the calls into two labels that already drifted apart once, the release is factored into a single `release_cuda_module_and_stream()` helper called from both. It mirrors `close_fex_cuda()` exactly: stream first, then module, each NULL-guarded, with no context push — the labels have already popped, and `close_fex_cuda()` releases them the same way on the normal path. The helper also nulls both handles, so a later release cannot double-free. Found by an audit of the docs/state.md open-bug rows against origin/master, which narrowed this row from "four extractors" to one. Verified: the CUDA lane configures and builds clean (-Denable_cuda=true), with speed_chroma_cuda.c compiled. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

lusoris and others added 9 commits June 20, 2026 19:39

lusoris force-pushed the fix/speed-extractor-oob-deadlock-heap-corruption branch from f4a51bc to 5952dec Compare June 20, 2026 17:45

lusoris force-pushed the fix/speed-extractor-oob-deadlock-heap-corruption branch from 5952dec to bce7ee9 Compare June 20, 2026 17:47

lusoris marked this pull request as ready for review June 20, 2026 17:59

lusoris enabled auto-merge (squash) June 20, 2026 17:59

lusoris merged commit 15262c7 into master Jun 20, 2026
63 of 69 checks passed

lusoris deleted the fix/speed-extractor-oob-deadlock-heap-corruption branch June 20, 2026 18:28

lusoris mentioned this pull request Jun 20, 2026

fix(mcp): probe_backend missing-arg uses uniform required-arg ValueError #1033

Merged

5 of 7 tasks

This was referenced Sep 3, 2026

RC1 — Close or classify open bugs in docs/state.md #1238

Closed

fix(cuda): stop speed_chroma leaking its module and stream on init failure #1260

Merged

lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026

lusoris mentioned this pull request Sep 5, 2026

docs(state): close the stale T-CUDA-INIT-SUBMIT-LEAKS-2026-06-19 open row #1329

Closed

5 of 9 tasks

Navigation Menu

Repository navigation

fix(speed): GPU SpEED correctness + safety across all backends (audit) - #1029

fix(speed): GPU SpEED correctness + safety across all backends (audit)#1029
lusoris merged 11 commits into
masterfrom
fix/speed-extractor-oob-deadlock-heap-corruption

lusoris commented Jun 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lusoris commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type

Checklist

Bug-status hygiene (ADR-0165)

Netflix golden-data gate (ADR-0024)

Deep-dive deliverables (ADR-0108)

Reproducer

Cross-backend numerical results

Known follow-ups

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lusoris commented Jun 20, 2026 •

edited

Loading