hang: non_exhaustive VideoConfig/AudioConfig with constructors#1485
Conversation
Mark both catalog config structs as `#[non_exhaustive]` so future optional fields (e.g. a `broadcast: Option<PathRelativeOwned>`) can be added without a major bump. External callers now build configs via `VideoConfig::new(codec)` and `AudioConfig::new(codec, sample_rate, channel_count)`, then assign the optional fields they care about. Follows the pattern from 9f780fa (`AuthToken`, `moq_msf::Track`). Migrates every in-tree struct-literal site across `hang` and `moq-mux`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
WalkthroughThis PR introduces public constructors for 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Origin/main (#1485) marked VideoConfig and AudioConfig #[non_exhaustive] and added `::new()` constructors that the existing in-tree call sites now use. Resolved by taking main's version of every moq-mux import site and the hang/examples/video.rs example (they use the new constructor pattern) and re-adding the optional `broadcast` field to the structs + constructors. The catalog tests in root.rs likewise switch to the builder style. Written by Claude Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Main landed #1485 (`hang: non_exhaustive VideoConfig/AudioConfig with constructors`) while this PR was open. The struct-literal calls in `moq-audio` and `moq-ffi` are no longer allowed; switch to `AudioConfig::new(codec, sample_rate, channel_count)` and assign the optional `bitrate` / `description` / `container` fields afterward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Main landed #1485 (`hang: non_exhaustive VideoConfig/AudioConfig with constructors`) while this PR was open. The struct-literal calls in `moq-audio` and `moq-ffi` are no longer allowed; switch to `AudioConfig::new(codec, sample_rate, channel_count)` and assign the optional `bitrate` / `description` / `container` fields afterward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
hang::catalog::VideoConfigandAudioConfigas#[non_exhaustive]so additional optional fields can be added without bumping the major version.VideoConfig::new(codec)andAudioConfig::new(codec, sample_rate, channel_count)constructors that clear every optional field and defaultcontainerviaContainer::default(). Callers assign whichever optional fields they need afterwards.hangandmoq-muxto the new constructors.Why
Without
#[non_exhaustive], every additive field on these public structs is a SemVer-breaking change for downstream Rust users that construct via struct literals. The previous attempt to add#[non_exhaustive]was rolled back becausemoq-mux(a separate crate) could no longer build via struct literals; thenew(...)constructors are the unblocker.This follows the pattern established in 9f780fa ("Tighten public APIs ahead of release: non_exhaustive + builders") for
AuthTokenandmoq_msf::Track.Notes for reviewers
VideoConfig::newonly takescodec(everything else is optional or hasDefault);AudioConfig::newtakescodec,sample_rate, andchannel_countsince the latter two have no sensible default and a wrong value silently distorts playback.impl Into<{Video,Audio}Codec>, so callers can pass anH264 { ... }(or other variant payload) directly without.into().rs/moq-mux/src/container/mkv/{import,export_test}.rsand several test cases inrs/moq-mux/src/catalog/hang/producer.rs— because they would otherwise fail to compile against the#[non_exhaustive]types.Test plan
just check(rustc, clippy -D warnings, fmt, cargo doc, cargo sort, biome, remark)just rs test— all suites pass, including the catalog roundtrip tests inhangand the hang/msf catalog tests inmoq-mux🤖 Generated with Claude Code