[kixelated]

Finishes quest/m1/vaapi-radeonsi.md.

Summary

• Bumps moq-vaapi to 0.2.1 (root Cargo.toml, Cargo.lock). 0.2.1 carries moq-dev/vaapi#13: a packed slice header, so radeonsi writes the SPS, PPS, and slice NAL header; a QP ceiling of 51 instead of 36, so rate control can reach the target bitrate; and explicit VPP color spaces on Mesa. The workspace-hack needs no change (cargo hakari generate --diff is clean).
• rs/moq-video/src/encode/backend/vaapi.rs module docs now say the tests pass on radeonsi, and the 1080p ladder test's #[ignore] reason no longer says the device must be Intel.
• Deletes the quest and its links.

Verification

On this host's AMD Radeon RX 9070 (/dev/dri/renderD129, radeonsi, gfx1201) with nixpkgs Mesa 26.2.6 and libva 2.24.1:

MOQ_VAAPI_DEVICE=/dev/dri/renderD129 LIBVA_DRIVER_NAME=radeonsi \
  LIBVA_DRIVERS_PATH=<mesa-26.2.6>/lib/dri LD_LIBRARY_PATH=<libva-2.24.1>/lib \
  moq_video-<hash> vaapi --include-ignored --test-threads=1
test result: ok. 20 passed; 0 failed; 0 ignored

Built with cargo test -p moq-video --all-features --lib. All 20 VA-API tests pass, including vaapi_1080p_ladder_stays_on_the_gpu, which panics instead of skipping when no hardware is found. The libva log shows every test opening radeonsi_drv_video.so. Without libva on the library path, the same tests skip and report pass in 0.03s, so the numbers above come from the hardware run, not the skip. just check passes.

Choices made without asking

• Tested with nixpkgs Mesa 26.2.6 rather than the host's Ubuntu Mesa 26.0.8. The dev shell's binaries cannot load the host's libva-drm without mixing in host libdrm and glibc. The quest's earlier evidence was from Mesa 26.2.5.
• Changed the ladder test's ignore reason from "Intel VA-API" to "VA-API", since it passes on AMD too.

Public API: none. Wire: none.

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, medium effort

(Written by Claude Opus 5.5 with medium effort)

🤖 Generated with Claude Code

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1b998-7d1f-7bb0-97c6-95f5555f53b8, round 1
• Head: 3648673
• Base: origin/main at 079a646
• Score: 10/10, Merge of main plus a doc fix naming the ignored hardware test that fails rather than skips; no findings.
• Findings: none

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 26 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

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: bb95ca0d-37c7-4b0f-9f43-aa095d336bb4

Commits

Reviewing files that changed from the base of the PR and between 079a646 and 3648673.

Files selected for processing: 1

• rs/moq-video/src/encode/backend/vaapi.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

[pullfrog]

ℹ️ No critical issues — one doc wording correction inline.

Reviewed changes

• moq-vaapi bump: root Cargo.toml and Cargo.lock move 0.2.0 → 0.2.1. The lock checksum matches the published tarball exactly, and its .cargo_vcs_info.json points at the tag commit that carries moq-dev/vaapi#13 plus the release commit; the 0.2.0→0.2.1 source diff has no public API change.
• Module docs and ignore reason: rs/moq-video/src/encode/backend/vaapi.rs records the radeonsi validation and the ladder test's reason no longer says "Intel".
• Quest removal: deletes quest/m1/vaapi-radeonsi.md, its m1 ## Required bullet, and the inbound link from quest/m3/h265-min-width.md. No references to the file or title remain, the test-inventory allowlist still points at quest/m4/vaapi-render-tests.md, and the Quest check passes.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed head f90f93a.

No actionable findings. The manifest and lockfile select moq-vaapi 0.2.1, whose upstream tag identifies that release, and upstream #13 is merged. Taking the packed-header, rate-control and VPP fixes through the dependency avoids duplicating driver-specific work in moq-video. The integration calls and test assertions are unchanged; the hardware test's ignore reason is broadened without weakening its assertions. Quest removal and link cleanup match the planned remaining work.

Direction: retain the upstream dependency fix. The documented hardware result is specific to RX 9070/radeonsi with Mesa 26.2.6, rather than evidence for every AMD/driver combination.

Verification limits: GitHub-only static review of the diff, complete VA-API integration/test module, upstream release metadata and merged PR description. I did not build, run hardware tests, reproduce image/bitrate measurements, verify the registry tarball, or independently validate the author's 20-test result. Rechecked open/non-draft state, full head and prior reviews immediately before posting.

[pullfrog]

✅ No new issues found.

Reviewed changes

The only code delta since the prior pullfrog review is a doc comment correction in rs/moq-video/src/encode/backend/vaapi.rs; merging main also collapsed the dependency bump and quest deletion, which already landed upstream.

• Hardware note corrected: the module docs now record Mesa 26.2.6 and name vaapi_1080p_ladder_stays_on_the_gpu as the one test that fails rather than skips without a device, resolving the prior inline feedback.
• Ignore reason broadened: the ladder test's #[ignore] reason no longer claims to require an Intel device.
• Merge of main: #8235 vendored moq-vaapi in-tree at 0.2.1 and deleted the quest, leaving this PR doc-only.

ℹ️ PR title and body still describe changes main already landed

The body still says this PR bumps moq-vaapi in Cargo.toml/Cargo.lock and deletes quest/m1/vaapi-radeonsi.md, but the merge with main (which now carries #8235) leaves only the doc comment in the diff. A squash merge would record a commit message claiming changes that are not part of this PR.

Technical details

# Stale PR description after merging main

## Affected sites
- PR body, "Summary" bullets — claims a `moq-vaapi` 0.2.0 → 0.2.1 bump in root `Cargo.toml`/`Cargo.lock` and quest deletion, neither of which is in `git diff 079a6460f..HEAD`.
- PR title, "(moq-vaapi 0.2.1)" — the dependency is now the in-tree `rs/moq-vaapi` path dependency at 0.2.1.

## Required outcome
- The landing commit message describes what this PR actually changes: the module docs and the ladder test's ignore reason.

## Suggested approach (optional)
- Trim the Summary to the doc correction, or retitle to name the doc fix rather than the dependency bump.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}