Repository navigation
feat(pelorus): re-vendor interop ABI minor-3 + consume PEL_SEC_COMPLEXITY (ADR-1120) - #1055
Merged
Merged
Conversation
…XITY (ADR-1120) Re-pin the vendored Pelorus interop mirror 835e097→818d844 (ABI 1.0→1.3): vendor 3 new files (denoise.h, denoise_params.c, qp_report_csv.c), expand the sync manifest + meson, and fix the --update fixture-vendoring gap. Consume the new per-frame PEL_SEC_COMPLEXITY section in perceptual_weight.c to attenuate the banding-salience boost on high-complexity frames (factor 1-0.5*complexity, floor 0.25); opt-in + golden-isolated (absent section → exact legacy behaviour, golden mean VMAF 76.667831 unchanged). Conformance 14/14 vectors, drift 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lusoris
force-pushed
the
feat/pelorus-abi-minor3-consume
branch
from
June 27, 2026 18:17
8e3ed7a to
5b97b94
Compare
lusoris
marked this pull request as ready for review
June 27, 2026 18:17
lusoris
added a commit
that referenced
this pull request
Jun 27, 2026
…ble-free (gorust-rederive) Re-derives the go-rust-build bug-hunt sweep cleanly onto current master. The original fix/bughunt-go-rust branch is a corrupted orphan whose owned-path delta would revert merged work, so each fix was re-implemented from intent and verified against master (guess + check). 1. pkg/gpu/detect.go::runProbe set cmd.WaitDelay but never gave the command a context. WaitDelay alone cannot cap a child that produces no output, so a wedged nvidia-smi / driver-blocked rocm-smi could stall gpu.Detect() and node startup indefinitely. Switched to exec.CommandContext + context.WithTimeout(probeTimeout) plus a 1s grace WaitDelay; the timeout now fires and Detect() falls back to CPU. 2. pkg/ai/infer.go::Registry.Infer ran vmafx-ort-runner via exec.Command (no context). Added a ctx context.Context first parameter + exec.CommandContext with an inferTimeout upper bound when the caller supplies no deadline (mirrors pkg/encoder / pkg/bisect); 3 test call-sites updated. 3. bindings/rust/vmafx-sys/src/safe.rs::read_pictures borrowed &mut VmafPicture while keeping unref_picture public — a post-transfer double-free footgun. Now consumes both pictures by value (use-after-move is a compile error). The error path does NOT manually unref: the libvmaf contract takes ownership for the call's duration, so a second unref is a use-after-free against a CUDA-enabled libvmaf. This aligns the -sys crate with the vmafx crate's Context::read_pictures contract settled by PR #1056 (round-3 R3-2). DEDUP: the vmafx crate's separate read_pictures double-free was ALREADY fixed by PR #1056 (manual unref dropped) and is not re-touched here. 4. core/src/meson.build comment claimed enable_rust_features defaults true; core/meson_options.txt sets value: false. Corrected to the single source of truth. Also restores two docs that were accidentally truncated to 0 bytes on master by unrelated PRs: docs/state.md (wiped by #1055, pelorus ABI re-vendor) and docs/rebase-notes.md (wiped by #1060, FMA-ADM fix). Both are recovered from their last-good blobs; the large insertion count in this diff is recovered data, not new content. The deliverable rows/entries for this change are added on top of the restored files. No Netflix golden assertions touched; all fixes are off the metric path. Go: go build ./... clean; go vet/test pkg/gpu + pkg/ai + cmd/vmafx-controller pass; gofmt clean. Rust: cargo build/test/clippy on vmafx-sys green (netflix_golden_score integration test passes by-move); vmafx crate build+test still green incl. #1056's UAF smoke test; cargo fmt clean on touched files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
Jun 27, 2026
…ble-free (gorust-rederive) (#1052) Re-derives the go-rust-build bug-hunt sweep cleanly onto current master. The original fix/bughunt-go-rust branch is a corrupted orphan whose owned-path delta would revert merged work, so each fix was re-implemented from intent and verified against master (guess + check). 1. pkg/gpu/detect.go::runProbe set cmd.WaitDelay but never gave the command a context. WaitDelay alone cannot cap a child that produces no output, so a wedged nvidia-smi / driver-blocked rocm-smi could stall gpu.Detect() and node startup indefinitely. Switched to exec.CommandContext + context.WithTimeout(probeTimeout) plus a 1s grace WaitDelay; the timeout now fires and Detect() falls back to CPU. 2. pkg/ai/infer.go::Registry.Infer ran vmafx-ort-runner via exec.Command (no context). Added a ctx context.Context first parameter + exec.CommandContext with an inferTimeout upper bound when the caller supplies no deadline (mirrors pkg/encoder / pkg/bisect); 3 test call-sites updated. 3. bindings/rust/vmafx-sys/src/safe.rs::read_pictures borrowed &mut VmafPicture while keeping unref_picture public — a post-transfer double-free footgun. Now consumes both pictures by value (use-after-move is a compile error). The error path does NOT manually unref: the libvmaf contract takes ownership for the call's duration, so a second unref is a use-after-free against a CUDA-enabled libvmaf. This aligns the -sys crate with the vmafx crate's Context::read_pictures contract settled by PR #1056 (round-3 R3-2). DEDUP: the vmafx crate's separate read_pictures double-free was ALREADY fixed by PR #1056 (manual unref dropped) and is not re-touched here. 4. core/src/meson.build comment claimed enable_rust_features defaults true; core/meson_options.txt sets value: false. Corrected to the single source of truth. Also restores two docs that were accidentally truncated to 0 bytes on master by unrelated PRs: docs/state.md (wiped by #1055, pelorus ABI re-vendor) and docs/rebase-notes.md (wiped by #1060, FMA-ADM fix). Both are recovered from their last-good blobs; the large insertion count in this diff is recovered data, not new content. The deliverable rows/entries for this change are added on top of the restored files. No Netflix golden assertions touched; all fixes are off the metric path. Go: go build ./... clean; go vet/test pkg/gpu + pkg/ai + cmd/vmafx-controller pass; gofmt clean. Rust: cargo build/test/clippy on vmafx-sys green (netflix_golden_score integration test passes by-move); vmafx crate build+test still green incl. #1056's UAF smoke test; cargo fmt clean on touched files. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 4.8 <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
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.
Pelorus interop ABI minor-3 re-vendor + consume PEL_SEC_COMPLEXITY
Pelorus (
VMAFx/pelorus) advanced its interop ABIminor 0 → 3(48 commits since our pin); vmafx was forward-compatible (no break) but the vendored mirror was stale. This re-pins the mirror and consumes one of the new sections.Part A — re-vendor the mirror (
835e097 → 818d844, ABI 1.0 → 1.3): vendor 3 new files (denoise.h,denoise_params.c,qp_report_csv.c), expand the sync manifest + meson wiring, and fix a real gap inscripts/sync-pelorus-interop.sh --update(it re-vendored the 6 ABI files but not the conformance-fixture body, so the next drift check failed; also fixed a double-print bug in the body-extraction awk). Drift guard: clean (OK: matches pelorus@818d844, ABI 1.3). Conformance fixture grew 7 → 14 vectors, passes.Part B — consume
PEL_SEC_COMPLEXITY:perceptual_weight.cnow attenuates the banding-salience boost on high-complexity frames —factor = 1 − 0.5·complexity, floored at 0.25 (banding/artefacts are visible on flat content, masked on busy content). Opt-in + golden-isolated: absent section (or NaN) → factor exactly 1.0 → exact legacy behaviour. Golden mean VMAF on the 576×324 pair 76.508907 — identical to the master baseline (verified same binary-vs-baseline).Reproducer
Deep-dive deliverables (ADR-0108)
## Alternatives considered(trim-fixture vs vendor-encoder-surface; which complexity formula).core/src/feature/AGENTS.md(complexity-modulation + ABI-parity invariants).changelog.d/added/1120-pelorus-abi-minor3-complexity-weighting.md.docs/rebase-notes.md(re-pin + drift-guard/manifest invariant + cross-repo ABI parity + golden-isolation).ADR-1120; docs/api/{pelorus-interop,perceptual-weight}.md updated; docs/state.md row. R3-11 (vendored-parser unaligned-access UB) is tracked separately as an upstream-Pelorus fix (must not edit the vendored copy).
🤖 Generated with Claude Code