fix(recordings): set explicit rate control on VAAPI playback transcode

[davlaw]

Summary

The HEVC → H.264 playback-compatibility transcode in recording_transcode.c encodes with h264_vaapi but passes no rate-control options, so the driver falls back to its default quality level (No quality level set; using default (20), hidden by -loglevel error) with no bound on bitrate.

This adds -rc_mode CQP -qp 24 to the VAAPI invocation.

Why CQP and not -b:v/-maxrate: some VAAPI drivers support only CQP. On an Intel HD 630 (Kaby Lake, iHD), -b:v 6M -maxrate 6M fails outright with Driver does not support any RC mode compatible with selected options (supported modes: CQP) and the encoder refuses to open. CQP is the mode drivers support most widely. If a driver rejects it anyway, the existing software fallback still produces the cache, so the worst case is the same as today's VAAPI-failure path.

The software (libx264) path is unchanged; x264 already defaults to CRF 23.

Measured

10 s clip copied straight from a 2560x1920 HEVC camera, transcoded with the exact old and new argument lists on an HD 630:

	size	bitrate
HEVC source	1.7 MB	1.4 Mbit/s
before (no rate control)	10.4 MB	8.3 Mbit/s
after (CQP, qp 24)	6.3 MB	5.0 Mbit/s

That's 40% smaller. On busier scenes the gap is larger: the same unbounded-quality setting in a live hardware republish on this host produced ~21 Mbit/s, enough to make the recordings player stall. With qp 24 it dropped to ~10-13 Mbit/s and playback was smooth.

Tests

New test_vaapi_transcode_sets_explicit_rate_control puts a wrapper ffmpeg on PATH (same technique as the existing concurrency tests, now sharing an install_ffmpeg_wrapper() helper). The wrapper records the VAAPI invocation's arguments and exit status, then passes through to the real encoder. Checking the exit status proves the driver actually accepted the options; the request could otherwise succeed quietly via the software fallback. The test is skipped when /dev/dri/renderD128 is absent, so it is a no-op on CI.

• Failed before the fix (VAAPI encode is missing -rc_mode CQP), passes after.
• test_recording_transcode: 17/17 on a VAAPI host.
• Full ctest otherwise unchanged from main. Unrelated pre-existing failures on current main: test_url_utils (test_url_apply_credentials_replaces_existing_credentials expects %3A, gets %3a) and the two argv-driven SOD tools (test_sod_unified, test_sod_voc), which print usage when run with no arguments.

No architecture-specific code; only an argument list changes.

---

Investigated, implemented and tested by Claude (Anthropic) via Claude Code, working with @davlaw.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The new test can incorrectly fail on systems with a render node but without a usable VAAPI pipeline.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Skip VAAPI test when baseline pipeline is unsupported · New
• Assert the rate-control QP argument equals exactly 24 · New

What changed in this PR

Adds explicit CQP rate control to VAAPI playback transcoding to reduce output bitrate.

Changes:

• Configures h264_vaapi with CQP and QP 24.
• Refactors the FFmpeg test wrapper and adds VAAPI argument validation.

File	Description
src/​video/​recording_transcode.c	Adds explicit VAAPI rate-control arguments.
tests/​unit/​test_recording_transcode.c	Adds VAAPI regression coverage and shared wrapper setup.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The regression test is neither selected by CI nor runnable on GPU-less CI workers.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Skip VAAPI test when baseline pipeline is unsupported
• Assert the rate-control QP argument equals exactly 24

Previously missed (1)

In code that hasn't changed since last review

Add recording transcode coverage to CI and GPU-independent tests

tests/​unit/​test_recording_transcode.c:639

This regression is not exercised by CI. test_recording_transcode is absent from both explicit build and ctest allowlists in .github/workflows/integration-test.yml:69-123; moreover, this case is ignored on GPU-less runners. Please add the target to those allowlists and split out a hardware-independent wrapper test that verifies the production argv, retaining the driver-backed check as an optional smoke test.