fix(moq-video): convert and label RGB captures in one color space

[kixelated]

Stacked on #2552 (the render work), which is where Color and Color::infer come from. Base this on main once that lands.

The bug

Every RGB-to-I420 conversion hardcoded BT.601 limited range, and nothing wrote a VUI color description into the encoded bitstream. So a 1080p screen capture on Windows or Linux was converted with BT.601, encoded untagged, and then decoded by any standards-compliant player using the usual size heuristic (BT.601 at 576 lines and below, BT.709 above). The player applies the BT.709 inverse to BT.601 samples.

Saturated colors skew. Pure red arrives as (255, 24, 0), confirmed by round-tripping a 1080p frame through I420::from_rgba and back through the BT.709 inverse. Grays sit on the achromatic axis and are untouched, which is why this survived casual inspection.

The fix

Both halves, against one source of truth. Color::infer(size) already encoded the player's heuristic for the renderer, so:

• the RGB conversions pick their matrix from it instead of hardcoding BT.601, and
• the encoders write that same answer into the bitstream's VUI.

The pixels and the label are derived from the same function for the same size, so they cannot drift. encode::Config gains an optional color override for callers feeding frames the crate did not convert; None means Color::infer.

Why both, rather than just the VUI

These are complementary, not alternatives. Writing the VUI removes the guess for a compliant decoder, but it does not reach every path:

Backend	VUI	Verified
openh264	VuiConfig	SPS parsed back, 480p + 1080p
VideoToolbox	3 session properties	SPS parsed back, 480p + 1080p
NVENC	h264VUIParameters / hevcVUIParameters	compiles (Linux, container)
Media Foundation	4 MF_MT_* attrs on both media types	compiles on the Windows CI runner
VAAPI	not expressible	conversion half only

VAEncSequenceParameterBufferH264 has no colour description fields at all (its vui_fields covers aspect ratio, timing, and bitstream restriction), so the VAAPI backend cannot tag its output today. Media Foundation treats the color attributes as a request a driver may ignore. Picking the matrix by resolution is what keeps both of those correct, because it makes the untagged interpretation the right one.

Testing

The regression tests read the color description back out of the emitted SPS rather than trusting our own config, so a backend that silently drops the VUI fails rather than passing on bookkeeping. h264-reader is a new dev-dependency for that, rather than hand-rolling exp-Golomb in a test.

• rgb_conversion_follows_the_size_heuristic covers 480p/576p/720p/1080p. Mutation-checked both ways: reverting the matrix while keeping the label honest reproduces [255, 24, 0] exactly, and reverting the label alone also fails.
• the_sps_declares_the_color_space (openh264) and videotoolbox_sps_declares_the_color_space parse the real bitstream at 480p and 1080p.
• 64 tests pass on macOS; cargo check --all-targets with nvenc,vaapi is clean in a Fedora container; --features render still builds.

Reviewer notes

• Media Foundation compiles but is not run. The Windows CI job is green (it caught a missing IMFMediaType import along the way), so the code builds against the real SDK. Nothing exercises it at runtime, since CI has no encoder hardware.
• VAAPI's VUI is the one gap. moq-vaapi writes its own packed SPS, so the colour description could go in there, but that is a separate repo and a version bump. Not urgent: the conversion half already makes VAAPI correct.
• Color::limited() moves back onto Color from render/color.rs. feat(moq-video): render decoded frames on the GPU, and carry color spaces end to end #2552 made it a private render helper on the reasoning that deriving a matrix is the shader's business; the encoders now need it for video_full_range_flag, so it is no longer render-only. weights() stays in render/.
• The from_yuyv fix in feat(moq-video): render decoded frames on the GPU, and carry color spaces end to end #2552 (64bb705) and mine were the same change, arrived at independently. Kept feat(moq-video): render decoded frames on the GPU, and carry color spaces end to end #2552's version and dropped my duplicate test.
• No wire or catalog change, so no draft update. Whether hang's catalog should carry the color description is a real question, but it is redundant with the bitstream and needs a draft-lcurley-moq-hang.md update, so it belongs in its own PR.

Update: resized frames (review follow-up)

The encoder took its VUI from the config size, but Frame::resize carries the
pixels' color space across. Scaling an HD frame down to a sub-576-line rung
produced BT.709 samples under a BT.601 label, the same mismatch one layer up.

Surface::color is now public and answers for macOS pixel buffers by reading the
matrix VideoToolbox copies out of the source VUI, so decoded frames report their
real space. moq-transcode opens its encoders from the first decoded frame and
passes that space through, so a rung inherits the source's rather than inventing
one. Encoder::encode warns once on a residual mismatch and names
Config::color as the fix; it warns rather than rejects because the VUI is fixed
at session open, so the alternatives are a mislabeled stream or a dropped one,
and a gateway should keep serving.

(written by Opus 5)

[sourcery-ai]

Hey - I've left some high level feedback:

• In test_util::declared_color, panicking on unexpected matrix_coefficients values (panic!("unexpected matrix_coefficients {other}")) makes this helper fragile to any future encoder/driver emitting other valid matrices; consider returning None or a dedicated error instead so tests can handle that gracefully.
• The transfer characteristics used for BT.601 differ between backends (e.g., NVENC uses SMPTE170M while MediaFoundation/VideoToolbox use the 709 transfer curve); it may be worth aligning these choices or documenting the intentional mismatch so behavior is consistent across encoders.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `test_util::declared_color`, panicking on unexpected `matrix_coefficients` values (`panic!("unexpected matrix_coefficients {other}")`) makes this helper fragile to any future encoder/driver emitting other valid matrices; consider returning `None` or a dedicated error instead so tests can handle that gracefully.
- The transfer characteristics used for BT.601 differ between backends (e.g., NVENC uses `SMPTE170M` while MediaFoundation/VideoToolbox use the 709 transfer curve); it may be worth aligning these choices or documenting the intentional mismatch so behavior is consistent across encoders.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}