Repository navigation
fix(speed): size speed_temporal and speed_chroma frame buffers correctly - #1643
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lusoris
force-pushed
the
fix/speed-temporal-prescale-overflow
branch
from
September 30, 2026 20:03
38b0dde to
e7f65f9
Compare
| mu_assert("extraction failed", extract_err == 0); | ||
| for (unsigned i = 0; i < FB_FRAMES; i++) { | ||
| mu_assert("same scaled size, different score: the resample was skipped", | ||
| score_a[i] == score_b[i]); |
19 of 26 tasks
Allocates speed_temporal frame buffers using alloc_height so that speed_prescale > 1 (which resamples to a taller plane) does not overrun the heap under AddressSanitizer or corrupt memory in release builds (Netflix/vmaf#1626, Netflix/vmaf#1627). In addition, speed_prescale_resamples() detects resampling when lround(dim * prescale) changes plane dimensions. Derives speed_chroma buffer extents using vmaf_chroma_extent() ceiling division on CPU, CUDA, and HIP instead of integer floor division, preventing picture_copy() from overflowing buffers on odd dimensions in subsampled formats (4:2:0, 4:2:2). Fixes launch dimensions in speed_temporal_cuda solve kernel to cap threads per block to 256 (8 warps) and scale blocks, avoiding CUDA invalid value errors when u_nb > 256. Adds test_speed_frame_buffers covering prescale factors 1.0, 1.5, 2.0, 4.0 and odd dimensions under ASan.
lusoris
force-pushed
the
fix/speed-temporal-prescale-overflow
branch
from
September 30, 2026 23:57
e7f65f9 to
9a20d77
Compare
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
…es() #1643 replaced speed_internal.c's SI_ALMOST_EQUAL with the shared speed_prescale_resamples() helper. The device geometry spelled the same rule out by hand with the removed macro; it now calls the helper, so the CPU and the device twins decide the resample from one function.
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
* perf(cuda): run cambi and SpEED entirely on the device cambi_cuda, speed_chroma_cuda and speed_temporal_cuda no longer hand work back to the host inside a frame. Each frame reads back one small result block (88 bytes for cambi, 40 for SpEED) and waits once, in collect(); the twins read the planes the CUDA engine already uploaded and upload nothing of their own. cambi_cuda used to download the distorted picture, preprocess it on the host and read the image and mask back at every scale for host c-values and top-K pooling. It now runs the ADR-1357 design on CUDA: twelve kernels for preprocessing, the spatial mask, decimation, the mode filter, the column-histogram c-values and an exact 128-bit top-K sum. Scores equal the CPU's to the last bit whenever cambi.c's own double sum is exact. The twin also gains cambi.c's guard against windows above 65 x 65. The SpEED twins run the ADR-1358 chain, 25x25 eigenvalues and QR included, in nine kernels shared through speed_cuda_pipeline.c. Every rounding the CPU performs is spelled with a round-to-nearest intrinsic and the fatbin builds with --fmad=false, so the scores equal a CPU build that rounds log2f correctly and does not fuse multiply-adds. cambi.c exports the host helpers both device twins need, and speed_internal_gpu_configure() sets up the SYCL and CUDA SpEED pipelines; the SYCL twins drop their private copies. CPU output is unchanged. There is no NVIDIA device on this host. The kernels were built for sm_80 to sm_120 and checked frame by frame through a host emulation of the CUDA driver; the two state rows stay open with the commands to verify and time the port on ryzen-4090-arc. Three rows are opened for what the checks found: the lanczos4 prescale drift of both device twins, the CPU speed_temporal buffer overflow with speed_prescale above 1, and the dev image's -march=native FMA drift of the CPU SpEED scores. ADR-1379, ADR-1380, Research-1379. * fix(ci): retire the issue-provenance anchors of the removed CAMBI host code The Release Script Contract failed on #1639: check-issue-reference-provenance pins five historical blocks that cite lusoris/vmaf#857 and #870, four of them in integer_cambi_cuda.c (the dispatch helpers and the host download step the device-resident rewrite removed) and one in docs/metrics/cambi.md. The four code contracts go with the code they described. The CAMBI page keeps its history as an "Implementation note (before ADR-1379)" block that still names lusoris/vmaf#870, so that contract stays. * fix(speed): correct 48 log2 hard cases in CUDA and SYCL twins The fp32-pair log2 evaluation misrounds 48 positive finite floats whose exact log2 falls closer than 2^-45 to a rounding boundary. Both device twins now look the input up in a shared table (speed_log2_hard_cases.h) and return the correctly rounded result. The common path costs one fraction-field compare. The CUDA cambi twin also drops a score < 0.0 clamp that the CPU does not perform. Contract tests verify the table entries against quad-precision log2 and detect removal of the correction call in both twins. * fix(ci): suppress modernize-use-using in speed_gpu_common.h and update hardware evidence * fix(tidy): name the includers and cite ADR-1138 in the speed_gpu_common.h NOLINT bracket The bracket suppresses modernize-use-using for the header's typedef structs, which clang-tidy parses as C++ under core/tools/vmaf.cpp (through feature_dimensions.h and speed_internal.h) and under the SYCL SpEED translation units; C cannot spell the `using` alias it proposes. The CPU lane of the Tidy Ratchet had counted six findings there (0 -> 6). The comment now names the C and C++ includers and cites ADR-1138, which prescribes this file-scoped shape for C code parsed as C++. A CPU-lane ratchet run scoped to vmaf.cpp measures 0 findings in the header. The source ADR citation registry follows the new citation. * test(cuda): cover the speed_temporal_cuda solve launch at 1920x1080 test_cuda_speed_temporal_parity_1080p builds the temporal parity test at 1920x1080, where the luma plane has 312 SpEED blocks. The host-split twin on master launched its solve kernel with ((blocks + 7) / 8) * 32 threads per block, 1248 > 1024, so every frame at 1080p and above failed with CUDA_ERROR_INVALID_VALUE (speed_temporal_cuda.c:425 at 10f27ef). The 768x432 and 960x540 fixtures stay under 256 blocks and never reached it. On the RTX 4090 the test fails against 10f27ef with that error and passes with the ADR-1380 twin. * docs: record the RTX 4090 verification of the CUDA CAMBI and SpEED twins Measured on ryzen-4090-arc (RTX 4090, icx release build without -march=native; before = origin/master 10f27ef built the same way): - every per-frame cambi, speed_chroma_u/v/uv and speed_temporal equals --backend cpu at --precision max on the Netflix 576x324 pair (48/48) and on BBB 3840x2160 (50/50); - compute-sanitizer memcheck, racecheck and synccheck report no error or hazard on the five CAMBI and SpEED parity tests; - one readback and one stream synchronisation per frame (CUPTI count); - 4K ms/frame before -> after: cambi_cuda 64.71 -> 6.01, speed_chroma_cuda 24.90 -> 6.89, speed_temporal_cuda fails -> 5.88; - speed_log2() correctly rounded on every positive finite float, on the RTX 4090 and on the Arc A380. T-CUDA-CAMBI-HOST-RESIDUAL-2026-09-29, T-CUDA-SPEED-HOST-RESIDUAL-2026-09-29 and T-CUDA-SPEED-TEMPORAL-SOLVE-LAUNCH-1080P-2026-09-30 are closed. The lanczos4 row now carries device numbers (up to 2.1e-2 on a smooth 1080p gradient), and the Arc A380 SpEED row records that its check is blocked by the xe kernel driver, which returns wrong values from SYCL scratch memory, rather than a vmafx bug. ADR-1379, ADR-1380, Research-1379, the CAMBI and SpEED pages, the CUDA backend page, the changelog fragments, the CUDA AGENTS.md and the rebase notes replace the emulation-only statements with these measurements. * fix(ci): regenerate the ADR citation map after rebasing onto #1643 * fix(speed): take the device prescale rule from speed_prescale_resamples() #1643 replaced speed_internal.c's SI_ALMOST_EQUAL with the shared speed_prescale_resamples() helper. The device geometry spelled the same rule out by hand with the removed macro; it now calls the helper, so the CPU and the device twins decide the resample from one function.
This was referenced Sep 30, 2026
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
…s (ADR-1398) The vmaf CLI formerly refused odd-sized raw YUV 4:2:0 and 4:2:2 inputs with "odd width/height %d not allowed..." via validate_chroma_alignment() (ADR-0461), whereas .y4m inputs were accepted because frame_w was padded to a multiple of 16. The C engine and raw readers handle odd dimensions with ceiling chroma (vmaf_chroma_extent(), picture_geometry.h, PR #1643, PR #1664). Per user decision on 2026-10-01 ("Accept both (Recommended)"), the CLI now accepts raw YUV inputs with odd dimensions using ceiling chroma, matching .y4m. - core/tools/vmaf.cpp: validate_chroma_alignment() returns 0. - core/tools/test/test_vmaf_raw_odd_dims.sh: add positive (19x19, 1921x1081, 19x20, 20x19, 19x19 422), boundary (1x1 420/422), and negative (file size mismatch exits 2 cleanly) regression tests. - core/tools/test/test_vmaf_option_dict_ownership.sh: test Case 2 with mismatched dimensions instead of odd height to preserve early-exit dictionary leak coverage. - python/test/vmafx_cli_test.py: add unit tests asserting raw YUV scores match Y4M bit-identically for odd dimensions, 1x1 boundary, and file size error handling. - docs: document odd dimensions support in docs/usage/cli.md and core/tools/AGENTS.md, record ADR-1398, update docs/state.md, docs/rebase-notes.md, changelog fragment, and citations registry. Closes T-CLI-RAW-ODD-DIMENSIONS-REFUSED-2026-10-01.
lusoris
added a commit
that referenced
this pull request
Oct 1, 2026
…s (ADR-1398) (#1672) * fix(tools): accept odd dimensions for raw YUV chroma-subsampled inputs (ADR-1398) The vmaf CLI formerly refused odd-sized raw YUV 4:2:0 and 4:2:2 inputs with "odd width/height %d not allowed..." via validate_chroma_alignment() (ADR-0461), whereas .y4m inputs were accepted because frame_w was padded to a multiple of 16. The C engine and raw readers handle odd dimensions with ceiling chroma (vmaf_chroma_extent(), picture_geometry.h, PR #1643, PR #1664). Per user decision on 2026-10-01 ("Accept both (Recommended)"), the CLI now accepts raw YUV inputs with odd dimensions using ceiling chroma, matching .y4m. - core/tools/vmaf.cpp: validate_chroma_alignment() returns 0. - core/tools/test/test_vmaf_raw_odd_dims.sh: add positive (19x19, 1921x1081, 19x20, 20x19, 19x19 422), boundary (1x1 420/422), and negative (file size mismatch exits 2 cleanly) regression tests. - core/tools/test/test_vmaf_option_dict_ownership.sh: test Case 2 with mismatched dimensions instead of odd height to preserve early-exit dictionary leak coverage. - python/test/vmafx_cli_test.py: add unit tests asserting raw YUV scores match Y4M bit-identically for odd dimensions, 1x1 boundary, and file size error handling. - docs: document odd dimensions support in docs/usage/cli.md and core/tools/AGENTS.md, record ADR-1398, update docs/state.md, docs/rebase-notes.md, changelog fragment, and citations registry. Closes T-CLI-RAW-ODD-DIMENSIONS-REFUSED-2026-10-01. * docs: regenerate the indexes and the citation map after rebasing
9 of 12 tasks
lusoris
added a commit
that referenced
this pull request
Oct 2, 2026
…each with its size and the upstream change that ends it (ADR-1479 to ADR-1486) The reference for code inherited from Netflix/vmaf is Netflix's source; a difference needs an ADR. The upstream parity audit of 2026-10-02 found deliberate differences that had none of their own, or whose ADR (ADR-1033) names neither upstream's behaviour nor the size: - ADR-1479 ciede on 4:2:2: chroma flags (fork PR #1050); 0.153 on 48 of 48 frames; Netflix/vmaf#1611. - ADR-1480 speed_temporal buffers at speed_prescale above 1 (#1643); up to 195, upstream segfaults on two fixtures; Netflix/vmaf#1627. - ADR-1481 a failing extractor fails the run (#871); status only, 78 probe runs where upstream is silent and 88 where it crashes. - ADR-1482 integer adm on frames of 17 to 32 pixels (#1473, #1507); scale 3 up to 0.23; Netflix/vmaf#1599, #1600. - ADR-1483 odd-sized chroma planes round up (4f08d32); psnr_cb / cr up to 0.684 / 0.826 dB, ciede 0.198. - ADR-1484 float_ms_ssim magnitude before pow() (#641, ADR-1033 item 2); NaN upstream on the 10 px checkerboard; Netflix/vmaf#1665. - ADR-1485 apsnr of a plane without error (#641, item 1); 114 against 60 dB; Netflix/vmaf#1666. - ADR-1486 float_motion scale-1 stride (#641, item 9); up to 25.1; Netflix/vmaf#1667. Each ADR gives upstream's file and line at Netflix 9e48141b, the fork's lines, the reason found in the fork's pull request, commit or code, and the measured size from the audit. Documentation only.
lusoris
added a commit
that referenced
this pull request
Oct 7, 2026
… to #1668 Add a "Confirmed not-affected" row for each of the fork's open upstream pull requests from #1631 to #1668 (25 rows), naming the fork file, test or ADR that shows the fork already carries the fix, covers it another way or is not affected. #1643 is the one open item (a test-only x87 comparison with the same line in the fork). #1634 is recorded as closed: the fork keeps integer AIM unclipped, as upstream defines it. Also records that the ten pull requests that conflicted with upstream master acdd9376e were rebased on 2026-10-07. Documentation only.
lusoris
added a commit
that referenced
this pull request
Oct 7, 2026
… to #1668 (#2404) * docs(state): reconcile the fork's open Netflix/vmaf pull requests #1631 to #1668 Add a "Confirmed not-affected" row for each of the fork's open upstream pull requests from #1631 to #1668 (25 rows), naming the fork file, test or ADR that shows the fork already carries the fix, covers it another way or is not affected. #1643 is the one open item (a test-only x87 comparison with the same line in the fork). #1634 is recorded as closed: the fork keeps integer AIM unclipped, as upstream defines it. Also records that the ten pull requests that conflicted with upstream master acdd9376e were rebased on 2026-10-07. Documentation only. * docs: regenerate the indexes and the citation map after rebasing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allocates
speed_temporalframe buffers usingalloc_heightso thatspeed_prescale > 1(upsampling) does not overrun the heap under AddressSanitizer or corrupt memory in release builds (Netflix/vmaf#1626, Netflix/vmaf#1627). In addition,speed_prescale_resamples()resamples whenlround(dim * prescale)alters plane extents even iffabs(prescale - 1.0) < 1e-3.Derives
speed_chromaframe buffer extents on CPU, CUDA, and HIP usingvmaf_chroma_extent()ceiling division instead of integer floor division, preventingpicture_copy()from overflowing buffers on odd dimensions in subsampled formats (4:2:0, 4:2:2). Also boundsspeed_temporal_cudasolve launch block dimensions to 256 threads (8 warps) and scales blocks to avoid CUDA launch errors for large systems.Type
feat— new featurefix— bug fixperf— performance improvementrefactor— no behavior changedocs— documentation onlytest— test-onlybuild/ci— tooling / infraport— cherry-pick from upstream Netflix/vmafsycl/cuda/simd— backend-specificChecklist
make format && make lintis green locally.python3 scripts/ci/run_meson_test.py -- -C build./cross-backend-diffand the worst ULP is ≤ 2..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md).!orBREAKING CHANGE:and the migration path is documented below.docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt— do not editdocs/adr/README.mddirectly (regenerated byscripts/docs/concat-adr-index.sh; see ADR-0221).Bug-status hygiene (ADR-0165)
docs/state.mdupdated in this PR with a row in the appropriate section (Open / Recently closed / Confirmed not-affected / Deferred), ORno state delta: REASON.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Cross-backend numerical results
Scores at
speed_prescale <= 1.0remain bit-identical to master. GPU twins (CUDA on RTX 4090, SYCL on Arc A380) verified at prescale 1.0, 1.5, 2.0 and odd dimensions without overflow or NaN/Inf scores.Deep-dive deliverables (ADR-0108)
AGENTS.mdinvariant note — added tocore/src/feature/AGENTS.md.changelog.d/fixed/speed-temporal-prescale-overflow.mdandchangelog.d/fixed/speed-chroma-odd-size-overflow.md.docs/rebase-notes.md.Reproducer
Known follow-ups
None.