fix(core): bundle of core C bugfixes (#148 errno + #307 retval checks + #308 ENOSYS stubs + #316 dispatch token-boundary) by lusoris · Pull Request #525 · VMAFx/vmafx

lusoris · 2026-06-01T19:41:43Z

Summary

Bundles four open DRAFT PRs (#148, #307, #308, #316) into a single reviewable
unit. Each source PR has been rebased onto master tip (40d192e) and applied
via git am --3way; additive conflicts in docs/rebase-notes.md and AGENTS.md
files were resolved with the union strategy per the 2026-06-01 triage directive.
Source PRs are closed as superseded.

Included fixes:

fix(core): propagate correct errno codes — PR #125 defects #148 — errno propagation in vmaf_write_output_with_format, vmaf_cuda_state_init, vmaf_close, vmaf_cuda_preallocate_pictures, vmaf_init (CERT ERR33-C).
fix(tools): check fseek / vmaf_picture_alloc / vmaf_read_pictures returns in vmaf_bench #307 — unchecked fseek / vmaf_picture_alloc / vmaf_read_pictures returns in vmaf_bench (JPL Power-of-10 r7).
fix(core): emit -ENOSYS stubs for libvmaf_hip.h / libvmaf_metal.h when backend OFF #308 — -ENOSYS stubs for libvmaf_hip.h / libvmaf_metal.h entry points when the backend is disabled at build time.
fix(gpu-dispatch): enforce token boundary on strategy-name match #316 — token-boundary enforcement on strategy-name matching in GPU dispatch to prevent partial-string false positives.

Type

fix — bug fix

Checklist

Commits follow Conventional Commits (the commit-msg hook enforces this).
make format && make lint is green locally.
Unit tests pass: meson test -C build.
If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2.
If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md).
If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below.
If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt — do not edit docs/adr/README.md directly.

Bug-status hygiene (ADR-0165)

docs/state.md updated in this PR with a row
in the appropriate section (Open / Recently closed / Confirmed
not-affected / Deferred), OR no state delta: bundle replaces source PRs — no new bug rows.

Netflix golden-data gate (ADR-0024)

I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
If I believe a golden value must change, I have explained why below AND pinged @lusoris for a CODEOWNERS exception.

Cross-backend numerical results

errno-only / stub / dispatch changes — no algorithm delta; ADR-0214 parity gate unaffected

Performance (if `perf` or `feat`)

Not applicable — error-path-only changes.

Deep-dive deliverables (ADR-0108)

Research digest — no digest needed: these are straightforward bugfixes with no design alternatives; each source PR carried a pre-push gate pass and a code-review rationale.
Decision matrix — no alternatives: only-one-way fix; the correct behavior (propagate errno, return -ENOSYS on missing backend, enforce token boundary) is non-negotiable per CERT ERR33-C and JPL Power-of-10 r7.
AGENTS.md invariant note — updated core/src/hip/AGENTS.md and core/src/metal/AGENTS.md to document stubs.c and the meson wiring; no rebase-sensitive invariants for the errno or dispatch changes.
Reproducer / smoke-test command — see Reproducer section below.
CHANGELOG fragment — changelog.d/fixed/bundle-core-c.md added in this PR.
Rebase note — entries for pr125-errno-defects, vmaf-bench-unchecked-returns, and HIP/Metal -ENOSYS stubs added to docs/rebase-notes.md (entries were included in the applied patches from source PRs).

Reproducer

# #148 errno propagation — verify vmaf_cuda_state_init returns -ENOSYS on no-CUDA host
meson test -C build --suite=fast

# #307 unchecked returns — exercise vmaf_bench
build/tools/vmaf_bench --help

# #308 ENOSYS stubs — CPU-only build still links without HIP/Metal
meson setup build-cpu -Denable_cuda=false -Denable_hip=false -Denable_metal=false && ninja -C build-cpu

# #316 dispatch token boundary — run fast test suite
meson test -C build --suite=fast

Known follow-ups

None. Each fix is self-contained.

Breaking changes / migration

None.

Supersedes PRs #148, #307, #308, #316 — source branches preserved, PRs closed with comment.

Five actionable errno defects from the PR #125 code review: 1. vmaf_write_output_with_format: capture errno immediately after open(2) / fdopen(3) failure; return -errno instead of hardcoded -EINVAL so callers receive the OS-precise error code. 2. vmaf_cuda_state_init: map driver-library-missing → -ENOSYS and cuInit(0) failure → -ENODEV, mirroring the SYCL/HIP pattern already established in sycl/common.cpp and cuda/cuda_helper.cuh. 3. vmaf_close: propagate return values from vmaf_thread_pool_wait() and vmaf_framesync_destroy() (CERT ERR33-C / Power-of-10 #7). NULL-pool case (n_threads == 0) correctly returns 0. 4. vmaf_cuda_preallocate_pictures: return -EBUSY on double-call to prevent silent ring-buffer leak and in-flight picture corruption. 5. vmaf_init: return the actual sub-init error code rather than a hardcoded -ENOMEM for every error path. Fast test suite: 49/49. Pre-commit: all checks pass. ADR-0214 parity gate: unaffected (errno-only changes, no algorithm delta). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…urns in vmaf_bench Discharge three S9 (JPL Power-of-10 r7) unchecked-return findings in `core/tools/vmaf_bench.c` surfaced by the 2026-05-30 audit. Independent of PR #304 (the vmaf.c/vmaf_bench.c CUDA/SYCL state-leak fix); no overlapping hunks. * `yuv_pair_read_frame` — the two `fseek` calls were `(void)`-cast, silencing the lint but hiding the semantic bug: a failed `fseek` leaves the FILE position undefined, after which `fread` silently feeds the wrong bytes into the benchmark. Now checks each `fseek` and returns `-EIO` with a `perror` diagnostic on failure. * `run_sycl_gpu_profile` per-frame loop — `vmaf_picture_alloc` returns were discarded; on allocation failure the subsequent `yuv_pair_read_frame -> ref->data[0]` dereference would crash on a sentinel-zero `VmafPicture`. Now captures the return code, logs, unrefs any already-allocated sibling, and breaks the loop. * `run_sycl_gpu_profile` end-of-stream block — the final `vmaf_read_pictures(vmaf, NULL, NULL, 0)` flush surfaces pooling / aggregation errors via its int return that the previous code discarded; now captured and propagated. `printf` / `vmaf_close` returns explicitly `(void)`-cast to match the surrounding file convention. Adds `#include <errno.h>` for `EIO`. No behavior change on success paths. Builds clean (`meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false && ninja -C build-cpu`); clang-tidy on the touched file produces no new warnings vs master (line numbers shift only).

…n backend OFF Both `core/include/libvmaf/libvmaf_hip.h` and `core/include/libvmaf/libvmaf_metal.h` document the contract that every public entry point returns `-ENOSYS` when libvmaf is built without the relevant backend. The real bodies in `core/src/hip/common.c`, `core/src/metal/common.mm`, `core/src/metal/picture_import.mm`, and the `vmaf_hip_import_state` / `vmaf_metal_import_state` / `vmaf_metal_read_imported_pictures` definitions inside `core/src/libvmaf.c` all sit behind `#ifdef HAVE_HIP` / `#ifdef HAVE_METAL`, so a default `-Denable_hip=false -Denable_metal=disabled` build emitted none of those symbols into `libvmaf.so` and any downstream link that referenced them failed. Fix: add `core/src/hip/stubs.c` + `core/src/metal/stubs.c` that mirror the canonical `core/src/dnn/dnn_api.c` `VMAF_HAVE_DNN` stub pattern and wire each TU into `libvmaf_feature_static_lib` via `hip_sources` / `metal_sources` only when the backend is disabled. The stubs return `-ENOSYS`, set out-params to NULL on the pointer-returning entry points, and `vmaf_hip_available()` / `vmaf_metal_available()` correctly return 0. Verified locally with: meson setup build-cpu core -Denable_hip=false -Denable_metal=disabled \\ -Denable_cuda=false -Denable_sycl=false \\ -Denable_dnn=disabled ninja -C build-cpu nm -D build-cpu/src/libvmaf.so | grep -E "vmaf_(hip|metal)_" all 14 documented public symbols are present (T-type) and a smoke main linking against libvmaf.so observes -ENOSYS / NULL out-params on every entry point and 0 from the availability probes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The shared VMAF_<BACKEND>_DISPATCH env-variable parser in core/src/gpu_dispatch_parse.h matched strategy names with strncmp(v, strategy_names[idx], slen) and accepted any string with the strategy name as a prefix. So VMAF_CUDA_DISPATCH=feature:directx silently routed to the valid "direct" strategy instead of being treated as unknown. A typo in any backend's dispatch env-var was indistinguishable from a valid override at the parse layer. Add a token-boundary check after the strncmp: the byte at v[slen] must be one of '\0', ',', '\n', ' ', or '\t' for a match to succeed. The terminator set mirrors the grammar documented in the header's leading doc comment plus newline (for env values read line-by-line in tests). Adds core/test/test_gpu_dispatch_parse.c (9 cases) wired into core/test/meson.build under the `fast` suite. The negative-control (prefix-without-boundary) cases fail against the previous code and pass against the fix, so any regression is caught locally and in CI. **Research digest** — no digest needed: trivial bug fix with a small, targeted reproducer. **Decision matrix** — no alternatives: only-one-way fix. The terminator set is mechanically derived from the doc-commented grammar plus '\n' for line-buffered tests. **AGENTS.md invariant note** — no rebase-sensitive invariants: file is fork-added (ADR-0483) and absent in upstream. **Reproducer / smoke-test command** — meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \ -Denable_hip=false -Denable_metal=disabled -Denable_dnn=disabled ninja -C build-cpu test/test_gpu_dispatch_parse -j4 meson test -C build-cpu --no-rebuild test_gpu_dispatch_parse **CHANGELOG fragment** — changelog.d/fixed/gpu-dispatch-parse-strict-match.md **Rebase note** — docs/rebase-notes.md entry gpu-dispatch-parse-strict-strategy-match (2026-05-30). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR bundles four previously-drafted fixes into a single set of core C correctness hardening changes: improved errno/error-code propagation, added return-value checks in vmaf_bench, ensured HIP/Metal public APIs still link when those backends are disabled (via -ENOSYS stubs), and tightened GPU dispatch strategy parsing to require token-boundary matches.

Changes:

Propagate accurate OS/library error codes (errno preservation, -ENOSYS/-ENODEV mappings, cleanup error propagation, -EBUSY double-call guard).
Harden vmaf_bench by checking previously-ignored error returns and adding a dedicated dispatch-parser regression test.
Emit HIP/Metal public API stubs when backends are disabled, and wire them via Meson; document the wiring in AGENTS and rebase notes/changelog.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
docs/rebase-notes.md	Adds rebase-tracking entries for the bundled fixes (includes one placeholder to clean up).
core/tools/vmaf_bench.c	Adds missing return checks for `fseek`, `vmaf_picture_alloc`, and flush return handling.
core/test/test_gpu_dispatch_parse.c	New regression tests to enforce strict token-boundary strategy matching.
core/test/meson.build	Builds/registers the new `test_gpu_dispatch_parse` in the fast suite.
core/src/metal/stubs.c	Adds `-ENOSYS` Metal stubs when Metal backend is not built.
core/src/metal/AGENTS.md	Documents the new Metal stubs TU and its Meson wiring location.
core/src/meson.build	Wires HIP/Metal stub TUs into builds when those backends are disabled.
core/src/libvmaf.c	Improves error propagation in init/close/write-output and adds CUDA prealloc double-call guard.
core/src/hip/stubs.c	Adds `-ENOSYS` HIP stubs when HIP backend is not built.
core/src/hip/AGENTS.md	Documents the new HIP stubs TU and its Meson wiring location.
core/src/gpu_dispatch_parse.h	Enforces token-boundary match after `strncmp` for dispatch strategies.
core/src/cuda/common.c	Refines CUDA init error mapping to `-ENOSYS`/`-ENODEV` for better caller behavior.
changelog.d/fixed/vmaf-bench-unchecked-returns.md	Changelog entry for `vmaf_bench` return-check hardening.
changelog.d/fixed/pr125-errno-defects.md	Changelog entry for errno/error-code propagation fixes.
changelog.d/fixed/hip-metal-enosys-stubs.md	Changelog entry for HIP/Metal `-ENOSYS` stub emission when disabled.
changelog.d/fixed/gpu-dispatch-parse-strict-match.md	Changelog entry for strict dispatch strategy matching.
changelog.d/fixed/bundle-core-c.md	Aggregate changelog entry for the bundled fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    err = vmaf_read_pictures(vmaf, NULL, NULL, 0);
+    if (err)
+        (void)fprintf(stderr, "vmaf_read_pictures(flush) failed (err=%d)\n", err);
+    (void)vmaf_close(vmaf);
+    return err;


+
+## HIP/Metal -ENOSYS stubs for public API (2026-05-30)
+
+no rebase impact: REASON — all touched code is fork-local. The two


+        /* Capture errno immediately — it is clobbered by fprintf(3). */
+        const int open_errno = errno;
+        (void)fprintf(stderr, "could not open file: %s\n", output_path);
+        return -open_errno;


+        /* Same: capture before fprintf clobbers errno. */
+        const int fdopen_errno = errno;
+        (void)fprintf(stderr, "could not open file: %s\n", output_path);
 #ifdef _WIN32
        (void)_close(outfd);


lusoris and others added 5 commits June 2, 2026 16:13

chore(changelog): add bundle-core-c fragment for PRs #148 #307 #308 #316

d0b66b0

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

lusoris force-pushed the fix/core-c-bugfix-bundle-148-307-308-316 branch from c38aca0 to d0b66b0 Compare June 2, 2026 14:14

lusoris marked this pull request as ready for review June 2, 2026 14:14

Copilot AI review requested due to automatic review settings June 2, 2026 14:14

Copilot started reviewing on behalf of lusoris June 2, 2026 14:14 View session

lusoris merged commit f6e25f1 into master Jun 2, 2026
59 of 81 checks passed

lusoris deleted the fix/core-c-bugfix-bundle-148-307-308-316 branch June 2, 2026 14:14

Copilot AI reviewed Jun 2, 2026

View reviewed changes

lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026

Navigation Menu

Repository navigation

fix(core): bundle of core C bugfixes (#148 errno + #307 retval checks + #308 ENOSYS stubs + #316 dispatch token-boundary) - #525

fix(core): bundle of core C bugfixes (#148 errno + #307 retval checks + #308 ENOSYS stubs + #316 dispatch token-boundary)#525
lusoris merged 5 commits into
masterfrom
fix/core-c-bugfix-bundle-148-307-308-316

lusoris commented Jun 1, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		## HIP/Metal -ENOSYS stubs for public API (2026-05-30)

		no rebase impact: REASON — all touched code is fork-local. The two

Conversation

lusoris commented Jun 1, 2026

Summary

Type

Checklist

Bug-status hygiene (ADR-0165)

Netflix golden-data gate (ADR-0024)

Cross-backend numerical results

Performance (if perf or feat)

Deep-dive deliverables (ADR-0108)

Reproducer

Known follow-ups

Breaking changes / migration

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Performance (if `perf` or `feat`)