Skip to content

Commit 9968539

Browse files
kixelatedclaude
andauthored
moq-rtc production hardening: A/V sync, DELETE teardown, dual-stack ICE, VP9, NACK (#1869)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8fc863f commit 9968539

10 files changed

Lines changed: 403 additions & 61 deletions

File tree

doc/bin/rtc.md

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,14 @@ and are the same shape regardless of HTTP role.
2525

2626
### Keyframe latency on the egress side
2727

28-
WebRTC subscribers expect a keyframe within ~2 s of joining. If the
29-
upstream MoQ broadcast uses long GOPs, freshly-connected WHEP / WHIP-out
30-
peers see a black screen until the next natural keyframe arrives.
31-
`KeyframeRequest` events from the peer are logged but not propagated
32-
upstream; PLI-to-MoQ back-pressure is a future enhancement.
28+
A freshly-connected WHEP / WHIP-out peer subscribes at the *current*
29+
(in-progress) MoQ group, which begins at a keyframe, so it gets a
30+
decodable start without waiting for the next GOP boundary. If the peer
31+
loses keyframe packets, str0m fulfils its NACK retransmissions from the
32+
video send buffer, which is sized to cover a large keyframe plus the rest
33+
of the current group. MoQ has no PLI path back to the publisher, so
34+
`KeyframeRequest` (PLI/FIR) events from the peer are logged but not
35+
propagated upstream.
3336

3437
The egress paths (WHEP server, WHIP client) negotiate H.264, H.265, VP8,
3538
VP9, AV1, and Opus. The ingest paths (WHIP server, WHEP client) currently
@@ -70,13 +73,28 @@ moq-rtc --relay https://relay.example.com --broadcast my-stream \
7073
### Server flags
7174

7275
- `--listen`: HTTP bind address (default `[::]:8088`).
76+
- `--udp-bind`: UDP address the shared WebRTC media socket binds to
77+
(default `0.0.0.0:0`, i.e. an OS-picked port for dev/loopback). Every
78+
WHIP/WHEP session shares this one port (demuxed by ICE ufrag), so a
79+
deployment behind a firewall pins it (e.g. `0.0.0.0:8089`) and opens just
80+
that one media port. Pair it with `--public-addr` so the advertised ICE
81+
candidate uses the pinned port.
7382
- `--tls-cert` / `--tls-key`: serve HTTPS instead. Most WHIP clients
7483
require it in practice.
7584

7685
### Client flags
7786

7887
- `--url`: remote WHIP or WHEP resource URL.
7988

89+
### Session teardown
90+
91+
The bundled WHIP/WHEP servers honor an HTTP `DELETE` to the resource URL
92+
returned in the `Location` header (`/<broadcast>/<resource-id>`), per
93+
RFC 9725. It ends the session promptly, releasing its broadcast
94+
announcement and shared-media-port registration instead of waiting for the
95+
ICE disconnect timeout. Embedders that own their own routing can call
96+
`Server::terminate(resource_id)` to do the same.
97+
8098
## Codec mapping
8199

82100
| WebRTC codec | MoQ catalog | Egress | Ingest |

rs/moq-rtc/src/client/whep.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,12 @@ pub(crate) async fn dial(client: &Client, url: Url, broadcast: moq_net::Broadcas
6161
tracing::info!(%url, "whep client connected");
6262

6363
// 1:1 socket (no demux on the client): pump its datagrams into the session.
64-
// Report the first advertised candidate as the local address (str0m matches
65-
// each datagram's destination against a host candidate, not the socket bind).
66-
let local = candidates[0];
64+
// The session tags each datagram with the advertised candidate matching its
65+
// family (str0m matches the destination against a host candidate, not the bind).
6766
let inbound = session::spawn_socket_reader(socket.clone());
68-
let session = session::Session::ingest(rtc, socket, local, inbound, sink);
67+
let session = session::Session::ingest(rtc, socket, candidates, inbound, sink);
6968
tokio::spawn(async move {
70-
if let Err(err) = session.run().await {
71-
tracing::warn!(%err, "whep client session ended");
72-
}
69+
session::log_session_end("whep client", session.run().await);
7370
});
7471

7572
Ok(())

rs/moq-rtc/src/client/whip.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,12 @@ pub(crate) async fn dial(client: &Client, url: Url, broadcast: moq_net::Broadcas
6767
tracing::info!(%url, "whip client connected");
6868

6969
// 1:1 socket (no demux on the client): pump its datagrams into the session.
70-
// Report the first advertised candidate as the local address (str0m matches
71-
// each datagram's destination against a host candidate, not the socket bind).
72-
let local = candidates[0];
70+
// The session tags each datagram with the advertised candidate matching its
71+
// family (str0m matches the destination against a host candidate, not the bind).
7372
let inbound = session::spawn_socket_reader(socket.clone());
74-
let session = session::Session::egress(rtc, socket, local, inbound, source);
73+
let session = session::Session::egress(rtc, socket, candidates, inbound, source);
7574
tokio::spawn(async move {
76-
if let Err(err) = session.run().await {
77-
tracing::warn!(%err, "whip client session ended");
78-
}
75+
session::log_session_end("whip client", session.run().await);
7976
});
8077

8178
Ok(())

rs/moq-rtc/src/codec/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ impl Track {
8282
/// Audio track for an Opus rendition.
8383
pub async fn opus(broadcast: &moq_net::BroadcastConsumer, name: &str) -> Result<Self> {
8484
let container = moq_mux::catalog::hang::Container::Legacy;
85+
// `None` subscription => start at the latest (in-progress) group. Groups
86+
// begin at a keyframe, so a late joiner gets a decodable start immediately
87+
// rather than waiting for the next group boundary.
8588
let track = broadcast.track(name)?.subscribe(None)?.await?;
8689
let consumer = moq_mux::container::Consumer::new(track, container);
8790
Ok(Self {
@@ -96,6 +99,8 @@ impl Track {
9699
/// `config.description` (avc1/hvc1 vs avc3/hev1).
97100
pub async fn video(broadcast: &moq_net::BroadcastConsumer, name: &str, config: &VideoConfig) -> Result<Self> {
98101
let container: moq_mux::catalog::hang::Container = (&config.container).try_into()?;
102+
// `None` subscription => start at the latest (in-progress) group, which
103+
// begins at a keyframe, so a late-joining peer gets a decodable start.
99104
let track = broadcast.track(name)?.subscribe(None)?.await?;
100105
let consumer = moq_mux::container::Consumer::new(track, container);
101106

rs/moq-rtc/src/codec/vp9.rs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ impl codec::Bridge for Bridge {
4545
self.announce();
4646
let pts = moq_net::Timestamp::from_micros(frame.timestamp_us)
4747
.map_err(|err| crate::Error::Other(anyhow::anyhow!("invalid timestamp: {err}")))?;
48-
// VP9 uncompressed header: bit 2 is frame_type (0 = keyframe).
49-
let keyframe = frame.payload.first().map(|b| (b & 0b0000_0100) == 0).unwrap_or(false);
48+
let keyframe = is_keyframe(&frame.payload);
5049
self.track
5150
.write(moq_mux::container::Frame {
5251
timestamp: pts,
@@ -59,8 +58,64 @@ impl codec::Bridge for Bridge {
5958
}
6059
}
6160

61+
/// Detect a VP9 keyframe from the uncompressed header's first byte (VP9 spec
62+
/// §6.2), reading bits MSB-first: `frame_marker(2)`, `profile_low(1)`,
63+
/// `profile_high(1)`, a `reserved(1)` bit only when profile == 3,
64+
/// `show_existing_frame(1)`, then `frame_type(1)` (0 == KEY_FRAME). A
65+
/// show-existing frame carries no frame_type and is never a keyframe.
66+
fn is_keyframe(payload: &[u8]) -> bool {
67+
let Some(&b) = payload.first() else {
68+
return false;
69+
};
70+
let profile = (((b >> 4) & 1) << 1) | ((b >> 5) & 1); // (high << 1) | low
71+
// Bits consumed from the MSB: 2 (marker) + 2 (profile), plus profile 3's reserved bit.
72+
let mut pos = 4;
73+
if profile == 3 {
74+
pos += 1;
75+
}
76+
let show_existing_frame = (b >> (7 - pos)) & 1;
77+
if show_existing_frame == 1 {
78+
return false;
79+
}
80+
pos += 1;
81+
let frame_type = (b >> (7 - pos)) & 1;
82+
frame_type == 0
83+
}
84+
6285
impl Drop for Bridge {
6386
fn drop(&mut self) {
6487
self.catalog.lock().video.renditions.remove(self.track.track().name());
6588
}
6689
}
90+
91+
#[cfg(test)]
92+
mod tests {
93+
use super::is_keyframe;
94+
95+
// frame_marker = 0b10 in the top two bits for every well-formed header.
96+
#[test]
97+
fn profile0_keyframe_and_interframe() {
98+
// profile 0, show_existing_frame = 0, frame_type = 0 (key) / 1 (inter).
99+
assert!(is_keyframe(&[0b1000_0010]));
100+
assert!(!is_keyframe(&[0b1000_0110]));
101+
}
102+
103+
#[test]
104+
fn profile0_show_existing_frame_is_not_keyframe() {
105+
// profile 0, show_existing_frame = 1: no frame_type follows.
106+
assert!(!is_keyframe(&[0b1000_1000]));
107+
}
108+
109+
#[test]
110+
fn profile3_keyframe_and_interframe() {
111+
// profile 3 (both profile bits set) inserts a reserved bit before
112+
// show_existing_frame, shifting frame_type one position right.
113+
assert!(is_keyframe(&[0b1011_0000])); // reserved=0, show=0, frame_type=0
114+
assert!(!is_keyframe(&[0b1011_0010])); // frame_type=1
115+
}
116+
117+
#[test]
118+
fn empty_payload_is_not_keyframe() {
119+
assert!(!is_keyframe(&[]));
120+
}
121+
}

rs/moq-rtc/src/server/mod.rs

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@ pub mod whip;
1111

1212
mod mux;
1313

14+
use std::collections::HashMap;
1415
use std::net::SocketAddr;
15-
use std::sync::Arc;
16+
use std::sync::{Arc, Mutex};
1617

1718
use axum::Router;
18-
use tokio::sync::OnceCell;
19+
use axum::extract::{Path, State};
20+
use axum::http::StatusCode;
21+
use tokio::sync::{OnceCell, oneshot};
1922

2023
use crate::Result;
2124
use mux::Mux;
@@ -76,6 +79,10 @@ struct Inner {
7679
/// The shared media socket + demux, bound lazily on the first accept so
7780
/// `Server::new` can stay synchronous (and an idle server binds no port).
7881
mux: OnceCell<Mux>,
82+
/// Live sessions keyed by resource id, each holding a cancel sender the
83+
/// session task selects on. Lets [`Server::terminate`] (and the bundled
84+
/// `DELETE` route) end a session by its `Location` id.
85+
sessions: Mutex<HashMap<String, oneshot::Sender<()>>>,
7986
}
8087

8188
impl Server {
@@ -88,6 +95,7 @@ impl Server {
8895
publisher,
8996
subscriber,
9097
mux: OnceCell::new(),
98+
sessions: Mutex::new(HashMap::new()),
9199
}),
92100
}
93101
}
@@ -129,4 +137,77 @@ impl Server {
129137
pub(crate) fn subscriber(&self) -> &moq_net::OriginConsumer {
130138
&self.inner.subscriber
131139
}
140+
141+
/// Register a session under its resource id, returning the cancel receiver
142+
/// the session task selects on. Called by [`whip::accept`] / [`whep::accept`]
143+
/// before spawning the session.
144+
pub(crate) fn register_session(&self, resource_id: String) -> oneshot::Receiver<()> {
145+
let (tx, rx) = oneshot::channel();
146+
self.inner.sessions.lock().unwrap().insert(resource_id, tx);
147+
rx
148+
}
149+
150+
/// Drop a session's registry entry once it has ended on its own.
151+
pub(crate) fn unregister_session(&self, resource_id: &str) {
152+
self.inner.sessions.lock().unwrap().remove(resource_id);
153+
}
154+
155+
/// Terminate a negotiated session by its resource id (the `Location` path
156+
/// component from the WHIP/WHEP response). Returns `true` if a live session
157+
/// was found and signalled to stop; the session task then releases its
158+
/// broadcast announcement and mux registration. Embedders that own their own
159+
/// HTTP routing call this to honor a WHIP/WHEP `DELETE`; the bundled routers
160+
/// already wire it to the `DELETE` method.
161+
pub fn terminate(&self, resource_id: &str) -> bool {
162+
if let Some(cancel) = self.inner.sessions.lock().unwrap().remove(resource_id) {
163+
let _ = cancel.send(());
164+
true
165+
} else {
166+
false
167+
}
168+
}
169+
}
170+
171+
/// Shared `DELETE` handler for both bundled routers: parse the resource id from
172+
/// the trailing path segment and terminate the matching session.
173+
pub(crate) async fn delete(State(server): State<Server>, Path(path): Path<String>) -> StatusCode {
174+
match crate::sdp::parse_resource_id(&path) {
175+
Ok(id) if server.terminate(&id.to_string()) => StatusCode::OK,
176+
Ok(_) => StatusCode::NOT_FOUND,
177+
Err(_) => StatusCode::BAD_REQUEST,
178+
}
179+
}
180+
181+
#[cfg(test)]
182+
mod tests {
183+
use super::*;
184+
185+
fn server() -> Server {
186+
let publisher = moq_net::Origin::random().produce();
187+
let subscriber = moq_net::Origin::random().produce().consume();
188+
Server::new(Config::default(), publisher, subscriber)
189+
}
190+
191+
#[test]
192+
fn terminate_unknown_session_is_false() {
193+
assert!(!server().terminate("00000000-0000-0000-0000-000000000000"));
194+
}
195+
196+
#[test]
197+
fn terminate_registered_session_once() {
198+
let server = server();
199+
let id = "11111111-1111-1111-1111-111111111111";
200+
let _cancel = server.register_session(id.to_string());
201+
assert!(server.terminate(id), "first terminate finds the session");
202+
assert!(!server.terminate(id), "second terminate is a no-op");
203+
}
204+
205+
#[test]
206+
fn unregister_drops_the_entry() {
207+
let server = server();
208+
let id = "22222222-2222-2222-2222-222222222222";
209+
let _cancel = server.register_session(id.to_string());
210+
server.unregister_session(id);
211+
assert!(!server.terminate(id), "unregistered session can't be terminated");
212+
}
132213
}

rs/moq-rtc/src/server/mux.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,19 +121,12 @@ impl Mux {
121121
self.socket.clone()
122122
}
123123

124-
/// ICE host candidates to advertise in the SDP answer.
124+
/// ICE host candidates to advertise in the SDP answer. The session tags each
125+
/// inbound datagram with the family-matching candidate; never empty (falls
126+
/// back to the bound address).
125127
pub(crate) fn candidates(&self) -> &[SocketAddr] {
126128
&self.candidates
127129
}
128-
129-
/// The local address to report to str0m as each datagram's destination. str0m
130-
/// requires it to match an advertised host candidate (it drops STUN whose
131-
/// destination is an "unknown interface"), and the shared socket binds a
132-
/// wildcard, so report the first advertised candidate rather than the bind. The
133-
/// candidate list is never empty (it falls back to the bound address).
134-
pub(crate) fn local(&self) -> SocketAddr {
135-
self.candidates[0]
136-
}
137130
}
138131

139132
/// Read the shared socket forever, routing each datagram to its session.

rs/moq-rtc/src/server/whep.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ pub use crate::server::Response;
1919

2020
/// Build the WHEP axum router.
2121
pub fn router(server: Server) -> Router {
22-
Router::new().route("/{*path}", post(handle)).with_state(server)
22+
Router::new()
23+
.route("/{*path}", post(handle).delete(crate::server::delete))
24+
.with_state(server)
2325
}
2426

2527
async fn handle(server: State<Server>, path: Path<String>, headers: HeaderMap, body: Bytes) -> HttpResponse {
@@ -109,14 +111,21 @@ pub async fn accept(
109111

110112
let answer = rtc.sdp_api().accept_offer(offer).map_err(Error::Rtc)?;
111113
let resource_id = sdp::new_resource_id();
112-
let session = session::Session::egress(rtc, mux.socket(), mux.local(), inbound, source);
114+
let session = session::Session::egress(rtc, mux.socket(), mux.candidates().to_vec(), inbound, source);
113115

116+
// Register before spawning so a DELETE that races startup still finds the
117+
// session; the task unregisters itself when it ends.
118+
let cancel = server.register_session(resource_id.clone());
119+
let task_server = server.clone();
120+
let task_resource = resource_id.clone();
114121
tokio::spawn(async move {
115122
// Hold the mux registration for the session's lifetime (unregisters on exit).
116123
let _registration = registration;
117-
if let Err(err) = session.run().await {
118-
tracing::warn!(%err, "whep session ended");
124+
tokio::select! {
125+
res = session.run() => session::log_session_end("whep server", res),
126+
_ = cancel => tracing::debug!("whep session terminated by DELETE"),
119127
}
128+
task_server.unregister_session(&task_resource);
120129
});
121130

122131
Ok(Response {

rs/moq-rtc/src/server/whip.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ pub use crate::server::Response;
2020

2121
/// Build the WHIP axum router.
2222
pub fn router(server: Server) -> Router {
23-
Router::new().route("/{*path}", post(handle)).with_state(server)
23+
Router::new()
24+
.route("/{*path}", post(handle).delete(crate::server::delete))
25+
.with_state(server)
2426
}
2527

2628
async fn handle(
@@ -109,16 +111,23 @@ pub async fn accept(
109111

110112
let answer = rtc.sdp_api().accept_offer(offer).map_err(Error::Rtc)?;
111113
let resource_id = sdp::new_resource_id();
112-
let session = session::Session::ingest(rtc, mux.socket(), mux.local(), inbound, sink);
114+
let session = session::Session::ingest(rtc, mux.socket(), mux.candidates().to_vec(), inbound, sink);
113115

116+
// Register before spawning so a DELETE that races the first packet still
117+
// finds the session; the task unregisters itself when it ends.
118+
let cancel = server.register_session(resource_id.clone());
119+
let task_server = server.clone();
120+
let task_resource = resource_id.clone();
114121
tokio::spawn(async move {
115122
// Hold the announcement guard + mux registration for the session's
116123
// lifetime; both release (unannounce / unregister) on exit.
117124
let _publish = publish;
118125
let _registration = registration;
119-
if let Err(err) = session.run().await {
120-
tracing::warn!(%err, "whip session ended");
126+
tokio::select! {
127+
res = session.run() => session::log_session_end("whip server", res),
128+
_ = cancel => tracing::debug!("whip session terminated by DELETE"),
121129
}
130+
task_server.unregister_session(&task_resource);
122131
});
123132

124133
Ok(Response {

0 commit comments

Comments
 (0)