lusoris · Jun 3, 2026 · May 29, 2026
@@ -0,0 +1,11 @@
+## libvmaf API error-path audit (ADR-0787)
+
+Identified six error-path defects in the libvmaf public C API (Research-0787):
+`vmaf_write_output_with_format()` returns `-EINVAL` instead of `-errno` on file-open failure;
+`vmaf_cuda_state_init()` returns `-EINVAL` for driver-not-found (should be `-ENOSYS`) and
+no-GPU (should be `-ENODEV`); `vmaf_close()` silently drops return values from
+`vmaf_framesync_destroy()` and `vmaf_thread_pool_wait()`; `vmaf_cuda_preallocate_pictures()`
+lacks the `-EBUSY` double-call guard present in SYCL/Vulkan; `vmaf_cuda_state_free()` has a
+mismatched single-pointer/int-return signature vs. all other backend `state_free()` functions.
+Fixes for items 1–3, 5–6 are planned for a follow-up implementation PR; item 4 (ABI break)
+is deferred to the next major version bump.
@@ -0,0 +1,61 @@
+# ADR-0787 — libvmaf Public API Error-Path Consistency Audit
+
+| Field     | Value                                                             |
+|-----------|-------------------------------------------------------------------|
+| Number    | 0787                                                              |
+| Title     | libvmaf Public API Error-Path Consistency Audit                   |
+| Status    | Accepted                                                          |
+| Date      | 2026-05-29                                                        |
+| Authors   | Claude Sonnet 4.6                                                 |
+| Tags      | api, error-handling, cuda, sycl, hip, consistency                 |
+
+## Context
+
+A full audit of every `VMAF_EXPORT` function in `core/include/libvmaf/` was requested to
+establish ground truth on: (1) the error-return convention, (2) consistent use of errno
+codes, (3) silent-error paths (returning 0 on real failure), (4) void functions that mask
+errors, and (5) cross-backend parity. See Research-0787 for the full findings.
+
+## Decision
+
+The audit confirms the following convention is in use and should be maintained across all
+backends: `0` on success, `< 0` negative errno on failure. No VMAF-specific error code
+namespace is needed.
+
+Six defects were identified (see Research-0787 §6). The following fixes are approved and
+should be addressed in a follow-up implementation PR:
+
+1. `vmaf_write_output_with_format()` returns `-EINVAL` for filesystem errors — fix to
+   return `-errno`.
+2. `vmaf_cuda_state_init()` returns `-EINVAL` for driver-not-found (should be `-ENOSYS`)
+   and no-GPU (should be `-ENODEV`).
+3. `vmaf_close()` drops return values of `vmaf_framesync_destroy()`,
+   `vmaf_thread_pool_wait()`, and `vmaf_picture_unref()` — fix with `(void)` casts.
+4. `vmaf_cuda_state_free()` has mismatched signature vs. all other backends (single-ptr,
+   int-return) — tracked for a future ABI-bump PR; current callers are warned.
+5. `vmaf_cuda_preallocate_pictures()` lacks the `-EBUSY` guard present in SYCL/Vulkan.
+6. `vmaf_init()` funnels all sub-init errors to `-ENOMEM` — fix to propagate `err`.
+
+## Alternatives considered
+
+- Introducing a VMAF-specific error code enum: rejected. The errno convention is already
+  established throughout the codebase and matches the FFmpeg filter integration contract.
+  A parallel enum would require a translation layer.
+
+- Treating the `state_free` signature mismatch as a no-op: rejected. The single-pointer
+  convention forces callers to manually null the handle. The double-pointer convention
+  (used by SYCL, HIP, Metal, Vulkan) is strictly safer. Fixing CUDA requires an ABI-break
+  PR with a version bump.
+
+## Consequences
+
+- An implementation PR will address items 1, 2, 3, 5, and 6 without ABI impact.
+- Item 4 (`vmaf_cuda_state_free` ABI break) is deferred to the next major ABI bump.
+- The audit output is captured as Research-0787 under `docs/research/`.
+
+## References
+
+- Research-0787: `docs/research/research-0787-libvmaf-api-error-path-audit.md`
+- Files audited: `core/include/libvmaf/*.h`, `core/src/libvmaf.c`,
+  `core/src/cuda/common.c`, `core/src/sycl/common.cpp`, `core/src/hip/common.c`,
+  `core/src/picture.c`, `core/src/feature/feature_collector.c`
@@ -748,6 +748,7 @@ ADRs may exist there for local session continuity, but the tracked
 | [ADR-0776](0776-copyright-header-sweep.md) | Copyright header sweep: remove "and Claude (Anthropic)" from all Lusoris fork notices; add missing headers to `ai/` + `mcp-server/` Python files; add vendor attribution to `pdjson.c`/`.h`. | Accepted | 2026-05-29 | license, docs |
 | [ADR-0782](0782-otel-tracing.md) | OpenTelemetry tracing and metrics schema for the VMAFX platform | Accepted | observability, otel, tracing, metrics, go, fork-local |
 | [ADR-0786](0786-vmafx-operator-stage2-reconcilers.md) | vmafx-operator Stage 2: VmafxJob gRPC-poll reconciler, VmafxNode 60-s stale-heartbeat gate, VmafxModelTraining checkpoint event emission, webhook URI/GPU-vendor validation, per-controller minimum-permission RBAC, 7-spec envtest suite. | Accepted | 2026-05-29 | go, k8s, operator, webhook, rbac, phase4b, fork-local |
+| [ADR-0787](0787-libvmaf-api-error-path-audit.md) | libvmaf public API error-path consistency audit: convention confirmed (0/-errno), 6 defects identified (wrong errno codes, dropped return values, CUDA state_free ABI mismatch, missing EBUSY guard) | Accepted | 2026-05-29 | api, error-handling, cuda, sycl, hip, consistency |
 | [ADR-0789](0789-rust-crate-audit.md.stub) | Rust crate audit (Research-0760): unsafe justification, cbindgen header drift, transitive vulnerability scan, Cargo.lock health, ADR-0707 dispatch contract — see `docs/research/research-0760-rust-crate-audit.md` | Proposed | rust, security, audit, research, adr-0707 |
 | [ADR-0792](0792-hardcoded-yuv-path-env-overrides.md) | Env-var overrides for hardcoded YUV and testdata paths | Accepted | workspace, ci, testdata |
 | [ADR-0795](0795-prev-ref-thread-safety.md) | Clarify and harden VmafFeatureExtractor.prev_ref thread-safety invariant | Accepted | threading, feature-extractor, batch-threading, correctness |

@@ -42333,6 +42333,26 @@ __ldg() pattern. The `integer_vif_cuda.c` conflict resolution keeps the HEAD sid
 
 ---
 
+### ADR-0787 — libvmaf API error-path audit (2026-05-29)
+
+No rebase impact: this PR adds only documentation files (research digest, ADR, changelog
+fragment) and no C/Python source changes.
+
+All files modified are fork-local:
+`docs/research/research-0787-libvmaf-api-error-path-audit.md` (new),
+`docs/adr/0787-libvmaf-api-error-path-audit.md` (new),
+`docs/adr/README.md` (new row),
+`changelog.d/fixed/0787-libvmaf-api-error-path-audit.md` (new),
+`docs/rebase-notes.md` (this entry).
+
+The six implementation fixes recommended by the audit (`vmaf_write_output_with_format` errno,
+`vmaf_cuda_state_init` error codes, `vmaf_close` unchecked returns, CUDA EBUSY guard,
+`vmaf_init` error propagation) will land in a separate fix PR that will carry its own
+rebase-notes entry. The `vmaf_cuda_state_free` ABI-normalisation is deferred to a
+major-version PR.
+
+---
+
 ### ADR-0815 — vmafx-operator + vmafx-node distroless Dockerfiles (2026-05-29)
 
 No rebase impact on upstream C/Python code.

@@ -0,0 +1,192 @@
+# Research-0787 — libvmaf Public API Error-Path Consistency Audit
+
+**Date:** 2026-05-29
+**Author:** Claude Sonnet 4.6 (automated research agent)
+**Scope:** All VMAF_EXPORT functions across `core/include/libvmaf/*.h` and their implementations
+**Status:** Complete — findings ready for fix PR
+
+---
+
+## 1. Convention
+
+All `VMAF_EXPORT int` functions uniformly document and implement:
+- `0` on success
+- `< 0` (negative errno code) on failure
+
+This convention is documented in every public header via `@return 0 on success, or < 0 (a
+negative errno code) on error` (e.g., `libvmaf.h` line 106, `libvmaf_cuda.h` line 44,
+`libvmaf_sycl.h` line 58). The convention is consistently applied across ~45 public
+`int`-returning functions. No VMAF-specific error code namespace exists; the API reuses POSIX
+errno values throughout.
+
+---
+
+## 2. Errno Usage — Consistency Findings
+
+### 2a. -EINVAL vs -ENOMEM (generally correct)
+
+NULL-argument guards return `-EINVAL`. Allocation failures return `-ENOMEM`. These are applied
+consistently across the CPU, CUDA, SYCL, HIP, Metal, and Vulkan backends.
+
+### 2b. -EAGAIN (correct, intentional)
+
+`vmaf_feature_collector_get_score()` returns `-EAGAIN` when a feature is valid but not yet
+written (retroactive scoring; see ADR-0154). This is documented in-source and correct.
+
+### 2c. -EBUSY (fork-added, consistent but undocumented)
+
+`-EBUSY` is returned by `vmaf_sycl_preallocate_pictures()`, `vmaf_vulkan_preallocate_pictures()`,
+and `vmaf_use_tiny_model()` / related DNN functions when called a second time after the
+resource is already initialised. The CUDA equivalent (`vmaf_cuda_preallocate_pictures()`) does
+**not** guard against double-call — it silently overwrites state. This is an inconsistency
+across backends.
+
+### 2d. File-open error returns wrong code (BUG)
+
+`vmaf_write_output_with_format()` (`libvmaf.c` lines 2948–2964) returns `-EINVAL` when
+`open(2)` or `fdopen(3)` fails. The real cause is a filesystem error (path not found,
+permission denied, read-only filesystem), which should surface as `-EIO` or `-errno`. The
+errno value is discarded and a hardcoded `-EINVAL` is returned, making the error
+indistinguishable from a bad-argument error to callers.
+
+### 2e. CUDA state_init conflates driver-missing with bad-argument (BUG)
+
+`vmaf_cuda_state_init()` returns `-EINVAL` for both:
+- driver library load failure (`cuda_load_functions` fails — should be `-ENODEV` or `-ENOSYS`)
+- `cuInit()` failure indicating no visible CUDA device (should be `-ENODEV`)
+
+By contrast, `vmaf_hip_state_init()` and `vmaf_sycl_state_init()` correctly return `-ENODEV`
+for device-not-found conditions.
+
+---
+
+## 3. Silent-Error Findings
+
+### 3a. vmaf_close() drops return values (DEFECT)
+
+`vmaf_close()` calls `vmaf_framesync_destroy()` and `vmaf_thread_pool_wait()` (both `int`-returning)
+without checking or `(void)`-casting their return values. These violations are CERT INT15-C and
+NASA Power-of-10 rule (every non-void return value must be checked or explicitly discarded).
+The return values are discarded silently, which means thread-pool errors during shutdown are
+invisible to the caller.
+
+Additionally, `vmaf_picture_unref()` is called inside `vmaf_close()` without capturing its
+return value.
+
+### 3b. vmaf_init() all-paths return -ENOMEM (minor imprecision)
+
+`vmaf_init()`'s error ladder always falls through to `return -ENOMEM` regardless of which
+sub-init failed. In practice, the only sub-inits that can fail are allocations, so this is
+currently benign. However, if `vmaf_feature_extractor_list_audit()` were to return `-EINVAL`
+for a registry bug, that would be silently converted to `-ENOMEM`.
+
+---
+
+## 4. void-Returning Public Functions
+
+The following `VMAF_EXPORT void` functions exist across backends:
+
+| Function | Notes |
+|---|---|
+| `vmaf_model_destroy()` | Correct: destructor, NULL-safe |
+| `vmaf_model_collection_destroy()` | Correct: destructor, NULL-safe |
+| `vmaf_dnn_session_close()` | Correct: destructor, NULL-safe |
+| `vmaf_mcp_close()` | Correct: destructor, NULL-safe |
+| `vmaf_sycl_state_free()` | Correct: destructor, NULL-safe, double-ptr |
+| `vmaf_sycl_dmabuf_free()` | Correct: free, NULL-safe |
+| `vmaf_sycl_profiling_disable()` | Correct: fire-and-forget state reset |
+| `vmaf_sycl_profiling_print()` | Acceptable: diagnostic output; errors logged only |
+| `vmaf_hip_state_free()` | Correct: destructor, NULL-safe, double-ptr |
+| `vmaf_metal_state_free()` | Correct: destructor, NULL-safe, double-ptr |
+| `vmaf_vulkan_state_free()` | Correct: destructor, NULL-safe, double-ptr |
+
+None of these silently mask a fatal condition. All destructors are NULL-safe.
+
+---
+
+## 5. Cross-Backend Inconsistencies
+
+### 5a. state_free() signature mismatch (BUG)
+
+`vmaf_cuda_state_free()` has a different signature from all other backend `state_free` functions:
+
+| Backend | Signature |
+|---|---|
+| CUDA | `int vmaf_cuda_state_free(VmafCudaState *cu_state)` — returns `int`, takes single pointer |
+| SYCL | `void vmaf_sycl_state_free(VmafSyclState **sycl_state)` — returns `void`, takes double pointer |
+| HIP | `void vmaf_hip_state_free(VmafHipState **state)` — returns `void`, takes double pointer |
+| Metal | `void vmaf_metal_state_free(VmafMetalState **state)` — returns `void`, takes double pointer |
+| Vulkan | `void vmaf_vulkan_state_free(VmafVulkanState **state)` — returns `void`, takes double pointer |
+
+CUDA's `state_free` (a) returns `int` (always 0, never an error), and (b) takes a single
+pointer rather than a double-pointer (so callers must manually null the pointer after the call).
+The three newer backends (SYCL, HIP, Metal) all use a double-pointer convention that
+auto-nulls the handle.
+
+### 5b. preallocate_pictures double-call guard
+
+SYCL and Vulkan return `-EBUSY` on second call. CUDA silently overwrites (see §2c).
+
+### 5c. device-not-found error code
+
+CUDA: returns `-EINVAL` (incorrect). SYCL, HIP: return `-ENODEV` (correct).
+
+---
+
+## 6. Recommendations (no implementation in this PR)
+
+1. **Fix `vmaf_write_output_with_format()` file-open error code**: capture `errno` before
+   logging and return `-errno` (or `-EIO` as the canonical I/O fallback). File:
+   `core/src/libvmaf.c` lines 2948–2964.
+
+2. **Fix `vmaf_cuda_state_init()` device/driver error codes**: return `-ENOSYS` when
+   `cuda_load_functions` fails (driver not present), `-ENODEV` when `cuInit()` fails (driver
+   present but no visible GPU). File: `core/src/cuda/common.c` lines 154–183.
+
+3. **Fix `vmaf_close()` unchecked return values**: `(void)`-cast
+   `vmaf_framesync_destroy()`, `vmaf_thread_pool_wait()`, and `vmaf_picture_unref()` calls, or
+   propagate their errors into the return value. File: `core/src/libvmaf.c` lines 1434–1437.
+
+4. **Normalise `vmaf_cuda_state_free()` to match other backends**: change to
+   `void vmaf_cuda_state_free(VmafCudaState **cu_state)` (double-pointer, void return),
+   auto-null the handle. This is a **public ABI break** — requires a major ADR and version bump.
+   Alternatively: add `vmaf_cuda_state_free2(VmafCudaState **)` and deprecate the old form.
+
+5. **Add `-EBUSY` guard to `vmaf_cuda_preallocate_pictures()`**: match SYCL/Vulkan behaviour
+   to prevent silent state overwrite on double-call. File: `core/src/libvmaf.c` lines 354–383.
+
+6. **`vmaf_init()` error propagation**: replace the single `return -ENOMEM` fallthrough with
+   `return err` so registry-audit failures (`-EINVAL`) are surfaced correctly. Low priority as
+   it is currently only reachable from a programming error, not a runtime condition.
+
+---
+
+## 7. Confirmed NOT Issues
+
+- `-EBUSY` from SYCL/Vulkan/DNN on double-call: intentional and correct.
+- `void` destructors: all NULL-safe, none mask fatal errors.
+- `-EAGAIN` from `vmaf_feature_collector_get_score`: intentional, documented.
+- `return 0.0` in static helper `resolve_feature_score_from_collector()` (lines 1351/1358):
+  this is a `double`-returning internal helper, not a public `int` function. The 0.0 is a
+  legitimate fallback score, not a false-success errno.
+- `vmaf_init()` swallowing sub-init error codes via `-ENOMEM` fallthrough: benign in practice
+  (all sub-inits only fail on OOM today), but warrants the fix in rec. 6 for defensive hygiene.
+
+---
+
+## Sources
+
+- `core/include/libvmaf/libvmaf.h`
+- `core/include/libvmaf/libvmaf_cuda.h`
+- `core/include/libvmaf/libvmaf_sycl.h`
+- `core/include/libvmaf/libvmaf_hip.h`
+- `core/include/libvmaf/libvmaf_metal.h`
+- `core/include/libvmaf/libvmaf_vulkan.h`
+- `core/include/libvmaf/dnn.h`
+- `core/src/libvmaf.c`
+- `core/src/cuda/common.c`
+- `core/src/cuda/cuda_helper.cuh`
+- `core/src/hip/common.c`
+- `core/src/sycl/common.cpp`
+- `core/src/picture.c`
+- `core/src/feature/feature_collector.c`