Repository navigation
Conversation
lusoris
force-pushed
the
fix/cuda-adm-cm-row-rounding
branch
2 times, most recently
from
October 2, 2026 18:45
bfae424 to
4cb62aa
Compare
4KVCD
added a commit
to 4KVCD/libvmaf-fast
that referenced
this pull request
Oct 7, 2026
… modules and destroy the streams of the motion, vif and adm extractors By lusoris (Netflix#1645), at 3d95f6c, the commit of it that was tested. libvmaf's CUDA extractors loaded their kernel modules and created two streams in every init and never released them: with the CUDA context kept alive, as in VideoMetricsLab where a decoder or another metric holds it, every scorer leaked them. Conflict in integer_adm_cuda.c: the contrast masking kernels' names are this fork's (from Netflix#1648), taken from the PR's s->adm_cm_module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… sums once per row The CPU (adm_cm(), i4_adm_cm()) sums a whole row of cubed contrast masking values and applies (accum_inner + add_shift_inner_accum) >> shift_inner_accum once. The CUDA kernels apply that shift to the partial sum of every warp before the atomic add: the fused scale 0 kernel to each 32x8 tile, adm_cm_reduce_line_kernel to each warp of 128 columns. Rounding is not additive, so the accumulators drift from the CPU's, by O(1) per extra warp. In the near-zero accumulators of smooth content that is a large relative error: integer_adm_scale0 of a 38x38 structured frame is off by 0.31. adm_cm_line_kernel stores its masked values per pixel in the accumulator buffer, as i4_adm_cm_line_kernel already does, and adm_cm_reduce_line_kernel (one block per band and row, reduced in shared memory) cubes them and applies the shift once per row. The scale 0 path launches it after the fused kernel, so the reduction constants of the scale 0 bands, which the kernel already carried, are used. The scale 0 rounding constant of the horizontal and vertical bands was computed on the host as 1 << (shift - 1) with a shift of 0 for frame widths 17 to 32. That is 2^31 on x86. It is now computed in the reduction kernel, with no rounding term for a shift of 0. Add test_cuda_adm_cm_row_rounding, which compares integer_adm_scale0 of adm_cuda with the scalar CPU adm on random and structured frames of 38x38 to 200x120 at 8 and 10 bits.
lusoris
force-pushed
the
fix/cuda-adm-cm-row-rounding
branch
from
October 8, 2026 17:16
4cb62aa to
bdb926b
Compare
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
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.
integer_adm_scale0ofadm_cudadiffers from the CPU by up to 0.41 on frames of 17 to 64 pixels. The rounding of the contrast masking sum explains about 90% of the scale 0 error summed over the sweep below. This is bug 2 of #1564 for scale 0; the same change covers the scale 1 to 3 reduction, where no frame of the sweep shows an effect.Cause
The CPU (
adm_cm(),i4_adm_cm()) sums a whole row of cubed contrast masking values inaccum_innerand applies(accum_inner + add_shift_inner_accum) >> shift_inner_accumonce per row. The CUDA kernels apply that shift to every partial sum before the atomic add:adm_cm_line_kernel(scale 0, fused): to the warp-reduced sum of a tile of 32 columns by 8 rows;adm_cm_reduce_line_kernel(scales 1 to 3): to the warp-reduced sum of 128 columns.Rounding is not additive, so the accumulators drift from the CPU's by O(1) per extra warp. In the large accumulators of busy frames that is below the precision of the final
floatscore. In the near-zero accumulators of smooth content it is a large relative error: a row sum the CPU rounds down to 0 becomes 1 in a tile that holds eight rows.A second defect sits in the scale 0 setup.
adm_cm_device()computed the rounding constant of the horizontal and vertical cubes on the host as1 << (shift - 1). The shift isceil(log2(w) - 4), which is 0 for a scale 0 band of 9 to 16 samples (frame widths 17 to 32), so the constant is 2^31 on x86 and the CPU's is 0.Reproducer
Master
8e7a1ac4e,-Denable_cuda=true, RTX 4090. The CLI prints six digits, so the numbers below are from a C API program that prints%.17g;test_cuda_adm_cm_row_roundingmakes the same comparison. A structured 38x38 8-bit frame (a ramp with a 3x3 checkerboard, the lower right quadrant mirrored; the generator is the test's),integer_adm_scale0:integer_adm_scale0adm_cuda, masteradm_cuda, this branch212 configurations (17x17 to 64x64 and fifteen other sizes up to 320x180, 8 and 10 bits, random and structured content, two frames),
integer_adm_scale0ofadm_cudaagainst the scalar CPU, largest absolute difference over the configurations of the size group:Rows (size, bit depth) with a scale 0 difference: 88 of 106 on master, 30 on this branch. Sum over the rows of the largest difference: 16.85 on master, 1.68 on this branch. What remains on the branch is not this cause: the 17 to 28 pixel rows and 17x64 / 64x17 are the border clamp of the scale 0 contrast masking (next change in the series), and 55x55 and 320x180 at 8 bits are the angle test of the decouple stage (a separate change).
Fix
adm_cm_line_kernelstores its masked values per sample intmp_accum, asi4_adm_cm_line_kernelalready does.adm_cm_reduce_line_kernelis launched for both paths with one block per band and row (it already carried the constants of the scale 0 bands): the block cubes the values, reduces them in shared memory and thread 0 applies the shift once. The rounding constants of the reduction areshift ? 1u << (shift - 1) : 0. TheWarpShifthost structure and its host-side constants are removed.Tests
test_cuda_adm_cm_row_rounding(new,libvmaf/test/, built withenable_cuda; it reports a skip when no CUDA device can be opened) comparesinteger_adm_scale0ofadm_cudawith the scalar CPUadmon 38x38, 48x48, 64x64, 130x40 and 200x120 at 8 and 10 bits, random and structured content, with a tolerance of 1e-9. Unpatched master fails it (38x38, 8 bit, random: 9.7e-6, structured: 0.31); this branch passes it. All sizes have both dimensions of 33 or more, where the scalar CPU on master equals the CPU with the fixes of #1599 and #1600.Validation
Master
8e7a1ac4e, 2026-10-01. x86-64 Linux, GCC 16.2.1, nvcc 13.4, RTX 4090.Rebased on master
9e48141b(2026-10-02). On that base, with-Denable_cuda=true -Denable_float=true -Denable_checkasm=true,meson testgives 28 of 29 (the failure istest_cuda_pic_preallocation, which also fails on unpatched9e48141b, 27 of 28 there), and the CPU-Db_sanitize=address,undefined -Db_lto=falsebuild gives 22 of 25 on master and here (test_predictandtest_pic_preallocationabort under LeakSanitizer andcheckasmaborts on a heap-buffer-overflow inadm_dwt2_16(integer_adm.c:2603); all three also abort on unpatched9e48141b). The CLI output of the three Netflix pairs (--gpumask 0,vmaf_v0.6.1) against unpatched9e48141b: identical on all three pairs (src01 76.668905, checkerboard 1 px 35.068667, 10 px 7.985899). The sweeps and the other measurements below were taken on8e7a1ac4eand not repeated; the files and x86 code paths they depend on are unchanged since (the one upstream change in between is an arm64-only ADM kernel).adm_cudaat%.17g(integer_adm2, scales 0 to 3, numerators and denominators):src01576x324 8 bit (48 frames) and 10 bit (3 frames), checkerboard 1 px and 10 px 1920x1080 (3 frames each), KristenAndSara 1280x720,akiyo352x288,sparks480x270 10 bit (5 frames): identical to both. The 160x90 clip of the Netflix test resources (48 frames) is identical to master; its remaining difference to the CPU is the border of scales 1 to 3 (previous change). On the 18x22akiyoclip the largest scale 0 difference to the CPU goes from 7.2e-2 to 2.4e-2. No golden assertion changes.bbb(2 frames): master and the merge of the five branches both equal the CPU on every output. The scale 1 to 3 reduction therefore has no measured effect on any frame I ran; it is changed because it is the same kernel and the CPU's contract is one rounding per row.libvmaf/test/meson.buildconflicts textually (every branch adds its test at the same place);adm_cm.cuandinteger_adm_cuda.cmerge cleanly. cuda: keep kernel parameter structs and accumulators out of local memory #1595 and cuda: build the kernels with clang without the CUDA toolkit headers #1596 change the sameWarpShiftlines ofadm_cm.cu, which this change removes: whichever lands second needs a rebase.meson teston the merge of the five branches: 28 of 29 pass.test_cuda_pic_preallocationaborts with SIGSEGV intest_cuda_picture_preallocation_method_host_pinned; it does the same on unpatched master.The workflow run on this PR needs a maintainer's approval.