Repository navigation
Conversation
lusoris
force-pushed
the
fix/speed-temporal-prescale-overflow
branch
from
September 30, 2026 14:56
e551d18 to
e1b59fc
Compare
lusoris
marked this pull request as ready for review
September 30, 2026 14:56
14 of 26 tasks
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 30, 2026
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
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 30, 2026
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
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 30, 2026
…tly (#1643) * fix(speed): size speed_temporal and speed_chroma frame buffers correctly 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
2 times, most recently
from
October 1, 2026 18:11
dbfa44a to
ee6783c
Compare
lusoris
force-pushed
the
fix/speed-temporal-prescale-overflow
branch
from
October 2, 2026 05:22
ee6783c to
1c0a5de
Compare
speed_temporal allocated its four frame buffers as float_stride * h (the source height), but filter_and_downscale() copies alloc_height rows out of them and writes the prescaled frame back at scaled_height rows. Any speed_prescale above 1 overflowed the heap. speed_chroma already sizes the same buffers with alloc_height. For speed_prescale <= 1, alloc_height == h, so default output does not change. Fixes Netflix#1626. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Extracts three 576x324 frames per prescale and checks the last frame's score is finite. Before the alloc_height fix the 1.5 and 2.0 cases read and wrote past the frame buffers: ASan reports a heap-buffer-overflow in filter_and_downscale() and a plain build segfaults, so the test catches a regression without a sanitizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lusoris
force-pushed
the
fix/speed-temporal-prescale-overflow
branch
from
October 2, 2026 18:38
1c0a5de to
11deee9
Compare
9 of 12 tasks
lusoris
added a commit
to VMAFx/vmafx
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.
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
1 participant
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.
Fixes #1626.
speed_temporalallocated its four frame buffers asfloat_stride * h, the source height.filter_and_downscale(), however, copiesalloc_heightrows out of them and writes the prescaled frame back atscaled_heightrows. So anyspeed_prescaleabove 1 overflowed the heap. This change sizes the buffers withalloc_height, asspeed_chromaalready does (speed.c:1352,1356).For
speed_prescale <= 1,alloc_height == h, so the allocation and the default output are unchanged.The second commit adds
test_speed_temporal_prescaletotest_feature_extractor. It runs three 576x324 frames atspeed_prescale1.0, 1.5 and 2.0 and checks that the last score is finite.Evidence
gcc 16.2.1, release builds and
-Db_sanitize=address,undefinedbuilds, this branch on master9e48141b; the master column was measured oncea2b4d8, and the code involved (speed.c) is unchanged in9e48141b. Both builds carry the new test.Rebased on master 9e48141 (2026-10-02).
Regression test (
test/test_feature_extractor):-Db_sanitize=address,undefinedfilter_and_downscale(speed.c:967)The test therefore catches the bug in the existing CI, which runs without a sanitizer.
CLI under ASan, 576x324 test pair,
--feature speed_temporal=speed_prescale=N:speed_prescaleDefault output unchanged: with
--feature speed_temporal --feature speed_chromaat default options, the per-frame JSON metrics of master and this branch are identical on all 48 frames of the 576x324 pair.Neither commit adds a compiler warning; on
9e48141bthe unpatched and the patched build report the same warnings (23 in the release build, 10 in the sanitizer build), all in files this PR does not touch (float_motion.c,model.c,output.c,predict.c,svm.cpp,x86_64.h).The upstream CI for this PR still needs a maintainer to approve the workflow run; the results above are from a local build.