fix(cambi): copy 10-bit planes row by row so full_ref with a wider source scores the right rows by lusoris · Pull Request #2111 · VMAFx/vmafx

lusoris · 2026-10-05T11:46:29Z

Summary

CPU cambi with full_ref=true, 10-bit input and a source (src_width / src_height) larger than the picture scored a distorted plane whose rows were shifted. decimate_same_size_16b() copied the plane with one memcpy of stride * height samples at the input's stride into a working picture that full_ref allocates MAX(src, enc) wide, so every row after the first landed at the wrong offset. On the 10-bit Sparks pair, frame 0 cambi was 0.0048362395598900215 with src_width=960:src_height=540 and 0.3734397949735747 without.

The 10-bit branch now copies one row at a time with both strides. cambi is the distorted picture's encode-size score (cambi.c::extract, docs/metrics/cambi.md), so it no longer depends on full_ref or the source size. Every other configuration keeps its bits. This closes T-CAMBI-10BIT-FULLREF-WIDE-SOURCE-ROWS-2026-10-05, found by the RC4 cambi lane (#2090).

Upstream: Netflix/vmaf master (0497a0f29, libvmaf/src/feature/cambi.c:751) has the same single copy, so it has the same defect. Reported upstream as Netflix/vmaf#1670; the state row and the rebase note link it.
Twins: cambi_cuda, cambi_sycl and cambi_hip never had the defect. They declare no full_ref and convert on the device from the picture's own pitch, and a full_ref request with --backend cuda|sycl|hip runs the CPU extractor (the CLI prints cambi_<backend> cannot honour option 'full_ref'; computing it on the CPU). cambi_metal calls the fixed vmaf_cambi_preprocessing() on the host, so this change fixes it too. That is by inspection only; the Metal device check is carried with the other Metal rows.
Lint: cambi.c now spells the null pointer NULL under the ADR-1138 NOLINTBEGIN(modernize-use-nullptr) bracket. Its _MSC_VER-guarded nullptr macro failed scripts/dev/preflight.sh --stage msvcism as soon as the file was touched. The AGENTS.d page records the bracket.

Type

fix — bug fix

Checklist

Commits follow Conventional Commits (the commit-msg hook enforces this).
make format && make lint is green locally (commit hooks; scripts/dev/preflight.sh --stage msvcism PASS).
Unit tests pass: the CAMBI tests below, on CPU, CUDA, SYCL and HIP builds.
If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2 (no GPU or SIMD source changed; the exact-twin tests compare with ==).
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).
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 (no ADR: bug fix).

Bug-status hygiene (ADR-0165)

docs/state.md updated in this PR: T-CAMBI-10BIT-FULLREF-WIDE-SOURCE-ROWS-2026-10-05 added under "Recently closed" with the evidence. The open row exists only on feat(rust): add cambi_rust, a bit-identical Rust twin of the cambi extractor #2090's branch; that branch drops it when it rebases.

Netflix golden-data gate (ADR-0024)

I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
No golden value needs to change. make test-netflix-golden (GCC golden build): 280 passed, 3 skipped, 0 failed. Netflix python/test/cambi_test.py, which includes the full-reference and scaled-reference cases: 19 passed.

Cross-backend numerical results

All runs use --precision max and compare bits (==).

Check	Before	After
Sparks 10-bit, `cambi` with `full_ref=true:src_width=960:src_height=540` against no-reference `cambi`, 5 frames	0 of 5 equal (largest difference 0.369)	5 of 5
Sparks 10-bit, the same request with `--backend cuda` / `sycl` / `hip` (CPU extractor) against the twin's own no-reference `cambi`		5 of 5 on each
`test_cuda_exact_twins` (RTX 4090), new CAMBI case: CPU with `full_ref=true:src_width=1280:src_height=960` against `cambi_cuda`, 4 frames at 8 and 10 bits	10-bit fails (frame 0: CPU 2.2860845229574367, CUDA 4.673693073325393)	pass
`test_sycl_exact_twins` (Arc A380, xe), the same case	10-bit fails (same numbers)	pass
`test_hip_exact_twins` (gfx1036), the same case	10-bit fails (same numbers)	pass
CPU matrix: Netflix pair at 8, 10, 12, 16 bits and 4:2:2 10-bit, both 1080p checkerboards, Sparks; no-reference, `full_ref`, source twice and two thirds the picture, `enc_width=384:enc_height=216` (40 runs)		37 runs identical to before (524 frames). The 3 runs that move are the 10-bit runs with a source twice the picture, whose `cambi` now equals the no-reference run.

scripts/ci/exact_twins.d/cambi.{cuda,sycl,hip} remain valid. test_cuda_cambi_parity, test_sycl_cambi_parity and test_hip_cambi_parity pass.

Failing first

core/test/test_cambi_full_ref_wide_source against origin/master's cambi.c (each case run alone):

test_same_size_10b_wider_working_stride: wider working stride: 3006 misplaced samples  fail
test_same_size_10b_wider_input_stride:   wider input stride: 3006 misplaced samples    fail
test_full_ref_wide_source_8bit:  pass
test_full_ref_wide_source_10bit:
  10-bit full_ref, source 640x480 frame 0: cambi=2.5075090092021859 (no-reference 5.1443752275234989) source=4.4877977720494853 full_reference=0
  10-bit full_ref, source 640x480 frame 1: cambi=5.1199412243487199 (no-reference 5.1364877810621472) ...  fail
test_full_ref_wide_source_12bit: pass

After the fix, 5 of 5 pass.

Deep-dive deliverables (ADR-0108)

Research digest — no digest needed: trivial. The defect, its measurement and the fix are in the state row.
Decision matrix — no alternatives: only-one-way fix. A copy has to honour both strides; the score semantics come from cambi.c::extract and the metric docs.
AGENTS.md invariant note — core/src/feature/AGENTS.d/cambi.md: the 10-bit copy stays row by row, upstream's single memcpy is never brought back, and the ADR-1138 bracket is the file's one suppression. The generated index is refreshed.
Reproducer / smoke-test command — pasted below under "Reproducer".
CHANGELOG fragment — changelog.d/fixed/cambi-full-ref-wide-source-rows.md.
Rebase note — docs/rebase-notes.md, "CAMBI copies a same-size 10-bit plane row by row": an upstream sync that touches the copy keeps the fork's row loop, and the fork's version is dropped only when upstream merges an equivalent row-by-row copy for Netflix/vmaf#1670.

Docs: docs/metrics/cambi.md ("Full-reference mode") now says that cambi does not depend on the source options, and that 10-bit runs with a larger source were wrong before this fix.

tidy: cpu, cuda, hip, sycl, arm64 core/src/feature/cambi.c core/test/test_cambi_full_ref_wide_source.c, 0 findings; cuda core/test/test_cuda_exact_twins.c, hip core/test/test_hip_exact_twins.c, sycl core/test/test_sycl_exact_twins.c, 0 findings (container lanes, clang-tidy 22.1.8). No allowance shrank, so no baseline was written.

Reproducer

vmaf -r python/test/resource/yuv/sparks_ref_480x270.yuv42010le.yuv \
     -d python/test/resource/yuv/sparks_dis_480x270.yuv42010le.yuv \
     -w 480 -h 270 -p 420 -b 10 \
     --feature cambi=full_ref=true:src_width=960:src_height=540 \
     --no_prediction --precision max
# frame 0 cambi_srch_540_srcw_960: 0.3734397949735747 (was 0.0048362395598900215)

python3 scripts/ci/run_meson_test.py -- -C build test_cambi_full_ref_wide_source test_cambi \
    test_cambi_dispatch_invariance test_cambi_stage_simd test_feature_isa_invariance
# device builds: test_cuda_exact_twins, test_sycl_exact_twins, test_hip_exact_twins

Known follow-ups

The upstream parity matrix (scripts/dev/upstream_parity_matrix.py) runs full_ref=true:src_width=576:src_height=324. On its 10-bit fixture that source is the picture size, so the matrix never reaches this configuration, and no scripts/ci/upstream_parity.d/ fragment is needed. A variant with a larger source would record the deliberate divergence from upstream.
cambi_metal on a device: carried with the Metal rows (macOS tester bundle).

…ite its heatmaps On the Apple M4 Pro tester report (#2118) integer_cambi_metal produced no score anything could read. init_fex_metal() built the feature-name dictionary after cambi_metal_resolve_dimensions() had written the resolved encode and source sizes into five FEATURE_PARAM option slots, so every name carried them (cambi_encbd_8_ench_324_encw_576_srch_324_srcw_576 instead of Cambi_feature_cambi_score). All nine exact parity cases failed, the gate's cambi cell was ERROR on every fixture, and the default model stopped with "problem generating pooled VMAF score". The dictionary is now built first, as cambi.c::init and the CUDA, SYCL and HIP twins do, and a failed resolution releases it. The twin also implements heatmaps_path, its last option gap (test_twin_cambi). cambi_internal.h exports cambi.c's heatmap writers as vmaf_cambi_open_heatmaps(), vmaf_cambi_dump_c_values() and vmaf_cambi_close_heatmaps(). The CPU extractor calls them too, and the twin writes the distorted picture's c-values with them before the pooling reorders them. CPU scores and heatmap files are byte-identical before and after. close_cambi() now reports -EIO when a heatmap file fails to close. cambi.c spells the null pointer NULL under the ADR-1138 bracket, as #2111 does. test_metal_twin_option_tables_contract.py now checks every Metal twin for an option slot written before its names, and it fails on master. test_cambi_heatmap_writers covers the exported writers.

* ci(actions): check composite actions' schema and run blocks actionlint reads workflows only and rejects an action.yml, so the two composite actions under .github/actions/ were read by no gate. check-github-actions (check-jsonschema) validates the manifest schema; scripts/ci/check_composite_actions.py checks the structure and runs shellcheck over every bash and sh run block with actionlint's own ignore list. Both run in pre-commit (so in CI) and in make lint-actions. * ci(actions): type the composite-action checker for mypy scripts/ci/check_composite_actions.py keeps the PyYAML import suppression with its reason and types its manifest argument, as the pre-push mypy gate requires.

…urce scores the right rows (#2111) * fix(cambi): copy 10-bit planes row by row so full_ref with a wider source scores the right rows With full_ref=true, 10-bit input and src_width / src_height larger than the picture, cambi.c scored a distorted plane whose rows were shifted. The working pictures are allocated MAX(src, enc) wide, and the 10-bit same-size conversion copied `stride * height` samples in one memcpy at the input's stride. On the 10-bit Sparks pair, frame 0 `cambi` was 0.0048 with src_width=960:src_height=540 and 0.3734 without. decimate_same_size_16b() now copies one row at a time with both strides. `cambi` is the distorted picture's encode-size score and no longer depends on full_ref or the source size; every other configuration keeps its bits (37 of 40 matrix runs identical, the three that move are the 10-bit wide-source runs). - core/test/test_cambi_full_ref_wide_source.c fails without the fix (3006 of 3072 samples misplaced in each stride direction; 10-bit `cambi` 2.5075 against 5.1444). - The CUDA, SYCL and HIP exact-twin tests gain a case that gives only the CPU full_ref with a source twice the picture; it failed on all three before the fix and passes after (RTX 4090, Arc A380, gfx1036). The twins never had the defect: they declare no full_ref and convert from the picture's own pitch. cambi_metal calls the fixed CPU conversion. - cambi.c spells the null pointer NULL under the ADR-1138 bracket; the _MSC_VER-guarded nullptr macro failed the msvcism preflight stage. Upstream Netflix/vmaf master has the same single copy. * docs(state): link the upstream report of the cambi 10-bit row copy The maintainer approved an upstream report, filed as Netflix/vmaf#1670. The docs/state.md row now cross-links it, as the bug-tracking rule asks for upstream reports. The rebase note says when the fork's row loop may go: only once upstream merges an equivalent row-by-row copy that honours both strides, and the regression test stays either way.

…ite its heatmaps On the Apple M4 Pro tester report (#2118) integer_cambi_metal produced no score anything could read. init_fex_metal() built the feature-name dictionary after cambi_metal_resolve_dimensions() had written the resolved encode and source sizes into five FEATURE_PARAM option slots, so every name carried them (cambi_encbd_8_ench_324_encw_576_srch_324_srcw_576 instead of Cambi_feature_cambi_score). All nine exact parity cases failed, the gate's cambi cell was ERROR on every fixture, and the default model stopped with "problem generating pooled VMAF score". The dictionary is now built first, as cambi.c::init and the CUDA, SYCL and HIP twins do, and a failed resolution releases it. The twin also implements heatmaps_path, its last option gap (test_twin_cambi). cambi_internal.h exports cambi.c's heatmap writers as vmaf_cambi_open_heatmaps(), vmaf_cambi_dump_c_values() and vmaf_cambi_close_heatmaps(). The CPU extractor calls them too, and the twin writes the distorted picture's c-values with them before the pooling reorders them. CPU scores and heatmap files are byte-identical before and after. close_cambi() now reports -EIO when a heatmap file fails to close. cambi.c spells the null pointer NULL under the ADR-1138 bracket, as #2111 does. test_metal_twin_option_tables_contract.py now checks every Metal twin for an option slot written before its names, and it fails on master. test_cambi_heatmap_writers covers the exported writers.

…ite its heatmaps (VMAFx#2132) * fix(metal): report the CAMBI twin's score under the CPU's name and write its heatmaps On the Apple M4 Pro tester report (VMAFx#2118) integer_cambi_metal produced no score anything could read. init_fex_metal() built the feature-name dictionary after cambi_metal_resolve_dimensions() had written the resolved encode and source sizes into five FEATURE_PARAM option slots, so every name carried them (cambi_encbd_8_ench_324_encw_576_srch_324_srcw_576 instead of Cambi_feature_cambi_score). All nine exact parity cases failed, the gate's cambi cell was ERROR on every fixture, and the default model stopped with "problem generating pooled VMAF score". The dictionary is now built first, as cambi.c::init and the CUDA, SYCL and HIP twins do, and a failed resolution releases it. The twin also implements heatmaps_path, its last option gap (test_twin_cambi). cambi_internal.h exports cambi.c's heatmap writers as vmaf_cambi_open_heatmaps(), vmaf_cambi_dump_c_values() and vmaf_cambi_close_heatmaps(). The CPU extractor calls them too, and the twin writes the distorted picture's c-values with them before the pooling reorders them. CPU scores and heatmap files are byte-identical before and after. close_cambi() now reports -EIO when a heatmap file fails to close. cambi.c spells the null pointer NULL under the ADR-1138 bracket, as VMAFx#2111 does. test_metal_twin_option_tables_contract.py now checks every Metal twin for an option slot written before its names, and it fails on master. test_cambi_heatmap_writers covers the exported writers. * docs: regenerate the indexes and the citation map after rebasing

This was referenced Oct 5, 2026

cambi: 10-bit full_ref with a larger source copies the distorted plane at the wrong stride Netflix/vmaf#1670

Open

fix(metal): report the CAMBI twin's score under the CPU's name and write its heatmaps #2132

Merged

lusoris force-pushed the fix/cambi-fullref-wide-source-rows branch from 3524e34 to 8be8e20 Compare October 5, 2026 14:20

lusoris and others added 2 commits October 5, 2026 17:13

lusoris force-pushed the fix/cambi-fullref-wide-source-rows branch from 8be8e20 to cc5548c Compare October 5, 2026 15:36

lusoris merged commit cc5548c into master Oct 5, 2026
11 of 79 checks passed

lusoris deleted the fix/cambi-fullref-wide-source-rows branch October 5, 2026 15:37

github-actions Bot added the type:bug Something isn't working label Oct 5, 2026

lusoris mentioned this pull request Oct 5, 2026

ci(tidy): fail when a translation unit is read by no clang-tidy lane or excepted (ADR-1142) #2196

Merged

14 of 16 tasks

lusoris mentioned this pull request Oct 7, 2026

feat(rust): add cambi_rust, a bit-identical Rust twin of the cambi extractor #2090

Merged

11 of 26 tasks

Navigation Menu

Repository navigation

fix(cambi): copy 10-bit planes row by row so full_ref with a wider source scores the right rows - #2111

fix(cambi): copy 10-bit planes row by row so full_ref with a wider source scores the right rows#2111
lusoris merged 2 commits into
masterfrom
fix/cambi-fullref-wide-source-rows

lusoris commented Oct 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lusoris commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type

Checklist

Bug-status hygiene (ADR-0165)

Netflix golden-data gate (ADR-0024)

Cross-backend numerical results

Failing first

Deep-dive deliverables (ADR-0108)

Reproducer

Known follow-ups

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lusoris commented Oct 5, 2026 •

edited

Loading