Repository navigation
Conversation
lusoris
force-pushed
the
fix/hip-integer-ssim-int64-kernel
branch
from
September 19, 2026 08:47
b79998a to
a75a8cd
Compare
lusoris
added a commit
that referenced
this pull request
Sep 19, 2026
HIP extractors scored some frames against the next frame's samples. HIP pictures are pageable host memory, eleven extractors copied them to the device with a bare hipMemcpy2DAsync and returned from submit(), and the caller refilled the picture while the copy was still reading it. The CLI's pool is LIFO, so the distorted picture of frame N is the first buffer refilled for frame N + 1. Every plane taken from VmafPicture::data now goes through one helper, vmaf_hip_picture_upload() in core/src/hip/picture_hip.c. It enqueues the copies and waits on an event recorded after them, so a null-stream caller does not wait for every other stream on the device, and it still waits when a copy fails part-way. integer_ssim_hip, which #1480 fixed with a local stream synchronise, uses the helper too. float_adm_hip, float_vif_hip, float_motion_hip and psnr_hip upload on their private stream instead of the null stream their kernels use. Affected through vmaf_read_pictures(): ciede_hip, float_adm_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, float_vif_hip, psnr_hip and vif_hip. float_motion_hip, motion_hip and motion_v2_hip upload the reference only, which libvmaf keeps alive for one more frame through prev_ref, so they could not show the defect there; through the extractor API their copy was still reading after submit() on 10 of 10 runs. Measured on a gfx1036 over the 48-frame Netflix pair, ten runs per extractor against the scalar CPU. Before, at 576x324: float_psnr_hip 8 distinct outputs and 10.7 dB off, psnr_hip 10 and 7.3 dB, vif_hip 10 and 0.30, float_ssim_hip 10 and 0.215, float_moment_hip 8, ciede_hip 8, float_vif_hip 3, float_adm_hip 2. With eleven extractors in one process the output was identical on every run and wrong on 46 of 48 frames, so a determinism check alone passed the defect. After: every extractor gives one output in ten runs, alone, together and at 1920x1080, and stays inside its parity tolerance (worst 3.4e-5). The wait costs host time. At 1080p, median of five alternating runs: vmaf_float_v0.6.1 18.8 to 14.8 fps (-21 %) and vif_hip -7 %; vmaf_v0.6.1, eleven extractors together and the other single extractors are within noise. On this single-queue iGPU a copy cannot start until the previous extractor's kernels leave the GPU. Moving those kernels off the null stream did not help. Pinned staging buffers would, and are tracked as T-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19. The touched-file rule applies to all twelve extractors, so they are also brought to zero clang-tidy and zero HISS findings: init() and close() share one release function in place of the goto ladders, the oversized launch, init and collect functions are split, kernel argument arrays cast each entry, and core/src/hip/hip_handle.h converts the uintptr_t handles through a union in one place. The release functions drain the stream before freeing a buffer; float_psnr_hip, float_moment_hip and float_ssim_hip used to free first. The 16 uncited NOLINTs in integer_vif_hip.c suppressed a check that never fired there and are gone. The refactor changes no score. Over the 48 frames the four extractors the parent already got right, and float_adm_hip and float_vif_hip, are bit-identical to the parent; float_psnr_hip, float_moment_hip and psnr_hip equal the CPU exactly; ciede_hip, float_ssim_hip and vif_hip are bit-identical on every frame the parent's race missed and keep the same worst delta against the CPU on the same frame.
lusoris
added a commit
that referenced
this pull request
Sep 19, 2026
…frames test_hip_upload_race fails on the parent commit on every run and passes with the upload wait. It runs the twelve HIP extractors that upload from VmafPicture::data through two checks over one 576x324 multi-frame fixture. test_pooled_parity is the path a user takes: eight frames from a picture pool sized like the CLI's through vmaf_read_pictures(), each frame within the extractor's own parity tolerance of its CPU twin. On the parent it fails 10 of 10 runs with 6 to 8 extractors off: ciede_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, psnr_hip and vif_hip every time, float_adm_hip 8 times, float_vif_hip 5 times. test_recycle_after_submit states the invariant directly. Through the extractor API both pictures are overwritten with the next frame the moment submit() returns, and every score must be bit-identical to a run that leaves them alone. vmaf_read_pictures() keeps the reference picture alive for one more frame through prev_ref, so the pooled check cannot see an extractor that uploads the reference only. This one can: on the parent it catches motion_hip, motion_v2_hip and float_motion_hip on 10 of 10 runs. The frame generator and the pool loop move out of test_hip_ssim_parity.c into hip_pooled_fixture.h, which both tests include. Its luma is unchanged, so #1480's SSIM gate sees the same frames; chroma now moves with the frame as well, so ciede and the chroma PSNR planes are sensitive to a stale plane. The test skips without a HIP device or device kernels.
lusoris
force-pushed
the
fix/hip-integer-ssim-int64-kernel
branch
from
September 19, 2026 10:22
a75a8cd to
dd76203
Compare
lusoris
changed the title
lusoris
added a commit
that referenced
this pull request
Sep 19, 2026
HIP extractors scored some frames against the next frame's samples. HIP pictures are pageable host memory, eleven extractors copied them to the device with a bare hipMemcpy2DAsync and returned from submit(), and the caller refilled the picture while the copy was still reading it. The CLI's pool is LIFO, so the distorted picture of frame N is the first buffer refilled for frame N + 1. Every plane taken from VmafPicture::data now goes through one helper, vmaf_hip_picture_upload() in core/src/hip/picture_hip.c. It enqueues the copies and waits on an event recorded after them, so a null-stream caller does not wait for every other stream on the device, and it still waits when a copy fails part-way. integer_ssim_hip, which #1480 fixed with a local stream synchronise, uses the helper too. float_adm_hip, float_vif_hip, float_motion_hip and psnr_hip upload on their private stream instead of the null stream their kernels use. Affected through vmaf_read_pictures(): ciede_hip, float_adm_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, float_vif_hip, psnr_hip and vif_hip. float_motion_hip, motion_hip and motion_v2_hip upload the reference only, which libvmaf keeps alive for one more frame through prev_ref, so they could not show the defect there; through the extractor API their copy was still reading after submit() on 10 of 10 runs. Measured on a gfx1036 over the 48-frame Netflix pair, ten runs per extractor against the scalar CPU. Before, at 576x324: float_psnr_hip 8 distinct outputs and 10.7 dB off, psnr_hip 10 and 7.3 dB, vif_hip 10 and 0.30, float_ssim_hip 10 and 0.215, float_moment_hip 8, ciede_hip 8, float_vif_hip 3, float_adm_hip 2. With eleven extractors in one process the output was identical on every run and wrong on 46 of 48 frames, so a determinism check alone passed the defect. After: every extractor gives one output in ten runs, alone, together and at 1920x1080, and stays inside its parity tolerance (worst 3.4e-5). The wait costs host time. At 1080p, median of five alternating runs: vmaf_float_v0.6.1 18.8 to 14.8 fps (-21 %) and vif_hip -7 %; vmaf_v0.6.1, eleven extractors together and the other single extractors are within noise. On this single-queue iGPU a copy cannot start until the previous extractor's kernels leave the GPU. Moving those kernels off the null stream did not help. Pinned staging buffers would, and are tracked as T-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19. The touched-file rule applies to all twelve extractors, so they are also brought to zero clang-tidy and zero HISS findings: init() and close() share one release function in place of the goto ladders, the oversized launch, init and collect functions are split, kernel argument arrays cast each entry, and core/src/hip/hip_handle.h converts the uintptr_t handles through a union in one place. The release functions drain the stream before freeing a buffer; float_psnr_hip, float_moment_hip and float_ssim_hip used to free first. The 16 uncited NOLINTs in integer_vif_hip.c suppressed a check that never fired there and are gone. The refactor changes no score. Over the 48 frames the four extractors the parent already got right, and float_adm_hip and float_vif_hip, are bit-identical to the parent; float_psnr_hip, float_moment_hip and psnr_hip equal the CPU exactly; ciede_hip, float_ssim_hip and vif_hip are bit-identical on every frame the parent's race missed and keep the same worst delta against the CPU on the same frame.
lusoris
added a commit
that referenced
this pull request
Sep 19, 2026
…frames test_hip_upload_race fails on the parent commit on every run and passes with the upload wait. It runs the twelve HIP extractors that upload from VmafPicture::data through two checks over one 576x324 multi-frame fixture. test_pooled_parity is the path a user takes: eight frames from a picture pool sized like the CLI's through vmaf_read_pictures(), each frame within the extractor's own parity tolerance of its CPU twin. On the parent it fails 10 of 10 runs with 6 to 8 extractors off: ciede_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, psnr_hip and vif_hip every time, float_adm_hip 8 times, float_vif_hip 5 times. test_recycle_after_submit states the invariant directly. Through the extractor API both pictures are overwritten with the next frame the moment submit() returns, and every score must be bit-identical to a run that leaves them alone. vmaf_read_pictures() keeps the reference picture alive for one more frame through prev_ref, so the pooled check cannot see an extractor that uploads the reference only. This one can: on the parent it catches motion_hip, motion_v2_hip and float_motion_hip on 10 of 10 runs. The frame generator and the pool loop move out of test_hip_ssim_parity.c into hip_pooled_fixture.h, which both tests include. Its luma is unchanged, so #1480's SSIM gate sees the same frames; chroma now moves with the frame as well, so ciede and the chroma PSNR planes are sensitive to a stale plane. The test skips without a HIP device or device kernels.
lusoris
force-pushed
the
fix/hip-integer-ssim-int64-kernel
branch
from
September 19, 2026 11:44
dd76203 to
97f628f
Compare
14 of 26 tasks
integer_ssim_score.hip was an 11-tap float Gaussian while the CPU `ssim` extractor (integer_ssim.c) runs a 9-tap int64 kernel, so the HIP twin scored 4.53e-3 away from the CPU on its parity fixture. ADR-1154 kept it unflagged for that reason, which meant model-driven `ssim` under --backend hip ran on the CPU. Port the kernel and the host arithmetic from the CUDA twin (cuda/integer_ssim/integer_ssim_score.cu + ssim_cuda.c): - pass 1 writes six W x H int64 moment planes (mux, muy, x2, xy, y2, w) with the CPU's boundary truncation, not clamping or mirroring; - pass 2 runs the vertical taps and evaluates the per-pixel term exactly as ssim_reduce_row_range() does, including the (0.01 * 0.01) / (0.03 * 0.03) constants, and is built with -ffp-contract=off so that nothing fuses into an FMA; - the per-block sums use a shared-memory tree over the whole 16x8 block instead of warpSize shuffles, so wave32 and wave64 add in the same order; - the host returns sum(term) / sum(weight), as calc_ssim() does. Porting it exposed a staging race, and this commit fixes it for this extractor. The host pictures are pageable memory, and the CLI's picture pool refills a slot as soon as submit() returns. A hipMemcpy2DAsync from that memory could still be reading, so a different set of frames was scored against the next frame's samples on every multi-frame run (up to 0.2 off on the 48-frame Netflix pair). submit() now waits for its two uploads. The other HIP twins that upload the same way are left as they are. Measured on a gfx1036 against the scalar CPU (--cpumask 0xFFFFFFFF), every frame of 19 inputs: 8/10/12/16 bpc, 1x1 up to 1920x1080, odd sizes and windows smaller than the kernel. The worst delta is 1.06e-11, on a 1080p checkerboard whose score is -0.53. The 48-frame Netflix 576x324 pair is at 2.3e-14, and 1x1 is exact. The CUDA twin on an RTX 4090 has the same worst case, 1.06e-11. Also: - The extractor now carries VMAF_FEATURE_EXTRACTOR_HIP, and the HIP dispatch table lists `integer_ssim_hip` and `ssim`. - It accepts any frame size, like the CPU. It used to reject frames smaller than 11x11. - test_hip_ssim_parity is no longer should_fail. - clang-tidy, HIP lane: integer_ssim_hip.c goes from 27 findings to 0 (baseline 39 -> 0, scoped write) and dispatch_strategy.c from 2 to 0 (ADR-1138 bracket). Measured with the kernel TU added to the compile database, the kernel goes from 11 to 0.
…d fixtures test_hip_ssim_parity fed one 8-bit frame. That fixture misses the upload race fixed in the previous commit: the race needs a picture buffer that is refilled with the next frame while the copy still reads it. The 16-bit horizontal kernel and the partial last block also had no coverage. The test now: - feeds 8 frames from a picture pool the size of the CLI's (3 slots, no worker threads) and compares every frame. The content moves and the distortion grows per frame, so a frame scored against another frame's samples shows up as a delta; - is registered at 8 bpc, at 10 bpc (-DFIXTURE_BPC=10u, the 16-bit kernel), at an odd 577x323 size, and at 960x540 through the large-fixture list, which explicitly left it out while the kernel was the float one; - checks at places=4 (1e-4), the gate of the CUDA, SYCL and Metal integer_ssim twins. It was places=3; - checks that `ssim` resolves to integer_ssim_hip under the HIP flag and that the flag is set. This needs no device; - marks the -ENOSYS scaffold path as skipped, as the no-device path already was. It used to report a pass. On gfx1036 the deltas are 4.8e-14 (256x144 8-bit), 6.4e-14 (10-bit), 1.3e-13 (577x323) and 5.6e-13 (960x540). With the upload wait removed, the 256x144 and 960x540 variants failed 10 of 10 runs, with frames off by up to 7.3e-2. clang-tidy, HIP lane: the file goes from 18 findings to 0. It had 17 modernize-use-nullptr, now under the ADR-1138 bracket, and one function-size finding.
…port Recorded from a clean clone with the pinned engine. The three entries it adds are brace blocks the engine counts as functions: an anonymous namespace and an extern "C" block in integer_ssim_score.hip, and the g_hip_features array initializer in dispatch_strategy.c.
…upload race User and maintainer docs for the two previous commits. - docs/backends/hip/overview.md: 18 of 19 extractors are now active; integer_adm_hip is the one still unflagged. The integer_ssim_hip row, source listing and kernel note describe the int64 kernel; the kernel note used to say it emitted `integer_ssim`, which it never did. A new section covers the kernel, its measured precision and how it gets selected. Models under --backend hip use it. `--feature ssim` names the CPU extractor, so the CLI needs `--feature integer_ssim_hip`. A model that sets enable_db or clip_db computes `ssim` on the CPU. A known-issue note covers the upload race in the other HIP twins. - docs/metrics/ssim.md: the HIP row gives the measured precision instead of a target. The page also claimed that `--feature ssim` auto-selects the backend's twin. It does not: vmaf_use_feature() resolves an extractor name. Checked on this machine: `--feature ssim --backend hip` and `--feature ssim --backend cuda` both return the CPU's scores bit for bit. Only model-driven dispatch selects the twin. - docs/metrics/features.md: the `ssim` backends line said only a HIP twin existed and that the SIMD windows stay scalar. It now lists the AVX2 / NEON paths and all four GPU twins. - core/src/feature/hip/AGENTS.md: the ".flags = 0 until the int64 kernel lands" note becomes the invariants the port has to keep: the same kernel and truncation, the same per-pixel expression with -ffp-contract=off, a reduction independent of the wavefront size, and the upload wait. - docs/state.md: T-GAP-HIP-INTEGER-SSIM-FLOAT-KERNEL-DEFERRED-2026-09-02 moves to Recently closed with the measurements. It opens T-HIP-PAGEABLE-UPLOAD-RACE-2026-09-18 for the other twins: float_ssim_hip and psnr_hip are reproduced (frames off by up to 8.4e-2 and 10.4 dB over the 48-frame Netflix pair), and nine more use the same upload pattern. - docs/rebase-notes.md, changelog.d/fixed/ and the rendered CHANGELOG.md. ADR-0564 and ADR-1154 are Accepted and are not edited. ADR-1154's "17 of 19 active" and its deferral of integer_ssim_hip are now out of date. Its deferral was conditional on the int64 kernel landing, so this closes it rather than reversing it.
HIP extractors scored some frames against the next frame's samples. HIP pictures are pageable host memory, eleven extractors copied them to the device with a bare hipMemcpy2DAsync and returned from submit(), and the caller refilled the picture while the copy was still reading it. The CLI's pool is LIFO, so the distorted picture of frame N is the first buffer refilled for frame N + 1. Every plane taken from VmafPicture::data now goes through one helper, vmaf_hip_picture_upload() in core/src/hip/picture_hip.c. It enqueues the copies and waits on an event recorded after them, so a null-stream caller does not wait for every other stream on the device, and it still waits when a copy fails part-way. integer_ssim_hip, which #1480 fixed with a local stream synchronise, uses the helper too. float_adm_hip, float_vif_hip, float_motion_hip and psnr_hip upload on their private stream instead of the null stream their kernels use. Affected through vmaf_read_pictures(): ciede_hip, float_adm_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, float_vif_hip, psnr_hip and vif_hip. float_motion_hip, motion_hip and motion_v2_hip upload the reference only, which libvmaf keeps alive for one more frame through prev_ref, so they could not show the defect there; through the extractor API their copy was still reading after submit() on 10 of 10 runs. Measured on a gfx1036 over the 48-frame Netflix pair, ten runs per extractor against the scalar CPU. Before, at 576x324: float_psnr_hip 8 distinct outputs and 10.7 dB off, psnr_hip 10 and 7.3 dB, vif_hip 10 and 0.30, float_ssim_hip 10 and 0.215, float_moment_hip 8, ciede_hip 8, float_vif_hip 3, float_adm_hip 2. With eleven extractors in one process the output was identical on every run and wrong on 46 of 48 frames, so a determinism check alone passed the defect. After: every extractor gives one output in ten runs, alone, together and at 1920x1080, and stays inside its parity tolerance (worst 3.4e-5). The wait costs host time. At 1080p, median of five alternating runs: vmaf_float_v0.6.1 18.8 to 14.8 fps (-21 %) and vif_hip -7 %; vmaf_v0.6.1, eleven extractors together and the other single extractors are within noise. On this single-queue iGPU a copy cannot start until the previous extractor's kernels leave the GPU. Moving those kernels off the null stream did not help. Pinned staging buffers would, and are tracked as T-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19. The touched-file rule applies to all twelve extractors, so they are also brought to zero clang-tidy and zero HISS findings: init() and close() share one release function in place of the goto ladders, the oversized launch, init and collect functions are split, kernel argument arrays cast each entry, and core/src/hip/hip_handle.h converts the uintptr_t handles through a union in one place. The release functions drain the stream before freeing a buffer; float_psnr_hip, float_moment_hip and float_ssim_hip used to free first. The 16 uncited NOLINTs in integer_vif_hip.c suppressed a check that never fired there and are gone. The refactor changes no score. Over the 48 frames the four extractors the parent already got right, and float_adm_hip and float_vif_hip, are bit-identical to the parent; float_psnr_hip, float_moment_hip and psnr_hip equal the CPU exactly; ciede_hip, float_ssim_hip and vif_hip are bit-identical on every frame the parent's race missed and keep the same worst delta against the CPU on the same frame.
…frames test_hip_upload_race fails on the parent commit on every run and passes with the upload wait. It runs the twelve HIP extractors that upload from VmafPicture::data through two checks over one 576x324 multi-frame fixture. test_pooled_parity is the path a user takes: eight frames from a picture pool sized like the CLI's through vmaf_read_pictures(), each frame within the extractor's own parity tolerance of its CPU twin. On the parent it fails 10 of 10 runs with 6 to 8 extractors off: ciede_hip, float_moment_hip, float_psnr_hip, float_ssim_hip, psnr_hip and vif_hip every time, float_adm_hip 8 times, float_vif_hip 5 times. test_recycle_after_submit states the invariant directly. Through the extractor API both pictures are overwritten with the next frame the moment submit() returns, and every score must be bit-identical to a run that leaves them alone. vmaf_read_pictures() keeps the reference picture alive for one more frame through prev_ref, so the pooled check cannot see an extractor that uploads the reference only. This one can: on the parent it catches motion_hip, motion_v2_hip and float_motion_hip on 10 of 10 runs. The frame generator and the pool loop move out of test_hip_ssim_parity.c into hip_pooled_fixture.h, which both tests include. Its luma is unchanged, so #1480's SSIM gate sees the same frames; chroma now moves with the frame as well, so ciede and the chroma PSNR planes are sensitive to a stale plane. The test skips without a HIP device or device kernels.
…tors The upload-race fix touched twelve HIP extractors and picture_hip.c, and the touched-file rule brought each of them to zero findings. This records it with the guarded scoped write, `tidy-ratchet.py --lane hip --write --only`, over those thirteen translation units and the two HIP tests. The hip lane goes from 1134 to 845 clang-tidy findings and from 16 to 0 uncited NOLINTs (all 16 were in integer_vif_hip.c). Largest drops: float_adm_hip.c 60 to 0, float_ssim_hip.c 39 to 0, float_vif_hip.c and integer_vif_hip.c 29 to 0. Measured with clang-tidy 22.1.8, the version the baseline already records.
Closes T-HIP-PAGEABLE-UPLOAD-RACE-2026-09-18 in docs/state.md with the measured before and after numbers, and opens T-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19 for what the fix costs: 21 % of vmaf_float_v0.6.1's throughput at 1080p on a gfx1036, noise for vmaf_v0.6.1. The state row records which extractors were live (the eight that upload the distorted picture), which were latent (the three that upload the reference only, which libvmaf keeps alive through prev_ref) and which were never affected, so the next audit does not have to redo it. core/src/feature/hip/AGENTS.md gains the invariant: an extractor must not return from submit() while an upload from a pooled host picture is in flight. It names the helper, the stream to pass, why the reference picture is not exempt, and the test a new extractor joins. docs/backends/hip/overview.md replaces the known-issue note with what changed for a user, tells them to recompute multi-frame HIP scores taken before the fix, and adds a "Picture uploads" section with the throughput numbers and a two-run check. It also says why identical output is not proof: with several extractors in one process the old defect was deterministic. Adds the changelog fragment, the rendered CHANGELOG.md and the rebase note.
…ll AMD GPU Opens T-HIP-TEST-SUITE-OVERSUBSCRIPTION-2026-09-19. Meson starts about 40 HIP test processes at once and a gfx1036 has two SDMA queues, so amdgpu reports queue exhaustion on every suite run and a HIP test occasionally hangs until its 60 s timeout. One of five suite runs on this branch lost test_hip_ssim_parity_odd that way. It is not the upload wait: under 16 concurrent HIP loops the same test hung on the parent commit 4 times in 100 and on this branch 2 in 100, interleaved, and neither side produced a parity failure. The row also records one parity failure of that test (2.65e-2) seen once, right after meson had killed the hung process. It did not recur in 290 further runs, under load or after killing HIP processes mid-run, nor on the parent, so it is recorded as unexplained rather than explained away, with the one structural lead: 577 is the only test width whose picture stride differs from the packed row.
The agent-facing files moved to the internal register on master (#1482) while this stack was open. The two sections it adds to core/src/feature/hip/AGENTS.md are rewritten to match; the caveman floor check confirms no code span, identifier, number or rule was lost.
…d fix Recorded from a clean clone with the pinned engine. The entries it removes are all in the eleven HIP extractors the upload fix cleaned; it adds none.
vmaf_hip_picture_upload() waits for the copies it enqueued, which is only sufficient if every accepted copy was counted. Two assertions state that: the count never exceeds the plane count, and success means all planes were enqueued. They also satisfy the Power-of-10 assertion density gate, which rejects a fork-added function of 20 lines or more without one.
The keep-both rebase resolution duplicated the HIP upload race row: the branch moves it from Open bugs to Recently closed, so the union kept both copies.
lusoris
force-pushed
the
fix/hip-integer-ssim-int64-kernel
branch
from
September 19, 2026 12:28
97f628f to
8c01f42
Compare
13 of 14 tasks
Contributor
Author
|
Folded into #1497 together with the other two, and closing here. The three kept invalidating each other: each one touches Nothing was dropped. Every source path of this branch is byte-identical in #1497, verified with |
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
integer_ssim_hipnow computes what the CPU computes: the 9-tap int64 kernel, ported from the CUDA twin. Before, it ran an 11-tap float Gaussian, 4.5e-3 off the CPU, so it was deferred to a CPU fallback (.flags = 0, ADR-0564) andtest_hip_ssim_paritywas registeredshould_fail. Both are gone: the extractor is flaggedVMAF_FEATURE_EXTRACTOR_HIP, is in the HIP dispatch table, and passes its parity test at the 1e-4 gate the other twins use (the test's old 1e-3 is tightened).Parity against the scalar CPU, every frame of 19 inputs (8/10/12/16 bpc, sizes from 1x1 to 1920x1080, odd sizes, frames smaller than the window):
The remaining difference is only the summation order of per-pixel terms.
Bug found on the way, and fixed here: HIP uploads raced the picture pool. HIP pictures are pageable host memory, and eleven extractors copied them to the device with a bare
hipMemcpy2DAsyncand returned fromsubmit()without waiting. The CLI's picture pool is LIFO, so frame N's distorted buffer is the first one refilled for frame N+1, and frames were scored against the next frame's samples.float_psnr_hippsnr_hipvif_hipfloat_ssim_hipfloat_moment_hipciede_hipfloat_vif_hipfloat_adm_hipfloat_motion_hip,motion_hip,motion_v2_hipupload the reference only, whichvmaf_read_pictures()happens to keep alive one more frame; through the extractor API their copy was still in flight on 10 of 10 runs).vmaf_hip_picture_upload()incore/src/hip/picture_hip.c, enqueues the copies and waits on an event recorded after them, also when a copy fails part-way. All twelve uploading extractors use it on their private stream, includinginteger_ssim_hip.test_hip_upload_race: twelve extractors, pooled frames against the CPU, plus both pictures overwritten the momentsubmit()returns with bit-identical scores required. It fails 10 of 10 runs on the old code and passes 100 of 100 at 576x324 and at 577x323.Cost, stated plainly: the wait costs throughput on this single-queue iGPU.
vmaf_float_v0.6.1at 1080p goes from 18.8 to 14.8 fps, 21 % slower;vif_hip7 %;vmaf_v0.6.1and the other extractors are within noise. Pinned staging buffers owned by the extractor are the remedy, opened asT-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19. Wrong scores are not an acceptable price for the throughput.The old 11x11 minimum frame size is removed; the CPU extractor has none.
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.integer_ssim_hip.cgoes from 27 clang-tidy findings to 0 (baseline 39 → 0),test_hip_ssim_parity.cfrom 18 to 0,core/src/hip/dispatch_strategy.cfrom 2 to 0, andinteger_ssim_score.hipfrom 11 to 0. Its baseline entry is left at 11 because the kernel-TU generator only exists on fix(gpu): make the integer ADM twins agree with the CPU on tiny frames and full-range content #1476's branch. 0 uncited NOLINTs.meson test -C build. The HIP fast suite has no failures, and all fourtest_hip_ssim_parityvariants pass (8-bit, 10-bit, odd 577x323, 960x540). The CPU-only build passes 137/137.scripts/dev/preflight.shpasses every stage./cross-backend-diffand the worst ULP is ≤ 2: see the parity table. HIP equals CUDA against the CPU..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md): no new files.!orBREAKING CHANGE:and the migration path is documented below: not breaking. Models that request integer SSIM on HIP now run on the GPU instead of falling back to the CPU, with the same scores.docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt: no ADR. The port completes ADR-0564's conditional deferral rather than reversing a decision.Bug-status hygiene (ADR-0165)
docs/state.md:T-GAP-HIP-INTEGER-SSIM-FLOAT-KERNEL-DEFERRED-2026-09-02closed;T-HIP-PAGEABLE-UPLOAD-RACE-2026-09-18opened.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Deep-dive deliverables (ADR-0108)
core/src/feature/hip/AGENTS.md.changelog.d/fixed/hip-integer-ssim-int64-kernel.md.docs/rebase-notes.md.Reproducer
meson setup build/hip core -Denable_hip=true -Denable_hipcc=true -Denable_cuda=false \ -Denable_sycl=false -Db_lto=false --buildtype=release ninja -C build/hip meson test -C build/hip --num-processes 2 test_hip_ssim_parity test_hip_ssim_parity_10bit \ test_hip_ssim_parity_odd test_hip_ssim_parity_large test_hip_upload_raceAlso in this PR
.standards-baseline.jsonis re-recorded with the pinned engine from a clean clone, as the rebase notes prescribe. Debt fell from 1,623 to 1,501 infractions: 114 entries go with the eleven cleaned extractors. The line-keyed baseline also reported three moved HISS-04 findings as new. Two of them are the scanner reading an anonymous namespace and anextern "C"block as functions, worth fixing in praetor.Known follow-ups
T-HIP-UPLOAD-WAIT-THROUGHPUT-2026-09-19: pinned staging buffers to win back the throughput the upload wait costs.T-HIP-TEST-SUITE-OVERSUBSCRIPTION-2026-09-19: meson starts about 40 HIP processes at once and this iGPU has two SDMA queues; one of five parallel suite runs lost a test to its 60 s timeout. It predates this PR (the parent hangs 4 of 100 under load).test_hip_adm_parityis still registeredshould_failfor a staging gap that has since been fixed, so it reports an unexpected pass;test_hip_adm_small_borderandtest_hip_adm_wide_roundingfail withvmaf_feature_score_at_index failed, which is not the fault their marker cites. Separate fix.No GPU
integer_ssimtwin declares the CPU'senable_db/clip_dboptions; a model that sets them falls back to the CPU for that feature, as the HIP page now says.--feature ssimwith--backend hip(orcuda) runs the CPU extractor, yet the output JSON reportsbackend_used: "hip". The docs are corrected; the misleading field is a separate fix.The hip clang-tidy baseline is tightened from 1,134 to 845 findings and from 16 to 0 uncited NOLINTs, written with
--write --onlyfor the touched files.The two sections this PR adds to
core/src/feature/hip/AGENTS.mdare in the internal register that docs(agents): write the agent-facing docs in the internal register #1482 moved the agent files to; the caveman floor check confirms no fact was lost.