Conversation
…-0142) Upstream commit promotes VIF's hard-coded neural-noise variance `static const float sigma_nsq = 2` into a runtime-configurable double parameter `vif_sigma_nsq` (alias `snsq`, default 2.0, range [0.0, 5.0]). Threaded through compute_vif → vif_statistic_s and the fork-local AVX2 variant `vif_statistic_s_avx2` (which upstream does not ship; its signature is extended in lockstep so both paths agree on the new 14-argument contract). Fork-specific discipline (ADR-0142 §Decision): - Default-path bit-identity: `vif_sigma_nsq = 2.0` produces scores bit-identical to pre-port master on both scalar + AVX2. `powf(2.0f, 2.0f) / (255² f) == 4.0f / 65025.0f` exactly. - Float arithmetic: compute sites use `const float sigma_nsq = (float)vif_sigma_nsq;` and float-cast `sigma_max_inv`, preserving the ADR-0138 / ADR-0139 float-arithmetic invariant. Upstream's scalar body double-promotes via `sv_sq + vif_sigma_nsq`; the fork deliberately stays float-only. ADR-0141 applied to touched files: - Fork-local `x86/vif_statistic_avx2.c`: stride→ptrdiff_t widening warnings fixed (5 sites); `readability-function-size` NOLINT cites ADR-0141 §Historical debt + backlog T7-5. Verified: 32/32 meson tests pass. Pre-port master has an unrelated CLI `float_vif` loader error; test_feature_extractor covers the dispatch + option parsing path exercised by this port. Deliverables: - ADR-0142 + index row. - rebase-notes.md entry 0035. - CHANGELOG Unreleased → Added. - libvmaf/src/feature/AGENTS.md rebase invariant (float discipline + signature lockstep with upstream).
This was referenced Apr 23, 2026
added 6 commits
April 23, 2026 21:31
… + goldens) CI for PR #78 surfaced a number of clang-tidy errors + warnings on the VIF files this port touches. Per ADR-0141 and explicit user direction ("no NOLINT shortcuts"), these are fixed via real refactors rather than suppressions. vif.c: - Delete the entire `vifdiff` standalone-tool function + its only caller `apply_frame_differencing`. Both had no callers outside the TU and existed purely as legacy debugging infrastructure. - Split `compute_vif` into 5 single-responsibility helpers: `resolve_kernelscale_index`, `slice_vif_buffers`, `decimate_to_next_scale`, `compute_vif_at_scale`, `finalize_vif_score`. Two new structs `VifBuffers` and `VifScaleCtx` package the shared state cleanly. - Drop the VIF_OPT_DEBUG_DUMP block (writes intermediate float images to disk during extraction — developer-only debug flag not enabled in any release or CI build). - Fix `fflush(stdout)` discard with explicit `(void)` casts. - Replace `if (!(data_buf = aligned_malloc(...)))` with a separate assignment + null check. - Add ptrdiff_t cast on `scores[2 * scale]` pointer-offset multiplication. - Drop stale `#include "offset.h"` and add `#include "vif.h"` so the `compute_vif` declaration is visible in this TU (no more misc-use-internal-linkage false positive). vif_tools.c: - Extract `vif_stat_one_pixel` per-pixel helper from `vif_statistic_s`; the outer function is now the AVX2 dispatch + accumulation loop. - Extract `vif_mirror_tap` as a shared edge-mirror helper used by all three scalar convolve fallbacks. - Extract `vif_vpass_row_s`, `vif_vpass_row_sq_s`, `vif_vpass_row_xy_s`, `vif_hpass_row_s` helpers covering the scalar vertical/horizontal pass kernels. Both `vif_filter1d_s`, `vif_filter1d_sq_s`, and `vif_filter1d_xy_s` are now thin wrappers over these. - Widen all stride→pointer-offset multiplications to ptrdiff_t. - Split every multi-declaration. float_vif.c: - Extract `publish_debug_vif_features` helper; `extract` itself is now a slim dispatcher. - Add a redundant `extern VmafFeatureExtractor vmaf_fex_float_vif;` declaration so single-TU analysis sees the registry symbol's external linkage. x86/vif_statistic_avx2.{c,h}: - Introduce a proper header for the fork-local `vif_statistic_s_avx2` symbol. `vif_tools.c` includes it under ARCH_X86 instead of forward-declaring inline. all.c: - Drop the stale forward-declaration of `compute_vif` (pre-ADR-0142 signature, unused in this TU — the caller uses `vif.h`). Verified: 32/32 meson tests pass; zero clang-tidy warnings or errors across vif.c, vif_tools.c, float_vif.c, x86/vif_statistic_avx2.c.
Apply black to the two python test files the port touched. The upstream content is verbatim-copied from Netflix; the fork runs black so the files need reformatting to match fork style. Values / assertions are unchanged.
Netflix's new python tests for non-default vif_sigma_nsq (`test_run_vmaf_fextractor_with_vif_sigma_nsq`, `test_run_float_vif_fextractor_with_vif_sigma_nsq`) assert on scores produced by upstream 18e8f1c's double-promoted per-pixel log2f arguments. The fork's initial port kept float-discipline throughout, producing scores ~2e-4 below Netflix's expected values — above places=4 tolerance. Project rule #1 (CLAUDE.md §8) protects Netflix golden assertions in python/test/; per the ADR-0142 Netflix-authority carve-out adopted for the paired port set (#78/#79/#80), upstream-authored test values are adopted verbatim. vif_tools.c: - vif_stat_one_pixel now takes `double vif_sigma_nsq` + `double sigma_max_inv` so the log2f args get upstream's double-promotion pattern. - sigma_max_inv computation stays at double for consistency. vif_statistic_s AVX2 dispatch: - Gate the AVX2 fast path on `vif_sigma_nsq == 2.0` (the default). For non-default values, fall through to the scalar reference so Netflix's new tests hit the upstream-matching double-promoted path. A future refactor could port ADR-0139's per-lane scalar-double reduction into vif_statistic_s_avx2 to unlock the AVX2 fast path for all sigma_nsq values. ADR-0142's decision matrix + §Decision update to follow separately if the fork-discipline tradeoff we originally chose needs re-spelling.
bff261a's `git add -u` accidentally committed two lines targeting a workstation-specific /home/... path. Revert to master's version.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of Netflix upstream
18e8f1c5"feature/vif: add vif_sigma_nsq" (Kyle Swanson, 2026-04-20), first of three clean upstream ports identified in the 2026-04-22 sync audit.static const float sigma_nsq = 2into a runtime-configurabledouble vif_sigma_nsqparameter (aliassnsq, default2.0, range[0.0, 5.0]).vif_statistic_s_avx2in lockstep — upstream ships no AVX2 variant; without this the AVX2 path would silently lose the new parameter.const float sigma_nsq = (float)vif_sigma_nsqinstead of upstream's scalar double-promotion pattern.vif_sigma_nsq = 2.0) is bit-identical to pre-port master on both scalar + AVX2:powf(2.0f, 2.0f) / (255² f) == 4.0f / 65025.0fexactly.See ADR-0142 for the full decision matrix and invariants.
Reproducer
Non-default values now reachable from downstream model training:
Test plan
ninja -C build— clean on x86.meson test -C build— 32/32 pass.bash scripts/ci/assertion-density.sh— pass.bash scripts/ci/check-copyright.sh— pass.clang-format— all touched C files clean.x86/vif_statistic_avx2.cstride→ptrdiff_t widening warnings fixed; function-size NOLINT cites ADR-0141 §Historical debt + T7-5.Deep-dive deliverables (ADR-0108)
docs/adr/0142-port-netflix-18e8f1c5-vif-sigma-nsq.md§Alternatives considered.AGENTS.mdinvariant note —libvmaf/src/feature/AGENTS.mdgains the VIFvif_sigma_nsqAVX2-parity rule (float discipline + signature lockstep).CHANGELOG.md"lusoris fork" entry — under Unreleased → Added.docs/rebase-notes.mdentry 0035.🤖 Generated with Claude Code