Repository navigation
feat(motion): compute motion_five_frame_window on the CUDA, SYCL and HIP twins (ADR-1491) - #1893
Merged
Merged
Conversation
lusoris
force-pushed
the
port/upstream-motion-five-frame-window
branch
from
October 2, 2026 22:21
d8e4985 to
31009e9
Compare
lusoris
force-pushed
the
port/motion-five-frame-window-gpu-twins
branch
from
October 2, 2026 22:29
3dce37b to
de52f4b
Compare
15 of 26 tasks
lusoris
force-pushed
the
port/upstream-motion-five-frame-window
branch
2 times, most recently
from
October 3, 2026 00:10
2564d32 to
6d857a4
Compare
Base automatically changed from
port/upstream-motion-five-frame-window
to
master
October 3, 2026 00:10
lusoris
force-pushed
the
port/motion-five-frame-window-gpu-twins
branch
from
October 3, 2026 00:27
de52f4b to
381a1e3
Compare
…e the function-size limit (#1898) * test(ssim): split the 8x8 float_ssim extractor test so it stays inside the function-size limit test_float_ssim_extractor_8x8_is_zero() (fork #1882) trips readability-function-size: every mu_assert() is a branch. Since #1882 the Tidy Ratchet lanes count one finding above their baselines on master (hip 505 against 504, arm64 116 against 115, and the required cpu lane). The context setup moves into open_float_ssim_8x8() and one frame of the run into extract_8x8_frame(); the test keeps the loop and the teardown. The return values the test dropped (vmaf_picture_unref(), the context's close and destroy) are now checked. The test asserts the same values and passes; no baseline is edited. With the split, scripts/dev/tidy-lane.sh measures cpu 70, cuda 620, hip 504 and arm64 115 warnings, each its baseline.
…HIP twins (ADR-1491) (#1893) * feat(motion): compute motion_five_frame_window on the CUDA, SYCL and HIP twins (ADR-1491) The CPU extractors motion and motion_v2 have Netflix's five-frame window since ADR-1478; their GPU twins handed the option to the CPU. The CUDA, SYCL and HIP twins of both extractors compute it now, and every output equals the CPU's bit for bit on an RTX 4090, an Arc A380 and a gfx1036. No kernel changes: the SAD kernels take the two planes they difference as arguments, and with the option one of them is the frame two back. - CUDA and motion_v2_sycl keep a ring of three raw planes instead of two. The CUDA twins wait on the previous frame's event from frame 1 on, so the chain of events reaches the copy of the frame two back. - HIP keeps two planes instead of one; frame n reads plane n % 2 and the copy behind the SAD overwrites it. - motion_sycl gives its two planes fixed roles (frame n-2, frame n-1) and advances them with two device copies behind the graph replay. The kernel is enqueued on every frame: the combined command graph is recorded once and replayed, so a frame's work cannot depend on its index. - motion2 and motion3 come from the CPU's own vmaf_motion_window_flush(). With the option the motion twins store the SAD score per frame and run the window at the flush; their three-frame path is unchanged. The motion_v2 twins use the function for both windows and lose their copies of the CPU flush. - The six twins carry the CPU's option row. motion_sycl refuses the option together with its own motion_add_uv, which the CPU has no mode for. - The Metal twins are unchanged: they do not declare the option, so the CPU extractor computes it there. Not run, no device. The parity gate has two exact cells for it, motion_mffw and motion_v2_mffw: 0 on all three backends on the Netflix pair and on 200 frames of BBB 3840x2160. test_{cuda,sycl,hip}_motion_five_frame_window compare every output of both extractors with == for six option sets, 11, 1, 2 and 3 frames, at 8 and 10 bits; a twin that takes the SAD against the previous frame fails it on 112 outputs. * docs: regenerate the indexes and the citation map after rebasing
lusoris
force-pushed
the
port/motion-five-frame-window-gpu-twins
branch
from
October 3, 2026 00:42
381a1e3 to
94ef3fd
Compare
This was referenced Oct 3, 2026
lusoris
added a commit
that referenced
this pull request
Oct 3, 2026
…g-tidy baseline (#1915) * refactor(motion): keep motion_window.h's typedef inside the SYCL clang-tidy baseline #1893 made integer_motion_sycl.cpp and integer_motion_v2_sycl.cpp include motion_window.h. clang-tidy then reports modernize-use-using on its `typedef struct VmafMotionWindow` when it reads the header from those C++ units, which puts the sycl lane at 155 warnings against a baseline of 154. The header is also included by C translation units, where `using` does not exist. It therefore takes the same cited NOLINT block that feature_collector.h uses (ADR-1138), rather than a rewrite. No behaviour change. `scripts/dev/tidy-lane.sh sycl` in the dev container reports 154 warnings, matching the baseline (exit 0); on master it exits 2 with motion_window.h at 0 -> 1.
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.
Summary
The CUDA, SYCL and HIP twins of
motionandmotion_v2now compute Netflix'smotion_five_frame_window, and every output equals the CPU's bit for bit. Follows #1887 (the CPU port, ADR-1478, on master as6d857a402), which has these twins hand the option to the CPU extractor. A GPU run of avmaf_v1.0.16_hfr_*model keeps its motion feature on the device.ADR-1491 records the design. Maintainer decision (popup, 2026-10-02): "Port now, CPU and twins (Recommended)".
Base:
master(restacked onto6d857a402after #1887 landed; one commit).What changed
No kernel changes on any backend: the SAD kernels already take the two planes they difference as arguments.
motion2/motion3motion_cudaraw[n % 3]written,raw[(n + 1) % 3]read); the previous frame's event is waited for from frame 1 on, so the chain reaches the copy of framen-2collect(), the CPU'svmaf_motion_window_flush()inflush(); three-frame path unchangedmotion_v2_cudavmaf_motion_window_flush()for both windows; its copy of the CPU flush is gonemotion_hipnreads planen % 2, the copy behind the SAD overwrites itmotion_cudamotion_v2_hipmotion_v2_cudamotion_sycln-2,n-1), advanced by two device copies behind the graph replay; the kernel runs on every frame, because the combined command graph is recorded once and replayedmotion_cudamotion_v2_syclmotion_v2_cudamotion_metal,motion_v2_metalVMAF_OPT_FLAG_DEFAULT_ONLY, no-ENOTSUP).motion_syclrefuses the option together with its ownmotion_add_uv(-ENOTSUP): the CPUmotionhas no chroma mode to equal.motiontwins publishmotion2andmotion3at the end of the run, as the CPU does, not frame by frame.motion_mffwandmotion_v2_mffw(the option with the moving average, the option set of the HFR models), declared exact for CUDA, SYCL and HIP inscripts/ci/exact_twins.d/.core/test/motion_five_frame_twin_parity.h(fixture, six option sets, comparison) andtest_{cuda,sycl,hip}_motion_five_frame_window; option-table rows in the threetest_*_twin_option_parity;test_gpu_option_value_capability_contract.pynow holds the six twins to full-range options;test_motion_five_frame_windowchecks the dispatch verdict of every registered twin. The device-free source contractstest_{cuda,sycl,hip}_kernel_source_contract.pypinned the text of themotion_v2twins' own flush (no re-weighting, the one-frame case); they now pin that each twin callsvmaf_motion_window_flush()and reads no stored score back, with planted regressions for both.Verification
==on every output unless stated otherwise.test_<backend>_motion_five_frame_window: both extractors, six option sets (alone; moving average + debug; every motion option;motion_force_zero;motion_v2alone and with every option), 11 / 1 / 2 / 3 frames, 8 and 10 bitsVMAF_SYCL_USE_GRAPH=1andVMAF_SYCL_NO_GRAPH=1; and the add_uv refusal--precision max: Netflix 576x324 (8 and 10 bit), 1080p checkerboard, BBB 3840x2160 50 frames; three--featureoption sets andvmaf_v1.0.16_hfr_3d0hmotion_mffw,motion_v2_mffw(exact) on the Netflix pair (48 frames) and BBB 4K (200 frames)motion,motion_debug,motion_v2(exact), same clips*_exact_twins,*_motion3_parity,*_motion_sad_score,*_motion_v2_parity,*_motion_tiny_frames,*_twin_option_parity(+test_hip_upload_race,test_sycl_motion_add_uv_parity,test_sycl_kernel_scratch)n-1instead ofn-2,test_cuda_motion_five_frame_windowfails on 112 outputs.sycl-aotsuite (every SYCL translation unit for the 19 default targets): see "SYCL ahead-of-time" below.scripts/ci/test_cross_backend_parity_gate.py: 91 passed;core/test/test_parity_gate_metric_names.py,test_gpu_option_value_capability_contract.py: pass.381a1e3ba, restacked onto master6d857a402), every device test alone under its device lock: CUDA + HIP build (GCC, nvcc, hipcc) 290 device-free and 140 device tests pass; SYCL build (icx) 272 device-free and 67 device tests pass, 1 skipped (test_cuda_parity_gate_default_run, a build without CUDA). The gate cellsmotion,motion_debug,motion_v2,motion_mffwandmotion_v2_mffwwere measured again on this head: 0 on all three backends, both clips.SYCL ahead-of-time:
VMAF_SYCL_AOT_JOBS=8 meson test -C build-sycl --suite sycl-aot:test_sycl_aot_default_targetsOK on this head (35 SYCL translation units for the 19 default targets, 150 s); the restack brought master's SYCL changes (integer_adm_sycl.cpp,integer_psnr_hvs_sycl.cpp,sycl_exact_fp.h). No kernel is added or changed: the scratch-memory ratchet (test_sycl_kernel_scratch, pass) and the sub-group sizes are those of master.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. (clang-format, black, ruff, markdownlint through the commit hooks;praetorctl auditon the touched files: clean after splittingenqueue_motion_work().)python3 scripts/ci/run_meson_test.py -- -C build. (Device tests listed under "Verification".)/cross-backend-diffand the worst ULP is ≤ 2. (Parity gate on all three backends, exact cells: 0.).c/.cpp/.cu/.h/.hpp, it has the appropriate license header. (Three tests and one test header,EUPL-1.2.)!orBREAKING CHANGE:and the migration path is documented below. (Not breaking: twins gain an option the CPU has.)docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt.Bug-status hygiene (ADR-0165)
docs/state.mdupdated in this PR:T-GPU-MOTION-FIVE-FRAME-WINDOW-2026-10-02moved from "Open bugs" to "Recently closed" with the measurements.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests. (No Python test and no CPU extractor changes here.)Cross-backend numerical results
Deep-dive deliverables (ADR-0108)
## Alternatives considered.AGENTS.mdinvariant note —core/src/feature/cuda/AGENTS.d/motion-five-frame-window.md,core/src/feature/hip/AGENTS.d/motion-five-frame-window.md,core/src/feature/sycl/AGENTS.d/motion-five-frame-window.md, and the pages that described the old behaviour; index entry indocs/development/rebase-sensitive-invariants.md.changelog.d/changed/gpu-motion-five-frame-window.md.docs/rebase-notes.md, "The motion twins computemotion_five_frame_window(ADR-1491, 2026-10-02)".Reproducer
# On master without this PR the twin is not used: the receipt reads {"extractor": "motion", "backend": "cpu"}. core/build/tools/vmaf --backend cuda \ --reference python/test/resource/yuv/src01_hrc00_576x324.yuv \ --distorted python/test/resource/yuv/src01_hrc01_576x324.yuv \ --width 576 --height 324 --pixel_format 420 --bitdepth 8 \ --model version=vmaf_v1.0.16_hfr_3d0h --precision max --json --output /dev/stdout python3 scripts/ci/cross_backend_parity_gate.py --vmaf-binary core/build/tools/vmaf \ --reference testdata/ref_576x324_48f.yuv --distorted testdata/dis_576x324_48f.yuv \ --width 576 --height 324 --backends cpu cuda --features motion_mffw motion_v2_mffwKnown follow-ups
motion_syclcopies one more luma plane per frame (two device copies instead of one). No performance claim here; measuring it belongs to RC8 (candidate map of ADR-1490).