Skip to content

build: check in the generated bindings and drop the build script - #8

Merged
kixelated merged 1 commit into
mainfrom
bindings/checked-in
Nov 6, 2026
Merged

kixelated merged 1 commit into
mainfrom
bindings/checked-in

Conversation

@kixelated

@kixelated kixelated commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Part of the moq-video VAAPI quest, moq-dev/moq#8013: the vaapi feature should cost nothing at build time so moq-video can turn it back on by default.

Problem

Building moq-vaapi runs bindgen from build.rs, which needs libclang on the build machine. Every crate that depends on moq-video with vaapi on inherits that, which is why moq-video ships it off by default.

Approach

  • just bindgen generates src/bindings/libva.rs from the vendored libva 2.23 headers, and the file is checked in. build.rs, bindgen_gen.rs, and the bindgen/regex build dependencies are gone. This matches how moq-nvenc and moq-v4l carry their bindings.
  • libva pads VAProcPipelineCaps and VAProcPipelineParameterBuffer by pointer width (__x86_64__ || __LP64__). The recipe generates for x86_64-unknown-linux-gnu and i686-unknown-linux-gnu, then merges the two: lines they share go in as they are, and the differing va_reserved lines go under cfg(any(target_arch = "x86_64", all(target_pointer_width = "64", not(windows)))) and its negation. Any other difference fails the recipe, so a header bump that adds more width-dependent layout cannot slip through.
  • just ci regenerates the bindings and fails on a diff, so the checked-in file stays what the headers produce. just vendor regenerates them after refreshing the headers.
  • The libva_1_X_or_higher cfgs came from the build script reading the header version. The headers are pinned at 2.23, so every gated item was always compiled; the gates are removed.
  • The flake swaps clang for rust-bindgen (bindgen-cli 0.72.1) and drops LIBCLANG_PATH.

Impact

  • Building the crate no longer needs libclang or bindgen. The published tarball excludes libva/ and libva-wrapper.h, which only the recipe reads.
  • The public bindings keep every item and derive they had. bindgen 0.72 adds *_raw bitfield accessors and the raw_get/raw_set helpers on bitfield units, which are additive.

Validation

  • nix develop --command just ci passes, including the regeneration check.
  • Compared the old generated bindings with the new file: the same public items, the same derives, and only the additions above.
  • The merge check fails as intended on a synthetic extra difference between the two targets.

Alternatives

  • Generating per target at build time with a pure-Rust parser: still a build step, and nothing like it handles libva's headers today.
  • One generation with va_reserved hand-patched: silently wrong if a later header adds another width-dependent field, which the two-target diff catches.

Review

Codex (quest review), one round: no findings.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

The bindings are generated once from the vendored libva headers by `just
bindgen` and checked in, so building the crate needs neither bindgen nor
libclang. `just ci` regenerates them and fails on a difference.

libva pads `VAProcPipelineCaps` and `VAProcPipelineParameterBuffer` by
pointer width, so the recipe generates for x86_64 and i686 and merges the
two under a `cfg` on those fields. Any other difference between the two
fails the recipe.

The `libva_1_X_or_higher` cfgs go with the build script: the headers are
pinned at libva 2.23, so every gated item was always compiled in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated added a commit to moq-dev/moq that referenced this pull request Nov 6, 2026
Wire moq-video's VAAPI backends to the moq-vaapi changes in
moq-dev/vaapi#8 through #11: H.265 Main encode and decode beside H.264,
Surface::resize through the processor's pooled output surfaces, and
checked-in libva bindings. With no bindgen left, `vaapi` is on by default
in moq-video, moq-transcode, and moq-cli, and moq-c's Nix build drops its
bindgen hook.

Needs the moq-vaapi release carrying those PRs; the workspace bump follows
it. The quest is rewritten to that remaining work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 6, 2026 09:25
@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown

Explain Change →

Walkthrough

The crate now includes checked-in VA-API bindings generated from vendored headers. The justfile provides binding-generation and drift-check recipes, and the vendor recipe regenerates bindings after refreshing headers. The build script and shared bindgen builder are removed. Several buffer and surface fields, variants, and handling paths no longer use libva-version compile-time gates.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title describes the checked-in bindings and removal of the build script.
Description check Passed The description covers the binding generation changes, removal of the build script, and related checks.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (4 skipped: 4 unsupported.)

Finishing Touches

Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Review coverage is incomplete: 1 file could not be fully reviewed. Findings from completed review steps are included; see review info for details.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3fd7a694-fc31-49fe-a2db-dd102fe4d980
Commits

Reviewing files that changed from the base of the PR and between af121c7 and f92a8fc.

Files selected for processing: 15

  • Cargo.toml
  • README.md
  • bindgen_gen.rs
  • build.rs
  • flake.nix
  • justfile
  • src/bindings.rs
  • src/bindings/libva.rs
  • src/buffer.rs
  • src/buffer/av1.rs
  • src/buffer/enc_misc.rs
  • src/buffer/hevc.rs
  • src/buffer/jpeg_baseline.rs
  • src/lib.rs
  • src/surface.rs
Files with no reviewable changes: 2

  • bindgen_gen.rs
  • build.rs
Files not fully reviewed because their changes were unavailable: 1

  • src/bindings/libva.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@kixelated
kixelated merged commit 55f8929 into main Nov 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant