Research 1166 — upstream Netflix/vmaf issue harvest, 2026-09-03¶

Companion to ADR-1166.

This is the durable output of the harvest: every candidate that was looked at, its verdict, and the evidence behind it — including the negative verdicts. A future session that wonders "does Netflix/vmaf#N affect us?" should read this table before re-investigating.

Fork layout reminder: upstream's libvmaf/src/... maps to core/src/... (ADR-0700), several C files became C++ twins, and CUDA / SYCL / HIP / Metal backends are fork-added. An upstream line number is never directly usable.

Verdict vocabulary¶

Summary table¶

Fixed in this batch¶

Netflix/vmaf#1582 — single-bounce mirror + negative border bound¶

Two distinct defects in the float convolution, both carried verbatim.

Defect 1 — single-bounce mirror. convolution_edge_s, _sq_s and _xy_s in core/src/feature/common/convolution_internal.h each open-coded

if (j_tap < 0)            j_tap = -j_tap;
else if (j_tap >= width)  j_tap = width - (j_tap - width + 2);   /* == 2*w - j - 2 */

One bounce only lands in range when the plane is at least radius + 1 across. At size == 2, a tap of -2 folds to +2 (still >= size) and a tap of +3 folds to -1. There is no convolution_mirror() helper in the fork (grep -rn convolution_mirror → 0 hits before this PR).

Defect 2 — negative border bound. convolution.c derived borders_right = vmaf_floorn(width - (filter_width - radius), step), which is negative whenever the plane is narrower than the filter. The trailing loop then started at a negative index and wrote dst[i * dst_stride - 1] — a heap underflow write. borders_left = vmaf_ceiln(radius, step) can likewise exceed the width, walking the leading loop past the row.

Reachability. Not latent. Two live CPU paths reach the defective sizes with supported input and no non-default option beyond the one named:

• --feature float_vif on any frame in 9..15 px. float_vif's own guard admitted >= 9, but compute_vif() runs a four-scale ladder that halves the working dimension per scale and re-convolves with that scale's Gaussian ({17, 9, 5, 3} taps at the default kernelscale). The binding constraint is scale 3: w0 / 8 >= 2, i.e. w0 >= 16. ASan reports a heap-buffer-overflow READ at convolution_internal.h for 15x15 / 12x12 / 10x10 / 9x9 and is clean at 16 and above — the break is exactly at 16, as the arithmetic predicts.
• --feature float_motion with motion_add_uv=true on a 4x4 YUV420P frame. motion_check_min_dim() validated the luma dimensions only, while motion_blur_plane() is called per plane with ref_pic->w[c] / ref_pic->h[c]. 4x4 YUV420P gives a 2x2 chroma plane, below the 3x3 minimum. The same 4x4 run with default options (luma only) is clean, which isolates the chroma plane as the sole cause.

Fix. A single convolution_reflect101(idx, size) helper that folds repeatedly until the index is in range (with a size <= 1 short circuit, which would otherwise not terminate), used by all three edge helpers; and a convolution_clamp_borders() that clamps both bounds into [0, dim]. For every size >= radius + 1 the fold exits on the first iteration and yields the identical index, so nothing in contract moves — pinned by test_large_plane_bit_identical in core/test/test_convolution_edge_small.c, which compares a 24x24 run against an explicit single-bounce reference and asserts bit equality.

The two guard gaps are closed at the guard, not only in the kernel:

• float_vif now derives its minimum from vif_get_min_dim(kernelscale) — max over the ladder of ((filter_width_s / 2) + 1) << s, which is 16 at the default kernelscale — instead of a hard-coded 9.
• float_motion validates every plane it will actually convolve, deriving the chroma dimensions with picture.c's own (dim + ss) >> ss geometry.

Regression test. core/test/test_convolution_edge_small.c embeds each plane in a NaN-poisoned buffer: an escaping tap reads NaN and taints the output, an escaping write replaces a poison NaN with a finite value. On the pre-fix tree test_scalar_5tap_small_planes fails on the first case; after the fix all four cases pass. Plus the guard cases in test_float_vif_min_dim.c (9x9 and 15x15 now rejected, 16x16 accepted) and test_motion_min_dim.c (motion_add_uv at 4x4 / 3x3 rejected, 5x5 accepted).

Netflix/vmaf#1581 — same mirror, motion extractors¶

The candidate's framing ("latent hardening") was too weak: the chroma path above is a live heap out-of-bounds read on plain CPU, and it is fixed here.

The motion extractors' own mirror() bodies (integer_motion.c:148, integer_motion_v2.c:152, x86/motion_avx2.c:27, x86/motion_avx512.c:30, arm64/motion_v2_neon.c:46, and the CUDA / HIP / Metal twins) are left single-bounce on purpose. They sit downstream of an init() guard that has rejected w < 3 || h < 3 since Research-0094, so the defective sizes never reach them. That is a deliberate divergence from upstream #1581, which fixes mirror() so tiny frames can be scored; the fork errors out instead. Recorded in docs/rebase-notes.md because /sync-upstream will collide here.

The Metal extractors were the exception — see #1580 below.

Netflix/vmaf#1580 — motion mirror at tiny frames¶

Mostly ALREADY-FIXED. The fork found this independently (docs/research/0094-motion-v2-flush-dict-leak-round7.md, "Finding E-1") and discharged it with min-dim init() guards. Audit of all 15 motion extractor translation units: 12 guarded, 3 not — and the 3 were exactly the fork-added Metal extractors, written after that sweep:

• core/src/feature/metal/integer_motion_metal.mm (only rejected motion_add_uv, then assigned frame_w/frame_h and allocated)
• core/src/feature/metal/integer_motion_v2_metal.mm
• core/src/feature/metal/float_motion_metal.mm

All three are registered and shipped (feature_extractor.cpp:229/232/234), so --feature float_motion_metal on a 1- or 2-pixel-tall frame read out of bounds on device. The guard now runs as the first statement after the fex->priv cast, before vmaf_metal_context_new, so no cleanup path is needed. Runtime reproduction needs an Apple GPU; the gap is grep-provable and the fix needs no hardware to write, which is why this is AFFECTS-FORK and not NEEDS-HARDWARE.

test_motion_min_dim.c::test_metal_motion_min_dim exercises the rejection host-side (it degrades to a no-op on a build without HAVE_METAL).

Adjacent finding, deliberately NOT fixed here. core/src/feature/metal/integer_motion_v2.metal:54 uses 2 * sup - idx - 1, while the CUDA twin (cuda/integer_adm/../integer_motion_v2/motion_v2_score.cu:50) and the CPU source (integer_motion_v2.c:157) both use 2 * size - idx - 2. The Metal file's own header comment claims it "matches the CUDA twin" — it does not. This is the boundary-row-replication off-by-one that PR #120 fixed for motion v1, so Metal motion_v2 is very likely off-parity at every frame size, not only tiny ones. Fixing it moves Metal motion_v2 scores and needs a cross-backend parity run on Apple hardware: own PR, own ADR. docs/state.md row added.

Netflix/vmaf#1242 — VmafFeatureDictionary ownership¶

Three findings, all present.

1. Leak on the -ENOMEM path. core/src/model.c returned -ENOMEM straight out of the loop when vmaf_dictionary_merge() failed, skipping the unconditional vmaf_dictionary_free(&opts_dict) at the tail. Under the contract the fork itself publishes, that is an unconditional leak of the caller's dictionary. The correct shape already existed in-tree, in the unbuilt C++ twin core/src/model.cpp (err = -ENOMEM; break;); core/src/meson.build compiles model.c, not model.cpp.
2. Collection wrapper compounded it and swallowed the error. vmaf_model_collection_feature_overload() discarded vmaf_dictionary_copy()'s return value, never freed the partially built copy, silently skipped the remaining sub-models, and could still return 0 from the lead-model call. It also dereferenced *model_collection without checking it and never checked model / feature_name / opts_dict.
3. The headers contradicted each other. core/include/libvmaf/feature.h said "on failure the caller still owns the dictionary"; model.h and ADR-0806 said ownership transfers "on both success and failure". The fork's own test core/test/test_model_collection_api.c:274-277 follows feature.h — correct against the implementation, a textbook CWE-415 double free against the model.h wording. A third-party caller reading either header alone gets it wrong on some path.

Fix. model.c breaks to the common exit instead of returning; the collection wrapper captures the copy result, frees the partial copy, folds the error into err, and gains the missing argument guards; model.cpp is kept convergent. The contract is written once, identically, in feature.h, model.h (both call sites) and libvmaf.h, in the shape the implementation actually has:

the dictionary is consumed on every path EXCEPT the argument-validation guards (-EINVAL from a NULL or unknown argument), where nothing was consumed and the caller still owns it.

That is also what vmaf_use_feature() does — core/src/libvmaf.c:1607-1650 returns -EINVAL for !vmaf / !feature_name / unknown feature name without touching opts_dict, and consumes on every other path. ADR-0806's line citations were stale (model.c:196, libvmaf.c:1520-1525); it is now marked Superseded by ADR-1166 rather than edited in place.

No production double free today: the only in-tree production callers are core/tools/vmaf.cpp:471-473 and :520-522, and neither frees the dict afterwards. The exposure was leaks on the OOM / -EINVAL paths plus the contract landmine for external API users.

Regression test. core/test/test_model_feature_overload_ownership.c pins the guard paths (caller may still free), the success path (consumed), and drives the merge-failure branch deterministically without malloc-fail injection: a heap-allocated empty dictionary makes vmaf_dictionary_merge() return NULL, which is exactly the branch that leaked. The leak itself is what LeakSanitizer reports in the ASan lane. The NULL-collection case is undefined behaviour pre-fix (the optimiser deletes it under LTO, leaving a garbage return) and a defined -EINVAL after.

Netflix/vmaf#743 — Windows console mojibake¶

Present verbatim. core/tools/spinner.h carries the 56-entry UTF-8 braille table (336 non-ASCII bytes = 56 × 2 glyphs × 3 bytes), and core/tools/vmaf.cpp emitted it with a plain byte-oriented fprintf to stderr, inside the if (istty && !c->quiet) block — the interactive-console case.

Grep-provable API misuse: nothing in the tree set the console output code page or enabled VT processing. grep -rn "SetConsoleOutputCP\|CP_UTF8\|GetConsoleOutputCP\|_setmode\|_O_U8TEXT" core/ returned nothing; the one #include <windows.h> in the CLI is commented as being there for QueryPerformanceCounter (ADR-1081).

Decoding the exact frame bytes under the default console code pages, Linux-side:

Same line, second defect: the trailing \033[K erase-to-EOL was also unconditional, and legacy conhost has ENABLE_VIRTUAL_TERMINAL_PROCESSING off by default, so it prints literally.

The surface is live on Windows: libvmaf-build-matrix.yml builds the MinGW64 CPU leg and the two MSVC + CUDA / SYCL legs, all of which compile vmaf.cpp.

Fix. A WindowsConsoleGuard RAII object declared at the top of main() switches the console to UTF-8 and enables VT for the run, restoring the previous code page and mode in its destructor — which C++ runs on every goto cleanup path too, since the guard is declared before every jump target. spinner.h grows an ASCII fallback table plus spinner_table_for_codepage() and spinner_erase_eol(); vmaf.cpp resolves both from the console's actual capabilities. Everything Windows-specific is #ifdef _WIN32; on POSIX the selectors return the braille table and \033[K unconditionally, so the emitted bytes are unchanged.

Regression test. core/test/test_spinner.cpp drives the selectors with the code pages a real conhost reports (437, 1252, 936, 0) and asserts the ASCII table; asserts the braille table for 65001; asserts the erase sequence is VT-gated; and pins the braille table's first/last entries byte-for-byte plus strlen(entry) == 6 for all 56, so the POSIX output cannot drift.

Netflix/vmaf#1573 — build-system hunks¶

Hunk (a) — picture_cuda.c uninitialised priv->cuda.state — is ALREADY-FIXED: core/src/cuda/picture_cuda.c:190 sets it in the pinned path and :251 in the device path.

Hunk (b) — AFFECTS-FORK, and worse here than upstream. core/src/meson.build fed nvcc relative includes (-I ./src -I ../src -I ../include ...). Since ADR-0700 moved the project root to core/, those only resolve when the build directory is a direct child of core/. With the layout the fork's own docs use (meson setup build core from the repo root) the CUDA build hard-fails: core/src/./feature/cuda/integer_adm/adm_dwt2.cu:23:10: fatal error: cuda/integer_adm_cuda.h: No such file or directory. The neighbouring SYCL block already did this correctly with absolute meson.current_source_dir() paths, so the fix pattern was in-tree. The Windows pthread-shim include was relative for the same reason and is now absolute too.

Hunk (c) — AFFECTS-FORK. core/tools/test/meson.build registered test_vmaf_cuda_gpumask with only suite/timeout — no depends, no workdir — while the script invokes ./tools/vmaf. Meson only rebuilds the targets a selected test declares (mtest.py rebuild_deps(): "if not targets: return True"), so selecting it as a subset built nothing and the script died with exit status 127. Reproduced on a freshly configured, uncompiled CUDA build dir. The two neighbouring fork-added tests (test_vmaf_per_shot, test_vmaf_roi_high_bitdepth) had the same omission. All three now declare depends and workdir.

Netflix/vmaf#1551 (and the live third of #1422) — the MSVC clz shim¶

core/src/feature/compat_builtin.h carried the exact defective shim upstream Netflix/vmaf#1551 replaces — and it is fork-added, adopted from #1422's proposal, not inherited:

static inline int __builtin_clz(unsigned x)             { return (int)__lzcnt(x); }
static inline int __builtin_clzll(unsigned long long x) { return (int)__lzcnt64(x); }

LZCNT encodes as F3 0F BD. On an x86-64 without ABM/LZCNT (Intel Nehalem through Ivy Bridge; AMD pre-Barcelona) the F3 prefix is ignored and the instruction retires as BSR, returning the index of the highest set bit instead of the leading-zero count. No fault, no diagnostic.

Two of the four call sites are on the generic scalar path, behind no SIMD gate:

CI cannot catch it: the MSVC leg is a real shipped path (it runs CPU unit tests and uploads install/bin/vmaf.exe as the windows-msvc-cuda-vmaf artifact), but every hosted Windows runner is LZCNT-capable, so the shim tests correct there and the divergence only appears on a user's older machine. No runtime gate and no /arch baseline exists in the tree.

Fix. _BitScanReverse / _BitScanReverse64, which are BSR by definition and present on every x86-64 part, plus a _M_X64 || _M_IX86 architecture test on the guard (neither intrinsic exists on MSVC ARM64, so that leg previously failed to compile) and a 32-bit fallback path. The names stay __builtin_clz*: the four call sites are upstream-verbatim and the rebase story depends on the spelling (ADR-0141 §2).

Do not adopt the __lzcnt form of Netflix/vmaf#1422 — that is the defect, and #1551 is upstream's own retraction of it. This is recorded inline in the header and enforced by scripts/ci/check-msvc-clz-shim.sh.

Regression tests. scripts/ci/check-msvc-clz-shim.sh (registered as a fast-suite meson test) fails on the pre-fix header and passes after; it also scans the rest of core/src so the intrinsic cannot come back elsewhere. core/test/test_compat_clz.c unit-tests the 31 - msb / 63 - msb arithmetic — the part the __lzcnt form got wrong — on every platform, and on the MSVC legs exercises the real shim bodies.

The other two thirds of #1422 are dead here: the integer_vif.c pointer-typing hunk was already rewritten fork-side (vif_buffers_alloc carves the aligned allocation with an explicit uint8_t *data and per-field casts, and its block comment names the MSVC C2036 problem), and the HAVE_UNISTD_H / HAVE_DIRECT_H / HAVE_STRUCT_TIMESPEC meson feature-detection hunk is NOT-APPLICABLE — the fork solved the same portability problem by point-gating the includes (rebase-notes round-21 items (l)/(m)).

Correction — the guard excluded ARM64 on a false premise¶

The first form of this fix carried the architecture test _MSC_VER && !__clang__ && (_M_X64 || _M_IX86), documented in both the header and the CI gate as necessary because "__lzcnt and _BitScanReverse are x86-only". That premise is wrong. Per the MSVC intrinsics reference:

Only __lzcnt is genuinely x86-only. The consequence of the wrong premise is that MSVC ARM64 matched no branch: this header is the sole definition of __builtin_clz for integer_adm.c and integer_vif.h, which sit on the generic scalar path and are compiled for every target, so the leg failed to compile outright rather than falling back to anything. The fork runs no MSVC ARM64 CI leg, so the break was latent.

The allowlist now covers every architecture MSVC targets, selects _BitScanReverse64 on x64/ARM64 and the two-step 32-bit reconstruction elsewhere, and scripts/ci/check-msvc-clz-shim.sh asserts the ARM64 arm specifically — a narrowed allowlist now fails the gate, verified by negative test alongside the existing __lzcnt negative test.

Netflix/vmaf#1178 — C++ runtime missing from Libs.private¶

core/src/meson.build built libvmaf_private_libs as [thread_lib, math_lib] and only extended it for SYCL and the Metal frameworks. Every generated libvmaf.pc in this tree read:

Libs: -L${libdir} -lvmaf
Libs.private: -pthread -lm

The fork is far more exposed than upstream: the undefined symbols come not just from vendored svm.cpp but from the fork's own C++ conversions (luminance_tools.cpp, feature_extractor.cpp, feature_collector.cpp, log.cpp, read_json_model.cpp, the C++23 picture pools, ...).

Reproduced with the existing static archive:

$ gcc t.c -I core/include build/src/libvmaf.a -pthread -lm -o t     # == Libs + Libs.private
/usr/bin/ld: undefined reference to `operator new(unsigned long)'
/usr/bin/ld: undefined reference to `operator delete(void*, unsigned long)'
...
$ gcc t.c -I core/include -L build/src $(pkg-config --static --libs libvmaf) -o t   # after the fix
GOOD LINK OK

Corroboration that this already bit a real consumer: docs/adr/0198-volk-priv-remap-static-archive.md:130-133 records the BtbN-style fully-static FFmpeg reproducer with -lstdc++ added by hand, precisely because libvmaf.pc did not carry it.

Upstream's patch cannot be ported: else compiler.get_id() == 'clang': is not meson syntax, and its clang→-lc++ mapping is wrong on Linux, where clang defaults to libstdc++. The fork detects the STL actually in use (_LIBCPP_VERSION via cxx.get_define) instead of keying off the compiler id, and skips MSVC/clang-cl, which auto-link the runtime with #pragma comment(lib).

Regression test. The CI step "Verify static pkgconfig" was grep-only; it never attempted a link. It now compiles a 3-line C consumer with the C driver against exactly what pkg-config --static --libs libvmaf reports — the test that actually reproduces the downstream FFmpeg failure — and keeps the -lm / -pthread / C++-runtime greps as cheap extras.

Note: the ORT dependency the candidate flagged as an "adjacent gap" is already present in Libs.private on a DNN-enabled configure; the C++ runtime was the only missing element.

CLAUDE.md §12 r14 check: no ffmpeg-patches/ update is required. The change adds no public C-API entry point, no configure flag, no LIBVMAFContext field, and renames no symbol the check_pkg_config probes look for — the existing probes simply start succeeding under --pkg-config-flags=--static.

Round-4 review findings — measurements¶

The Metal motion mirror: which dimensions actually break¶

The round-3 guard (w < 3 || h < 3) was derived from the 5-tap radius, but the kernels do not index a 5-tap neighbourhood directly — they load a TILE_W x TILE_H = 20x20 threadgroup tile at origin bid * 16 - HALF_FW, so the mirror helper is handed idx in [-2, 16*bid + 17]. A single bounce (2 * (sup - 1) - idx) only lands in range when idx <= 2 * (sup - 1).

Enumerated over the real tile span for every dimension (all workgroups, all 400 tile elements):

The 17 case is the one no radius-derived guard would predict, and it is why the fix belongs in the kernel rather than in the host-side floor. Replacing the single bounce with an iterative fold was verified over dims 1..299:

• never out of range, always terminating (sup <= 1 short-circuits);
• bit-identical to the single bounce for every index one bounce already handled, so no in-contract score moves.

Reflection convention: Metal v2 was the last backend still diverging¶

The ADM kernels' 2 * sup - idx - 1 is correct and deliberately untouched: ADM uses whole-sample reflection, matching adm_tools.c::dwt2_src_indices_filt_s, CUDA's calculate_indices() and the SYCL twin. Convention differs per metric; it must be checked per metric.

float_vif minimum dimension across backends¶

All four now call vif_get_min_dim(). This required an extern "C" guard on vif_tools.h, which had none — it was included only by C translation units, so the C++ (SYCL) and Objective-C++ (Metal) callers would otherwise have demanded mangled symbols.

Finding not upheld: the Libs.private libc++ detection¶

The review held that cxx.get_define('_LIBCPP_VERSION', ...) ignores an explicit -stdlib=libc++. Tested against the installed meson with a probe project:

project('probe', 'cpp')
cxx = meson.get_compiler('cpp')
message('FOO=[' + cxx.get_define('FOO') + ']')

meson setup b -Dcpp_args=-DFOO=42 reports FOO=[42], so compiler checks do observe the project's cpp_args; the _LIBCPP_VERSION probe therefore sees -stdlib=libc++ exactly as its comment claims. No change made.

Confirmed but deferred¶

Each of these is real and located; each has a docs/state.md row. None is in this PR, and the reason is given.

Reproducer commands¶

# CPU-only build used throughout
meson setup build core -Denable_cuda=false -Denable_sycl=false -Denable_dnn=disabled
meson compile -C build -j 8

# The regression tests added by this harvest
meson test -C build --suite=fast -j 4

# The two convolution defects, isolated (fails before the fix, passes after)
./build/test/test_convolution_edge_small

# The clz shim guard (fails on the pre-fix header)
bash scripts/ci/check-msvc-clz-shim.sh

# The pkg-config static-link defect
PKG_CONFIG_PATH=$PWD/build/meson-private pkg-config --static --libs libvmaf
# pre-fix:  -L/usr/local/lib -lvmaf -pthread -lm
# post-fix: -L/usr/local/lib -lvmaf -pthread -lm -lstdc++

# Netflix golden gate — must not move
CUDA_VISIBLE_DEVICES= make test-netflix-golden

Golden values (unchanged)¶

Float VMAF v0.6.1, build/tools/vmaf, on the three canonical CPU pairs:

make test-netflix-golden: 271 passed, 12 skipped, 0 failed.