moq-rtc production hardening: A/V sync, DELETE teardown, dual-stack ICE, VP9, NACK#1869
Merged
Conversation
str0m hands the raw RTP header timestamp on each MediaData event. Per RFC 3550 that base is random and independent for every track, and str0m applies no RTCP sender-report correlation, so the WHIP-server / WHEP-client ingest paths were publishing audio and video on unrelated, multi-hour-offset timelines. The hang catalog/player aligns tracks by timestamp, so A/V was desynced and broadcasts started at an arbitrary offset. Add a per-session IngestClock that anchors each track on its first frame to that packet's arrival time (relative to the first frame in the session) and then advances by the wrap-safe extended RTP delta. The first frame of the session maps to 0 and audio/video share one timeline. Also guard the session loop with rtc.is_alive(): a dead Rtc makes poll_output return a never-firing timeout instead of erroring, which would hang the task forever holding the broadcast announcement and mux registration. Document the --udp-bind server flag (shared media port) in doc/bin/rtc.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cbf7T71JbPhmaodn1aQxxE
… wider NACK buffer Follow-up production hardening on top of the timestamp-rebase fix: - WHIP/WHEP DELETE / session teardown (RFC 9725). The bundled routers now honor DELETE on the resource URL, ending a session promptly and releasing its broadcast announcement + shared-media-port registration instead of waiting for the ICE disconnect timeout. Server keeps a resource_id -> cancel-sender registry; the session task selects on it and unregisters on exit. Exposed as Server::terminate for embedders that own their routing. Wires up the previously-unused sdp::parse_resource_id / SessionNotFound. - Dual-stack ICE candidate tagging. The session now tags each inbound datagram with the advertised candidate whose address family matches the packet source, instead of always candidates[0], so an IPv6 peer on a v4+v6 deployment isn't told its packet arrived on an IPv4 host candidate. Session::ingest/egress take the candidate list; Mux::local() is gone. - VP9 keyframe detection now parses the uncompressed header properly: profile 3's extra reserved bit and show_existing_frame (which carries no frame_type) are handled, not just profile 0-2. - Late joiners: a fresh WHEP subscriber already starts at the current (in-progress) MoQ group, which begins at a keyframe (documented at the subscribe call). MoQ has no PLI path upstream, so widen str0m's egress video send buffer above the default so the peer can NACK-recover a large keyframe and the rest of the current group while it's still in flight. Adds unit tests for VP9 parsing, candidate selection, and the session registry; updates doc/bin/rtc.md (keyframe behavior, DELETE, send buffer). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cbf7T71JbPhmaodn1aQxxE
- IngestClock: only rebase on ingest sessions (the work now lives inside the MediaRole::Ingest guard instead of running for every MediaData event), and use a signed wall delta from the session epoch so a track whose first frame is dequeued after the epoch but actually arrived earlier keeps its lead instead of being clamped to the epoch by an unsigned subtraction. - Downgrade ordinary peer disconnects (Error::SessionClosed) from warn to debug via a shared session::log_session_end helper, so normal WebRTC churn stays out of the warning stream. Used by all four session tasks. Adds a regression test for the before-epoch cross-track case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cbf7T71JbPhmaodn1aQxxE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Production-readiness review of
moq-rtcA review + hardening pass on the WebRTC (WHIP/WHEP) gateway on
dev(through #1864, the shared-UDP-port mux). The crate was already in good shape (clean module split, embeddable library boundary,#[non_exhaustive]configs/errors, good docs). This PR fixes the correctness/robustness issues found. Clippy is clean (only a pre-existingmoq-nativewarning) and tests pass (22: 17 unit + 5 integration).Fixes
A/V desync on ingest (the headline bug). str0m hands the raw RTP header timestamp per
MediaData. Per RFC 3550 that base is random and independent per track, with no RTCP sender-report correlation, so WHIP-server / WHEP-client ingest was publishing audio and video on unrelated, multi-hour-offset timelines (broken lip-sync, arbitrary start offset). Added a per-sessionIngestClockthat anchors each track on its first frame to that packet's arrival time (relative to the first frame in the session) and advances by the wrap-safe extended RTP delta. First frame maps to0and audio/video share one timeline.Dead-session leak backstop. A dead
Rtcmakespoll_outputreturn a never-firing timeout instead of erroring, hanging the task forever holding the broadcast announcement + mux registration. Added anrtc.is_alive()guard so those release.WHIP/WHEP
DELETE/ session teardown (RFC 9725). The bundled routers now honorDELETEon the resource URL, ending a session promptly and releasing its announcement + shared-media-port registration instead of waiting for the ICE disconnect timeout.Serverkeeps aresource_id -> cancel-senderregistry; the session task selects on it and unregisters on exit. Exposed asServer::terminatefor embedders that own their routing. Wires up the previously-unusedsdp::parse_resource_id/Error::SessionNotFound.Dual-stack ICE candidate tagging. The session tags each inbound datagram with the advertised candidate whose address family matches the packet source, instead of always
candidates[0], so an IPv6 peer on a v4+v6 deployment isn't told its packet arrived on an IPv4 host candidate.VP9 keyframe detection now parses the uncompressed header properly: profile 3's extra reserved bit and
show_existing_frame(which carries noframe_type), not just profile 0-2.Late joiners. A fresh WHEP subscriber already starts at the current (in-progress) MoQ group, which begins at a keyframe (now documented at the subscribe call). MoQ has no PLI path upstream, so str0m's egress video send buffer is widened above the default (1000 -> 3000 packets) so the peer can NACK-recover a large keyframe and the rest of the current group while it's still in flight.
Log hygiene. Ordinary peer disconnects (
Error::SessionClosed) now log at debug, not warn, via a sharedsession::log_session_endhelper, so normal WebRTC churn stays out of the warning stream.Docs.
doc/bin/rtc.md: documented--udp-bind, the updated keyframe/NACK behavior, andDELETEteardown.Public API changes (on
dev, breaking allowed)Server::terminate(&str) -> bool.session::Session::ingest/egress:local: SocketAddr->locals: Vec<SocketAddr>. (Documented embedder path iswhip::accept/whep::accept/Client, which are unaffected.)Mux::local()(waspub(crate)).Still flagged (not in this PR)
Test plan
cargo clippy -p moq-rtc --all-targets --all-features(clean)cargo test -p moq-rtc --all-features(22 passed: 17 unit + 5 integration)cargo fmt --check -p moq-rtc🤖 Generated with Claude Code
(Written by Claude)