Skip to content

fix(sycl): score 4:0:0 input in psnr_hvs_sycl and psnr_hvs_hip - #1692

Merged
lusoris merged 1 commit into
masterfrom
fix/psnr-hvs-sycl-hip-yuv400
Oct 1, 2026
Merged

lusoris merged 1 commit into
masterfrom
fix/psnr-hvs-sycl-hip-yuv400

Conversation

@lusoris

@lusoris lusoris commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

psnr_hvs_sycl and psnr_hvs_hip refused 4:0:0 pictures at init() (YUV400P unsupported). The CPU extractor and psnr_hvs_cuda score the luma plane of such input and emit psnr_hvs_y and psnr_hvs; both twins now do the same. The HIP twin always dispatched three planes and had no enable_chroma option, so it gains the CPU extractor's option (default true) and a plane count. Closes T-SYCL-HIP-PSNR-HVS-YUV400-REFUSED-2026-10-01, found while writing #1689.

Stacked on #1689 (fix/psnr-hvs-sycl-hip-exact-sum, open when this was written): the PR base is that branch, because the change extends its shared test header core/test/psnr_hvs_twin_parity.h and closes a row it opens. Retarget to master once #1689 has merged.

Type

  • feat — new feature
  • fix — bug fix
  • perf — performance improvement
  • refactor — no behavior change
  • docs — documentation only
  • test — test-only
  • build / ci — tooling / infra
  • port — cherry-pick from upstream Netflix/vmaf
  • sycl / cuda / simd — backend-specific

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally. The pre-commit hook set passed on the whole diff. The clang-tidy ratchet was measured on the touched translation units in their lanes (--only): integer_psnr_hvs_sycl.cpp and test_sycl_psnr_hvs_parity.c (SYCL), integer_psnr_hvs_hip.c and test_hip_psnr_hvs_parity.c (HIP), test_cuda_psnr_hvs_parity.c (CUDA), with psnr_hvs_twin_parity.h through them: 0 findings in the touched files, no baseline change. The whole-tree lint was not run.
  • Unit tests pass: python3 scripts/ci/run_meson_test.py -- -C build --suite=fast on a CPU build and on a CPU + CUDA build (RTX 4090); on the SYCL build (Arc A380) test_sycl_psnr_hvs_parity, _simd32, _large, test_sycl_shared_planes, test_sycl_twin_option_parity, test_sycl_kernel_scratch; on the HIP build (gfx1036) test_hip_psnr_hvs_parity, _large, test_hip_twin_option_parity. On a HIP build without device kernels (-Denable_hipcc=false) test_hip_psnr_hvs_parity skips (exit 77).
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2. The worst difference is 0: the parity tests compare every output bit for bit.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md). No new source file.
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt. No ADR: the behaviour is the CPU extractor's and the CUDA twin's.

Bug-status hygiene (ADR-0165)

  • docs/state.md updated in this PR: T-SYCL-HIP-PSNR-HVS-YUV400-REFUSED-2026-10-01 moved to Recently closed.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
  • If I believe a golden value must change, I have explained why below AND pinged @lusoris for a CODEOWNERS exception.

No CPU extractor code changes.

Cross-backend numerical results

test_{cuda,sycl,hip}_psnr_hvs_parity compare the twin with the CPU extractor of the same binary, bit for bit, on psnr_hvs_y, psnr_hvs_cb, psnr_hvs_cr and psnr_hvs. New cases, all identical on an Arc A380 (compiler's SIMD16 and forced SIMD32), a gfx1036 and an RTX 4090:

case                                   before (SYCL, HIP)                 after (CUDA, SYCL, HIP)
4:0:0 8-bit 64x48                      frame refused (-EINVAL)            identical: psnr_hvs_y, psnr_hvs
enable_chroma=false, 4:2:0 8-bit       SYCL: not compared; HIP: no option  identical: psnr_hvs_y, psnr_hvs
enable_chroma=false, 4:2:2 10-bit      SYCL: not compared; HIP: no option  identical: psnr_hvs_y, psnr_hvs

The existing cases (8 to 12 bits, 4:2:0 / 4:2:2 / 4:4:4, 3840x2160) stay identical.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: the twins adopt the plane rule of third_party/xiph/psnr_hvs.c::init and of psnr_hvs_cuda; nothing was investigated beyond reading them. The finding is recorded in Research-1401 §6.
  • Decision matrix — no alternatives: only-one-way fix (score 4:0:0 as the CPU extractor does).
  • AGENTS.md invariant note — core/src/feature/hip/AGENTS.md and core/src/feature/sycl/AGENTS.md (the plane count and what follows it).
  • Reproducer / smoke-test command — pasted below under "Reproducer".
  • CHANGELOG fragment — changelog.d/fixed/sycl-hip-psnr-hvs-yuv400.md.
  • Rebase note — docs/rebase-notes.md, "psnr_hvs_sycl and psnr_hvs_hip score 4:0:0; psnr_hvs_hip takes enable_chroma".

Reproducer

ninja -C build test/test_sycl_psnr_hvs_parity && ./build/test/test_sycl_psnr_hvs_parity
ninja -C build-hip test/test_hip_psnr_hvs_parity && ./build-hip/test/test_hip_psnr_hvs_parity

On the previous twins both fail in test_psnr_hvs_every_layout_identical with read: the 4:0:0 frame is refused.

Known follow-ups

  • The Metal twin is unchanged and outside these tests.
  • The vmaf CLI rejects --pixel_format 400 for raw input, so 4:0:0 reaches the twins through the library API (and through y4m input, which was not tried here).

Breaking changes / migration

None. One new extractor option: psnr_hvs_hip takes enable_chroma (default true), documented in docs/metrics/psnr-hvs.md.

@lusoris
lusoris force-pushed the fix/psnr-hvs-sycl-hip-exact-sum branch from af99885 to 05a2421 Compare October 1, 2026 12:53
@lusoris
lusoris force-pushed the fix/psnr-hvs-sycl-hip-exact-sum branch from 05a2421 to d329d18 Compare October 1, 2026 13:28
@github-actions github-actions Bot added the type:bug Something isn't working label Oct 1, 2026
@lusoris
lusoris force-pushed the fix/psnr-hvs-sycl-hip-exact-sum branch 3 times, most recently from fa073b5 to 33df23d Compare October 1, 2026 14:39
Base automatically changed from fix/psnr-hvs-sycl-hip-exact-sum to master October 1, 2026 14:40
@lusoris
lusoris force-pushed the fix/psnr-hvs-sycl-hip-yuv400 branch from ac813f1 to f0e255e Compare October 1, 2026 15:02
psnr_hvs_sycl and psnr_hvs_hip refused 4:0:0 pictures at init()
("YUV400P unsupported"). The CPU extractor and psnr_hvs_cuda score the
luma plane of such input and emit psnr_hvs_y and psnr_hvs. Both twins now
do the same.

- SYCL: validate_hvs_input() no longer rejects the format;
  configure_hvs_geometry() sets one active plane for 4:0:0 or
  enable_chroma=false, as third_party/xiph/psnr_hvs.c::init does.
- HIP: the twin always dispatched three planes and had no enable_chroma
  option. It gains the CPU extractor's option (default true) and a plane
  count that its allocation, staging, uploads, kernel arguments and
  scores follow.
- core/test/psnr_hvs_twin_parity.h compares 4:0:0 for every twin and adds
  an enable_chroma=false case; the CUDA, SYCL and HIP tests run it.

Measured on an Arc A380 (also at forced SIMD32), a gfx1036 and an RTX
4090: psnr_hvs_y and psnr_hvs are bit-identical to the CPU for 4:0:0 and
for enable_chroma=false. On the previous twins the SYCL and HIP tests
fail at the 4:0:0 case (the frame is refused). CPU scores are unchanged.

Closes T-SYCL-HIP-PSNR-HVS-YUV400-REFUSED-2026-10-01.
@lusoris
lusoris force-pushed the fix/psnr-hvs-sycl-hip-yuv400 branch from f0e255e to cc255b9 Compare October 1, 2026 15:23
@lusoris
lusoris merged commit 344b561 into master Oct 1, 2026
63 of 73 checks passed
@lusoris
lusoris deleted the fix/psnr-hvs-sycl-hip-yuv400 branch October 1, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant