Skip to content

Commit f37b862

Browse files
kixelatedclaude
andcommitted
moq-net: resolve track info via model TrackConsumer::info(), not subscribe
Addresses PR review: the publisher must not open a subscription just to learn a track's immutable properties. Add a first-class info path to the model and route every property lookup (publisher TRACK reply, subscriber SUBSCRIBE) and the cache through it. Model (broadcast.rs): - TrackConsumer::info() -> InfoPending: resolves a track's immutable Track (timescale/compression/cache) without subscribing. Warm (a live producer exists, or the value is cached) it resolves with no round trip; cold it queues a dynamic info request. - New dynamic info-request channel mirroring requested_track/TrackRequest: BroadcastDynamic::requested_info() -> InfoRequest::resolve(Track)/deny, plus a combined requested() -> DynamicRequest{Track,Info} so one loop serves both. - track_info cache keyed by name (a re-announce replaces the broadcast and State, invalidating it). TrackRequest::accept warms it, so a subscribe and a concurrent TRACK coalesce. Group-by-group fetches (which keep no track-level producer) reuse the one cached lookup. Publisher: recv_track now calls consume_track(name).info().await instead of subscribing-and-dropping. The spawn stays (a cold relay lookup is an upstream round trip; the accept loop must not block on it). Subscriber: the relay serves downstream info requests (run_info) by fetching TRACK_INFO upstream and caching it; its own lite-05 SUBSCRIBE path now resolves props through info() too, so a downstream's parallel TRACK + SUBSCRIBE collapse to a single upstream TRACK fetch. Tests cover warm/cold/coalesced/NotFound info() and accept-warms-cache. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0ba9972 commit f37b862

3 files changed

Lines changed: 437 additions & 25 deletions

File tree

rs/moq-net/src/lite/publisher.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,9 @@ impl<S: web_transport_trait::Session> Publisher<S> {
564564
let broadcast = self.origin.get_broadcast(&req.broadcast);
565565
let version = self.version;
566566

567+
// Spawn rather than serve inline: resolving the info can be a cold upstream
568+
// TRACK fetch (relay case), and the publisher's accept loop in `run` must
569+
// stay free to handle other control streams meanwhile.
567570
web_async::spawn(async move {
568571
if let Err(err) = Self::run_track_info(&mut stream, &track, broadcast, version).await {
569572
match &err {
@@ -587,15 +590,11 @@ impl<S: web_transport_trait::Session> Publisher<S> {
587590
) -> Result<(), Error> {
588591
let broadcast = consumer.ok_or(Error::NotFound)?;
589592

590-
// The immutable properties are delivered when the (possibly dynamic)
591-
// producer is accepted, so resolving them means subscribing. We read them
592-
// off the subscriber and drop it immediately; a SUBSCRIBE flighted in
593-
// parallel coalesces onto the same upstream producer, which linger keeps
594-
// alive across this brief gap.
595-
let track = broadcast
596-
.consume_track(track_name)
597-
.subscribe(crate::Subscription::default())
598-
.await?;
593+
// Resolve the immutable properties without subscribing. Warm (a producer
594+
// exists or the info is cached) this returns with no round trip; cold (a
595+
// relay with no prior subscription) it triggers a single upstream TRACK
596+
// fetch via the model's info-request channel.
597+
let track = broadcast.consume_track(track_name).info().await?;
599598

600599
// Mirror the negotiation in `run_subscribe` so the subscriber decodes
601600
// frames the same way it'll see them served.

rs/moq-net/src/lite/subscriber.rs

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,12 @@ impl<S: web_transport_trait::Session> Subscriber<S> {
463463
}
464464

465465
async fn run_broadcast(self, path: PathOwned, mut broadcast: BroadcastDynamic) {
466-
// Actually start serving subscriptions.
466+
// Actually start serving subscriptions and info lookups.
467467
loop {
468468
// Keep serving requests until there are no more consumers.
469469
// This way we'll clean up the task when the broadcast is no longer needed.
470470
let request = tokio::select! {
471-
request = broadcast.requested_track() => match request {
471+
request = broadcast.requested() => match request {
472472
Ok(request) => request,
473473
Err(err) => {
474474
tracing::debug!(%err, "broadcast closed");
@@ -478,12 +478,45 @@ impl<S: web_transport_trait::Session> Subscriber<S> {
478478
_ = self.session.closed() => break,
479479
};
480480

481-
let mut this = self.clone();
482481
let path = path.clone();
483-
let broadcast = broadcast.clone();
484-
web_async::spawn(async move {
485-
this.run_subscribe(path, broadcast, request).await;
486-
});
482+
match request {
483+
crate::DynamicRequest::Track(request) => {
484+
let mut this = self.clone();
485+
let broadcast = broadcast.clone();
486+
web_async::spawn(async move {
487+
this.run_subscribe(path, broadcast, request).await;
488+
});
489+
}
490+
crate::DynamicRequest::Info(request) => {
491+
let this = self.clone();
492+
web_async::spawn(async move {
493+
this.run_info(path, request).await;
494+
});
495+
}
496+
}
497+
}
498+
}
499+
500+
/// Serve a downstream info-only request by fetching the track's immutable
501+
/// properties from upstream over a TRACK stream, then caching them in the
502+
/// model. Subsequent info()/subscribe/fetch reuse the cache (no round trip).
503+
async fn run_info(&self, path: PathOwned, request: crate::InfoRequest) {
504+
let name = request.name().to_string();
505+
let upstream = path.as_path();
506+
match self.fetch_track_info(&upstream, &name).await {
507+
Ok(info) => {
508+
let mut track = Track::new(&name);
509+
track.timescale = info.timescale;
510+
track.cache = info.cache;
511+
// The model carries compression as a bool; the codec set is
512+
// {none, deflate}, so the flag round-trips losslessly.
513+
track.compress = info.compression != Compression::None;
514+
request.resolve(track);
515+
}
516+
Err(err) => {
517+
tracing::debug!(broadcast = %self.log_path(&path), track = %name, %err, "track info fetch failed");
518+
request.deny(err);
519+
}
487520
}
488521
}
489522

@@ -570,7 +603,6 @@ impl<S: web_transport_trait::Session> Subscriber<S> {
570603
let ordered = msg.ordered;
571604
let max_latency = msg.max_latency;
572605
let start_group = msg.start_group;
573-
let broadcast_path = msg.broadcast.clone();
574606

575607
// SubscribeUpdate only exists on Lite03+; older versions take the
576608
// immediate-FIN path with no linger.
@@ -609,24 +641,35 @@ impl<S: web_transport_trait::Session> Subscriber<S> {
609641
}
610642

611643
// Resolve the track's immutable properties (compression/timescale/cache).
612-
// lite-05 fetches TRACK_INFO on its own stream (already racing the SUBSCRIBE
613-
// above); older drafts take them from SUBSCRIBE_OK, which carries no such
614-
// properties, so they fall back to the untimed/uncompressed defaults.
644+
// lite-05 has no SUBSCRIBE_OK: resolve them via the model's info(), which is
645+
// warm (cached) when a downstream TRACK or a prior lookup already fetched
646+
// them, and otherwise coalesces with those into one upstream TRACK fetch.
647+
// The SUBSCRIBE above is already in flight, so groups still arrive in one
648+
// round trip and buffer on `resolved` until this lands. Older drafts take
649+
// the props from SUBSCRIBE_OK, which carries none, so they fall back to the
650+
// untimed/uncompressed defaults.
615651
let (compression, timescale, cache) = if matches!(self.version, Version::Lite05Wip) {
616-
tokio::select! {
652+
let props = tokio::select! {
617653
err = broadcast.closed() => {
618654
request.deny(err.clone());
619655
return SessionOutcome::BroadcastClosed(err);
620656
}
621-
res = self.fetch_track_info(&broadcast_path, name) => match res {
622-
Ok(info) => (info.compression, info.timescale, info.cache),
657+
res = broadcast.consume().consume_track(name).info() => match res {
658+
Ok(props) => props,
623659
Err(err) => {
624660
stream.writer.abort(&err);
625661
request.deny(err.clone());
626662
return SessionOutcome::Error(err);
627663
}
628664
}
629-
}
665+
};
666+
// Codec set is {none, deflate}, so the model's bool maps back losslessly.
667+
let compression = if props.compress {
668+
Compression::Deflate
669+
} else {
670+
Compression::None
671+
};
672+
(compression, props.timescale, props.cache)
630673
} else {
631674
let resp = tokio::select! {
632675
err = broadcast.closed() => {

0 commit comments

Comments
 (0)