Skip to content

refactor(iqa): rename reserved-identifier surface + lint cascade sweep (ADR-0148)#84

Merged
lusoris merged 1 commit intomasterfrom
refactor/iqa-rename-t7-6
Apr 24, 2026
Merged

refactor(iqa): rename reserved-identifier surface + lint cascade sweep (ADR-0148)#84
lusoris merged 1 commit intomasterfrom
refactor/iqa-rename-t7-6

Conversation

@lusoris
Copy link
Copy Markdown
Owner

@lusoris lusoris commented Apr 24, 2026

Summary

Rename every _iqa_* / struct _kernel / _ssim_int / _map_reduce / _map / _reduce / _context / _ms_ssim_* / _ssim_* / _alloc_buffers / _free_buffers symbol and the four underscore-prefixed header guards to non-reserved spellings across the IQA tree (21 files). Sweeps the ADR-0141 touched-file lint cascade that surfaced (~40 pre-existing warnings invisible to prior diff-based CI runs).

The rename's stated motivation in rebase-notes 0039 ("clear remaining reserved-identifier suppressions") turned out to be wrong on inspection — there were zero NOLINTs to clear. The actual win is the IQA tree is now baseline lint-clean, so future PRs touching ssim.c / ms_ssim.c / iqa/*.{c,h} / convolve_*.{c,h} no longer trip the touched-file lint cascade.

Bit-identical VMAF score: VMAF_CPU_MASK=0 vs =255 on Netflix golden pair src01_hrc00/01_576x324 with --feature float_ssim --feature float_ms_ssim → diff exit 0. Full vmaf_v0.6.1 score on frame 0: 83.856284 (unchanged from master).

Type

  • refactor — no behavior change

Checklist

  • Commits follow Conventional Commits.
  • make format && make lint is green locally.
  • Unit tests pass: meson test -C build → 32/32.
  • Touched IQA / SIMD paths — bit-identical between scalar and SIMD on Netflix golden pair (diff exit 0).
  • No new .c/.h/.cpp files added.
  • Not a breaking change.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Cross-backend numerical results

VMAF_CPU_MASK=0 vs =255 (scalar vs SIMD), --feature float_ssim --feature float_ms_ssim:
  All metrics  scalar-vs-simd = 0-ULP  (diff exit 0 on Netflix golden-pair #1)

Full vmaf_v0.6.1 model run, frame 0:
  vmaf="83.856284"  (identical pre/post this PR)

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: mechanical rename + lint sweep.
  • Decision matrix — captured in ADR-0148 §Alternatives considered.
  • AGENTS.md invariant note — added to libvmaf/src/feature/AGENTS.md (IQA reserved-identifier rename + load-bearing NOLINT brackets).
  • Reproducer / smoke-test command — pasted below under "Reproducer".
  • CHANGELOG.md "lusoris fork" entry — bullet added under ### Changed in CHANGELOG.md.
  • Rebase note — entry 0041 in docs/rebase-notes.md.

Reproducer

ninja -C build && meson test -C build

for mask in 0 255; do
  VMAF_CPU_MASK=$mask ./build/tools/vmaf \
    --reference python/test/resource/yuv/src01_hrc00_576x324.yuv \
    --distorted python/test/resource/yuv/src01_hrc01_576x324.yuv \
    --width 576 --height 324 --pixel_format 420 --bitdepth 8 \
    -m version=vmaf_v0.6.1 \
    --feature float_ssim --feature float_ms_ssim \
    -o /tmp/iqa_$mask.xml
done
diff <(grep -v fyi /tmp/iqa_0.xml) <(grep -v fyi /tmp/iqa_255.xml)
# expect exit 0 (bit-identical scalar vs SIMD on float_ssim / float_ms_ssim)

# Plus per-touched-file clang-tidy:
git diff --name-only HEAD | grep -E '\.(c|h)$' | grep -v arm64 | while read f; do
  clang-tidy -p build "$f" --quiet 2>&1 | grep -E "warning:|error:" | grep -v "Suppressed\|generated\|-warnings-as-errors"
done
# expect zero output

Known follow-ups

  • arm64/convolve_neon.{c,h} were touched (rename only) but excluded from clang-tidy on x86 hosts because clang-tidy can't parse <arm_neon.h> intrinsics there. CI's lint workflow already excludes ^libvmaf/src/feature/arm64/.
  • ADR-0141 §Historical debt's mention of _iqa_* reserved-identifier suppressions can be deleted once this lands (they don't exist).

🤖 Generated with Claude Code

…p (ADR-0148)

Rename every `_iqa_*` / `struct _kernel` / `_ssim_int` /
`_map_reduce` / `_map` / `_reduce` / `_context` / `_ms_ssim_*` /
`_ssim_*` / `_alloc_buffers` / `_free_buffers` symbol and the four
underscore-prefixed header guards to non-reserved spellings across
the IQA tree (21 files). Sweeps the ADR-0141 touched-file lint
cascade that surfaced (~40 pre-existing warnings invisible to prior
diff-based CI runs).

Symbol rename map:
 - _iqa_{convolve,decimate,filter_pixel,get_pixel,img_filter,
   ssim,*_set_dispatch}              -> iqa_*
 - struct _kernel                     -> struct iqa_kernel
 - _ssim_int (typedef)                -> iqa_ssim_int
 - _map_reduce / _map / _reduce       -> iqa_{map_reduce,map_fn,reduce}
 - _context                           -> ms_ssim_context
 - _ms_ssim_map / _ssim_map           -> ms_ssim_map_fn / ssim_map_fn
 - _ms_ssim_reduce / _ssim_reduce     -> ms_ssim_reduce_fn / ssim_reduce_fn
 - _alloc_buffers / _free_buffers     -> ms_ssim_{alloc,free}_buffers
 - _calc_scale-style headers (4)      -> *_INCLUDED

Lint sweep on touched files:
 - misc-use-internal-linkage:
     static where TU-local (ssim_map_fn, ssim_reduce_fn,
     ms_ssim_map_fn, ms_ssim_reduce_fn, ms_ssim_alloc/free_buffers),
     cross-TU NOLINT for compute_ssim / compute_ms_ssim with inline
     justification, extern-registered NOLINT for vmaf_fex_float_*ssim.
 - readability-isolate-declaration: split every multi-decl statement.
 - readability-function-size: refactor calc_ssim, compute_ssim,
   compute_ms_ssim, run_gauss_tests via small named static helpers.
 - bugprone-implicit-widening-of-multiplication-result: cast to
   size_t/ptrdiff_t at every malloc(w*h*...) and pointer-offset site.
 - bugprone-multi-level-implicit-pointer-conversion: explicit (void*)
   cast at free(lines) site in integer_ssim.c.
 - misc-unused-parameters: (void)cast feature-extractor lifecycle
   callback parameters that the registry signature requires.
 - clang-analyzer-security.ArrayBound: scoped NOLINTBEGIN/END around
   ssim_accumulate_row + ssim_reduce_row_range inner kernel loops
   (k_min/k_max clamping is provably correct but escapes the
   analyzer across helper boundaries).
 - clang-analyzer-unix.Malloc: scoped NOLINTBEGIN/END around the
   test_iqa_convolve.c helpers (test exits process on failure;
   small allocations leak by design at process end).
 - mem.h: dropped unused #include from ssim.c.
 - printf/fflush: (void)cast return values at error-path sites.

Verification:
 - meson test -C build: 32/32 pass.
 - clang-tidy -p build on every touched .c/.h (excl. arm64): zero
   warnings.
 - VMAF_CPU_MASK=0 vs =255 on src01_hrc00/01_576x324 with
   --feature float_ssim --feature float_ms_ssim: diff exit 0.
 - Full vmaf_v0.6.1 score on frame 0: 83.856284 (unchanged).

Closes backlog item T7-6.

Deliverables (ADR-0108):
 1. research digest: no digest needed - mechanical rename + lint sweep
 2. decision matrix: ADR-0148 §Alternatives considered
 3. AGENTS.md invariant: libvmaf/src/feature/AGENTS.md
 4. reproducer: VMAF_CPU_MASK=0 vs =255 with float_ssim/float_ms_ssim
    on Netflix golden pair, diff exit 0
 5. CHANGELOG: fork entry under Changed
 6. rebase-notes: entry 0041

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lusoris lusoris merged commit 985be1b into master Apr 24, 2026
45 checks passed
@lusoris lusoris deleted the refactor/iqa-rename-t7-6 branch April 24, 2026 00:25
@github-actions github-actions Bot mentioned this pull request Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant