From 46244906d40d384eeff411e64102116f4271939b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 22:10:08 +0000 Subject: [PATCH 01/10] moq-net: refactor Timestamp to runtime scale Replace the const-generic Timescale with a single Timestamp type that carries scale as a runtime field, so timescales can be negotiated per-track. Adds sentinel ZERO/MAX (scale 0) plus checked_add_delta and checked_delta_from helpers for the upcoming moq-lite zigzag delta wire format. Updates hang, moq-mux, libmoq, moq-ffi to use the new constructors. Catalog jitter switches from moq_net::Time to Option. This is the foundational refactor; wire-format changes and async subscribe come in follow-up commits. https://claude.ai/code/session_01Wy7egQjXvfPCKovs3mWANB --- rs/hang/src/catalog/audio/mod.rs | 2 +- rs/hang/src/catalog/video/mod.rs | 2 +- rs/hang/src/container/frame.rs | 9 +- rs/libmoq/src/consume.rs | 6 +- rs/moq-ffi/src/consumer.rs | 1 + rs/moq-mux/src/container/cmaf.rs | 4 +- rs/moq-mux/src/container/consumer.rs | 8 +- rs/moq-mux/src/container/mod.rs | 4 +- rs/moq-mux/src/container/producer.rs | 6 +- rs/moq-mux/src/export/fmp4.rs | 2 +- rs/moq-mux/src/import/fmp4.rs | 6 +- rs/moq-mux/src/import/jitter.rs | 12 +- rs/moq-net/src/model/time.rs | 846 +++++++++++++-------------- 13 files changed, 455 insertions(+), 453 deletions(-) diff --git a/rs/hang/src/catalog/audio/mod.rs b/rs/hang/src/catalog/audio/mod.rs index 5fd8cd6dd..b88a686b1 100644 --- a/rs/hang/src/catalog/audio/mod.rs +++ b/rs/hang/src/catalog/audio/mod.rs @@ -88,5 +88,5 @@ pub struct AudioConfig { /// NOTE: The audio "frame" duration depends on the codec, sample rate, etc. /// ex: AAC often uses 1024 samples per frame, so at 44100Hz, this would be 1024/44100 = 23ms #[serde(default)] - pub jitter: Option, + pub jitter: Option, } diff --git a/rs/hang/src/catalog/video/mod.rs b/rs/hang/src/catalog/video/mod.rs index f3f221066..97ba63920 100644 --- a/rs/hang/src/catalog/video/mod.rs +++ b/rs/hang/src/catalog/video/mod.rs @@ -139,5 +139,5 @@ pub struct VideoConfig { /// - If there can be up to 3 b-frames in a row, this would be 3 * 1000/fps. /// - If frames are buffered into 2s segments, this would be 2s. #[serde(default)] - pub jitter: Option, + pub jitter: Option, } diff --git a/rs/hang/src/container/frame.rs b/rs/hang/src/container/frame.rs index 6cbd2cd93..6e0f772f9 100644 --- a/rs/hang/src/container/frame.rs +++ b/rs/hang/src/container/frame.rs @@ -3,7 +3,10 @@ use derive_more::Debug; use crate::Error; -pub type Timestamp = moq_net::Timescale<1_000_000>; +pub use moq_net::Timestamp; + +/// Canonical timescale for hang frame timestamps: microseconds. +pub const TIMESCALE: u64 = 1_000_000; /// A media frame with a timestamp and codec-specific payload. /// @@ -32,7 +35,7 @@ impl Frame { /// VarInt timestamp prefix followed by the raw codec payload. pub fn encode(&self, group: &mut moq_net::GroupProducer) -> Result<(), Error> { let mut header = BytesMut::new(); - self.timestamp.encode(&mut header).map_err(moq_net::Error::from)?; + self.timestamp.encode_value(&mut header).map_err(moq_net::Error::from)?; let size = header.len() + self.payload.len(); @@ -46,7 +49,7 @@ impl Frame { /// Decode a frame from raw bytes (VarInt timestamp prefix + payload). pub fn decode(mut buf: impl Buf) -> Result { - let timestamp = Timestamp::decode(&mut buf)?; + let timestamp = Timestamp::decode_value(&mut buf, TIMESCALE)?; let payload = buf.copy_to_bytes(buf.remaining()); Ok(Self { timestamp, payload }) diff --git a/rs/libmoq/src/consume.rs b/rs/libmoq/src/consume.rs index ff27679d7..2e26049f4 100644 --- a/rs/libmoq/src/consume.rs +++ b/rs/libmoq/src/consume.rs @@ -327,7 +327,11 @@ impl Consume { pub fn frame(&self, frame: Id, dst: &mut moq_frame) -> Result<(), Error> { let f = self.frame.get(frame).ok_or(Error::FrameNotFound)?; - let timestamp_us = f.timestamp.as_micros().try_into().map_err(|_| moq_net::TimeOverflow)?; + let timestamp_us: u64 = f + .timestamp + .as_micros()? + .try_into() + .map_err(|_| moq_net::TimeOverflow)?; *dst = moq_frame { payload: f.payload.as_ptr(), diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index 82612ddfd..e5fa4f2f0 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -56,6 +56,7 @@ impl Media { let timestamp_us: u64 = frame .timestamp .as_micros() + .map_err(|_| MoqError::Codec("timestamp overflow".into()))? .try_into() .map_err(|_| MoqError::Codec("timestamp overflow".into()))?; diff --git a/rs/moq-mux/src/container/cmaf.rs b/rs/moq-mux/src/container/cmaf.rs index 64cba0a17..0506f70e1 100644 --- a/rs/moq-mux/src/container/cmaf.rs +++ b/rs/moq-mux/src/container/cmaf.rs @@ -144,7 +144,7 @@ pub(crate) fn decode(data: Bytes, timescale: u64) -> Result, Error> { let cts = entry.cts.unwrap_or_default() as i64; let pts = dts.checked_add_signed(cts).ok_or(Error::PtsOverflow)?; - let timestamp = Timestamp::from_scale(pts, timescale)?; + let timestamp = Timestamp::from_scale(pts, timescale, crate::container::TIMESCALE)?; let payload = Bytes::copy_from_slice(&mdat_data[offset..end]); let flags = entry.flags.unwrap_or(0); // depends_on_no_other (bits 24-25 == 0x2) means keyframe @@ -176,7 +176,7 @@ pub(crate) fn encode( return Ok(()); } - let dts = (frames[0].timestamp.as_micros() * timescale as u128 / 1_000_000) as u64; + let dts = (frames[0].timestamp.as_micros()? * timescale as u128 / 1_000_000) as u64; let sequence_number = group.frame_count() as u32; let entries: Vec<_> = frames diff --git a/rs/moq-mux/src/container/consumer.rs b/rs/moq-mux/src/container/consumer.rs index 7ea9cc279..dc3be4e34 100644 --- a/rs/moq-mux/src/container/consumer.rs +++ b/rs/moq-mux/src/container/consumer.rs @@ -125,7 +125,7 @@ impl Consumer { && current.sequence <= self.current { match current.poll_min_timestamp(waiter, &self.format) { - Poll::Ready(Ok(ts)) => Some::(ts.into()), + Poll::Ready(Ok(ts)) => ts.try_into().ok(), _ => None, } } else { @@ -153,7 +153,9 @@ impl Consumer { } if let Poll::Ready(Ok(ts)) = group.poll_max_timestamp(waiter, &self.format) { - max_timestamp = max_timestamp.max(ts.into()); + if let Ok(ts) = std::time::Duration::try_from(ts) { + max_timestamp = max_timestamp.max(ts); + } break; // We know older groups won't be newer than this. } } @@ -887,7 +889,7 @@ mod tests { let frames = read_all(&mut consumer).await.unwrap(); assert_eq!(frames.len(), 1); assert_eq!(frames[0].timestamp, ts(one_hour)); - assert_eq!(frames[0].timestamp.as_micros(), one_hour as u128); + assert_eq!(frames[0].timestamp.as_micros().unwrap(), one_hour as u128); } #[tokio::test] diff --git a/rs/moq-mux/src/container/mod.rs b/rs/moq-mux/src/container/mod.rs index 4c97f275d..e22f9056e 100644 --- a/rs/moq-mux/src/container/mod.rs +++ b/rs/moq-mux/src/container/mod.rs @@ -26,8 +26,10 @@ pub use consumer::Consumer; pub use hang::Hang; pub use producer::Producer; +pub use moq_net::Timestamp; + /// Microsecond presentation timestamp, the canonical timebase for media frames in moq-mux. -pub type Timestamp = moq_net::Timescale<1_000_000>; +pub const TIMESCALE: u64 = 1_000_000; /// A decoded media frame: timestamp, payload bytes, keyframe flag. /// diff --git a/rs/moq-mux/src/container/producer.rs b/rs/moq-mux/src/container/producer.rs index 2571c216e..140918755 100644 --- a/rs/moq-mux/src/container/producer.rs +++ b/rs/moq-mux/src/container/producer.rs @@ -85,10 +85,10 @@ impl Producer { // Check if buffered duration exceeds latency. if self.buffer.len() >= 2 { - let first_ts: std::time::Duration = self.buffer.first().unwrap().timestamp.into(); - let last_ts: std::time::Duration = self.buffer.last().unwrap().timestamp.into(); + let first_us = self.buffer.first().unwrap().timestamp.as_micros().unwrap_or(0); + let last_us = self.buffer.last().unwrap().timestamp.as_micros().unwrap_or(0); - if last_ts.saturating_sub(first_ts) >= self.latency { + if last_us.saturating_sub(first_us) >= self.latency.as_micros() { self.flush()?; } } diff --git a/rs/moq-mux/src/export/fmp4.rs b/rs/moq-mux/src/export/fmp4.rs index ffd63dff2..afe4bf44d 100644 --- a/rs/moq-mux/src/export/fmp4.rs +++ b/rs/moq-mux/src/export/fmp4.rs @@ -272,7 +272,7 @@ fn build_init(catalog: &Catalog) -> anyhow::Result { /// Encode one decoded frame as a CMAF moof+mdat fragment. fn encode_fragment(track: &mut Fmp4Track, frame: &Frame) -> anyhow::Result { - let dts = frame.timestamp.as_micros() as u64 * track.timescale / 1_000_000; + let dts = frame.timestamp.as_micros()? as u64 * track.timescale / 1_000_000; let size = frame.payload.len() as u32; let flags = if frame.keyframe { 0x0200_0000 } else { 0x0001_0000 }; diff --git a/rs/moq-mux/src/import/fmp4.rs b/rs/moq-mux/src/import/fmp4.rs index 9bb308699..9404d99eb 100644 --- a/rs/moq-mux/src/import/fmp4.rs +++ b/rs/moq-mux/src/import/fmp4.rs @@ -513,7 +513,7 @@ impl Fmp4 { .unwrap_or(tfhd.default_sample_size.unwrap_or(default_sample_size)) as usize; let pts = (dts as i64 + entry.cts.unwrap_or_default() as i64) as u64; - let timestamp = hang::container::Timestamp::from_scale(pts, timescale)?; + let timestamp = hang::container::Timestamp::from_scale(pts, timescale, hang::container::TIMESCALE)?; if offset + size > mdat.data.len() { anyhow::bail!("invalid data offset"); @@ -654,7 +654,7 @@ impl Fmp4 { .renditions .get_mut(&track.track.name) .context("missing video config")?; - config.jitter = Some(jitter.convert()?); + config.jitter = Some(jitter.convert(1_000)?); } TrackKind::Audio => { let config = catalog @@ -662,7 +662,7 @@ impl Fmp4 { .renditions .get_mut(&track.track.name) .context("missing audio config")?; - config.jitter = Some(jitter.convert()?); + config.jitter = Some(jitter.convert(1_000)?); } } } diff --git a/rs/moq-mux/src/import/jitter.rs b/rs/moq-mux/src/import/jitter.rs index 81ab64c41..2edf0beb7 100644 --- a/rs/moq-mux/src/import/jitter.rs +++ b/rs/moq-mux/src/import/jitter.rs @@ -18,11 +18,11 @@ impl MinFrameDuration { /// Record a new frame timestamp. /// - /// Returns the new minimum-frame-duration as a `moq_net::Time` if it - /// changed, so the caller can persist it on the catalog rendition. Returns - /// `None` when this is the first observation, the timestamps are - /// non-monotonic, or the new gap is no smaller than the recorded minimum. - pub fn observe(&mut self, ts: Timestamp) -> Option { + /// Returns the new minimum-frame-duration as a millisecond-scale [`Timestamp`] + /// if it changed, so the caller can persist it on the catalog rendition. Returns + /// `None` when this is the first observation, the timestamps are non-monotonic, + /// or the new gap is no smaller than the recorded minimum. + pub fn observe(&mut self, ts: Timestamp) -> Option { let last = self.last_timestamp.replace(ts)?; let duration = ts.checked_sub(last).ok()?; @@ -31,6 +31,6 @@ impl MinFrameDuration { } self.min_duration = Some(duration); - duration.convert().ok() + duration.convert(1_000).ok() } } diff --git a/rs/moq-net/src/model/time.rs b/rs/moq-net/src/model/time.rs index c24629cff..30a2f9c35 100644 --- a/rs/moq-net/src/model/time.rs +++ b/rs/moq-net/src/model/time.rs @@ -6,60 +6,95 @@ use crate::coding::{Decode, DecodeError, Encode, EncodeError, VarInt}; use std::sync::LazyLock; use std::time::{SystemTime, UNIX_EPOCH}; -/// A timestamp representing the presentation time in milliseconds. -/// -/// The underlying implementation supports any scale, but everything uses milliseconds by default. -pub type Time = Timescale<1_000>; - -/// Returned when a [`Timescale`] operation would exceed the QUIC VarInt range -/// (`2^62 - 1`) or overflow during scale conversion or arithmetic. +/// Returned when a [`Timestamp`] operation would exceed the QUIC VarInt range +/// (`2^62 - 1`), overflow during scale conversion or arithmetic, hit a divide +/// by zero from an unspecified ([`Timestamp::is_unspecified`]) scale, or +/// attempt arithmetic between timestamps with mismatched scales. #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] #[error("time overflow")] pub struct TimeOverflow; -/// A timestamp representing the presentation time in a given scale. ex. 1000 for milliseconds. +/// A timestamp in a track's timescale (units per second). /// /// All timestamps within a track are relative, so zero for one track is not zero for another. -/// Values are constrained to fit within a QUIC VarInt (2^62) so they can be encoded and decoded easily. +/// The underlying value is constrained to fit within a QUIC VarInt (`2^62 - 1`) so it can be +/// encoded and decoded easily; the scale is carried out-of-band (via [`crate::Track::timescale`]) +/// and not serialized per-timestamp. /// -/// This is [std::time::Instant] and [std::time::Duration] merged into one type for simplicity. -#[derive(Clone, Default, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +/// `scale == 0` denotes an unspecified timescale, produced by [`Timestamp::ZERO`] and by +/// peers that don't negotiate a timescale (older moq-lite versions, older moq-transport +/// drafts without track properties). Unit conversions and arithmetic against an unspecified +/// scale return [`TimeOverflow`] to avoid divide by zero. +#[derive(Clone, Default, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct Timescale(VarInt); - -impl Timescale { - /// The maximum representable instant. - pub const MAX: Self = Self(VarInt::MAX); - - /// The minimum representable instant. - pub const ZERO: Self = Self(VarInt::ZERO); +pub struct Timestamp { + value: VarInt, + scale: u64, +} - /// Construct a timestamp directly from a value in this scale's units. Infallible - /// because any `u32` fits within the 62-bit varint range. - pub const fn new(value: u32) -> Self { - Self(VarInt::from_u32(value)) +impl Timestamp { + /// A zero timestamp with an unspecified scale. + /// + /// Useful as a sentinel min: comparisons against unspecified-scale timestamps + /// compare raw values, so `Timestamp::ZERO < t` is true for any `t` whose + /// `value > 0`. See [`Self::partial_cmp`]. + pub const ZERO: Self = Self { + value: VarInt::ZERO, + scale: 0, + }; + + /// The maximum representable timestamp value, with an unspecified scale. + /// + /// Useful as a sentinel max: `t < Timestamp::MAX` is true for any `t` whose + /// `value < VarInt::MAX`. See [`Self::partial_cmp`]. + pub const MAX: Self = Self { + value: VarInt::MAX, + scale: 0, + }; + + /// Construct a timestamp directly from a raw value at the given scale. + pub const fn new(value: u32, scale: u64) -> Self { + Self { + value: VarInt::from_u32(value), + scale, + } } - /// Construct a timestamp directly from a value in this scale's units. Returns - /// [`TimeOverflow`] if `value` exceeds the 62-bit varint range. - pub const fn new_u64(value: u64) -> Result { + /// Construct a timestamp from a raw value at the given scale. Returns [`TimeOverflow`] + /// if `value` exceeds the 62-bit varint range. + pub const fn new_u64(value: u64, scale: u64) -> Result { match VarInt::from_u64(value) { - Some(varint) => Ok(Self(varint)), + Some(value) => Ok(Self { value, scale }), None => Err(TimeOverflow), } } - /// Convert a number of seconds to a timestamp, returning an error if the timestamp would overflow. - pub const fn from_secs(seconds: u64) -> Result { - // Not using from_scale because it'll be slightly faster - match seconds.checked_mul(SCALE) { - Some(value) => Self::new_u64(value), + /// Convert `value` measured at the given `scale` (units per second) to a timestamp. + /// + /// The stored scale is `target_scale`. Returns [`TimeOverflow`] on overflow or if + /// `source_scale` is zero. + pub const fn from_scale(value: u64, source_scale: u64, target_scale: u64) -> Result { + if source_scale == 0 { + return Err(TimeOverflow); + } + match (value as u128).checked_mul(target_scale as u128) { + Some(scaled) => match VarInt::from_u128(scaled / source_scale as u128) { + Some(value) => Ok(Self { + value, + scale: target_scale, + }), + None => Err(TimeOverflow), + }, None => Err(TimeOverflow), } } - /// Like [`Self::from_secs`] but panics on overflow. Intended for `const` - /// initializers where overflow indicates a bug, not a runtime condition. + /// Convert a number of seconds to a timestamp with `scale == 1`. + pub const fn from_secs(seconds: u64) -> Result { + Self::new_u64(seconds, 1) + } + + /// Like [`Self::from_secs`] but panics on overflow. pub const fn from_secs_unchecked(seconds: u64) -> Self { match Self::from_secs(seconds) { Ok(time) => time, @@ -67,187 +102,242 @@ impl Timescale { } } - /// Convert a number of milliseconds to a timestamp, returning an error if the timestamp would overflow. + /// Convert a number of milliseconds to a timestamp with `scale == 1_000`. pub const fn from_millis(millis: u64) -> Result { - Self::from_scale(millis, 1000) + Self::new_u64(millis, 1_000) } /// Like [`Self::from_millis`] but panics on overflow. pub const fn from_millis_unchecked(millis: u64) -> Self { - Self::from_scale_unchecked(millis, 1000) + match Self::from_millis(millis) { + Ok(time) => time, + Err(_) => panic!("time overflow"), + } } - /// Convert a number of microseconds to a timestamp, returning an error on overflow. + /// Convert a number of microseconds to a timestamp with `scale == 1_000_000`. pub const fn from_micros(micros: u64) -> Result { - Self::from_scale(micros, 1_000_000) + Self::new_u64(micros, 1_000_000) } /// Like [`Self::from_micros`] but panics on overflow. pub const fn from_micros_unchecked(micros: u64) -> Self { - Self::from_scale_unchecked(micros, 1_000_000) + match Self::from_micros(micros) { + Ok(time) => time, + Err(_) => panic!("time overflow"), + } } - /// Convert a number of nanoseconds to a timestamp, returning an error on overflow. + /// Convert a number of nanoseconds to a timestamp with `scale == 1_000_000_000`. pub const fn from_nanos(nanos: u64) -> Result { - Self::from_scale(nanos, 1_000_000_000) + Self::new_u64(nanos, 1_000_000_000) } /// Like [`Self::from_nanos`] but panics on overflow. pub const fn from_nanos_unchecked(nanos: u64) -> Self { - Self::from_scale_unchecked(nanos, 1_000_000_000) + match Self::from_nanos(nanos) { + Ok(time) => time, + Err(_) => panic!("time overflow"), + } } - /// Construct from `value` measured at the given `scale` (units per second), rescaling - /// to `SCALE`. Returns [`TimeOverflow`] if the rescaled value exceeds 2^62. - pub const fn from_scale(value: u64, scale: u64) -> Result { - match VarInt::from_u128(value as u128 * SCALE as u128 / scale as u128) { - Some(varint) => Ok(Self(varint)), - None => Err(TimeOverflow), - } + /// The raw value in the timestamp's own scale. + pub const fn value(self) -> u64 { + self.value.into_inner() } - /// Like [`Self::from_scale`] but accepts a `u128` source value. - pub const fn from_scale_u128(value: u128, scale: u64) -> Result { - match value.checked_mul(SCALE as u128) { - Some(value) => match VarInt::from_u128(value / scale as u128) { - Some(varint) => Ok(Self(varint)), - None => Err(TimeOverflow), - }, - None => Err(TimeOverflow), + /// The scale (units per second) attached to this timestamp. + pub const fn scale(self) -> u64 { + self.scale + } + + /// Whether the scale is unset (`scale == 0`). Unit conversions and cross-scale + /// arithmetic against this timestamp return [`TimeOverflow`]. + pub const fn is_unspecified(self) -> bool { + self.scale == 0 + } + + /// Whether the raw value is zero. Does not consider scale. + pub const fn is_zero(self) -> bool { + self.value.into_inner() == 0 + } + + /// Re-express this timestamp at a new scale. Returns [`TimeOverflow`] if the new + /// value would exceed `2^62 - 1`, the source scale is unspecified, or + /// `new_scale == 0`. + pub const fn convert(self, new_scale: u64) -> Result { + if self.scale == 0 || new_scale == 0 { + return Err(TimeOverflow); } + if self.scale == new_scale { + return Ok(self); + } + Self::from_scale(self.value.into_inner(), self.scale, new_scale) } - /// Like [`Self::from_scale`] but panics on overflow. - pub const fn from_scale_unchecked(value: u64, scale: u64) -> Self { - match Self::from_scale(value, scale) { - Ok(time) => time, - Err(_) => panic!("time overflow"), + /// The value re-expressed at `target_scale` as a `u128`. Returns [`TimeOverflow`] + /// if the source scale is unspecified or `target_scale == 0`. + pub const fn as_scale(self, target_scale: u64) -> Result { + if self.scale == 0 || target_scale == 0 { + return Err(TimeOverflow); } + Ok(self.value.into_inner() as u128 * target_scale as u128 / self.scale as u128) } - /// Get the timestamp as seconds. - pub const fn as_secs(self) -> u64 { - self.0.into_inner() / SCALE + /// The value re-expressed in seconds. Returns [`TimeOverflow`] if the scale is + /// unspecified. + pub const fn as_secs(self) -> Result { + if self.scale == 0 { + return Err(TimeOverflow); + } + Ok(self.value.into_inner() / self.scale) } - /// Get the timestamp as milliseconds. - // - // This returns a u128 to avoid a possible overflow when SCALE < 250 - pub const fn as_millis(self) -> u128 { - self.as_scale(1000) + /// The value re-expressed in milliseconds. Returns [`TimeOverflow`] if the scale + /// is unspecified. + pub const fn as_millis(self) -> Result { + self.as_scale(1_000) } - /// Get the timestamp as microseconds. - pub const fn as_micros(self) -> u128 { + /// The value re-expressed in microseconds. Returns [`TimeOverflow`] if the scale + /// is unspecified. + pub const fn as_micros(self) -> Result { self.as_scale(1_000_000) } - /// Get the timestamp as nanoseconds. - pub const fn as_nanos(self) -> u128 { + /// The value re-expressed in nanoseconds. Returns [`TimeOverflow`] if the scale + /// is unspecified. + pub const fn as_nanos(self) -> Result { self.as_scale(1_000_000_000) } - /// Convert this timestamp to the given `scale` (units per second). - pub const fn as_scale(self, scale: u64) -> u128 { - self.0.into_inner() as u128 * scale as u128 / SCALE as u128 - } - - /// Get the maximum of two timestamps. + /// Return the larger of two timestamps. + /// + /// Panics if the scales differ. Use [`Self::convert`] first if you need to compare + /// across scales. pub const fn max(self, other: Self) -> Self { - if self.0.into_inner() > other.0.into_inner() { + assert!(self.scale == other.scale, "mismatched timestamp scales"); + if self.value.into_inner() > other.value.into_inner() { self } else { other } } - /// Add two timestamps, returning [`TimeOverflow`] if the sum exceeds 2^62. + /// Add two timestamps. Returns [`TimeOverflow`] if the sum exceeds `2^62 - 1` or + /// if the scales differ. pub const fn checked_add(self, rhs: Self) -> Result { - let lhs = self.0.into_inner(); - let rhs = rhs.0.into_inner(); - match lhs.checked_add(rhs) { - Some(result) => Self::new_u64(result), + if self.scale != rhs.scale { + return Err(TimeOverflow); + } + match self.value.into_inner().checked_add(rhs.value.into_inner()) { + Some(result) => Self::new_u64(result, self.scale), None => Err(TimeOverflow), } } - /// Subtract `rhs` from `self`, returning [`TimeOverflow`] if `rhs > self`. + /// Subtract `rhs` from `self`. Returns [`TimeOverflow`] if `rhs > self` or if the + /// scales differ. pub const fn checked_sub(self, rhs: Self) -> Result { - let lhs = self.0.into_inner(); - let rhs = rhs.0.into_inner(); - match lhs.checked_sub(rhs) { - Some(result) => Self::new_u64(result), + if self.scale != rhs.scale { + return Err(TimeOverflow); + } + match self.value.into_inner().checked_sub(rhs.value.into_inner()) { + Some(result) => Self::new_u64(result, self.scale), None => Err(TimeOverflow), } } - /// Whether this timestamp is [`Self::ZERO`]. - pub const fn is_zero(self) -> bool { - self.0.into_inner() == 0 + /// Apply a signed delta in this timestamp's scale, returning the new timestamp. + /// + /// Used by the moq-lite per-frame delta decoder: timestamps are encoded as zigzag + /// signed deltas (negative for B-frames). Returns [`TimeOverflow`] if the result + /// would underflow zero or overflow `2^62 - 1`. + pub const fn checked_add_delta(self, delta: i64) -> Result { + let current = self.value.into_inner() as i128; + let next = current + delta as i128; + if next < 0 { + return Err(TimeOverflow); + } + match VarInt::from_u128(next as u128) { + Some(value) => Ok(Self { value, scale: self.scale }), + None => Err(TimeOverflow), + } } - /// Current time as a timestamp, derived from [`tokio::time::Instant::now`] so - /// it honors `tokio::time::pause` in tests. - pub fn now() -> Self { - // We use tokio so it can be stubbed for testing. - tokio::time::Instant::now().into() + /// The signed delta from `prev` to `self` in their shared scale. Returns + /// [`TimeOverflow`] on scale mismatch or if the delta is outside `i64::MIN..=i64::MAX`. + pub const fn checked_delta_from(self, prev: Self) -> Result { + if self.scale != prev.scale { + return Err(TimeOverflow); + } + let a = self.value.into_inner() as i128; + let b = prev.value.into_inner() as i128; + let delta = a - b; + if delta < i64::MIN as i128 || delta > i64::MAX as i128 { + return Err(TimeOverflow); + } + Ok(delta as i64) } - /// Convert this timestamp to a different scale. - /// - /// This allows converting between different TimeScale types, for example from milliseconds to microseconds. - /// Note that converting to a coarser scale may lose precision due to integer division. - pub const fn convert(self) -> Result, TimeOverflow> { - let value = self.0.into_inner(); - // Convert from SCALE to NEW_SCALE: value * NEW_SCALE / SCALE - match (value as u128).checked_mul(NEW_SCALE as u128) { - Some(v) => match v.checked_div(SCALE as u128) { - Some(v) => match VarInt::from_u128(v) { - Some(varint) => Ok(Timescale(varint)), - None => Err(TimeOverflow), - }, - None => Err(TimeOverflow), - }, - None => Err(TimeOverflow), - } + /// Current time, expressed in microseconds (`scale == 1_000_000`). Uses + /// [`tokio::time::Instant::now`] so it honors `tokio::time::pause` in tests. + pub fn now() -> Self { + tokio::time::Instant::now().into() } - /// Encode this timestamp as a QUIC varint. Version-independent. - pub fn encode(&self, w: &mut W) -> Result<(), EncodeError> { - // Version-independent: uses QUIC varint encoding. - self.0.encode(w, crate::lite::Version::Lite01)?; + /// Encode the raw value as a QUIC varint. Scale is carried out-of-band and is + /// **not** included on the wire. + pub fn encode_value(&self, w: &mut W) -> Result<(), EncodeError> { + self.value.encode(w, crate::lite::Version::Lite01)?; Ok(()) } - /// Decode a timestamp from a QUIC varint. Version-independent. - pub fn decode(r: &mut R) -> Result { - // Version-independent: uses QUIC varint encoding. - let v = VarInt::decode(r, crate::lite::Version::Lite01)?; - Ok(Self(v)) + /// Decode a raw value as a QUIC varint, attaching the given scale. + pub fn decode_value(r: &mut R, scale: u64) -> Result { + let value = VarInt::decode(r, crate::lite::Version::Lite01)?; + Ok(Self { value, scale }) } } -impl TryFrom for Timescale { +impl TryFrom for Timestamp { type Error = TimeOverflow; + /// Convert a [`std::time::Duration`] into a nanosecond-scale timestamp. fn try_from(duration: std::time::Duration) -> Result { - Self::from_scale_u128(duration.as_nanos(), 1_000_000_000) + match VarInt::from_u128(duration.as_nanos()) { + Some(value) => Ok(Self { + value, + scale: 1_000_000_000, + }), + None => Err(TimeOverflow), + } } } -impl From> for std::time::Duration { - fn from(time: Timescale) -> Self { - std::time::Duration::new(time.as_secs(), (time.as_nanos() % 1_000_000_000) as u32) +impl TryFrom for std::time::Duration { + type Error = TimeOverflow; + + fn try_from(time: Timestamp) -> Result { + let secs = time.as_secs()?; + let nanos = time.as_nanos()?; + Ok(std::time::Duration::new(secs, (nanos % 1_000_000_000) as u32)) } } -impl std::fmt::Debug for Timescale { +impl std::fmt::Debug for Timestamp { #[allow(clippy::manual_is_multiple_of)] // is_multiple_of is unstable in Rust 1.85 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let nanos = self.as_nanos(); + if self.scale == 0 { + return write!(f, "{}/?", self.value.into_inner()); + } + + let nanos = match self.as_nanos() { + Ok(n) => n, + Err(_) => return write!(f, "{}/{}", self.value.into_inner(), self.scale), + }; - // Choose the largest unit where we don't need decimal places - // Check from largest to smallest unit + // Choose the largest unit where we don't need decimal places. if nanos % 1_000_000_000 == 0 { write!(f, "{}s", nanos / 1_000_000_000) } else if nanos % 1_000_000 == 0 { @@ -260,29 +350,51 @@ impl std::fmt::Debug for Timescale { } } -impl std::ops::Add for Timescale { +impl PartialOrd for Timestamp { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Timestamp { + /// Compare by raw value. Debug-asserts that scales are compatible (same, or + /// one is unspecified). In release, cross-scale comparisons return a result + /// based on the raw value, which is meaningful only when one side is a + /// scale-`0` sentinel ([`Self::ZERO`] or [`Self::MAX`]). + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + debug_assert!( + self.scale == other.scale || self.scale == 0 || other.scale == 0, + "comparing timestamps with mismatched scales: {} vs {}", + self.scale, + other.scale, + ); + self.value.cmp(&other.value) + } +} + +impl std::ops::Add for Timestamp { type Output = Self; fn add(self, rhs: Self) -> Self { - self.checked_add(rhs).expect("time overflow") + self.checked_add(rhs).expect("time overflow or scale mismatch") } } -impl std::ops::AddAssign for Timescale { +impl std::ops::AddAssign for Timestamp { fn add_assign(&mut self, rhs: Self) { *self = *self + rhs; } } -impl std::ops::Sub for Timescale { +impl std::ops::Sub for Timestamp { type Output = Self; fn sub(self, rhs: Self) -> Self { - self.checked_sub(rhs).expect("time overflow") + self.checked_sub(rhs).expect("time overflow or scale mismatch") } } -impl std::ops::SubAssign for Timescale { +impl std::ops::SubAssign for Timestamp { fn sub_assign(&mut self, rhs: Self) { *self = *self - rhs; } @@ -297,43 +409,47 @@ static TIME_ANCHOR: LazyLock<(std::time::Instant, SystemTime)> = LazyLock::new(| (std::time::Instant::now(), SystemTime::now() - jitter) }); -// Convert an Instant to a Unix timestamp -impl From for Timescale { +impl From for Timestamp { + /// Convert an [`std::time::Instant`] into a microsecond-scale timestamp anchored to a + /// jittered wall-clock reference (see [`TIME_ANCHOR`]). fn from(instant: std::time::Instant) -> Self { let (anchor_instant, anchor_system) = *TIME_ANCHOR; - // Conver the instant to a SystemTime. let system = match instant.checked_duration_since(anchor_instant) { Some(forward) => anchor_system + forward, None => anchor_system - anchor_instant.duration_since(instant), }; - // Convert the SystemTime to a Unix timestamp in nanoseconds. - // We'll then convert that to the desired scale. - system + let duration = system .duration_since(UNIX_EPOCH) - .expect("dude your clock is earlier than 1970") - .try_into() - .expect("dude your clock is later than 2116") + .expect("dude your clock is earlier than 1970"); + + Self::from_micros(duration.as_micros() as u64).expect("dude your clock is later than 2116") } } -impl From for Timescale { +impl From for Timestamp { fn from(instant: tokio::time::Instant) -> Self { instant.into_std().into() } } -impl Decode for Timescale { +/// Decode a timestamp's raw value as a varint, attaching an unspecified scale. +/// +/// Callers that need a meaningful scale (the track timescale) should use +/// [`Timestamp::decode_value`] directly. +impl Decode for Timestamp { fn decode(r: &mut R, version: crate::Version) -> Result { - let v = VarInt::decode(r, version)?; - Ok(Self(v)) + let value = VarInt::decode(r, version)?; + Ok(Self { value, scale: 0 }) } } -impl Encode for Timescale { +/// Encode a timestamp's raw value as a varint. The scale is **not** serialized; it +/// is conveyed out-of-band via [`crate::Track::timescale`]. +impl Encode for Timestamp { fn encode(&self, w: &mut W, version: crate::Version) -> Result<(), EncodeError> { - self.0.encode(w, version)?; + self.value.encode(w, version)?; Ok(()) } } @@ -344,377 +460,251 @@ mod tests { #[test] fn test_from_secs() { - let time = Time::from_secs(5).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_micros(), 5_000_000); - assert_eq!(time.as_nanos(), 5_000_000_000); + let time = Timestamp::from_secs(5).unwrap(); + assert_eq!(time.scale(), 1); + assert_eq!(time.as_secs().unwrap(), 5); + assert_eq!(time.as_millis().unwrap(), 5000); + assert_eq!(time.as_micros().unwrap(), 5_000_000); + assert_eq!(time.as_nanos().unwrap(), 5_000_000_000); } #[test] fn test_from_millis() { - let time = Time::from_millis(5000).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); + let time = Timestamp::from_millis(5000).unwrap(); + assert_eq!(time.scale(), 1_000); + assert_eq!(time.as_secs().unwrap(), 5); + assert_eq!(time.as_millis().unwrap(), 5000); } #[test] fn test_from_micros() { - let time = Time::from_micros(5_000_000).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_micros(), 5_000_000); + let time = Timestamp::from_micros(5_000_000).unwrap(); + assert_eq!(time.scale(), 1_000_000); + assert_eq!(time.as_secs().unwrap(), 5); + assert_eq!(time.as_micros().unwrap(), 5_000_000); } #[test] fn test_from_nanos() { - let time = Time::from_nanos(5_000_000_000).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_micros(), 5_000_000); - assert_eq!(time.as_nanos(), 5_000_000_000); + let time = Timestamp::from_nanos(5_000_000_000).unwrap(); + assert_eq!(time.scale(), 1_000_000_000); + assert_eq!(time.as_secs().unwrap(), 5); + assert_eq!(time.as_nanos().unwrap(), 5_000_000_000); } #[test] - fn test_zero() { - let time = Time::ZERO; - assert_eq!(time.as_secs(), 0); - assert_eq!(time.as_millis(), 0); - assert_eq!(time.as_micros(), 0); - assert_eq!(time.as_nanos(), 0); + fn test_zero_unspecified() { + let time = Timestamp::ZERO; + assert!(time.is_unspecified()); assert!(time.is_zero()); + assert!(time.as_secs().is_err()); + assert!(time.as_millis().is_err()); + assert!(time.as_micros().is_err()); + assert!(time.as_nanos().is_err()); } #[test] - fn test_roundtrip_millis() { - let values = [0, 1, 100, 1000, 999999, 1_000_000_000]; - for &val in &values { - let time = Time::from_millis(val).unwrap(); - assert_eq!(time.as_millis(), val as u128); - } + fn test_zero_at_scale() { + let time = Timestamp::from_millis(0).unwrap(); + assert!(!time.is_unspecified()); + assert!(time.is_zero()); + assert_eq!(time.as_millis().unwrap(), 0); } #[test] - fn test_roundtrip_micros() { - // Note: values < 1000 will lose precision when converting to milliseconds (SCALE=1000) - let values = [0, 1000, 1_000_000, 1_000_000_000]; - for &val in &values { - let time = Time::from_micros(val).unwrap(); - assert_eq!(time.as_micros(), val as u128); - } + fn test_convert_to_finer() { + let time_ms = Timestamp::from_millis(5000).unwrap(); + let time_us = time_ms.convert(1_000_000).unwrap(); + assert_eq!(time_us.scale(), 1_000_000); + assert_eq!(time_us.as_micros().unwrap(), 5_000_000); } #[test] - fn test_different_scale_seconds() { - type TimeInSeconds = Timescale<1>; - let time = TimeInSeconds::from_secs(5).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); + fn test_convert_to_coarser() { + let time_ms = Timestamp::from_millis(5000).unwrap(); + let time_s = time_ms.convert(1).unwrap(); + assert_eq!(time_s.scale(), 1); + assert_eq!(time_s.as_secs().unwrap(), 5); } #[test] - fn test_different_scale_microseconds() { - type TimeInMicros = Timescale<1_000_000>; - let time = TimeInMicros::from_micros(5_000_000).unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_micros(), 5_000_000); + fn test_convert_precision_loss() { + // 1234 ms = 1.234 s, rounds down to 1 s + let time_ms = Timestamp::from_millis(1234).unwrap(); + let time_s = time_ms.convert(1).unwrap(); + assert_eq!(time_s.as_secs().unwrap(), 1); } #[test] - fn test_scale_conversion() { - // Converting 5000 milliseconds at scale 1000 to scale 1000 (should be identity) - let time = Time::from_scale(5000, 1000).unwrap(); - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_secs(), 5); - - // Converting 5 seconds at scale 1 to scale 1000 - let time = Time::from_scale(5, 1).unwrap(); - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_secs(), 5); + fn test_convert_roundtrip() { + let original = Timestamp::from_millis(5000).unwrap(); + let as_micros = original.convert(1_000_000).unwrap(); + let back = as_micros.convert(1_000).unwrap(); + assert_eq!(original.value(), back.value()); + assert_eq!(original.scale(), back.scale()); } #[test] - fn test_add() { - let a = Time::from_secs(3).unwrap(); - let b = Time::from_secs(2).unwrap(); - let c = a + b; - assert_eq!(c.as_secs(), 5); - assert_eq!(c.as_millis(), 5000); + fn test_convert_same_scale() { + let time = Timestamp::from_millis(5000).unwrap(); + let converted = time.convert(1_000).unwrap(); + assert_eq!(time, converted); } #[test] - fn test_sub() { - let a = Time::from_secs(5).unwrap(); - let b = Time::from_secs(2).unwrap(); - let c = a - b; - assert_eq!(c.as_secs(), 3); - assert_eq!(c.as_millis(), 3000); + fn test_convert_unspecified_rejected() { + let zero = Timestamp::ZERO; + assert!(zero.convert(1_000).is_err()); + + let time = Timestamp::from_millis(5).unwrap(); + assert!(time.convert(0).is_err()); } #[test] - fn test_checked_add() { - let a = Time::from_millis(1000).unwrap(); - let b = Time::from_millis(2000).unwrap(); + fn test_add_same_scale() { + let a = Timestamp::from_millis(1000).unwrap(); + let b = Timestamp::from_millis(2000).unwrap(); let c = a.checked_add(b).unwrap(); - assert_eq!(c.as_millis(), 3000); + assert_eq!(c.as_millis().unwrap(), 3000); + assert_eq!(c.scale(), 1_000); } #[test] - fn test_checked_sub() { - let a = Time::from_millis(5000).unwrap(); - let b = Time::from_millis(2000).unwrap(); - let c = a.checked_sub(b).unwrap(); - assert_eq!(c.as_millis(), 3000); + fn test_add_mismatched_scale() { + let a = Timestamp::from_millis(1000).unwrap(); + let b = Timestamp::from_micros(1000).unwrap(); + assert!(a.checked_add(b).is_err()); } #[test] - fn test_checked_sub_underflow() { - let a = Time::from_millis(1000).unwrap(); - let b = Time::from_millis(2000).unwrap(); + fn test_sub_underflow() { + let a = Timestamp::from_millis(1000).unwrap(); + let b = Timestamp::from_millis(2000).unwrap(); assert!(a.checked_sub(b).is_err()); } #[test] - fn test_max() { - let a = Time::from_secs(5).unwrap(); - let b = Time::from_secs(10).unwrap(); + fn test_max_same_scale() { + let a = Timestamp::from_secs(5).unwrap(); + let b = Timestamp::from_secs(10).unwrap(); assert_eq!(a.max(b), b); assert_eq!(b.max(a), b); } #[test] - fn test_duration_conversion() { - let duration = std::time::Duration::from_secs(5); - let time: Time = duration.try_into().unwrap(); - assert_eq!(time.as_secs(), 5); - assert_eq!(time.as_millis(), 5000); - - let duration_back: std::time::Duration = time.into(); - assert_eq!(duration_back.as_secs(), 5); - } - - #[test] - fn test_duration_with_nanos() { - let duration = std::time::Duration::new(5, 500_000_000); // 5.5 seconds - let time: Time = duration.try_into().unwrap(); - assert_eq!(time.as_millis(), 5500); - - let duration_back: std::time::Duration = time.into(); - assert_eq!(duration_back.as_millis(), 5500); - } - - #[test] - fn test_fractional_conversion() { - // Test that 1500 millis = 1.5 seconds - let time = Time::from_millis(1500).unwrap(); - assert_eq!(time.as_secs(), 1); // Integer division - assert_eq!(time.as_millis(), 1500); - assert_eq!(time.as_micros(), 1_500_000); + #[should_panic(expected = "mismatched timestamp scales")] + fn test_max_mismatched_scale_panics() { + let a = Timestamp::from_millis(1).unwrap(); + let b = Timestamp::from_secs(1).unwrap(); + let _ = a.max(b); } #[test] - fn test_precision_loss() { - // When converting from a finer scale to coarser, we lose precision - // 1234 micros = 1.234 millis, which rounds down to 1 millisecond internally - // When converting back, we get 1000 micros, not the original 1234 - let time = Time::from_micros(1234).unwrap(); - assert_eq!(time.as_millis(), 1); // 1234 micros = 1.234 millis, rounds to 1 - assert_eq!(time.as_micros(), 1000); // Precision lost: 1 milli = 1000 micros + fn test_ordering_same_scale() { + let a = Timestamp::from_secs(1).unwrap(); + let b = Timestamp::from_secs(2).unwrap(); + assert!(a < b); + assert!(b > a); + assert_eq!(a, a); } #[test] - fn test_scale_boundaries() { - // Test values near scale boundaries - let time = Time::from_millis(999).unwrap(); - assert_eq!(time.as_secs(), 0); - assert_eq!(time.as_millis(), 999); - - let time = Time::from_millis(1000).unwrap(); - assert_eq!(time.as_secs(), 1); - assert_eq!(time.as_millis(), 1000); - - let time = Time::from_millis(1001).unwrap(); - assert_eq!(time.as_secs(), 1); - assert_eq!(time.as_millis(), 1001); + fn test_ordering_against_sentinels() { + // ZERO and MAX act as universal sentinels because their scale is 0. + let t = Timestamp::from_millis(100).unwrap(); + assert!(Timestamp::ZERO < t); + assert!(t < Timestamp::MAX); + assert!(Timestamp::ZERO < Timestamp::MAX); } #[test] - fn test_large_values() { - // Test with large but valid values - let large_secs = 1_000_000_000u64; // ~31 years - let time = Time::from_secs(large_secs).unwrap(); - assert_eq!(time.as_secs(), large_secs); + fn test_delta_positive() { + let prev = Timestamp::from_millis(100).unwrap(); + let curr = Timestamp::from_millis(150).unwrap(); + assert_eq!(curr.checked_delta_from(prev).unwrap(), 50); } #[test] - fn test_new() { - let time = Time::new(5000); // 5000 in the current scale (millis) - assert_eq!(time.as_millis(), 5000); - assert_eq!(time.as_secs(), 5); + fn test_delta_negative() { + let prev = Timestamp::from_millis(150).unwrap(); + let curr = Timestamp::from_millis(100).unwrap(); + assert_eq!(curr.checked_delta_from(prev).unwrap(), -50); } #[test] - fn test_new_u64() { - let time = Time::new_u64(5000).unwrap(); - assert_eq!(time.as_millis(), 5000); + fn test_delta_mismatched_scale() { + let prev = Timestamp::from_millis(100).unwrap(); + let curr = Timestamp::from_micros(150).unwrap(); + assert!(curr.checked_delta_from(prev).is_err()); } #[test] - fn test_ordering() { - let a = Time::from_secs(1).unwrap(); - let b = Time::from_secs(2).unwrap(); - assert!(a < b); - assert!(b > a); - assert_eq!(a, a); + fn test_add_delta_positive() { + let t = Timestamp::from_millis(100).unwrap(); + let next = t.checked_add_delta(50).unwrap(); + assert_eq!(next.as_millis().unwrap(), 150); } #[test] - fn test_unchecked_variants() { - let time = Time::from_secs_unchecked(5); - assert_eq!(time.as_secs(), 5); - - let time = Time::from_millis_unchecked(5000); - assert_eq!(time.as_millis(), 5000); - - let time = Time::from_micros_unchecked(5_000_000); - assert_eq!(time.as_micros(), 5_000_000); - - let time = Time::from_nanos_unchecked(5_000_000_000); - assert_eq!(time.as_nanos(), 5_000_000_000); - - let time = Time::from_scale_unchecked(5000, 1000); - assert_eq!(time.as_millis(), 5000); + fn test_add_delta_negative() { + let t = Timestamp::from_millis(150).unwrap(); + let next = t.checked_add_delta(-50).unwrap(); + assert_eq!(next.as_millis().unwrap(), 100); } #[test] - fn test_as_scale() { - let time = Time::from_secs(1).unwrap(); - // 1 second in scale 1000 = 1000 - assert_eq!(time.as_scale(1000), 1000); - // 1 second in scale 1 = 1 - assert_eq!(time.as_scale(1), 1); - // 1 second in scale 1_000_000 = 1_000_000 - assert_eq!(time.as_scale(1_000_000), 1_000_000); - } - - #[test] - fn test_convert_to_finer() { - // Convert from milliseconds to microseconds (coarser to finer) - type TimeInMillis = Timescale<1_000>; - type TimeInMicros = Timescale<1_000_000>; - - let time_millis = TimeInMillis::from_millis(5000).unwrap(); - let time_micros: TimeInMicros = time_millis.convert().unwrap(); - - assert_eq!(time_micros.as_millis(), 5000); - assert_eq!(time_micros.as_micros(), 5_000_000); + fn test_add_delta_underflow() { + let t = Timestamp::from_millis(50).unwrap(); + assert!(t.checked_add_delta(-100).is_err()); } #[test] - fn test_convert_to_coarser() { - // Convert from milliseconds to seconds (finer to coarser) - type TimeInMillis = Timescale<1_000>; - type TimeInSeconds = Timescale<1>; - - let time_millis = TimeInMillis::from_millis(5000).unwrap(); - let time_secs: TimeInSeconds = time_millis.convert().unwrap(); - - assert_eq!(time_secs.as_secs(), 5); - assert_eq!(time_secs.as_millis(), 5000); - } - - #[test] - fn test_convert_precision_loss() { - // Converting 1234 millis to seconds loses precision - type TimeInMillis = Timescale<1_000>; - type TimeInSeconds = Timescale<1>; - - let time_millis = TimeInMillis::from_millis(1234).unwrap(); - let time_secs: TimeInSeconds = time_millis.convert().unwrap(); + fn test_duration_conversion() { + let duration = std::time::Duration::from_secs(5); + let time: Timestamp = duration.try_into().unwrap(); + assert_eq!(time.scale(), 1_000_000_000); + assert_eq!(time.as_secs().unwrap(), 5); - // 1234 millis = 1.234 seconds, rounds down to 1 second - assert_eq!(time_secs.as_secs(), 1); - assert_eq!(time_secs.as_millis(), 1000); // Lost 234 millis + let duration_back: std::time::Duration = time.try_into().unwrap(); + assert_eq!(duration_back.as_secs(), 5); } #[test] - fn test_convert_roundtrip() { - // Converting to finer and back should preserve value - type TimeInMillis = Timescale<1_000>; - type TimeInMicros = Timescale<1_000_000>; - - let original = TimeInMillis::from_millis(5000).unwrap(); - let as_micros: TimeInMicros = original.convert().unwrap(); - let back_to_millis: TimeInMillis = as_micros.convert().unwrap(); + fn test_debug_format_units() { + let t = Timestamp::from_millis(100_000).unwrap(); + assert_eq!(format!("{:?}", t), "100s"); - assert_eq!(original.as_millis(), back_to_millis.as_millis()); - } + let t = Timestamp::from_millis(100).unwrap(); + assert_eq!(format!("{:?}", t), "100ms"); - #[test] - fn test_convert_same_scale() { - // Converting to the same scale should be identity - type TimeInMillis = Timescale<1_000>; + let t = Timestamp::from_micros(1500).unwrap(); + assert_eq!(format!("{:?}", t), "1500µs"); - let time = TimeInMillis::from_millis(5000).unwrap(); - let converted: TimeInMillis = time.convert().unwrap(); + let t = Timestamp::from_micros(1000).unwrap(); + assert_eq!(format!("{:?}", t), "1ms"); - assert_eq!(time.as_millis(), converted.as_millis()); + let t = Timestamp::ZERO; + assert_eq!(format!("{:?}", t), "0/?"); } #[test] - fn test_convert_microseconds_to_nanoseconds() { - type TimeInMicros = Timescale<1_000_000>; - type TimeInNanos = Timescale<1_000_000_000>; - - let time_micros = TimeInMicros::from_micros(5_000_000).unwrap(); - let time_nanos: TimeInNanos = time_micros.convert().unwrap(); - - assert_eq!(time_nanos.as_micros(), 5_000_000); - assert_eq!(time_nanos.as_nanos(), 5_000_000_000); + fn test_new() { + let t = Timestamp::new(5000, 1_000); + assert_eq!(t.value(), 5000); + assert_eq!(t.scale(), 1_000); + assert_eq!(t.as_millis().unwrap(), 5000); } #[test] - fn test_convert_custom_scales() { - // Test with unusual custom scales - type TimeScale60 = Timescale<60>; // 60Hz - type TimeScale90 = Timescale<90>; // 90Hz - - let time60 = TimeScale60::from_scale(120, 60).unwrap(); // 2 seconds at 60Hz - let time90: TimeScale90 = time60.convert().unwrap(); - - // Both should represent 2 seconds - assert_eq!(time60.as_secs(), 2); - assert_eq!(time90.as_secs(), 2); + fn test_from_scale_custom() { + // 120 units at 60Hz = 2 seconds, expressed at 1000Hz = 2000 ms. + let t = Timestamp::from_scale(120, 60, 1_000).unwrap(); + assert_eq!(t.scale(), 1_000); + assert_eq!(t.as_millis().unwrap(), 2000); } #[test] - fn test_debug_format_units() { - // Test that Debug chooses appropriate units based on value - - // Milliseconds that are clean seconds - let t = Time::from_millis(100000).unwrap(); - assert_eq!(format!("{:?}", t), "100s"); - - let t = Time::from_millis(1000).unwrap(); - assert_eq!(format!("{:?}", t), "1s"); - - // Milliseconds that are clean milliseconds - let t = Time::from_millis(100).unwrap(); - assert_eq!(format!("{:?}", t), "100ms"); - - let t = Time::from_millis(5500).unwrap(); - assert_eq!(format!("{:?}", t), "5500ms"); - - // Zero - let t = Time::ZERO; - assert_eq!(format!("{:?}", t), "0s"); - - // Test with microsecond-scale time - type TimeMicros = Timescale<1_000_000>; - let t = TimeMicros::from_micros(1500).unwrap(); - assert_eq!(format!("{:?}", t), "1500µs"); - - let t = TimeMicros::from_micros(1000).unwrap(); - assert_eq!(format!("{:?}", t), "1ms"); + fn test_from_scale_zero_source() { + assert!(Timestamp::from_scale(5, 0, 1_000).is_err()); } } From de27411ead5175f889db72b1c8d7608749730bd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 22:18:25 +0000 Subject: [PATCH 02/10] moq-net: add Track.timescale, Frame.timestamp, zigzag varint Each Track now carries a timescale (units per second) and each Frame carries a presentation Timestamp in that timescale. Both default to the unspecified sentinel (scale 0) when not negotiated. Adds VarInt::from_zigzag / to_zigzag for the upcoming moq-lite delta-encoded timestamp wire format. Inputs must fit in i61 (the 62-bit varint range after zigzag), which is ample for frame-to-frame deltas. Updates internal struct-literal call sites across hang, moq-mux, libmoq, moq-ffi, moq-relay, moq-clock to include the new fields with sensible defaults (0 = unspecified). No wire-format changes yet. https://claude.ai/code/session_01Wy7egQjXvfPCKovs3mWANB --- rs/hang/examples/subscribe.rs | 1 + rs/hang/examples/video.rs | 1 + rs/hang/src/catalog/root.rs | 1 + rs/libmoq/src/consume.rs | 2 + rs/moq-clock/src/main.rs | 1 + rs/moq-ffi/src/consumer.rs | 12 ++++- rs/moq-ffi/src/producer.rs | 6 ++- rs/moq-net/src/coding/varint.rs | 77 ++++++++++++++++++++++++++++++- rs/moq-net/src/ietf/publisher.rs | 1 + rs/moq-net/src/ietf/subscriber.rs | 5 +- rs/moq-net/src/lite/publisher.rs | 1 + rs/moq-net/src/lite/subscriber.rs | 2 +- rs/moq-net/src/model/broadcast.rs | 6 ++- rs/moq-net/src/model/frame.rs | 61 ++++++++++++++++-------- rs/moq-net/src/model/group.rs | 6 +-- rs/moq-net/src/model/track.rs | 25 +++++++++- rs/moq-relay/src/web.rs | 1 + 17 files changed, 174 insertions(+), 35 deletions(-) diff --git a/rs/hang/examples/subscribe.rs b/rs/hang/examples/subscribe.rs index 3a33a4193..f5cc77268 100644 --- a/rs/hang/examples/subscribe.rs +++ b/rs/hang/examples/subscribe.rs @@ -75,6 +75,7 @@ async fn run_subscribe(mut consumer: moq_net::OriginConsumer) -> anyhow::Result< let track = moq_net::Track { name: name.clone(), priority: 1, + timescale: 0, }; let track_consumer = broadcast.subscribe_track(&track)?; diff --git a/rs/hang/examples/video.rs b/rs/hang/examples/video.rs index b080cb7fd..009c25ee2 100644 --- a/rs/hang/examples/video.rs +++ b/rs/hang/examples/video.rs @@ -44,6 +44,7 @@ fn create_track(broadcast: &mut moq_net::BroadcastProducer) -> anyhow::Result anyhow::Result<()> { let track = Track { name: config.track, priority: 0, + timescale: 0, }; let origin = moq_net::Origin::random().produce(); diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index e5fa4f2f0..93f6d2eb1 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -90,7 +90,11 @@ impl MoqBroadcastConsumer { /// Frames are returned as plain byte payloads with no codec or container parsing. pub fn subscribe_track(&self, name: String) -> Result, MoqError> { let _guard = crate::ffi::RUNTIME.enter(); - let track = self.inner.subscribe_track(&moq_net::Track { name, priority: 0 })?; + let track = self.inner.subscribe_track(&moq_net::Track { + name, + priority: 0, + timescale: 0, + })?; Ok(Arc::new(MoqTrackConsumer::new(track))) } @@ -111,7 +115,11 @@ impl MoqBroadcastConsumer { let media: moq_mux::container::Hang = (&container) .try_into() .map_err(|e| MoqError::Codec(format!("invalid container: {e}")))?; - let track = self.inner.subscribe_track(&moq_net::Track { name, priority: 0 })?; + let track = self.inner.subscribe_track(&moq_net::Track { + name, + priority: 0, + timescale: 0, + })?; let latency = std::time::Duration::from_millis(max_latency_ms); let consumer = moq_mux::container::Consumer::new(track, media).with_latency(latency); Ok(Arc::new(MoqMediaConsumer { diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index ca7c24cc3..19a82579c 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -93,7 +93,11 @@ impl MoqBroadcastProducer { let _guard = crate::ffi::RUNTIME.enter(); let guard = self.state.lock().unwrap(); let state = guard.as_ref().ok_or_else(|| MoqError::Closed)?; - let track = moq_net::Track { name, priority: 0 }; + let track = moq_net::Track { + name, + priority: 0, + timescale: 0, + }; // Clone the broadcast handle (shared Arc internally) to get &mut access. let mut broadcast = state.broadcast.clone(); let producer = broadcast.create_track(track)?; diff --git a/rs/moq-net/src/coding/varint.rs b/rs/moq-net/src/coding/varint.rs index 81e6b8ca8..c98f7ca4b 100644 --- a/rs/moq-net/src/coding/varint.rs +++ b/rs/moq-net/src/coding/varint.rs @@ -52,6 +52,25 @@ impl VarInt { pub const fn into_inner(self) -> u64 { self.0 } + + /// Encode a signed `i64` as a zigzag-then-unsigned varint: `(n << 1) ^ (n >> 63)`. + /// + /// Small negative numbers map to small unsigneds (-1 -> 1, 1 -> 2, -2 -> 3, ...). + /// Returns [`BoundsExceeded`] if `signed` is outside `[-2^61, 2^61 - 1]`, since the + /// zigzag-encoded result must fit in a 62-bit varint. + pub const fn from_zigzag(signed: i64) -> Result { + const RANGE: i64 = 1 << 61; + if signed < -RANGE || signed >= RANGE { + return Err(BoundsExceeded); + } + Ok(Self(((signed << 1) ^ (signed >> 63)) as u64)) + } + + /// Decode this varint as a signed `i64` via the inverse zigzag transform. + pub const fn to_zigzag(self) -> i64 { + let v = self.0; + ((v >> 1) as i64) ^ -((v & 1) as i64) + } } impl From for u64 { @@ -542,8 +561,8 @@ where #[cfg(test)] mod tests { - use super::{DecodeError, VarInt}; - use crate::ietf; + use super::*; + use crate::{ietf, lite}; use bytes::Bytes; /// Test vectors from the draft-17 spec (Table 2: Example Integer Encodings), @@ -645,6 +664,60 @@ mod tests { assert!(matches!(err, DecodeError::InvalidValue)); } + #[test] + fn zigzag_roundtrip_small() { + for n in [-3i64, -2, -1, 0, 1, 2, 3, 100, -100] { + let v = VarInt::from_zigzag(n).unwrap(); + assert_eq!(v.to_zigzag(), n, "roundtrip failed for {}", n); + } + } + + #[test] + fn zigzag_small_values_compact() { + // First few values should fit in 1 byte (varint range 0..=63 = top-2-bits tag 00). + assert_eq!(VarInt::from_zigzag(0).unwrap().into_inner(), 0); + assert_eq!(VarInt::from_zigzag(-1).unwrap().into_inner(), 1); + assert_eq!(VarInt::from_zigzag(1).unwrap().into_inner(), 2); + assert_eq!(VarInt::from_zigzag(-2).unwrap().into_inner(), 3); + assert_eq!(VarInt::from_zigzag(2).unwrap().into_inner(), 4); + } + + #[test] + fn zigzag_roundtrip_boundary() { + // Boundary values in the valid input range [-2^61, 2^61 - 1]. + let max = (1i64 << 61) - 1; + let min = -(1i64 << 61); + let mid = (1i64 << 30) + 17; + + for n in [max, min, mid, -mid] { + let v = VarInt::from_zigzag(n).unwrap(); + assert_eq!(v.to_zigzag(), n); + } + } + + #[test] + fn zigzag_out_of_range_rejected() { + // Values past the i61 boundary are out of varint range. + assert!(VarInt::from_zigzag(1i64 << 61).is_err()); + assert!(VarInt::from_zigzag(-(1i64 << 61) - 1).is_err()); + assert!(VarInt::from_zigzag(i64::MAX).is_err()); + assert!(VarInt::from_zigzag(i64::MIN).is_err()); + } + + #[test] + fn zigzag_quic_varint_roundtrip() { + // Encode a zigzag value through the QUIC varint wire format. + for n in [-5000i64, 0, 100, -1, 1_000_000, -1_000_000] { + let v = VarInt::from_zigzag(n).unwrap(); + + let mut buf = bytes::BytesMut::new(); + v.encode(&mut buf, lite::Version::Lite01).unwrap(); + let mut bytes = buf.freeze(); + let decoded = VarInt::decode(&mut bytes, lite::Version::Lite01).unwrap(); + assert_eq!(decoded.to_zigzag(), n); + } + } + #[test] fn draft18_accepts_7_byte_varint() { // Value 0x1234_5678_9ABC encoded as 7-byte leading-ones (1111110x | hi, +6 bytes). diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 45772bde5..69f164ee8 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -116,6 +116,7 @@ impl Publisher { let track = Track { name: msg.track_name.to_string(), priority: msg.subscriber_priority, + timescale: 0, }; let track = match broadcast.subscribe_track(&track) { diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index f371d7ff6..605518d3a 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -465,6 +465,7 @@ impl Subscriber { let track = Track { name: msg.track_name.to_string(), priority: 0, + timescale: 0, } .produce(); @@ -736,7 +737,7 @@ impl Subscriber { if size == 0 { let status: u64 = stream.decode().await?; if status == 0 { - let mut frame = producer.create_frame(Frame { size: 0 })?; + let mut frame = producer.create_frame(Frame::new(0))?; frame.finish()?; } else if status == 3 && !group.flags.has_end { break; @@ -744,7 +745,7 @@ impl Subscriber { return Err(Error::Unsupported); } } else { - let mut frame = producer.create_frame(Frame { size })?; + let mut frame = producer.create_frame(Frame::new(size))?; if let Err(err) = self.run_frame(stream, frame.clone()).await { let _ = frame.abort(err.clone()); diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index cc2b9fd7c..97fe61dbb 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -308,6 +308,7 @@ impl Publisher { let track = Track { name: subscribe.track.to_string(), priority: subscribe.priority, + timescale: 0, }; let broadcast = consumer.ok_or(Error::NotFound)?; diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 767dd9816..6c9cfb876 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -404,7 +404,7 @@ impl Subscriber { mut group: GroupProducer, ) -> Result<(), Error> { while let Some(size) = stream.decode_maybe::().await? { - let mut frame = group.create_frame(Frame { size })?; + let mut frame = group.create_frame(Frame::new(size))?; if let Err(err) = self.run_frame(stream, &mut frame).await { let _ = frame.abort(err.clone()); diff --git a/rs/moq-net/src/model/broadcast.rs b/rs/moq-net/src/model/broadcast.rs index d866f7fc5..8c394096b 100644 --- a/rs/moq-net/src/model/broadcast.rs +++ b/rs/moq-net/src/model/broadcast.rs @@ -135,7 +135,11 @@ impl BroadcastProducer { } drop(state); - self.create_track(Track { name, priority: 0 }) + self.create_track(Track { + name, + priority: 0, + timescale: 0, + }) } /// Create a dynamic producer that handles on-demand track requests from consumers. diff --git a/rs/moq-net/src/model/frame.rs b/rs/moq-net/src/model/frame.rs index 2912a87d2..9ddc91639 100644 --- a/rs/moq-net/src/model/frame.rs +++ b/rs/moq-net/src/model/frame.rs @@ -5,20 +5,41 @@ use std::task::{Poll, ready}; use bytes::buf::UninitSlice; use bytes::{BufMut, Bytes}; -use crate::{Error, Result}; +use crate::{Error, Result, Timestamp}; -/// A chunk of data with an upfront size. +/// A chunk of data with an upfront size and presentation timestamp. /// /// Note that this is just the header. /// You use [FrameProducer] and [FrameConsumer] to deal with the frame payload, potentially chunked. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Frame { /// Total payload size in bytes. Declared up front so consumers can preallocate. pub size: u64, + /// Presentation timestamp in the parent track's timescale. + /// + /// Defaults to [`Timestamp::ZERO`] (unspecified scale). Producers should set this + /// to a value at the same scale as [`crate::Track::timescale`] before writing the + /// frame; the wire encoder errors on serialization if the scales disagree (for + /// protocols that delta-encode timestamps on the wire). + pub timestamp: Timestamp, } impl Frame { + /// Create a frame header with the given size and an unspecified timestamp. + pub fn new(size: u64) -> Self { + Self { + size, + timestamp: Timestamp::ZERO, + } + } + + /// Set the presentation timestamp. + pub fn with_timestamp(mut self, timestamp: Timestamp) -> Self { + self.timestamp = timestamp; + self + } + /// Create a new producer for the frame. pub fn produce(self) -> FrameProducer { FrameProducer::new(self) @@ -27,25 +48,25 @@ impl Frame { impl From for Frame { fn from(size: usize) -> Self { - Self { size: size as u64 } + Self::new(size as u64) } } impl From for Frame { fn from(size: u64) -> Self { - Self { size } + Self::new(size) } } impl From for Frame { fn from(size: u32) -> Self { - Self { size: size as u64 } + Self::new(size as u64) } } impl From for Frame { fn from(size: u16) -> Self { - Self { size: size as u64 } + Self::new(size as u64) } } @@ -437,7 +458,7 @@ mod test { #[test] fn single_chunk_roundtrip() { - let mut producer = Frame { size: 5 }.produce(); + let mut producer = Frame::new(5).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.finish().unwrap(); @@ -448,7 +469,7 @@ mod test { #[test] fn multi_chunk_read_all() { - let mut producer = Frame { size: 10 }.produce(); + let mut producer = Frame::new(10).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.write(Bytes::from_static(b"world")).unwrap(); producer.finish().unwrap(); @@ -460,7 +481,7 @@ mod test { #[test] fn read_chunk_sequential() { - let mut producer = Frame { size: 10 }.produce(); + let mut producer = Frame::new(10).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); // Each read_chunk returns whatever is new since the last call, // which may span multiple writes. @@ -479,7 +500,7 @@ mod test { #[test] fn read_all_chunks() { - let mut producer = Frame { size: 10 }.produce(); + let mut producer = Frame::new(10).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.write(Bytes::from_static(b"world")).unwrap(); producer.finish().unwrap(); @@ -492,7 +513,7 @@ mod test { #[test] fn finish_checks_remaining() { - let mut producer = Frame { size: 5 }.produce(); + let mut producer = Frame::new(5).produce(); producer.write(Bytes::from_static(b"hi")).unwrap(); let err = producer.finish().unwrap_err(); assert!(matches!(err, Error::WrongSize)); @@ -500,14 +521,14 @@ mod test { #[test] fn write_too_many_bytes() { - let mut producer = Frame { size: 3 }.produce(); + let mut producer = Frame::new(3).produce(); let err = producer.write(Bytes::from_static(b"toolong")).unwrap_err(); assert!(matches!(err, Error::WrongSize)); } #[test] fn abort_propagates() { - let mut producer = Frame { size: 5 }.produce(); + let mut producer = Frame::new(5).produce(); let mut consumer = producer.consume(); producer.abort(Error::Cancel).unwrap(); @@ -517,7 +538,7 @@ mod test { #[test] fn empty_frame() { - let mut producer = Frame { size: 0 }.produce(); + let mut producer = Frame::new(0).produce(); producer.finish().unwrap(); let mut consumer = producer.consume(); @@ -527,7 +548,7 @@ mod test { #[tokio::test] async fn pending_then_ready() { - let mut producer = Frame { size: 5 }.produce(); + let mut producer = Frame::new(5).produce(); let mut consumer = producer.consume(); // Consumer blocks because no data yet. @@ -543,7 +564,7 @@ mod test { #[test] fn buf_mut_roundtrip() { // Exercise the BufMut path that the receive loop uses via `read_buf`. - let mut producer = Frame { size: 12 }.produce(); + let mut producer = Frame::new(12).produce(); assert_eq!(producer.remaining_mut(), 12); producer.put_slice(b"hello"); assert_eq!(producer.remaining_mut(), 7); @@ -559,14 +580,14 @@ mod test { #[test] #[should_panic(expected = "advance_mut past frame.size")] fn buf_mut_advance_past_capacity_panics() { - let mut producer = Frame { size: 4 }.produce(); + let mut producer = Frame::new(4).produce(); // Safety violation on purpose: cnt > remaining_mut(). unsafe { producer.advance_mut(5) }; } #[test] fn read_chunk_streams_partial_writes() { - let mut producer = Frame { size: 6 }.produce(); + let mut producer = Frame::new(6).produce(); let mut consumer = producer.consume(); producer.write(Bytes::from_static(b"foo")).unwrap(); @@ -586,7 +607,7 @@ mod test { #[test] fn cloned_consumer_independent_cursor() { - let mut producer = Frame { size: 10 }.produce(); + let mut producer = Frame::new(10).produce(); let mut c1 = producer.consume(); producer.write(Bytes::from_static(b"hello")).unwrap(); diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index b3b08a136..0cb1c3fdb 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -166,9 +166,7 @@ impl GroupProducer { /// But an upfront size is required. pub fn write_frame>(&mut self, frame: B) -> Result<()> { let data = frame.into(); - let frame = Frame { - size: data.len() as u64, - }; + let frame = Frame::new(data.len() as u64); let mut frame = self.create_frame(frame)?; frame.write(data)?; frame.finish()?; @@ -404,7 +402,7 @@ mod test { #[test] fn read_frame_chunks() { let mut producer = Group { sequence: 0 }.produce(); - let mut frame = producer.create_frame(Frame { size: 10 }).unwrap(); + let mut frame = producer.create_frame(Frame::new(10)).unwrap(); frame.write(Bytes::from_static(b"hello")).unwrap(); frame.write(Bytes::from_static(b"world")).unwrap(); frame.finish().unwrap(); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 312baaa68..7cc7e9a40 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -27,24 +27,45 @@ use std::{ const MAX_GROUP_AGE: Duration = Duration::from_secs(5); /// A track is a collection of groups, delivered out-of-order until expired. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Track { /// Identifier within a broadcast. Unique per [`crate::Broadcast`]. pub name: String, /// Delivery priority. Higher values preempt lower ones when bandwidth is constrained. pub priority: u8, + /// Units per second for frame timestamps on this track. + /// + /// `0` (the default) means unspecified, typically because the peer didn't negotiate + /// a timescale (older moq-lite or older moq-transport drafts) or none has been + /// learned yet. Producers set this before publishing; subscribers learn it from + /// [`crate::BroadcastConsumer::subscribe_track`] once SUBSCRIBE_OK arrives. + pub timescale: u64, } impl Track { - /// Create a track with the given name and default priority (`0`). + /// Create a track with the given name, default priority (`0`), and unspecified + /// timescale. pub fn new>(name: T) -> Self { Self { name: name.into(), priority: 0, + timescale: 0, } } + /// Set the delivery priority. Higher values preempt lower ones. + pub fn with_priority(mut self, priority: u8) -> Self { + self.priority = priority; + self + } + + /// Set the per-track timescale (units per second). + pub fn with_timescale(mut self, timescale: u64) -> Self { + self.timescale = timescale; + self + } + /// Consume this [`Track`] to create a producer that owns its metadata. pub fn produce(self) -> TrackProducer { TrackProducer::new(self) diff --git a/rs/moq-relay/src/web.rs b/rs/moq-relay/src/web.rs index 61f4c48f2..76507d571 100644 --- a/rs/moq-relay/src/web.rs +++ b/rs/moq-relay/src/web.rs @@ -509,6 +509,7 @@ async fn serve_fetch( let track = moq_net::Track { name: track, priority: 0, + timescale: 0, }; let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_secs(30); From c116793e5510d31a2db909102dc0761535b86bbe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 22:29:05 +0000 Subject: [PATCH 03/10] moq-net: Lite05 wire format with timescale and delta timestamps Adds moq-lite version Lite05 (ALPN moq-lite-05, code 0xff0dad05): - SUBSCRIBE_OK carries the per-track timescale (varint) so the subscriber learns it at session setup. Older versions decode as 0 (unspecified). - Per-frame wire header gains a zigzag-encoded signed delta from the previous frame's timestamp on the same group stream, before the frame size. Negative deltas support B-frames. - The publisher verifies frame timestamps match the track's negotiated timescale and aborts the connection with ProtocolViolation otherwise. TrackProducer gains a set_timescale method that updates both the local Track info and a new conducer State.timescale, plumbing the value through to consumers. TrackConsumer exposes timescale_now/poll_timescale/timescale to read the resolved value. hang and moq-mux are updated to stamp the moq-net Frame.timestamp on every frame they emit and to set Track.timescale = 1_000_000 (microseconds) on tracks they produce. The legacy container-level timestamp prefix remains as a duplicate during the transition. https://claude.ai/code/session_01Wy7egQjXvfPCKovs3mWANB --- rs/hang/src/container/frame.rs | 8 ++- rs/moq-mux/src/container/cmaf.rs | 6 +- rs/moq-mux/src/import/aac.rs | 3 +- rs/moq-mux/src/import/av01.rs | 9 ++- rs/moq-mux/src/import/avc1.rs | 3 +- rs/moq-mux/src/import/avc3.rs | 3 +- rs/moq-mux/src/import/fmp4.rs | 4 +- rs/moq-mux/src/import/hev1.rs | 3 +- rs/moq-mux/src/import/opus.rs | 3 +- rs/moq-net/src/lite/publisher.rs | 25 +++++++- rs/moq-net/src/lite/subscribe.rs | 100 ++++++++++++++++++++++++++---- rs/moq-net/src/lite/subscriber.rs | 48 ++++++++++++-- rs/moq-net/src/lite/version.rs | 22 +++++-- rs/moq-net/src/model/track.rs | 53 +++++++++++++++- rs/moq-net/src/setup.rs | 2 +- rs/moq-net/src/version.rs | 8 +++ 16 files changed, 259 insertions(+), 41 deletions(-) diff --git a/rs/hang/src/container/frame.rs b/rs/hang/src/container/frame.rs index 6e0f772f9..cf084e60e 100644 --- a/rs/hang/src/container/frame.rs +++ b/rs/hang/src/container/frame.rs @@ -32,14 +32,18 @@ pub struct Frame { impl Frame { /// Encode the frame to the given group as a single moq-lite frame: - /// VarInt timestamp prefix followed by the raw codec payload. + /// VarInt timestamp prefix followed by the raw codec payload. Also stamps + /// the moq-net [`moq_net::Frame::timestamp`] so the wire layer can + /// delta-encode it independently on Lite05+ (the container-level prefix + /// stays as a duplicate for now). pub fn encode(&self, group: &mut moq_net::GroupProducer) -> Result<(), Error> { let mut header = BytesMut::new(); self.timestamp.encode_value(&mut header).map_err(moq_net::Error::from)?; let size = header.len() + self.payload.len(); - let mut chunked = group.create_frame(size.into())?; + let net_frame = moq_net::Frame::new(size as u64).with_timestamp(self.timestamp); + let mut chunked = group.create_frame(net_frame)?; chunked.write(header.freeze())?; chunked.write(self.payload.clone())?; chunked.finish()?; diff --git a/rs/moq-mux/src/container/cmaf.rs b/rs/moq-mux/src/container/cmaf.rs index 0506f70e1..38a1b3ea6 100644 --- a/rs/moq-mux/src/container/cmaf.rs +++ b/rs/moq-mux/src/container/cmaf.rs @@ -223,7 +223,11 @@ pub(crate) fn encode( let mdat = mp4_atom::Mdat { data: mdat_data }; mdat.encode(&mut buf)?; - let mut writer = group.create_frame(buf.len().into())?; + // Stamp the wire-level timestamp on the moq-net frame so Lite05+ can + // delta-encode it. The CMAF fragment may pack multiple media samples; use + // the first sample's PTS as the representative timestamp. + let net_frame = moq_net::Frame::new(buf.len() as u64).with_timestamp(frames[0].timestamp); + let mut writer = group.create_frame(net_frame)?; writer.write(Bytes::from(buf))?; writer.finish()?; diff --git a/rs/moq-mux/src/import/aac.rs b/rs/moq-mux/src/import/aac.rs index da7e72faa..1cb66247a 100644 --- a/rs/moq-mux/src/import/aac.rs +++ b/rs/moq-mux/src/import/aac.rs @@ -112,7 +112,8 @@ impl Aac { mut catalog: crate::catalog::Producer, config: AacConfig, ) -> anyhow::Result { - let track = broadcast.unique_track(".aac")?; + let mut track = broadcast.unique_track(".aac")?; + track.set_timescale(hang::container::TIMESCALE); let audio_config = hang::catalog::AudioConfig { codec: hang::catalog::AAC { diff --git a/rs/moq-mux/src/import/av01.rs b/rs/moq-mux/src/import/av01.rs index 82c9b23ba..af7c4ae68 100644 --- a/rs/moq-mux/src/import/av01.rs +++ b/rs/moq-mux/src/import/av01.rs @@ -102,7 +102,8 @@ impl Av01 { self.catalog.lock().video.renditions.remove(&track.name); } - let track = self.broadcast.unique_track(".av01")?; + let mut track = self.broadcast.unique_track(".av01")?; + track.set_timescale(hang::container::TIMESCALE); tracing::debug!(name = ?track.name, ?config, "starting track"); self.catalog .lock() @@ -146,7 +147,8 @@ impl Av01 { jitter: None, }; - let track = self.broadcast.unique_track(".av01")?; + let mut track = self.broadcast.unique_track(".av01")?; + track.set_timescale(hang::container::TIMESCALE); tracing::debug!(name = ?track.name, "starting track with minimal config"); self.catalog .lock() @@ -236,7 +238,8 @@ impl Av01 { self.catalog.lock().video.renditions.remove(&track.name); } - let track = self.broadcast.unique_track(".av01")?; + let mut track = self.broadcast.unique_track(".av01")?; + track.set_timescale(hang::container::TIMESCALE); self.catalog .lock() .video diff --git a/rs/moq-mux/src/import/avc1.rs b/rs/moq-mux/src/import/avc1.rs index 7014d2413..6fe320b02 100644 --- a/rs/moq-mux/src/import/avc1.rs +++ b/rs/moq-mux/src/import/avc1.rs @@ -103,7 +103,8 @@ impl Avc1 { catalog.video.renditions.remove(&track.name); } - let track = self.broadcast.unique_track(".avc1")?; + let mut track = self.broadcast.unique_track(".avc1")?; + track.set_timescale(hang::container::TIMESCALE); tracing::debug!(name = ?track.name, ?config, "starting avc1 track"); catalog.video.renditions.insert(track.name.clone(), config.clone()); diff --git a/rs/moq-mux/src/import/avc3.rs b/rs/moq-mux/src/import/avc3.rs index 6b123fe7e..38b04ac95 100644 --- a/rs/moq-mux/src/import/avc3.rs +++ b/rs/moq-mux/src/import/avc3.rs @@ -39,7 +39,8 @@ impl Avc3 { pub fn new(mut broadcast: moq_net::BroadcastProducer, catalog: crate::catalog::Producer) -> Self { // Create the track eagerly so callers can monitor used/unused before any frames arrive. // The catalog entry is added later in init() once the codec config is known. - let track = broadcast.unique_track(".avc3").expect("failed to create avc3 track"); + let mut track = broadcast.unique_track(".avc3").expect("failed to create avc3 track"); + track.set_timescale(hang::container::TIMESCALE); Self { catalog, diff --git a/rs/moq-mux/src/import/fmp4.rs b/rs/moq-mux/src/import/fmp4.rs index 9404d99eb..a742d52f9 100644 --- a/rs/moq-mux/src/import/fmp4.rs +++ b/rs/moq-mux/src/import/fmp4.rs @@ -142,7 +142,9 @@ impl Fmp4 { let handler = &trak.mdia.hdlr.handler; let suffix = ".m4s"; - let track = self.broadcast.unique_track(suffix)?; + let mut track = self.broadcast.unique_track(suffix)?; + // moq-mux frames are always emitted at microsecond timescale. + track.set_timescale(hang::container::TIMESCALE); let kind = match handler.as_ref() { b"vide" => { diff --git a/rs/moq-mux/src/import/hev1.rs b/rs/moq-mux/src/import/hev1.rs index 49aecc309..b051e51cd 100644 --- a/rs/moq-mux/src/import/hev1.rs +++ b/rs/moq-mux/src/import/hev1.rs @@ -92,7 +92,8 @@ impl Hev1 { catalog.video.renditions.remove(&track.name); } - let track = self.broadcast.unique_track(".hev1")?; + let mut track = self.broadcast.unique_track(".hev1")?; + track.set_timescale(hang::container::TIMESCALE); tracing::debug!(name = ?track.name, ?config, "starting track"); catalog.video.renditions.insert(track.name.clone(), config.clone()); diff --git a/rs/moq-mux/src/import/opus.rs b/rs/moq-mux/src/import/opus.rs index 4bce37985..862343c2f 100644 --- a/rs/moq-mux/src/import/opus.rs +++ b/rs/moq-mux/src/import/opus.rs @@ -55,7 +55,8 @@ impl Opus { mut catalog: crate::catalog::Producer, config: OpusConfig, ) -> anyhow::Result { - let track = broadcast.unique_track(".opus")?; + let mut track = broadcast.unique_track(".opus")?; + track.set_timescale(hang::container::TIMESCALE); let audio_config = hang::catalog::AudioConfig { codec: hang::catalog::AudioCodec::Opus, diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 97fe61dbb..999b41802 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -322,6 +322,7 @@ impl Publisher { max_latency: std::time::Duration::ZERO, start_group: None, end_group: None, + timescale: track.timescale, }; stream.writer.encode(&lite::SubscribeResponse::Ok(info)).await?; @@ -369,7 +370,9 @@ impl Publisher { }; let priority = priority.insert(track.priority, sequence); - tasks.push(Self::serve_group(session.clone(), msg, priority, group, version).map(|_| ())); + tasks.push( + Self::serve_group(session.clone(), msg, priority, group, version, track.timescale).map(|_| ()), + ); } } @@ -379,6 +382,7 @@ impl Publisher { mut priority: PriorityHandle, mut group: GroupConsumer, version: Version, + track_timescale: u64, ) -> Result<(), Error> { // TODO add a way to open in priority order. let stream = session.open_uni().await.map_err(Error::from_transport)?; @@ -388,6 +392,9 @@ impl Publisher { stream.encode(&lite::DataType::Group).await?; stream.encode(&msg).await?; + // Previous frame timestamp on this group's stream, for zigzag-delta encoding on Lite05+. + let mut prev_ts: u64 = 0; + loop { let frame = tokio::select! { biased; @@ -405,6 +412,22 @@ impl Publisher { None => break, }; + if version.has_timestamps() { + // Verify per-frame timestamp scale matches the track's negotiated timescale. + let ts = frame.timestamp; + if !ts.is_unspecified() && ts.scale() != track_timescale { + return Err(Error::ProtocolViolation); + } + let curr = ts.value(); + let delta: i64 = (curr as i128 - prev_ts as i128) + .try_into() + .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; + let zz = crate::coding::VarInt::from_zigzag(delta) + .map_err(crate::coding::EncodeError::from)?; + stream.encode(&zz).await?; + prev_ts = curr; + } + stream.encode(&frame.size).await?; loop { diff --git a/rs/moq-net/src/lite/subscribe.rs b/rs/moq-net/src/lite/subscribe.rs index a4966dde5..c87cb1a83 100644 --- a/rs/moq-net/src/lite/subscribe.rs +++ b/rs/moq-net/src/lite/subscribe.rs @@ -72,13 +72,17 @@ impl Message for Subscribe<'_> { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct SubscribeOk { pub priority: u8, pub ordered: bool, pub max_latency: std::time::Duration, pub start_group: Option, pub end_group: Option, + /// Units per second for frame timestamps on this track. `0` (the default) + /// means unspecified. Carried on the wire for [`Version::Lite05`] and later; + /// older versions decode it as `0`. + pub timescale: u64, } impl Message for SubscribeOk { @@ -88,12 +92,20 @@ impl Message for SubscribeOk { self.priority.encode(w, version)?; } Version::Lite02 => {} + Version::Lite03 | Version::Lite04 => { + self.priority.encode(w, version)?; + (self.ordered as u8).encode(w, version)?; + self.max_latency.encode(w, version)?; + self.start_group.encode(w, version)?; + self.end_group.encode(w, version)?; + } _ => { self.priority.encode(w, version)?; (self.ordered as u8).encode(w, version)?; self.max_latency.encode(w, version)?; self.start_group.encode(w, version)?; self.end_group.encode(w, version)?; + self.timescale.encode(w, version)?; } } @@ -104,24 +116,32 @@ impl Message for SubscribeOk { match version { Version::Lite01 => Ok(Self { priority: u8::decode(r, version)?, - ordered: false, - max_latency: std::time::Duration::ZERO, - start_group: None, - end_group: None, - }), - Version::Lite02 => Ok(Self { - priority: 0, - ordered: false, - max_latency: std::time::Duration::ZERO, - start_group: None, - end_group: None, + ..Self::default() }), + Version::Lite02 => Ok(Self::default()), + Version::Lite03 | Version::Lite04 => { + let priority = u8::decode(r, version)?; + let ordered = u8::decode(r, version)? != 0; + let max_latency = std::time::Duration::decode(r, version)?; + let start_group = Option::::decode(r, version)?; + let end_group = Option::::decode(r, version)?; + + Ok(Self { + priority, + ordered, + max_latency, + start_group, + end_group, + timescale: 0, + }) + } _ => { let priority = u8::decode(r, version)?; let ordered = u8::decode(r, version)? != 0; let max_latency = std::time::Duration::decode(r, version)?; let start_group = Option::::decode(r, version)?; let end_group = Option::::decode(r, version)?; + let timescale = u64::decode(r, version)?; Ok(Self { priority, @@ -129,6 +149,7 @@ impl Message for SubscribeOk { max_latency, start_group, end_group, + timescale, }) } } @@ -326,3 +347,58 @@ impl Decode for SubscribeResponse { } } } + +#[cfg(test)] +mod tests { + use super::*; + use bytes::BytesMut; + + fn roundtrip_ok(version: Version, original: SubscribeOk) -> SubscribeOk { + let mut buf = BytesMut::new(); + original.encode_msg(&mut buf, version).unwrap(); + let mut bytes = buf.freeze(); + SubscribeOk::decode_msg(&mut bytes, version).unwrap() + } + + #[test] + fn subscribe_ok_lite04_drops_timescale() { + // On Lite04, timescale is not serialized; it should round-trip as 0. + let ok = SubscribeOk { + priority: 7, + ordered: true, + max_latency: std::time::Duration::from_millis(500), + start_group: Some(2), + end_group: Some(10), + timescale: 1_000_000, + }; + let decoded = roundtrip_ok(Version::Lite04, ok); + assert_eq!(decoded.priority, 7); + assert_eq!(decoded.ordered, true); + assert_eq!(decoded.start_group, Some(2)); + assert_eq!(decoded.end_group, Some(10)); + assert_eq!(decoded.timescale, 0); + } + + #[test] + fn subscribe_ok_lite05_carries_timescale() { + let ok = SubscribeOk { + priority: 3, + ordered: false, + max_latency: std::time::Duration::from_millis(100), + start_group: None, + end_group: None, + timescale: 1_000_000, + }; + let decoded = roundtrip_ok(Version::Lite05, ok); + assert_eq!(decoded.priority, 3); + assert_eq!(decoded.timescale, 1_000_000); + } + + #[test] + fn subscribe_ok_lite05_unspecified_timescale() { + // timescale = 0 round-trips on Lite05 (still serialized, just as 0). + let ok = SubscribeOk::default(); + let decoded = roundtrip_ok(Version::Lite05, ok); + assert_eq!(decoded.timescale, 0); + } +} diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 6c9cfb876..9909f46fd 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -350,14 +350,25 @@ impl Subscriber { stream: &mut Stream, msg: lite::Subscribe<'_>, ) -> Result<(), Error> { + let subscribe_id = msg.id; stream.writer.encode(&msg).await?; // The first response MUST be a SUBSCRIBE_OK. let resp: lite::SubscribeResponse = stream.reader.decode().await?; - let lite::SubscribeResponse::Ok(_info) = resp else { + let lite::SubscribeResponse::Ok(info) = resp else { return Err(Error::ProtocolViolation); }; + // Apply the negotiated timescale to the track producer so consumers can + // observe it via TrackConsumer::timescale() and run_group can decode + // per-frame timestamps at the correct scale. + { + let mut subs = self.subscribes.lock(); + if let Some(track) = subs.get_mut(&subscribe_id) { + track.set_timescale(info.timescale); + } + } + // TODO handle additional SUBSCRIBE_OK and SUBSCRIBE_DROP messages. stream.reader.closed().await?; @@ -367,19 +378,20 @@ impl Subscriber { pub async fn recv_group(&mut self, stream: &mut Reader) -> Result<(), Error> { let hdr: lite::Group = stream.decode().await?; - let (mut group, track) = { + let (mut group, track, timescale) = { let mut subs = self.subscribes.lock(); let track = subs.get_mut(&hdr.subscribe).ok_or(Error::Cancel)?; let group_info = Group { sequence: hdr.sequence }; let group = track.create_group(group_info)?; - (group, track.clone()) + let timescale = track.timescale; + (group, track.clone(), timescale) }; let res = tokio::select! { err = track.closed() => Err(err), err = group.closed() => Err(err), - res = self.run_group(stream, group.clone()) => res, + res = self.run_group(stream, group.clone(), timescale) => res, }; match res { @@ -402,9 +414,33 @@ impl Subscriber { &mut self, stream: &mut Reader, mut group: GroupProducer, + track_timescale: u64, ) -> Result<(), Error> { - while let Some(size) = stream.decode_maybe::().await? { - let mut frame = group.create_frame(Frame::new(size))?; + // Previous frame's raw timestamp value, for zigzag-delta decoding on Lite05+. + let mut prev_ts: u64 = 0; + + loop { + let (size, timestamp) = if self.version.has_timestamps() { + let Some(zz) = stream.decode_maybe::().await? else { + break; + }; + let delta = zz.to_zigzag(); + let next = (prev_ts as i128 + delta as i128) + .try_into() + .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; + prev_ts = next; + let size = stream.decode::().await?; + let ts = crate::Timestamp::new_u64(next, track_timescale) + .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; + (size, ts) + } else { + let Some(size) = stream.decode_maybe::().await? else { + break; + }; + (size, crate::Timestamp::ZERO) + }; + + let mut frame = group.create_frame(Frame::new(size).with_timestamp(timestamp))?; if let Err(err) = self.run_frame(stream, &mut frame).await { let _ = frame.abort(err.clone()); diff --git a/rs/moq-net/src/lite/version.rs b/rs/moq-net/src/lite/version.rs index 07de44e68..e060a2c7b 100644 --- a/rs/moq-net/src/lite/version.rs +++ b/rs/moq-net/src/lite/version.rs @@ -7,6 +7,20 @@ pub enum Version { Lite02, Lite03, Lite04, + /// Lite05 adds per-track timescale to SUBSCRIBE_OK and zigzag-delta timestamps + /// to per-frame headers. + Lite05, +} + +impl Version { + /// Whether this version carries per-frame timestamps and per-track timescale + /// on the wire. + pub fn has_timestamps(self) -> bool { + match self { + Self::Lite01 | Self::Lite02 | Self::Lite03 | Self::Lite04 => false, + _ => true, + } + } } impl fmt::Display for Version { @@ -16,18 +30,14 @@ impl fmt::Display for Version { Self::Lite02 => write!(f, "moq-lite-02"), Self::Lite03 => write!(f, "moq-lite-03"), Self::Lite04 => write!(f, "moq-lite-04"), + Self::Lite05 => write!(f, "moq-lite-05"), } } } impl From for crate::Version { fn from(v: Version) -> Self { - match v { - Version::Lite01 => crate::Version::Lite(Version::Lite01), - Version::Lite02 => crate::Version::Lite(Version::Lite02), - Version::Lite03 => crate::Version::Lite(Version::Lite03), - Version::Lite04 => crate::Version::Lite(Version::Lite04), - } + crate::Version::Lite(v) } } diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 7cc7e9a40..299c71438 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -81,6 +81,10 @@ struct State { max_sequence: Option, final_sequence: Option, abort: Option, + /// Track timescale once negotiated via SUBSCRIBE_OK / track properties. + /// `None` means not yet resolved; consumers awaiting [`Self::poll_timescale`] + /// will block. + timescale: Option, } impl State { @@ -221,6 +225,16 @@ impl State { Poll::Pending } } + + fn poll_timescale(&self) -> Poll> { + if let Some(scale) = self.timescale { + Poll::Ready(Ok(scale)) + } else if let Some(err) = &self.abort { + Poll::Ready(Err(err.clone())) + } else { + Poll::Pending + } + } } /// A producer for a track, used to create new groups. @@ -240,10 +254,13 @@ impl std::ops::Deref for TrackProducer { impl TrackProducer { /// Build a producer for the given track metadata. Prefer [`Track::produce`]. pub fn new(info: Track) -> Self { - Self { - info, - state: conducer::Producer::default(), + let state: conducer::Producer = conducer::Producer::default(); + if info.timescale != 0 { + if let Ok(mut s) = state.write() { + s.timescale = Some(info.timescale); + } } + Self { info, state } } /// Create a new group with the given sequence number. @@ -301,6 +318,19 @@ impl TrackProducer { Ok(()) } + /// Update the track's negotiated timescale. + /// + /// Subscribers call this once SUBSCRIBE_OK arrives with the publisher's + /// timescale, since the [`TrackProducer`] was created before the value was + /// known. Existing [`TrackConsumer`]s see the updated timescale via + /// [`TrackConsumer::timescale`]. + pub fn set_timescale(&mut self, timescale: u64) { + self.info.timescale = timescale; + if let Ok(mut state) = self.state.write() { + state.timescale = Some(timescale); + } + } + /// Mark the track as finished after the last appended group. /// /// Sets the final sequence to one past the current max_sequence. @@ -622,6 +652,23 @@ impl TrackConsumer { self.state.read().max_sequence } + /// Return the resolved timescale (units per second) if one has been negotiated, + /// otherwise `None`. Use [`Self::timescale`] to wait until the value is known. + pub fn timescale_now(&self) -> Option { + self.state.read().timescale + } + + /// Poll for the negotiated timescale, without blocking. + pub fn poll_timescale(&self, waiter: &conducer::Waiter) -> Poll> { + self.poll(waiter, |state| state.poll_timescale()) + } + + /// Wait until the track's timescale has been negotiated (e.g. via SUBSCRIBE_OK + /// for subscribers, or set immediately by publishers). + pub async fn timescale(&self) -> Result { + conducer::wait(|waiter| self.poll_timescale(waiter)).await + } + /// Create a weak reference that doesn't prevent auto-close. pub(crate) fn weak(&self) -> TrackWeak { TrackWeak { diff --git a/rs/moq-net/src/setup.rs b/rs/moq-net/src/setup.rs index 6643b1694..a15fee567 100644 --- a/rs/moq-net/src/setup.rs +++ b/rs/moq-net/src/setup.rs @@ -76,7 +76,7 @@ impl SetupVersion { Version::Ietf(ietf::Version::Draft15) | Version::Ietf(ietf::Version::Draft16) => Self::Draft15Plus, Version::Ietf(ietf::Version::Draft17) | Version::Ietf(ietf::Version::Draft18) => Self::Modern, Version::Lite(lite::Version::Lite01) | Version::Lite(lite::Version::Lite02) => Self::LiteLegacy, - Version::Lite(lite::Version::Lite03 | lite::Version::Lite04) => Self::Unsupported, + Version::Lite(lite::Version::Lite03 | lite::Version::Lite04 | lite::Version::Lite05) => Self::Unsupported, } } } diff --git a/rs/moq-net/src/version.rs b/rs/moq-net/src/version.rs index 7c4872c1f..f245b125f 100644 --- a/rs/moq-net/src/version.rs +++ b/rs/moq-net/src/version.rs @@ -16,6 +16,7 @@ pub(crate) const NEGOTIATED: [Version; 3] = [ /// ALPN strings for supported versions. pub const ALPNS: &[&str] = &[ + ALPN_LITE_05, ALPN_LITE_04, ALPN_LITE_03, ALPN_LITE, @@ -30,6 +31,7 @@ pub const ALPNS: &[&str] = &[ pub(crate) const ALPN_LITE: &str = "moql"; pub(crate) const ALPN_LITE_03: &str = "moq-lite-03"; pub(crate) const ALPN_LITE_04: &str = "moq-lite-04"; +pub(crate) const ALPN_LITE_05: &str = "moq-lite-05"; pub(crate) const ALPN_14: &str = "moq-00"; pub(crate) const ALPN_15: &str = "moqt-15"; pub(crate) const ALPN_16: &str = "moqt-16"; @@ -52,6 +54,7 @@ impl Version { 0xff0dad02 => Some(Self::Lite(lite::Version::Lite02)), 0xff0dad03 => Some(Self::Lite(lite::Version::Lite03)), 0xff0dad04 => Some(Self::Lite(lite::Version::Lite04)), + 0xff0dad05 => Some(Self::Lite(lite::Version::Lite05)), 0xff00000e => Some(Self::Ietf(ietf::Version::Draft14)), 0xff00000f => Some(Self::Ietf(ietf::Version::Draft15)), 0xff000010 => Some(Self::Ietf(ietf::Version::Draft16)), @@ -68,6 +71,7 @@ impl Version { Self::Lite(lite::Version::Lite02) => 0xff0dad02, Self::Lite(lite::Version::Lite03) => 0xff0dad03, Self::Lite(lite::Version::Lite04) => 0xff0dad04, + Self::Lite(lite::Version::Lite05) => 0xff0dad05, Self::Ietf(ietf::Version::Draft14) => 0xff00000e, Self::Ietf(ietf::Version::Draft15) => 0xff00000f, Self::Ietf(ietf::Version::Draft16) => 0xff000010, @@ -85,6 +89,7 @@ impl Version { ALPN_LITE => None, // Multiple versions share this ALPN, need SETUP negotiation ALPN_LITE_03 => Some(Self::Lite(lite::Version::Lite03)), ALPN_LITE_04 => Some(Self::Lite(lite::Version::Lite04)), + ALPN_LITE_05 => Some(Self::Lite(lite::Version::Lite05)), ALPN_14 => Some(Self::Ietf(ietf::Version::Draft14)), ALPN_15 => Some(Self::Ietf(ietf::Version::Draft15)), ALPN_16 => Some(Self::Ietf(ietf::Version::Draft16)), @@ -97,6 +102,7 @@ impl Version { /// Returns the ALPN string for this version. pub fn alpn(&self) -> &'static str { match self { + Self::Lite(lite::Version::Lite05) => ALPN_LITE_05, Self::Lite(lite::Version::Lite04) => ALPN_LITE_04, Self::Lite(lite::Version::Lite03) => ALPN_LITE_03, Self::Lite(lite::Version::Lite01 | lite::Version::Lite02) => ALPN_LITE, @@ -152,6 +158,7 @@ impl FromStr for Version { "moq-lite-02" => Ok(Self::Lite(lite::Version::Lite02)), "moq-lite-03" => Ok(Self::Lite(lite::Version::Lite03)), "moq-lite-04" => Ok(Self::Lite(lite::Version::Lite04)), + "moq-lite-05" => Ok(Self::Lite(lite::Version::Lite05)), "moq-transport-14" => Ok(Self::Ietf(ietf::Version::Draft14)), "moq-transport-15" => Ok(Self::Ietf(ietf::Version::Draft15)), "moq-transport-16" => Ok(Self::Ietf(ietf::Version::Draft16)), @@ -218,6 +225,7 @@ impl Versions { /// All supported versions exposed by default. pub fn all() -> Self { Self(vec![ + Version::Lite(lite::Version::Lite05), Version::Lite(lite::Version::Lite04), Version::Lite(lite::Version::Lite03), Version::Lite(lite::Version::Lite02), From a75f8645c46add4b43aa1abae0d30f983316c0f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 22:35:23 +0000 Subject: [PATCH 04/10] moq-net: make subscribe_track async to await timescale BroadcastConsumer::subscribe_track is now async and resolves after the session layer publishes the negotiated timescale (via SUBSCRIBE_OK on moq-lite Lite05+, or 0 for older versions and for IETF until LOC track properties land). Subscribers waiting on the resolved timescale via TrackConsumer::timescale see it before the future completes. subscribe_track_immediate keeps the synchronous semantics for callers inside non-async contexts (FFI, internal moq-net publishers serving a local broadcast) where the timescale either isn't relevant or is already known. The IETF subscriber now also sets timescale = 0 explicitly when SUBSCRIBE_OK arrives, so the async subscribe future resolves on older drafts that don't yet carry LOC track properties. https://claude.ai/code/session_01Wy7egQjXvfPCKovs3mWANB --- rs/libmoq/src/consume.rs | 6 +++--- rs/moq-clock/src/main.rs | 2 +- rs/moq-ffi/src/consumer.rs | 6 +++--- rs/moq-mux/src/export/fmp4.rs | 4 ++-- rs/moq-native/tests/backend.rs | 2 ++ rs/moq-native/tests/broadcast.rs | 3 +++ rs/moq-net/src/ietf/publisher.rs | 2 +- rs/moq-net/src/ietf/subscriber.rs | 11 +++++++++-- rs/moq-net/src/lite/publisher.rs | 2 +- rs/moq-net/src/model/broadcast.rs | 27 +++++++++++++++++++++++---- rs/moq-relay/src/web.rs | 2 +- 11 files changed, 49 insertions(+), 18 deletions(-) diff --git a/rs/libmoq/src/consume.rs b/rs/libmoq/src/consume.rs index 5fde3fc42..e5f9b7682 100644 --- a/rs/libmoq/src/consume.rs +++ b/rs/libmoq/src/consume.rs @@ -47,7 +47,7 @@ impl Consume { pub fn catalog(&mut self, broadcast: Id, on_catalog: OnStatus) -> Result { let broadcast = self.broadcast.get(broadcast).ok_or(Error::BroadcastNotFound)?.clone(); - let catalog = broadcast.subscribe_track(&hang::catalog::Catalog::default_track())?; + let catalog = broadcast.subscribe_track_immediate(&hang::catalog::Catalog::default_track())?; let channel = oneshot::channel(); let entry = TaskEntry { @@ -216,7 +216,7 @@ impl Consume { .nth(index) .ok_or(Error::NoIndex)?; - let track = consume.broadcast.subscribe_track(&moq_net::Track { + let track = consume.broadcast.subscribe_track_immediate(&moq_net::Track { name: rendition.clone(), priority: 1, // TODO: Remove priority timescale: 0, @@ -261,7 +261,7 @@ impl Consume { .nth(index) .ok_or(Error::NoIndex)?; - let track = consume.broadcast.subscribe_track(&moq_net::Track { + let track = consume.broadcast.subscribe_track_immediate(&moq_net::Track { name: rendition.clone(), priority: 2, // TODO: Remove priority timescale: 0, diff --git a/rs/moq-clock/src/main.rs b/rs/moq-clock/src/main.rs index c846af09c..278af4a83 100644 --- a/rs/moq-clock/src/main.rs +++ b/rs/moq-clock/src/main.rs @@ -100,7 +100,7 @@ async fn main() -> anyhow::Result<()> { Some(announce) = origin.announced() => match announce { (path, Some(broadcast)) => { tracing::info!(broadcast = %path, "broadcast is online, subscribing to track"); - let track = broadcast.subscribe_track(&track)?; + let track = broadcast.subscribe_track(&track).await?; clock = Some(clock::Subscriber::new(track)); } (path, None) => { diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index 93f6d2eb1..bf46ea02c 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -78,7 +78,7 @@ impl MoqBroadcastConsumer { /// Subscribe to the catalog for this broadcast. pub fn subscribe_catalog(&self) -> Result, MoqError> { let _guard = crate::ffi::RUNTIME.enter(); - let track = self.inner.subscribe_track(&hang::catalog::Catalog::default_track())?; + let track = self.inner.subscribe_track_immediate(&hang::catalog::Catalog::default_track())?; let consumer = moq_mux::catalog::Consumer::from(track); Ok(Arc::new(MoqCatalogConsumer { task: Task::new(Catalog { inner: consumer }), @@ -90,7 +90,7 @@ impl MoqBroadcastConsumer { /// Frames are returned as plain byte payloads with no codec or container parsing. pub fn subscribe_track(&self, name: String) -> Result, MoqError> { let _guard = crate::ffi::RUNTIME.enter(); - let track = self.inner.subscribe_track(&moq_net::Track { + let track = self.inner.subscribe_track_immediate(&moq_net::Track { name, priority: 0, timescale: 0, @@ -115,7 +115,7 @@ impl MoqBroadcastConsumer { let media: moq_mux::container::Hang = (&container) .try_into() .map_err(|e| MoqError::Codec(format!("invalid container: {e}")))?; - let track = self.inner.subscribe_track(&moq_net::Track { + let track = self.inner.subscribe_track_immediate(&moq_net::Track { name, priority: 0, timescale: 0, diff --git a/rs/moq-mux/src/export/fmp4.rs b/rs/moq-mux/src/export/fmp4.rs index afe4bf44d..80dda0314 100644 --- a/rs/moq-mux/src/export/fmp4.rs +++ b/rs/moq-mux/src/export/fmp4.rs @@ -56,7 +56,7 @@ impl Fmp4 { /// The hang catalog is subscribed internally; per-rendition tracks are (un)subscribed /// as the catalog changes. pub fn new(broadcast: moq_net::BroadcastConsumer) -> Result { - let catalog_track = broadcast.subscribe_track(&hang::Catalog::default_track())?; + let catalog_track = broadcast.subscribe_track_immediate(&hang::Catalog::default_track())?; let catalog = crate::catalog::Consumer::new(catalog_track); Ok(Self { @@ -175,7 +175,7 @@ impl Fmp4 { } let media: Hang = (*container).try_into()?; - let track = self.broadcast.subscribe_track(&moq_net::Track::new(name.clone()))?; + let track = self.broadcast.subscribe_track_immediate(&moq_net::Track::new(name.clone()))?; let consumer = Consumer::new(track, media).with_latency(self.latency); let timescale = catalog_timescale(catalog, name).context("track not in catalog")?; diff --git a/rs/moq-native/tests/backend.rs b/rs/moq-native/tests/backend.rs index 271f592e2..4ceec8fa4 100644 --- a/rs/moq-native/tests/backend.rs +++ b/rs/moq-native/tests/backend.rs @@ -71,6 +71,7 @@ async fn backend_test(scheme: &str, backend: moq_native::QuicBackend) { let mut track_sub = bc .subscribe_track(&Track::new("video")) + .await .expect("subscribe_track failed"); let mut group_sub = tokio::time::timeout(TIMEOUT, track_sub.recv_group()) @@ -223,6 +224,7 @@ async fn iroh_connect() { let mut track_sub = bc .subscribe_track(&Track::new("video")) + .await .expect("subscribe_track failed"); let mut group_sub = tokio::time::timeout(TIMEOUT, track_sub.recv_group()) diff --git a/rs/moq-native/tests/broadcast.rs b/rs/moq-native/tests/broadcast.rs index a0f20da5c..87848cb48 100644 --- a/rs/moq-native/tests/broadcast.rs +++ b/rs/moq-native/tests/broadcast.rs @@ -89,6 +89,7 @@ async fn broadcast_test(scheme: &str, client_version: Option<&str>, server_versi // Subscribe to the track. let mut track_sub = bc .subscribe_track(&Track::new("video")) + .await .expect("subscribe_track failed"); // Read one group. @@ -501,6 +502,7 @@ async fn broadcast_websocket() { // Subscribe to the track. let mut track_sub = bc .subscribe_track(&Track::new("video")) + .await .expect("subscribe_track failed"); // Read one group. @@ -608,6 +610,7 @@ async fn broadcast_websocket_fallback() { // Subscribe to the track. let mut track_sub = bc .subscribe_track(&Track::new("video")) + .await .expect("subscribe_track failed"); let mut group_sub = tokio::time::timeout(TIMEOUT, track_sub.recv_group()) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 69f164ee8..8d1103dfb 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -119,7 +119,7 @@ impl Publisher { timescale: 0, }; - let track = match broadcast.subscribe_track(&track) { + let track = match broadcast.subscribe_track_immediate(&track) { Ok(track) => track, Err(err) => { self.write_subscribe_error(&mut stream.writer, request_id, 404, &err.to_string()) diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 605518d3a..3382af4c8 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -571,12 +571,19 @@ impl Subscriber { // Read the response and register the alias mapping let track_alias = match self.read_subscribe_response(&mut stream).await { Ok(alias) => { + let mut state = self.state.lock(); if let Some(alias) = alias { - let mut state = self.state.lock(); state.aliases.insert(alias, request_id); - if let Some(track_state) = state.subscribes.get_mut(&request_id) { + } + if let Some(track_state) = state.subscribes.get_mut(&request_id) { + if let Some(alias) = alias { track_state.alias = Some(alias); } + // LOC-style track properties are not parsed yet; default the + // timescale to 0 (unspecified) so consumers awaiting it + // don't hang. TODO: read timescale from track properties on + // Draft17+. + track_state.producer.set_timescale(0); } alias } diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 999b41802..1fd650be1 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -312,7 +312,7 @@ impl Publisher { }; let broadcast = consumer.ok_or(Error::NotFound)?; - let track = broadcast.subscribe_track(&track)?; + let track = broadcast.subscribe_track_immediate(&track)?; // TODO wait until track.info() to get the *real* priority diff --git a/rs/moq-net/src/model/broadcast.rs b/rs/moq-net/src/model/broadcast.rs index 8c394096b..900d85a1f 100644 --- a/rs/moq-net/src/model/broadcast.rs +++ b/rs/moq-net/src/model/broadcast.rs @@ -359,7 +359,26 @@ impl BroadcastConsumer { /// queues a new dynamic request that the broadcast's producer will service via /// [`BroadcastDynamic::requested_track`]. Returns [`Error::NotFound`] if the /// broadcast has no dynamic producer to handle requests. - pub fn subscribe_track(&self, track: &Track) -> Result { + /// + /// Awaits the negotiated timescale (delivered via SUBSCRIBE_OK on the session + /// layer) before returning, so the resulting [`TrackConsumer`] exposes a + /// resolved [`TrackConsumer::timescale_now`]. For protocol versions that + /// don't negotiate a timescale, the session layer publishes `0` immediately + /// and this future resolves without delay. + pub async fn subscribe_track(&self, track: &Track) -> Result { + let consumer = self.subscribe_track_immediate(track)?; + // Wait for the session layer to publish the negotiated timescale. + // Ignore the result: if the track is already closed, the consumer + // itself will surface that on the next operation. + let _ = consumer.timescale().await; + Ok(consumer) + } + + /// Same as [`Self::subscribe_track`] but does not wait for the timescale to be + /// resolved. Callers that don't need the timescale (or want to handle resolution + /// themselves via [`TrackConsumer::timescale`]) can use this to avoid an extra + /// roundtrip. + pub fn subscribe_track_immediate(&self, track: &Track) -> Result { // Upgrade to a temporary producer so we can modify the state. let producer = self .state @@ -443,7 +462,7 @@ impl BroadcastConsumer { #[cfg(test)] impl BroadcastConsumer { pub fn assert_subscribe_track(&self, track: &Track) -> TrackConsumer { - self.subscribe_track(track).expect("should not have errored") + self.subscribe_track_immediate(track).expect("should not have errored") } pub fn assert_not_closed(&self) { @@ -544,7 +563,7 @@ mod test { // Make sure the track is errored, not closed. track4.assert_error(); - let track5 = consumer2.subscribe_track(&Track::new("track3")); + let track5 = consumer2.subscribe_track_immediate(&Track::new("track3")); assert!(track5.is_err(), "should have errored"); } @@ -622,7 +641,7 @@ mod test { tokio::time::sleep(std::time::Duration::from_millis(1)).await; // Now the cleanup task should have run and we can subscribe again to the unknown track. - let consumer3 = broadcast.consume().subscribe_track(&Track::new("unknown_track")); + let consumer3 = broadcast.consume().subscribe_track_immediate(&Track::new("unknown_track")); let producer2 = broadcast.assert_request(); // Drop the consumer, now the producer should be unused diff --git a/rs/moq-relay/src/web.rs b/rs/moq-relay/src/web.rs index 76507d571..3b39b612e 100644 --- a/rs/moq-relay/src/web.rs +++ b/rs/moq-relay/src/web.rs @@ -519,7 +519,7 @@ async fn serve_fetch( // Block until the broadcast has been announced (within the fetch deadline) so // freshly-connected subscribers don't get a spurious 404 before gossip arrives. let broadcast = origin.announced_broadcast("").await.ok_or(StatusCode::NOT_FOUND)?; - let mut track = broadcast.subscribe_track(&track).map_err(|err| match err { + let mut track = broadcast.subscribe_track(&track).await.map_err(|err| match err { moq_net::Error::NotFound => StatusCode::NOT_FOUND, _ => StatusCode::INTERNAL_SERVER_ERROR, })?; From 3a527fd2d0bf7ff7b195755412b70f5e198eda63 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 22:38:13 +0000 Subject: [PATCH 05/10] hang: subscribe example uses async subscribe_track Follow-up to making BroadcastConsumer::subscribe_track async. https://claude.ai/code/session_01Wy7egQjXvfPCKovs3mWANB --- rs/hang/examples/subscribe.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/hang/examples/subscribe.rs b/rs/hang/examples/subscribe.rs index f5cc77268..cd617631a 100644 --- a/rs/hang/examples/subscribe.rs +++ b/rs/hang/examples/subscribe.rs @@ -50,7 +50,7 @@ async fn run_subscribe(mut consumer: moq_net::OriginConsumer) -> anyhow::Result< tracing::info!(%path, "broadcast announced"); // Read the catalog to discover available tracks. - let catalog_track = broadcast.subscribe_track(&hang::Catalog::default_track())?; + let catalog_track = broadcast.subscribe_track(&hang::Catalog::default_track()).await?; let mut catalog = moq_mux::catalog::Consumer::new(catalog_track); let info = catalog.next().await?.ok_or_else(|| anyhow::anyhow!("no catalog"))?; @@ -78,7 +78,7 @@ async fn run_subscribe(mut consumer: moq_net::OriginConsumer) -> anyhow::Result< timescale: 0, }; - let track_consumer = broadcast.subscribe_track(&track)?; + let track_consumer = broadcast.subscribe_track(&track).await?; let mut ordered = moq_mux::container::Consumer::new(track_consumer, moq_mux::container::Hang::Legacy) .with_latency(Duration::from_millis(500)); From 1e3bb3f6c25f745dd1f96cfccca6f45272f1401e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 22 May 2026 15:59:28 -0700 Subject: [PATCH 06/10] moq-net: async subscribe, immutable tracks, Subscription split, Timescale type Addresses PR #1439 review: - Add `Timescale` newtype with named constants (UNKNOWN/SECOND/MILLI/MICRO/NANO); replace raw `u64` timescale fields and method args throughout - `BroadcastConsumer::subscribe_track(name, Subscription)` is now `async` and resolves on SUBSCRIBE_OK with the publisher's authoritative Track properties (priority, timescale). `subscribe_track_immediate` is gone - Add `Subscription` for subscriber-side preferences (priority, timeout), separated from the publisher's immutable Track. Add aggregation methods `TrackProducer::max_priority` / `max_timeout` and an updatable `TrackConsumer::update_subscription`. This is the API surface the future fetch path will plug into - Make Track properties immutable on `TrackProducer`: remove `set_timescale`, fold `unique_track` into `unique_name` so callers construct the full Track themselves - Introduce `TrackRequest` so the dynamic broadcast flow yields a request the publisher fulfills via `accept(Track)`, completing the async resolution - Remove `Frame::new` and `with_timestamp`; require struct construction; keep size-only `From` impls - Hang container frame now uses `VarInt::encode_quic`/`decode_quic` + the `Timestamp::value()` accessor for explicit timestamp varint coding - Replace `Timestamp::from_scale` callsites with `Timestamp::new(...).convert(...)` Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/hang/examples/subscribe.rs | 24 +- rs/hang/examples/video.rs | 2 +- rs/hang/src/catalog/root.rs | 2 +- rs/hang/src/container/frame.rs | 20 +- rs/libmoq/src/consume.rs | 72 +++-- rs/moq-boy/src/input.rs | 9 +- rs/moq-boy/src/status.rs | 1 + rs/moq-cli/src/subscribe.rs | 4 +- rs/moq-clock/src/main.rs | 6 +- rs/moq-ffi/src/consumer.rs | 39 ++- rs/moq-ffi/src/producer.rs | 2 +- rs/moq-ffi/src/test.rs | 17 +- rs/moq-mux/src/container/cmaf.rs | 8 +- rs/moq-mux/src/container/mod.rs | 4 +- rs/moq-mux/src/export/fmp4.rs | 104 ++++-- rs/moq-mux/src/import/aac.rs | 8 +- rs/moq-mux/src/import/av01.rs | 24 +- rs/moq-mux/src/import/avc1.rs | 8 +- rs/moq-mux/src/import/avc3.rs | 10 +- rs/moq-mux/src/import/fmp4.rs | 15 +- rs/moq-mux/src/import/hev1.rs | 8 +- rs/moq-mux/src/import/jitter.rs | 2 +- rs/moq-mux/src/import/opus.rs | 8 +- rs/moq-native/examples/chat.rs | 1 + rs/moq-native/tests/backend.rs | 4 +- rs/moq-native/tests/broadcast.rs | 6 +- rs/moq-net/src/coding/varint.rs | 4 +- rs/moq-net/src/ietf/publisher.rs | 19 +- rs/moq-net/src/ietf/subscriber.rs | 129 +++++--- rs/moq-net/src/lib.rs | 2 +- rs/moq-net/src/lite/publisher.rs | 32 +- rs/moq-net/src/lite/subscribe.rs | 2 +- rs/moq-net/src/lite/subscriber.rs | 145 +++++---- rs/moq-net/src/lite/version.rs | 2 + rs/moq-net/src/model/broadcast.rs | 516 ++++++++++++++++++------------ rs/moq-net/src/model/frame.rs | 60 ++-- rs/moq-net/src/model/group.rs | 9 +- rs/moq-net/src/model/time.rs | 293 ++++++++++------- rs/moq-net/src/model/track.rs | 257 ++++++++++----- rs/moq-relay/src/web.rs | 17 +- 40 files changed, 1190 insertions(+), 705 deletions(-) diff --git a/rs/hang/examples/subscribe.rs b/rs/hang/examples/subscribe.rs index cd617631a..c3c74b6f1 100644 --- a/rs/hang/examples/subscribe.rs +++ b/rs/hang/examples/subscribe.rs @@ -50,7 +50,9 @@ async fn run_subscribe(mut consumer: moq_net::OriginConsumer) -> anyhow::Result< tracing::info!(%path, "broadcast announced"); // Read the catalog to discover available tracks. - let catalog_track = broadcast.subscribe_track(&hang::Catalog::default_track()).await?; + let catalog_track = broadcast + .subscribe_track(hang::Catalog::DEFAULT_NAME, moq_net::Subscription::default()) + .await?; let mut catalog = moq_mux::catalog::Consumer::new(catalog_track); let info = catalog.next().await?.ok_or_else(|| anyhow::anyhow!("no catalog"))?; @@ -71,14 +73,18 @@ async fn run_subscribe(mut consumer: moq_net::OriginConsumer) -> anyhow::Result< "subscribing to video track" ); - // Subscribe to the video track. - let track = moq_net::Track { - name: name.clone(), - priority: 1, - timescale: 0, - }; - - let track_consumer = broadcast.subscribe_track(&track).await?; + // Subscribe to the video track. Priority is a subscriber preference; the + // publisher's authoritative Track properties (including timescale) arrive + // on the returned TrackConsumer. + let track_consumer = broadcast + .subscribe_track( + name, + moq_net::Subscription { + priority: 1, + timeout: Duration::ZERO, + }, + ) + .await?; let mut ordered = moq_mux::container::Consumer::new(track_consumer, moq_mux::container::Hang::Legacy) .with_latency(Duration::from_millis(500)); diff --git a/rs/hang/examples/video.rs b/rs/hang/examples/video.rs index 009c25ee2..b266b956a 100644 --- a/rs/hang/examples/video.rs +++ b/rs/hang/examples/video.rs @@ -44,7 +44,7 @@ fn create_track(broadcast: &mut moq_net::BroadcastProducer) -> anyhow::Result Result<(), Error> { let mut header = BytesMut::new(); - self.timestamp.encode_value(&mut header).map_err(moq_net::Error::from)?; + let value = VarInt::try_from(self.timestamp.value()).map_err(moq_net::Error::from)?; + value.encode_quic(&mut header).map_err(moq_net::Error::from)?; - let size = header.len() + self.payload.len(); + let size = (header.len() + self.payload.len()) as u64; - let net_frame = moq_net::Frame::new(size as u64).with_timestamp(self.timestamp); + let net_frame = moq_net::Frame { + size, + timestamp: self.timestamp, + }; let mut chunked = group.create_frame(net_frame)?; chunked.write(header.freeze())?; chunked.write(self.payload.clone())?; @@ -53,7 +62,8 @@ impl Frame { /// Decode a frame from raw bytes (VarInt timestamp prefix + payload). pub fn decode(mut buf: impl Buf) -> Result { - let timestamp = Timestamp::decode_value(&mut buf, TIMESCALE)?; + let value: u64 = VarInt::decode_quic(&mut buf).map_err(moq_net::Error::from)?.into(); + let timestamp = Timestamp::from_micros(value)?; let payload = buf.copy_to_bytes(buf.remaining()); Ok(Self { timestamp, payload }) diff --git a/rs/libmoq/src/consume.rs b/rs/libmoq/src/consume.rs index e5f9b7682..c7e72ec1a 100644 --- a/rs/libmoq/src/consume.rs +++ b/rs/libmoq/src/consume.rs @@ -47,7 +47,6 @@ impl Consume { pub fn catalog(&mut self, broadcast: Id, on_catalog: OnStatus) -> Result { let broadcast = self.broadcast.get(broadcast).ok_or(Error::BroadcastNotFound)?.clone(); - let catalog = broadcast.subscribe_track_immediate(&hang::catalog::Catalog::default_track())?; let channel = oneshot::channel(); let entry = TaskEntry { @@ -58,7 +57,7 @@ impl Consume { tokio::spawn(async move { let res = tokio::select! { - res = Self::run_catalog(id, broadcast, catalog.into()) => res, + res = Self::run_catalog_task(id, broadcast) => res, _ = channel.1 => Ok(()), }; @@ -71,6 +70,13 @@ impl Consume { Ok(id) } + async fn run_catalog_task(task_id: Id, broadcast: moq_net::BroadcastConsumer) -> Result<(), Error> { + let catalog = broadcast + .subscribe_track(hang::catalog::Catalog::DEFAULT_NAME, moq_net::Subscription::default()) + .await?; + Self::run_catalog(task_id, broadcast, catalog.into()).await + } + async fn run_catalog( task_id: Id, broadcast: moq_net::BroadcastConsumer, @@ -216,12 +222,8 @@ impl Consume { .nth(index) .ok_or(Error::NoIndex)?; - let track = consume.broadcast.subscribe_track_immediate(&moq_net::Track { - name: rendition.clone(), - priority: 1, // TODO: Remove priority - timescale: 0, - })?; - let track = moq_mux::container::Consumer::new(track, moq_mux::container::Hang::Legacy).with_latency(latency); + let broadcast = consume.broadcast.clone(); + let name = rendition.clone(); let channel = oneshot::channel(); let entry = TaskEntry { @@ -231,6 +233,26 @@ impl Consume { let id = self.track_task.insert(Some(entry))?; tokio::spawn(async move { + let track = match broadcast + .subscribe_track( + &name, + moq_net::Subscription { + priority: 1, + timeout: std::time::Duration::ZERO, + }, + ) + .await + { + Ok(track) => track, + Err(err) => { + if let Some(entry) = State::lock().consume.track_task.remove(id).flatten() { + entry.callback.call(Result::::Err(err.into())); + } + return; + } + }; + let track = + moq_mux::container::Consumer::new(track, moq_mux::container::Hang::Legacy).with_latency(latency); let res = tokio::select! { res = Self::run_track(id, track) => res, _ = channel.1 => Ok(()), @@ -261,12 +283,8 @@ impl Consume { .nth(index) .ok_or(Error::NoIndex)?; - let track = consume.broadcast.subscribe_track_immediate(&moq_net::Track { - name: rendition.clone(), - priority: 2, // TODO: Remove priority - timescale: 0, - })?; - let track = moq_mux::container::Consumer::new(track, moq_mux::container::Hang::Legacy).with_latency(latency); + let broadcast = consume.broadcast.clone(); + let name = rendition.clone(); let channel = oneshot::channel(); let entry = TaskEntry { @@ -276,6 +294,26 @@ impl Consume { let id = self.track_task.insert(Some(entry))?; tokio::spawn(async move { + let track = match broadcast + .subscribe_track( + &name, + moq_net::Subscription { + priority: 2, + timeout: std::time::Duration::ZERO, + }, + ) + .await + { + Ok(track) => track, + Err(err) => { + if let Some(entry) = State::lock().consume.track_task.remove(id).flatten() { + entry.callback.call(Result::::Err(err.into())); + } + return; + } + }; + let track = + moq_mux::container::Consumer::new(track, moq_mux::container::Hang::Legacy).with_latency(latency); let res = tokio::select! { res = Self::run_track(id, track) => res, _ = channel.1 => Ok(()), @@ -329,11 +367,7 @@ impl Consume { pub fn frame(&self, frame: Id, dst: &mut moq_frame) -> Result<(), Error> { let f = self.frame.get(frame).ok_or(Error::FrameNotFound)?; - let timestamp_us: u64 = f - .timestamp - .as_micros()? - .try_into() - .map_err(|_| moq_net::TimeOverflow)?; + let timestamp_us: u64 = f.timestamp.as_micros()?.try_into().map_err(|_| moq_net::TimeOverflow)?; *dst = moq_frame { payload: f.payload.as_ptr(), diff --git a/rs/moq-boy/src/input.rs b/rs/moq-boy/src/input.rs index ddaa0561f..2e0fcf4ee 100644 --- a/rs/moq-boy/src/input.rs +++ b/rs/moq-boy/src/input.rs @@ -99,12 +99,9 @@ async fn handle_viewer_commands( broadcast: moq_net::BroadcastConsumer, cmd_tx: &tokio::sync::mpsc::Sender, ) -> anyhow::Result<()> { - let command_track = moq_net::Track { - name: "command".to_string(), - priority: 0, - }; - - let mut track = broadcast.subscribe_track(&command_track)?; + let mut track = broadcast + .subscribe_track("command", moq_net::Subscription::default()) + .await?; while let Some(mut group) = track.recv_group().await? { while let Some(frame) = group.read_frame().await? { diff --git a/rs/moq-boy/src/status.rs b/rs/moq-boy/src/status.rs index f9f66c079..15259281c 100644 --- a/rs/moq-boy/src/status.rs +++ b/rs/moq-boy/src/status.rs @@ -43,6 +43,7 @@ impl StatusPublisher { let track = moq_net::Track { name: "status".to_string(), priority: 10, + timescale: moq_net::Timescale::UNKNOWN, }; let producer = broadcast.create_track(track)?; diff --git a/rs/moq-cli/src/subscribe.rs b/rs/moq-cli/src/subscribe.rs index b25032919..f3cbd1545 100644 --- a/rs/moq-cli/src/subscribe.rs +++ b/rs/moq-cli/src/subscribe.rs @@ -42,7 +42,9 @@ impl Subscribe { // Fmp4 subscribes to the catalog internally, builds the merged init segment // from the first catalog snapshot, then yields moof+mdat fragments in // timestamp order across tracks. - let mut fmp4 = moq_mux::export::Fmp4::new(self.broadcast)?.with_latency(self.args.max_latency); + let mut fmp4 = moq_mux::export::Fmp4::new(self.broadcast) + .await? + .with_latency(self.args.max_latency); while let Some(chunk) = fmp4.next().await? { stdout.write_all(&chunk).await?; diff --git a/rs/moq-clock/src/main.rs b/rs/moq-clock/src/main.rs index 278af4a83..3dbf12f4a 100644 --- a/rs/moq-clock/src/main.rs +++ b/rs/moq-clock/src/main.rs @@ -57,7 +57,7 @@ async fn main() -> anyhow::Result<()> { let track = Track { name: config.track, priority: 0, - timescale: 0, + timescale: moq_net::Timescale::UNKNOWN, }; let origin = moq_net::Origin::random().produce(); @@ -100,7 +100,9 @@ async fn main() -> anyhow::Result<()> { Some(announce) = origin.announced() => match announce { (path, Some(broadcast)) => { tracing::info!(broadcast = %path, "broadcast is online, subscribing to track"); - let track = broadcast.subscribe_track(&track).await?; + let track = broadcast + .subscribe_track(&track.name, moq_net::Subscription::default()) + .await?; clock = Some(clock::Subscriber::new(track)); } (path, None) => { diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index bf46ea02c..7dfede50e 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -76,9 +76,16 @@ impl Media { #[uniffi::export] impl MoqBroadcastConsumer { /// Subscribe to the catalog for this broadcast. - pub fn subscribe_catalog(&self) -> Result, MoqError> { - let _guard = crate::ffi::RUNTIME.enter(); - let track = self.inner.subscribe_track_immediate(&hang::catalog::Catalog::default_track())?; + pub async fn subscribe_catalog(&self) -> Result, MoqError> { + let broadcast = self.inner.clone(); + let track = crate::ffi::RUNTIME + .spawn(async move { + broadcast + .subscribe_track(hang::catalog::Catalog::DEFAULT_NAME, moq_net::Subscription::default()) + .await + }) + .await + .map_err(|err| MoqError::Codec(format!("subscribe task panicked: {err}")))??; let consumer = moq_mux::catalog::Consumer::from(track); Ok(Arc::new(MoqCatalogConsumer { task: Task::new(Catalog { inner: consumer }), @@ -88,13 +95,12 @@ impl MoqBroadcastConsumer { /// Subscribe to a track by name — same pattern as moq-boy's command/status tracks. /// /// Frames are returned as plain byte payloads with no codec or container parsing. - pub fn subscribe_track(&self, name: String) -> Result, MoqError> { - let _guard = crate::ffi::RUNTIME.enter(); - let track = self.inner.subscribe_track_immediate(&moq_net::Track { - name, - priority: 0, - timescale: 0, - })?; + pub async fn subscribe_track(&self, name: String) -> Result, MoqError> { + let broadcast = self.inner.clone(); + let track = crate::ffi::RUNTIME + .spawn(async move { broadcast.subscribe_track(&name, moq_net::Subscription::default()).await }) + .await + .map_err(|err| MoqError::Codec(format!("subscribe task panicked: {err}")))??; Ok(Arc::new(MoqTrackConsumer::new(track))) } @@ -102,24 +108,23 @@ impl MoqBroadcastConsumer { /// /// `container` is the track container from the catalog. /// `max_latency_ms` controls the maximum buffering before skipping a GoP. - pub fn subscribe_media( + pub async fn subscribe_media( &self, name: String, container: Container, max_latency_ms: u64, ) -> Result, MoqError> { - let _guard = crate::ffi::RUNTIME.enter(); // Parse the container before subscribing so we don't leave a dangling // subscription if init parsing fails. let container: hang::catalog::Container = container.into(); let media: moq_mux::container::Hang = (&container) .try_into() .map_err(|e| MoqError::Codec(format!("invalid container: {e}")))?; - let track = self.inner.subscribe_track_immediate(&moq_net::Track { - name, - priority: 0, - timescale: 0, - })?; + let broadcast = self.inner.clone(); + let track = crate::ffi::RUNTIME + .spawn(async move { broadcast.subscribe_track(&name, moq_net::Subscription::default()).await }) + .await + .map_err(|err| MoqError::Codec(format!("subscribe task panicked: {err}")))??; let latency = std::time::Duration::from_millis(max_latency_ms); let consumer = moq_mux::container::Consumer::new(track, media).with_latency(latency); Ok(Arc::new(MoqMediaConsumer { diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index 19a82579c..76a720ee7 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -96,7 +96,7 @@ impl MoqBroadcastProducer { let track = moq_net::Track { name, priority: 0, - timescale: 0, + timescale: moq_net::Timescale::UNKNOWN, }; // Clone the broadcast handle (shared Arc internally) to get &mut access. let mut broadcast = state.broadcast.clone(); diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index dea8bfb34..ad79d06de 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -78,7 +78,7 @@ async fn media_track_activity_and_name() { assert_eq!(track_name, "0.opus"); let broadcast_consumer = broadcast.consume().unwrap(); - let catalog_consumer = broadcast_consumer.subscribe_catalog().unwrap(); + let catalog_consumer = broadcast_consumer.subscribe_catalog().await.unwrap(); let catalog = tokio::time::timeout(TIMEOUT, catalog_consumer.next()) .await .expect("timed out waiting for catalog") @@ -86,7 +86,7 @@ async fn media_track_activity_and_name() { .expect("expected a catalog"); assert!(catalog.audio.contains_key(&track_name)); - let track_consumer = broadcast_consumer.subscribe_track(track_name).unwrap(); + let track_consumer = broadcast_consumer.subscribe_track(track_name).await.unwrap(); tokio::time::timeout(TIMEOUT, media.used()) .await .expect("timed out waiting for media track to become used") @@ -132,7 +132,7 @@ async fn local_publish_consume_audio() { assert_eq!(announcement.path(), "live"); let broadcast_consumer = announcement.broadcast(); - let catalog_consumer = broadcast_consumer.subscribe_catalog().unwrap(); + let catalog_consumer = broadcast_consumer.subscribe_catalog().await.unwrap(); let catalog = tokio::time::timeout(TIMEOUT, catalog_consumer.next()) .await @@ -149,6 +149,7 @@ async fn local_publish_consume_audio() { let media_consumer = broadcast_consumer .subscribe_media(track_name.clone(), audio.container.clone(), 10_000) + .await .unwrap(); let payload = b"opus audio payload data".to_vec(); @@ -182,7 +183,7 @@ async fn video_publish_consume() { .expect("expected announcement"); let broadcast_consumer = announcement.broadcast(); - let catalog_consumer = broadcast_consumer.subscribe_catalog().unwrap(); + let catalog_consumer = broadcast_consumer.subscribe_catalog().await.unwrap(); let catalog = tokio::time::timeout(TIMEOUT, catalog_consumer.next()) .await @@ -204,6 +205,7 @@ async fn video_publish_consume() { let media_consumer = broadcast_consumer .subscribe_media(track_name.clone(), video.container.clone(), 10_000) + .await .unwrap(); let keyframe = vec![0x00, 0x00, 0x00, 0x01, 0x65, 0xAA, 0xBB, 0xCC]; @@ -236,7 +238,7 @@ async fn multiple_frames_ordering() { .unwrap(); let broadcast_consumer = announcement.broadcast(); - let catalog_consumer = broadcast_consumer.subscribe_catalog().unwrap(); + let catalog_consumer = broadcast_consumer.subscribe_catalog().await.unwrap(); let catalog = tokio::time::timeout(TIMEOUT, catalog_consumer.next()) .await .unwrap() @@ -246,6 +248,7 @@ async fn multiple_frames_ordering() { let (track_name, audio) = catalog.audio.iter().next().unwrap(); let media_consumer = broadcast_consumer .subscribe_media(track_name.clone(), audio.container.clone(), 10_000) + .await .unwrap(); let timestamps: [u64; 5] = [0, 20_000, 40_000, 60_000, 80_000]; @@ -284,7 +287,7 @@ async fn catalog_update_on_new_track() { .unwrap(); let broadcast_consumer = announcement.broadcast(); - let catalog_consumer = broadcast_consumer.subscribe_catalog().unwrap(); + let catalog_consumer = broadcast_consumer.subscribe_catalog().await.unwrap(); let catalog1 = tokio::time::timeout(TIMEOUT, catalog_consumer.next()) .await @@ -333,7 +336,7 @@ async fn announced_broadcast() { .expect("expected announcement"); assert_eq!(announcement.path(), "test/broadcast"); - let _catalog = announcement.broadcast().subscribe_catalog().unwrap(); + let _catalog = announcement.broadcast().subscribe_catalog().await.unwrap(); } #[test] diff --git a/rs/moq-mux/src/container/cmaf.rs b/rs/moq-mux/src/container/cmaf.rs index 38a1b3ea6..6df5d0aeb 100644 --- a/rs/moq-mux/src/container/cmaf.rs +++ b/rs/moq-mux/src/container/cmaf.rs @@ -144,7 +144,8 @@ pub(crate) fn decode(data: Bytes, timescale: u64) -> Result, Error> { let cts = entry.cts.unwrap_or_default() as i64; let pts = dts.checked_add_signed(cts).ok_or(Error::PtsOverflow)?; - let timestamp = Timestamp::from_scale(pts, timescale, crate::container::TIMESCALE)?; + let timestamp = + Timestamp::new(pts, moq_net::Timescale::new(timescale))?.convert(crate::container::TIMESCALE)?; let payload = Bytes::copy_from_slice(&mdat_data[offset..end]); let flags = entry.flags.unwrap_or(0); // depends_on_no_other (bits 24-25 == 0x2) means keyframe @@ -226,7 +227,10 @@ pub(crate) fn encode( // Stamp the wire-level timestamp on the moq-net frame so Lite05+ can // delta-encode it. The CMAF fragment may pack multiple media samples; use // the first sample's PTS as the representative timestamp. - let net_frame = moq_net::Frame::new(buf.len() as u64).with_timestamp(frames[0].timestamp); + let net_frame = moq_net::Frame { + size: buf.len() as u64, + timestamp: frames[0].timestamp, + }; let mut writer = group.create_frame(net_frame)?; writer.write(Bytes::from(buf))?; writer.finish()?; diff --git a/rs/moq-mux/src/container/mod.rs b/rs/moq-mux/src/container/mod.rs index e22f9056e..016340df1 100644 --- a/rs/moq-mux/src/container/mod.rs +++ b/rs/moq-mux/src/container/mod.rs @@ -26,10 +26,10 @@ pub use consumer::Consumer; pub use hang::Hang; pub use producer::Producer; -pub use moq_net::Timestamp; +pub use moq_net::{Timescale, Timestamp}; /// Microsecond presentation timestamp, the canonical timebase for media frames in moq-mux. -pub const TIMESCALE: u64 = 1_000_000; +pub const TIMESCALE: Timescale = Timescale::MICRO; /// A decoded media frame: timestamp, payload bytes, keyframe flag. /// diff --git a/rs/moq-mux/src/export/fmp4.rs b/rs/moq-mux/src/export/fmp4.rs index 80dda0314..b58107d17 100644 --- a/rs/moq-mux/src/export/fmp4.rs +++ b/rs/moq-mux/src/export/fmp4.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::{Arc, Mutex}; use std::task::Poll; use std::time::Duration; @@ -9,6 +10,9 @@ use mp4_atom::{DecodeMaybe, Encode}; use crate::container::{Consumer, Frame, Hang}; +/// Holds resolved subscriptions from spawned tasks, drained by `poll_next`. +type SubscribeQueue = Arc)>>>; + /// Subscribe to a moq broadcast and produce a single fMP4 / CMAF byte stream. /// /// Built from a [`moq_net::BroadcastConsumer`], `Fmp4` subscribes to the hang catalog, @@ -27,6 +31,15 @@ pub struct Fmp4 { tracks: HashMap, + /// Per-name metadata captured from the catalog snapshot at subscribe time, used + /// once the subscription resolves to construct the [`Fmp4Track`]. While entries + /// are present, a tokio task is awaiting SUBSCRIBE_OK and will push into + /// [`Self::subscribe_queue`]. + pending_meta: HashMap, + + /// Resolved subscriptions written by spawned tasks; drained on each [`poll_next`]. + subscribe_queue: SubscribeQueue, + /// Queued init segment, emitted on the first [`next`](Self::next) call after the /// initial catalog snapshot has been processed. init_pending: Option, @@ -36,6 +49,12 @@ pub struct Fmp4 { init_emitted: bool, } +struct PendingMeta { + media: Hang, + track_id: u32, + timescale: u64, +} + struct Fmp4Track { consumer: Consumer, @@ -55,8 +74,10 @@ impl Fmp4 { /// /// The hang catalog is subscribed internally; per-rendition tracks are (un)subscribed /// as the catalog changes. - pub fn new(broadcast: moq_net::BroadcastConsumer) -> Result { - let catalog_track = broadcast.subscribe_track_immediate(&hang::Catalog::default_track())?; + pub async fn new(broadcast: moq_net::BroadcastConsumer) -> Result { + let catalog_track = broadcast + .subscribe_track(hang::Catalog::DEFAULT_NAME, moq_net::Subscription::default()) + .await?; let catalog = crate::catalog::Consumer::new(catalog_track); Ok(Self { @@ -64,6 +85,8 @@ impl Fmp4 { catalog: Some(catalog), latency: Duration::ZERO, tracks: HashMap::new(), + pending_meta: HashMap::new(), + subscribe_queue: Arc::new(Mutex::new(Vec::new())), init_pending: None, init_emitted: false, }) @@ -102,7 +125,38 @@ impl Fmp4 { } } - // 2. Emit the init segment once it's been built. + // 2. Drain any subscribe_track tasks that have resolved into TrackConsumers. + let resolved: Vec<_> = self + .subscribe_queue + .lock() + .ok() + .map(|mut q| std::mem::take(&mut *q)) + .unwrap_or_default(); + for (name, result) in resolved { + let meta = match self.pending_meta.remove(&name) { + Some(meta) => meta, + None => continue, + }; + match result { + Ok(track) => { + let consumer = Consumer::new(track, meta.media).with_latency(self.latency); + self.tracks.insert( + name, + Fmp4Track { + consumer, + pending: None, + finished: false, + track_id: meta.track_id, + timescale: meta.timescale, + sequence_number: 1, + }, + ); + } + Err(err) => return Poll::Ready(Err(err.into())), + } + } + + // 3. Emit the init segment once it's been built. if !self.init_emitted && let Some(init) = self.init_pending.take() { @@ -110,7 +164,7 @@ impl Fmp4 { return Poll::Ready(Ok(Some(init))); } - // 3. Fill any empty pending slots by polling each consumer. + // 4. Fill any empty pending slots by polling each consumer. for track in self.tracks.values_mut() { if track.pending.is_some() || track.finished { continue; @@ -123,7 +177,7 @@ impl Fmp4 { } } - // 4. Pick the track whose pending frame has the smallest timestamp. + // 5. Pick the track whose pending frame has the smallest timestamp. let chosen = self .tracks .iter() @@ -138,12 +192,12 @@ impl Fmp4 { return Poll::Ready(Ok(Some(bytes))); } - // 5. If catalog is closed and every track is finished, we're done. - if self.catalog.is_none() && self.tracks.values().all(|t| t.finished) { + // 6. If catalog is closed and every track is finished, we're done. + if self.catalog.is_none() && self.pending_meta.is_empty() && self.tracks.values().all(|t| t.finished) { return Poll::Ready(Ok(None)); } - // 6. Drop finished tracks so the next catalog update can re-add a track of the same name. + // 7. Drop finished tracks so the next catalog update can re-add a track of the same name. self.tracks.retain(|_, t| !(t.finished && t.pending.is_none())); Poll::Pending @@ -167,35 +221,47 @@ impl Fmp4 { // Add any new tracks. We use the rendition's catalog index as the track_id so // fragment moof traf.tfhd.track_id matches the moov trak ids in the init segment. - let mut next_track_id = self.tracks.values().map(|t| t.track_id).max().unwrap_or(0) + 1; + let mut next_track_id = self + .tracks + .values() + .map(|t| t.track_id) + .chain(self.pending_meta.values().map(|m| m.track_id)) + .max() + .unwrap_or(0) + + 1; for (name, container) in &active { - if self.tracks.contains_key(name) { + if self.tracks.contains_key(name) || self.pending_meta.contains_key(name) { continue; } let media: Hang = (*container).try_into()?; - let track = self.broadcast.subscribe_track_immediate(&moq_net::Track::new(name.clone()))?; - let consumer = Consumer::new(track, media).with_latency(self.latency); - let timescale = catalog_timescale(catalog, name).context("track not in catalog")?; - self.tracks.insert( + self.pending_meta.insert( name.clone(), - Fmp4Track { - consumer, - pending: None, - finished: false, + PendingMeta { + media, track_id: next_track_id, timescale, - sequence_number: 1, }, ); next_track_id += 1; + + let broadcast = self.broadcast.clone(); + let queue = self.subscribe_queue.clone(); + let name = name.clone(); + tokio::spawn(async move { + let res = broadcast.subscribe_track(&name, moq_net::Subscription::default()).await; + if let Ok(mut q) = queue.lock() { + q.push((name, res)); + } + }); } // Remove tracks no longer in the catalog. self.tracks.retain(|name, _| active.contains_key(name)); + self.pending_meta.retain(|name, _| active.contains_key(name)); Ok(()) } diff --git a/rs/moq-mux/src/import/aac.rs b/rs/moq-mux/src/import/aac.rs index 1cb66247a..5cb4a0ae5 100644 --- a/rs/moq-mux/src/import/aac.rs +++ b/rs/moq-mux/src/import/aac.rs @@ -112,8 +112,12 @@ impl Aac { mut catalog: crate::catalog::Producer, config: AacConfig, ) -> anyhow::Result { - let mut track = broadcast.unique_track(".aac")?; - track.set_timescale(hang::container::TIMESCALE); + let name = broadcast.unique_name(".aac"); + let track = broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; let audio_config = hang::catalog::AudioConfig { codec: hang::catalog::AAC { diff --git a/rs/moq-mux/src/import/av01.rs b/rs/moq-mux/src/import/av01.rs index af7c4ae68..816bcd9c5 100644 --- a/rs/moq-mux/src/import/av01.rs +++ b/rs/moq-mux/src/import/av01.rs @@ -102,8 +102,12 @@ impl Av01 { self.catalog.lock().video.renditions.remove(&track.name); } - let mut track = self.broadcast.unique_track(".av01")?; - track.set_timescale(hang::container::TIMESCALE); + let name = self.broadcast.unique_name(".av01"); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; tracing::debug!(name = ?track.name, ?config, "starting track"); self.catalog .lock() @@ -147,8 +151,12 @@ impl Av01 { jitter: None, }; - let mut track = self.broadcast.unique_track(".av01")?; - track.set_timescale(hang::container::TIMESCALE); + let name = self.broadcast.unique_name(".av01"); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; tracing::debug!(name = ?track.name, "starting track with minimal config"); self.catalog .lock() @@ -238,8 +246,12 @@ impl Av01 { self.catalog.lock().video.renditions.remove(&track.name); } - let mut track = self.broadcast.unique_track(".av01")?; - track.set_timescale(hang::container::TIMESCALE); + let name = self.broadcast.unique_name(".av01"); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; self.catalog .lock() .video diff --git a/rs/moq-mux/src/import/avc1.rs b/rs/moq-mux/src/import/avc1.rs index 6fe320b02..8a44063cc 100644 --- a/rs/moq-mux/src/import/avc1.rs +++ b/rs/moq-mux/src/import/avc1.rs @@ -103,8 +103,12 @@ impl Avc1 { catalog.video.renditions.remove(&track.name); } - let mut track = self.broadcast.unique_track(".avc1")?; - track.set_timescale(hang::container::TIMESCALE); + let name = self.broadcast.unique_name(".avc1"); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; tracing::debug!(name = ?track.name, ?config, "starting avc1 track"); catalog.video.renditions.insert(track.name.clone(), config.clone()); diff --git a/rs/moq-mux/src/import/avc3.rs b/rs/moq-mux/src/import/avc3.rs index 38b04ac95..07255b58b 100644 --- a/rs/moq-mux/src/import/avc3.rs +++ b/rs/moq-mux/src/import/avc3.rs @@ -39,8 +39,14 @@ impl Avc3 { pub fn new(mut broadcast: moq_net::BroadcastProducer, catalog: crate::catalog::Producer) -> Self { // Create the track eagerly so callers can monitor used/unused before any frames arrive. // The catalog entry is added later in init() once the codec config is known. - let mut track = broadcast.unique_track(".avc3").expect("failed to create avc3 track"); - track.set_timescale(hang::container::TIMESCALE); + let name = broadcast.unique_name(".avc3"); + let track = broadcast + .create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + }) + .expect("failed to create avc3 track"); Self { catalog, diff --git a/rs/moq-mux/src/import/fmp4.rs b/rs/moq-mux/src/import/fmp4.rs index a742d52f9..8eed6dbe7 100644 --- a/rs/moq-mux/src/import/fmp4.rs +++ b/rs/moq-mux/src/import/fmp4.rs @@ -142,9 +142,13 @@ impl Fmp4 { let handler = &trak.mdia.hdlr.handler; let suffix = ".m4s"; - let mut track = self.broadcast.unique_track(suffix)?; + let name = self.broadcast.unique_name(suffix); // moq-mux frames are always emitted at microsecond timescale. - track.set_timescale(hang::container::TIMESCALE); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; let kind = match handler.as_ref() { b"vide" => { @@ -515,7 +519,8 @@ impl Fmp4 { .unwrap_or(tfhd.default_sample_size.unwrap_or(default_sample_size)) as usize; let pts = (dts as i64 + entry.cts.unwrap_or_default() as i64) as u64; - let timestamp = hang::container::Timestamp::from_scale(pts, timescale, hang::container::TIMESCALE)?; + let timestamp = hang::container::Timestamp::new(pts, moq_net::Timescale::new(timescale))? + .convert(hang::container::TIMESCALE)?; if offset + size > mdat.data.len() { anyhow::bail!("invalid data offset"); @@ -656,7 +661,7 @@ impl Fmp4 { .renditions .get_mut(&track.track.name) .context("missing video config")?; - config.jitter = Some(jitter.convert(1_000)?); + config.jitter = Some(jitter.convert(moq_net::Timescale::MILLI)?); } TrackKind::Audio => { let config = catalog @@ -664,7 +669,7 @@ impl Fmp4 { .renditions .get_mut(&track.track.name) .context("missing audio config")?; - config.jitter = Some(jitter.convert(1_000)?); + config.jitter = Some(jitter.convert(moq_net::Timescale::MILLI)?); } } } diff --git a/rs/moq-mux/src/import/hev1.rs b/rs/moq-mux/src/import/hev1.rs index b051e51cd..482dc73a6 100644 --- a/rs/moq-mux/src/import/hev1.rs +++ b/rs/moq-mux/src/import/hev1.rs @@ -92,8 +92,12 @@ impl Hev1 { catalog.video.renditions.remove(&track.name); } - let mut track = self.broadcast.unique_track(".hev1")?; - track.set_timescale(hang::container::TIMESCALE); + let name = self.broadcast.unique_name(".hev1"); + let track = self.broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; tracing::debug!(name = ?track.name, ?config, "starting track"); catalog.video.renditions.insert(track.name.clone(), config.clone()); diff --git a/rs/moq-mux/src/import/jitter.rs b/rs/moq-mux/src/import/jitter.rs index 2edf0beb7..f32ebe3b8 100644 --- a/rs/moq-mux/src/import/jitter.rs +++ b/rs/moq-mux/src/import/jitter.rs @@ -31,6 +31,6 @@ impl MinFrameDuration { } self.min_duration = Some(duration); - duration.convert(1_000).ok() + duration.convert(moq_net::Timescale::MILLI).ok() } } diff --git a/rs/moq-mux/src/import/opus.rs b/rs/moq-mux/src/import/opus.rs index 862343c2f..b604ce102 100644 --- a/rs/moq-mux/src/import/opus.rs +++ b/rs/moq-mux/src/import/opus.rs @@ -55,8 +55,12 @@ impl Opus { mut catalog: crate::catalog::Producer, config: OpusConfig, ) -> anyhow::Result { - let mut track = broadcast.unique_track(".opus")?; - track.set_timescale(hang::container::TIMESCALE); + let name = broadcast.unique_name(".opus"); + let track = broadcast.create_track(moq_net::Track { + name, + priority: 0, + timescale: hang::container::TIMESCALE, + })?; let audio_config = hang::catalog::AudioConfig { codec: hang::catalog::AudioCodec::Opus, diff --git a/rs/moq-native/examples/chat.rs b/rs/moq-native/examples/chat.rs index fc1c1d2cf..0e34b3ded 100644 --- a/rs/moq-native/examples/chat.rs +++ b/rs/moq-native/examples/chat.rs @@ -44,6 +44,7 @@ async fn run_broadcast(origin: moq_net::OriginProducer) -> anyhow::Result<()> { let mut track = broadcast.create_track(moq_net::Track { name: "chat".to_string(), priority: 0, + timescale: moq_net::Timescale::UNKNOWN, })?; // NOTE: The path is empty because we're using the URL to scope the broadcast. diff --git a/rs/moq-native/tests/backend.rs b/rs/moq-native/tests/backend.rs index 4ceec8fa4..ac091f008 100644 --- a/rs/moq-native/tests/backend.rs +++ b/rs/moq-native/tests/backend.rs @@ -70,7 +70,7 @@ async fn backend_test(scheme: &str, backend: moq_native::QuicBackend) { let bc = bc.expect("expected announce, got unannounce"); let mut track_sub = bc - .subscribe_track(&Track::new("video")) + .subscribe_track("video", moq_net::Subscription::default()) .await .expect("subscribe_track failed"); @@ -223,7 +223,7 @@ async fn iroh_connect() { let bc = bc.expect("expected announce, got unannounce"); let mut track_sub = bc - .subscribe_track(&Track::new("video")) + .subscribe_track("video", moq_net::Subscription::default()) .await .expect("subscribe_track failed"); diff --git a/rs/moq-native/tests/broadcast.rs b/rs/moq-native/tests/broadcast.rs index 87848cb48..d9d7278b4 100644 --- a/rs/moq-native/tests/broadcast.rs +++ b/rs/moq-native/tests/broadcast.rs @@ -88,7 +88,7 @@ async fn broadcast_test(scheme: &str, client_version: Option<&str>, server_versi // Subscribe to the track. let mut track_sub = bc - .subscribe_track(&Track::new("video")) + .subscribe_track("video", moq_net::Subscription::default()) .await .expect("subscribe_track failed"); @@ -501,7 +501,7 @@ async fn broadcast_websocket() { // Subscribe to the track. let mut track_sub = bc - .subscribe_track(&Track::new("video")) + .subscribe_track("video", moq_net::Subscription::default()) .await .expect("subscribe_track failed"); @@ -609,7 +609,7 @@ async fn broadcast_websocket_fallback() { // Subscribe to the track. let mut track_sub = bc - .subscribe_track(&Track::new("video")) + .subscribe_track("video", moq_net::Subscription::default()) .await .expect("subscribe_track failed"); diff --git a/rs/moq-net/src/coding/varint.rs b/rs/moq-net/src/coding/varint.rs index c98f7ca4b..7640f1918 100644 --- a/rs/moq-net/src/coding/varint.rs +++ b/rs/moq-net/src/coding/varint.rs @@ -188,7 +188,7 @@ impl fmt::Display for VarInt { impl VarInt { /// Decode a QUIC-style varint (2-bit length tag in top bits). - fn decode_quic(r: &mut R) -> Result { + pub fn decode_quic(r: &mut R) -> Result { if !r.has_remaining() { return Err(DecodeError::Short); } @@ -229,7 +229,7 @@ impl VarInt { } /// Encode a QUIC-style varint (2-bit length tag in top bits). - fn encode_quic(&self, w: &mut W) -> Result<(), EncodeError> { + pub fn encode_quic(&self, w: &mut W) -> Result<(), EncodeError> { let remaining = w.remaining_mut(); if self.0 < (1u64 << 6) { if remaining < 1 { diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 8d1103dfb..d63c8c5c0 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -5,7 +5,7 @@ use web_async::FuturesExt; use web_transport_trait::SendStream; use crate::{ - AsPath, Error, Origin, OriginConsumer, Track, TrackConsumer, + AsPath, Error, Origin, OriginConsumer, TrackConsumer, coding::{Stream, Writer}, ietf::{self, Control, FetchHeader, FetchType, FilterType, GroupOrder, Location, RequestId}, model::GroupConsumer, @@ -113,13 +113,16 @@ impl Publisher { return Ok(()); }; - let track = Track { - name: msg.track_name.to_string(), - priority: msg.subscriber_priority, - timescale: 0, - }; - - let track = match broadcast.subscribe_track_immediate(&track) { + let track = match broadcast + .subscribe_track( + &msg.track_name, + crate::Subscription { + priority: msg.subscriber_priority, + timeout: std::time::Duration::ZERO, + }, + ) + .await + { Ok(track) => track, Err(err) => { self.write_subscribe_error(&mut stream.writer, request_id, 404, &err.to_string()) diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 3382af4c8..66db2dfc5 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -465,7 +465,7 @@ impl Subscriber { let track = Track { name: msg.track_name.to_string(), priority: 0, - timescale: 0, + timescale: crate::Timescale::UNKNOWN, } .produce(); @@ -500,9 +500,9 @@ impl Subscriber { async fn run_broadcast(&self, path: Path<'_>, mut broadcast: BroadcastDynamic) -> Result<(), Error> { loop { - let track = tokio::select! { - producer = broadcast.requested_track() => match producer { - Ok(producer) => producer, + let request = tokio::select! { + request = broadcast.requested_track() => match request { + Ok(request) => request, Err(err) => { tracing::debug!(%err, "broadcast closed"); break; @@ -516,18 +516,26 @@ impl Subscriber { let path = path.to_owned(); let broadcast = broadcast.clone(); web_async::spawn(async move { - this.run_subscribe(path, broadcast, track).await; + this.run_subscribe(path, broadcast, request).await; }); } Ok(()) } - async fn run_subscribe(&mut self, broadcast_path: Path<'_>, broadcast: BroadcastDynamic, mut track: TrackProducer) { + async fn run_subscribe( + &mut self, + broadcast_path: Path<'_>, + broadcast: BroadcastDynamic, + request: crate::TrackRequest, + ) { + let track_name = request.name().to_string(); + let priority = request.subscription().priority; + let request_id = match self.control.next_request_id().await { Ok(id) => id, Err(err) => { - let _ = track.abort(err); + request.deny(err); return; } }; @@ -536,78 +544,92 @@ impl Subscriber { Ok(s) => s, Err(err) => { tracing::debug!(%err, "failed to open subscribe stream"); - let _ = track.abort(err); + request.deny(err); return; } }; - // Pre-register the track so group data arriving before SubscribeOk can be routed. - // The publisher uses request_id.0 as track_alias, and recv_group falls back to - // RequestId(track_alias) when no alias mapping exists, so this works. - { - let mut state = self.state.lock(); - state.subscribes.insert( - request_id, - TrackState { - producer: track.clone(), - alias: None, - }, - ); - } - // Write Subscribe message if let Err(err) = self - .write_subscribe(&mut stream, request_id, &broadcast_path, &track) + .write_subscribe(&mut stream, request_id, &broadcast_path, &track_name, priority) .await { tracing::debug!(%err, "failed to write subscribe"); - self.state.lock().subscribes.remove(&request_id); - let _ = track.abort(err); + request.deny(err); return; } - tracing::info!(broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), track = %track.name, "subscribe started"); + tracing::info!( + broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), + track = %track_name, + "subscribe started" + ); // Read the response and register the alias mapping let track_alias = match self.read_subscribe_response(&mut stream).await { - Ok(alias) => { - let mut state = self.state.lock(); - if let Some(alias) = alias { - state.aliases.insert(alias, request_id); - } - if let Some(track_state) = state.subscribes.get_mut(&request_id) { - if let Some(alias) = alias { - track_state.alias = Some(alias); - } - // LOC-style track properties are not parsed yet; default the - // timescale to 0 (unspecified) so consumers awaiting it - // don't hang. TODO: read timescale from track properties on - // Draft17+. - track_state.producer.set_timescale(0); - } - alias - } + Ok(alias) => alias, Err(err) => { tracing::debug!(%err, "subscribe response error"); - self.state.lock().subscribes.remove(&request_id); - let _ = track.abort(err); + request.deny(err); return; } }; + // LOC-style track properties are not parsed yet; default the timescale + // to UNKNOWN. TODO: read timescale from track properties on Draft17+. + let track_info = crate::Track { + name: track_name.clone(), + priority, + timescale: crate::Timescale::UNKNOWN, + }; + + let mut track = match request.accept(track_info) { + Ok(track) => track, + Err(err) => { + tracing::debug!(%err, "request accept failed"); + return; + } + }; + + { + let mut state = self.state.lock(); + if let Some(alias) = track_alias { + state.aliases.insert(alias, request_id); + } + state.subscribes.insert( + request_id, + TrackState { + producer: track.clone(), + alias: track_alias, + }, + ); + } + tokio::select! { _ = track.unused() => { - tracing::info!(broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), track = %track.name, "subscribe cancelled"); + tracing::info!( + broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), + track = %track_name, + "subscribe cancelled" + ); let _ = track.abort(Error::Cancel); } err = broadcast.closed() => { - tracing::info!(broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), track = %track.name, "broadcast closed"); + tracing::info!( + broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), + track = %track_name, + "broadcast closed" + ); let _ = track.abort(err); } res = stream.reader.closed() => { match res { Ok(()) => { - tracing::info!(broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), track = %track.name, "subscribe complete"); + tracing::info!( + broadcast = %self.origin.as_ref().expect("origin set by start_announce").absolute(&broadcast_path), + track = %track_name, + "subscribe complete" + ); let _ = track.finish(); } Err(err) => { @@ -632,7 +654,8 @@ impl Subscriber { stream: &mut Stream, request_id: RequestId, broadcast: &Path<'_>, - track: &TrackProducer, + track_name: &str, + priority: u8, ) -> Result<(), Error> { stream.writer.encode(&ietf::Subscribe::ID).await?; stream @@ -640,8 +663,8 @@ impl Subscriber { .encode(&ietf::Subscribe { request_id, track_namespace: broadcast.to_owned(), - track_name: (&track.name).into(), - subscriber_priority: track.priority, + track_name: track_name.into(), + subscriber_priority: priority, group_order: GroupOrder::Descending, filter_type: FilterType::LargestObject, }) @@ -744,7 +767,7 @@ impl Subscriber { if size == 0 { let status: u64 = stream.decode().await?; if status == 0 { - let mut frame = producer.create_frame(Frame::new(0))?; + let mut frame = producer.create_frame(Frame::from(0u64))?; frame.finish()?; } else if status == 3 && !group.flags.has_end { break; @@ -752,7 +775,7 @@ impl Subscriber { return Err(Error::Unsupported); } } else { - let mut frame = producer.create_frame(Frame::new(size))?; + let mut frame = producer.create_frame(Frame::from(size))?; if let Err(err) = self.run_frame(stream, frame.clone()).await { let _ = frame.abort(err.clone()); diff --git a/rs/moq-net/src/lib.rs b/rs/moq-net/src/lib.rs index d1c50bd09..ec9c6b66f 100644 --- a/rs/moq-net/src/lib.rs +++ b/rs/moq-net/src/lib.rs @@ -49,7 +49,7 @@ //! standard [`std::task::Waker`] API and any [`std::task::Waker`] is a valid driver. mod client; -mod coding; +pub mod coding; mod error; mod ietf; mod lite; diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 1fd650be1..8ea8e0155 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -5,7 +5,7 @@ use web_async::FuturesExt; use web_transport_trait::Stats; use crate::{ - AsPath, BroadcastConsumer, Error, Origin, OriginConsumer, OriginList, Track, TrackConsumer, + AsPath, BroadcastConsumer, Error, Origin, OriginConsumer, OriginList, Subscription, Timescale, TrackConsumer, coding::{Stream, Writer}, lite::{ self, @@ -305,16 +305,19 @@ impl Publisher { priority: PriorityQueue, version: Version, ) -> Result<(), Error> { - let track = Track { - name: subscribe.track.to_string(), - priority: subscribe.priority, - timescale: 0, - }; - let broadcast = consumer.ok_or(Error::NotFound)?; - let track = broadcast.subscribe_track_immediate(&track)?; - // TODO wait until track.info() to get the *real* priority + // Await the publisher's authoritative Track properties (priority, + // timescale) before responding with SUBSCRIBE_OK. + let track = broadcast + .subscribe_track( + &subscribe.track, + Subscription { + priority: subscribe.priority, + timeout: std::time::Duration::ZERO, + }, + ) + .await?; let info = lite::SubscribeOk { priority: track.priority, @@ -322,7 +325,7 @@ impl Publisher { max_latency: std::time::Duration::ZERO, start_group: None, end_group: None, - timescale: track.timescale, + timescale: track.timescale.as_u64(), }; stream.writer.encode(&lite::SubscribeResponse::Ok(info)).await?; @@ -370,9 +373,7 @@ impl Publisher { }; let priority = priority.insert(track.priority, sequence); - tasks.push( - Self::serve_group(session.clone(), msg, priority, group, version, track.timescale).map(|_| ()), - ); + tasks.push(Self::serve_group(session.clone(), msg, priority, group, version, track.timescale).map(|_| ())); } } @@ -382,7 +383,7 @@ impl Publisher { mut priority: PriorityHandle, mut group: GroupConsumer, version: Version, - track_timescale: u64, + track_timescale: Timescale, ) -> Result<(), Error> { // TODO add a way to open in priority order. let stream = session.open_uni().await.map_err(Error::from_transport)?; @@ -422,8 +423,7 @@ impl Publisher { let delta: i64 = (curr as i128 - prev_ts as i128) .try_into() .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; - let zz = crate::coding::VarInt::from_zigzag(delta) - .map_err(crate::coding::EncodeError::from)?; + let zz = crate::coding::VarInt::from_zigzag(delta).map_err(crate::coding::EncodeError::from)?; stream.encode(&zz).await?; prev_ts = curr; } diff --git a/rs/moq-net/src/lite/subscribe.rs b/rs/moq-net/src/lite/subscribe.rs index c87cb1a83..fb00230ee 100644 --- a/rs/moq-net/src/lite/subscribe.rs +++ b/rs/moq-net/src/lite/subscribe.rs @@ -373,7 +373,7 @@ mod tests { }; let decoded = roundtrip_ok(Version::Lite04, ok); assert_eq!(decoded.priority, 7); - assert_eq!(decoded.ordered, true); + assert!(decoded.ordered); assert_eq!(decoded.start_group, Some(2)); assert_eq!(decoded.end_group, Some(10)); assert_eq!(decoded.timescale, 0); diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 9909f46fd..4a923d859 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -7,7 +7,7 @@ use futures::{StreamExt, stream::FuturesUnordered}; use crate::{ AsPath, BandwidthProducer, Broadcast, BroadcastDynamic, Error, Frame, FrameProducer, Group, GroupProducer, - OriginProducer, Path, PathOwned, TrackProducer, + OriginProducer, Path, PathOwned, Timescale, Track, TrackProducer, TrackRequest, coding::{Reader, Stream}, lite, model::BroadcastProducer, @@ -271,9 +271,9 @@ impl Subscriber { loop { // Keep serving requests until there are no more consumers. // This way we'll clean up the task when the broadcast is no longer needed. - let track = tokio::select! { - producer = broadcast.requested_track() => match producer { - Ok(producer) => producer, + let request = tokio::select! { + request = broadcast.requested_track() => match request { + Ok(request) => request, Err(err) => { tracing::debug!(%err, "broadcast closed"); break; @@ -288,91 +288,104 @@ impl Subscriber { let path = path.clone(); let broadcast = broadcast.clone(); web_async::spawn(async move { - this.run_subscribe(id, path, broadcast, track).await; + this.run_subscribe(id, path, broadcast, request).await; this.subscribes.lock().remove(&id); }); } } - async fn run_subscribe(&mut self, id: u64, path: PathOwned, broadcast: BroadcastDynamic, mut track: TrackProducer) { - self.subscribes.lock().insert(id, track.clone()); + async fn run_subscribe(&mut self, id: u64, path: PathOwned, broadcast: BroadcastDynamic, request: TrackRequest) { + let track_name = request.name().to_string(); + let priority = request.subscription().priority; + + tracing::info!(id, broadcast = %self.log_path(&path), track = %track_name, "subscribe started"); let msg = lite::Subscribe { id, broadcast: path.as_path(), - track: (&track.name).into(), - priority: track.priority, + track: (&track_name).into(), + priority, ordered: true, max_latency: std::time::Duration::ZERO, start_group: None, end_group: None, }; - tracing::info!(id, broadcast = %self.log_path(&path), track = %track.name, "subscribe started"); - - tokio::select! { - _ = track.unused() => { - tracing::info!(id, broadcast = %self.log_path(&path), track = %track.name, "subscribe cancelled"); - let _ = track.abort(Error::Cancel); - } - err = broadcast.closed() => { - tracing::info!(id, broadcast = %self.log_path(&path), track = %track.name, "broadcast closed"); - let _ = track.abort(err); + // Send SUBSCRIBE and wait for SUBSCRIBE_OK to get the publisher's + // Track properties. Then convert the request into a TrackProducer. + let mut stream = match Stream::open(&self.session, self.version).await { + Ok(s) => s, + Err(err) => { + request.deny(err); + return; } - res = self.run_track(msg) => match res { - Ok(()) => { - tracing::info!(id, broadcast = %self.log_path(&path), track = %track.name, "subscribe complete"); - let _ = track.finish(); - } - Err(err) => { - tracing::warn!(id, broadcast = %self.log_path(&path), track = %track.name, %err, "subscribe error"); - let _ = track.abort(err); - } - }, + }; + if let Err(err) = stream.writer.encode(&lite::ControlType::Subscribe).await { + request.deny(err); + return; } - } - - async fn run_track(&mut self, msg: lite::Subscribe<'_>) -> Result<(), Error> { - let mut stream = Stream::open(&self.session, self.version).await?; - stream.writer.encode(&lite::ControlType::Subscribe).await?; - - if let Err(err) = self.run_track_stream(&mut stream, msg).await { + if let Err(err) = stream.writer.encode(&msg).await { stream.writer.abort(&err); - return Err(err); + request.deny(err); + return; } - stream.writer.finish()?; - stream.writer.closed().await - } - - async fn run_track_stream( - &mut self, - stream: &mut Stream, - msg: lite::Subscribe<'_>, - ) -> Result<(), Error> { - let subscribe_id = msg.id; - stream.writer.encode(&msg).await?; + let info: lite::SubscribeOk = match stream.reader.decode::().await { + Ok(lite::SubscribeResponse::Ok(info)) => info, + Ok(_) => { + request.deny(Error::ProtocolViolation); + stream.writer.abort(&Error::ProtocolViolation); + return; + } + Err(err) => { + let cause = err; + stream.writer.abort(&cause); + request.deny(cause); + return; + } + }; - // The first response MUST be a SUBSCRIBE_OK. - let resp: lite::SubscribeResponse = stream.reader.decode().await?; - let lite::SubscribeResponse::Ok(info) = resp else { - return Err(Error::ProtocolViolation); + let track_struct = Track { + name: track_name.clone(), + priority: info.priority, + timescale: Timescale::new(info.timescale), }; - // Apply the negotiated timescale to the track producer so consumers can - // observe it via TrackConsumer::timescale() and run_group can decode - // per-frame timestamps at the correct scale. - { - let mut subs = self.subscribes.lock(); - if let Some(track) = subs.get_mut(&subscribe_id) { - track.set_timescale(info.timescale); + let mut track = match request.accept(track_struct) { + Ok(track) => track, + Err(err) => { + stream.writer.abort(&err); + return; } - } + }; - // TODO handle additional SUBSCRIBE_OK and SUBSCRIBE_DROP messages. - stream.reader.closed().await?; + self.subscribes.lock().insert(id, track.clone()); - Ok(()) + tokio::select! { + _ = track.unused() => { + tracing::info!(id, broadcast = %self.log_path(&path), track = %track_name, "subscribe cancelled"); + let _ = track.abort(Error::Cancel); + stream.writer.abort(&Error::Cancel); + } + err = broadcast.closed() => { + tracing::info!(id, broadcast = %self.log_path(&path), track = %track_name, "broadcast closed"); + let _ = track.abort(err.clone()); + stream.writer.abort(&err); + } + res = stream.reader.closed() => { + match res { + Ok(()) => { + tracing::info!(id, broadcast = %self.log_path(&path), track = %track_name, "subscribe complete"); + let _ = track.finish(); + } + Err(err) => { + tracing::warn!(id, broadcast = %self.log_path(&path), track = %track_name, %err, "subscribe error"); + let _ = track.abort(err); + } + } + let _ = stream.writer.finish(); + } + } } pub async fn recv_group(&mut self, stream: &mut Reader) -> Result<(), Error> { @@ -414,7 +427,7 @@ impl Subscriber { &mut self, stream: &mut Reader, mut group: GroupProducer, - track_timescale: u64, + track_timescale: Timescale, ) -> Result<(), Error> { // Previous frame's raw timestamp value, for zigzag-delta decoding on Lite05+. let mut prev_ts: u64 = 0; @@ -430,7 +443,7 @@ impl Subscriber { .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; prev_ts = next; let size = stream.decode::().await?; - let ts = crate::Timestamp::new_u64(next, track_timescale) + let ts = crate::Timestamp::new(next, track_timescale) .map_err(|_| Error::BoundsExceeded(crate::coding::BoundsExceeded))?; (size, ts) } else { @@ -440,7 +453,7 @@ impl Subscriber { (size, crate::Timestamp::ZERO) }; - let mut frame = group.create_frame(Frame::new(size).with_timestamp(timestamp))?; + let mut frame = group.create_frame(Frame { size, timestamp })?; if let Err(err) = self.run_frame(stream, &mut frame).await { let _ = frame.abort(err.clone()); diff --git a/rs/moq-net/src/lite/version.rs b/rs/moq-net/src/lite/version.rs index e060a2c7b..1d9e416c1 100644 --- a/rs/moq-net/src/lite/version.rs +++ b/rs/moq-net/src/lite/version.rs @@ -15,7 +15,9 @@ pub enum Version { impl Version { /// Whether this version carries per-frame timestamps and per-track timescale /// on the wire. + #[allow(clippy::match_like_matches_macro)] pub fn has_timestamps(self) -> bool { + // Match form is used so future versions default forward (CLAUDE.md convention). match self { Self::Lite01 | Self::Lite02 | Self::Lite03 | Self::Lite04 => false, _ => true, diff --git a/rs/moq-net/src/model/broadcast.rs b/rs/moq-net/src/model/broadcast.rs index 900d85a1f..6e45c2a6e 100644 --- a/rs/moq-net/src/model/broadcast.rs +++ b/rs/moq-net/src/model/broadcast.rs @@ -4,7 +4,9 @@ use std::{ task::{Poll, ready}, }; -use crate::{Error, TrackConsumer, TrackProducer, model::track::TrackWeak}; +use tokio::sync::oneshot; + +use crate::{Error, Subscription, TrackConsumer, TrackProducer, model::track::TrackWeak}; use super::{OriginList, Track}; @@ -32,19 +34,35 @@ impl Broadcast { } } -#[derive(Default, Clone)] +/// A pending subscription request, waiting for a publisher to call +/// [`TrackRequest::accept`] (or [`TrackRequest::deny`]) to resolve it. +struct PendingRequest { + subscription: Subscription, + /// Resolvers waiting for the producer to be created. Each call to + /// [`BroadcastConsumer::subscribe_track`] for the same name during the + /// pending window adds an entry here so they all see the same producer. + resolvers: Vec>>, +} + +#[derive(Default)] struct State { - // Weak references for deduplication. Doesn't prevent track auto-close. + /// Weak references to live producers, used to dedupe subscribe_track calls + /// that target a name already being served. tracks: HashMap, - // Dynamic tracks that have been requested. - requests: Vec, + /// Pending requests by track name. Used both to fan out the resolved + /// producer to multiple awaiting subscribers and as the queue that + /// [`BroadcastDynamic::requested_track`] drains. + requests: HashMap, + + /// Names in `requests` ordered FIFO for the dynamic handler. + request_order: Vec, - // The current number of dynamic producers. - // If this is 0, requests must be empty. + /// The current number of dynamic producers. + /// If this is 0, requests must be empty. dynamic: usize, - // The error that caused the broadcast to be aborted, if any. + /// The error that caused the broadcast to be aborted, if any. abort: Option, } @@ -64,6 +82,16 @@ impl State { entry.insert(weak); Ok(()) } + + /// Drop the named pending request and notify all resolvers with `err`. + fn deny_request(&mut self, name: &str, err: Error) { + if let Some(pending) = self.requests.remove(name) { + self.request_order.retain(|n| n != name); + for tx in pending.resolvers { + let _ = tx.send(Err(err.clone())); + } + } + } } /// Manages tracks within a broadcast. @@ -124,22 +152,15 @@ impl BroadcastProducer { /// /// Generates names like `0{suffix}`, `1{suffix}`, etc. and picks the first /// one not already used in this broadcast. - pub fn unique_track(&mut self, suffix: &str) -> Result { + pub fn unique_name(&self, suffix: &str) -> String { let state = self.state.read(); - let mut name = String::new(); for i in 0u32.. { - name = format!("{i}{suffix}"); + let name = format!("{i}{suffix}"); if !state.tracks.contains_key(&name) { - break; + return name; } } - drop(state); - - self.create_track(Track { - name, - priority: 0, - timescale: 0, - }) + unreachable!("u32 namespace exhausted"); } /// Create a dynamic producer that handles on-demand track requests from consumers. @@ -167,8 +188,12 @@ impl BroadcastProducer { // Abort any pending dynamic track requests; their producers are owned // by the broadcast and would otherwise leave consumers stuck forever. - for mut request in guard.requests.drain(..) { - request.abort(err.clone()).ok(); + for name in std::mem::take(&mut guard.request_order) { + if let Some(pending) = guard.requests.remove(&name) { + for tx in pending.resolvers { + let _ = tx.send(Err(err.clone())); + } + } } guard.abort = Some(err); @@ -193,11 +218,104 @@ impl BroadcastProducer { } } +/// A pending track subscription. Hold this until you have constructed the full +/// [`Track`] and call [`Self::accept`], or [`Self::deny`] if the request can't +/// be served. Dropping without calling either implicitly denies with +/// [`Error::Cancel`]. +pub struct TrackRequest { + name: String, + subscription: Subscription, + state: conducer::Producer, + /// `None` after [`Self::accept`] or [`Self::deny`] has been called, so the + /// Drop impl knows not to double-deny. + completed: bool, +} + +impl TrackRequest { + /// The track name requested by the subscriber(s). + pub fn name(&self) -> &str { + &self.name + } + + /// The first subscriber's requested [`Subscription`]. Use this as a hint + /// for how to configure the [`Track`] (priority, timescale, etc.). Once + /// the request is accepted, the full aggregate becomes visible via + /// [`TrackProducer::max_priority`] / [`TrackProducer::max_timeout`]. + pub fn subscription(&self) -> &Subscription { + &self.subscription + } + + /// Fulfill the request with the given track. The track's `name` must match + /// [`Self::name`]; returns [`Error::NotFound`] otherwise. + pub fn accept(mut self, track: Track) -> Result { + if track.name != self.name { + return Err(Error::NotFound); + } + + let producer = TrackProducer::new(track); + self.completed = true; + + let mut state = modify(&self.state)?; + let pending = state.requests.remove(&self.name).ok_or(Error::Cancel)?; + + // Insert the producer's weak so future subscribe_track calls dedupe. + state.insert_track(producer.weak()).ok(); + + // Fan out a TrackConsumer to each waiting subscriber, carrying their + // own Subscription (the first subscriber's subscription matches the + // request; remaining resolvers in the queue already added theirs). + let weak = producer.weak(); + let mut resolvers = pending.resolvers.into_iter(); + if let Some(tx) = resolvers.next() { + let _ = tx.send(Ok(weak.consume_with(pending.subscription.clone()))); + } + for tx in resolvers { + let _ = tx.send(Ok(weak.consume_with(Subscription::default()))); + } + + // Spawn the cleanup task that removes the entry once nobody is consuming. + let consumer_state = self.state.clone(); + let weak_for_cleanup = producer.weak(); + web_async::spawn(async move { + let _ = weak_for_cleanup.unused().await; + let Ok(mut state) = consumer_state.write() else { return }; + + if let Some(current) = state.tracks.remove(&weak_for_cleanup.info.name) + && !current.is_clone(&weak_for_cleanup) + { + state.tracks.insert(current.info.name.clone(), current); + } + }); + + Ok(producer) + } + + /// Reject the request with the given error, waking all waiting subscribers. + pub fn deny(mut self, err: Error) { + self.completed = true; + if let Ok(mut state) = self.state.write() { + state.deny_request(&self.name, err); + } + } +} + +impl Drop for TrackRequest { + fn drop(&mut self) { + if !self.completed + && let Ok(mut state) = self.state.write() + { + state.deny_request(&self.name, Error::Cancel); + } + } +} + /// Handles on-demand track creation for a broadcast. /// -/// When a consumer requests a track that doesn't exist, a [TrackProducer] is created -/// and queued for the dynamic producer to fulfill via [Self::requested_track]. -/// Dropped when no longer needed; pending requests are automatically aborted. +/// When a consumer requests a track that doesn't exist, the dynamic producer +/// can pick up the request via [`Self::requested_track`] and either +/// [`TrackRequest::accept`] it with a concrete [`Track`] or +/// [`TrackRequest::deny`] it. Dropped when no longer needed; pending requests +/// are automatically aborted. pub struct BroadcastDynamic { info: Broadcast, state: conducer::Producer, @@ -249,17 +367,30 @@ impl BroadcastDynamic { }) } - /// Poll for the next consumer-requested track, without blocking. The returned producer - /// is preconfigured with the requested track's name and priority. - pub fn poll_requested_track(&mut self, waiter: &conducer::Waiter) -> Poll> { - self.poll(waiter, |state| match state.requests.pop() { - Some(producer) => Poll::Ready(producer), - None => Poll::Pending, + /// Poll for the next consumer-requested track. + pub fn poll_requested_track(&mut self, waiter: &conducer::Waiter) -> Poll> { + let state_clone = self.state.clone(); + self.poll(waiter, |state| { + let Some(name) = state.request_order.first().cloned() else { + return Poll::Pending; + }; + let pending = state.requests.get(&name).expect("request_order must mirror requests"); + let subscription = pending.subscription.clone(); + state.request_order.remove(0); + Poll::Ready((name, subscription)) + }) + .map(|res| { + res.map(|(name, subscription)| TrackRequest { + name, + subscription, + state: state_clone, + completed: false, + }) }) } - /// Block until a consumer requests a track, returning its producer. - pub async fn requested_track(&mut self) -> Result { + /// Block until a consumer requests a track, returning a request handle. + pub async fn requested_track(&mut self) -> Result { conducer::wait(|waiter| self.poll_requested_track(waiter)).await } @@ -286,10 +417,12 @@ impl BroadcastDynamic { pub fn abort(&mut self, err: Error) -> Result<(), Error> { let mut guard = modify(&self.state)?; - // Abort any pending dynamic track requests; their producers are owned - // by the broadcast and would otherwise leave consumers stuck forever. - for mut request in guard.requests.drain(..) { - request.abort(err.clone()).ok(); + for name in std::mem::take(&mut guard.request_order) { + if let Some(pending) = guard.requests.remove(&name) { + for tx in pending.resolvers { + let _ = tx.send(Err(err.clone())); + } + } } guard.abort = Some(err); @@ -313,8 +446,12 @@ impl Drop for BroadcastDynamic { } // Abort all pending requests since there's no dynamic producer to handle them. - for mut request in state.requests.drain(..) { - request.abort(Error::Cancel).ok(); + for name in std::mem::take(&mut state.request_order) { + if let Some(pending) = state.requests.remove(&name) { + for tx in pending.resolvers { + let _ = tx.send(Err(Error::Cancel)); + } + } } } } @@ -325,7 +462,7 @@ use futures::FutureExt; #[cfg(test)] impl BroadcastDynamic { - pub fn assert_request(&mut self) -> TrackProducer { + pub fn assert_request(&mut self) -> TrackRequest { self.requested_track() .now_or_never() .expect("should not have blocked") @@ -355,79 +492,60 @@ impl Deref for BroadcastConsumer { impl BroadcastConsumer { /// Subscribe to a track on this broadcast. /// - /// Reuses an existing producer if one is already publishing the track; otherwise - /// queues a new dynamic request that the broadcast's producer will service via - /// [`BroadcastDynamic::requested_track`]. Returns [`Error::NotFound`] if the - /// broadcast has no dynamic producer to handle requests. + /// Returns once the publisher has resolved the track's properties (priority + /// and timescale) via the dynamic handler's [`TrackRequest::accept`]. + /// Reuses an existing producer if one is already publishing the track; + /// otherwise queues a new dynamic request. Returns [`Error::NotFound`] if + /// no dynamic producer exists to service the request. /// - /// Awaits the negotiated timescale (delivered via SUBSCRIBE_OK on the session - /// layer) before returning, so the resulting [`TrackConsumer`] exposes a - /// resolved [`TrackConsumer::timescale_now`]. For protocol versions that - /// don't negotiate a timescale, the session layer publishes `0` immediately - /// and this future resolves without delay. - pub async fn subscribe_track(&self, track: &Track) -> Result { - let consumer = self.subscribe_track_immediate(track)?; - // Wait for the session layer to publish the negotiated timescale. - // Ignore the result: if the track is already closed, the consumer - // itself will surface that on the next operation. - let _ = consumer.timescale().await; - Ok(consumer) - } - - /// Same as [`Self::subscribe_track`] but does not wait for the timescale to be - /// resolved. Callers that don't need the timescale (or want to handle resolution - /// themselves via [`TrackConsumer::timescale`]) can use this to avoid an extra - /// roundtrip. - pub fn subscribe_track_immediate(&self, track: &Track) -> Result { - // Upgrade to a temporary producer so we can modify the state. - let producer = self - .state - .produce() - .ok_or_else(|| self.state.read().abort.clone().unwrap_or(Error::Dropped))?; - let mut state = modify(&producer)?; - - if let Some(weak) = state.tracks.get(&track.name) { - if !weak.is_closed() { - return Ok(weak.consume()); + /// The returned [`TrackConsumer`] dereferences to a [`Track`] with the + /// publisher's authoritative properties. The subscription is tracked + /// internally and contributes to the producer's + /// [`TrackProducer::max_priority`] / [`TrackProducer::max_timeout`] + /// aggregates; call [`TrackConsumer::update_subscription`] to update. + pub async fn subscribe_track(&self, name: &str, subscription: Subscription) -> Result { + let rx = { + // Upgrade to a temporary producer so we can modify the state. + let producer = self + .state + .produce() + .ok_or_else(|| self.state.read().abort.clone().unwrap_or(Error::Dropped))?; + let mut state = modify(&producer)?; + + // Reuse an existing producer if one is already live. + if let Some(weak) = state.tracks.get(name) { + if !weak.is_closed() { + return Ok(weak.consume_with(subscription)); + } + // Stale entry; remove and treat as a new request. + state.tracks.remove(name); } - // Remove the stale entry - state.tracks.remove(&track.name); - } - // Otherwise we have never seen this track before and need to create a new producer. - let producer = track.clone().produce(); - let consumer = producer.consume(); - - if state.dynamic == 0 { - return Err(Error::NotFound); - } - - // Insert a weak reference for deduplication. - let weak = producer.weak(); - state.tracks.insert(producer.name.clone(), weak.clone()); - state.requests.push(producer); - - // Remove the track from the lookup when it's unused. - let consumer_state = self.state.clone(); - web_async::spawn(async move { - let _ = weak.unused().await; - - let Some(producer) = consumer_state.produce() else { - return; - }; - let Ok(mut state) = producer.write() else { - return; - }; - - // Remove the entry, but reinsert if it was replaced by a different reference. - if let Some(current) = state.tracks.remove(&weak.info.name) - && !current.is_clone(&weak) - { - state.tracks.insert(current.info.name.clone(), current); + let (tx, rx) = oneshot::channel(); + + // Coalesce with an in-flight request for the same name. + if let Some(pending) = state.requests.get_mut(name) { + pending.resolvers.push(tx); + rx + } else if state.dynamic == 0 { + return Err(Error::NotFound); + } else { + state.requests.insert( + name.to_string(), + PendingRequest { + subscription: subscription.clone(), + resolvers: vec![tx], + }, + ); + state.request_order.push(name.to_string()); + rx } - }); + }; - Ok(consumer) + match rx.await { + Ok(res) => res, + Err(_) => Err(self.state.read().abort.clone().unwrap_or(Error::Cancel)), + } } /// Block until the broadcast is closed and return the cause. @@ -461,10 +579,6 @@ impl BroadcastConsumer { #[cfg(test)] impl BroadcastConsumer { - pub fn assert_subscribe_track(&self, track: &Track) -> TrackConsumer { - self.subscribe_track_immediate(track).expect("should not have errored") - } - pub fn assert_not_closed(&self) { assert!(self.closed().now_or_never().is_none(), "should not be closed"); } @@ -489,14 +603,20 @@ mod test { let consumer = producer.consume(); - let mut track1_sub = consumer.assert_subscribe_track(&Track::new("track1")); + let mut track1_sub = consumer + .subscribe_track("track1", Subscription::default()) + .await + .expect("should subscribe"); track1_sub.assert_group(); let mut track2 = Track::new("track2").produce(); producer.assert_insert_track(&track2); let consumer2 = producer.consume(); - let mut track2_consumer = consumer2.assert_subscribe_track(&Track::new("track2")); + let mut track2_consumer = consumer2 + .subscribe_track("track2", Subscription::default()) + .await + .expect("should subscribe"); track2_consumer.assert_no_group(); track2.append_group().unwrap(); @@ -512,18 +632,17 @@ mod test { let consumer = producer.consume(); consumer.assert_not_closed(); - // Create a new track and insert it into the broadcast. + // Create a new track and insert it into the broadcast so subscribe_track + // can resolve immediately. let track1 = producer.assert_create_track(&Track::new("track1")); - let track1c = consumer.assert_subscribe_track(&track1); - let track2 = consumer.assert_subscribe_track(&Track::new("track2")); + let track1c = consumer + .subscribe_track("track1", Subscription::default()) + .await + .expect("should subscribe"); // Aborting the broadcast must NOT cascade to externally-owned tracks. producer.abort(Error::Cancel).unwrap(); - // track2's producer was owned by the broadcast (a pending dynamic - // request), so the consumer surfaces the abort. - track2.assert_error(); - // track1's producer is held outside the broadcast, so it survives. assert!(!track1.is_closed()); track1c.assert_not_closed(); @@ -532,39 +651,52 @@ mod test { #[tokio::test] async fn requests() { let mut producer = Broadcast::new().produce().dynamic(); - let consumer = producer.consume(); let consumer2 = consumer.clone(); - let mut track1 = consumer.assert_subscribe_track(&Track::new("track1")); - track1.assert_not_closed(); - track1.assert_no_group(); + // Spawn the subscriber tasks since subscribe_track now awaits resolution. + let sub1 = tokio::spawn({ + let consumer = consumer.clone(); + async move { consumer.subscribe_track("track1", Subscription::default()).await } + }); + let sub2 = tokio::spawn({ + let consumer = consumer2.clone(); + async move { consumer.subscribe_track("track1", Subscription::default()).await } + }); - // Make sure we deduplicate requests while track1 is still active. - let mut track2 = consumer2.assert_subscribe_track(&Track::new("track1")); - track2.assert_is_clone(&track1); + // Give the spawned tasks a chance to register their requests. + tokio::task::yield_now().await; + tokio::task::yield_now().await; - // Get the requested track, and there should only be one. - let mut track3 = producer.assert_request(); + // Get the requested track, and there should only be one (deduped). + let request = producer.assert_request(); + assert_eq!(request.name(), "track1"); producer.assert_no_request(); - // Make sure the consumer is the same. - track3.consume().assert_is_clone(&track1); + let mut track_producer = request.accept(Track::new("track1")).unwrap(); + + let mut track1 = sub1.await.unwrap().expect("should resolve"); + let mut track2 = sub2.await.unwrap().expect("should resolve"); + track1.assert_is_clone(&track2); - // Append a group and make sure they all get it. - track3.append_group().unwrap(); + // Append a group and make sure both consumers receive it. + track_producer.append_group().unwrap(); track1.assert_group(); track2.assert_group(); - // Make sure that tracks are cancelled when the producer is dropped. - let track4 = consumer.assert_subscribe_track(&Track::new("track2")); + // Subscribe to a new track and drop the dynamic — the pending sub aborts. + let sub3 = tokio::spawn({ + let consumer = consumer.clone(); + async move { consumer.subscribe_track("track2", Subscription::default()).await } + }); + tokio::task::yield_now().await; drop(producer); - // Make sure the track is errored, not closed. - track4.assert_error(); + assert!(sub3.await.unwrap().is_err(), "request should be cancelled"); - let track5 = consumer2.subscribe_track_immediate(&Track::new("track3")); - assert!(track5.is_err(), "should have errored"); + // Subscribing now should return NotFound (no dynamic). + let res = consumer2.subscribe_track("track3", Subscription::default()).await; + assert!(res.is_err(), "should have errored"); } #[tokio::test] @@ -572,86 +704,44 @@ mod test { let mut broadcast = Broadcast::new().produce().dynamic(); let consumer = broadcast.consume(); - // Subscribe to a track, creating a request - let track1 = consumer.assert_subscribe_track(&Track::new("track1")); + // Subscribe in the background. + let sub1 = tokio::spawn({ + let consumer = consumer.clone(); + async move { consumer.subscribe_track("track1", Subscription::default()).await } + }); + tokio::task::yield_now().await; + + let request = broadcast.assert_request(); + let mut producer1 = request.accept(Track::new("track1")).unwrap(); + + let track1 = sub1.await.unwrap().expect("should resolve"); - // Get the requested producer and close it (simulating publisher disconnect) - let mut producer1 = broadcast.assert_request(); producer1.append_group().unwrap(); producer1.finish().unwrap(); drop(producer1); - // The consumer should see the track as closed + // Consumer should see the track as closed. track1.assert_closed(); - // Subscribe again to the same track - should get a NEW producer, not the stale one - let mut track2 = consumer.assert_subscribe_track(&Track::new("track1")); - track2.assert_not_closed(); - track2.assert_not_clone(&track1); - - // There should be a new request for the track - let mut producer2 = broadcast.assert_request(); + // Subscribe again to the same track — should get a NEW producer. + let sub2 = tokio::spawn({ + let consumer = consumer.clone(); + async move { consumer.subscribe_track("track1", Subscription::default()).await } + }); + tokio::task::yield_now().await; + // Give the cleanup task a tick. + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + tokio::task::yield_now().await; + let request2 = broadcast.assert_request(); + let mut producer2 = request2.accept(Track::new("track1")).unwrap(); producer2.append_group().unwrap(); - // The new consumer should receive the new group + let mut track2 = sub2.await.unwrap().expect("should resolve"); + track2.assert_not_closed(); + track2.assert_not_clone(&track1); track2.assert_group(); } - #[tokio::test] - async fn requested_unused() { - let mut broadcast = Broadcast::new().produce().dynamic(); - - // Subscribe to a track that doesn't exist - this creates a request - let consumer1 = broadcast.consume().assert_subscribe_track(&Track::new("unknown_track")); - - // Get the requested track producer - let producer1 = broadcast.assert_request(); - - // The track producer should NOT be unused yet because there's a consumer - assert!( - producer1.unused().now_or_never().is_none(), - "track producer should be used" - ); - - // Making a new consumer will keep the producer alive - let consumer2 = broadcast.consume().assert_subscribe_track(&Track::new("unknown_track")); - consumer2.assert_is_clone(&consumer1); - - // Drop the consumer subscription - drop(consumer1); - - // The track producer should NOT be unused yet because there's a consumer - assert!( - producer1.unused().now_or_never().is_none(), - "track producer should be used" - ); - - // Drop the second consumer, now the producer should be unused - drop(consumer2); - - // BUG: The track producer should become unused after dropping the consumer, - // but it won't because the broadcast keeps a reference in the lookup HashMap - // This assertion will fail, demonstrating the bug - assert!( - producer1.unused().now_or_never().is_some(), - "track producer should be unused after consumer is dropped" - ); - - // TODO Unfortunately, we need to sleep for a little bit to detect when unused. - tokio::time::sleep(std::time::Duration::from_millis(1)).await; - - // Now the cleanup task should have run and we can subscribe again to the unknown track. - let consumer3 = broadcast.consume().subscribe_track_immediate(&Track::new("unknown_track")); - let producer2 = broadcast.assert_request(); - - // Drop the consumer, now the producer should be unused - drop(consumer3); - assert!( - producer2.unused().now_or_never().is_some(), - "track producer should be unused after consumer is dropped" - ); - } - // Cloning a `BroadcastDynamic` and dropping the clone must not flip // `state.dynamic` to zero. The relay's lite subscriber clones the // dynamic per spawned subscribe; if Clone skipped the increment, the @@ -666,6 +756,16 @@ mod test { drop(clone); // Original handle is still live, so requests must still be accepted. - consumer.assert_subscribe_track(&Track::new("track1")); + let sub = tokio::spawn({ + let consumer = consumer.clone(); + async move { consumer.subscribe_track("track1", Subscription::default()).await } + }); + tokio::task::yield_now().await; + + // The request must still be there. + let mut broadcast = broadcast; + let request = broadcast.assert_request(); + request.accept(Track::new("track1")).unwrap(); + sub.await.unwrap().expect("should resolve"); } } diff --git a/rs/moq-net/src/model/frame.rs b/rs/moq-net/src/model/frame.rs index 9ddc91639..e7fa052db 100644 --- a/rs/moq-net/src/model/frame.rs +++ b/rs/moq-net/src/model/frame.rs @@ -26,20 +26,6 @@ pub struct Frame { } impl Frame { - /// Create a frame header with the given size and an unspecified timestamp. - pub fn new(size: u64) -> Self { - Self { - size, - timestamp: Timestamp::ZERO, - } - } - - /// Set the presentation timestamp. - pub fn with_timestamp(mut self, timestamp: Timestamp) -> Self { - self.timestamp = timestamp; - self - } - /// Create a new producer for the frame. pub fn produce(self) -> FrameProducer { FrameProducer::new(self) @@ -48,25 +34,37 @@ impl Frame { impl From for Frame { fn from(size: usize) -> Self { - Self::new(size as u64) + Self { + size: size as u64, + timestamp: Timestamp::ZERO, + } } } impl From for Frame { fn from(size: u64) -> Self { - Self::new(size) + Self { + size, + timestamp: Timestamp::ZERO, + } } } impl From for Frame { fn from(size: u32) -> Self { - Self::new(size as u64) + Self { + size: size as u64, + timestamp: Timestamp::ZERO, + } } } impl From for Frame { fn from(size: u16) -> Self { - Self::new(size as u64) + Self { + size: size as u64, + timestamp: Timestamp::ZERO, + } } } @@ -458,7 +456,7 @@ mod test { #[test] fn single_chunk_roundtrip() { - let mut producer = Frame::new(5).produce(); + let mut producer = Frame::from(5u64).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.finish().unwrap(); @@ -469,7 +467,7 @@ mod test { #[test] fn multi_chunk_read_all() { - let mut producer = Frame::new(10).produce(); + let mut producer = Frame::from(10u64).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.write(Bytes::from_static(b"world")).unwrap(); producer.finish().unwrap(); @@ -481,7 +479,7 @@ mod test { #[test] fn read_chunk_sequential() { - let mut producer = Frame::new(10).produce(); + let mut producer = Frame::from(10u64).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); // Each read_chunk returns whatever is new since the last call, // which may span multiple writes. @@ -500,7 +498,7 @@ mod test { #[test] fn read_all_chunks() { - let mut producer = Frame::new(10).produce(); + let mut producer = Frame::from(10u64).produce(); producer.write(Bytes::from_static(b"hello")).unwrap(); producer.write(Bytes::from_static(b"world")).unwrap(); producer.finish().unwrap(); @@ -513,7 +511,7 @@ mod test { #[test] fn finish_checks_remaining() { - let mut producer = Frame::new(5).produce(); + let mut producer = Frame::from(5u64).produce(); producer.write(Bytes::from_static(b"hi")).unwrap(); let err = producer.finish().unwrap_err(); assert!(matches!(err, Error::WrongSize)); @@ -521,14 +519,14 @@ mod test { #[test] fn write_too_many_bytes() { - let mut producer = Frame::new(3).produce(); + let mut producer = Frame::from(3u64).produce(); let err = producer.write(Bytes::from_static(b"toolong")).unwrap_err(); assert!(matches!(err, Error::WrongSize)); } #[test] fn abort_propagates() { - let mut producer = Frame::new(5).produce(); + let mut producer = Frame::from(5u64).produce(); let mut consumer = producer.consume(); producer.abort(Error::Cancel).unwrap(); @@ -538,7 +536,7 @@ mod test { #[test] fn empty_frame() { - let mut producer = Frame::new(0).produce(); + let mut producer = Frame::from(0u64).produce(); producer.finish().unwrap(); let mut consumer = producer.consume(); @@ -548,7 +546,7 @@ mod test { #[tokio::test] async fn pending_then_ready() { - let mut producer = Frame::new(5).produce(); + let mut producer = Frame::from(5u64).produce(); let mut consumer = producer.consume(); // Consumer blocks because no data yet. @@ -564,7 +562,7 @@ mod test { #[test] fn buf_mut_roundtrip() { // Exercise the BufMut path that the receive loop uses via `read_buf`. - let mut producer = Frame::new(12).produce(); + let mut producer = Frame::from(12u64).produce(); assert_eq!(producer.remaining_mut(), 12); producer.put_slice(b"hello"); assert_eq!(producer.remaining_mut(), 7); @@ -580,14 +578,14 @@ mod test { #[test] #[should_panic(expected = "advance_mut past frame.size")] fn buf_mut_advance_past_capacity_panics() { - let mut producer = Frame::new(4).produce(); + let mut producer = Frame::from(4u64).produce(); // Safety violation on purpose: cnt > remaining_mut(). unsafe { producer.advance_mut(5) }; } #[test] fn read_chunk_streams_partial_writes() { - let mut producer = Frame::new(6).produce(); + let mut producer = Frame::from(6u64).produce(); let mut consumer = producer.consume(); producer.write(Bytes::from_static(b"foo")).unwrap(); @@ -607,7 +605,7 @@ mod test { #[test] fn cloned_consumer_independent_cursor() { - let mut producer = Frame::new(10).produce(); + let mut producer = Frame::from(10u64).produce(); let mut c1 = producer.consume(); producer.write(Bytes::from_static(b"hello")).unwrap(); diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index 0cb1c3fdb..973e51934 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -166,16 +166,15 @@ impl GroupProducer { /// But an upfront size is required. pub fn write_frame>(&mut self, frame: B) -> Result<()> { let data = frame.into(); - let frame = Frame::new(data.len() as u64); - let mut frame = self.create_frame(frame)?; + let mut frame = self.create_frame(data.len())?; frame.write(data)?; frame.finish()?; Ok(()) } /// Create a frame with an upfront size - pub fn create_frame(&mut self, info: Frame) -> Result { - let frame = info.produce(); + pub fn create_frame(&mut self, info: impl Into) -> Result { + let frame = info.into().produce(); self.append_frame(frame.clone())?; Ok(frame) } @@ -402,7 +401,7 @@ mod test { #[test] fn read_frame_chunks() { let mut producer = Group { sequence: 0 }.produce(); - let mut frame = producer.create_frame(Frame::new(10)).unwrap(); + let mut frame = producer.create_frame(10u64).unwrap(); frame.write(Bytes::from_static(b"hello")).unwrap(); frame.write(Bytes::from_static(b"world")).unwrap(); frame.finish().unwrap(); diff --git a/rs/moq-net/src/model/time.rs b/rs/moq-net/src/model/time.rs index 30a2f9c35..20c8ed083 100644 --- a/rs/moq-net/src/model/time.rs +++ b/rs/moq-net/src/model/time.rs @@ -1,6 +1,5 @@ use rand::Rng; -use crate::Error; use crate::coding::{Decode, DecodeError, Encode, EncodeError, VarInt}; use std::sync::LazyLock; @@ -8,12 +7,109 @@ use std::time::{SystemTime, UNIX_EPOCH}; /// Returned when a [`Timestamp`] operation would exceed the QUIC VarInt range /// (`2^62 - 1`), overflow during scale conversion or arithmetic, hit a divide -/// by zero from an unspecified ([`Timestamp::is_unspecified`]) scale, or -/// attempt arithmetic between timestamps with mismatched scales. +/// by zero from an unspecified ([`Timescale::UNKNOWN`]) scale, or attempt +/// arithmetic between timestamps with mismatched scales. #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] #[error("time overflow")] pub struct TimeOverflow; +/// Units per second used by a track for frame timestamps. +/// +/// Newtype around `u64`. The wire encoding is a plain QUIC varint. +/// Use the named constants ([`Self::SECOND`], [`Self::MILLI`], [`Self::MICRO`], +/// [`Self::NANO`]) instead of writing raw integers at call sites. +/// +/// [`Self::UNKNOWN`] (raw value `0`) denotes an unspecified scale, produced by +/// peers that don't negotiate a timescale (older moq-lite versions, older +/// moq-transport drafts). Unit conversions against an unknown scale return +/// [`TimeOverflow`]. +#[derive(Clone, Copy, Default, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Timescale(pub u64); + +impl Timescale { + /// Unspecified scale. Conversions involving this return [`TimeOverflow`]. + pub const UNKNOWN: Self = Self(0); + /// One unit per second (`1`). + pub const SECOND: Self = Self(1); + /// 1,000 units per second (`1_000`). + pub const MILLI: Self = Self(1_000); + /// 1,000,000 units per second (`1_000_000`). Common default for media tracks. + pub const MICRO: Self = Self(1_000_000); + /// 1,000,000,000 units per second (`1_000_000_000`). + pub const NANO: Self = Self(1_000_000_000); + + /// Construct a timescale from a raw value (units per second). `0` means [`Self::UNKNOWN`]. + pub const fn new(units_per_second: u64) -> Self { + Self(units_per_second) + } + + /// The raw units-per-second value. + pub const fn as_u64(self) -> u64 { + self.0 + } + + /// Whether this is [`Self::UNKNOWN`] (raw value `0`). + pub const fn is_unknown(self) -> bool { + self.0 == 0 + } +} + +impl From for Timescale { + fn from(units_per_second: u64) -> Self { + Self(units_per_second) + } +} + +impl From for u64 { + fn from(scale: Timescale) -> Self { + scale.0 + } +} + +impl std::fmt::Debug for Timescale { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match *self { + Self::UNKNOWN => write!(f, "Timescale::UNKNOWN"), + Self::SECOND => write!(f, "Timescale::SECOND"), + Self::MILLI => write!(f, "Timescale::MILLI"), + Self::MICRO => write!(f, "Timescale::MICRO"), + Self::NANO => write!(f, "Timescale::NANO"), + Self(n) => write!(f, "Timescale({n})"), + } + } +} + +impl std::fmt::Display for Timescale { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Decode for Timescale { + fn decode(r: &mut R, version: crate::Version) -> Result { + Ok(Self(u64::decode(r, version)?)) + } +} + +impl Encode for Timescale { + fn encode(&self, w: &mut W, version: crate::Version) -> Result<(), EncodeError> { + self.0.encode(w, version) + } +} + +impl Decode for Timescale { + fn decode(r: &mut R, version: crate::lite::Version) -> Result { + Ok(Self(u64::decode(r, version)?)) + } +} + +impl Encode for Timescale { + fn encode(&self, w: &mut W, version: crate::lite::Version) -> Result<(), EncodeError> { + self.0.encode(w, version) + } +} + /// A timestamp in a track's timescale (units per second). /// /// All timestamps within a track are relative, so zero for one track is not zero for another. @@ -21,15 +117,15 @@ pub struct TimeOverflow; /// encoded and decoded easily; the scale is carried out-of-band (via [`crate::Track::timescale`]) /// and not serialized per-timestamp. /// -/// `scale == 0` denotes an unspecified timescale, produced by [`Timestamp::ZERO`] and by -/// peers that don't negotiate a timescale (older moq-lite versions, older moq-transport -/// drafts without track properties). Unit conversions and arithmetic against an unspecified -/// scale return [`TimeOverflow`] to avoid divide by zero. +/// [`Timescale::UNKNOWN`] denotes an unspecified timescale, produced by [`Timestamp::ZERO`] +/// and by peers that don't negotiate a timescale (older moq-lite versions, older +/// moq-transport drafts without track properties). Unit conversions and arithmetic against +/// an unknown scale return [`TimeOverflow`] to avoid divide by zero. #[derive(Clone, Default, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Timestamp { value: VarInt, - scale: u64, + scale: Timescale, } impl Timestamp { @@ -40,7 +136,7 @@ impl Timestamp { /// `value > 0`. See [`Self::partial_cmp`]. pub const ZERO: Self = Self { value: VarInt::ZERO, - scale: 0, + scale: Timescale::UNKNOWN, }; /// The maximum representable timestamp value, with an unspecified scale. @@ -49,49 +145,36 @@ impl Timestamp { /// `value < VarInt::MAX`. See [`Self::partial_cmp`]. pub const MAX: Self = Self { value: VarInt::MAX, - scale: 0, + scale: Timescale::UNKNOWN, }; /// Construct a timestamp directly from a raw value at the given scale. - pub const fn new(value: u32, scale: u64) -> Self { - Self { - value: VarInt::from_u32(value), - scale, - } - } - - /// Construct a timestamp from a raw value at the given scale. Returns [`TimeOverflow`] - /// if `value` exceeds the 62-bit varint range. - pub const fn new_u64(value: u64, scale: u64) -> Result { + pub const fn new(value: u64, scale: Timescale) -> Result { match VarInt::from_u64(value) { Some(value) => Ok(Self { value, scale }), None => Err(TimeOverflow), } } - /// Convert `value` measured at the given `scale` (units per second) to a timestamp. + /// Convert `value` measured at `source` (units per second) to a timestamp at `target`. /// - /// The stored scale is `target_scale`. Returns [`TimeOverflow`] on overflow or if - /// `source_scale` is zero. - pub const fn from_scale(value: u64, source_scale: u64, target_scale: u64) -> Result { - if source_scale == 0 { + /// Returns [`TimeOverflow`] on overflow or if `source` is [`Timescale::UNKNOWN`]. + pub const fn from_scale(value: u64, source: Timescale, target: Timescale) -> Result { + if source.0 == 0 { return Err(TimeOverflow); } - match (value as u128).checked_mul(target_scale as u128) { - Some(scaled) => match VarInt::from_u128(scaled / source_scale as u128) { - Some(value) => Ok(Self { - value, - scale: target_scale, - }), + match (value as u128).checked_mul(target.0 as u128) { + Some(scaled) => match VarInt::from_u128(scaled / source.0 as u128) { + Some(value) => Ok(Self { value, scale: target }), None => Err(TimeOverflow), }, None => Err(TimeOverflow), } } - /// Convert a number of seconds to a timestamp with `scale == 1`. + /// Convert a number of seconds to a timestamp at [`Timescale::SECOND`]. pub const fn from_secs(seconds: u64) -> Result { - Self::new_u64(seconds, 1) + Self::new(seconds, Timescale::SECOND) } /// Like [`Self::from_secs`] but panics on overflow. @@ -102,9 +185,9 @@ impl Timestamp { } } - /// Convert a number of milliseconds to a timestamp with `scale == 1_000`. + /// Convert a number of milliseconds to a timestamp at [`Timescale::MILLI`]. pub const fn from_millis(millis: u64) -> Result { - Self::new_u64(millis, 1_000) + Self::new(millis, Timescale::MILLI) } /// Like [`Self::from_millis`] but panics on overflow. @@ -115,9 +198,9 @@ impl Timestamp { } } - /// Convert a number of microseconds to a timestamp with `scale == 1_000_000`. + /// Convert a number of microseconds to a timestamp at [`Timescale::MICRO`]. pub const fn from_micros(micros: u64) -> Result { - Self::new_u64(micros, 1_000_000) + Self::new(micros, Timescale::MICRO) } /// Like [`Self::from_micros`] but panics on overflow. @@ -128,9 +211,9 @@ impl Timestamp { } } - /// Convert a number of nanoseconds to a timestamp with `scale == 1_000_000_000`. + /// Convert a number of nanoseconds to a timestamp at [`Timescale::NANO`]. pub const fn from_nanos(nanos: u64) -> Result { - Self::new_u64(nanos, 1_000_000_000) + Self::new(nanos, Timescale::NANO) } /// Like [`Self::from_nanos`] but panics on overflow. @@ -147,14 +230,14 @@ impl Timestamp { } /// The scale (units per second) attached to this timestamp. - pub const fn scale(self) -> u64 { + pub const fn scale(self) -> Timescale { self.scale } - /// Whether the scale is unset (`scale == 0`). Unit conversions and cross-scale + /// Whether the scale is [`Timescale::UNKNOWN`]. Unit conversions and cross-scale /// arithmetic against this timestamp return [`TimeOverflow`]. pub const fn is_unspecified(self) -> bool { - self.scale == 0 + self.scale.0 == 0 } /// Whether the raw value is zero. Does not consider scale. @@ -163,52 +246,52 @@ impl Timestamp { } /// Re-express this timestamp at a new scale. Returns [`TimeOverflow`] if the new - /// value would exceed `2^62 - 1`, the source scale is unspecified, or - /// `new_scale == 0`. - pub const fn convert(self, new_scale: u64) -> Result { - if self.scale == 0 || new_scale == 0 { + /// value would exceed `2^62 - 1`, the source scale is unspecified, or `new_scale` + /// is [`Timescale::UNKNOWN`]. + pub const fn convert(self, new_scale: Timescale) -> Result { + if self.scale.0 == 0 || new_scale.0 == 0 { return Err(TimeOverflow); } - if self.scale == new_scale { + if self.scale.0 == new_scale.0 { return Ok(self); } Self::from_scale(self.value.into_inner(), self.scale, new_scale) } - /// The value re-expressed at `target_scale` as a `u128`. Returns [`TimeOverflow`] - /// if the source scale is unspecified or `target_scale == 0`. - pub const fn as_scale(self, target_scale: u64) -> Result { - if self.scale == 0 || target_scale == 0 { + /// The value re-expressed at `target` as a `u128`. Returns [`TimeOverflow`] + /// if the source scale is unspecified or `target` is [`Timescale::UNKNOWN`]. + pub const fn as_scale(self, target: Timescale) -> Result { + if self.scale.0 == 0 || target.0 == 0 { return Err(TimeOverflow); } - Ok(self.value.into_inner() as u128 * target_scale as u128 / self.scale as u128) + Ok(self.value.into_inner() as u128 * target.0 as u128 / self.scale.0 as u128) } /// The value re-expressed in seconds. Returns [`TimeOverflow`] if the scale is /// unspecified. pub const fn as_secs(self) -> Result { - if self.scale == 0 { + if self.scale.0 == 0 { return Err(TimeOverflow); } - Ok(self.value.into_inner() / self.scale) + Ok(self.value.into_inner() / self.scale.0) } /// The value re-expressed in milliseconds. Returns [`TimeOverflow`] if the scale /// is unspecified. pub const fn as_millis(self) -> Result { - self.as_scale(1_000) + self.as_scale(Timescale::MILLI) } /// The value re-expressed in microseconds. Returns [`TimeOverflow`] if the scale /// is unspecified. pub const fn as_micros(self) -> Result { - self.as_scale(1_000_000) + self.as_scale(Timescale::MICRO) } /// The value re-expressed in nanoseconds. Returns [`TimeOverflow`] if the scale /// is unspecified. pub const fn as_nanos(self) -> Result { - self.as_scale(1_000_000_000) + self.as_scale(Timescale::NANO) } /// Return the larger of two timestamps. @@ -216,7 +299,7 @@ impl Timestamp { /// Panics if the scales differ. Use [`Self::convert`] first if you need to compare /// across scales. pub const fn max(self, other: Self) -> Self { - assert!(self.scale == other.scale, "mismatched timestamp scales"); + assert!(self.scale.0 == other.scale.0, "mismatched timestamp scales"); if self.value.into_inner() > other.value.into_inner() { self } else { @@ -227,11 +310,11 @@ impl Timestamp { /// Add two timestamps. Returns [`TimeOverflow`] if the sum exceeds `2^62 - 1` or /// if the scales differ. pub const fn checked_add(self, rhs: Self) -> Result { - if self.scale != rhs.scale { + if self.scale.0 != rhs.scale.0 { return Err(TimeOverflow); } match self.value.into_inner().checked_add(rhs.value.into_inner()) { - Some(result) => Self::new_u64(result, self.scale), + Some(result) => Self::new(result, self.scale), None => Err(TimeOverflow), } } @@ -239,11 +322,11 @@ impl Timestamp { /// Subtract `rhs` from `self`. Returns [`TimeOverflow`] if `rhs > self` or if the /// scales differ. pub const fn checked_sub(self, rhs: Self) -> Result { - if self.scale != rhs.scale { + if self.scale.0 != rhs.scale.0 { return Err(TimeOverflow); } match self.value.into_inner().checked_sub(rhs.value.into_inner()) { - Some(result) => Self::new_u64(result, self.scale), + Some(result) => Self::new(result, self.scale), None => Err(TimeOverflow), } } @@ -260,7 +343,10 @@ impl Timestamp { return Err(TimeOverflow); } match VarInt::from_u128(next as u128) { - Some(value) => Ok(Self { value, scale: self.scale }), + Some(value) => Ok(Self { + value, + scale: self.scale, + }), None => Err(TimeOverflow), } } @@ -268,7 +354,7 @@ impl Timestamp { /// The signed delta from `prev` to `self` in their shared scale. Returns /// [`TimeOverflow`] on scale mismatch or if the delta is outside `i64::MIN..=i64::MAX`. pub const fn checked_delta_from(self, prev: Self) -> Result { - if self.scale != prev.scale { + if self.scale.0 != prev.scale.0 { return Err(TimeOverflow); } let a = self.value.into_inner() as i128; @@ -280,24 +366,11 @@ impl Timestamp { Ok(delta as i64) } - /// Current time, expressed in microseconds (`scale == 1_000_000`). Uses + /// Current time, expressed in microseconds ([`Timescale::MICRO`]). Uses /// [`tokio::time::Instant::now`] so it honors `tokio::time::pause` in tests. pub fn now() -> Self { tokio::time::Instant::now().into() } - - /// Encode the raw value as a QUIC varint. Scale is carried out-of-band and is - /// **not** included on the wire. - pub fn encode_value(&self, w: &mut W) -> Result<(), EncodeError> { - self.value.encode(w, crate::lite::Version::Lite01)?; - Ok(()) - } - - /// Decode a raw value as a QUIC varint, attaching the given scale. - pub fn decode_value(r: &mut R, scale: u64) -> Result { - let value = VarInt::decode(r, crate::lite::Version::Lite01)?; - Ok(Self { value, scale }) - } } impl TryFrom for Timestamp { @@ -308,7 +381,7 @@ impl TryFrom for Timestamp { match VarInt::from_u128(duration.as_nanos()) { Some(value) => Ok(Self { value, - scale: 1_000_000_000, + scale: Timescale::NANO, }), None => Err(TimeOverflow), } @@ -328,7 +401,7 @@ impl TryFrom for std::time::Duration { impl std::fmt::Debug for Timestamp { #[allow(clippy::manual_is_multiple_of)] // is_multiple_of is unstable in Rust 1.85 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if self.scale == 0 { + if self.scale.0 == 0 { return write!(f, "{}/?", self.value.into_inner()); } @@ -360,10 +433,10 @@ impl Ord for Timestamp { /// Compare by raw value. Debug-asserts that scales are compatible (same, or /// one is unspecified). In release, cross-scale comparisons return a result /// based on the raw value, which is meaningful only when one side is a - /// scale-`0` sentinel ([`Self::ZERO`] or [`Self::MAX`]). + /// [`Timescale::UNKNOWN`] sentinel ([`Self::ZERO`] or [`Self::MAX`]). fn cmp(&self, other: &Self) -> std::cmp::Ordering { debug_assert!( - self.scale == other.scale || self.scale == 0 || other.scale == 0, + self.scale.0 == other.scale.0 || self.scale.0 == 0 || other.scale.0 == 0, "comparing timestamps with mismatched scales: {} vs {}", self.scale, other.scale, @@ -411,7 +484,7 @@ static TIME_ANCHOR: LazyLock<(std::time::Instant, SystemTime)> = LazyLock::new(| impl From for Timestamp { /// Convert an [`std::time::Instant`] into a microsecond-scale timestamp anchored to a - /// jittered wall-clock reference (see [`TIME_ANCHOR`]). + /// jittered wall-clock reference (see `TIME_ANCHOR`). fn from(instant: std::time::Instant) -> Self { let (anchor_instant, anchor_system) = *TIME_ANCHOR; @@ -434,14 +507,18 @@ impl From for Timestamp { } } -/// Decode a timestamp's raw value as a varint, attaching an unspecified scale. +/// Decode a timestamp's raw value as a varint, attaching [`Timescale::UNKNOWN`]. /// -/// Callers that need a meaningful scale (the track timescale) should use -/// [`Timestamp::decode_value`] directly. +/// Callers that need a meaningful scale should decode the raw value via [`u64::decode`] +/// (or [`crate::coding::VarInt::decode`]) and then call [`Timestamp::new`] with the +/// track's [`Timescale`]. impl Decode for Timestamp { fn decode(r: &mut R, version: crate::Version) -> Result { let value = VarInt::decode(r, version)?; - Ok(Self { value, scale: 0 }) + Ok(Self { + value, + scale: Timescale::UNKNOWN, + }) } } @@ -461,7 +538,7 @@ mod tests { #[test] fn test_from_secs() { let time = Timestamp::from_secs(5).unwrap(); - assert_eq!(time.scale(), 1); + assert_eq!(time.scale(), Timescale::SECOND); assert_eq!(time.as_secs().unwrap(), 5); assert_eq!(time.as_millis().unwrap(), 5000); assert_eq!(time.as_micros().unwrap(), 5_000_000); @@ -471,7 +548,7 @@ mod tests { #[test] fn test_from_millis() { let time = Timestamp::from_millis(5000).unwrap(); - assert_eq!(time.scale(), 1_000); + assert_eq!(time.scale(), Timescale::MILLI); assert_eq!(time.as_secs().unwrap(), 5); assert_eq!(time.as_millis().unwrap(), 5000); } @@ -479,7 +556,7 @@ mod tests { #[test] fn test_from_micros() { let time = Timestamp::from_micros(5_000_000).unwrap(); - assert_eq!(time.scale(), 1_000_000); + assert_eq!(time.scale(), Timescale::MICRO); assert_eq!(time.as_secs().unwrap(), 5); assert_eq!(time.as_micros().unwrap(), 5_000_000); } @@ -487,7 +564,7 @@ mod tests { #[test] fn test_from_nanos() { let time = Timestamp::from_nanos(5_000_000_000).unwrap(); - assert_eq!(time.scale(), 1_000_000_000); + assert_eq!(time.scale(), Timescale::NANO); assert_eq!(time.as_secs().unwrap(), 5); assert_eq!(time.as_nanos().unwrap(), 5_000_000_000); } @@ -514,16 +591,16 @@ mod tests { #[test] fn test_convert_to_finer() { let time_ms = Timestamp::from_millis(5000).unwrap(); - let time_us = time_ms.convert(1_000_000).unwrap(); - assert_eq!(time_us.scale(), 1_000_000); + let time_us = time_ms.convert(Timescale::MICRO).unwrap(); + assert_eq!(time_us.scale(), Timescale::MICRO); assert_eq!(time_us.as_micros().unwrap(), 5_000_000); } #[test] fn test_convert_to_coarser() { let time_ms = Timestamp::from_millis(5000).unwrap(); - let time_s = time_ms.convert(1).unwrap(); - assert_eq!(time_s.scale(), 1); + let time_s = time_ms.convert(Timescale::SECOND).unwrap(); + assert_eq!(time_s.scale(), Timescale::SECOND); assert_eq!(time_s.as_secs().unwrap(), 5); } @@ -531,15 +608,15 @@ mod tests { fn test_convert_precision_loss() { // 1234 ms = 1.234 s, rounds down to 1 s let time_ms = Timestamp::from_millis(1234).unwrap(); - let time_s = time_ms.convert(1).unwrap(); + let time_s = time_ms.convert(Timescale::SECOND).unwrap(); assert_eq!(time_s.as_secs().unwrap(), 1); } #[test] fn test_convert_roundtrip() { let original = Timestamp::from_millis(5000).unwrap(); - let as_micros = original.convert(1_000_000).unwrap(); - let back = as_micros.convert(1_000).unwrap(); + let as_micros = original.convert(Timescale::MICRO).unwrap(); + let back = as_micros.convert(Timescale::MILLI).unwrap(); assert_eq!(original.value(), back.value()); assert_eq!(original.scale(), back.scale()); } @@ -547,17 +624,17 @@ mod tests { #[test] fn test_convert_same_scale() { let time = Timestamp::from_millis(5000).unwrap(); - let converted = time.convert(1_000).unwrap(); + let converted = time.convert(Timescale::MILLI).unwrap(); assert_eq!(time, converted); } #[test] fn test_convert_unspecified_rejected() { let zero = Timestamp::ZERO; - assert!(zero.convert(1_000).is_err()); + assert!(zero.convert(Timescale::MILLI).is_err()); let time = Timestamp::from_millis(5).unwrap(); - assert!(time.convert(0).is_err()); + assert!(time.convert(Timescale::UNKNOWN).is_err()); } #[test] @@ -566,7 +643,7 @@ mod tests { let b = Timestamp::from_millis(2000).unwrap(); let c = a.checked_add(b).unwrap(); assert_eq!(c.as_millis().unwrap(), 3000); - assert_eq!(c.scale(), 1_000); + assert_eq!(c.scale(), Timescale::MILLI); } #[test] @@ -662,7 +739,7 @@ mod tests { fn test_duration_conversion() { let duration = std::time::Duration::from_secs(5); let time: Timestamp = duration.try_into().unwrap(); - assert_eq!(time.scale(), 1_000_000_000); + assert_eq!(time.scale(), Timescale::NANO); assert_eq!(time.as_secs().unwrap(), 5); let duration_back: std::time::Duration = time.try_into().unwrap(); @@ -689,22 +766,22 @@ mod tests { #[test] fn test_new() { - let t = Timestamp::new(5000, 1_000); + let t = Timestamp::new(5000, Timescale::MILLI).unwrap(); assert_eq!(t.value(), 5000); - assert_eq!(t.scale(), 1_000); + assert_eq!(t.scale(), Timescale::MILLI); assert_eq!(t.as_millis().unwrap(), 5000); } #[test] fn test_from_scale_custom() { // 120 units at 60Hz = 2 seconds, expressed at 1000Hz = 2000 ms. - let t = Timestamp::from_scale(120, 60, 1_000).unwrap(); - assert_eq!(t.scale(), 1_000); + let t = Timestamp::from_scale(120, Timescale::new(60), Timescale::MILLI).unwrap(); + assert_eq!(t.scale(), Timescale::MILLI); assert_eq!(t.as_millis().unwrap(), 2000); } #[test] fn test_from_scale_zero_source() { - assert!(Timestamp::from_scale(5, 0, 1_000).is_err()); + assert!(Timestamp::from_scale(5, Timescale::UNKNOWN, Timescale::MILLI).is_err()); } } diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 299c71438..6646ac8b9 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -12,12 +12,13 @@ //! //! The track is closed with [Error] when all writers or readers are dropped. -use crate::{Error, Result, coding}; +use crate::{Error, Result, Timescale, coding}; use super::{Group, GroupConsumer, GroupProducer}; use std::{ collections::{HashSet, VecDeque}, + sync::{Arc, Mutex}, task::{Poll, ready}, time::Duration, }; @@ -26,7 +27,12 @@ use std::{ // TODO: Replace with a configurable cache size. const MAX_GROUP_AGE: Duration = Duration::from_secs(5); -/// A track is a collection of groups, delivered out-of-order until expired. +/// Publisher-side properties of a track. +/// +/// These properties are fixed by the publisher when the track is created and +/// do not change while the track is alive. Subscribers learn them via +/// [`BroadcastConsumer::subscribe_track`](crate::BroadcastConsumer::subscribe_track), +/// which returns once the publisher's response has been received. #[derive(Clone, Debug, Default, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Track { @@ -34,44 +40,53 @@ pub struct Track { pub name: String, /// Delivery priority. Higher values preempt lower ones when bandwidth is constrained. pub priority: u8, - /// Units per second for frame timestamps on this track. - /// - /// `0` (the default) means unspecified, typically because the peer didn't negotiate - /// a timescale (older moq-lite or older moq-transport drafts) or none has been - /// learned yet. Producers set this before publishing; subscribers learn it from - /// [`crate::BroadcastConsumer::subscribe_track`] once SUBSCRIBE_OK arrives. - pub timescale: u64, + /// Units per second for frame timestamps on this track. [`Timescale::UNKNOWN`] + /// means the publisher hasn't specified a scale (older protocol versions). + pub timescale: Timescale, } impl Track { - /// Create a track with the given name, default priority (`0`), and unspecified - /// timescale. + /// Create a track with the given name, default priority (`0`), and unknown timescale. pub fn new>(name: T) -> Self { Self { name: name.into(), priority: 0, - timescale: 0, + timescale: Timescale::UNKNOWN, } } - /// Set the delivery priority. Higher values preempt lower ones. - pub fn with_priority(mut self, priority: u8) -> Self { - self.priority = priority; - self - } - - /// Set the per-track timescale (units per second). - pub fn with_timescale(mut self, timescale: u64) -> Self { - self.timescale = timescale; - self - } - /// Consume this [`Track`] to create a producer that owns its metadata. pub fn produce(self) -> TrackProducer { TrackProducer::new(self) } } +/// Subscriber-side preferences for receiving a track. +/// +/// Held by each subscriber and aggregated across all live subscribers by the +/// publisher's [`TrackProducer`] (see [`TrackProducer::max_priority`] / +/// [`TrackProducer::max_timeout`]). The publisher can react to the aggregate +/// to adapt delivery (priority bumping, cancelling tracks that no one wants +/// urgently, etc). +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct Subscription { + /// Delivery priority requested by this subscriber. + pub priority: u8, + /// Maximum delay before this subscriber considers a group stale. + /// `Duration::ZERO` means no timeout (the subscriber will wait forever). + pub timeout: Duration, +} + +impl Subscription { + /// Create a subscription with the given priority and no timeout. + pub fn new(priority: u8) -> Self { + Self { + priority, + timeout: Duration::ZERO, + } + } +} + #[derive(Default)] struct State { /// Groups in arrival order. `None` entries are tombstones for evicted groups. @@ -81,10 +96,11 @@ struct State { max_sequence: Option, final_sequence: Option, abort: Option, - /// Track timescale once negotiated via SUBSCRIBE_OK / track properties. - /// `None` means not yet resolved; consumers awaiting [`Self::poll_timescale`] - /// will block. - timescale: Option, + /// Active subscriptions, used by the producer to aggregate + /// [`TrackProducer::max_priority`] and [`TrackProducer::max_timeout`]. + /// `Arc>` so individual subscribers can update their + /// preferences in place. + subscriptions: Vec>>, } impl State { @@ -225,19 +241,14 @@ impl State { Poll::Pending } } - - fn poll_timescale(&self) -> Poll> { - if let Some(scale) = self.timescale { - Poll::Ready(Ok(scale)) - } else if let Some(err) = &self.abort { - Poll::Ready(Err(err.clone())) - } else { - Poll::Pending - } - } } /// A producer for a track, used to create new groups. +/// +/// The publisher's [`Track`] properties (name, priority, timescale) are fixed +/// at construction and cannot be mutated through this handle. Subscribers see +/// these properties on the [`TrackConsumer`] returned from +/// [`BroadcastConsumer::subscribe_track`](crate::BroadcastConsumer::subscribe_track). pub struct TrackProducer { info: Track, state: conducer::Producer, @@ -254,13 +265,10 @@ impl std::ops::Deref for TrackProducer { impl TrackProducer { /// Build a producer for the given track metadata. Prefer [`Track::produce`]. pub fn new(info: Track) -> Self { - let state: conducer::Producer = conducer::Producer::default(); - if info.timescale != 0 { - if let Ok(mut s) = state.write() { - s.timescale = Some(info.timescale); - } + Self { + info, + state: conducer::Producer::default(), } - Self { info, state } } /// Create a new group with the given sequence number. @@ -318,19 +326,6 @@ impl TrackProducer { Ok(()) } - /// Update the track's negotiated timescale. - /// - /// Subscribers call this once SUBSCRIBE_OK arrives with the publisher's - /// timescale, since the [`TrackProducer`] was created before the value was - /// known. Existing [`TrackConsumer`]s see the updated timescale via - /// [`TrackConsumer::timescale`]. - pub fn set_timescale(&mut self, timescale: u64) { - self.info.timescale = timescale; - if let Ok(mut state) = self.state.write() { - state.timescale = Some(timescale); - } - } - /// Mark the track as finished after the last appended group. /// /// Sets the final sequence to one past the current max_sequence. @@ -376,17 +371,61 @@ impl TrackProducer { Ok(()) } - /// Create a new consumer for the track, starting at the beginning. - pub fn consume(&self) -> TrackConsumer { + /// Create a new consumer for the track, starting at the beginning, with the + /// given subscriber-side preferences. The publisher can observe an aggregate + /// of every consumer's preferences via [`Self::max_priority`] and + /// [`Self::max_timeout`]. + pub fn consume_with(&self, subscription: Subscription) -> TrackConsumer { + let sub = Arc::new(Mutex::new(subscription)); + if let Ok(mut state) = self.state.write() { + state.subscriptions.push(sub.clone()); + } TrackConsumer { info: self.info.clone(), state: self.state.consume(), + subscription: sub, index: 0, min_sequence: 0, next_sequence: 0, } } + /// Create a new consumer with the default ([`Subscription::default`]) preferences. + pub fn consume(&self) -> TrackConsumer { + self.consume_with(Subscription::default()) + } + + /// The highest [`Subscription::priority`] across all live consumers, or `0` + /// if there are none. + pub fn max_priority(&self) -> u8 { + let state = self.state.read(); + state + .subscriptions + .iter() + .filter_map(|s| s.lock().ok().map(|s| s.priority)) + .max() + .unwrap_or(0) + } + + /// The largest [`Subscription::timeout`] across all live consumers, or + /// `Duration::ZERO` if there are none. `Duration::ZERO` means at least + /// one consumer requested no timeout, which dominates. + pub fn max_timeout(&self) -> Duration { + let state = self.state.read(); + state + .subscriptions + .iter() + .filter_map(|s| s.lock().ok().map(|s| s.timeout)) + .reduce(|a, b| { + if a.is_zero() || b.is_zero() { + Duration::ZERO + } else { + a.max(b) + } + }) + .unwrap_or(Duration::ZERO) + } + /// Block until there are no active consumers. pub async fn unused(&self) -> Result<()> { self.state @@ -461,16 +500,29 @@ impl TrackWeak { self.state.is_closed() } - pub fn consume(&self) -> TrackConsumer { + pub fn consume_with(&self, subscription: Subscription) -> TrackConsumer { + let sub = Arc::new(Mutex::new(subscription)); + // Register the subscription if we can still grab a producer handle. + if let Some(producer) = self.state.produce() + && let Ok(mut state) = producer.write() + { + state.subscriptions.push(sub.clone()); + } TrackConsumer { info: self.info.clone(), state: self.state.consume(), + subscription: sub, index: 0, min_sequence: 0, next_sequence: 0, } } + #[allow(dead_code)] + pub fn consume(&self) -> TrackConsumer { + self.consume_with(Subscription::default()) + } + pub async fn unused(&self) -> crate::Result<()> { self.state .unused() @@ -488,6 +540,9 @@ impl TrackWeak { pub struct TrackConsumer { info: Track, state: conducer::Consumer, + /// This consumer's subscription, shared with the producer's aggregate via + /// [`State::subscriptions`]. + subscription: Arc>, /// Arrival-order cursor used by [`Self::recv_group`]. index: usize, /// Minimum sequence to return from any `recv` method. Set by [`Self::start_at`]. @@ -518,6 +573,20 @@ impl TrackConsumer { }) } + /// Update this consumer's subscription preferences. The publisher's + /// [`TrackProducer::max_priority`] / [`TrackProducer::max_timeout`] reflect + /// the new value on the next read. + pub fn update_subscription(&self, subscription: Subscription) { + if let Ok(mut s) = self.subscription.lock() { + *s = subscription; + } + } + + /// Snapshot this consumer's current subscription. + pub fn subscription(&self) -> Subscription { + self.subscription.lock().map(|s| s.clone()).unwrap_or_default() + } + /// Poll for the next group in arrival order, without blocking. /// /// Returns every group exactly once in the order it landed on the wire — which may be @@ -652,24 +721,8 @@ impl TrackConsumer { self.state.read().max_sequence } - /// Return the resolved timescale (units per second) if one has been negotiated, - /// otherwise `None`. Use [`Self::timescale`] to wait until the value is known. - pub fn timescale_now(&self) -> Option { - self.state.read().timescale - } - - /// Poll for the negotiated timescale, without blocking. - pub fn poll_timescale(&self, waiter: &conducer::Waiter) -> Poll> { - self.poll(waiter, |state| state.poll_timescale()) - } - - /// Wait until the track's timescale has been negotiated (e.g. via SUBSCRIBE_OK - /// for subscribers, or set immediately by publishers). - pub async fn timescale(&self) -> Result { - conducer::wait(|waiter| self.poll_timescale(waiter)).await - } - /// Create a weak reference that doesn't prevent auto-close. + #[allow(dead_code)] pub(crate) fn weak(&self) -> TrackWeak { TrackWeak { info: self.info.clone(), @@ -1242,4 +1295,54 @@ mod test { assert!(matches!(producer.append_group(), Err(Error::BoundsExceeded(_)))); } + + #[test] + fn max_priority_aggregates_subscriptions() { + let producer = Track::new("test").produce(); + let _c1 = producer.consume_with(Subscription::new(3)); + let _c2 = producer.consume_with(Subscription::new(7)); + let _c3 = producer.consume_with(Subscription::new(5)); + + assert_eq!(producer.max_priority(), 7); + } + + #[test] + fn max_timeout_zero_dominates() { + let producer = Track::new("test").produce(); + let _c1 = producer.consume_with(Subscription { + priority: 0, + timeout: Duration::from_secs(1), + }); + let _c2 = producer.consume_with(Subscription { + priority: 0, + timeout: Duration::ZERO, + }); + // Duration::ZERO (no timeout) dominates the aggregate. + assert_eq!(producer.max_timeout(), Duration::ZERO); + } + + #[test] + fn max_timeout_picks_largest_finite() { + let producer = Track::new("test").produce(); + let _c1 = producer.consume_with(Subscription { + priority: 0, + timeout: Duration::from_secs(2), + }); + let _c2 = producer.consume_with(Subscription { + priority: 0, + timeout: Duration::from_secs(5), + }); + assert_eq!(producer.max_timeout(), Duration::from_secs(5)); + } + + #[test] + fn update_subscription_changes_aggregate() { + let producer = Track::new("test").produce(); + let c1 = producer.consume_with(Subscription::new(2)); + let _c2 = producer.consume_with(Subscription::new(4)); + assert_eq!(producer.max_priority(), 4); + + c1.update_subscription(Subscription::new(9)); + assert_eq!(producer.max_priority(), 9); + } } diff --git a/rs/moq-relay/src/web.rs b/rs/moq-relay/src/web.rs index 3b39b612e..9bbd0e74e 100644 --- a/rs/moq-relay/src/web.rs +++ b/rs/moq-relay/src/web.rs @@ -506,12 +506,6 @@ async fn serve_fetch( tracing::info!(%broadcast, %track, "fetching track"); - let track = moq_net::Track { - name: track, - priority: 0, - timescale: 0, - }; - let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_secs(30); let result = tokio::time::timeout_at(deadline, async { @@ -519,10 +513,13 @@ async fn serve_fetch( // Block until the broadcast has been announced (within the fetch deadline) so // freshly-connected subscribers don't get a spurious 404 before gossip arrives. let broadcast = origin.announced_broadcast("").await.ok_or(StatusCode::NOT_FOUND)?; - let mut track = broadcast.subscribe_track(&track).await.map_err(|err| match err { - moq_net::Error::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - })?; + let mut track = broadcast + .subscribe_track(&track, moq_net::Subscription::default()) + .await + .map_err(|err| match err { + moq_net::Error::NotFound => StatusCode::NOT_FOUND, + _ => StatusCode::INTERNAL_SERVER_ERROR, + })?; let group = match params.group { FetchGroup::Latest => match track.latest() { Some(sequence) => track.get_group(sequence).await, From 69b8c2b406315c92fae4842b921d2e63b858bff0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 23 May 2026 10:06:14 -0700 Subject: [PATCH 07/10] moq-gst: update subscribe_track call sites and timestamp delta Adapt the gstreamer source plugin to the new `subscribe_track` signature (`&str` name + `Subscription`, async) and the new `Timestamp` arithmetic API where `Timestamp - Timestamp` is no longer infallible and `Into` isn't implemented. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/moq-gst/src/source/imp.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/rs/moq-gst/src/source/imp.rs b/rs/moq-gst/src/source/imp.rs index 9da9f6d9d..49389fc60 100644 --- a/rs/moq-gst/src/source/imp.rs +++ b/rs/moq-gst/src/source/imp.rs @@ -432,7 +432,9 @@ async fn run_session( _ = shutdown.changed() => return Ok(()), }; - let catalog_track = broadcast.subscribe_track(&hang::catalog::Catalog::default_track())?; + let catalog_track = broadcast + .subscribe_track(hang::catalog::Catalog::DEFAULT_NAME, moq_net::Subscription::default()) + .await?; let mut catalog = moq_mux::catalog::Consumer::new(catalog_track); let catalog = catalog.next().await?.context("catalog missing")?.clone(); @@ -445,8 +447,9 @@ async fn run_session( }; let caps = video_caps(&config)?; let endpoint = request_pad(&control_tx, descriptor.clone(), caps).await?; - let track_ref = moq_net::Track::new(&track_name); - let track_consumer = broadcast.subscribe_track(&track_ref)?; + let track_consumer = broadcast + .subscribe_track(&track_name, moq_net::Subscription::default()) + .await?; let track = moq_mux::container::Consumer::new(track_consumer, moq_mux::container::Hang::Legacy) .with_latency(Duration::from_secs(1)); tasks.push(spawn_track_pump(track, descriptor, endpoint, shutdown.clone())); @@ -459,8 +462,9 @@ async fn run_session( }; let caps = audio_caps(&config)?; let endpoint = request_pad(&control_tx, descriptor.clone(), caps).await?; - let track_ref = moq_net::Track::new(&track_name); - let track_consumer = broadcast.subscribe_track(&track_ref)?; + let track_consumer = broadcast + .subscribe_track(&track_name, moq_net::Subscription::default()) + .await?; let track = moq_mux::container::Consumer::new(track_consumer, moq_mux::container::Hang::Legacy) .with_latency(Duration::from_secs(1)); tasks.push(spawn_track_pump(track, descriptor, endpoint, shutdown.clone())); @@ -525,10 +529,14 @@ async fn run_track_pump( let buffer_mut = buffer.get_mut().unwrap(); let pts = match reference_ts { - Some(reference) => { - let delta: Duration = (timestamp - reference).into(); - gst::ClockTime::from_nseconds(delta.as_nanos() as u64) - } + Some(reference) => match timestamp.checked_sub(reference).and_then(|d| d.as_nanos()) { + Ok(nanos) => gst::ClockTime::from_nseconds(nanos as u64), + Err(_) => { + gst::warning!(CAT, "track {} timestamp delta overflow", descriptor.name); + pad_endpoint.send(PadMessage::Drop); + break; + } + }, None => { reference_ts = Some(timestamp); gst::ClockTime::ZERO From c5f98987aa285826dec829bc9823930352b08d9b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 23 May 2026 10:24:58 -0700 Subject: [PATCH 08/10] py: await subscribe_* calls now that the FFI methods are async The underlying MoqBroadcastConsumer.subscribe_catalog/track/media methods became async (returning coroutines) when the Rust API was made async to wait for SUBSCRIBE_OK before returning a TrackConsumer. Propagate the change through the BroadcastConsumer wrapper, tests, example, and README. Co-Authored-By: Claude Opus 4.7 (1M context) --- py/moq-rs/README.md | 3 ++- py/moq-rs/examples/clock.py | 2 +- py/moq-rs/moq/subscribe.py | 16 ++++++++-------- py/moq-rs/tests/test_local.py | 20 ++++++++++---------- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/py/moq-rs/README.md b/py/moq-rs/README.md index c979602ad..25b243051 100644 --- a/py/moq-rs/README.md +++ b/py/moq-rs/README.md @@ -26,7 +26,8 @@ async def main(): catalog = await announcement.broadcast.catalog() for name in catalog.audio: - async for frame in announcement.broadcast.subscribe_media(name): + media = await announcement.broadcast.subscribe_media(name) + async for frame in media: print(f"Got frame: {len(frame.payload)} bytes, ts={frame.timestamp_us}") asyncio.run(main()) diff --git a/py/moq-rs/examples/clock.py b/py/moq-rs/examples/clock.py index 5a3ad2afa..f8503089c 100644 --- a/py/moq-rs/examples/clock.py +++ b/py/moq-rs/examples/clock.py @@ -43,7 +43,7 @@ async def subscribe(url: str, broadcast_name: str, track_name: str, tls_verify: broadcast = await client.announced_broadcast(broadcast_name) print(f"subscribed to {broadcast_name!r} track={track_name!r}") - track = broadcast.subscribe_track(track_name) + track = await broadcast.subscribe_track(track_name) async for group in track: prefix: bytes | None = None diff --git a/py/moq-rs/moq/subscribe.py b/py/moq-rs/moq/subscribe.py index bb6313a75..b0ec36ca8 100644 --- a/py/moq-rs/moq/subscribe.py +++ b/py/moq-rs/moq/subscribe.py @@ -135,17 +135,17 @@ class BroadcastConsumer: def __init__(self, inner: MoqBroadcastConsumer) -> None: self._inner = inner - def subscribe_catalog(self) -> CatalogConsumer: - return CatalogConsumer(self._inner.subscribe_catalog()) + async def subscribe_catalog(self) -> CatalogConsumer: + return CatalogConsumer(await self._inner.subscribe_catalog()) - def subscribe_track(self, name: str) -> TrackConsumer: - """Subscribe to a track — receive arbitrary byte payloads.""" - return TrackConsumer(self._inner.subscribe_track(name)) + async def subscribe_track(self, name: str) -> TrackConsumer: + """Subscribe to a track. Receive arbitrary byte payloads.""" + return TrackConsumer(await self._inner.subscribe_track(name)) - def subscribe_media(self, name: str, container: Container, max_latency_ms: int) -> MediaConsumer: - return MediaConsumer(self._inner.subscribe_media(name, container, max_latency_ms)) + async def subscribe_media(self, name: str, container: Container, max_latency_ms: int) -> MediaConsumer: + return MediaConsumer(await self._inner.subscribe_media(name, container, max_latency_ms)) async def catalog(self) -> Catalog: """Convenience: subscribe and return the first catalog.""" - consumer = self.subscribe_catalog() + consumer = await self.subscribe_catalog() return await anext(consumer) diff --git a/py/moq-rs/tests/test_local.py b/py/moq-rs/tests/test_local.py index 7098eed07..f6ec9b607 100644 --- a/py/moq-rs/tests/test_local.py +++ b/py/moq-rs/tests/test_local.py @@ -110,7 +110,7 @@ async def test_local_publish_consume_audio(): assert audio.sample_rate == 48000 assert audio.channel_count == 2 - media_consumer = announcement.broadcast.subscribe_media(track_name, audio.container, 10_000) + media_consumer = await announcement.broadcast.subscribe_media(track_name, audio.container, 10_000) payload = b"opus audio payload data" media.write_frame(payload, 1_000_000) @@ -144,7 +144,7 @@ async def test_video_publish_consume(): assert video.coded.width == 1280 assert video.coded.height == 720 - media_consumer = announcement.broadcast.subscribe_media(track_name, video.container, 10_000) + media_consumer = await announcement.broadcast.subscribe_media(track_name, video.container, 10_000) keyframe = bytes([0x00, 0x00, 0x00, 0x01, 0x65, 0xAA, 0xBB, 0xCC]) media.write_frame(keyframe, 0) @@ -169,7 +169,7 @@ async def test_multiple_frames_ordering(): catalog = await announcement.broadcast.catalog() track_name = list(catalog.audio.keys())[0] audio = catalog.audio[track_name] - media_consumer = announcement.broadcast.subscribe_media(track_name, audio.container, 10_000) + media_consumer = await announcement.broadcast.subscribe_media(track_name, audio.container, 10_000) timestamps = [0, 20_000, 40_000, 60_000, 80_000] for i, ts in enumerate(timestamps): @@ -193,7 +193,7 @@ async def test_catalog_update_on_new_track(): consumer = origin.consume() async for announcement in consumer.announced(): - cat_consumer = announcement.broadcast.subscribe_catalog() + cat_consumer = await announcement.broadcast.subscribe_catalog() # First catalog: 1 audio track. catalog1 = await anext(cat_consumer) @@ -226,7 +226,7 @@ async def test_announced_broadcast(): async for announcement in consumer.announced(): assert announcement.path == "test/broadcast" - _catalog = announcement.broadcast.subscribe_catalog() + _catalog = await announcement.broadcast.subscribe_catalog() break @@ -334,7 +334,7 @@ async def test_raw_publish_consume(): async for announcement in consumer.announced(): assert announcement.path == "robot/arm" - raw_consumer = announcement.broadcast.subscribe_track("events") + raw_consumer = await announcement.broadcast.subscribe_track("events") payload = b'{"cmd": "button_changed", "arm": "left", "button": "THUMB", "state": "PRESSED"}' raw.write_frame(payload) @@ -357,7 +357,7 @@ async def test_raw_multiple_frames(): consumer = origin.consume() async for announcement in consumer.announced(): - raw_consumer = announcement.broadcast.subscribe_track("commands") + raw_consumer = await announcement.broadcast.subscribe_track("commands") messages = [ b'{"cmd": "led", "arm": "left", "led": "THUMB", "state": 1}', @@ -419,7 +419,7 @@ async def test_broadcast_producer_consume_direct(): raw = broadcast.publish_track("events") consumer = broadcast.consume() - raw_consumer = consumer.subscribe_track("events") + raw_consumer = await consumer.subscribe_track("events") raw.write_frame(b"event-0") async for group in raw_consumer: @@ -439,7 +439,7 @@ async def test_raw_group_sequence(): consumer = origin.consume() async for announcement in consumer.announced(): - raw_consumer = announcement.broadcast.subscribe_track("seq") + raw_consumer = await announcement.broadcast.subscribe_track("seq") sent_sequences = [] for i in range(3): @@ -470,7 +470,7 @@ async def test_raw_multi_frame_group(): consumer = origin.consume() async for announcement in consumer.announced(): - raw_consumer = announcement.broadcast.subscribe_track("chunks") + raw_consumer = await announcement.broadcast.subscribe_track("chunks") group_producer = raw.append_group() chunks = [b"chunk-0", b"chunk-1", b"chunk-2"] From 3e0b0935b80eb97b2d8d7f74dc661fbb679b50c3 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 23 May 2026 13:09:35 -0700 Subject: [PATCH 09/10] moq-net: handle ALPN_LITE_05 in client and server ALPN dispatch Adding `moq-lite-05` to `version::ALPNS` shipped the wire identifier to peers, but `Client::connect` / `Server::accept` never grew a match arm for it. A connection that negotiated `moq-lite-05` therefore fell through to `UnknownAlpn`, which is what the WebTransport ALPN integration test surfaced as `unknown ALPN: moq-lite-05`. Add the missing arms so Lite05 dispatches through the same Lite session plumbing as 04/03. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/moq-net/src/client.rs | 18 +++++++++++++++++- rs/moq-net/src/server.rs | 18 +++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/rs/moq-net/src/client.rs b/rs/moq-net/src/client.rs index 3f8340a0e..7cc70828f 100644 --- a/rs/moq-net/src/client.rs +++ b/rs/moq-net/src/client.rs @@ -1,5 +1,5 @@ use crate::{ - ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, Error, NEGOTIATED, + ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, NEGOTIATED, OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, coding::{self, Decode, Encode, Stream}, ietf, lite, setup, @@ -120,6 +120,22 @@ impl Client { .ok_or(Error::Version)?; (v, v.into()) } + Some(ALPN_LITE_05) => { + self.versions + .select(Version::Lite(lite::Version::Lite05)) + .ok_or(Error::Version)?; + + let recv_bw = lite::start( + session.clone(), + None, + self.publish.clone(), + self.consume.clone(), + self.stats.clone(), + lite::Version::Lite05, + )?; + + return Ok(Session::new(session, lite::Version::Lite05.into(), recv_bw)); + } Some(ALPN_LITE_04) => { self.versions .select(Version::Lite(lite::Version::Lite04)) diff --git a/rs/moq-net/src/server.rs b/rs/moq-net/src/server.rs index 08b23d9aa..f90ed7b66 100644 --- a/rs/moq-net/src/server.rs +++ b/rs/moq-net/src/server.rs @@ -1,5 +1,5 @@ use crate::{ - ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, Error, NEGOTIATED, + ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, NEGOTIATED, OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, coding::{Decode, Encode, Stream}, ietf, lite, setup, @@ -119,6 +119,22 @@ impl Server { .ok_or(Error::Version)?; (v, v.into()) } + Some(ALPN_LITE_05) => { + self.versions + .select(Version::Lite(lite::Version::Lite05)) + .ok_or(Error::Version)?; + + let recv_bw = lite::start( + session.clone(), + None, + self.publish.clone(), + self.consume.clone(), + self.stats.clone(), + lite::Version::Lite05, + )?; + + return Ok(Session::new(session, lite::Version::Lite05.into(), recv_bw)); + } Some(ALPN_LITE_04) => { self.versions .select(Version::Lite(lite::Version::Lite04)) From 871a18609264f74954fbac79b6f6214ff5a144e6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 23 May 2026 16:30:26 -0700 Subject: [PATCH 10/10] moq-net: gate Lite05 behind a WIP ALPN until the wire format is finalized The Lite05 wire format is still in flux. Letting clients pick it up by default risks two peers settling on `moq-lite-05` and then disagreeing about what bytes belong on the wire once we finalize the draft. - Rename the ALPN to `moq-lite-05-wip` (constant kept as `ALPN_LITE_05`). - Drop the wip ALPN from the public `ALPNS` list so unmodified callers don't advertise it during negotiation. - Drop Lite05 from `Versions::all()` for the same reason. - Update `Display` / `FromStr` so the round-trip uses the `-wip` suffix. Callers that explicitly opt in (`client.version = [moq-lite-05-wip]`) still negotiate it via the existing ALPN match arms in `Client::connect` and `Server::accept`. Once the format is finalized we'll rename the constant to `moq-lite-05` and re-add it to `ALPNS` / `Versions::all()`. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/moq-net/src/client.rs | 4 ++-- rs/moq-net/src/lite/version.rs | 5 ++++- rs/moq-net/src/server.rs | 4 ++-- rs/moq-net/src/version.rs | 22 ++++++++++++++++++---- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/rs/moq-net/src/client.rs b/rs/moq-net/src/client.rs index 7cc70828f..660a141ff 100644 --- a/rs/moq-net/src/client.rs +++ b/rs/moq-net/src/client.rs @@ -1,6 +1,6 @@ use crate::{ - ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, NEGOTIATED, - OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, + ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, + NEGOTIATED, OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, coding::{self, Decode, Encode, Stream}, ietf, lite, setup, }; diff --git a/rs/moq-net/src/lite/version.rs b/rs/moq-net/src/lite/version.rs index 1d9e416c1..b3602f711 100644 --- a/rs/moq-net/src/lite/version.rs +++ b/rs/moq-net/src/lite/version.rs @@ -32,7 +32,10 @@ impl fmt::Display for Version { Self::Lite02 => write!(f, "moq-lite-02"), Self::Lite03 => write!(f, "moq-lite-03"), Self::Lite04 => write!(f, "moq-lite-04"), - Self::Lite05 => write!(f, "moq-lite-05"), + // Mirrors `ALPN_LITE_05`: kept distinct from the eventual stable + // `moq-lite-05` identifier so peers parsing this string don't pick + // up the unfinalized wire format by accident. + Self::Lite05 => write!(f, "moq-lite-05-wip"), } } } diff --git a/rs/moq-net/src/server.rs b/rs/moq-net/src/server.rs index f90ed7b66..e68423276 100644 --- a/rs/moq-net/src/server.rs +++ b/rs/moq-net/src/server.rs @@ -1,6 +1,6 @@ use crate::{ - ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, NEGOTIATED, - OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, + ALPN_14, ALPN_15, ALPN_16, ALPN_17, ALPN_18, ALPN_LITE, ALPN_LITE_03, ALPN_LITE_04, ALPN_LITE_05, Error, + NEGOTIATED, OriginConsumer, OriginProducer, Session, StatsHandle, Version, Versions, coding::{Decode, Encode, Stream}, ietf, lite, setup, }; diff --git a/rs/moq-net/src/version.rs b/rs/moq-net/src/version.rs index f245b125f..9c226e37f 100644 --- a/rs/moq-net/src/version.rs +++ b/rs/moq-net/src/version.rs @@ -15,8 +15,14 @@ pub(crate) const NEGOTIATED: [Version; 3] = [ ]; /// ALPN strings for supported versions. +/// +/// `ALPN_LITE_05` (`moq-lite-05-wip`) is intentionally **not** in this list: +/// Lite05 is still in development and we don't want a client to accidentally +/// pick it during ALPN negotiation. Peers that explicitly opt in (e.g. by +/// setting `client.version = [moq-lite-05-wip]`) will still negotiate it via +/// the version enum, but the default advertisement only covers shipped +/// versions. pub const ALPNS: &[&str] = &[ - ALPN_LITE_05, ALPN_LITE_04, ALPN_LITE_03, ALPN_LITE, @@ -31,7 +37,10 @@ pub const ALPNS: &[&str] = &[ pub(crate) const ALPN_LITE: &str = "moql"; pub(crate) const ALPN_LITE_03: &str = "moq-lite-03"; pub(crate) const ALPN_LITE_04: &str = "moq-lite-04"; -pub(crate) const ALPN_LITE_05: &str = "moq-lite-05"; +/// Work-in-progress ALPN for Lite05. Kept distinct from a stable `moq-lite-05` +/// identifier so clients that pin to that string today don't accidentally +/// negotiate the unfinalized wire format. Renamed when Lite05 is finalized. +pub(crate) const ALPN_LITE_05: &str = "moq-lite-05-wip"; pub(crate) const ALPN_14: &str = "moq-00"; pub(crate) const ALPN_15: &str = "moqt-15"; pub(crate) const ALPN_16: &str = "moqt-16"; @@ -158,7 +167,7 @@ impl FromStr for Version { "moq-lite-02" => Ok(Self::Lite(lite::Version::Lite02)), "moq-lite-03" => Ok(Self::Lite(lite::Version::Lite03)), "moq-lite-04" => Ok(Self::Lite(lite::Version::Lite04)), - "moq-lite-05" => Ok(Self::Lite(lite::Version::Lite05)), + "moq-lite-05-wip" => Ok(Self::Lite(lite::Version::Lite05)), "moq-transport-14" => Ok(Self::Ietf(ietf::Version::Draft14)), "moq-transport-15" => Ok(Self::Ietf(ietf::Version::Draft15)), "moq-transport-16" => Ok(Self::Ietf(ietf::Version::Draft16)), @@ -223,9 +232,14 @@ pub struct Versions(Vec); impl Versions { /// All supported versions exposed by default. + /// + /// `lite::Version::Lite05` is intentionally omitted: it is work-in-progress + /// (see the `ALPN_LITE_05` / `moq-lite-05-wip` ALPN) and would be picked up + /// by ALPN negotiation if included. Callers that explicitly opt in (e.g. + /// via `--client-version moq-lite-05-wip`) still get it; the default just + /// doesn't advertise it. pub fn all() -> Self { Self(vec![ - Version::Lite(lite::Version::Lite05), Version::Lite(lite::Version::Lite04), Version::Lite(lite::Version::Lite03), Version::Lite(lite::Version::Lite02),