build(ffmpeg): migrate the patch stack to n9.0.1 (precompiled SPIR-V), fix #44, and clear the maintenance backlog - #47
Merged
Conversation
This was referenced Aug 30, 2026
…ent agent cleanup)
The side-data parser computed section/header pointers as base+offset and cast them to the concrete struct before reading fields. The R5 padding guarantee is relative to the blob base, so any caller handing in a misaligned base inherited that misalignment into every field read -- UB, and a real fault on strict alignment targets and under -fsanitize=alignment. Read the header and each directory entry through memcpy into aligned locals instead. Also reject a header_size that is not a multiple of 8: that would put dir[] on a misaligned start, which is corrupt framing from an untrusted producer rather than a short buffer. No wire change: no struct field, no _Static_assert size, no section bit moves, so PELORUS_ABI_MINOR stays 3. Verified: the new test_misaligned_blob_base fixture traps the OLD parser at interop.c:202 under -fsanitize=alignment -fno-sanitize-recover, and passes clean against the new one (ASan+UBSan, all 16 fixtures). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README.md was structurally broken: three Modules rows and three roadmap bullets had been appended after the Support section by successive filter PRs, so the front page ended mid-sentence and aa/deblock/borderfix were missing from both the table and the landed list. Restore them to their sections, finish the two truncated sentences, de-duplicate the Step 8/9 labels (aa/deblock/borderfix are now 10/11/12), correct four wrong patch numbers (dehalo 0014, aa 0015, deblock 0017, borderfix 0018) and stop marking the shipped denoise filter as roadmap. Retarget the operational references from n8.1.1 to n9.0.1 and from the deleted /home/kilian/dev/ffmpeg-8 default to ffmpeg-9. ADRs and CHANGELOG keep their n8.1.1 mentions -- those are historical records, not live configuration. Also make ffmpeg-patches/test/build-and-run.sh executable; the README quick start invoked it directly and failed with a permission error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…library -> spirv_compiler FFmpeg 9 renamed the configure dependency (0 occurrences of spirv_library remain upstream) and moved Vulkan shaders to build-time SPIR-V, so each filter's .comp.glsl must land in libavfilter/vulkan/ and be registered in that directory's Makefile. install_vk_shader() does both, idempotently, for all nine Vulkan filters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All 18 patches apply to a pristine n9.0.1 via git am --3way, configure succeeds with libpelorus found, and every filter and shader compiles: 10 filter objects and 9 SPIR-V shader objects, zero errors, all 9 shader symbols resolving between the filter object and its .comp.spv.o. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FFmpeg 9 compiles shaders at build time, so spirv_compiler is satisfied by a working glslc rather than by --enable-libshaderc (which drove the deleted runtime GLSL path). Clone n9.0.1, drop the flag, assert glslc is present before configure so a missing compiler fails loudly instead of silently disabling every Vulkan filter, and pin the runner to ubuntu-24.04 to match the release-specific LunarG apt list -- on ubuntu-latest that list silently stops resolving when the image moves on. Add a sanitizer job. Issue #44 (a misaligned blob base cast straight into a struct) passed build, tests, clang-format and clang-tidy; only -fsanitize=alignment finds it, and the repo had no sanitizer gate at all. Invocation verified locally: 11/11 fast suite green under ASan+UBSan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…istency Master is 40 commits past the v0.1.0 tag: 10 FFmpeg patches became 18, two filters became eleven plus a BSF, and the interop ABI went 1.0 -> 1.3. All of it was additive -- no libpelorus API removed, the ABI stayed append-only -- so this is a minor bump. The changelog had never been rotated: [Unreleased] still contained the three fragments v0.1.0 actually shipped. Classified every fragment by its introducing commit against the tag date; exactly three predate it (interop ABI, deband, analyze), matching the v0.1.0 tag message. Those move to a released [0.1.0] section and the remaining 46 stay unreleased. The version lived in two hand-edited places with nothing checking they agreed. meson.build now parses PELORUS_VERSION_STR out of pelorus.h and errors on skew; verified by inducing a mismatch and confirming configure fails. Raise the patch stack's pkg-config floor to libpelorus >= 0.2.0: filters now use interop symbols that did not exist in 0.1.0. Not tagged -- the tag is pushed once the remaining migration work is verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add the n9.0.1 rebase-notes entry (ADR-0108 deliverable #6) recording every upstream API change the bump required, including the NVENC_HAVE_QP_MAP_MODE removal that would have silently compiled the ROI feature out rather than failing the build. ADR-0113 and ADR-0137 read Accepted in their own files but Proposed in the index; correct the index. A sweep confirms no other ADR has status skew. Remove a stray comp.spv dropped at the repo root by a manual shader compile, gitignore the three extra build trees, and add .gitattributes forcing LF on *.patch -- git am parses those byte-exactly and a CRLF rewrite would silently break the apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…signs out AGENTS.md hard rule 4 required keeping each filter's reference .comp and its inline GLSL in lockstep. FFmpeg 9 removed the runtime GLSL builder, so there is no second copy to sync -- the shader lives once as a .comp.glsl compiled to SPIR-V at build time. Rule 4 is rewritten around what is actually dangerous now: the .glsl binding order must match the C descriptor array and the push-constant block must match the opts struct, and NEITHER is compiler-checked, so a mismatch is silent corruption rather than a build failure. Also records that spec-constant ids 253/254/255 are reserved. The shader-lockstep-warn hook was policing the dead rule; repoint it at the new invariant. Verified by feeding it both file kinds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e stack
FFmpeg 9 changed the include style inside libavcodec/bsf/: the relative forms
("bsf.h", "cbs_h265.h", "hevc/hevc.h", ...) no longer resolve from that
subdirectory, so patch 0010 failed with 'fatal error: bsf.h: No such file or
directory'. Upstream made the same change to its own BSFs. Qualify all eight.
Full surface now verified against a pristine n9.0.1: 18/18 patches apply,
configure succeeds, and 13 C objects (9 Vulkan filters, scenecut, the FGS BSF,
vulkan_encode for the qpmap patch, nvenc for the three nvenc patches) plus 10
SPIR-V shader objects compile with zero errors.
qsvenc/libaomenc/libsvtav1 are not compile-checked here -- they need vendor SDKs
absent from this box. Their patches apply; the existing 'compile-verified only'
caveat for those paths is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
install_vk_shader() emitted OBJS-$(PELORUS_<NAME>_VULKAN_FILTER), an UNDEFINED make variable that expands to empty -- so no shader object was ever pulled into the build and every migrated filter would have failed at LINK with an undefined ff_pelorus_<name>_comp_spv_data. My earlier verification missed this because I named the .spv.o targets explicitly on the make command line, which builds them through the pattern rule whether or not OBJS references them. Caught by the adversarial review of the denoise migration. Re-verified the right way -- by LINKING. Plain 'make ffmpeg' (naming no .spv.o) against a pristine n9.0.1 with all 18 patches applied: all 9 shader objects are built by OBJS alone, the binary links, and it registers 10 pelorus filters plus the pelorus_fgs BSF. Also retire the stale lockstep headers in the reference .comp shaders: they told maintainers to hand-synchronise against inline GLSL that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…at no longer exists
docs/backends/vulkan.md still told readers to install libshaderc for spirv_library and described building GLSL at runtime through the GLSLC macros. Both are gone in FFmpeg 9. Document the real model: a build-time glslc probed by check_glslc, ff_vk_shader_load + ff_vk_shader_link against precompiled SPIR-V, and the new wgc_z parameter. Rewrite the authoring rules around what is now actually dangerous -- binding order and push-constant offsets are hand-maintained and NOT compiler-checked, so a mismatch is silent corruption -- and record that spec-constant ids 253/254/255 are reserved. Six per-filter metrics docs repeated the retired lockstep claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Prerequisites said libshaderc/spirv_library and the configure line passed --enable-libshaderc; FFmpeg 9 removed that runtime path and probes a build-time glslc for spirv_compiler instead. Add the link gate. Naming a .spv.o on a make command line builds it via the pattern rule regardless of whether OBJS references it, so a mis-registered shader passes a targeted build and fails only at link -- exactly the defect this migration hit. Document finishing with 'make ffmpeg' and checking the filters register in the linked binary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The migration is no longer compile-only. All 9 Vulkan filters execute on three vendors (RTX 4090, Arc A380, RADV), planes=1 gives bit-exact chroma (u:inf v:inf) on every luma-only filter -- the exact ADR-0129 invariant -- the three analyzers are byte-identical pass-throughs, meta=1 exercises libpelorus at runtime, and a five-filter chain runs on real 2160p content. Remaining gap recorded honestly: no validation-layer run, since the layers are not installed on this box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The qpmap patch had added spirv_compiler to vulkan_encode_deps, which silently
disabled h264_vulkan, hevc_vulkan AND av1_vulkan -- three upstream encoders --
on any build host without glslc. An opt-in Pelorus feature must never do that,
and Pelorus CI would never have caught it because CI always installs the LunarG
SDK.
Introduce a derived config item instead: pelorus_qpmap_gpu_if="vulkan_encode
spirv_compiler". The Makefile registers the shader under
CONFIG_PELORUS_QPMAP_GPU and vulkan_encode.c guards the rasterizer on it,
falling back to the host raster path that already existed.
Verified in both configurations on a pristine n9.0.1:
glslc present -> CONFIG_PELORUS_QPMAP_GPU 1, both objects build
glslc absent -> configure succeeds, CONFIG_H264_VULKAN_ENCODER still 1,
CONFIG_PELORUS_QPMAP_GPU 0, vulkan_encode.o builds, no spv.o
Regenerated stack re-verified end to end: 18/18 apply, make ffmpeg links, 10
filters + the BSF register, 10 shader objects pulled in by OBJS, and 9/9 filters
execute on the GPU.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-0114 (encoder steering) shipped across all five encoder legs and is in the release; mark it Accepted. ADR-0132 (per-shot CRF) was measured and loses. Layer 1, the complexity-scalar producer, shipped as PEL_SEC_COMPLEXITY in ABI 1.3 and is retained because tune=auto consumes the same signal; layers 2-3, applying it as CRF steering, are an honest negative (bench v0.13, PR #34), and ADR-0142 generalises it: source-side rate control loses structurally to the encoder's own RC. Mark it Rejected, matching how ADR-0135 and ADR-0141 record measurement rejections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shader gate could vanish. find_program('glslangValidator', required: false)
fell through to a message() when absent, so CI could report a green fast suite
having run ZERO shader tests. It is now required whenever -Dshaders is on (the
default); -Dshaders=false remains the deliberate opt-out, verified.
The gate also only checked the REFERENCE shaders. Since ADR-0143 the filters
execute ffmpeg-patches/files/vulkan/pelorus_*.comp.glsl, which nothing outside a
full FFmpeg build compiled. Add all ten to the fast suite using FFmpeg's exact
GLSLCFLAGS, so a broken shipped shader fails the local gate first. Fast suite:
11 -> 21 tests.
CI: give every job timeout-minutes (a hung network step otherwise burns the
360-minute default) and set persist-credentials: false on every checkout -- the
ffmpeg-stack job builds third-party code in a tree that was carrying a usable
GITHUB_TOKEN in .git/config.
Repo: drop a 2.5-month-old stash holding 77 files of pure .bench-out YUV/mkv
artifacts (no source), then gc. Pack size 175.55 MiB -> 1.24 MiB, matching the
remote; fsck clean. Gitignore .bench-out/ so it cannot recur.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…asting-through-void .clang-tidy suppressed bugprone-casting-through-void -- the one check that locates the cast-a-byte-pointer-to-a-struct pattern behind issue #44. It was suppressed because the pack path legitimately did that on a calloc'd (therefore aligned) buffer, so the rule fired on safe code. Build the header and each directory entry in an aligned local and memcpy them out instead. Now neither direction casts, the rule fires on nothing, and it can stay enabled as a real guard against reintroducing #44 rather than being switched off. The wire format is unchanged, proven rather than assumed: packing the same two-section blob with the old and new code yields byte-identical output (336 bytes, identical hex). Fast suite 21/21 and ASan+UBSan 21/21 stay green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… and the 0135 negative The pinned corpus URL 404s: download.blender.org no longer serves BigBuckBunny_640x360.m4v (HEAD and ranged GET both, no working mirror found), and the local cache holds only derived YUV, so the harness cannot run cold. Deliberately NOT re-pinned to another clip -- the pinned sha256 is what every number in bench-results.md was measured against, so swapping the source would invalidate the comparison history rather than fix it. Re-pinning resets the baseline and is the maintainer's call, so it is documented, not silently done. docs/metrics/mc.md never documented PEL_SEC_MOTION_CONF (ABI minor 2, ADR-0131) even though the filter emits it. ADR-0135 had no changelog fragment although its precedent ADR-0141 does; negative results are deliverables here. rebase-notes labelled all 19 sections v0.1.0; that tag shipped only patches 0001 and 0002 (git ls-tree), so 17 sections are relabelled v0.2.0. The missing 0003 and 0004 sections are recorded as a known gap rather than back-filled from memory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two reports were appended after 'Open / next' and mislabelled v0.3 and v0.4, colliding with the real v0.3 and v0.4 earlier in the file, so the document had duplicate headings and ended on stranded content. Renumber them v0.18/v0.19, move them into the body, and state plainly that the numbers are insertion order rather than chronology (this work landed with ADR-0131 and ADR-0133). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Folded in rather than merged: this branch already rewrites both workflow files (timeout-minutes, persist-credentials, the n9.0.1 build and the new sanitizer job), so PR #46 would have conflicted on exactly these lines. It also covers the checkout in the sanitizer job, which did not exist when #46 was opened. SHA verified independently against upstream rather than taken from the PR: actions/checkout tag v7.0.1 resolves to 3d3c42e5aac5ba805825da76410c181273ba90b1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git format-patch stamps each patch with the commit Date:, so two runs of generate.sh produced 18 patches differing only in a timestamp. That made "do the committed artifacts still match generate.sh?" unanswerable and would have hidden real drift in the noise -- which is why the audit found the committed patches were not produced by a single run and nothing checked them. Pin GIT_AUTHOR_DATE/GIT_COMMITTER_DATE. The commits are synthetic anyway (format-patch runs --zero-commit), so a fixed date costs nothing. Verified: two independent runs now emit byte-identical patches. With that, add the gate: CI regenerates the stack and fails if ffmpeg-patches/ differs, catching a hand-edited *.patch (which the README forbids) or a files/ change landed without regenerating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…c errors Installed the layers and ran all nine filters under VK_LAYER_KHRONOS_validation. Four VUID types appear; every one is also emitted by stock upstream filters doing the same kind of work, so none is a Pelorus defect. Three come from a bare hwupload,hwdownload chain with NO filter at all (FFmpeg's own hwcontext_vulkan upload/download path). 03909 comes from upstream gblur and nlmeans. 07454 is emitted by analyze, grain_estimate and mc -- and also by upstream vf_scdet_vulkan, sixteen times, using the identical ff_vk_frame_barrier(ALL_COMMANDS -> COMPUTE_SHADER, SHADER_READ) call. Pelorus's barrier and queue code is byte-identical to pre-migration master, so the migration did not introduce it. Picking the right control was the whole game: against gblur_vulkan alone 07454 looks Pelorus-specific; against scdet_vulkan, the actual analogue, it is plainly upstream. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s clone depths The CI reproducibility gate I just added would have failed on its first run, and local testing against a shallow clone caught it before a 2.5-hour CI round trip. git abbreviates the blob hashes in a patch's `index` lines based on how many objects the repository holds. A full FFmpeg clone emits 10 hex chars; CI's `--depth 1` clone emits 7. So the identical sources produced different patch bytes depending on clone depth, and 'does the committed stack match a fresh generate.sh run?' had no stable answer. --full-index emits the complete 40-char hashes, removing the dependence entirely and giving 'git am --3way' the unambiguous blob ids it wants anyway. Verified: generating from the full clone and from a --depth 1 shallow clone now yields byte-identical patches, and the regenerated stack still applies 18/18, configures, links, and registers all 10 filters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lusoris
force-pushed
the
build/ffmpeg-9-migration
branch
from
August 30, 2026 18:49
594fbbe to
968e470
Compare
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.
What
Fixes #44.
Migrates the FFmpeg patch stack from n8.1.1 to n9.0.1, fixes the open
side-data alignment UB (#44), and clears the maintenance backlog a 12-agent
audit surfaced. Version bumped 0.1.0 → 0.2.0 (not tagged here).
Why it is not a rebase
FFmpeg 9 deleted the runtime inline-GLSL shader API every Pelorus filter was
built on —
GLSLC/GLSLA/GLSLF/GLSLD,FFVulkanShader.src,ff_vk_shader_init(),ff_vk_shader_print()— replacing it with build-timeSPIR-V. Ten API breaks in total, four of which only surface when you actually
compile, and one of which fails silently:
shader_init/shader_print/.srcdeletedspirv_library→spirv_compilergenerate.shanchor assertadd_descriptor_set→void, lost an argumentff_vk_filter_process_*gaineduint32_t wgc_zshader_loadwantsuint32_t[], notint[]libavcodec/bsf/*.cneeds fully-qualified includesNVENC_HAVE_QP_MAP_MODEThe sibling
VMAFx/vmafxmigration (7f6e6356b) is weaker prior art than itlooks: its patches are filter-only around
vf_libvmaf.c, needed no API change,and it did not verify compilation.
The architectural win
Each shader now lives once, as
ffmpeg-patches/files/vulkan/pelorus_*.comp.glsl,compiled to SPIR-V at build time. AGENTS.md hard rule 4 and the
shader-lockstep-warnhook existed only to police the old duplication — which hadalready produced the ADR-0129 defect (a split
GLSLFemitting raw macro body intoa shader). That whole bug class is now designed out rather than policed.
Verification
Against a pristine n9.0.1 worktree:
git am --3waymake ffmpeglinks, 0 undefined referencespelorus_fgsBSF register in the linked binaryOBJSu:inf v:inf) on all six luma-only filters atplanes=1— the exact ADR-0129 invariant; the three analyzers are byte-identical pass-throughshwupload,hwdownloadchain with no filter, and07454by upstreamvf_scdet_vulkan(16×), the SSBO-readback analogueTwo defects caught by review that a targeted build hid
OBJSlines were missing theCONFIG_prefix — an undefined makevariable, so no shader object would ever have been pulled in and every filter would
have failed at link. It hid because naming a
.spv.oon a make command line buildsit through the pattern rule regardless. Now verified by linking.
spirv_compilertovulkan_encode_deps, silently disablingupstream
h264_vulkan/hevc_vulkan/av1_vulkanon any host without glslc. Replacedwith a derived
pelorus_qpmap_gpu_if="vulkan_encode spirv_compiler"; verified in bothconfigurations that the upstream encoders survive with no shader compiler present.
Issue #44 — fixed and proven
The parser computed
base + offsetand cast straight into structs; the R5 paddingguarantee is relative to the blob base, so a misaligned base made every field read UB.
Now memcpy-based, plus a
header_sizealignment check the issue did not mention.The new
test_misaligned_blob_basefixture traps the old parser atinterop.c:202under
-fsanitize=alignmentand passes the new one. Wire format proven byte-identical(336 bytes, same hex). No ABI change —
PELORUS_ABI_MINORstays 3.bugprone-casting-through-void— the check that finds this pattern — was suppressed;the pack path is now cast-free too, so it is enabled as a real guard.
Gates added
-fsanitize=alignmentfinds it, and there was no sanitizer gate at all.generate.shis now byte-deterministic (it pinnednothing, so two runs differed by a timestamp and drift was unverifiable). CI now
regenerates and fails on any diff.
glslangValidatorwasrequired: false), and the shipped.comp.glslshaders are now gate-checked.Fast suite 11 → 21 tests.
ubuntu-24.04(the LunarG apt list isrelease-specific),
timeout-minuteson every job,persist-credentials: false,checkout v7.0.1 (supersedes chore(deps): update actions/checkout action to v7.0.1 #46, which would have conflicted on these lines).
Maintenance
README was structurally broken — truncated mid-sentence at EOF with three orphaned
table rows dumped after
## Support; repaired, with four wrong patch numbers corrected.CLAUDE.md/AGENTS.md project state refreshed. ADR-0114 → Accepted, ADR-0132 → Rejected
(its CRF steering is an honest negative; the complexity producer is retained). ADR index
now fully consistent. Repo pack 175.55 MiB → 1.24 MiB (a stash of pure
.bench-outartifacts, then gc). 20 merged local + 2 remote branches deleted.
Deliverables
ADR-0143 · per-surface docs (
backends/vulkan.md,metrics/*, both READMEs) ·changelog fragments · rebase notes · regenerated patch stack.
Known gaps, stated honestly
qsvenc/libaomenc/libsvtav1are not compile-checked — they need the InteloneVPL, libaom and SVT-AV1 SDKs, absent on this box. Their patches apply. This matches
the project's pre-existing caveat for those paths.
Deliberately not re-pinned: the pinned sha256 is what every number in
bench-results.mdwas measured against, so swapping clips would invalidate thecomparison history rather than repair it. Re-pinning resets the baseline and is an
explicit call.
🤖 Generated with Claude Code