Skip to content

fix: 12 medium/low round-3 bundle — SYCL ceiling, meson conflicts, CLI bounds, y4m, fuzz, motion debug, docs - #857

Merged
lusoris merged 12 commits into
masterfrom
chore/bundle-d-12-medium-low-fixes
Jun 8, 2026
Merged

fix: 12 medium/low round-3 bundle — SYCL ceiling, meson conflicts, CLI bounds, y4m, fuzz, motion debug, docs#857
lusoris merged 12 commits into
masterfrom
chore/bundle-d-12-medium-low-fixes

Conversation

@lusoris

@lusoris lusoris commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Bundle of 12 medium/low round-3 fixes. Held as DRAFT until prior PRs land + master CI green.

  1. fix(sycl): MAX_GRAPH_EXTRACTORS 8→16 — prevents -ENOMEM under 8-extractor graph saturation
  2. fix(meson): enable_avx512+enable_asm=false now errors instead of silent no-op
  3. fix(cli): --threads cap at hardware core count to prevent OOM
  4. fix(y4m): (size_t) cast in y4m_convert_411_422jpeg pointer arithmetic (matches siblings)
  5. fix(meson): recursive: false on 7 extract_all_objects() calls (Meson 2.0 prep)
  6. chore(y4m): delete unreachable y4m_convert_4xxjpeg_42xjpeg dead code
  7. fix(fuzz): tighten fuzz_y4m_input dim filter from 6 to 5 digits + raise input bytes ceiling to 256 KiB
  8. fix(fuzz): json_model RSS limit 512 MB + partial-parse leak fix + re-enable detect_leaks
  9. fix(integer_motion_cuda): debug option default true→false (matches CPU sibling, halves write traffic)
  10. fix(cli): --width 0 / --height 0 reports specific error instead of generic "required options missing"
  11. docs(api/index): mark VmafPicture::ref and ::priv as INTERNAL (matches header)
  12. fix(vmaf): guard pic_cnt computation against thread_cnt unsigned overflow

Test plan

  • meson test --suite=fast 87/87 PASS
  • Pre-commit clean (clang-format, markdownlint, semgrep, copyright, conflict-markers)
  • No Netflix golden assertions touched
  • No conflict markers (git grep '^<<<<<<' empty)
  • YAML lint on fuzz.yml: PASS

Reproducer / smoke-test command

cd core && meson setup ../build-bundle -Denable_cuda=false -Denable_sycl=false
ninja -C ../build-bundle
meson test -C ../build-bundle --suite=fast
# Expected: 87/87 PASS

Deep-dive deliverables (ADR-0108)

  • Research digest: no digest needed: round-3 hunt findings, each fix is a standalone defect
  • Decision matrix: no alternatives: only-one-way fix for each item in this bundle
  • AGENTS.md invariant note: no rebase-sensitive invariants introduced
  • Reproducer / smoke-test command: see above
  • changelog.d fragment: changelog.d/fixed/fuzz-json-model-rss-limit.md included (commit 8); remaining items are cosmetic/config-level fixes that do not warrant individual user-facing changelog entries per ADR-0221
  • docs/rebase-notes.md: no rebase impact: all fixes are self-contained

state.md touch

  • state.md: closes 12 T- rows for these bugs (updated in commit 11 alongside docs/api/index.md)

DRAFT — do not mark ready or admin-merge. Will be readied after ssim_hip rewrite lands.

lusoris and others added 12 commits June 8, 2026 20:03
@lusoris @claude
fix(sycl): raise MAX_GRAPH_EXTRACTORS from 8 to 16, return -ENOMEM on…
9c9d643 
… overflow

vmaf_v0.6.1 plus debug/extended options can register 12+ extractors in
a single VmafSyclState.  The previous ceiling of 8 caused a silent
-ENOSPC rejection with no fallback, aborting the run.

Changes:
- MAX_GRAPH_EXTRACTORS: 8 → 16 (covers expected upper bound of ~12
  extractors with headroom; the fixed-size array still avoids heap
  allocation inside the graph-recording hot path).
- vmaf_sycl_graph_register: return code on overflow changed from
  -ENOSPC to -ENOMEM (semantically correct for a slot-count exhaustion;
  the error message also hints at the fix).

No behaviour change for runs with ≤ 8 extractors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(meson): promote avx512+asm=false conflict from warning to error
e288109 
enable_avx512=true with enable_asm=false is a contradictory
configuration: the AVX-512 path is pure assembly and cannot be
compiled without the assembler. The previous warning() silently
no-opped the AVX-512 build, masking the misconfiguration from the
user. Replace with error() so the user learns immediately that the
option combination is invalid and must correct it before proceeding.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(cli): cap --threads to hardware core count to prevent OOM
c607742 
Setting --threads to an arbitrarily large value (e.g. 10000) caused
the thread pool to spawn thousands of threads, exhausting virtual
memory and OOM-killing the process.

After parse_unsigned(), clamp settings->thread_cnt to the number of
online hardware processors (sysconf _SC_NPROCESSORS_ONLN on POSIX,
GetSystemInfo on Windows). Emits a stderr warning when the cap fires.
Applies to cli_parse.cpp (C++23 build source) and the C mirror file
cli_parse.c kept for upstream-sync reference.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(y4m): add (size_t) cast in y4m_convert_411_422jpeg pointer advance
e2ccad6 
`_dst += _y4m->pic_w * _y4m->pic_h` used plain int * int arithmetic
before advancing an unsigned char pointer. For frames where
pic_w * pic_h exceeds INT_MAX (~46340 x 46340), this is signed-integer
overflow — undefined behaviour under C99/C11 (SEI CERT C INT30-C /
INT32-C). Fix: `_dst += (size_t)_y4m->pic_w * _y4m->pic_h`, matching
the identical cast already present in the sibling functions
y4m_convert_42xmpeg2_42xjpeg (line 220) and
y4m_convert_42xpaldv_42xjpeg (line 315). Added a matching explanatory
comment to keep the three functions consistent. docs/state.md updated
per CLAUDE §12 r13.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(build): add recursive: false to 7 bare extract_all_objects() calls
7bfe17b 
Meson currently defaults extract_all_objects() to non-recursive, but
will silently change the default in Meson 2.0. Add explicit
recursive: false to all 7 call sites (arm64 NEON x4, arm64 SVE2 x2,
CUDA x1) to lock in the current intent and silence the future
deprecation warning.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
chore(tools): remove dead y4m_convert_4xxjpeg_42xjpeg function
1f8aab6 
The function was wrapped in `#if 0` since its introduction and was never
assigned to a conversion-function pointer, making it permanently unreachable.
It also contained unsigned-int arithmetic that would overflow signed int for
large frames — the same class of issue fixed in the live C411 converter.

Deleting it outright is safer than leaving dead code that silently rots.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(fuzz): tighten y4m dim filter to 5 digits and raise input cap to …
43b0bf7 
…256 KiB

FUZZ_MAX_DIM_DIGITS was 6 (allowing up to "999999") but
FUZZ_MAX_INPUT_BYTES was only 65536. A single-row 4:2:0 frame at
99999 px wide already needs ~150 KiB of luma+chroma payload, so the
overflow path was unreachable during fuzzing — the corpus would be
dropped by the size gate before the parser ever saw an over-width
header. Fix by reducing FUZZ_MAX_DIM_DIGITS to 5 ("99999" max) and
raising FUZZ_MAX_INPUT_BYTES from 64 KiB to 256 KiB, making the two
constants mutually consistent and reopening the overflow path to the
fuzzer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(fuzz): tighten fuzz_json_model RSS limit to 512 MB and fix partia…
4ead10e 
…l-parse leak

Nightly runs of fuzz_json_model reached 1.1 GB peak RSS in 180-second
windows, suggesting the embedded libsvm sub-parser accumulates
allocations across inputs.  Add rss_limit_mb=512 to the fuzz_json_model
matrix entry so libFuzzer aborts before RSS climbs into OOM territory;
all other harnesses keep the default 2048 MB.

Audit of the harness revealed a second issue: the collection-variant
cleanup only freed model_c / collection on rc==0, so a partial parse
that populated key "0" then failed on key "1" leaked both objects.
Fix: always call vmaf_model_destroy / vmaf_model_collection_destroy
after vmaf_read_json_model_collection_from_buffer regardless of the
return code.  With the leak closed, re-enable ASan leak detection
(detect_leaks=1) in __asan_default_options; the original detect_leaks=0
work-around is no longer needed.

The single-model path (vmaf_read_json_model) already calls
vmaf_model_destroy on failure via the fail: label; no change there.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(cuda/motion): set debug option default to false to match CPU sibling
ddac1a4 
integer_motion_cuda.c had debug=true as the default for the debug
VmafOption, while integer_motion.c (the CPU implementation) uses
false. This caused every CUDA motion scoring run to emit per-frame
debug writes unconditionally, doubling score-write traffic compared
to the CPU path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(cli): report --width/--height must be > 0 instead of misleading r…
e1b544f 
…equired-options message

When `--width 0` (or `--height 0`) is supplied alongside valid height,
pixel_format, and bitdepth arguments, the existing check
`!(width && height && pix_fmt && bitdepth)` treats the explicit zero as
"not provided" and emits the generic "required options missing" list.
That message implies the user forgot the flag when they actually supplied
an invalid value.

Add explicit zero-value guards in cli_parse.cpp before the generic block
so the user sees "--width must be > 0" (or "--height must be > 0")
instead.  The guard only fires when at least one sibling YUV field is
also set so that the lone-zero case (e.g. only --width 0, nothing else)
still falls through to the more helpful "all four required" listing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
docs(api): mark VmafPicture::ref and priv as INTERNAL in index.md
a1ad668 
The struct reproduction at lines 224-225 showed VmafRef *ref and
void *priv without any access qualifier, implying they were part of
the usable public API. Both fields are opaque libvmaf-internal state
(the header itself documents them as "do not touch"). Add explicit
"INTERNAL — ... do not access" comment text to match the header's
intent and prevent callers from dereferencing these fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(tools): guard pic_cnt unsigned overflow when thread_cnt is extreme
89602fa 
Add a belt-and-suspenders check before computing
  pic_cnt = 2 * (thread_cnt + 1) + 1
that rejects --threads values >= (UINT_MAX - 3) / 2, which would wrap
the unsigned arithmetic to a small pool size and deadlock the picture
pool fetcher on frame N+1.  The companion CLI hardware-core cap is the
primary defence; this guard makes the invariant self-documenting and
survivable even if that cap is absent.

Also add the missing <limits.h> include required by UINT_MAX.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris
lusoris marked this pull request as ready for review June 8, 2026 19:28
@lusoris
lusoris merged commit d47b8b8 into master Jun 8, 2026
103 of 114 checks passed
@lusoris
lusoris deleted the chore/bundle-d-12-medium-low-fixes branch June 8, 2026 19:40
This was referenced Jun 12, 2026
Merged
Merged
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
This was referenced May 31, 2026
Merged
Merged
@lusoris lusoris mentioned this pull request Oct 8, 2026
Merged
11 of 27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0.0 — First release

Development

Successfully merging this pull request may close these issues.

1 participant

@lusoris