Research-2043: CAMBI production lint without numerical changes¶
2026-09-21 strict-clean follow-up¶
The earlier 2026-09-08 cleanup documented ten narrow unusedFunction annotations and callback/null-pointer suppressions in cambi.c. That posture is superseded: the file now has zero NOLINT and zero Cppcheck suppression markers. The production fixes make every retained helper live, bound every search loop, and express the shared callback constraint in code while keeping the CAMBI score path byte-for-byte stable.
Findings and code fixes¶
| Finding | Root cause | Strict-clean implementation |
|---|---|---|
modernize-use-nullptr | Clang treats this C23 translation unit as supporting nullptr, while MSVC's C mode still needs NULL. | CAMBI_NULL_POINTER selects nullptr except under _MSC_VER, where it selects NULL. |
misc-use-internal-linkage | The extractor registry definition had no prior external declaration in this translation unit. | A matching extern VmafFeatureExtractor vmaf_fex_cambi declaration records the cross-TU contract. |
constStatement | The TVI bisection's exhaustive flag handling ended in an impossible (void)0 branch. | A fixed 16-step bisection has only the two state updates and the successful return. |
constParameterCallback | The shared extractor ABI supplies mutable picture pointers although CAMBI only reads them. | read_only_picture_view() adapts the ABI arguments to const views before validation and preprocessing. |
unusedFunction on ten private exports | Seven helpers were only called by optional GPU translation units and three were scaffold-only. | CPU/reference paths now use the same ten wrappers directly; the wrappers remain available to CUDA, HIP, SYCL, and Metal twins. |
| HISS-04 long function | The public option table occupied most of the translation unit's largest declaration. | CAMBI_OPTION expresses the same option records compactly, without changing names, aliases, defaults, limits, help text, or order. |
Three unbounded constructs were also made finite without changing the legal score path:
- TVI bisection performs 16 iterations, enough to exhaust the
uint16_tsample domain after its existing endpoint checks. - VLT scans through
UINT16_MAXand returns that endpoint when no sample can meet the requested luminance. The previousuint16_tincrement wrapped to zero forever for an unreachable threshold. - Quick-select bounds its outer work by
nand each partition scan by the initial partition span. Pivot choice, comparison direction, and swap order are unchanged.
Initialization now returns through fail_init() instead of goto, and the SWAP_FLOATS statement macro is an inline function. ceil_log2() is a 32-step for loop. These are control-flow changes only; constants, accumulation order, option semantics, feature names, and public callback types are unchanged. This is an internal bug/lint cleanup, so no new ADR, public usage document, or FFmpeg patch-stack refresh is required.
Alternatives considered¶
| Alternative | Decision | Reason |
|---|---|---|
| Keep narrow analyzer suppressions | Rejected | The whole-tree standard requires touched source to express the invariant in code. |
| Delete helpers unused by the CPU build | Rejected | They are the stable host seam for optional GPU twins and documented rebase invariants. |
| Change the shared extractor callback ABI to const pointers | Rejected | That would fan out across every extractor for a CAMBI-local read-only contract. |
| Bound the existing algorithms and route CPU work through retained helpers | Chosen | It removes the warning causes locally and preserves calculation order and cross-backend seams. |
Focused regression coverage¶
core/test/test_cambi.c directly includes the shipped implementation and now adds three termination/order checks:
- threshold extrema across differences 1, 2, 4, 8, 16, and 32 exercise the bounded TVI search and verify the returned transition;
- an infinite luminance threshold proves VLT terminates at
UINT16_MAX; - all-equal and descending inputs pin quick-select progress and partition order.
The test binary reports 25/25 passing cases. Before the VLT bound was added, the infinite-threshold regression timed out, so the test distinguishes the fixed implementation from the old wraparound loop.
Validation receipt¶
The strict-clean tree was compared with base e0d31c5bc using the repository's 48-frame 576x324 fixture, with prediction disabled and --precision max. Both dispatched CPU and --cpumask 0 scalar runs produce mean CAMBI score 0.51441210777008473. After removing volatile version and fps fields, all four base/current JSON outputs are byte-identical with SHA-256 2fed4234f9f8c018ac9af0810dbf43a0c7a30765bee00e4e55c23de983a1a523.
Validation commands:
ninja -C build test/test_cambi tools/vmaf
meson test -C build test_cambi --print-errorlogs
clang-tidy -p build core/src/feature/cambi.c --extra-arg=-flto
cppcheck --enable=all --check-level=exhaustive --inline-suppr \
--library=posix --library=scripts/ci/cppcheck-public-entrypoints.cfg \
--suppressions-list=.cppcheck-suppressions.txt \
--project=build/compile_commands.json \
--file-filter='*/core/src/feature/cambi.c' --error-exitcode=1
praetorctl audit -touched core/src/feature/cambi.c,core/test/test_cambi.c
Clang-tidy and Cppcheck report zero findings in cambi.c; the reduced-context Cppcheck command still reports the unrelated inline helper in untouched feature_collector.h, demonstrating that unusedFunction remains enabled. The touched HISS audit is clean at 1,330 active findings against the 1,411 baseline. No baseline, Netflix golden assertion, or snapshot was changed.
Durable run receipts belong under .workingdir/evidence/2026-09-21-cambi-strict-clean/; .workingdir2 is not a repository interface and is not referenced by the new workflow.
Historical 2026-09-08 receipt¶
The original cleanup made five const/shadow corrections and retained ten private helper trampolines using exact annotations because the CPU profile did not call them. It measured byte-identical .text/.rodata and zero focused clang-tidy findings with the then-current suppressions. That evidence remains useful as the starting point, but its annotation policy and .workingdir2 location are historical and no longer describe the source.