Skip to content

port: reconcile with upstream Netflix/vmaf September 2026 (ADM/VIF SIMD fixes) - #1473

Merged
lusoris merged 10 commits into
masterfrom
port/upstream-2026-09
Sep 20, 2026
Merged

lusoris merged 10 commits into
masterfrom
port/upstream-2026-09

Conversation

@lusoris

@lusoris lusoris commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Reconciles the fork with Netflix/vmaf's eight commits from 2026-09-14/16. Two of their SIMD bugs were live here and are fixed. A third upstream change retires our Apple-only ADM path. Tracing a fourth found a real out-of-bounds read, where upstream had only masked the symptom. Scores for normal frame sizes are unchanged on x86 and aarch64.

  • 03b5562c5, AVX2 ADM decouple overshoot (fixed). The tail bound was a multiple of 8 from zero, not from left, so the last store ran up to 6 int16 past right in 372 of 992 band widths. Taken verbatim.
  • ea012e387, NEON ADM DWT2 out-of-bounds write (fixed). The vector loop had no tail bound and wrote past every band into the next band's first element, from over-read data. NEON scores were wrong and differed between runs at small widths. Now bounded, with a scalar tail.
  • cba9343ed, Darwin three-tap path (retired, ADR-1257). Upstream now uses the four-tap column 0 everywhere, so the Apple-only compatibility wrapper goes. The x86 and aarch64 builds give bit-identical akiyo results (132.7331520020318, every frame metric equal).
  • 1786bd961, ADM scale 3 at 17–32 px (root cause fixed). Upstream zeroes data_buf, which hides the symptom. The cause is the DWT index table: for n_half == 2 it restarted the mirrored tail at 0 and read row/column -1. Fixed at the source; the zeroing is kept as defence in depth.
  • checkasm (not adopted, per decision). Its lesson is: simd_bitexact_test.h gains a guard-band check (nothing written outside the compared region) and the ADM/VIF SIMD tests sweep small sizes. Both bugs above were writes outside the compared region, which the old tests could not see.
  • Undefined rounding constant at widths 17–32 (fixed; found by this PR's sanitizer run). At those widths one scale-0 shift is by zero bits, and the AVX2 and AVX-512 paths computed its rounding term as (uint32_t)pow(2, shift - 1), i.e. infinity converted to an integer. The AVX-512 build turns that into 0xFFFFFFFF, which is the whole cause of the AVX-512 small-width divergence this PR first recorded as open: scale 0 was up to 0.01 off scalar and is now bit-exact. The AVX2 build happened to get the right 0. Upstream has the same expression, in its scalar path too.
  • Not needed: 8f7d50d29 (the fork's bound already covers all six DWT2 kernels), c023bb7cb (fixed in fix(arm64): NEON parity tests for every uncovered kernel, and 7 defects they found #1156), 1801915be (CI for checkasm). 86da14d03 is perf(cambi): vectorize the spatial-mask dp and mask rows on AVX2, AVX-512 and NEON #1472.

Reviewer action needed (golden-file guard): retiring the three-tap path changes the macOS result of three akiyo assertions in python/test/vmafexec_test.py. Each is 88.030322 if _IS_DARWIN else 88.030463, a Darwin branch the fork added in #760. It must become Netflix's own 88.030463. That file is guarded, so a maintainer applies these three lines by hand; the diff is below. The other three _IS_DARWIN values come from a macOS libm difference in svm_predict (ADR-0418) and stay. Until the three lines land, the macOS Python lane is expected to fail on those three tests.

-            88.030322 if _IS_DARWIN else 88.030463,
+            88.030463,

(applied at lines 941, 1051 and 1107)

Found and recorded for separate PRs:

  • Integer adm_cm AVX2/AVX-512 is not bit-exact with scalar on full-range noise (T-ADM-CM-SIMD-NOISE-NOT-BIT-EXACT-2026-09-18). Upstream has it too.
  • The CUDA and HIP twins mis-score frames 17–32 px wide (scale 0 up to 0.2 off, NaN at 32x32), and the CUDA, HIP and SYCL twins accept frames below 17x17 (T-GPU-ADM-TINY-FRAME-SHIFT-2026-09-18). The fix is written and verified on CUDA and SYCL hardware; it goes in a stacked PR because it touches GPU files outside this port.

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.
  • Unit tests pass: meson test -C build.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2: the fixes remove out-of-region stores and an out-of-bounds read; the akiyo run is bit-identical between x86 and aarch64, and the fast suite passes on both.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below: scalar, AVX2, AVX-512 and NEON give identical scores at every width 17–32; the GPU twins are listed below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md).
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below: not breaking.
  • 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.

Bug-status hygiene (ADR-0165)

  • docs/state.md: four rows closed (T-UPSTREAM-03B5562C5-ADM-DECOUPLE-AVX2-OVERSHOOT-2026-09-18, T-UPSTREAM-EA012E387-ADM-DWT2-NEON-OOB-WRITE-2026-09-18, T-ADM-SCALE3-TINY-FRAME-OOB-READ-2026-09-18, T-ADM-AVX512-SMALL-WIDTH-SCALE0-2026-09-18) and two opened (T-ADM-CM-SIMD-NOISE-NOT-BIT-EXACT-2026-09-18, T-GPU-ADM-TINY-FRAME-SHIFT-2026-09-18).

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: the three Darwin branches above, which restore Netflix's own value.

Deep-dive deliverables (ADR-0108)

  • Research digestdocs/research/2063-upstream-sync-2026-09-adm-vif-simd.md: per-commit triage, reproducers and measurements.
  • Decision matrixdocs/adr/1257-retire-darwin-adm-dwt2-legacy-dispatch.md, ## Alternatives considered.
  • AGENTS.md invariant notecore/src/feature/AGENTS.md, core/src/feature/x86/AGENTS.md, core/src/feature/arm64/AGENTS.md, core/test/AGENTS.md.
  • Reproducer / smoke-test command — below.
  • User docsdocs/metrics/features.md, new "Small frames" section under ADM: the 17x17 minimum, what changed at 17–32 px, and the GPU caveat.
  • CHANGELOG fragmentchangelog.d/fixed/upstream-2026-09-adm-simd-out-of-region-stores.md, changelog.d/fixed/adm-scale3-tiny-frame-oob-read.md, changelog.d/fixed/adm-avx512-tiny-width-rounding.md, changelog.d/changed/adm-darwin-neon-dwt2-four-tap.md.
  • Rebase notedocs/rebase-notes.md: what each upstream commit became here, and what the next sync must keep.

Reproducer

meson setup build core -Denable_cuda=false -Denable_sycl=false -Db_lto=false && ninja -C build
meson test -C build test_integer_adm_simd test_integer_adm_tiny_frames test_adm_dwt2_x86 \
  test_integer_vif_avx2_stages test_integer_vif_avx512_stages
# aarch64 (NEON) via cross build + qemu:
meson setup build-a64 core --cross-file <aarch64 cross file> -Denable_dnn=disabled -Db_lto=false
ninja -C build-a64 && meson test -C build-a64 test_adm_dwt2_neon test_vif_neon test_integer_adm_tiny_frames

Verified locally

Check Result
CPU build + fast suite (x86) 138 passed, 0 failed, no compiler warnings
aarch64 cross build + fast suite under qemu 136 passed, 0 failed
ADM/VIF/NEON tests under qemu 17 passed
akiyo x86 vs aarch64 (--precision max) bit-identical
clang-tidy on every touched C file (x86 and aarch64 compile commands) 0 findings
pre-commit, every hook on the changed files passes
Fast suite under ASan + UBSan (clang, the CI lane's flags) 138 passed; the old AVX2 expression trips UBSan at the same line CI reported
SIMD vs scalar, widths 17–32 at h 70 (test_integer_adm_tiny_widths_simd_matches_scalar) bit-identical on AVX-512 and on NEON under qemu; fails at 17x70 with the old AVX-512 expression
Scalar and AVX2 scores before vs after, 18x18 to 640x360 identical
mkdocs build --strict passes

Known follow-ups

  • adm_cm noise bit-exactness (T-ADM-CM-SIMD-NOISE-NOT-BIT-EXACT-2026-09-18).
  • The GPU tiny-frame fix (T-GPU-ADM-TINY-FRAME-SHIFT-2026-09-18), stacked on this branch.
  • Reporting the (uint32_t)pow(2, shift - 1) conversion to Netflix/vmaf.
  • The macOS Python lane confirms the Darwin change once the three assertion lines are applied; no macOS host was available.

Addendum 2026-09-19 — this PR now carries a maintainer-authorized golden change

Commit ed77a843b edits three assertAlmostEqual lines in python/test/vmafexec_test.py (941, 1051, 1107): 88.030322 if _IS_DARWIN else 88.030463 becomes 88.030463. Authorized explicitly by the maintainer (popup answer, 2026-09-19); it is the only golden edit in the queue.

It lives here because it is only true with this port. With the port macOS produces 88.030459 — inside places=4 of the Linux value, outside it for the old Darwin one — so the special case was what failed this PR's four macOS lanes. Without the port macOS still yields 88.030317, which is why the standalone attempt (#1502) could not pass and was closed in favour of this commit. The other three per-platform values in that file still measure as recorded and are untouched.

Also in this push: rebased onto master (e3570efd7), core/test/meson.build resolved as a union and validated by a full build plus meson test --suite=fast (140 of 140), the stale duplicate of the #1109 state row dropped in favour of master's re-checked copy, and the line-keyed standards baseline re-recorded.

Lusoris added 10 commits September 20, 2026 09:31
…MD fixes)

Takes the September upstream fixes the fork was exposed to and records
the ones it was not:

- 03b5562c5: adm_decouple_avx2's tail bound was a multiple of 8 from
  zero instead of from `left`, so the last vector store ran up to six
  int16 past `right` in 372 of 992 band widths. Taken verbatim.
- ea012e387: adm_dwt2_8_neon's vector loop had no tail bound and wrote
  one int16 past every band, into the next band's [0][0], from
  over-read data, which made NEON scores wrong and non-deterministic at
  small widths. Bounded with a scalar tail, as upstream does.
- cba9343ed: upstream now uses the four-tap column 0 everywhere, so the
  Apple-only three-tap wrapper is retired (ADR-1257). The three akiyo
  Darwin assertions in python/test/vmafexec_test.py are left for the
  maintainer to apply by hand, as the golden-file guard requires.
- 1786bd961: its adm_buffer_alloc zeroing is ported, and the read it
  masked is fixed at the source: for n_half == 2 (frame dimensions
  17..32) the DWT index tables restarted the mirrored tail at 0 and read
  row / column -1, which made integer_adm_scale3 vary between identical
  runs.

Instead of adopting checkasm, the parity tests gain its lesson: a
guard-band helper in simd_bitexact_test.h checks that nothing outside
the compared region is written, and the ADM and VIF SIMD tests sweep
small sizes. 8f7d50d29 (the fork's bound already covers every DWT2
kernel), c023bb7cb and the CI workflow need nothing; 86da14d03 is PR
…aths

At frame widths 17 to 32 the scale-0 horizontal and vertical cube shift is
0. The AVX2 and AVX-512 paths computed its rounding constant as
(uint32_t)pow(2, shift - 1), which converts infinity to an integer. The
ASan+UBSan lane caught it in test_integer_adm_tiny_frames.

The AVX-512 build converts with vcvttsd2usi and got 0xFFFFFFFF. That was the
whole cause of T-ADM-AVX512-SMALL-WIDTH-SCALE0-2026-09-18: scale 0 was up to
0.01 off scalar at those widths and is now bit-exact. The AVX2 build got 0,
the scalar value, by accident.

All 16 constants call adm_half_shift(), which moves from integer_adm.c into
adm_csf_fixed_point.h next to a shared adm_frame_size_check(). Scalar and
AVX2 scores are unchanged, as is every frame wider than 32 pixels. A new
sweep compares the SIMD dispatch with scalar at widths 17 to 32. It fails at
17x70 with the old AVX-512 expression and passes on NEON under QEMU.

The same investigation found the CUDA and HIP twins mis-scoring these widths
and the GPU twins accepting frames below 17x17. That is recorded as
T-GPU-ADM-TINY-FRAME-SHIFT-2026-09-18 and fixed in a separate PR.
…e assertions

Maintainer-authorized golden change (popup answer, 2026-09-19), folded
into the upstream reconciliation because it is only true together with
it: with this port macOS produces 88.030459, within places=4 of the
Linux value 88.030463 and not of the old Darwin value 88.030322, so the
special case was what failed the four macOS lanes on this PR. Without
the port macOS still yields 88.030317, which is why the change could not
merge on its own (#1502, closed in favour of this commit).

Exactly three assertion lines change. The other three per-platform
values in the file still measure as recorded and are untouched.
… internal-register AGENTS files

The rebase onto master resolved core/src/feature/arm64/AGENTS.md and
core/test/AGENTS.md as a union. Since #1482 rewrote every AGENTS file in
the internal register, a union there keeps master's text AND the
branch's stale prose copy of the same bullets: 22 and 49 resurrected
lines, found with a checker that compares what a restacked branch adds
against what it added before.

Following the documented procedure for this conflict class, both files
are master's versions again with only this branch's facts ported in:
the ADR-1257 bullet replaces the ADR-1057 Apple-wrapper bullet, and the
adm_dwt2_8_neon() loop-bound bullet and the SIMD guard-band testing
bullet are added. All fourteen code spans of the original bullets are
present in the result.
@lusoris
lusoris force-pushed the port/upstream-2026-09 branch from 575b416 to c1a4512 Compare September 20, 2026 07:42
@lusoris
lusoris merged commit bcf0f99 into master Sep 20, 2026
82 checks passed
@lusoris
lusoris deleted the port/upstream-2026-09 branch September 20, 2026 08:23
lusoris added a commit that referenced this pull request Sep 22, 2026
…ny frames, HIP buffer by pointer (#1507)

Integration train for the five-deep ADM/GPU stack behind #1473, folded into one
merge per maintainer direction. Each PR previously targeted the one below it, so
none could merge until every one below had.

- #1474 AVX2 / AVX-512 contrast masking wraps like scalar on full-range noise
- #1477 CPU 16-bit vertical DWT sum formed in int64 — int32 overflows at 16 bpc
  once three samples reach 42456
- #1476 Integer-ADM twins agree with the CPU on tiny frames; SYCL wrap and
  rounding fixes; GPU tidy-lane repairs
- #1478 The same 16-bit scale-0 vertical DWT fix in the CUDA, HIP and Metal twins
- #1481 HIP ADM kernels take `AdmBufferHip` by pointer (ADR-0759) rather than
  copying 328 bytes of arguments per launch

Required Checks Aggregator green on 9bae48c with no failing check; branch level
with master at 371ff58. Merged with admin bypass because the repository has a
single collaborator who cannot self-approve (ADR-1252).

Unblocks #1518, which depends on the SIMD contrast-masking fix — see
T-CUDA-ADM-SMALL-BORDER-PARITY-2026-09-21 in docs/state.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:port Port from upstream Netflix/vmaf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant