Repository navigation
Conversation
…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).
lusoris
marked this pull request as draft
May 30, 2026 14:42
lusoris
marked this pull request as ready for review
May 31, 2026 13:25
lusoris
marked this pull request as draft
May 31, 2026 13:54
lusoris
marked this pull request as ready for review
May 31, 2026 14:01
Contributor
Author
|
Closing as part of marathon cleanup 2026-05-31 (150 PRs merged today). Content likely superseded by sibling merges. Reopen if specific finding still needs work; bigger PRs preferred going forward per session feedback. |
Contributor
Author
|
Superseded by #525 — bundled per 2026-06-01 triage. |
lusoris
added a commit
that referenced
this pull request
Jun 2, 2026
lusoris
added a commit
that referenced
this pull request
Jun 2, 2026
#308 ENOSYS stubs + #316 dispatch token-boundary) (#525) * fix(core): propagate correct errno codes — PR #125 defects 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> * fix(tools): check fseek / vmaf_picture_alloc / vmaf_read_pictures returns 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). * fix(core): emit -ENOSYS stubs for libvmaf_hip.h / libvmaf_metal.h when 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> * fix(gpu-dispatch): enforce token boundary on strategy-name match 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> * chore(changelog): add bundle-core-c fragment for PRs #148 #307 #308 #316 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Lusoris <lusoris@pm.me>
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
Discharge three S9 (JPL Power-of-10 r7) unchecked-return findings in
core/tools/vmaf_bench.csurfaced by the 2026-05-30 audit. Independent of PR #304 (the vmaf.c/vmaf_bench.c CUDA/SYCL state-leak fix) — no overlapping hunks; verified by reading PR #304's diff before editing.yuv_pair_read_frame: the twofseekcalls were(void)-cast, silencing the lint but hiding the semantic bug — a failedfseekleaves the FILE position undefined, after whichfreadsilently feeds the wrong bytes into the benchmark. Now checks eachfseekand returns-EIOwith aperrordiagnostic on failure.run_sycl_gpu_profileper-frame loop:vmaf_picture_allocreturns were discarded; on allocation failure the subsequentyuv_pair_read_frame -> ref->data[0]dereference would crash on a sentinel-zeroVmafPicture. Now captures the return code, logs, unrefs any already-allocated sibling, and breaks the loop.run_sycl_gpu_profileend-of-stream block: the finalvmaf_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_closereturns explicitly(void)-cast to match the surrounding file convention.Adds
#include <errno.h>forEIO. No behavior change on success paths.Reproducer / smoke-test
The
run_sycl_gpu_profilefixes live behind#ifdef HAVE_SYCL; verifying them requires a SYCL toolchain build. Surrounding code patterns and signatures already validated againstcore/include/libvmaf/picture.handcore/include/libvmaf/libvmaf.h.ADR-0108 deliverables checklist
(void)-cast vs error-propagate tension was already resolved by JPL r7:(void)is only valid for pure-formatting calls (printf,fprintf); error-bearing returns (fseek,vmaf_picture_alloc,vmaf_read_pictures) must be checked.vmaf_bench.cis fork-added (no upstream Netflix counterpart intools/); no AGENTS.md surface change needed.changelog.d/fixed/vmaf-bench-unchecked-returns.md.docs/rebase-notes.mdentryvmaf-bench-unchecked-returns (2026-05-30).Test plan
meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false && ninja -C build-cpusucceeds (726/726 targets).clang-tidy -p build-cpu core/tools/vmaf_bench.c— no new warnings vs master.pre-commit run --files core/tools/vmaf_bench.c— all hooks pass.Do NOT admin-merge.