Repository navigation
fix: 9 critical/high round-3 bug bundle — SYCL dispatch + ADM/VIF dim guards + MCP concurrency + CUDA flush + docs - #855
Merged
Conversation
…dispatch threaded_extract_batch_func skipped CUDA-flagged extractors but not SYCL-flagged ones, causing every SYCL extractor to run once via the SYCL command-graph path and once via the CPU thread pool. This corrupts the feature-collector state and produces incorrect scores when --backend sycl is active. read_pictures_should_skip already guards both CUDA and SYCL (line 2283), so the pool-side guard was simply missing. This adds the symmetric check, matching the existing CUDA pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The 4-level DWT2 pipeline in integer_adm requires at least 17 pixels in each dimension. Frames where min(w,h) <= 16 previously caused a SIGSEGV by walking off the end of the scratch buffers during the first decomposition step. Add an early-return guard at the top of init() that rejects such inputs with -EINVAL and a descriptive log message, matching the pattern already used by float_ms_ssim (ADR-0153) and integer_motion (Research-0094). Add test_integer_adm_min_dim covering: - Rejection of 8x8, 16x16, 16x17, 17x16 (all < 17 in at least one dim) - Acceptance of 17x17 (exact minimum) and 576x324 (standard test frame) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n OOB
The 17-tap separable Gaussian at VIF scale 0 uses a reflect-101
mirror-padding formula: ii = 2*h - ii - 2. For h < 9 (half-width = 8,
worst-case index = h+7, mirrored = h-9 < 0) this produces a negative
array index, causing a heap-buffer-overflow read and a double-free in
close() (the vif_buf scratch pointer ends up corrupt).
Add two guards in float_vif init():
1. Raw-dimension guard (w < 9 || h < 9) fires before any string-option
access, providing an unconditional early exit.
2. Scaled-dimension guard (scaled_w < 9 || scaled_h < 9) fires after
prescale is applied, covering the case where vif_prescale < 1.0
shrinks a larger frame below the safe floor.
Both return -EINVAL with a human-readable log message.
Add log.h include (required for vmaf_log calls).
Add test_float_vif_min_dim to the fast suite: 4 rejection cases
(1x1, 8x8, 64x8, 8x64) and 2 acceptance cases (9x9 exact minimum,
576x324 Netflix golden resolution). The test helper applies option
defaults via vmaf_option_set before calling init() so that the
vif_prescale_method string field is not NULL for the acceptance path.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three bugs in docs/api/gpu.md relative to the canonical header and implementation in core/include/libvmaf/libvmaf_cuda.h and core/src/cuda/common.c: 1. Wrong signature: docs declared `void vmaf_cuda_state_free(VmafCudaState **state)` (void return, double-pointer). Correct form per header is `int vmaf_cuda_state_free(VmafCudaState *cu_state)` (int return, single pointer). All in-tree test callers (test_cuda_*_parity.c) use the correct single-pointer form. 2. Wrong ownership claim: docs stated "context owns the state after import; vmaf_close() frees it" and implied vmaf_cuda_state_free() was only an escape hatch for the pre-import path. In reality, vmaf_cuda_import_state() copies the struct *by value* into the VmafContext (see libvmaf.c:347 `vmaf->cuda.state = *cu_state`). vmaf_close() tears down the embedded copy; the caller must always call vmaf_cuda_state_free() on the original pointer after vmaf_close() to release the heap allocation from vmaf_cuda_state_init(). The implementation comment in common.c:276-288 and the AGENTS.md invariant note at core/src/cuda/AGENTS.md:75-85 are authoritative. 3. Wrong complete example: the code example omitted vmaf_cuda_state_free() after vmaf_close() and labelled vmaf_close() as "also frees the CUDA state". Fixed to call both in the correct order. Also removed the double-pointer form `vmaf_cuda_state_free(&cuda)` from the inline escape-hatch snippet; the function takes a single pointer. No header, C source, or test changes — docs only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… mkdtemp leak
Add `_SCORE_SEM = asyncio.Semaphore(int(os.environ.get("VMAF_MCP_MAX_CONCURRENT", 8)))`
at module level and wrap the subprocess-spawn region of `_run_vmaf_score` with
`async with _SCORE_SEM:` so unbounded concurrent MCP tool calls cannot exhaust
memory by spawning unlimited vmaf processes simultaneously.
Also replace the raw `tempfile.mkdtemp(prefix="vmaf-mcp-worst-")` + `finally: pass`
in `_describe_worst_frames` with `tempfile.TemporaryDirectory`, which guarantees
the PNG scratch directory is cleaned up on context exit instead of leaking on
every call.
Update `test_describe_worst_frames_allocates_unique_tmpdir_per_call` to match the
new cleanup behaviour (PNGs are gone after the call returns), and add
`test_score_sem_limits_concurrent_vmaf_subprocesses` which spawns 16 concurrent
`_run_vmaf_score` calls and asserts the peak concurrency never exceeds 8.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…active flush_context_threaded flushes every TEMPORAL extractor (including those also flagged VMAF_FEATURE_EXTRACTOR_CUDA) in its first loop. flush_context then unconditionally calls flush_context_cuda, which called vmaf_feature_extractor_context_flush a second time on the same extractors. The duplicate write into feature_collector returned -EINVAL, which propagated into cuCtxSynchronize and produced a spurious "context could not be synchronized" error when --threads >= 1 with the CUDA backend. Guard the flush call in flush_context_cuda: when vmaf->thread_pool is set and the extractor carries the TEMPORAL flag, skip the flush (it already ran in flush_context_threaded). The gpu_pending collect and cuCtxSynchronize are not guarded — they still run unconditionally to drain any last-frame work and ensure a clean device state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… user-facing docs The meson `enable_dnn` option is type=feature (not boolean), so valid values are enabled/disabled/auto. Six user-facing doc files were using the invalid `true` / `false` forms, which fail `meson setup`: - docs/api/dnn.md (lines 21, 393): -Denable_dnn=true → -Denable_dnn=enabled - docs/usage/vmaf-roi.md (line 41): -Denable_dnn=false → -Denable_dnn=disabled - docs/ai/models/fastdvdnet_pre.md (line 148): same - docs/ai/models/transnet_v2.md (line 124): same - docs/ai/overview.md (line 53): -Denable_dnn=false → -Denable_dnn=disabled - docs/api/index.md (line 109): same ADR bodies (ADR-0374, ADR-0660 — both Accepted/frozen) and historical research digests were left unchanged intentionally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Explains why CPU extractors with a thread pool are skipped in the serial loop (they are handled by threaded_extract_batch_func), while CUDA and SYCL extractors must NOT be skipped here — omitting one side of that invariant would silently leak GPU frames. Companion to the libvmaf-sycl-double-dispatch-skip fix; comment-only, no behaviour change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, VmafHipConfiguration, and cross-link to backends/hip The HIP section previously described the API in prose bullet points and contained a stale note claiming vmaf_hip_import_state returns -ENOSYS (fixed in ADR-0519). The CUDA Limitations section still contained a three-line bullet saying the HIP backend was "in flight" (T7-10 / PR #200). Changes: - Remove the stale "No HIP path in this header" bullet from the CUDA Limitations section (lines 210-213). - Replace the bare HIP prose section with a structured reference section mirroring the Metal section: ### Header, ### Core lifecycle API (table), ### State (C struct + function signatures), ### Ownership, ### Typical call sequence, ### Limitations and current feature coverage. - Document vmaf_hip_available() return semantics and VmafHipConfiguration fields (device_index, flags) from core/include/libvmaf/libvmaf_hip.h. - Note that HIP uses the caller-owns-state model (like SYCL) rather than context-owns-state (like CUDA), and explain the rationale. - Add ../backends/hip/overview.md cross-link to the Related section. - Remove the incorrect vmaf_hip_import_state -ENOSYS claim; the function has been fully implemented since ADR-0519. Local verify: mkdocs build --strict exits 0 (118 s build, no new errors). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| if (s->scaled_w < 9 || s->scaled_h < 9) { | ||
| vmaf_log(VMAF_LOG_LEVEL_ERROR, | ||
| "float_vif requires scaled width >= 9 and height >= 9 (got %dx%d)\n", s->scaled_w, |
| if (s->scaled_w < 9 || s->scaled_h < 9) { | ||
| vmaf_log(VMAF_LOG_LEVEL_ERROR, | ||
| "float_vif requires scaled width >= 9 and height >= 9 (got %dx%d)\n", s->scaled_w, | ||
| s->scaled_h); |
9 of 11 tasks
lusoris
added a commit
that referenced
this pull request
Jun 8, 2026
…dconfig (#860) Two CI failures from PR #855 tip (765af26): 1. test_run_vmaf_runner_local_explainer_with_bootstrap_model asserted VMAF_LE_score at places=4 (5e-5 tolerance) with a pre-NEON-fix value. After PR #834 corrected the neon_any_nonzero_s32 uint64-truncation bug, macOS arm64 Apple libm produces 75.40974... vs Linux 75.40980... (~6e-5 delta). Recalibrate to 75.40974269371469 and relax to places=3 per the ADR-0418 pattern used by all other bootstrap assertions in the same file. 2. Dockerfile was missing RUN ldconfig after make install. The NVIDIA CUDA Ubuntu 24.04 base image omits /usr/local/lib/x86_64-linux-gnu from /etc/ld.so.conf; meson strips RPATH on install; without ldconfig the vmaf binary cannot find libvmaf.so.3 at runtime and exits silently, producing zero smoke-test stdout. Closes T-LOCAL-EXPLAINER-BOOTSTRAP-NEON-RECAL-2026-06-08 Closes T-DOCKERFILE-LDCONFIG-MISSING-2026-06-08 Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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
Bundle of 9 critical/high fixes from round-3 hunt:
Test plan
Deep-dive deliverables (ADR-0108)
state.md touch