Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens two internal maintenance paths in the fork: the Vulkan kernel scaffolding's SSBO-binding limit and the CI script that parses ADR-0108 deliverables from PR bodies. It also updates the Vulkan subtree's rebase notes and the fork changelog to document those changes.
Changes:
- Replace the open-coded Vulkan kernel-template SSBO binding cap and stack-array size with a named constant.
- Extend
scripts/ci/deliverables-check.shto strip backslashes before matching deliverable checklist items in PR bodies. - Document the Vulkan-side invariant in
libvmaf/src/vulkan/AGENTS.mdand record the changes inCHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/ci/deliverables-check.sh |
Adjusts PR-body normalization so escaped markdown does not break deliverables matching. |
libvmaf/src/vulkan/kernel_template.h |
Replaces the hard-coded SSBO binding cap with a named constant used by validation and the local bindings array. |
libvmaf/src/vulkan/AGENTS.md |
Documents the new Vulkan kernel-template invariant for future rebases. |
CHANGELOG.md |
Adds Unreleased notes describing the kernel-template cap change and deliverables-check fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+102
| - **`vulkan/kernel_template.h` SSBO-binding cap is now a named | ||
| constant.** Replaced the open-coded `8U` upper bound and matching | ||
| `bindings[8]` stack array with `#define | ||
| VMAF_VULKAN_KERNEL_MAX_SSBO_BINDINGS 16U`. Current consumers top | ||
| out at 10 (the SSIM bundle in `ssimulacra2_vulkan.c` uses 8); the | ||
| new cap of 16 absorbs near-future kernels without further edits. | ||
| Vulkan's conformant minimum for `maxDescriptorSetStorageBuffers` | ||
| is 96, so the higher cap remains portable across drivers. No | ||
| behavioural change — same allowed kernel shapes, same scores, | ||
| same public C API. | ||
|
|
Comment on lines
+103
to
+106
| ### Fixed | ||
|
|
||
| - **`scripts/ci/deliverables-check.sh` now strips backslashes | ||
| from PR bodies before grepping the deliverable checklist.** The |
8 tasks
…heck backslash strip Two small hardening fixes uncovered during the GPU-dedup migration wave. Fix 1 — vulkan/kernel_template.h: replace the open-coded `8U` SSBO binding upper bound and matching `bindings[8]` stack array with a named `#define VMAF_VULKAN_KERNEL_MAX_SSBO_BINDINGS 16U`. Current consumers top out at 10; the higher cap absorbs near-future kernels without further edits, and Vulkan's conformant minimum for `maxDescriptorSetStorageBuffers` is 96 so it remains portable. No behavioural change — same allowed kernel shapes, same scores, same public C API. Fix 2 — scripts/ci/deliverables-check.sh: extend the markdown-strip `tr -d` set to also remove backslashes. PRs created via heredoc-quoted body strings in `gh pr create` calls escape embedded backticks; the trailing backslash survived the previous strip and broke the `- [x].*<item>` regex with backslash-space separators between tokens (e.g. "AGENTS.md\ invariant note"). Bit 18 PRs today. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
352efaa to
502fe11
Compare
…(6) rebase note) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lusoris
pushed a commit
that referenced
this pull request
May 3, 2026
Per CLAUDE.md §12 r13, every PR that closes a bug, opens a bug, or rules a Netflix upstream report not-affecting-the-fork updates docs/state.md in the same PR. Today's session shipped 7 such PRs without state.md updates (most were drafted in subagent-driven runs where the bookkeeping slipped). This is the catch-up. Recently closed (added): - vmaf_tiny_v1.onnx external-data ref fix (PR #296) - kernel_template SSBO cap 8->16 (PR #288 + named constant in #292) - deliverables-check.sh backslash strip (PR #292) - CI workflows draft-skip guard (PR #300) - CLAUDE.md §12 r14 ffmpeg-patches gate fix (PR #297) - ADR slug-drift cleanup (PR #304) - 1.07e-3 CPU vmaf_v0.6.1 score drift bisect (PR #305) — confirmed inherited from upstream a44e5e6 motion edge-mirror fix; not a fork regression. Snapshot regen in separate PR aligns with fork. No code changes. State.md only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
May 3, 2026
…307) * docs(state): record 2026-05-02 session bug-status changes (7 closes) Per CLAUDE.md §12 r13, every PR that closes a bug, opens a bug, or rules a Netflix upstream report not-affecting-the-fork updates docs/state.md in the same PR. Today's session shipped 7 such PRs without state.md updates (most were drafted in subagent-driven runs where the bookkeeping slipped). This is the catch-up. Recently closed (added): - vmaf_tiny_v1.onnx external-data ref fix (PR #296) - kernel_template SSBO cap 8->16 (PR #288 + named constant in #292) - deliverables-check.sh backslash strip (PR #292) - CI workflows draft-skip guard (PR #300) - CLAUDE.md §12 r14 ffmpeg-patches gate fix (PR #297) - ADR slug-drift cleanup (PR #304) - 1.07e-3 CPU vmaf_v0.6.1 score drift bisect (PR #305) — confirmed inherited from upstream a44e5e6 motion edge-mirror fix; not a fork regression. Snapshot regen in separate PR aligns with fork. No code changes. State.md only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: re-trigger after (6) opt-out fix --------- Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
13 tasks
lusoris
pushed a commit
that referenced
this pull request
May 4, 2026
Per ADR-0165 / CLAUDE.md §12 r13. Bookkeeping refresh of `docs/state.md` to close out the busy 2026-05-02/03 session: - Header date bumped 2026-04-29 → 2026-05-03. - Closed Issue #239 (FFmpeg `libvmaf_vulkan` wall-clock serialisation): row moved Open → Recently closed; cited PR #241 / commit `e266bf8e` and ADR-0251 (renumbered from 0235 by PR #310 dedup sweep). Closure verified by the v2 async pending-fence ring's `v2 ≤ 0.7 × v1` measurement gate flipping ADR-0251 to Accepted. - New Open-bugs row: `y4m_convert_411_422jpeg` heap-buffer-overflow on 4:1:1 with `dst_c_w == 1` (PR #348 fuzz). Reproducer parked at `libvmaf/test/fuzz/y4m_input_known_crashes/y4m_411_w2_h4_oob_dst.y4m`; fix follow-up PR TBD. - Recently closed audited for stale drafts: six rows updated with merged commit SHAs (#288, #292, #296, #297, #300, #304, #305) and the kernel-template citation corrected to ADR-0246 (post-dedup; ADR-0221 is now `changelog-adr-fragment-pattern.md`). - Netflix#955 deferred row: last-checked stamp refreshed to 2026-05-03; Netflix#1494 still `state=OPEN` per gh API. - "Update protocol" untouched; no row removed below its closure threshold; "Confirmed not-affected" entries unchanged. Six ADR-0108 deliverables: 1. Research digest — no digest needed: state-md bookkeeping. 2. Decision matrix — no alternatives: only-one-way fix per ADR-0165 / CLAUDE.md §12 r13. 3. AGENTS.md invariant note — no rebase-sensitive invariants. 4. Reproducer / smoke command — `mkdocs build --strict` + view the file; PR #348's harness reproduces the new Open bug. 5. CHANGELOG fragment — `changelog.d/changed/state-md-refresh-2026-05-03.md`. 6. Rebase note — no rebase impact: fork-local doc bookkeeping. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lusoris
added a commit
that referenced
this pull request
May 4, 2026
#352) Per ADR-0165 / CLAUDE.md §12 r13. Bookkeeping refresh of `docs/state.md` to close out the busy 2026-05-02/03 session: - Header date bumped 2026-04-29 → 2026-05-03. - Closed Issue #239 (FFmpeg `libvmaf_vulkan` wall-clock serialisation): row moved Open → Recently closed; cited PR #241 / commit `e266bf8e` and ADR-0251 (renumbered from 0235 by PR #310 dedup sweep). Closure verified by the v2 async pending-fence ring's `v2 ≤ 0.7 × v1` measurement gate flipping ADR-0251 to Accepted. - New Open-bugs row: `y4m_convert_411_422jpeg` heap-buffer-overflow on 4:1:1 with `dst_c_w == 1` (PR #348 fuzz). Reproducer parked at `libvmaf/test/fuzz/y4m_input_known_crashes/y4m_411_w2_h4_oob_dst.y4m`; fix follow-up PR TBD. - Recently closed audited for stale drafts: six rows updated with merged commit SHAs (#288, #292, #296, #297, #300, #304, #305) and the kernel-template citation corrected to ADR-0246 (post-dedup; ADR-0221 is now `changelog-adr-fragment-pattern.md`). - Netflix#955 deferred row: last-checked stamp refreshed to 2026-05-03; Netflix#1494 still `state=OPEN` per gh API. - "Update protocol" untouched; no row removed below its closure threshold; "Confirmed not-affected" entries unchanged. Six ADR-0108 deliverables: 1. Research digest — no digest needed: state-md bookkeeping. 2. Decision matrix — no alternatives: only-one-way fix per ADR-0165 / CLAUDE.md §12 r13. 3. AGENTS.md invariant note — no rebase-sensitive invariants. 4. Reproducer / smoke command — `mkdocs build --strict` + view the file; PR #348's harness reproduces the new Open bug. 5. CHANGELOG fragment — `changelog.d/changed/state-md-refresh-2026-05-03.md`. 6. Rebase note — no rebase impact: fork-local doc bookkeeping. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 4.7 (1M context) <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
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
Two small-but-load-bearing hardening fixes uncovered during today's GPU-dedup migration wave. Both are mechanical; they ship together to avoid a one-liner-per-PR ceremony.
libvmaf/src/vulkan/kernel_template.h: replace the open-coded8USSBO-binding upper bound and matchingbindings[8]stack array with a named#define VMAF_VULKAN_KERNEL_MAX_SSBO_BINDINGS 16U. Current consumers top out at 10 (the SSIM bundle inssimulacra2_vulkan.cuses 8); the higher cap absorbs near-future kernels without further edits. Vulkan's conformant minimum formaxDescriptorSetStorageBuffersis 96, so the higher cap remains portable across drivers. No behavioural change — same allowed kernel shapes, same scores, same public C API.scripts/ci/deliverables-check.sh: extend the markdown-striptr -dset to also remove backslashes. PRs created via heredoc-quoted body strings ingh pr createcalls escape embedded backticks; the trailing backslash survived the previous strip (which only handled backticks, asterisks, underscores) and broke the- [x].*<item>regex with backslash-space separators between tokens (e.g.AGENTS.md\ invariant note). Bit 18 PRs today.no docs needed: pure internal hardening — provided_features, scores, and public C API are identical.Test plan
Fix 2 (deliverables-check.sh): smoke-tested with a heredoc-quoted PR body containing literal-backslash-escaped backticks around
AGENTS.md. The script now printsOK (ticked): AGENTS.md invariant notefor that line and exits 0 on the full six-deliverable fixture.Fix 1 (kernel_template.h): clean compile + Vulkan smoke + async pending fence tests.
meson setup build-vulkan -Denable_vulkan=enabled -Denable_cuda=false -Denable_sycl=false libvmaf ninja -C build-vulkan src/libvmaf.so.3.0.0 meson test -C build-vulkan test_vulkan_smoke test_vulkan_async_pending_fenceResult locally:
2/2 OK(test_vulkan_smoke0.78s,test_vulkan_async_pending_fence2.27s).Deep-dive deliverables (per ADR-0108)
no digest needed: trivial fix derived from observation during today's GPU-dedup waveno alternatives: only-one-way fixAGENTS.mdinvariant note —libvmaf/src/vulkan/AGENTS.mdupdated under "Rebase-sensitive invariants" to document the newVMAF_VULKAN_KERNEL_MAX_SSBO_BINDINGSconstant and require both the bound check and the stack-array size to reference it. The deliverables-check fix is too small to need an AGENTS entry; ticked above.CHANGELOG.mdfragment — new entry under### Changed(kernel_template cap as named constant) and under a new### Fixedsection (deliverables-check backslash strip), both in the Unreleased lusoris-fork section.no rebase impact: fork-local only— neither file has an upstream counterpart (Netflix/vmaf has no Vulkan backend and no fork-specific CI scripts).Touched-file lint cleanliness (per CLAUDE.md §12 r12)
pre-commit run --files <touched>passes on all four files (clang-format, shfmt, shellcheck, semgrep, secrets, copyright, conventional-commit hook). One# shellcheck disable=SC1003annotation added on thetr -dline —trrecognises\\as a literal backslash escape (the intended behaviour); SC1003 is an informational shell-quoting hint, not atr-level concern. The disable carries an inline justification per the CLAUDE.md §12 r12 rule.State.md
No bug closed / opened / ruled-out by this PR; no
docs/state.mdupdate needed.