Skip to content

fix(core): vmaf_score_at_index EAGAIN guard misapplication (ADR-1073) - #766

Merged
lusoris merged 1 commit into
masterfrom
worktree-wf_fe1e7041-409-1
Jun 6, 2026
Merged

fix(core): vmaf_score_at_index EAGAIN guard misapplication (ADR-1073)#766
lusoris merged 1 commit into
masterfrom
worktree-wf_fe1e7041-409-1

Conversation

@lusoris

@lusoris lusoris commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • vmaf_score_at_index incorrectly propagated -EAGAIN for all frames after the first in multi-frame sequences. The ADR-0154 guard if (err && err != -EAGAIN) was applied to the model output score slot, suppressing vmaf_predict_score_at_index. Fix: change to if (err).
  • test_compute_vmaf_10bit fixture dimensions bumped 64→192 (defensive alignment with ADR-1072 convention).
  • compute_vmaf.c: revert n_threads back to 1 and remove all diagnostic code added during investigation.

Root cause

After frame 0 prediction creates the "vmaf" feature vector (capacity > 1, slot 0 written), frames 1+ find written=false in that vector → vmaf_feature_collector_get_score returns -EAGAIN. The ADR-0154 guard then suppresses the predict call entirely, so vmaf_score_pooled propagated -EAGAIN for every frame past the first. The MCP compute_vmaf handler surfaced this as a JSON-RPC error.

The retroactive-write input-feature case (integer_motion motion2/motion3) is correctly handled by the preceding flush; no -EAGAIN arrives from the input side at scoring time.

Reproducer

ninja -C core/build-wt
core/build-wt/test/test_mcp_smoke   # was: 16/18 pass; now: 18/18 pass
meson test -C core/build-wt --suite=fast  # 84/84

Deliverables checklist

  • Research digest: no digest needed: targeted single-function bug fix
  • Decision matrix: ADR-1073 ## Alternatives considered
  • AGENTS.md invariant: no rebase-sensitive invariants introduced
  • Reproducer / smoke-test: core/build-wt/test/test_mcp_smoke 18/18
  • changelog.d/fixed/1073-score-at-index-eagain-guard.md
  • docs/rebase-notes.md: no rebase impact: REASON in entry
  • docs/state.md: T-MCP-SCORE-POOLED-EAGAIN-2026-06-06 closed
  • docs/adr/1073-mcp-score-at-index-eagain-guard.md: Accepted
  • docs/adr/README.md: index row added

🤖 Generated with Claude Code

@lusoris @claude
fix(core): vmaf_score_at_index EAGAIN guard misapplication (ADR-1073)
165f5ba 
The ADR-0154 guard `if (err && err != -EAGAIN)` in vmaf_score_at_index
was intended to stop vmaf_predict_score_at_index from being called before
retroactive-write input features (integer_motion motion2/motion3) are
flushed.  It was correctly placed for the input-feature case but was also
applied to the model output score slot.

After frame 0 prediction creates the "vmaf" feature vector (capacity > 1,
slot 0 written), frames 1+ returned -EAGAIN from get_score (unwritten
slot); the guard suppressed the predict call entirely, propagating -EAGAIN
through vmaf_score_pooled for every frame past the first.

Fix: change `if (err && err != -EAGAIN)` to `if (err)`.  Input features
are fully available after flush, so no -EAGAIN arrives from that side at
scoring time.

Companion changes:
- test_compute_vmaf_10bit: fixture 64x64 -> 192x192 (defensive alignment
  with ADR-1072 test convention; original 64x64 comment mis-cited
  float_ms_ssim which vmaf_v0.6.1 does not use).
- compute_vmaf.c: revert n_threads 0->1 (debug artefact; real fix is in
  libvmaf.c), remove all diagnostic fprintf/snprintf/feature-check blocks.

Resolves: test_mcp_smoke::test_compute_vmaf_10bit (18/18 pass).
Fast suite: 84/84.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris
lusoris enabled auto-merge (squash) June 6, 2026 15:15
@lusoris
lusoris merged commit 46e665a into master Jun 6, 2026
46 of 61 checks passed
@lusoris
lusoris deleted the worktree-wf_fe1e7041-409-1 branch June 6, 2026 15:15
This was referenced Jun 6, 2026
Closed
Merged
lusoris added a commit that referenced this pull request Jun 7, 2026
@lusoris @claude
docs(state): backfill state.md rows for PRs #765–#771
4eec292 
Add three missing Recently-closed rows that were absent from state.md:

- T-GPU-POOL-UAF-OOM-ASAN-UBSAN-GAP-2026-06-06: PRs #767 + #770 added
  three huge-alloc tests to ASan/UBSan/TSan exclusion lists in both
  sanitizers.yml and tests-and-quality-gates.yml; CI SIGABRT spurious
  failures resolved.

- T-HIP-MOTION-DEBUG-BOOL-SYCL-GRAPH-DANGLING-2026-06-06: PR #768 fixed
  HIP motion test passing "1" for a VMAF_OPT_TYPE_BOOL option (should be
  "true") and a SYCL graph dangling-priv SIGSEGV when a VmafSyclState is
  shared across two sequential VmafContext instances.

- T-MOTION-FIVE-FRAME-WINDOW-PYTHON-SKIP-2026-06-06: PR #771 added
  @unittest.skip decorators to 9 Python test methods that set
  motion_five_frame_window=True, which returns -ENOTSUP from C per
  ADR-0337 pending prev_prev_ref plumbing.

PRs #765 (T-PREV-REF-BATCH-REFCOUNT-LEAK), #766 (T-MCP-SCORE-POOLED-EAGAIN),
and #769 (T-PIC-PREALLOC-ASAN-LEAK) were already tracked. PR #770 adds no
new bug row (CI wiring fix only, no new defect opened/closed).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Jun 8, 2026
@lusoris @claude
chore(bundle): drain 8 rebased DRAFT PRs (docs + helm + tests-coverag…
a42e4ea 
…e) (#845)

* docs(internal-headers): add Doxygen @brief/@PARAM to 10 core src/*.h (ADR-1096)

214 internal headers had no Doxygen coverage; only 26 had any @brief/@PARAM
annotation. Adds @brief, @PARAM[in/out], and @return comments to the ten
highest-traffic headers:

  framesync.h thread_pool.h picture_pool.h predict.h fex_ctx_vector.h
  ref.h mem.h log.h opt.h dict.h

Purely additive — no logic, no ABI, no public-header changes.
IDE hover-docs and doxygen -q now populate for all covered APIs.

ADR-1096 documents the coverage decision and follow-up scope.
AGENTS.md invariant added: update Doxygen blocks when signatures change.

no digest needed: trivial doc-only addition
no alternatives: only-one-way fix (add the comments)

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

* docs(api): add @thread-safety/@param/@return to all undocumented public C-API functions

Every VMAF_EXPORT function in core/include/libvmaf/ now carries a complete
Doxygen contract: @PARAM, @return, and @Thread-safety. Previously libvmaf_cuda.h
(5 functions), libvmaf_sycl.h (20), dnn.h (9), picture_v2.h (5), and
model.h (vmaf_model_version_next) were missing @Thread-safety entirely;
picture_v2.h stubs were also missing @param/@return. VmafPoolingMethod in
libvmaf.h lacked a @brief enum-level description.

The thread-safety contract is uniform: GPU backend setup functions (cuda/sycl/
hip/metal state init/import/free, preallocate, fetch) are not thread-safe —
one handle per driver thread. Pure query functions (vmaf_dnn_available,
vmaf_sycl_list_devices, vmaf_dnn_verify_signature, vmaf_backend_handle_name,
vmaf_model_version_next) are marked safe from any thread.

No C source files, build files, or ABI-visible signatures changed — Doxygen
comment additions only.

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

* docs(state): backfill state.md rows for PRs #765–#771

Add three missing Recently-closed rows that were absent from state.md:

- T-GPU-POOL-UAF-OOM-ASAN-UBSAN-GAP-2026-06-06: PRs #767 + #770 added
  three huge-alloc tests to ASan/UBSan/TSan exclusion lists in both
  sanitizers.yml and tests-and-quality-gates.yml; CI SIGABRT spurious
  failures resolved.

- T-HIP-MOTION-DEBUG-BOOL-SYCL-GRAPH-DANGLING-2026-06-06: PR #768 fixed
  HIP motion test passing "1" for a VMAF_OPT_TYPE_BOOL option (should be
  "true") and a SYCL graph dangling-priv SIGSEGV when a VmafSyclState is
  shared across two sequential VmafContext instances.

- T-MOTION-FIVE-FRAME-WINDOW-PYTHON-SKIP-2026-06-06: PR #771 added
  @unittest.skip decorators to 9 Python test methods that set
  motion_five_frame_window=True, which returns -ENOTSUP from C per
  ADR-0337 pending prev_prev_ref plumbing.

PRs #765 (T-PREV-REF-BATCH-REFCOUNT-LEAK), #766 (T-MCP-SCORE-POOLED-EAGAIN),
and #769 (T-PIC-PREALLOC-ASAN-LEAK) were already tracked. PR #770 adds no
new bug row (CI wiring fix only, no new defect opened/closed).

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

* chore(state): backfill 4 missing state.md rows from PRs #712–#747 batch

State.md updates promised by squash commits for PRs #723, #725, #729,
and #743 were lost (the pre-squash branch commits carried the changes
but the squash commits did not include state.md in their diffs).

Changes:
- Move T-JSON-MODEL-SLOPES-FEATURE-CAP-OOB-2026-05-30 from Open to
  Recently closed — fixed by PR #743 / ADR-0887 (vmaf_model_destroy
  heap-buffer-overflow via fuzz_json_model nightly harness).
- Add T-FFMPEG-PATCHES-SCORE-FMT-GAP-2026-06-06 to Recently closed —
  PR #723 / ADR-1064 wired score_fmt AVOption on all four FFmpeg vmaf
  filters; PR #740 fixed patch hunk counts.
- Add T-VENDORED-CJSON-PDJSON-SECURITY-2026-06-06 to Recently closed —
  PR #725 / ADR-1061 fixed five pdjson/cJSON security and correctness
  bugs (depth guard never compiled, size overflow x2, banned sprintf/
  strcpy at 12 sites, cJSON_GetArraySize int wrap).
- Add T-GO-STATICCHECK-R10-TIMER-BODY-2026-06-06 to Recently closed —
  PR #729 / ADR-1065 fixed Go timer leak (time.After in poll loop),
  missing body size cap on vmafx-controller, and missing ReadTimeout
  on both HTTP servers.
- Add second-pass _Updated: header summarising the PRs #712–#747 batch.

no rebase impact: state.md only, no code or API surface changes.

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

* docs(nav): add 10 orphaned pages to mkdocs.yml nav

Audit found the following docs/**.md files present on disk but absent from
both the mkdocs.yml nav and any inbound cross-reference:

- docs/ai/sidecar-online-training.md (k8s Phase 4b sidecar trainer)
- docs/server/auth.md (JWT auth gateway for vmafx-controller)
- docs/server/operator.md (kubebuilder Kubernetes operator)
- docs/server/rest.md (vmafx-server REST/OpenAPI surface)
- docs/development/ebpf-fuse-bypass.md (rclone FUSE eBPF bypass)
- docs/development/perf-claims-2026-05-10.md (May 2026 perf claims log)
- docs/sync-upstream/2026-05-02-sync-report.md (upstream sync report)
- docs/sync-upstream/2026-05-03-sync-report.md (upstream sync report)
- docs/upstream-ports/1b08bb4d-needs-manual-port.md (manual port note)

Also excludes docs/changelog.d/** from the mkdocs build — one changelog
fragment (cpp23-wave3.md) was placed under docs/changelog.d/ instead of
the repo-root changelog.d/; the fragment is referenced by the ADR index
but is not a standalone page and should not be rendered by mkdocs.

No code changes. No ADR required (nav-only housekeeping).

no rebase impact: docs/mkdocs.yml nav entries only
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(helm): values completeness — nameOverride, statePVCSize, node.metricsPort, extraPorts schema (ADR-1074)

Four gaps where values.yaml / values.schema.json diverged from template usage:

1. nameOverride/fullnameOverride: read by _helpers.tpl but absent from both
   values.yaml and the root additionalProperties:false schema; any user
   supplying --set nameOverride=foo received an immediate helm-lint failure.
   Added as string keys to both files.

2. statefulSet.statePVCSize: statefulset.yaml hardcoded `storage: 1Gi` for the
   per-replica MCP-state PVC. Exposed as statefulSet.statePVCSize (default 1Gi)
   and wired into the volumeClaimTemplates storage field.

3. node.metricsPort: port 9090 appeared hardcoded in three template locations
   (node Deployment containerPort, node-metrics Service port, NetworkPolicy
   allow rule). Exposed as node.metricsPort (default 9090) and unified.

4. service.extraPorts items schema: bare `"type": "array"` with no items
   definition accepted malformed port objects silently. Added items schema with
   required [name, port] and protocol enum.

All defaults preserve existing rendered output byte-for-byte.

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

* test(ai/scripts): add unit tests for calibrate_phase_f_recipes and analyze_knob_sweep helpers (round 3)

The existing test_calibrate_phase_f_recipes.py covers only the main()
invocation (run-provenance smoke).  The existing test_knob_sweep_analysis.py
covers pareto_frontier, stratify, and detect_recipe_regressions via a
20-row synthetic fixture.

This commit adds coverage for the remaining pure helper functions:

* test_calibrate_phase_f_recipes_unit.py (50 tests):
  - mos_to_vmaf_proxy: clamping, boundary MOS values, string coercion
  - saliency_benefit_to_intensity: threshold boundaries for all three labels
  - _iter_corpus_rows: valid rows, blank-line skip, malformed-JSON skip,
    missing-field skip, optional duration_s default
  - _ugc_target_vmaf_offset: empty/small corpus, symmetric distribution,
    heavy-tail sign, clamp bounds, rounding
  - _ugc_tight_interval_width: empty corpus fallback, uniform/wide
    distributions, floor/cap enforcement, rounding
  - _resolution_dominance: empty, single-resolution, split, dominant bucket,
    portrait-vs-landscape distinction
  - _ugc_saliency_benefit_fraction: fallback, no-qualify, all-qualify,
    half-qualify, high-MOS exclusion, square-aspect treatment
  - calibrate() integration: all four recipe classes, UGC/proxy provenance
    tags, required recipe keys, saliency label validity

* test_analyze_knob_sweep_unit.py (29 tests):
  - _stable_knob_repr: empty dict, single entry, alphabetical sort, non-Mapping
    input, numeric values, insertion-order invariance
  - _slug: alphanumeric passthrough, hyphen/underscore preserved, space/slash
    replacement, empty-string fallback, all-special-chars
  - _closest_bare_at_bitrate: no-bare-rows, within tolerance, outside
    tolerance, picks closest, exact match, zero-tolerance
  - write_slice_csv: filename pattern, header row, data rows, slug-safe names,
    auto-creates output directory
  - write_summary_md: file creation, slice count, no-regression message,
    regression table, auto-creates output directory

All 79 tests run without GPU, corpus, or model downloads (<100 ms total).

no rebase impact: test-only addition; no existing golden assertion modified.

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

* test(vmafx-server): replace //nolint:errcheck with explicit discards per ADR-0278

Replace all //nolint:errcheck suppression comments in grpc_server_handler_test.go
with explicit-discard patterns (_ = x.Close() / defer func() { _ = x.Close() }()).
Also remove duplicate TestGRPCScore_ScorerError function (the earlier copy was
accidentally left in; keep the one with the fuller doc comment at the bottom of
the file). Remove duplicate TestRunHTTP_BadAddress from main_extra_test.go.
Fix unchecked ln.Close() and conn.Close() in TestRunHTTPGracefulShutdown.

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

---------

Co-authored-by: Lusoris <lusoris@pm.me>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
@lusoris lusoris mentioned this pull request Sep 6, 2026
Closed
13 of 14 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