From e6cc9962364e0382fcc2d4a9c5d58d8ab07cab0a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 02:23:46 -0700 Subject: [PATCH 01/15] chore(quest): claim quest/m1/simulcast-rung-disable.md Quest-Claim: 7d0f7f4d11c3477b From 1216bca8e83721da957d312a5df4a7dab34aa6d0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 02:41:24 -0700 Subject: [PATCH 02/15] feat(rate)!: disable simulcast rungs the grant cannot fund Policy::lower sets a band boundary below which a rendition disables, once a session probes for its reservation. Idle reservations read the share they would get back, so a disabled rung resumes after its viewers left. Co-Authored-By: Claude Opus 5.5 --- rs/moq-mux/src/codec/h264/import.rs | 41 +++ rs/moq-mux/src/codec/h265/import.rs | 8 + rs/moq-mux/src/codec/video.rs | 18 ++ rs/moq-mux/src/rate.rs | 472 +++++++++++++++++++++++++--- rs/moq-net/src/model/bandwidth.rs | 142 +++++++-- rs/moq-video/src/encode/producer.rs | 26 +- 6 files changed, 636 insertions(+), 71 deletions(-) diff --git a/rs/moq-mux/src/codec/h264/import.rs b/rs/moq-mux/src/codec/h264/import.rs index 786a7299d6c..40f9d22f884 100644 --- a/rs/moq-mux/src/codec/h264/import.rs +++ b/rs/moq-mux/src/codec/h264/import.rs @@ -158,6 +158,14 @@ impl Import { Ok(()) } + /// Advertise whether this rendition is being produced, as the catalog's + /// [`enabled`](hang::catalog::VideoConfig::enabled): a publisher that stops encoding it + /// disables it, so viewers select another rendition rather than wait for frames. + pub fn set_enabled(&mut self, enabled: bool) -> Result<()> { + self.catalog.set_enabled(&mut self.track, enabled)?; + Ok(()) + } + /// Close the current group and open the next one at `sequence`. pub fn seek(&mut self, sequence: u64) -> Result<()> { self.track.seek(sequence)?; @@ -392,6 +400,39 @@ mod tests { assert_eq!((color.mastering, color.content_light), (None, None)); } + /// A disabled rendition stays disabled across a new SPS, and enabling it republishes. + #[tokio::test(start_paused = true)] + async fn a_disabled_rendition_survives_a_new_sps() { + use crate::codec::h264::fixtures::{PPS, SPS_BT709, SPS_HDR10}; + + let idr: &[u8] = &[0x65, 0x88, 0x84, 0x21]; + let (track, catalog) = setup("video"); + let mut import = Import::new(track, catalog.reserve(), Default::default()).unwrap(); + let mut split = Split::new(); + let mut keyframe = |import: &mut Import, ms: u64, sps: &[u8]| { + let mut annexb = BytesMut::new(); + for nal in [sps, PPS, idr] { + annexb.extend_from_slice(&[0, 0, 0, 1]); + annexb.extend_from_slice(nal); + } + let pts = moq_net::Timestamp::from_millis(ms).unwrap(); + let mut frames = split.decode(&annexb, pts).expect("split keyframe"); + frames.extend(split.flush(pts).expect("flush keyframe")); + import.decode(frames).expect("decode keyframe"); + }; + let enabled = || catalog.snapshot().video.renditions["video"].enabled; + + keyframe(&mut import, 0, SPS_HDR10); + assert!(enabled()); + import.set_enabled(false).unwrap(); + assert!(!enabled()); + keyframe(&mut import, 40, SPS_BT709); + assert_eq!(catalog.snapshot().video.renditions["video"].color.as_ref().unwrap().primaries, Some(1)); + assert!(!enabled(), "a reconfiguration keeps the rendition disabled"); + import.set_enabled(true).unwrap(); + assert!(enabled()); + } + /// A frame-only SPS declares its reorder depth, which holds for the track's life: a later SPS /// declaring less or none keeps it, and one declaring more is refused. #[tokio::test(start_paused = true)] diff --git a/rs/moq-mux/src/codec/h265/import.rs b/rs/moq-mux/src/codec/h265/import.rs index 5d1c6100c29..97c9a13066f 100644 --- a/rs/moq-mux/src/codec/h265/import.rs +++ b/rs/moq-mux/src/codec/h265/import.rs @@ -161,6 +161,14 @@ impl Import { Ok(()) } + /// Advertise whether this rendition is being produced, as the catalog's + /// [`enabled`](hang::catalog::VideoConfig::enabled): a publisher that stops encoding it + /// disables it, so viewers select another rendition rather than wait for frames. + pub fn set_enabled(&mut self, enabled: bool) -> Result<()> { + self.catalog.set_enabled(&mut self.track, enabled)?; + Ok(()) + } + /// Close the current group and open the next one at `sequence`. pub fn seek(&mut self, sequence: u64) -> Result<()> { self.track.seek(sequence)?; diff --git a/rs/moq-mux/src/codec/video.rs b/rs/moq-mux/src/codec/video.rs index e8b49ae0b0b..6d4708e6744 100644 --- a/rs/moq-mux/src/codec/video.rs +++ b/rs/moq-mux/src/codec/video.rs @@ -202,6 +202,8 @@ pub(crate) struct Catalog { resolved: Option, /// The static HDR metadata the bitstream carried in band, overlaid onto every config. hdr: crate::codec::color::Hdr, + /// Whether the publisher is producing this rendition, overlaid onto every config. + enabled: bool, } impl Catalog { @@ -212,6 +214,21 @@ impl Catalog { last: None, resolved: None, hdr: Default::default(), + enabled: true, + } + } + + /// Advertise whether the publisher is producing this rendition, republishing the last + /// config if it changed. Overlaid onto every later config, so a resolution change + /// while disabled stays disabled. + pub(crate) fn set_enabled(&mut self, track: &mut Track, enabled: bool) -> crate::Result<()> { + if self.enabled == enabled { + return Ok(()); + } + self.enabled = enabled; + match self.resolved.clone() { + Some(config) => self.publish(track, config), + None => Ok(()), } } @@ -238,6 +255,7 @@ impl Catalog { self.resolved = Some(config.clone()); self.hdr.apply(&mut config.color); self.hint.apply(&mut config); + config.enabled = self.enabled; if let Some(declared) = self.last.as_ref().and_then(|last| last.reorder) { match config.reorder { Some(reorder) if reorder > declared => { diff --git a/rs/moq-mux/src/rate.rs b/rs/moq-mux/src/rate.rs index 8b45f8dffbe..bc2ad211cfd 100644 --- a/rs/moq-mux/src/rate.rs +++ b/rs/moq-mux/src/rate.rs @@ -2,7 +2,9 @@ //! //! [`Control`] turns each grant from a [`bandwidth::Consumer`] into the target //! rate a sender should produce, so audio, video, and transcode back off the -//! same way. [`Follower`] drives it from a live grant. +//! same way. [`Follower`] drives it from a live grant. A rendition encoded beside +//! lower ones of the same source sets [`Policy::lower`], and stops once its grant +//! can no longer fund it, so the lower renditions keep their share. use std::{ pin::Pin, @@ -57,6 +59,19 @@ pub struct Policy { /// holds here and the transport's priority queue sheds the excess instead. /// Defaults to a tenth of `max`. pub min: bandwidth::Rate, + + /// The next lower rendition's [`max`](Self::max), when this one is encoded beside + /// lower renditions of the same source. + /// + /// A grant below the band boundary `(max + 2 * lower) / 3` disables this rendition, + /// since the lower one then serves viewers better than a starved copy of this one. + /// It resumes at the boundary once the grant clears it by the same 5% hysteresis + /// every move needs. It only disables while a session probes for it (see + /// [`Control::set_probed`]), since nothing else could raise the estimate to resume it. + /// + /// `None`, the default, for the lowest or only rendition, which never disables. A + /// `lower` at or above `max` leaves no band, so it never disables either. + pub lower: Option, } impl Policy { @@ -67,8 +82,28 @@ impl Policy { // A tenth of the ceiling: low enough to ride out a bad uplink, high // enough that what we do send is still worth decoding. min: bandwidth::Rate::from_bps(max.as_bps() / 10), + lower: None, } } + + /// The band boundary [`lower`](Self::lower) sets, below which this rendition + /// disables, or `None` when it never does. + fn boundary(&self) -> Option { + let lower = self.lower.filter(|lower| *lower < self.max)?; + let boundary = (u128::from(self.max.as_bps()) + 2 * u128::from(lower.as_bps())) / 3; + // Below `max`, which fits a `u64`. + Some(bandwidth::Rate::from_bps(boundary as u64)) + } +} + +/// What a sender should be doing, per its [`Control`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Target { + /// Encode at this rate, and advertise the rendition as enabled. + Enabled(bandwidth::Rate), + /// Stop encoding, and advertise the rendition as disabled. Only a [`Policy`] with a + /// [`lower`](Policy::lower) rendition ever asks for this. + Disabled, } /// Maps bandwidth estimates onto a target bitrate, per a [`Policy`]. @@ -79,14 +114,25 @@ impl Policy { /// straight to a sender without rate-limiting it further: /// /// ``` -/// # use moq_mux::rate::{Control, Policy}; +/// # use moq_mux::rate::{Control, Policy, Target}; /// # use moq_net::bandwidth; /// # use moq_clock::Instant; /// let mut control = Control::new(Policy::new(bandwidth::Rate::from_mbps(4))); /// // A 2 Mbps estimate takes the 4 Mbps target down to what the link will carry. /// assert_eq!( /// control.update(Some(bandwidth::Rate::from_mbps(2)), moq_clock::now()), -/// Some(bandwidth::Rate::from_mbps(2)) +/// Some(Target::Enabled(bandwidth::Rate::from_mbps(2))) +/// ); +/// +/// // A 5 Mbps rendition above a 2.5 Mbps one disables below 3.33 Mbps, once a +/// // session probes for it. +/// let mut policy = Policy::new(bandwidth::Rate::from_mbps(5)); +/// policy.lower = Some(bandwidth::Rate::from_kbps(2_500)); +/// let mut control = Control::new(policy); +/// control.set_probed(true); +/// assert_eq!( +/// control.update(Some(bandwidth::Rate::from_mbps(3)), moq_clock::now()), +/// Some(Target::Disabled) /// ); /// ``` /// @@ -100,6 +146,10 @@ impl Policy { pub struct Control { policy: Policy, target: bandwidth::Rate, + /// Whether the sender should encode at all; see [`Policy::lower`]. + enabled: bool, + /// Whether a session probes for this sender, which disabling needs. + probed: bool, /// When the target last moved, anchoring the [`RAMP`] limit. `None` /// until the first change, when there's nothing to ramp from. applied: Option, @@ -115,6 +165,7 @@ pub struct Control { #[derive(Clone, Copy, Debug)] struct Change { target: bandwidth::Rate, + enabled: bool, applied: Option, desired: bandwidth::Rate, } @@ -126,15 +177,33 @@ impl Control { Self { target: policy.max, policy, + enabled: true, + probed: false, applied: None, deadline: None, last: None, } } - /// The current target bitrate. - pub fn target(&self) -> bandwidth::Rate { - self.target + /// What the sender should currently be doing. + pub fn target(&self) -> Target { + match self.enabled { + true => Target::Enabled(self.target), + false => Target::Disabled, + } + } + + /// Whether a session probes the connection for this sender, as + /// [`Consumer::probed`](bandwidth::Consumer::probed) reports it, from the next + /// [`update`](Self::update) on. + /// + /// A rendition only disables while one does: without a sender probing for it, + /// the estimate stops rising once the connection is app-limited, so a stopped + /// rendition could never resume. One disabled when probing stops resumes at the + /// next estimate instead, following the grant down like any sender. `false` + /// until set. + pub fn set_probed(&mut self, probed: bool) { + self.probed = probed; } /// When to [`update`](Self::update) again with the latest estimate, changed or @@ -152,7 +221,7 @@ impl Control { /// A `None` estimate (no congestion controller, or disconnected) holds the /// current target rather than resetting to [`Policy::max`]: losing the /// estimate is not evidence the uplink got better. - pub fn update(&mut self, estimate: Option, now: Instant) -> Option { + pub fn update(&mut self, estimate: Option, now: Instant) -> Option { self.last = None; let Some(estimate) = estimate else { self.deadline = None; @@ -163,6 +232,41 @@ impl Control { // clamp below panic. let min = self.policy.min.min(self.policy.max); let desired = estimate.clamp(min, self.policy.max); + let boundary = self.policy.boundary().filter(|_| self.probed); + + if !self.enabled { + let resume = match boundary { + Some(boundary) => { + // Capped at the ceiling, or a boundary within the deadband of it could + // never be cleared by an estimate clamped to that ceiling. + let clear = boundary + .as_bps() + .saturating_add(band(boundary).as_bps()) + .min(self.policy.max.as_bps()); + if desired.as_bps() < clear { + return None; + } + // Restart at the boundary and ramp: the estimate was measured while + // this rendition sent nothing, so its whole share is the least + // trustworthy number here. + boundary.clamp(min, self.policy.max) + } + // Nothing probes for it any more, so it can't wait for the estimate to + // rise: follow the grant like any sender instead. + None => desired, + }; + self.last = Some(Change { + target: self.target, + enabled: false, + applied: self.applied, + desired, + }); + self.enabled = true; + self.target = resume; + self.applied = Some(now); + self.deadline = self.raise(desired); + return Some(Target::Enabled(resume)); + } let next = if desired <= self.target { // Attack: the pipe is closing, give the bandwidth back now. @@ -198,7 +302,8 @@ impl Control { // below the rate the caller asked for is not. // // Drops keep the deadband either way: sitting a little above a falling - // estimate is what it's for. + // estimate is what it's for. That is also what keeps a grant jittering + // around the band boundary from flapping the rendition off and on. let settling = next > self.target && next == self.policy.max; if next == self.target || (!settling && next.abs_diff(self.target) < band(self.target)) { self.deadline = self.raise(desired); @@ -207,13 +312,20 @@ impl Control { self.last = Some(Change { target: self.target, + enabled: true, applied: self.applied, desired, }); + if boundary.is_some_and(|boundary| next < boundary) { + // Only a new estimate clearing the boundary resumes it, so nothing is due. + self.enabled = false; + self.deadline = None; + return Some(Target::Disabled); + } self.target = next; self.applied = Some(now); self.deadline = self.raise(desired); - Some(next) + Some(Target::Enabled(next)) } /// Take back the target the last [`update`](Self::update) returned, because the @@ -227,11 +339,14 @@ impl Control { let Some(change) = self.last.take() else { return }; // The old anchor too, so the retry keeps the ramp allowance it had earned, // the same as a raise suppressed by the deadband. + let toggled = self.enabled != change.enabled; self.target = change.target; + self.enabled = change.enabled; self.applied = change.applied; let retry = now.checked_add(RETRY); - self.deadline = if change.desired < self.target { - // A returned drop already cleared the deadband, and drops don't ramp. + self.deadline = if toggled || change.desired < self.target { + // A returned drop already cleared the deadband, and drops don't ramp. A + // returned disable or resume already cleared its boundary. retry } else { self.raise(change.desired).zip(retry).map(|(at, retry)| at.max(retry)) @@ -277,6 +392,10 @@ impl Control { /// Wakes on every change to the grant and whenever the ramp allows the next step of /// a raise or the retry of a refused change, so a sender climbs back to its ceiling /// even after the grant goes quiet. +/// +/// Follow a [`Reservation`](bandwidth::Reservation)'s consumer: it reports whether a +/// session probes for it, which a rendition needs before it disables, and the slice it +/// would get back while unwatched, which it needs to resume after its viewers left. pub struct Follower { grant: bandwidth::Consumer, control: Control, @@ -305,17 +424,20 @@ impl Follower { /// Follow under `policy` from now on. A different policy starts over at its /// [`Policy::max`], as a fresh [`Follower`] would, and the next poll feeds it the - /// current grant. + /// current grant. A disabled sender stays disabled until that grant resumes it + /// under the new policy, since nothing told it to start. pub fn set_policy(&mut self, policy: Policy) { if policy != self.control.policy { + let enabled = self.control.enabled; self.control = Control::new(policy); + self.control.enabled = enabled; self.refeed = true; } } /// Poll for the next target the sender should produce at, or the error that /// closed the grant. - pub fn poll_next(&mut self, waiter: &kio::Waiter) -> Poll> { + pub fn poll_next(&mut self, waiter: &kio::Waiter) -> Poll> { loop { let estimate = if std::mem::take(&mut self.refeed) { self.grant.peek() @@ -327,6 +449,7 @@ impl Follower { ready!(self.poll_deadline(waiter)); self.grant.peek() }; + self.control.set_probed(self.grant.probed()); if let Some(target) = self.control.update(estimate, moq_clock::now()) { return Poll::Ready(Ok(target)); } @@ -334,7 +457,7 @@ impl Follower { } /// The next target the sender should produce at, or the error that closed the grant. - pub async fn next(&mut self) -> Result { + pub async fn next(&mut self) -> Result { kio::wait(|waiter| self.poll_next(waiter)).await } @@ -384,6 +507,11 @@ mod tests { bandwidth::Rate::from_bps(bps) } + /// An enabled target at `rate`. + fn on(rate: bandwidth::Rate) -> Target { + Target::Enabled(rate) + } + /// 4 Mbps ceiling, so the max/10 floor lands on a round 400 kbps. fn control() -> Control { Control::new(Policy::new(bps(4_000_000))) @@ -395,9 +523,9 @@ mod tests { // A 2 Mbps pipe: take the target down to it at once, no ramp, no waiting. assert_eq!( control.update(Some(bps(2_000_000)), moq_clock::now()), - Some(bps(2_000_000)) + Some(on(bps(2_000_000))) ); - assert_eq!(control.target(), bps(2_000_000)); + assert_eq!(control.target(), on(bps(2_000_000))); } #[test] @@ -409,7 +537,7 @@ mod tests { // Losing the estimate (disconnected) is not evidence the uplink is // healthy again, so the target must not jump back to max. assert_eq!(control.update(None, now + Duration::from_secs(10)), None); - assert_eq!(control.target(), bps(2_000_000)); + assert_eq!(control.target(), on(bps(2_000_000))); } #[test] @@ -417,15 +545,15 @@ mod tests { let mut control = control(); // A wildly optimistic estimate is not licence to exceed what was configured. assert_eq!(control.update(Some(bps(100_000_000)), moq_clock::now()), None); - assert_eq!(control.target(), bps(4_000_000)); + assert_eq!(control.target(), on(bps(4_000_000))); } #[test] fn target_never_falls_below_min() { let mut control = control(); // A near-dead uplink floors at min (max/10) rather than chasing to zero. - assert_eq!(control.update(Some(bps(1)), moq_clock::now()), Some(bps(400_000))); - assert_eq!(control.target(), bps(400_000)); + assert_eq!(control.update(Some(bps(1)), moq_clock::now()), Some(on(bps(400_000)))); + assert_eq!(control.target(), on(bps(400_000))); } #[test] @@ -439,7 +567,7 @@ mod tests { let raised = control .update(Some(bps(4_000_000)), start + Duration::from_secs(1)) .unwrap(); - assert_eq!(raised, bps(1_250_000)); + assert_eq!(raised, on(bps(1_250_000))); } /// Regression: the ramp allowance per tick (25%/s * 100ms = 2.5%) is smaller @@ -465,7 +593,7 @@ mod tests { let (tick, next) = raised.expect("a raise must eventually clear hysteresis"); // 5% of 1M needs 0.05/0.25 = 0.2s of ramp, i.e. the tick at 200ms. assert_eq!(tick, 2); - assert_eq!(next, bps(1_050_000)); + assert_eq!(next, on(bps(1_050_000))); } /// Regression: the ramp stops growing `next` once it reaches the ceiling, so a @@ -487,7 +615,7 @@ mod tests { } } - assert_eq!(last, Some(bps(4_000_000))); + assert_eq!(last, Some(on(bps(4_000_000)))); // And it stops there rather than reapplying the same value forever. assert_eq!( control.update(Some(bps(4_000_000)), start + Duration::from_secs(60)), @@ -521,13 +649,13 @@ mod tests { "estimate {estimate} is inside the deadband" ); } - assert_eq!(control.target(), bps(2_000_000)); + assert_eq!(control.target(), on(bps(2_000_000))); // And the deadband is still only a deadband: once the estimate does clear it, // the raise lands normally, without needing the ceiling exemption. assert_eq!( control.update(Some(bps(2_200_000)), start + Duration::from_secs(2)), - Some(bps(2_200_000)) + Some(on(bps(2_200_000))) ); } @@ -539,12 +667,12 @@ mod tests { // 2% under the current target: inside the 5% deadband, so no reconfigure. assert_eq!(control.update(Some(bps(1_960_000)), now + Duration::from_secs(1)), None); - assert_eq!(control.target(), bps(2_000_000)); + assert_eq!(control.target(), on(bps(2_000_000))); // 20% under: outside the deadband, so it applies. assert_eq!( control.update(Some(bps(1_600_000)), now + Duration::from_secs(2)), - Some(bps(1_600_000)) + Some(on(bps(1_600_000))) ); } @@ -555,9 +683,9 @@ mod tests { policy.min = bps(5_000_000); let mut control = Control::new(policy); - assert_eq!(control.target(), bps(1_000_000)); + assert_eq!(control.target(), on(bps(1_000_000))); assert_eq!(control.update(Some(bps(2_000_000)), moq_clock::now()), None); - assert_eq!(control.target(), bps(1_000_000)); + assert_eq!(control.target(), on(bps(1_000_000))); } /// Feed `estimate` at every deadline until the target settles, asserting each one @@ -569,6 +697,9 @@ mod tests { assert!(applied.len() < 1_000, "the deadline never settled: {applied:?}"); let next = control.update(Some(estimate), at); let next = next.unwrap_or_else(|| panic!("a deadline at {at:?} applied nothing after {applied:?}")); + let Target::Enabled(next) = next else { + panic!("a deadline at {at:?} disabled the sender after {applied:?}") + }; applied.push(next); last = Some(at); } @@ -585,7 +716,7 @@ mod tests { assert_eq!(control.deadline(), None, "a drop lands at once"); let recovered = start + Duration::from_secs(1); - assert_eq!(control.update(Some(bps(4_000_000)), recovered), Some(bps(1_250_000))); + assert_eq!(control.update(Some(bps(4_000_000)), recovered), Some(on(bps(1_250_000)))); let (applied, last) = follow(&mut control, bps(4_000_000)); assert_eq!(applied.last(), Some(&bps(4_000_000))); @@ -610,7 +741,7 @@ mod tests { let at = start + Duration::from_millis(7); control.update(Some(bps(max)), at); let (applied, _) = follow(&mut control, bps(max)); - assert_eq!(control.target(), bps(max), "{max} from {floor}: {applied:?}"); + assert_eq!(control.target(), on(bps(max)), "{max} from {floor}: {applied:?}"); } } } @@ -623,7 +754,7 @@ mod tests { control.update(Some(bps(1_000_000)), start).unwrap(); assert_eq!( control.update(Some(bps(1_100_000)), start + Duration::from_secs(1)), - Some(bps(1_100_000)) + Some(on(bps(1_100_000))) ); assert_eq!(control.deadline(), None); } @@ -660,7 +791,7 @@ mod tests { remainder.update(Some(bps(1_000_000)), start).unwrap(); assert_eq!( remainder.update(Some(bps(1_300_000)), start + Duration::from_secs(1)), - Some(bps(1_250_000)) + Some(on(bps(1_250_000))) ); assert_eq!(remainder.deadline(), None); } @@ -686,7 +817,7 @@ mod tests { let mut control = scheduled(); assert_eq!( control.update(Some(bps(800_000)), start + Duration::from_secs(2)), - Some(bps(800_000)) + Some(on(bps(800_000))) ); assert_eq!(control.deadline(), None); } @@ -707,7 +838,7 @@ mod tests { let failed = last.unwrap() + Duration::from_millis(1); control.failed(failed); - assert_eq!(control.target(), before, "the previous target is restored"); + assert_eq!(control.target(), on(before), "the previous target is restored"); let retry = control.deadline().expect("the failed raise is retried"); assert!( retry >= failed + RETRY, @@ -716,10 +847,10 @@ mod tests { // A second report has nothing left to take back. control.failed(failed + Duration::from_secs(1)); - assert_eq!(control.target(), before); + assert_eq!(control.target(), on(before)); assert_eq!(control.deadline(), Some(retry)); - assert_eq!(control.update(Some(bps(4_000_000)), retry), Some(bps(4_000_000))); + assert_eq!(control.update(Some(bps(4_000_000)), retry), Some(on(bps(4_000_000)))); assert_eq!(control.deadline(), None); } @@ -730,11 +861,11 @@ mod tests { let start = moq_clock::now(); control.update(Some(bps(1_000_000)), start).unwrap(); control.failed(start); - assert_eq!(control.target(), bps(4_000_000)); + assert_eq!(control.target(), on(bps(4_000_000))); assert_eq!(control.deadline(), Some(start + RETRY)); assert_eq!( control.update(Some(bps(1_000_000)), start + RETRY), - Some(bps(1_000_000)) + Some(on(bps(1_000_000))) ); assert_eq!(control.deadline(), None); } @@ -750,7 +881,7 @@ mod tests { None ); control.failed(start + Duration::from_secs(1)); - assert_eq!(control.target(), bps(2_000_000)); + assert_eq!(control.target(), on(bps(2_000_000))); assert_eq!(control.deadline(), None); } @@ -762,7 +893,7 @@ mod tests { let mut follower = Follower::new(grant.consume(), Policy::new(bps(4_000_000))); grant.set(Some(bps(1_000_000))).unwrap(); - assert_eq!(follower.next().await.unwrap(), bps(1_000_000)); + assert_eq!(follower.next().await.unwrap(), on(bps(1_000_000))); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(bps(4_000_000))).unwrap(); @@ -771,7 +902,9 @@ mod tests { let mut refused = false; // A minute without a wake is far past anything the ramp still needs. while let Ok(target) = moq_clock::timeout(Duration::from_secs(60), follower.next()).await { - let target = target.unwrap(); + let Target::Enabled(target) = target.unwrap() else { + panic!("a policy without a lower rendition disabled the sender") + }; if target == bps(4_000_000) && !refused { refused = true; follower.failed(); @@ -791,7 +924,7 @@ mod tests { let grant = bandwidth::Producer::new(); let mut follower = Follower::new(grant.consume(), Policy::new(bps(4_000_000))); grant.set(Some(bps(3_000_000))).unwrap(); - assert_eq!(follower.next().await.unwrap(), bps(3_000_000)); + assert_eq!(follower.next().await.unwrap(), on(bps(3_000_000))); follower.set_policy(Policy::new(bps(2_000_000))); assert_eq!(follower.policy().max, bps(2_000_000)); @@ -804,7 +937,7 @@ mod tests { ); follower.set_policy(Policy::new(bps(4_000_000))); - assert_eq!(follower.next().await.unwrap(), bps(3_000_000)); + assert_eq!(follower.next().await.unwrap(), on(bps(3_000_000))); follower.set_policy(Policy::new(bps(4_000_000))); assert!( moq_clock::timeout(Duration::from_secs(60), follower.next()) @@ -830,7 +963,19 @@ mod tests { grant.set(Some(bps(3_000_000))).unwrap(); follower.set_policy(Policy::new(bps(4_000_000))); - assert_eq!(follower.next().await.unwrap(), bps(3_000_000)); + assert_eq!(follower.next().await.unwrap(), on(bps(3_000_000))); + } + + /// A grant no session probes for, like a whole estimate, never disables a rendition, + /// however far it falls under the band boundary. + #[tokio::test(start_paused = true)] + async fn an_unprobed_follower_never_disables() { + let grant = bandwidth::Producer::new(); + let mut policy = Policy::new(bps(4_000_000)); + policy.lower = Some(bps(2_000_000)); + let mut follower = Follower::new(grant.consume(), policy); + grant.set(Some(bps(1_000_000))).unwrap(); + assert_eq!(follower.next().await.unwrap(), on(bps(1_000_000))); } /// A closed grant ends the follower with the reason. @@ -841,4 +986,241 @@ mod tests { drop(grant); assert!(follower.next().await.is_err()); } + + /// A 5 Mbps rendition above a 2.5 Mbps one, probed, so the boundary lands on 3_333_333. + fn rung() -> Control { + let mut policy = Policy::new(bps(5_000_000)); + policy.lower = Some(bps(2_500_000)); + let mut control = Control::new(policy); + control.set_probed(true); + control + } + + #[test] + fn a_rung_disables_below_its_boundary() { + let mut rung = rung(); + let start = moq_clock::now(); + + // Squeezed, but still inside its band: it adapts like any sender. + assert_eq!(rung.update(Some(bps(3_500_000)), start), Some(on(bps(3_500_000)))); + // Below it, the lower rendition is the better picture, so stop. + let off = start + Duration::from_millis(100); + assert_eq!(rung.update(Some(bps(3_000_000)), off), Some(Target::Disabled)); + assert_eq!(rung.target(), Target::Disabled); + assert_eq!(rung.deadline(), None, "only a new estimate resumes it"); + } + + /// The lowest rendition has no lower one, so no grant disables it: it floors at + /// `min` and leaves the rest to the transport, which can only probe with what it sends. + #[test] + fn the_lowest_rung_never_disables() { + let mut lowest = Control::new(Policy::new(bps(350_000))); + lowest.set_probed(true); + assert_eq!(lowest.update(Some(bps(1)), moq_clock::now()), Some(on(bps(35_000)))); + } + + /// Without a session probing for it, a stopped rendition could never see the estimate + /// rise, so it keeps encoding and follows its grant down instead. + #[test] + fn an_unprobed_rung_never_disables() { + let mut rung = rung(); + rung.set_probed(false); + assert_eq!( + rung.update(Some(bps(1_000_000)), moq_clock::now()), + Some(on(bps(1_000_000))) + ); + } + + /// A rendition disabled while probing resumes at the next estimate once probing stops, + /// following that grant rather than waiting on an estimate that can no longer rise. + #[test] + fn a_disabled_rung_resumes_when_probing_stops() { + let mut rung = rung(); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(1_000_000)), start), Some(Target::Disabled)); + + rung.set_probed(false); + assert_eq!( + rung.update(Some(bps(1_000_000)), start + Duration::from_secs(1)), + Some(on(bps(1_000_000))) + ); + } + + #[test] + fn a_rung_resumes_past_the_hysteresis_and_ramps() { + let mut rung = rung(); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(3_000_000)), start), Some(Target::Disabled)); + + // Back over the boundary, but not by the 5% hysteresis: stay off. + assert_eq!(rung.update(Some(bps(3_400_000)), start + Duration::from_secs(1)), None); + assert_eq!(rung.update(Some(bps(3_499_000)), start + Duration::from_secs(2)), None); + assert_eq!(rung.target(), Target::Disabled); + + // Clear of it: resume at the boundary rather than the whole estimate. + let resumed = start + Duration::from_secs(3); + assert_eq!(rung.update(Some(bps(5_000_000)), resumed), Some(on(bps(3_333_333)))); + + // Then ramp at 25%/s like any raise, on deadlines alone. + let (applied, _) = follow(&mut rung, bps(5_000_000)); + assert_eq!(applied.last(), Some(&bps(5_000_000)), "{applied:?}"); + } + + /// A grant jittering around the boundary must not flap the catalog: the deadband + /// keeps a rung on until the grant falls well under its applied target, and the + /// resume margin keeps it off until the grant is clearly back. + #[test] + fn jitter_at_the_boundary_does_not_flap() { + let mut rung = rung(); + let start = moq_clock::now(); + rung.update(Some(bps(3_400_000)), start).unwrap(); + + // About 3% either side of the boundary, every 100ms for 10s. + let jitter = |tick: u64| if tick.is_multiple_of(2) { 3_250_000 } else { 3_430_000 }; + for tick in 1..=100u64 { + let at = start + Duration::from_millis(100 * tick); + assert_eq!(rung.update(Some(bps(jitter(tick))), at), None, "tick {tick}"); + } + assert_eq!(rung.target(), on(bps(3_400_000))); + + // A real drop turns it off, and the same jitter can't bring it back. + let off = start + Duration::from_secs(20); + assert_eq!(rung.update(Some(bps(3_000_000)), off), Some(Target::Disabled)); + for tick in 1..=100u64 { + let at = off + Duration::from_millis(100 * tick); + assert_eq!(rung.update(Some(bps(jitter(tick))), at), None, "tick {tick}"); + } + assert_eq!(rung.target(), Target::Disabled); + } + + #[test] + fn a_missing_estimate_holds_a_disabled_rung() { + let mut rung = rung(); + let now = moq_clock::now(); + rung.update(Some(bps(1_000_000)), now).unwrap(); + assert_eq!(rung.update(None, now + Duration::from_secs(10)), None); + assert_eq!(rung.target(), Target::Disabled); + } + + /// A resume margin above the ceiling could never be met by an estimate clamped to + /// that ceiling, so it is capped there. + #[test] + fn a_rung_just_above_its_lower_still_resumes() { + let mut policy = Policy::new(bps(1_000_000)); + policy.lower = Some(bps(950_000)); + let mut rung = Control::new(policy); + rung.set_probed(true); + let now = moq_clock::now(); + assert_eq!(rung.update(Some(bps(500_000)), now), Some(Target::Disabled)); + assert_eq!( + rung.update(Some(bps(10_000_000)), now + Duration::from_secs(1)), + Some(on(bps(966_666))) + ); + } + + /// A lower rendition at or above this one's ceiling leaves no band to encode in. + #[test] + fn a_rung_without_a_band_never_disables() { + let mut policy = Policy::new(bps(1_000_000)); + policy.lower = Some(bps(1_000_000)); + let mut rung = Control::new(policy); + rung.set_probed(true); + assert_eq!(rung.update(Some(bps(1)), moq_clock::now()), Some(on(bps(100_000)))); + } + + /// A disable or resume the sender failed to apply is taken back and retried, so the + /// catalog follows what the encoder did rather than what was asked. + #[test] + fn a_failed_toggle_is_retried() { + let mut rung = rung(); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(3_000_000)), start), Some(Target::Disabled)); + rung.failed(start); + assert_eq!(rung.target(), on(bps(5_000_000))); + assert_eq!(rung.deadline(), Some(start + RETRY)); + assert_eq!(rung.update(Some(bps(3_000_000)), start + RETRY), Some(Target::Disabled)); + + let resumed = start + Duration::from_secs(5); + assert_eq!(rung.update(Some(bps(5_000_000)), resumed), Some(on(bps(3_333_333)))); + rung.failed(resumed); + assert_eq!(rung.target(), Target::Disabled); + assert_eq!(rung.deadline(), Some(resumed + RETRY)); + assert_eq!( + rung.update(Some(bps(5_000_000)), resumed + RETRY), + Some(on(bps(3_333_333))) + ); + } + + /// End to end through the allocator: descending priority down the renditions fills + /// the lower ones first, so a shrinking estimate disables the top rendition and leaves + /// the lower ones at their full share. The top ones come back once the estimate + /// does, even after their viewers have left, since an idle reservation still reads + /// the slice it would get back. + #[test] + fn the_top_rung_yields_to_the_lower_ones() { + use moq_net::{broadcast, track}; + + let estimate = bandwidth::Producer::new(); + let allocator = bandwidth::Allocator::new(estimate.consume()); + let broadcast = broadcast::Info::default().produce(); + // Lowest first, each a step below the last in priority and a doubling in ceiling. + let ladder = [(1_000_000, None), (2_000_000, Some(1_000_000)), (4_000_000, Some(2_000_000))]; + let mut tracks = Vec::new(); + let mut viewers = Vec::new(); + let mut reservations = Vec::new(); + let mut controls = Vec::new(); + for (index, (max, lower)) in ladder.into_iter().enumerate() { + let priority = 60 - index as i8; + let track = broadcast + .create_track(format!("{max}"), track::Info::default().with_priority(priority)) + .unwrap(); + viewers.push(Some(track.subscribe(None))); + reservations.push(allocator.reserve(&track.demand(), bps(max))); + let mut policy = Policy::new(bps(max)); + policy.lower = lower.map(bps); + let mut control = Control::new(policy); + control.set_probed(true); + controls.push(control); + tracks.push(track); + } + let start = moq_clock::now(); + let mut update = |at: Instant| -> Vec { + controls + .iter_mut() + .zip(&reservations) + .map(|(control, reservation)| { + control.update(reservation.peek(), at); + control.target() + }) + .collect() + }; + + // 4.5 Mbps funds the lower two in full and leaves 1.5 Mbps, under the top + // rendition's 2.67 Mbps boundary. + estimate.set(Some(bps(4_500_000))).unwrap(); + assert_eq!(update(start), [on(bps(1_000_000)), on(bps(2_000_000)), Target::Disabled]); + + // Shrinking further takes the middle one next, still keeping the lowest whole. + estimate.set(Some(bps(2_000_000))).unwrap(); + assert_eq!( + update(start + Duration::from_secs(1)), + [on(bps(1_000_000)), Target::Disabled, Target::Disabled] + ); + + // Below every boundary, the lowest still encodes at what it gets. + estimate.set(Some(bps(200_000))).unwrap(); + assert_eq!( + update(start + Duration::from_secs(2)), + [on(bps(200_000)), Target::Disabled, Target::Disabled] + ); + + // The viewers of the stopped renditions fall back to the lowest and leave, and + // the uplink recovers: both resume at their boundaries without a viewer. + viewers[1] = None; + viewers[2] = None; + estimate.set(Some(bps(8_000_000))).unwrap(); + let recovered = update(start + Duration::from_secs(3)); + assert_eq!(recovered[1], on(bps(1_333_333))); + assert_eq!(recovered[2], on(bps(2_666_666))); + } } diff --git a/rs/moq-net/src/model/bandwidth.rs b/rs/moq-net/src/model/bandwidth.rs index 467eb242f5a..4ad12f1c676 100644 --- a/rs/moq-net/src/model/bandwidth.rs +++ b/rs/moq-net/src/model/bandwidth.rs @@ -105,6 +105,7 @@ impl Producer { Consumer { inner: Inner::Whole(self.state.consume()), last: None, + probed: false, } } @@ -186,6 +187,7 @@ impl Weak { Consumer { inner: Inner::Whole(self.0.consume()), last: None, + probed: false, } } } @@ -237,6 +239,7 @@ impl Allocator { estimate: estimate.unwrap_or(Consumer { inner: Inner::Unavailable, last: None, + probed: false, }), registry: self.registry.clone(), } @@ -256,11 +259,20 @@ impl Allocator { } /// Follow the probe target, for the session that probes on the reservations' behalf. + /// + /// Holding it is what marks the reservations [`probed`](Consumer::probed), so only a + /// session whose transport has a sender takes one. #[cfg(any(feature = "async", test))] // Sessions follow their allocator's target; a custom session has no public way to yet. pub(crate) fn target(&self) -> Target { + let Ok(mut registry) = self.registry.write() else { + unreachable!("the allocator holds its own registry producer") + }; + registry.probes += 1; + drop(registry); Target { registry: Some(self.registry.consume()), + probing: self.registry.downgrade(), last: None, } } @@ -276,6 +288,7 @@ impl Allocator { estimate: Consumer { inner: Inner::Unavailable, last: None, + probed: false, }, registry: kio::Producer::default(), } @@ -313,8 +326,9 @@ impl Allocator { /// handle and reserving never keeps a track alive. /// /// Read the current slice through [`Reservation::consumer`], which reports `None` - /// while nothing is subscribed to the track or the connection has no estimate. - /// That tells a sender to hold its current rate rather than encode at zero. + /// while the connection has no estimate, telling a sender to hold its current rate + /// rather than encode at zero. While nothing is subscribed to the track it reports + /// the slice the track would get back, without claiming it from the others. pub fn reserve(&self, track: &track::Demand, max: Rate) -> Reservation { // Read the track before taking the registry lock, so the two are never held // at once and there's no order to get wrong. @@ -383,11 +397,12 @@ impl Reservation { /// A handle reading this reservation's current slice of the estimate. /// /// Cloneable and independent of the reservation: once the reservation is dropped - /// these report `None`, the same "hold your rate" a track nobody is watching reports. + /// these report `None`, the same "hold your rate" a missing estimate reports. pub fn consumer(&self) -> Consumer { Consumer { inner: Inner::Share(Box::new(self.share.clone())), last: None, + probed: false, } } @@ -457,6 +472,8 @@ struct Registry { reserved: u128, /// The application's override of `reserved` as the probe target. target: Option, + /// How many sessions probe for these reservations, each through its [`Target`]. + probes: usize, } impl Registry { @@ -487,9 +504,24 @@ struct Entry { pub(crate) struct Target { /// `None` once every allocator is gone. registry: Option>, + /// Weak, so a session never keeps the registry alive, only counts itself while it does. + probing: kio::Weak, last: Option, } +#[cfg(any(feature = "async", test))] +impl Drop for Target { + fn drop(&mut self) { + let Some(registry) = self.probing.upgrade() else { + return; + }; + let Ok(mut registry) = registry.write() else { + return; + }; + registry.probes -= 1; + } +} + #[cfg(any(feature = "async", test))] impl Target { /// The target once it differs from the last one returned, arming `waiter` otherwise. @@ -548,15 +580,27 @@ impl Share { let wants: Vec = self .claims() .into_iter() - .filter(|(_, demand)| demand.is_used()) + // Its own track counts while idle too, so an unwatched sender reads the slice + // it would get back: a rendition stopped for lack of bandwidth, whose viewers + // then left, needs that to know when to resume. + .filter(|(want, demand)| match want.id == self.id { + true => !demand.is_closed(), + false => demand.is_used(), + }) .map(|(want, _)| want) .collect(); allocate(estimate, &wants, self.id) } - /// This share's slice, arming `waiter` for everything that could move it: the - /// estimate, the set of registered tracks, and each track's demand. - fn poll_grant(&mut self, waiter: &kio::Waiter) -> Poll>> { + /// Whether a session probes for this share's registry. + fn probed(&self) -> bool { + self.registry.read().probes > 0 + } + + /// This share's slice and whether a session probes for it, arming `waiter` for + /// everything that could move either: the estimate, the registry, and each + /// track's demand. + fn poll_grant(&mut self, waiter: &kio::Waiter) -> Poll, bool)>> { // Drain the estimate rather than polling it once. A poll that returns Ready // registers no waker, and this share may still conclude its own slice didn't // move (its reservation caps it, so most estimate changes don't reach it) and @@ -582,11 +626,12 @@ impl Share { let wants: Vec = self .claims() .into_iter() - .filter(|(_, demand)| match demand.poll_state(waiter) { + .filter(|(want, demand)| match demand.poll_state(waiter) { track::DemandState::Active => true, - // An idle track claims nothing, so an unwatched encoder's reservation - // goes to whoever is actually sending. - track::DemandState::Idle => false, + // An idle track claims nothing from its siblings, so an unwatched encoder's + // reservation goes to whoever is actually sending. Its own reader still + // sees the slice it would get back; see `grant`. + track::DemandState::Idle => want.id == self.id, // Gone for good, and pruned by the next `register`. track::DemandState::Closed => false, }) @@ -597,7 +642,7 @@ impl Share { .estimate .peek() .and_then(|estimate| allocate(estimate, &wants, self.id)); - Poll::Ready(Ok(grant)) + Poll::Ready(Ok((grant, self.probed()))) } /// Snapshot the registry so the tracks can be read without holding its lock. @@ -630,8 +675,8 @@ impl Share { /// A reservation is what a sender can use, so handing it more is not a reason to /// send more than it was configured for. /// -/// `None` when `id` isn't among the wants, which is how an idle or closed track -/// reports "hold your rate" instead of a grant of zero. +/// `None` when `id` isn't among the wants, which is how a released or closed +/// reservation reports "hold your rate" instead of a grant of zero. fn allocate(estimate: Rate, wants: &[Want], id: u64) -> Option { // Plain bits per second inside: this is where the division actually happens, and // wrapping every intermediate would need arithmetic on `Rate` that no caller wants. @@ -670,6 +715,8 @@ fn allocate(estimate: Rate, wants: &[Want], id: u64) -> Option { pub struct Consumer { inner: Inner, last: Option, + /// The [`probed`](Self::probed) state [`poll_changed`](Self::poll_changed) last reported. + probed: bool, } /// What a [`Consumer`] is reading: the whole estimate, or one track's slice of it. @@ -691,6 +738,21 @@ impl Consumer { } } + /// Whether a session probes the connection for this slice's reservation, so the + /// estimate can rise to fund it even while its sender is stopped. + /// + /// Only a session whose transport has a sender probes. Without one the estimate + /// stops learning once the connection is app-limited, so a sender that stops for + /// lack of bandwidth could never see enough to resume. Always `false` for a whole + /// estimate, which no reservation backs. [`poll_changed`](Self::poll_changed) + /// reports a change to this too. + pub fn probed(&self) -> bool { + match &self.inner { + Inner::Share(share) => share.probed(), + Inner::Whole(_) | Inner::Unavailable => false, + } + } + /// Poll for a bandwidth change without blocking. /// /// `Ok(None)` means the estimate is unavailable *for now*: the backend @@ -699,6 +761,8 @@ impl Consumer { /// arrive. They're distinct because a caller holds its current rate for the /// first and stops watching for the second. /// + /// A slice is also reported, unchanged, when [`probed`](Self::probed) flips. + /// /// A backend with no bandwidth estimation at all yields no [Consumer] in the /// first place, so that case never reaches here. pub fn poll_changed(&mut self, waiter: &kio::Waiter) -> Poll>> { @@ -727,8 +791,11 @@ impl Consumer { // arming anything rather than reporting a `None` the caller would re-read forever. Inner::Unavailable => return Poll::Pending, Inner::Share(share) => match share.poll_grant(waiter) { - Poll::Ready(Ok(grant)) if grant == last => return Poll::Pending, - Poll::Ready(Ok(grant)) => grant, + Poll::Ready(Ok((grant, probed))) if grant == last && probed == self.probed => return Poll::Pending, + Poll::Ready(Ok((grant, probed))) => { + self.probed = probed; + grant + } Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), Poll::Pending => return Poll::Pending, }, @@ -880,8 +947,9 @@ mod tests { assert_eq!(second_share.peek(), Some(bps(1_000_000))); } - /// An unwatched track releases its reservation, since `publish_capture` stops - /// encoding entirely while nothing is subscribed. + /// An unwatched track releases its reservation to the others, since `publish_capture` + /// stops encoding entirely while nothing is subscribed, but still reads the slice it + /// would get back, which is what a stopped rendition its viewers left resumes on. #[test] fn an_idle_track_claims_nothing() { let estimate = Producer::new(); @@ -896,9 +964,8 @@ mod tests { let idle_share = allocator.reserve(&idle.demand(), bps(4_000_000)); assert_eq!(watched_share.peek(), Some(bps(2_000_000))); - // Not zero: an idle share reports "no opinion" so a sender that is mid-shutdown - // holds its rate instead of retuning to the floor on the way out. - assert_eq!(idle_share.peek(), None); + // What it would get if a viewer joined: an even split with the watched track. + assert_eq!(idle_share.peek(), Some(bps(1_000_000))); // It joins, and the split happens. let _idle_sub = idle.subscribe(None); @@ -1092,8 +1159,8 @@ mod tests { assert_eq!(second_reserved.peek(), Some(bps(2_000_000))); // The orphaned reader is woken and told, rather than being left parked on a slice - // that will never move again. It reports the same "hold your rate" as an unwatched - // track, not a grant of zero that would tell an encoder to stop. + // that will never move again. It reports the same "hold your rate" as a missing + // estimate, not a grant of zero that would tell an encoder to stop. assert_eq!(orphan.changed().await.unwrap(), None); assert_eq!(orphan.peek(), None); } @@ -1273,6 +1340,35 @@ mod tests { ); } + /// A reservation is probed while a session follows its allocator's target, and a + /// reader parked on an unchanged slice is woken when that flips. + #[test] + #[cfg(feature = "async")] + fn a_following_session_marks_reservations_probed() { + let estimate = Producer::new(); + estimate.set(Some(bps(2_000_000))).unwrap(); + let allocator = Allocator::new(estimate.consume()); + let (_broadcast, producer) = track(VIDEO); + let _sub = producer.subscribe(None); + let reservation = allocator.reserve(&producer.demand(), bps(4_000_000)); + let mut grant = reservation.consumer(); + let (woken, waiter) = Woken::new(); + assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); + assert!(!grant.probed()); + assert!(grant.poll_changed(&waiter).is_pending()); + + let target = allocator.target(); + assert!(woken.woken(), "a session starting to probe must wake the reader"); + assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); + assert!(grant.probed()); + assert!(grant.poll_changed(&waiter).is_pending()); + + drop(target); + assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); + assert!(!grant.probed()); + assert!(!estimate.consume().probed(), "a whole estimate is never probed"); + } + /// The override replaces the sum for every clone until it is cleared. #[test] fn an_override_replaces_the_sum() { diff --git a/rs/moq-video/src/encode/producer.rs b/rs/moq-video/src/encode/producer.rs index 2610d823ea6..2037424571a 100644 --- a/rs/moq-video/src/encode/producer.rs +++ b/rs/moq-video/src/encode/producer.rs @@ -220,6 +220,23 @@ impl Producer { Ok(()) } + /// Advertise whether this rendition is being encoded, as the catalog's + /// [`enabled`](hang::catalog::VideoConfig::enabled). + /// + /// Disable it when the encoder stops for lack of bandwidth (a + /// [`rate::Target::Disabled`](moq_mux::rate::Target::Disabled)), so viewers select a + /// lower rendition instead of waiting for frames, and mark the + /// [`discontinuity`](Self::discontinuity) too. The advertised rendition is otherwise + /// unchanged, its maximum bitrate included. Enable it again with the keyframe that + /// resumes it. + pub fn set_enabled(&mut self, enabled: bool) -> Result<(), Error> { + match &mut self.codecs { + Codecs::H264 { import, .. } => import.set_enabled(enabled)?, + Codecs::H265 { import, .. } => import.set_enabled(enabled)?, + } + Ok(()) + } + /// Finalize the track. /// /// Borrows rather than consumes, so a later [`abort`](Self::abort) can still @@ -570,7 +587,7 @@ type RateControl = Option; /// Cancel-safe: [`Follower::poll_next`](rate::Follower::poll_next) keeps its state in /// the follower, so losing this race to a frame drops no target. #[cfg(feature = "capture")] -async fn next_target(rate: &mut RateControl) -> Result { +async fn next_target(rate: &mut RateControl) -> Result { match rate { Some(follower) => follower.next().await, // Retired: park this arm forever so `select!` ignores it. @@ -586,15 +603,18 @@ async fn next_target(rate: &mut RateControl) -> Result, + target: Result, ) { let Some(follower) = rate.as_mut() else { return }; - let Ok(bitrate) = target else { + let Ok(target) = target else { tracing::debug!("bandwidth estimate ended; holding the current encoder bitrate"); *rate = None; return; }; + let rate::Target::Enabled(bitrate) = target else { + unreachable!("a capture follows its grant with no lower rendition, so it never disables") + }; match encoder.set_bitrate(bitrate).await { Ok(()) => tracing::debug!(bitrate = bitrate.as_bps(), "adjusted encoder bitrate"), From fc4e2e0fbb3858b34de848c863c57e73ee2e9b37 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 02:47:56 -0700 Subject: [PATCH 03/15] feat(ffi): disable a video rung its grant cannot fund MoqVideoProducerOptions::lower names the next lower rendition: the rung takes the priority below it, stops encoding and advertises itself disabled below its band, and reopens on a keyframe once the grant recovers. MoqReservation::probed reports whether a session probes for it. Co-Authored-By: Claude Opus 5.5 --- rs/moq-ffi/src/bandwidth.rs | 18 +- rs/moq-ffi/src/test.rs | 10 +- rs/moq-ffi/src/video.rs | 485 +++++++++++++++++++++++++++++++----- 3 files changed, 440 insertions(+), 73 deletions(-) diff --git a/rs/moq-ffi/src/bandwidth.rs b/rs/moq-ffi/src/bandwidth.rs index 74f8d079768..0de285870d3 100644 --- a/rs/moq-ffi/src/bandwidth.rs +++ b/rs/moq-ffi/src/bandwidth.rs @@ -52,8 +52,7 @@ impl MoqBandwidth { /// One track's standing claim on a [`MoqBandwidth`]. /// /// [`grant`](Self::grant) is [`moq_net::bandwidth::Reservation::peek`]: `None` -/// means no estimate or no demand, so hold the current rate, and `Some(0)` is a -/// real zero grant. +/// means no estimate, so hold the current rate, and `Some(0)` is a real zero grant. #[derive(uniffi::Object)] pub struct MoqReservation { inner: moq_net::bandwidth::Reservation, @@ -74,13 +73,24 @@ impl MoqReservation { impl MoqReservation { /// This reservation's slice right now, in bits per second. /// - /// `None` means no estimate or no demand: hold the current rate. `Some(0)` - /// is a real zero grant. + /// `None` means no estimate: hold the current rate. `Some(0)` is a real zero + /// grant. While nothing subscribes to the track, this is the slice it would get + /// back, without claiming it from the others. pub fn grant(&self) -> Option { let _guard = crate::ffi::enter(); self.inner.peek().map(moq_net::bandwidth::Rate::as_bps) } + /// Whether a session probes the connection for this reservation, so the estimate + /// can rise to fund it even while its sender is stopped. + /// + /// Stop a sender for lack of bandwidth only while this holds: without a probing + /// session the estimate stops rising, so it could never resume. + pub fn probed(&self) -> bool { + let _guard = crate::ffi::enter(); + self.inner.consumer().probed() + } + /// Change the ceiling, keeping the same claim. pub fn update(&self, max_bps: u64) { let _guard = crate::ffi::enter(); diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index c0bede188d1..b7121b47713 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -3555,6 +3555,7 @@ async fn video_raw_publish_consume() { }, track: Some("camera".into()), bandwidth: None, + lower: None, }, ) .unwrap(); @@ -3679,6 +3680,7 @@ async fn video_decode_frame_ownership() { }, track: Some("camera".into()), bandwidth: None, + lower: None, }, ) .unwrap(); @@ -3832,6 +3834,7 @@ async fn video_raw_publish_from_many_threads() { }, track: None, bandwidth: None, + lower: None, }, ) .unwrap(); @@ -3922,7 +3925,8 @@ async fn video_raw_publish_rejects_bad_frames() { }, output: output(), track: None, - bandwidth: None + bandwidth: None, + lower: None, } ) .is_err() @@ -3935,6 +3939,7 @@ async fn video_raw_publish_rejects_bad_frames() { output: output(), track: None, bandwidth: None, + lower: None, }, ) .unwrap(); @@ -6597,6 +6602,7 @@ async fn video_encoder_follows_a_shrinking_grant() { }, track: Some("camera".into()), bandwidth: Some(bandwidth), + lower: None, }, ) .unwrap(); @@ -6676,6 +6682,7 @@ async fn dropping_a_video_producer_stops_the_rate_follower() { output, track: Some("camera".into()), bandwidth: Some(bandwidth), + lower: None, }, ) .unwrap(); @@ -6705,6 +6712,7 @@ async fn set_bitrate_caps_a_later_bandwidth_grant() { output, track: Some("camera".into()), bandwidth: Some(bandwidth), + lower: None, }, ) .unwrap(); diff --git a/rs/moq-ffi/src/video.rs b/rs/moq-ffi/src/video.rs index a462aef35cc..7ecb4868ada 100644 --- a/rs/moq-ffi/src/video.rs +++ b/rs/moq-ffi/src/video.rs @@ -152,6 +152,15 @@ pub struct MoqVideoProducerOptions { /// Session bandwidth allocator; absent keeps the configured bitrate. #[uniffi(default = None)] pub bandwidth: Option>, + /// The next lower rendition of the same source, when this one is encoded beside it. + /// + /// Makes this rendition a simulcast rung: when the bandwidth grant can no longer fund + /// it, it stops encoding and advertises itself disabled, so the lower renditions keep + /// their share, and it resumes once the grant recovers. It takes the track priority + /// one below the lower rendition's, so the allocator fills the lower one first. Needs + /// [`bandwidth`](Self::bandwidth), and a ceiling above the lower rendition's. + #[uniffi(default = None)] + pub lower: Option>, } /// How the encoded stream is divided into independently decodable groups. @@ -342,8 +351,12 @@ fn block_on(future: impl std::future::Future) -> T { /// and dropped on another unbalances the per-thread COM apartment the Windows /// backend opens. The sink owns the thread instead, so every caller is welcome. struct VideoProducer { - encoder: moq_video::encode::Sink, + /// `None` while the rendition is disabled for lack of bandwidth. + encoder: Option, producer: moq_video::encode::Producer, + /// The encoder settings, to reopen at [`ceiling`](Self::ceiling) when a disabled + /// rendition resumes. + config: moq_video::encode::Config, format: MoqVideoPixelFormat, /// The encoded resolution, from the publish config. Frames carry only pixels, /// so this is what says how to read them. @@ -352,6 +365,8 @@ struct VideoProducer { /// every grant. Behind the encoder's lock, so the follower never reads an old, /// higher one and retunes over a newer cap. ceiling: moq_net::bandwidth::Rate, + /// The track priority this rendition was published at; a higher rung takes one below. + priority: i8, } /// Stops the follow thread when the producer is finished or dropped. @@ -400,21 +415,71 @@ impl VideoProducer { MoqVideoPixelFormat::Rgba => moq_video::Surface::rgba(&frame.data, self.size)?, }; + // Disabled: nothing is encoded until the grant resumes the rendition. + let Some(encoder) = self.encoder.as_mut() else { + return Ok(()); + }; let frame = moq_video::Frame::new(surface, moq_net::Timestamp::from_micros(frame.timestamp_us)?); // A backend that pipelines hands back an earlier frame's output, so this is // zero or more access units rather than one per call. - let encoded = block_on(self.encoder.encode(frame))?; + let encoded = block_on(encoder.encode(frame))?; self.producer.publish(&encoded)?; Ok(()) } + /// Apply a rate-control target, leaving the rendition as it was on an error. + /// + /// A disable drains the encoder into the track and closes it; a resume reopens one + /// at the ceiling, whose first frame is a keyframe on every backend, and retunes it + /// to the target. An encoder that cannot retune still resumes, at its ceiling, and + /// reports [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported) so rate + /// control retires. + fn apply(&mut self, target: moq_mux::rate::Target) -> Result<(), moq_video::Error> { + let bitrate = match target { + moq_mux::rate::Target::Enabled(bitrate) => bitrate, + moq_mux::rate::Target::Disabled => { + if let Some(encoder) = self.encoder.take() { + // The encoder is gone either way, so a lost tail only costs those frames. + let drained = block_on(encoder.finish()).and_then(|encoded| self.producer.publish(&encoded)); + if let Err(err) = drained { + tracing::warn!(error = %err, "failed to drain a disabled rendition's encoder"); + } + self.producer.discontinuity()?; + } + return self.producer.set_enabled(false); + } + }; + if let Some(encoder) = self.encoder.as_mut() { + return block_on(encoder.set_bitrate(bitrate)); + } + + let mut config = self.config.clone(); + config.bitrate = Some(self.ceiling); + let mut encoder = block_on(moq_video::encode::Sink::open(&config))?; + let retuned = match bitrate < self.ceiling { + true => block_on(encoder.set_bitrate(bitrate)), + false => Ok(()), + }; + if let Err(err) = &retuned + && !matches!(err, moq_video::Error::BitrateUnsupported(_)) + { + return retuned; + } + self.producer.set_enabled(true)?; + self.encoder = Some(encoder); + retuned + } + fn finish(self, finished: &crate::producer::Finished) -> Result<(), MoqError> { let Self { encoder, mut producer, .. } = self; // Drain the codec into the track before ending it, so the last frames land // in it rather than being dropped with the encoder. - let drained = block_on(encoder.finish()).and_then(|encoded| producer.publish(&encoded)); + let drained = match encoder { + Some(encoder) => block_on(encoder.finish()).and_then(|encoded| producer.publish(&encoded)), + None => Ok(()), + }; finalize(producer, drained, finished) } } @@ -467,7 +532,9 @@ impl MoqVideoProducer { /// Encode and publish one raw frame. /// /// A backend that pipelines publishes an earlier frame's output here, so a - /// call that emits nothing on the wire is normal rather than an error. + /// call that emits nothing on the wire is normal rather than an error. While a + /// [`lower`](MoqVideoProducerOptions::lower) rung is disabled for lack of + /// bandwidth, the frame is dropped without encoding. pub fn write(&self, frame: MoqVideoFrame) -> Result<(), MoqError> { self.check()?; let _guard = crate::ffi::runtime().enter(); @@ -497,8 +564,10 @@ impl MoqVideoProducer { let mut guard = self.inner.lock().map_err(|_| panic_error())?; let producer = guard.as_mut().ok_or(MoqError::Closed)?; // A keyframe is what a cut is on the wire: the importer closes the open - // group and starts a new one at it. - block_on(producer.encoder.cut())?; + // group and starts a new one at it. A disabled rung resumes on a keyframe anyway. + if let Some(encoder) = producer.encoder.as_mut() { + block_on(encoder.cut())?; + } Ok(()) } @@ -525,7 +594,10 @@ impl MoqVideoProducer { let mut guard = self.inner.lock().map_err(|_| panic_error())?; let producer = guard.as_mut().ok_or(MoqError::Closed)?; let bitrate = moq_net::bandwidth::Rate::from_bps(bitrate); - block_on(producer.encoder.set_bitrate(bitrate))?; + // A disabled rung has no encoder to retune; it reopens at the new ceiling. + if let Some(encoder) = producer.encoder.as_mut() { + block_on(encoder.set_bitrate(bitrate))?; + } // Under the same lock as the retune, so the follower sees both or neither. producer.ceiling = bitrate; } @@ -584,7 +656,8 @@ impl MoqVideoProducer { /// Set `options.bandwidth` to reserve this track's configured bitrate against the /// session's allocator and follow the grant with the same policy the Rust /// capture encoder uses. [`set_bitrate`](MoqVideoProducer::set_bitrate) is - /// the manual ceiling: it retunes the encoder and moves the reservation. + /// the manual ceiling: it retunes the encoder and moves the reservation. Set + /// `options.lower` too for each rendition above the lowest of one source. #[uniffi::constructor] pub fn new( catalog: &crate::media::MoqMediaCatalogProducer, @@ -598,8 +671,26 @@ impl MoqVideoProducer { output, track, bandwidth, + lower, } = options; let config = video_encoder_config(&input, &output)?; + // The lower rendition's handle, its ceiling, and the priority this one takes below it. + let lower = match lower { + Some(lower) => { + if bandwidth.is_none() { + return Err(MoqError::Video { + reason: "a lower rendition needs a bandwidth allocator to follow".into(), + }); + } + let guard = lower.inner.lock().map_err(|_| panic_error())?; + let below = guard.as_ref().ok_or(MoqError::Closed)?; + Some((Arc::downgrade(&lower.inner), below.ceiling, below.priority.saturating_sub(1))) + } + None => None, + }; + let priority = lower + .as_ref() + .map_or(hang::catalog::PRIORITY.video, |(_, _, priority)| *priority); // Both before the track exists: a config this machine can't encode should fail // without leaving a track advertised that will never carry frames. The probe @@ -608,31 +699,50 @@ impl MoqVideoProducer { let encoder = block_on(moq_video::encode::Sink::open(&config))?; let gop = config.gop.duration(config.framerate); let ceiling = video_ceiling(&config, &rendition); - let finished = broadcast.with_state(|state| Ok(state.finished.clone()))?; - let producer = broadcast.with_state(|state| match track { - Some(name) => { - let track = state - .broadcast - .create_track(name, state.catalog.track_info(hang::catalog::PRIORITY.video))?; - Ok(moq_video::encode::Producer::with_track( - track, - state.catalog.reserve().with_timeline_hint(gop), - rendition, - )?) + if let Some((_, below, _)) = &lower + && *below >= ceiling + { + return Err(MoqError::Video { + reason: format!( + "the lower rendition's {} bps is not below this one's {} bps", + below.as_bps(), + ceiling.as_bps() + ), + }); + } + // The suffix `moq_video::encode::Producer::new` would derive, built here so the + // track can take this rendition's priority. + let suffix = match &rendition.codec { + hang::catalog::VideoCodec::H264(_) => ".avc3", + hang::catalog::VideoCodec::H265(_) => ".hev1", + other => { + return Err(MoqError::Video { + reason: format!("{other} is not a codec this producer can publish"), + }); } - None => Ok(moq_video::encode::Producer::new( - state.broadcast.clone(), + }; + let finished = broadcast.with_state(|state| Ok(state.finished.clone()))?; + let producer = broadcast.with_state(|state| { + let info = state.catalog.track_info(priority); + let track = match track { + Some(name) => state.broadcast.create_track(name, info)?, + None => state.broadcast.unique_track(suffix, info)?, + }; + Ok(moq_video::encode::Producer::with_track( + track, state.catalog.reserve().with_timeline_hint(gop), rendition, - )?), + )?) })?; let inner = Arc::new(std::sync::Mutex::new(Some(VideoProducer { - encoder, + encoder: Some(encoder), producer, format: input.format, size: config.size(), + config, ceiling, + priority, }))); let applied = Arc::new(AtomicU64::new(ceiling.as_bps())); let reserved = bandwidth.as_ref().map(|bandwidth| { @@ -645,9 +755,16 @@ impl MoqVideoProducer { .demand(); bandwidth.reserve_demand(&demand, ceiling.as_bps()) }); - let follow = reserved - .as_ref() - .map(|reservation| spawn_follow(inner.clone(), reservation.consumer(), ceiling, applied.clone())); + let lower = lower.map(|(lower, _, _)| lower); + let follow = reserved.as_ref().map(|reservation| { + spawn_follow(Followed { + inner: inner.clone(), + lower, + consumer: reservation.consumer(), + ceiling, + applied: applied.clone(), + }) + }); Ok(Arc::new(MoqVideoProducer { inner, @@ -677,12 +794,18 @@ fn video_ceiling( }) } -fn spawn_follow( +/// What the follow thread reads and retunes. +struct Followed { inner: Arc>>, + /// The next lower rendition, whose ceiling sets this one's band. Weak, so a rung never + /// keeps the rendition below it publishing. + lower: Option>>>, consumer: moq_net::bandwidth::Consumer, ceiling: moq_net::bandwidth::Rate, applied: Arc, -) -> Follow { +} + +fn spawn_follow(followed: Followed) -> Follow { let (close, closed) = oneshot::channel(); let failed = Arc::new(AtomicBool::new(false)); let failure = failed.clone(); @@ -700,7 +823,7 @@ fn spawn_follow( tokio::select! { biased; _ = closed => {} - _ = follow_reservation(inner, consumer, ceiling, applied) => {} + _ = follow_reservation(followed) => {} } }) })); @@ -716,64 +839,87 @@ fn spawn_follow( } } +/// The policy a rendition follows: its manual `ceiling`, above the `lower` rendition's +/// current ceiling, if any. +fn video_policy(ceiling: moq_net::bandwidth::Rate, lower: Option) -> moq_mux::rate::Policy { + let mut policy = moq_mux::rate::Policy::new(ceiling); + policy.lower = lower; + policy +} + +/// The lower rendition's current ceiling, or `None` once it finished or was dropped, +/// which leaves this one nothing to fall back to, so it never disables. +fn lower_ceiling( + lower: &Option>>>, +) -> Option { + let lower = lower.as_ref()?.upgrade()?; + let guard = lower.lock().ok()?; + guard.as_ref().map(|producer| producer.ceiling) +} + /// Feed each grant through the same policy moq-video uses, and retune the live -/// encoder. Retires on [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported). -async fn follow_reservation( - inner: Arc>>, - consumer: moq_net::bandwidth::Consumer, - ceiling: moq_net::bandwidth::Rate, - applied: Arc, -) { - let mut follower = moq_mux::rate::Follower::new(consumer, moq_mux::rate::Policy::new(ceiling)); +/// encoder, disabling and resuming a rung. Retires on +/// [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported). +async fn follow_reservation(followed: Followed) { + let Followed { + inner, + lower, + consumer, + ceiling, + applied, + } = followed; + let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower))); loop { - // The ceiling is read under the encoder lock on every wake, before the follower - // filters a grant against it. `set_bitrate` updates the reservation after - // moving it, which wakes this poll. + // Both ceilings are read on every wake, before the follower filters a grant against + // them: `set_bitrate` (here or on the lower rendition) moves a reservation after + // moving its ceiling, which wakes this poll. The lower one is read first, so no + // two encoder locks are ever held at once. let next = kio::wait(|waiter| { + let below = lower_ceiling(&lower); let guard = inner.lock().unwrap(); let Some(producer) = guard.as_ref() else { return std::task::Poll::Ready(None); }; - follower.set_policy(moq_mux::rate::Policy::new(producer.ceiling)); + follower.set_policy(video_policy(producer.ceiling, below)); follower.poll_next(waiter).map(Some) }) .await; - let Some(Ok(bitrate)) = next else { + let Some(Ok(target)) = next else { return; }; + let below = lower_ceiling(&lower); let mut guard = inner.lock().unwrap(); let Some(producer) = guard.as_mut() else { return; }; - let ceiling = producer.ceiling; - let set = |bitrate| block_on(producer.encoder.set_bitrate(bitrate)); - match retune(&mut follower, ceiling, bitrate, set) { - Ok(Some(bitrate)) => applied.store(bitrate.as_bps(), Ordering::SeqCst), + let policy = video_policy(producer.ceiling, below); + match retune(&mut follower, policy, target, |target| producer.apply(target)) { + Ok(Some(moq_mux::rate::Target::Enabled(bitrate))) => applied.store(bitrate.as_bps(), Ordering::SeqCst), + Ok(Some(moq_mux::rate::Target::Disabled)) => applied.store(0, Ordering::SeqCst), Ok(None) => {} Err(_) => return, } } } -/// Retune through `set` to the follower's next `bitrate`, unless the manual `ceiling` -/// moved since it was asked for: then follow under the new ceiling, which -/// `MoqVideoProducer::set_bitrate` already applied, instead. Returns the bitrate applied, or -/// `Err` with the name of an encoder that cannot follow at all. +/// Apply the follower's next `target` through `apply`, unless the `policy` moved since it +/// was asked for: then follow under the new policy, whose manual ceiling +/// `MoqVideoProducer::set_bitrate` already applied, instead. Returns the target applied, +/// or `Err` with the name of an encoder that cannot follow at all. fn retune( follower: &mut moq_mux::rate::Follower, - ceiling: moq_net::bandwidth::Rate, - bitrate: moq_net::bandwidth::Rate, - set: impl FnOnce(moq_net::bandwidth::Rate) -> Result<(), moq_video::Error>, -) -> Result, &'static str> { - let policy = moq_mux::rate::Policy::new(ceiling); + policy: moq_mux::rate::Policy, + target: moq_mux::rate::Target, + apply: impl FnOnce(moq_mux::rate::Target) -> Result<(), moq_video::Error>, +) -> Result, &'static str> { if *follower.policy() != policy { follower.set_policy(policy); return Ok(None); } - match set(bitrate) { + match apply(target) { Ok(()) => { - tracing::debug!(bitrate = bitrate.as_bps(), "adjusted encoder bitrate"); - Ok(Some(bitrate)) + tracing::debug!(?target, "adjusted encoder bitrate"); + Ok(Some(target)) } Err(moq_video::Error::BitrateUnsupported(name)) => { tracing::warn!(encoder = name, "encoder cannot follow the bandwidth estimate"); @@ -782,7 +928,7 @@ fn retune( // The follower takes the target back and retries it, so a quiet grant still // recovers. Err(err) => { - tracing::warn!(error = %err, bitrate = bitrate.as_bps(), "failed to adjust encoder bitrate"); + tracing::warn!(error = %err, ?target, "failed to adjust encoder bitrate"); follower.failed(); Ok(None) } @@ -1204,6 +1350,7 @@ mod finish_tests { output, track: Some("camera".into()), bandwidth: Some(bandwidth), + lower: None, }, ) .unwrap(); @@ -1235,15 +1382,36 @@ mod finish_tests { mod follow_tests { use std::time::Duration; - use moq_mux::rate::{Follower, Policy}; + use moq_mux::rate::{Follower, Policy, Target}; use moq_net::bandwidth::Rate; use super::*; + /// Retune through `set`, which only ever sees an enabled target: these policies set no + /// lower rendition, so nothing disables. + fn rate( + follower: &mut Follower, + ceiling: Rate, + target: Target, + set: impl FnOnce(Rate) -> Result<(), moq_video::Error>, + ) -> Result, &'static str> { + retune(follower, Policy::new(ceiling), target, |target| match target { + Target::Enabled(bitrate) => set(bitrate), + Target::Disabled => unreachable!("no lower rendition"), + }) + } + + fn bitrate(target: Target) -> Rate { + let Target::Enabled(bitrate) = target else { + unreachable!("no lower rendition") + }; + bitrate + } + /// Retune at every target until the follower goes a minute without one. async fn quiet(follower: &mut Follower, ceiling: Rate, mut set: impl FnMut(Rate) -> Result<(), moq_video::Error>) { while let Ok(bitrate) = moq_clock::timeout(Duration::from_secs(60), follower.next()).await { - retune(follower, ceiling, bitrate.unwrap(), &mut set).unwrap(); + rate(follower, ceiling, bitrate.unwrap(), &mut set).unwrap(); } } @@ -1266,7 +1434,7 @@ mod follow_tests { grant.set(Some(Rate::from_mbps(1))).unwrap(); let bitrate = follower.next().await.unwrap(); - retune(&mut follower, ceiling, bitrate, &mut set).unwrap(); + rate(&mut follower, ceiling, bitrate, &mut set).unwrap(); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(ceiling)).unwrap(); quiet(&mut follower, ceiling, &mut set).await; @@ -1291,24 +1459,24 @@ mod follow_tests { }; grant.set(Some(Rate::from_mbps(1))).unwrap(); - assert_eq!(follower.next().await.unwrap(), Rate::from_mbps(1)); + assert_eq!(follower.next().await.unwrap(), Target::Enabled(Rate::from_mbps(1))); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(old)).unwrap(); let first = follower.next().await.unwrap(); - retune(&mut follower, old, first, set).unwrap(); + rate(&mut follower, old, first, set).unwrap(); // `set_bitrate` already retuned the encoder to the new cap. let due = follower.next().await.unwrap(); - assert!(due > first, "a raise toward the old ceiling"); - assert_eq!(retune(&mut follower, lowered, due, set), Ok(None)); + assert!(bitrate(due) > bitrate(first), "a raise toward the old ceiling"); + assert_eq!(rate(&mut follower, lowered, due, set), Ok(None)); assert_eq!(follower.policy().max, lowered); quiet(&mut follower, lowered, set).await; - assert_eq!(*applied.borrow(), [first], "nothing retuned past the new cap"); + assert_eq!(*applied.borrow(), [bitrate(first)], "nothing retuned past the new cap"); // A later attack and recovery climbs back to the new cap, not the old one. grant.set(Some(Rate::from_mbps(1))).unwrap(); let bitrate = follower.next().await.unwrap(); - retune(&mut follower, lowered, bitrate, set).unwrap(); + rate(&mut follower, lowered, bitrate, set).unwrap(); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(old)).unwrap(); quiet(&mut follower, lowered, set).await; @@ -1318,3 +1486,184 @@ mod follow_tests { assert!(applied.iter().all(|bitrate| *bitrate <= lowered), "{applied:?}"); } } + +#[cfg(test)] +mod rung_tests { + use moq_mux::rate::Target; + use moq_net::bandwidth::Rate; + + use super::*; + + fn input() -> MoqVideoEncoderInput { + MoqVideoEncoderInput { + format: MoqVideoPixelFormat::I420, + width: 320, + height: 240, + framerate: MoqVideoFrameRate { + numerator: 30, + denominator: 1, + }, + } + } + + fn output(bitrate: u64) -> MoqVideoEncoderOutput { + MoqVideoEncoderOutput { + codec: MoqVideoCodec::H264, + bitrate: Some(bitrate), + gop: None, + kind: MoqVideoEncoderKind::Software, + } + } + + fn frame(i: u64) -> MoqVideoFrame { + MoqVideoFrame { + timestamp_us: i * 33_333, + data: vec![(i * 40) as u8; 320 * 240 * 3 / 2], + } + } + + struct Ladder { + broadcast: Arc, + low: Arc, + high: Arc, + } + + fn ladder() -> Ladder { + let estimate = moq_net::bandwidth::Producer::new(); + let bandwidth = Arc::new(MoqBandwidth::new(moq_net::bandwidth::Allocator::new( + estimate.consume(), + ))); + let broadcast = crate::producer::MoqBroadcastProducer::new().unwrap(); + let catalog = crate::media::MoqMediaCatalogProducer::new(broadcast.clone()).unwrap(); + let publish = |track: &str, bitrate, lower| { + MoqVideoProducer::new( + &catalog, + MoqVideoProducerOptions { + input: input(), + output: output(bitrate), + track: Some(track.into()), + bandwidth: Some(bandwidth.clone()), + lower, + }, + ) + }; + let low = publish("low", 500_000, None).unwrap(); + let high = publish("high", 2_000_000, Some(low.clone())).unwrap(); + Ladder { broadcast, low, high } + } + + fn enabled(broadcast: &crate::producer::MoqBroadcastProducer, track: &str) -> bool { + broadcast + .with_state(|state| Ok(state.catalog.snapshot().video.renditions[track].enabled)) + .unwrap() + } + + async fn priority(broadcast: &crate::producer::MoqBroadcastProducer, track: &str) -> i8 { + let track = broadcast + .consume() + .unwrap() + .subscribe_track(track.into(), None) + .await + .unwrap(); + track.info().unwrap().priority + } + + /// A rung sits one priority below its lower rendition, so the allocator fills that first. + #[tokio::test] + async fn a_rung_takes_the_priority_below_its_lower() { + let ladder = ladder(); + assert_eq!(priority(&ladder.broadcast, "low").await, hang::catalog::PRIORITY.video); + assert_eq!(priority(&ladder.broadcast, "high").await, hang::catalog::PRIORITY.video - 1); + } + + /// A rung needs an allocator to follow and a ceiling above its lower rendition's. + #[tokio::test] + async fn a_rung_without_a_band_or_a_grant_is_refused() { + let ladder = ladder(); + let catalog = crate::media::MoqMediaCatalogProducer::new(ladder.broadcast.clone()).unwrap(); + let below = MoqVideoProducer::new( + &catalog, + MoqVideoProducerOptions { + input: input(), + output: output(400_000), + track: Some("below".into()), + bandwidth: Some(Arc::new(MoqBandwidth::new(moq_net::bandwidth::Allocator::unlimited()))), + lower: Some(ladder.low.clone()), + }, + ); + assert!(matches!(below, Err(MoqError::Video { .. }))); + let ungranted = MoqVideoProducer::new( + &catalog, + MoqVideoProducerOptions { + input: input(), + output: output(4_000_000), + track: Some("ungranted".into()), + bandwidth: None, + lower: Some(ladder.high.clone()), + }, + ); + assert!(matches!(ungranted, Err(MoqError::Video { .. }))); + } + + /// A disabled rung advertises itself disabled, drops the frames written to it, and + /// resumes on a keyframe in a new group, its advertised maximum untouched throughout. + #[tokio::test] + async fn a_disabled_rung_stops_encoding_until_it_resumes() { + let ladder = ladder(); + let track = ladder + .broadcast + .consume() + .unwrap() + // Every group, however old, since the track is read only once it has ended. + .subscribe_track( + "high".into(), + Some(crate::consumer::MoqSubscription { + priority: 0, + max_delay_us: None, + ranges: None, + order: None, + }), + ) + .await + .unwrap(); + ladder.high.write(frame(0)).unwrap(); + let bitrate = |ladder: &Ladder| { + ladder + .broadcast + .with_state(|state| Ok(state.catalog.snapshot().video.renditions["high"].bitrate)) + .unwrap() + }; + let advertised = bitrate(&ladder); + + let apply = |target| { + let mut guard = ladder.high.inner.lock().unwrap(); + guard.as_mut().unwrap().apply(target).unwrap(); + }; + apply(Target::Disabled); + assert!(!enabled(&ladder.broadcast, "high")); + assert!(enabled(&ladder.broadcast, "low"), "only the rung disables"); + for i in 1..10 { + ladder.high.write(frame(i)).unwrap(); + } + ladder.high.cut().unwrap(); + ladder.high.set_bitrate(3_000_000).unwrap(); + + apply(Target::Enabled(Rate::from_kbps(1_500))); + assert!(enabled(&ladder.broadcast, "high")); + ladder.high.write(frame(10)).unwrap(); + ladder.high.finish().unwrap(); + assert_eq!(bitrate(&ladder), advertised, "the advertised maximum never moves"); + + // The opening keyframe, the disable's discontinuity marker, then the resumed + // keyframe, each its own group: nothing written while disabled was encoded. + let mut groups = Vec::new(); + while let Some(group) = track.recv_group().await.unwrap() { + let mut frames = 0; + while group.read_frame().await.unwrap().is_some() { + frames += 1; + } + groups.push(frames); + } + assert_eq!(groups, [1, 1, 1]); + } +} From 6bb7266d7cf43561a572f25b6360035176b048e0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 02:51:06 -0700 Subject: [PATCH 04/15] feat(c)!: disable a video rung its grant cannot fund moq_encode_video takes the next lower rendition's handle, mirroring MoqVideoProducerOptions::lower. Co-Authored-By: Claude Opus 5.5 --- rs/moq-c/c-tests/decode-output.c | 2 +- rs/moq-c/src/state.rs | 5 + rs/moq-c/src/test.rs | 68 ++++++-- rs/moq-c/src/video.rs | 262 +++++++++++++++++++++++++------ 4 files changed, 275 insertions(+), 62 deletions(-) diff --git a/rs/moq-c/c-tests/decode-output.c b/rs/moq-c/c-tests/decode-output.c index 1293a21080b..3802b78c56b 100644 --- a/rs/moq-c/c-tests/decode-output.c +++ b/rs/moq-c/c-tests/decode-output.c @@ -237,7 +237,7 @@ int main(void) { moq_video_encoder_input input = {MOQ_VIDEO_PIXEL_FORMAT_RGBA, WIDTH, HEIGHT, {30000, 1001}}; moq_video_encoder_output output = {MOQ_VIDEO_CODEC_H264, 0, 0, MOQ_VIDEO_ENCODER_KIND_SOFTWARE, NULL, 0}; - int32_t producer = moq_encode_video((uint32_t)broadcast, &input, &output, 0); + int32_t producer = moq_encode_video((uint32_t)broadcast, &input, &output, 0, 0); if (producer <= 0) fail("error: moq_encode_video failed: %d (%s)\n", producer, moq_error()); if (moq_encode_video_cut((uint32_t)producer) < 0) diff --git a/rs/moq-c/src/state.rs b/rs/moq-c/src/state.rs index ec7832ee658..e2b99689598 100644 --- a/rs/moq-c/src/state.rs +++ b/rs/moq-c/src/state.rs @@ -63,6 +63,11 @@ impl Shared { self.0.lock().unwrap_or_else(PoisonError::into_inner) } + /// A handle that reads the resource without keeping it alive. + pub fn downgrade(&self) -> std::sync::Weak>> { + Arc::downgrade(&self.0) + } + /// Take the resource, for the call that ends it and needs to consume it. pub fn take(&self) -> Option { self.lock().take() diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 731d4bc3920..346632aaa08 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -4187,7 +4187,7 @@ fn video_raw_publish_consume() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); let rgba = gray_rgba(320, 240); let publish = |index: u64| { @@ -4354,7 +4354,7 @@ fn decode_first_frame(output: &moq_video_decoder_output) -> (u32, u32, usize) { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &encode_output, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &encode_output, 0, 0) }); let rgba = gray_rgba(320, 240); let publish = |index: u64| { @@ -4472,7 +4472,7 @@ fn video_raw_publish_from_many_threads() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); // A fresh caller thread per frame, never the one that published. let rgba = std::sync::Arc::new(gray_rgba(320, 240)); @@ -4544,8 +4544,8 @@ fn a_stalled_encode_does_not_block_unrelated_calls() { encoder: std::ptr::null(), encoder_len: 0, }; - let stalled = id(unsafe { moq_encode_video(broadcast, &input, &output, 0) }); - let other = id(unsafe { moq_encode_video(broadcast, &input, &output, 0) }); + let stalled = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); + let other = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); // Hold the lock a publish takes for the duration of its encode. let handle = State::lock().video.producer(Id::try_from(stalled).unwrap()).unwrap(); @@ -4626,7 +4626,7 @@ fn video_raw_publish_rejects_frame_size_mismatch() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); // A 640x480 buffer against a 320x240 encoder: the frame carries no dimensions // of its own, so this is caught as a wrong-sized picture. @@ -4668,14 +4668,14 @@ fn video_raw_publish_rejects_invalid_config() { encoder_len: 0, }; - assert!(unsafe { moq_encode_video(broadcast, std::ptr::null(), &valid_output, 0) } < 0); - assert!(unsafe { moq_encode_video(broadcast, &valid_input, std::ptr::null(), 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, std::ptr::null(), &valid_output, 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, std::ptr::null(), 0, 0) } < 0); let bad_format = moq_video_encoder_input { format: 99, ..valid_input }; - assert!(unsafe { moq_encode_video(broadcast, &bad_format, &valid_output, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &bad_format, &valid_output, 0, 0) } < 0); let zero_framerate = moq_video_encoder_input { framerate: moq_video_frame_rate { @@ -4684,7 +4684,7 @@ fn video_raw_publish_rejects_invalid_config() { }, ..valid_input }; - assert!(unsafe { moq_encode_video(broadcast, &zero_framerate, &valid_output, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &zero_framerate, &valid_output, 0, 0) } < 0); let zero_denominator = moq_video_encoder_input { framerate: moq_video_frame_rate { numerator: 30_000, @@ -4692,7 +4692,7 @@ fn video_raw_publish_rejects_invalid_config() { }, ..valid_input }; - let refused = unsafe { moq_encode_video(broadcast, &zero_denominator, &valid_output, 0) }; + let refused = unsafe { moq_encode_video(broadcast, &zero_denominator, &valid_output, 0, 0) }; assert!(refused < 0 && refused != MOQ_ERROR_PANIC); // Regression: dimensions arrive as a raw `u32` pair, and their product used to @@ -4703,7 +4703,7 @@ fn video_raw_publish_rejects_invalid_config() { height: u32::MAX - 1, ..valid_input }; - let refused = unsafe { moq_encode_video(broadcast, &unrepresentable, &valid_output, 0) }; + let refused = unsafe { moq_encode_video(broadcast, &unrepresentable, &valid_output, 0, 0) }; assert!(refused < 0 && refused != MOQ_ERROR_PANIC, "{refused}"); // A size no encoder can take, but whose arithmetic is fine, is the backend's @@ -4718,7 +4718,7 @@ fn video_raw_publish_rejects_invalid_config() { height: 2, ..valid_input }; - let huge = unsafe { moq_encode_video(broadcast, &merely_huge, &valid_output, 0) }; + let huge = unsafe { moq_encode_video(broadcast, &merely_huge, &valid_output, 0, 0) }; if huge > 0 { assert_eq!(moq_encode_video_finish(id(huge)), 0); } else { @@ -4733,13 +4733,13 @@ fn video_raw_publish_rejects_invalid_config() { codec: 99, ..valid_output }; - assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_codec, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_codec, 0, 0) } < 0); let bad_kind = moq_video_encoder_output { kind: 99, ..valid_output }; - assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_kind, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_kind, 0, 0) } < 0); // Handles for a producer that was never created. assert!(moq_encode_video_cut(0) < 0); @@ -5637,7 +5637,7 @@ fn encode_video_bitrate_caps_the_reservation() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, bandwidth) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, bandwidth, 0) }); let reservation = id(moq_encode_video_reservation(producer)); let (sub, cb) = subscribe_named_track(consume, b"0.avc3"); @@ -5664,6 +5664,42 @@ fn encode_video_bitrate_caps_the_reservation() { assert_eq!(moq_origin_close(origin), 0); } +/// A simulcast rung needs a bandwidth allocator and a bitrate above its lower rendition's. +#[test] +fn encode_video_rung_needs_a_grant_and_a_band() { + let (bandwidth, _estimate) = test_allocator(); + let origin = id(moq_origin_create()); + let broadcast = publish_broadcast(origin, b"rung"); + + let input = moq_video_encoder_input { + format: moq_video_pixel_format::MOQ_VIDEO_PIXEL_FORMAT_RGBA as u32, + width: 320, + height: 240, + framerate: moq_video_frame_rate { + numerator: 30, + denominator: 1, + }, + }; + let output = |bitrate| moq_video_encoder_output { + codec: moq_video_codec::MOQ_VIDEO_CODEC_H264 as u32, + bitrate, + gop: 0, + kind: moq_video_encoder_kind::MOQ_VIDEO_ENCODER_KIND_SOFTWARE as u32, + encoder: std::ptr::null(), + encoder_len: 0, + }; + let low = id(unsafe { moq_encode_video(broadcast, &input, &output(500_000), bandwidth, 0) }); + assert!(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), 0, low) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &input, &output(400_000), bandwidth, low) } < 0); + let high = id(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), bandwidth, low) }); + + assert_eq!(moq_encode_video_finish(high), 0); + assert_eq!(moq_encode_video_finish(low), 0); + assert_eq!(moq_bandwidth_close(bandwidth), 0); + assert_eq!(moq_publish_close(broadcast), 0); + assert_eq!(moq_origin_close(origin), 0); +} + /// Listen on an ephemeral loopback port with a generated certificate. fn listen(cb: &Callback) -> (u32, String, String) { let bind = "127.0.0.1:0"; diff --git a/rs/moq-c/src/video.rs b/rs/moq-c/src/video.rs index ced9a90b0b3..c374b35ca9c 100644 --- a/rs/moq-c/src/video.rs +++ b/rs/moq-c/src/video.rs @@ -226,61 +226,84 @@ fn video_ceiling( }) } +/// The policy a rendition follows: its manual `ceiling`, above the `lower` rendition's +/// current ceiling, if any. +fn video_policy(ceiling: moq_net::bandwidth::Rate, lower: Option) -> moq_mux::rate::Policy { + let mut policy = moq_mux::rate::Policy::new(ceiling); + policy.lower = lower; + policy +} + +/// The next lower rendition, whose ceiling sets a rung's band. Weak, so a rung never +/// keeps the rendition below it publishing. +type Lower = std::sync::Weak>>; + +/// The lower rendition's current ceiling, or `None` once it finished, which leaves +/// this one nothing to fall back to, so it never disables. +fn lower_ceiling(lower: &Option) -> Option { + let lower = lower.as_ref()?.upgrade()?; + let guard = lower.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + guard.as_ref().map(|encoder| encoder.ceiling) +} + /// Feed each grant through the same policy moq-video uses, and retune the live -/// encoder. Retires on [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported). +/// encoder, disabling and resuming a rung. Retires on +/// [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported). async fn follow_reservation( inner: Shared, + lower: Option, consumer: moq_net::bandwidth::Consumer, ceiling: moq_net::bandwidth::Rate, ) { - let mut follower = moq_mux::rate::Follower::new(consumer, moq_mux::rate::Policy::new(ceiling)); + let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower))); loop { - // The ceiling is read under the encoder lock on every wake, before the follower - // filters a grant against it. `moq_encode_video_bitrate` updates the reservation - // after moving it, which wakes this poll. + // Both ceilings are read on every wake, before the follower filters a grant against + // them: `moq_encode_video_bitrate` (here or on the lower rendition) moves a + // reservation after moving its ceiling, which wakes this poll. The lower one is + // read first, so no two encoder locks are ever held at once. let next = kio::wait(|waiter| { + let below = lower_ceiling(&lower); let guard = inner.lock(); let Some(producer) = guard.as_ref() else { return std::task::Poll::Ready(None); }; - follower.set_policy(moq_mux::rate::Policy::new(producer.ceiling)); + follower.set_policy(video_policy(producer.ceiling, below)); follower.poll_next(waiter).map(Some) }) .await; - let Some(Ok(bitrate)) = next else { + let Some(Ok(target)) = next else { return; }; + let below = lower_ceiling(&lower); let mut guard = inner.lock(); let Some(producer) = guard.as_mut() else { return; }; - let ceiling = producer.ceiling; - let set = |bitrate| block_on(producer.encoder.set_bitrate(bitrate)); - if retune(&mut follower, ceiling, bitrate, set).is_err() { + let policy = video_policy(producer.ceiling, below); + if retune(&mut follower, policy, target, |target| producer.apply(target)).is_err() { return; } } } -/// Retune through `set` to the follower's next `bitrate`, unless the manual `ceiling` -/// moved since it was asked for: then follow under the new ceiling, which -/// `moq_encode_video_bitrate` already applied, instead. Returns the bitrate applied, or +/// Apply the follower's next `target` through `apply`, unless the `policy` moved since it +/// was asked for: then follow under the new policy, whose manual ceiling +/// `moq_encode_video_bitrate` already applied, instead. Returns the target applied, or /// `Err` with the name of an encoder that cannot follow at all. fn retune( follower: &mut moq_mux::rate::Follower, - ceiling: moq_net::bandwidth::Rate, - bitrate: moq_net::bandwidth::Rate, - set: impl FnOnce(moq_net::bandwidth::Rate) -> Result<(), moq_video::Error>, -) -> Result, &'static str> { - let policy = moq_mux::rate::Policy::new(ceiling); + policy: moq_mux::rate::Policy, + target: moq_mux::rate::Target, + apply: impl FnOnce(moq_mux::rate::Target) -> Result<(), moq_video::Error>, +) -> Result, &'static str> { if *follower.policy() != policy { follower.set_policy(policy); return Ok(None); } - match set(bitrate) { + match apply(target) { Ok(()) => { - tracing::debug!(bitrate = bitrate.as_bps(), "adjusted encoder bitrate"); - Ok(Some(bitrate)) + tracing::debug!(?target, "adjusted encoder bitrate"); + Ok(Some(target)) } Err(moq_video::Error::BitrateUnsupported(name)) => { tracing::warn!(encoder = name, "encoder cannot follow the bandwidth estimate"); @@ -289,7 +312,7 @@ fn retune( // The follower takes the target back and retries it, so a quiet grant still // recovers. Err(err) => { - tracing::warn!(error = %err, bitrate = bitrate.as_bps(), "failed to adjust encoder bitrate"); + tracing::warn!(error = %err, ?target, "failed to adjust encoder bitrate"); follower.failed(); Ok(None) } @@ -306,8 +329,16 @@ fn retune( /// unbalancing the per-thread COM apartment the Windows backend opens. The sink /// owns the thread instead, so every caller is welcome. pub(crate) struct VideoEncoder { - encoder: moq_video::encode::Sink, + /// `None` while the rendition is disabled for lack of bandwidth. + encoder: Option, producer: moq_video::encode::Producer, + /// The encoder settings, to reopen at [`ceiling`](Self::ceiling) when a disabled + /// rendition resumes. + config: moq_video::encode::Config, + /// The track priority this rendition was published at; a higher rung takes one below. + priority: i8, + /// The next lower rendition, set when this one is a simulcast rung. + lower: Option, format: moq_video_pixel_format, /// The encoded resolution, from the publish config. Frames carry only pixels, /// so this is what says how to read them. @@ -424,10 +455,14 @@ impl VideoEncoder { moq_video_pixel_format::MOQ_VIDEO_PIXEL_FORMAT_RGBA => moq_video::Surface::rgba(data, size)?, }; + // Disabled: nothing is encoded until the grant resumes the rendition. + let Some(encoder) = self.encoder.as_mut() else { + return Ok(()); + }; let frame = moq_video::Frame::new(surface, moq_net::Timestamp::from_micros(timestamp_us)?); // A backend that pipelines hands back an earlier frame's output, so this is // zero or more access units rather than one per call. - let encoded = block_on(self.encoder.encode(frame))?; + let encoded = block_on(encoder.encode(frame))?; self.producer.publish(&encoded)?; Ok(()) } @@ -437,8 +472,11 @@ impl VideoEncoder { followed.check()?; } // A keyframe is what a cut is on the wire: the importer closes the open - // group and starts a new one at it. - Ok(block_on(self.encoder.cut())?) + // group and starts a new one at it. A disabled rung resumes on a keyframe anyway. + match self.encoder.as_mut() { + Some(encoder) => Ok(block_on(encoder.cut())?), + None => Ok(()), + } } fn publish_bitrate(&mut self, bitrate: u64) -> Result<(), Error> { @@ -446,7 +484,10 @@ impl VideoEncoder { followed.check()?; } let bitrate = moq_net::bandwidth::Rate::from_bps(bitrate); - block_on(self.encoder.set_bitrate(bitrate))?; + // A disabled rung has no encoder to retune; it reopens at the new ceiling. + if let Some(encoder) = self.encoder.as_mut() { + block_on(encoder.set_bitrate(bitrate))?; + } self.ceiling = bitrate; if let Some(reservation) = &self.reservation { reservation.update(bitrate); @@ -454,6 +495,49 @@ impl VideoEncoder { Ok(()) } + /// Apply a rate-control target, leaving the rendition as it was on an error. + /// + /// A disable drains the encoder into the track and closes it; a resume reopens one + /// at the ceiling, whose first frame is a keyframe on every backend, and retunes it + /// to the target. An encoder that cannot retune still resumes, at its ceiling, and + /// reports [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported) so rate + /// control retires. + fn apply(&mut self, target: moq_mux::rate::Target) -> Result<(), moq_video::Error> { + let bitrate = match target { + moq_mux::rate::Target::Enabled(bitrate) => bitrate, + moq_mux::rate::Target::Disabled => { + if let Some(encoder) = self.encoder.take() { + // The encoder is gone either way, so a lost tail only costs those frames. + let drained = block_on(encoder.finish()).and_then(|encoded| self.producer.publish(&encoded)); + if let Err(err) = drained { + tracing::warn!(error = %err, "failed to drain a disabled rendition's encoder"); + } + self.producer.discontinuity()?; + } + return self.producer.set_enabled(false); + } + }; + if let Some(encoder) = self.encoder.as_mut() { + return block_on(encoder.set_bitrate(bitrate)); + } + + let mut config = self.config.clone(); + config.bitrate = Some(self.ceiling); + let mut encoder = block_on(moq_video::encode::Sink::open(&config))?; + let retuned = match bitrate < self.ceiling { + true => block_on(encoder.set_bitrate(bitrate)), + false => Ok(()), + }; + if let Err(err) = &retuned + && !matches!(err, moq_video::Error::BitrateUnsupported(_)) + { + return retuned; + } + self.producer.set_enabled(true)?; + self.encoder = Some(encoder); + retuned + } + fn publish_finish(self) -> Result<(), Error> { let VideoEncoder { encoder, @@ -464,7 +548,10 @@ impl VideoEncoder { } = self; // Drain the codec into the track before ending it, so the last frames land // in it rather than being dropped with the encoder. - let drained = block_on(encoder.finish()).and_then(|encoded| producer.publish(&encoded)); + let drained = match encoder { + Some(encoder) => block_on(encoder.finish()).and_then(|encoded| producer.publish(&encoded)), + None => Ok(()), + }; finalize(producer, drained, &finished)?; // A follower panic only stops rate adaptation, so the track still ends // cleanly before the panic is reported. @@ -478,6 +565,10 @@ impl Video { /// The encoder and the rendition it will emit are both resolved by the caller, /// and before this, so a config this machine can't encode fails without leaving /// a track advertised that will never carry frames. + /// + /// A `lower` encoder makes this one a simulcast rung above it: it takes the track + /// priority one below, and disables when the grant [`follow`](Self::follow) reads + /// can no longer fund it. pub fn publish( &mut self, target: crate::publish::Target<'_>, @@ -485,17 +576,51 @@ impl Video { config: &moq_video::encode::Config, rendition: hang::catalog::VideoConfig, encoder: moq_video::encode::Sink, + lower: Option, ) -> Result { let reserved = target .catalog .reserve() .with_timeline_hint(config.gop.duration(config.framerate)); let ceiling = video_ceiling(config, &rendition); - let producer = moq_video::encode::Producer::new(target.producer.clone(), reserved, rendition)?; + let (lower, priority) = match lower { + Some(lower) => { + let lower = self.producer(lower)?; + let guard = lower.lock(); + let below = guard.as_ref().ok_or(Error::MediaNotFound)?; + if below.ceiling >= ceiling { + return Err(Error::InvalidConfig(format!( + "the lower rendition's {} bps is not below this one's {} bps", + below.ceiling.as_bps(), + ceiling.as_bps() + ))); + } + let priority = below.priority.saturating_sub(1); + drop(guard); + (Some(lower.downgrade()), priority) + } + None => (None, hang::catalog::PRIORITY.video), + }; + // The suffix `moq_video::encode::Producer::new` would derive, built here so the + // track can take this rendition's priority. + let suffix = match &rendition.codec { + hang::catalog::VideoCodec::H264(_) => ".avc3", + hang::catalog::VideoCodec::H265(_) => ".hev1", + other => { + return Err(Error::InvalidConfig(format!( + "{other} is not a codec this producer can publish" + ))); + } + }; + let track = target.producer.unique_track(suffix, reserved.track_info(priority))?; + let producer = moq_video::encode::Producer::with_track(track, reserved, rendition)?; let finished = target.finished.clone(); self.producers.insert(Shared::new(VideoEncoder { - encoder, + encoder: Some(encoder), producer, + config: config.clone(), + priority, + lower, format, size: config.size(), reservation: None, @@ -509,12 +634,12 @@ impl Video { /// Reserve the encoder's ceiling against `allocator` and follow the grant. pub(crate) fn follow(&self, id: Id, allocator: &moq_net::bandwidth::Allocator) -> Result<(), Error> { let shared = self.producer(id)?; - let (reservation, ceiling) = { + let (reservation, ceiling, lower) = { let mut guard = shared.lock(); let encoder = guard.as_mut().ok_or(Error::MediaNotFound)?; let reservation = Arc::new(allocator.reserve(&encoder.producer.demand(), encoder.ceiling)); encoder.reservation = Some(reservation.clone()); - (reservation, encoder.ceiling) + (reservation, encoder.ceiling, encoder.lower.clone()) }; let (close, closed) = oneshot::channel(); shared.lock().as_mut().ok_or(Error::MediaNotFound)?.follow = Some(close); @@ -523,7 +648,7 @@ impl Video { tokio::select! { biased; _ = closed => {} - _ = follow_reservation(follower, reservation.consumer(), ceiling) => {} + _ = follow_reservation(follower, lower, reservation.consumer(), ceiling) => {} } Ok(()) }); @@ -725,12 +850,20 @@ unsafe fn encoder_kind(output: &moq_video_encoder_output) -> Resultkind` is `MOQ_VIDEO_ENCODER_KIND_NAMED`. /// - `bandwidth` is a handle from [`crate::moq_session_bandwidth`], or 0 to hold the /// configured bitrate regardless of congestion. +/// - `lower` is the handle of the next lower rendition of the same source from this +/// function, or 0. A non-zero `lower` makes this encoder a simulcast rung: when the +/// bandwidth grant can no longer fund it, it stops encoding (frames published to it +/// are dropped) and advertises itself disabled, so the lower renditions keep their +/// share, and it resumes on a keyframe once the grant recovers. It takes the track +/// priority one below `lower`'s, so the allocator fills that one first. It needs a +/// `bandwidth`, and a bitrate above `lower`'s. #[unsafe(no_mangle)] pub unsafe extern "C" fn moq_encode_video( broadcast: u32, input: *const moq_video_encoder_input, output: *const moq_video_encoder_output, bandwidth: u32, + lower: u32, ) -> i32 { ffi::enter(move || { let broadcast = ffi::parse_id(broadcast)?; @@ -760,10 +893,23 @@ pub unsafe extern "C" fn moq_encode_video( let encoder = block_on(moq_video::encode::Sink::open(&config))?; let bandwidth = ffi::parse_id_optional(bandwidth)?; + let lower = ffi::parse_id_optional(lower)?; + if lower.is_some() && bandwidth.is_none() { + return Err(Error::InvalidConfig( + "a lower rendition needs a bandwidth allocator to follow".into(), + )); + } let mut state = State::lock(); let allocator = bandwidth.map(|id| state.bandwidth.allocator(id)).transpose()?; let State { publish, video, .. } = &mut *state; - let id = video.publish(publish.target(broadcast)?, format, &config, rendition.clone(), encoder)?; + let id = video.publish( + publish.target(broadcast)?, + format, + &config, + rendition.clone(), + encoder, + lower, + )?; if let Some(allocator) = allocator.as_ref() { video.follow(id, allocator)?; } @@ -817,7 +963,9 @@ pub unsafe extern "C" fn moq_encode_video_demand( /// picture in the pixel format and at the resolution declared by /// [`moq_video_encoder_input`]. /// A backend that pipelines publishes an earlier frame's output here, so a call -/// that emits nothing is normal rather than an error. +/// that emits nothing is normal rather than an error. While a rung published with a +/// `lower` rendition is disabled for lack of bandwidth, the frame is dropped +/// without encoding. /// /// # Safety /// - `frame` must point to a valid [`moq_video_encoder_frame`]. @@ -1093,8 +1241,11 @@ mod tests { let followed = crate::ffi::Background::new(async { panic!("injected rate follower panic") }); assert!(matches!(followed.finished().await, Err(Error::Panic))); let video = VideoEncoder { - encoder, + encoder: Some(encoder), producer, + config: config.clone(), + priority: hang::catalog::PRIORITY.video, + lower: None, format: moq_video_pixel_format::MOQ_VIDEO_PIXEL_FORMAT_I420, size: config.size(), reservation: None, @@ -1116,15 +1267,36 @@ mod tests { mod follow_tests { use std::time::Duration; - use moq_mux::rate::{Follower, Policy}; + use moq_mux::rate::{Follower, Policy, Target}; use moq_net::bandwidth::Rate; use super::*; + /// Retune through `set`, which only ever sees an enabled target: these policies set no + /// lower rendition, so nothing disables. + fn rate( + follower: &mut Follower, + ceiling: Rate, + target: Target, + set: impl FnOnce(Rate) -> Result<(), moq_video::Error>, + ) -> Result, &'static str> { + retune(follower, Policy::new(ceiling), target, |target| match target { + Target::Enabled(bitrate) => set(bitrate), + Target::Disabled => unreachable!("no lower rendition"), + }) + } + + fn bitrate(target: Target) -> Rate { + let Target::Enabled(bitrate) = target else { + unreachable!("no lower rendition") + }; + bitrate + } + /// Retune at every target until the follower goes a minute without one. async fn quiet(follower: &mut Follower, ceiling: Rate, mut set: impl FnMut(Rate) -> Result<(), moq_video::Error>) { while let Ok(bitrate) = moq_clock::timeout(Duration::from_secs(60), follower.next()).await { - retune(follower, ceiling, bitrate.unwrap(), &mut set).unwrap(); + rate(follower, ceiling, bitrate.unwrap(), &mut set).unwrap(); } } @@ -1147,7 +1319,7 @@ mod follow_tests { grant.set(Some(Rate::from_mbps(1))).unwrap(); let bitrate = follower.next().await.unwrap(); - retune(&mut follower, ceiling, bitrate, &mut set).unwrap(); + rate(&mut follower, ceiling, bitrate, &mut set).unwrap(); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(ceiling)).unwrap(); quiet(&mut follower, ceiling, &mut set).await; @@ -1172,24 +1344,24 @@ mod follow_tests { }; grant.set(Some(Rate::from_mbps(1))).unwrap(); - assert_eq!(follower.next().await.unwrap(), Rate::from_mbps(1)); + assert_eq!(follower.next().await.unwrap(), Target::Enabled(Rate::from_mbps(1))); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(old)).unwrap(); let first = follower.next().await.unwrap(); - retune(&mut follower, old, first, set).unwrap(); + rate(&mut follower, old, first, set).unwrap(); // `moq_encode_video_bitrate` already retuned the encoder to the new cap. let due = follower.next().await.unwrap(); - assert!(due > first, "a raise toward the old ceiling"); - assert_eq!(retune(&mut follower, lowered, due, set), Ok(None)); + assert!(bitrate(due) > bitrate(first), "a raise toward the old ceiling"); + assert_eq!(rate(&mut follower, lowered, due, set), Ok(None)); assert_eq!(follower.policy().max, lowered); quiet(&mut follower, lowered, set).await; - assert_eq!(*applied.borrow(), [first], "nothing retuned past the new cap"); + assert_eq!(*applied.borrow(), [bitrate(first)], "nothing retuned past the new cap"); // A later attack and recovery climbs back to the new cap, not the old one. grant.set(Some(Rate::from_mbps(1))).unwrap(); let bitrate = follower.next().await.unwrap(); - retune(&mut follower, lowered, bitrate, set).unwrap(); + rate(&mut follower, lowered, bitrate, set).unwrap(); tokio::time::advance(Duration::from_secs(1)).await; grant.set(Some(old)).unwrap(); quiet(&mut follower, lowered, set).await; From 3ba5e10d23c148b0ebdbf939bc4a6fd7a498c6d4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 03:15:34 -0700 Subject: [PATCH 05/15] feat(net)!: an idle JS reservation reads the share it would get Mirrors the Rust allocator: an unwatched track claims nothing from its siblings but its own reservation reads the slice it would get back. Co-Authored-By: Claude Opus 5.5 --- doc/lib/js/net.md | 2 +- js/net/src/bandwidth.test.ts | 16 ++++++++--- js/net/src/bandwidth.ts | 37 ++++++++++++++++++------- js/net/src/connection/bandwidth.test.ts | 3 +- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/doc/lib/js/net.md b/doc/lib/js/net.md index c6050cf7a59..20687451efd 100644 --- a/doc/lib/js/net.md +++ b/doc/lib/js/net.md @@ -41,7 +41,7 @@ broadcast.announce(); - **Discovery follows what you watch.** A `consume` origin asks the relay for announcements only under the prefixes its readers watch with `announced(scope)` or `request(path, { announced: true })`, stops asking about a prefix 2s after nothing at or beneath it is watched, and asks again on reconnect. A plain `request(path)` asks nothing and subscribes blind, but once it resolves through a discovered route it watches its path until closed, so it outlives the listing that found it. A relay that refuses an announce request turns that session's announced requests blind too. - **Connections share by URL and versions.** `new Connection({ url })` pools and reconnects with backoff, which the elements use. An ordered `versions` list, spelled like `moq-lite-06` as in Rust, applies to both WebTransport and WebSocket; equivalent lists share, while a different order or an omitted list gets its own pool entry. Supplying your own transport options or origin selects a private loop. Receive estimates are on demand: observe `connection.probe` reactively or subscribe while you need them; `peek()` alone opens no PROBE stream. - **GOAWAY moves the session.** The connection dials the replacement at once while the old session keeps serving its groups, up to `goaway.handover` (default 10s, or the relay's deadline when that is sooner). New requests keep opening on the old session until the replacement's route outranks it, which is a restart without an epoch: an unpinned announced request follows the replacement, while a blind or epoch-pinned request stays on its accepted instance. With an epoch, which travels only when `moq-lite-07-wip` is offered, the replacement serves the same broadcast, so open subscriptions move across without missing a group. An empty GOAWAY redials the same URL. A redirect stays on the same host unless `goaway.redirect` is `"follow"`, which also presents the tokens on the connection's `auth` handle to the new host. -- **One send estimate per connection.** `Bandwidth.Allocator` divides it by track priority, max-min fair within a tier. An idle track claims nothing. Publishers reserve against it so their targets sum to the estimate. +- **One send estimate per connection.** `Bandwidth.Allocator` divides it by track priority, max-min fair within a tier. An idle track claims nothing, though its reservation still reads the share it would get. Publishers reserve against it so their targets sum to the estimate. - **List a broadcast's tracks.** `for await (const event of broadcast.tracks())` reports each track the broadcast lists (`added`, with its `Track.Info`), one `synced`, then `added` and `removed` as tracks come and go, until the broadcast ends; leaving the loop stops listing. Over a session it needs `moq-lite-07-wip`, syncs at once with what this side holds, and the peer's tracks follow as `added`. A track served only on request is never listed. See [discovery](/concept/moq-lite#discovery). - **One track per name.** Concurrent subscriptions share one request and producer. `createTrack` on a name with a request still queued answers it. The application owns sequence numbers: the broadcast keeps nothing of an ended track, so a track that replaces it, created or requested, starts at 0. A name always means the same content, so the replacement continues past its predecessor's groups by writing its first group with an explicit sequence. - **Timing is per track, with no default.** A track that declares a `timescale` carries a `timestamp` on every frame; one without is [untimed](/concept/moq-lite#subscriptions) and its frames carry none. Omitting it means untimed, as in Rust. Nothing stamps a frame for you: the publisher passes the timestamp, and the `Timed` type (`{ value, at }`) carries one into the `@moq/json` and `@moq/flate` producers. diff --git a/js/net/src/bandwidth.test.ts b/js/net/src/bandwidth.test.ts index 0b36595661b..a4923a6ad2d 100644 --- a/js/net/src/bandwidth.test.ts +++ b/js/net/src/bandwidth.test.ts @@ -87,7 +87,7 @@ test("concurrent tracks split the estimate", () => { allocator.close(); }); -test("an idle track claims nothing", () => { +test("an idle track claims nothing", async () => { const estimate = new Signal(2_000_000); const allocator = new Allocator(estimate); @@ -99,10 +99,18 @@ test("an idle track claims nothing", () => { const idle = allocator.reserve(idleTrack.demand(), 4_000_000); expect(watched.peek()).toBe(2_000_000); - // Not zero: an idle share reports "no opinion" so a sender that is mid-shutdown - // holds its rate instead of retuning to the floor on the way out. - expect(idle.peek()).toBeUndefined(); + // What it would get if a viewer joined, so a rendition stopped for lack of + // bandwidth can resume after its viewers left. + expect(idle.peek()).toBe(1_000_000); + expect(idle.grant.peek()).toBe(1_000_000); + + // The idle share keeps following the estimate. + const next = idle.grant.changed(); + estimate.set(4_000_000); + expect(await next).toBe(2_000_000); + expect(watched.peek()).toBe(4_000_000); + estimate.set(2_000_000); idleTrack.subscribe(); expect(watched.peek()).toBe(1_000_000); expect(idle.peek()).toBe(1_000_000); diff --git a/js/net/src/bandwidth.ts b/js/net/src/bandwidth.ts index 4285b0a1217..2d54ef7b9d1 100644 --- a/js/net/src/bandwidth.ts +++ b/js/net/src/bandwidth.ts @@ -37,8 +37,8 @@ export interface Want { * A reservation is what a sender can use, so handing it more is not a reason to * send more than it was configured for. * - * `undefined` when `id` isn't among the wants, which is how an idle or closed - * track reports "hold your rate" instead of a grant of zero. + * `undefined` when `id` isn't among the wants, which is how a released or closed + * reservation reports "hold your rate" instead of a grant of zero. * * Rates are bits per second. */ @@ -100,6 +100,21 @@ function wantsOf(entries: readonly Entry[]): Want[] { return wants; } +/** + * `entry`'s slice of `estimate` among the demanded `wants`. + * + * An idle entry claims nothing from the others but still reads the slice it would + * get back: a rendition stopped for lack of bandwidth, whose viewers then left, + * needs that to know when to resume. + */ +function shareOf(estimate: number, wants: readonly Want[], entry: Entry): number | undefined { + if (entry.demand.closed.peek() !== undefined) return undefined; + if (entry.demand.wanted.peek() === undefined) { + wants = [...wants, { id: entry.id, priority: entry.priority, max: entry.max }]; + } + return allocate(estimate, wants, entry.id); +} + function ceiling(max: number): number { if (!Number.isFinite(max) || max < 0) { throw new Error(`reservation ceiling must be a finite non-negative number, got ${max}`); @@ -199,8 +214,9 @@ export class Allocator implements Handle { * {@link Reservation.update}, and close it to hand the room back. * * {@link Reservation.peek} / {@link Reservation.grant} report `undefined` - * while nothing is subscribed to the track or the connection has no estimate. - * That tells a sender to hold its current rate rather than encode at zero. + * while the connection has no estimate, telling a sender to hold its current + * rate rather than encode at zero. While nothing is subscribed to the track they + * report the slice the track would get back, without claiming it from the others. */ reserve(track: Demand, max: number): Reservation { max = ceiling(max); @@ -249,7 +265,7 @@ export class Allocator implements Handle { const entries = this.#entries.peek(); const wants = wantsOf(entries); for (const entry of entries) { - entry.grant.set(estimate == null ? undefined : allocate(estimate, wants, entry.id)); + entry.grant.set(estimate == null ? undefined : shareOf(estimate, wants, entry)); } } @@ -258,8 +274,9 @@ export class Allocator implements Handle { const estimate = this.#estimate?.peek(); if (estimate == null) return undefined; const entries = this.#entries.peek(); - if (!entries.some((entry) => entry.id === id)) return undefined; - return allocate(estimate, wantsOf(entries), id); + const entry = entries.find((candidate) => candidate.id === id); + if (!entry) return undefined; + return shareOf(estimate, wantsOf(entries), entry); } #update(id: number, max: number): void { @@ -323,9 +340,9 @@ export class Reservation implements Disposable { /** * This reservation's current slice of the estimate. * - * `undefined` while nothing is subscribed, the connection has no estimate, or - * this reservation has been closed: hold the current rate rather than encode - * at zero. + * `undefined` while the connection has no estimate or this reservation has + * been closed: hold the current rate rather than encode at zero. While nothing + * is subscribed it is the slice the track would get back. */ get grant(): Getter { return this.#grant; diff --git a/js/net/src/connection/bandwidth.test.ts b/js/net/src/connection/bandwidth.test.ts index 665f35744af..a0498fdd08c 100644 --- a/js/net/src/connection/bandwidth.test.ts +++ b/js/net/src/connection/bandwidth.test.ts @@ -108,7 +108,8 @@ test("an idle track on a live connection claims nothing", async () => { const idleShare = allocator.reserve(idle.demand(), 4_000_000); await waitUntil(() => watchedShare.peek() === 2_000_000); - expect(idleShare.peek()).toBeUndefined(); + // Claims nothing from the watched track, but reads what it would get back. + expect(idleShare.peek()).toBe(1_000_000); } finally { handle.close(); } From 506d63499f60da89b89ae5b5050c91772c6ae8ca Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 03:15:34 -0700 Subject: [PATCH 06/15] feat(bindings): pass a lower video rendition through every wrapper Python, Swift, and Go take a lower producer; Kotlin and Dart alias the regenerated options. Docs and MIGRATE notes cover the rung and the idle grant, and grant tests wait for their subscription to land. Co-Authored-By: Claude Opus 5.5 --- dart/moq_ffi/lib/src/moq.dart | 67 +++++++++++++++++++++++++++-- doc/lib/c/index.md | 2 +- doc/lib/cpp/index.md | 2 +- doc/lib/dart/index.md | 2 +- doc/lib/go/index.md | 2 +- doc/lib/kt/index.md | 2 +- doc/lib/py/index.md | 2 +- doc/lib/swift/index.md | 2 +- go/wrapper/session.go | 4 +- go/wrapper/video/video.go | 10 ++++- py/moq-rs/moq/session.py | 5 ++- py/moq-rs/moq/video.py | 9 +++- rs/moq-c/MIGRATE.md | 5 +++ rs/moq-c/src/bandwidth.rs | 5 ++- rs/moq-c/src/test.rs | 23 +++++----- rs/moq-c/src/video.rs | 3 +- rs/moq-ffi/MIGRATE.md | 5 +++ rs/moq-ffi/src/bandwidth.rs | 10 ----- rs/moq-ffi/src/test.rs | 31 ++++++++----- rs/moq-ffi/src/video.rs | 11 ++++- rs/moq-mux/MIGRATE.md | 6 +++ rs/moq-mux/src/codec/h264/import.rs | 9 +++- rs/moq-mux/src/rate.rs | 16 +++++-- rs/moq-net/MIGRATE.md | 6 +++ rs/moq-video/src/encode/producer.rs | 6 +-- swift/Sources/Moq/Bandwidth.swift | 5 ++- swift/Sources/Moq/Video.swift | 10 ++++- 27 files changed, 195 insertions(+), 65 deletions(-) diff --git a/dart/moq_ffi/lib/src/moq.dart b/dart/moq_ffi/lib/src/moq.dart index 1ccba994936..e0998e4265f 100644 --- a/dart/moq_ffi/lib/src/moq.dart +++ b/dart/moq_ffi/lib/src/moq.dart @@ -4676,11 +4676,13 @@ class MoqVideoProducerOptions { final MoqVideoEncoderOutput output; final String? track; final MoqBandwidth? bandwidth; + final MoqVideoProducer? lower; MoqVideoProducerOptions({ required this.input, required this.output, this.track = null, this.bandwidth = null, + this.lower = null, }); } @@ -4711,12 +4713,18 @@ class FfiConverterMoqVideoProducerOptions { ); final bandwidth = bandwidth_lifted.value; new_offset += bandwidth_lifted.bytesRead; + final lower_lifted = FfiConverterOptionalMoqVideoProducer.read( + Uint8List.view(buf.buffer, new_offset), + ); + final lower = lower_lifted.value; + new_offset += lower_lifted.bytesRead; return LiftRetVal( MoqVideoProducerOptions( input: input, output: output, track: track, bandwidth: bandwidth, + lower: lower, ), new_offset - buf.offsetInBytes, ); @@ -4728,6 +4736,7 @@ class FfiConverterMoqVideoProducerOptions { FfiConverterMoqVideoEncoderOutput.allocationSize(value.output) + FfiConverterOptionalString.allocationSize(value.track) + FfiConverterOptionalMoqBandwidth.allocationSize(value.bandwidth) + + FfiConverterOptionalMoqVideoProducer.allocationSize(value.lower) + 0; final buf = Uint8List(total_length); write(value, buf); @@ -4752,6 +4761,10 @@ class FfiConverterMoqVideoProducerOptions { value.bandwidth, Uint8List.view(buf.buffer, new_offset), ); + new_offset += FfiConverterOptionalMoqVideoProducer.write( + value.lower, + Uint8List.view(buf.buffer, new_offset), + ); return new_offset - buf.offsetInBytes; } @@ -4760,6 +4773,7 @@ class FfiConverterMoqVideoProducerOptions { FfiConverterMoqVideoEncoderOutput.allocationSize(value.output) + FfiConverterOptionalString.allocationSize(value.track) + FfiConverterOptionalMoqBandwidth.allocationSize(value.bandwidth) + + FfiConverterOptionalMoqVideoProducer.allocationSize(value.lower) + 0; } } @@ -15645,6 +15659,53 @@ class FfiConverterOptionalMoqVideoHint { } } +class FfiConverterOptionalMoqVideoProducer { + static MoqVideoProducer? lift(RustBuffer buf) { + return FfiConverterOptionalMoqVideoProducer.read(buf.asUint8List()).value; + } + + static LiftRetVal read(Uint8List buf) { + if (ByteData.view(buf.buffer, buf.offsetInBytes).getInt8(0) == 0) { + return LiftRetVal(null, 1); + } + final result = FfiConverterMoqVideoProducer.read( + Uint8List.view(buf.buffer, buf.offsetInBytes + 1), + ); + return LiftRetVal(result.value, result.bytesRead + 1); + } + + static int allocationSize([MoqVideoProducer? value]) { + if (value == null) { + return 1; + } + return FfiConverterMoqVideoProducer.allocationSize(value) + 1; + } + + static RustBuffer lower(MoqVideoProducer? value) { + if (value == null) { + return toRustBuffer(Uint8List.fromList([0])); + } + final buf = Uint8List( + FfiConverterOptionalMoqVideoProducer.allocationSize(value), + ); + FfiConverterOptionalMoqVideoProducer.write(value, buf); + return toRustBuffer(buf); + } + + static int write(MoqVideoProducer? value, Uint8List buf) { + if (value == null) { + buf[0] = 0; + return 1; + } + buf[0] = 1; + return FfiConverterMoqVideoProducer.write( + value, + Uint8List.view(buf.buffer, buf.offsetInBytes + 1), + ) + + 1; + } +} + class FfiConverterOptionalMoqVideoSurface { static MoqVideoSurface? lift(RustBuffer buf) { return FfiConverterOptionalMoqVideoSurface.read(buf.asUint8List()).value; @@ -20181,7 +20242,7 @@ void _checkApiChecksums() { 38021) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqreservation_grant() != 59401) { + if (uniffi_moq_ffi_checksum_method_moqreservation_grant() != 10773) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } if (uniffi_moq_ffi_checksum_method_moqreservation_update() != 9626) { @@ -20714,7 +20775,7 @@ void _checkApiChecksums() { if (uniffi_moq_ffi_checksum_method_moqvideoproducer_set_bitrate() != 28203) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqvideoproducer_write() != 6141) { + if (uniffi_moq_ffi_checksum_method_moqvideoproducer_write() != 17427) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } if (uniffi_moq_ffi_checksum_constructor_moqaudiocodec_aac() != 18170) { @@ -20822,7 +20883,7 @@ void _checkApiChecksums() { if (uniffi_moq_ffi_checksum_constructor_moqvideoencoder_new() != 22531) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_constructor_moqvideoproducer_new() != 34351) { + if (uniffi_moq_ffi_checksum_constructor_moqvideoproducer_new() != 51291) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } } diff --git a/doc/lib/c/index.md b/doc/lib/c/index.md index 4174047f9c0..7e96a1382eb 100644 --- a/doc/lib/c/index.md +++ b/doc/lib/c/index.md @@ -51,7 +51,7 @@ if (session < 0) return fail(moq_error()); ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. For simulcast, give each rendition the next one down as `moq_encode_video`'s `lower` handle: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/doc/lib/cpp/index.md b/doc/lib/cpp/index.md index d26680b96e6..6b5ce145e5b 100644 --- a/doc/lib/cpp/index.md +++ b/doc/lib/cpp/index.md @@ -129,7 +129,7 @@ ok(broadcast->announce(route)); ok(broadcast->close()); // keep the producer alive while publishing, then close explicitly ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Futures diff --git a/doc/lib/dart/index.md b/doc/lib/dart/index.md index f9aafe9df3b..157332279a8 100644 --- a/doc/lib/dart/index.md +++ b/doc/lib/dart/index.md @@ -129,7 +129,7 @@ mine.close(); await moq.close(); ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/doc/lib/go/index.md b/doc/lib/go/index.md index 703a20b27c7..c252cac0ca2 100644 --- a/doc/lib/go/index.md +++ b/doc/lib/go/index.md @@ -117,7 +117,7 @@ _ = video.Finish() broadcast.Close() // keep the producer reachable while publishing, then close explicitly ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `Lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/doc/lib/kt/index.md b/doc/lib/kt/index.md index 781f0fb57f7..2b8d99d6ea6 100644 --- a/doc/lib/kt/index.md +++ b/doc/lib/kt/index.md @@ -89,7 +89,7 @@ Moq.connect("https://relay.example.com").use { moq -> } ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/doc/lib/py/index.md b/doc/lib/py/index.md index 1799e4881d2..95c27d81faa 100644 --- a/doc/lib/py/index.md +++ b/doc/lib/py/index.md @@ -103,7 +103,7 @@ async def main(): asyncio.run(main()) ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/doc/lib/swift/index.md b/doc/lib/swift/index.md index b237f59c900..14a19b87d26 100644 --- a/doc/lib/swift/index.md +++ b/doc/lib/swift/index.md @@ -90,7 +90,7 @@ session.shutdown() try await session.closed() ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/go/wrapper/session.go b/go/wrapper/session.go index b631459a981..2f455a5273a 100644 --- a/go/wrapper/session.go +++ b/go/wrapper/session.go @@ -151,8 +151,8 @@ func (b *Bandwidth) Reserve(demand *TrackDemand, maxBps uint64) *Reservation { // Reservation is one track's standing claim on a Bandwidth. // -// Grant is a snapshot: nil means no estimate or no demand, so hold the current -// rate, and 0 is a real zero grant. +// Grant is a snapshot: nil means no estimate, so hold the current rate, and 0 is +// a real zero grant. While nothing subscribes it is the share the track would get. // Update(0) relinquishes the allocation while the track stays subscribed; the // handle remains usable and a later Update can claim bandwidth again. type Reservation struct { diff --git a/go/wrapper/video/video.go b/go/wrapper/video/video.go index e368d4be04b..23eb51b27d8 100644 --- a/go/wrapper/video/video.go +++ b/go/wrapper/video/video.go @@ -83,6 +83,10 @@ type ProducerOptions struct { Track *string // Bandwidth reserves the track against a session allocator when non-nil. Bandwidth *moq.Bandwidth + // Lower is the next rendition down a simulcast ladder: when non-nil, this one + // disables while the grant cannot fund it and resumes once it recovers. Needs + // Bandwidth and a bitrate above the lower rendition's. + Lower *Producer } // NewProducer opens a video track on the catalog's broadcast and registers its rendition. @@ -92,7 +96,11 @@ func NewProducer(catalog *media.CatalogProducer, options ProducerOptions) (*Prod raw := bridge.Bandwidth(options.Bandwidth) bw = &raw } - inner, err := ffi.NewMoqVideoProducer(bridge.CatalogProducer(catalog), ffi.MoqVideoProducerOptions{Input: options.Input, Output: options.Output, Track: options.Track, Bandwidth: bw}) + var lower **ffi.MoqVideoProducer + if options.Lower != nil { + lower = &options.Lower.inner + } + inner, err := ffi.NewMoqVideoProducer(bridge.CatalogProducer(catalog), ffi.MoqVideoProducerOptions{Input: options.Input, Output: options.Output, Track: options.Track, Bandwidth: bw, Lower: lower}) if err != nil { return nil, err } diff --git a/py/moq-rs/moq/session.py b/py/moq-rs/moq/session.py index a0fe61bfd61..566ba41ebfc 100644 --- a/py/moq-rs/moq/session.py +++ b/py/moq-rs/moq/session.py @@ -212,8 +212,9 @@ def reserve(self, demand: TrackDemand, max_bps: int) -> Reservation: class Reservation: """One track's standing claim on a :class:`Bandwidth`. - :meth:`grant` is a snapshot: ``None`` means no estimate or no demand, so - hold the current rate, and ``0`` is a real zero grant. + :meth:`grant` is a snapshot: ``None`` means no estimate, so hold the + current rate, and ``0`` is a real zero grant. While nothing subscribes it + is the share the track would get. """ def __init__(self, inner: MoqReservation) -> None: diff --git a/py/moq-rs/moq/video.py b/py/moq-rs/moq/video.py index e3fdb63b800..59da116cd65 100644 --- a/py/moq-rs/moq/video.py +++ b/py/moq-rs/moq/video.py @@ -86,8 +86,14 @@ def __init__( *, track: str | None = None, bandwidth: Bandwidth | None = None, + lower: Producer | None = None, ) -> None: - """Open a video track on the catalog's broadcast and register its rendition.""" + """Open a video track on the catalog's broadcast and register its rendition. + + Pass ``lower``, the next rendition down a simulcast ladder, to disable this + one while the bandwidth grant cannot fund it; it resumes once the grant + recovers. Needs ``bandwidth`` and a bitrate above the lower rendition's. + """ self._inner = MoqVideoProducer( catalog._inner, MoqVideoProducerOptions( @@ -95,6 +101,7 @@ def __init__( output=output, track=track, bandwidth=None if bandwidth is None else bandwidth._inner, + lower=None if lower is None else lower._inner, ), ) diff --git a/rs/moq-c/MIGRATE.md b/rs/moq-c/MIGRATE.md index 27fed02ed2a..a60f4c5329f 100644 --- a/rs/moq-c/MIGRATE.md +++ b/rs/moq-c/MIGRATE.md @@ -4,6 +4,11 @@ How to update your code for each breaking change, newest first. ## Unreleased +### `moq_encode_video` takes a lower rendition + +`moq_encode_video` takes a fifth `uint32_t lower` argument. Pass `0` to keep the old behavior, or the handle of the next lower rendition's encoder to disable this one while its bandwidth grant cannot fund it; that needs a `bandwidth` handle and a bitrate above the lower encoder's. +`moq_reservation_grant` reports the slice the track would get back while nothing subscribes to it, instead of no grant. + ### Sessions report a lifecycle snapshot `moq_session_connect` and `moq_session_request_accept` take a `moq_state_callback` instead of a `moq_status_callback`: `void on_state(void *user_data, const moq_connection_state *state)`. diff --git a/rs/moq-c/src/bandwidth.rs b/rs/moq-c/src/bandwidth.rs index 2ee23a081a0..7d120605ab9 100644 --- a/rs/moq-c/src/bandwidth.rs +++ b/rs/moq-c/src/bandwidth.rs @@ -88,8 +88,9 @@ pub extern "C" fn moq_bandwidth_reserve(bandwidth: u32, track: u32, max_bps: u64 /// This reservation's slice right now, in bits per second. /// -/// `present` is false when there is no estimate or no demand: hold the current -/// rate. `present` true and `bps` 0 is a real zero grant. +/// `present` is false when there is no estimate: hold the current rate. `present` +/// true and `bps` 0 is a real zero grant. While nothing subscribes to the track, +/// `bps` is the slice it would get back, without claiming it from the others. /// /// # Safety /// - `bps` and `present` must be valid pointers. diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 346632aaa08..2667ba2d893 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -5493,15 +5493,18 @@ fn grant(reservation: u32) -> Option { present.then_some(bps) } -fn wait_grant(reservation: u32) -> u64 { +/// Wait for `reservation`'s grant to reach `bps`, since a subscription reaches the +/// allocator asynchronously and an unwatched track already reads a would-be share. +fn wait_grant(reservation: u32, bps: u64) { let deadline = std::time::Instant::now() + TIMEOUT; loop { - if let Some(bps) = grant(reservation) { - return bps; + let last = grant(reservation); + if last == Some(bps) { + return; } assert!( std::time::Instant::now() < deadline, - "timed out waiting for a reservation grant" + "timed out waiting for a {bps} bps grant, last {last:?}" ); std::thread::sleep(Duration::from_millis(10)); } @@ -5546,8 +5549,8 @@ fn bandwidth_reservations_split_the_estimate() { .set(Some(moq_net::bandwidth::Rate::from_bps(3_000_000))) .unwrap(); - wait_grant(first); - wait_grant(second); + wait_grant(first, 1_500_000); + wait_grant(second, 1_500_000); let a = grant(first).expect("first grant"); let b = grant(second).expect("second grant"); assert!(a + b <= 3_000_000, "{a} + {b} oversubscribed"); @@ -5593,10 +5596,10 @@ fn bandwidth_handles_share_the_registry() { let (second_sub, second_cb) = subscribe_named_track(consume, b"b"); let reserved = id(moq_bandwidth_reserve(first, first_track, 4_000_000)); - assert_eq!(wait_grant(reserved), 3_000_000); + wait_grant(reserved, 3_000_000); let other = id(moq_bandwidth_reserve(second, second_track, 4_000_000)); - assert_eq!(wait_grant(other), 1_500_000); - assert_eq!(grant(reserved), Some(1_500_000)); + wait_grant(other, 1_500_000); + wait_grant(reserved, 1_500_000); assert_eq!(moq_reservation_close(reserved), 0); assert_eq!(moq_reservation_close(other), 0); @@ -5644,7 +5647,7 @@ fn encode_video_bitrate_caps_the_reservation() { estimate .set(Some(moq_net::bandwidth::Rate::from_bps(4_000_000))) .unwrap(); - assert_eq!(wait_grant(reservation), 4_000_000); + wait_grant(reservation, 4_000_000); assert_eq!(moq_encode_video_bitrate(producer, 1_000_000), 0); assert_eq!(grant(reservation), Some(1_000_000)); diff --git a/rs/moq-c/src/video.rs b/rs/moq-c/src/video.rs index c374b35ca9c..aabe973d85b 100644 --- a/rs/moq-c/src/video.rs +++ b/rs/moq-c/src/video.rs @@ -567,8 +567,7 @@ impl Video { /// a track advertised that will never carry frames. /// /// A `lower` encoder makes this one a simulcast rung above it: it takes the track - /// priority one below, and disables when the grant [`follow`](Self::follow) reads - /// can no longer fund it. + /// priority one below, and disables when its bandwidth grant can no longer fund it. pub fn publish( &mut self, target: crate::publish::Target<'_>, diff --git a/rs/moq-ffi/MIGRATE.md b/rs/moq-ffi/MIGRATE.md index 056aa8a1fe4..eff532e0be9 100644 --- a/rs/moq-ffi/MIGRATE.md +++ b/rs/moq-ffi/MIGRATE.md @@ -5,6 +5,11 @@ These are the UniFFI names; each language wrapper has its own `MIGRATE.md` in it ## Unreleased +### An idle reservation reads the share it would get + +`MoqReservation::grant` reports the slice the track would get back while nothing subscribes to it, instead of `None`; it still claims nothing from the other tracks. +A sender that relied on `None` to hold its rate while unwatched should check the track's demand instead. + ### Connection state `MoqClient::connect` returns a connecting session without waiting for its first dial to succeed, natively and in the browser. Read `MoqSession::state()` and pass its `revision` to `state_changed(after)` to observe first-dial failures and reconnects. `MoqConnectionState` carries `phase`, `connects`, `failure`, and `version` together. These replace `status()`, `connects()`, and both `failure()` polls. Cancel the returned session to stop dialing; cancelling or releasing the client does not close it. diff --git a/rs/moq-ffi/src/bandwidth.rs b/rs/moq-ffi/src/bandwidth.rs index 0de285870d3..e8a762000e5 100644 --- a/rs/moq-ffi/src/bandwidth.rs +++ b/rs/moq-ffi/src/bandwidth.rs @@ -81,16 +81,6 @@ impl MoqReservation { self.inner.peek().map(moq_net::bandwidth::Rate::as_bps) } - /// Whether a session probes the connection for this reservation, so the estimate - /// can rise to fund it even while its sender is stopped. - /// - /// Stop a sender for lack of bandwidth only while this holds: without a probing - /// session the estimate stops rising, so it could never resume. - pub fn probed(&self) -> bool { - let _guard = crate::ffi::enter(); - self.inner.consumer().probed() - } - /// Change the ceiling, keeping the same claim. pub fn update(&self, max_bps: u64) { let _guard = crate::ffi::enter(); diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index b7121b47713..26219b6f260 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -501,19 +501,30 @@ async fn media_track_reserves_through_its_demand() { let bandwidth = crate::bandwidth::MoqBandwidth::new(moq_net::bandwidth::Allocator::new(estimate.consume())); let broadcast = MoqBroadcastProducer::new().unwrap(); - let media = MoqMediaTrackProducer::audio( - &broadcast, - MoqMediaTarget::Named { name: None }, - audio_init(MoqAudioFormat::Opus, opus_head()), - ) - .unwrap(); + let audio = |name: &str| { + MoqMediaTrackProducer::audio( + &broadcast, + MoqMediaTarget::Named { + name: Some(name.into()), + }, + audio_init(MoqAudioFormat::Opus, opus_head()), + ) + .unwrap() + }; + let media = audio("media"); + let sibling = audio("sibling"); let demand = media.demand().unwrap(); + let sibling_demand = sibling.demand().unwrap(); + let _sibling_sub = subscribe(&broadcast, &sibling_demand.name()).await; let reservation = bandwidth.reserve(&demand, 4_000_000); - // No subscriber yet: no demand, so hold the current rate. - assert_eq!(reservation.grant(), None); + let sibling_reservation = bandwidth.reserve(&sibling_demand, 4_000_000); + // No subscriber yet: it claims nothing from its sibling, and reads what it would get. + assert_eq!(sibling_reservation.grant(), Some(3_000_000)); + assert_eq!(reservation.grant(), Some(1_500_000)); let _sub = subscribe(&broadcast, &demand.name()).await; - assert_eq!(reservation.grant(), Some(3_000_000)); + assert_eq!(sibling_reservation.grant(), Some(1_500_000)); + assert_eq!(reservation.grant(), Some(1_500_000)); } #[tokio::test] @@ -6608,7 +6619,7 @@ async fn video_encoder_follows_a_shrinking_grant() { .unwrap(); let reservation = video.reservation().expect("published against an allocator"); - assert_eq!(reservation.grant(), None, "no demand yet"); + assert_eq!(reservation.grant(), None, "no estimate yet"); let consumer = broadcast.consume().unwrap(); let _sub = consumer.subscribe_track("camera".into(), None).await.unwrap(); diff --git a/rs/moq-ffi/src/video.rs b/rs/moq-ffi/src/video.rs index 7ecb4868ada..2488e90c34c 100644 --- a/rs/moq-ffi/src/video.rs +++ b/rs/moq-ffi/src/video.rs @@ -684,7 +684,11 @@ impl MoqVideoProducer { } let guard = lower.inner.lock().map_err(|_| panic_error())?; let below = guard.as_ref().ok_or(MoqError::Closed)?; - Some((Arc::downgrade(&lower.inner), below.ceiling, below.priority.saturating_sub(1))) + Some(( + Arc::downgrade(&lower.inner), + below.ceiling, + below.priority.saturating_sub(1), + )) } None => None, }; @@ -1573,7 +1577,10 @@ mod rung_tests { async fn a_rung_takes_the_priority_below_its_lower() { let ladder = ladder(); assert_eq!(priority(&ladder.broadcast, "low").await, hang::catalog::PRIORITY.video); - assert_eq!(priority(&ladder.broadcast, "high").await, hang::catalog::PRIORITY.video - 1); + assert_eq!( + priority(&ladder.broadcast, "high").await, + hang::catalog::PRIORITY.video - 1 + ); } /// A rung needs an allocator to follow and a ceiling above its lower rendition's. diff --git a/rs/moq-mux/MIGRATE.md b/rs/moq-mux/MIGRATE.md index 5840560002e..b48e9a8b6b8 100644 --- a/rs/moq-mux/MIGRATE.md +++ b/rs/moq-mux/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Rate following can disable a simulcast rung + +`rate::Follower::next` and `poll_next` return a `rate::Target`, and `rate::Control::update` returns `Option`: `Target::Enabled(rate)` is the bitrate to encode at, and `Target::Disabled` means stop encoding and advertise the rendition disabled. +`rate::Policy` gains `lower`, the next lower rendition's ceiling, which only a rendition encoded beside lower ones sets; add `lower: None` to a struct literal, or build it with `Policy::new`. +Match `Target::Enabled(rate)` where you took the rate before. A policy without `lower` never yields `Target::Disabled`. + ### `Programs::finish` returns the program broadcasts `Programs::finish` now returns each program's `broadcast::Producer` instead of `()`. diff --git a/rs/moq-mux/src/codec/h264/import.rs b/rs/moq-mux/src/codec/h264/import.rs index 40f9d22f884..842f26a252f 100644 --- a/rs/moq-mux/src/codec/h264/import.rs +++ b/rs/moq-mux/src/codec/h264/import.rs @@ -427,7 +427,14 @@ mod tests { import.set_enabled(false).unwrap(); assert!(!enabled()); keyframe(&mut import, 40, SPS_BT709); - assert_eq!(catalog.snapshot().video.renditions["video"].color.as_ref().unwrap().primaries, Some(1)); + assert_eq!( + catalog.snapshot().video.renditions["video"] + .color + .as_ref() + .unwrap() + .primaries, + Some(1) + ); assert!(!enabled(), "a reconfiguration keeps the rendition disabled"); import.set_enabled(true).unwrap(); assert!(enabled()); diff --git a/rs/moq-mux/src/rate.rs b/rs/moq-mux/src/rate.rs index bc2ad211cfd..65fb746f17f 100644 --- a/rs/moq-mux/src/rate.rs +++ b/rs/moq-mux/src/rate.rs @@ -716,7 +716,10 @@ mod tests { assert_eq!(control.deadline(), None, "a drop lands at once"); let recovered = start + Duration::from_secs(1); - assert_eq!(control.update(Some(bps(4_000_000)), recovered), Some(on(bps(1_250_000)))); + assert_eq!( + control.update(Some(bps(4_000_000)), recovered), + Some(on(bps(1_250_000))) + ); let (applied, last) = follow(&mut control, bps(4_000_000)); assert_eq!(applied.last(), Some(&bps(4_000_000))); @@ -1164,7 +1167,11 @@ mod tests { let allocator = bandwidth::Allocator::new(estimate.consume()); let broadcast = broadcast::Info::default().produce(); // Lowest first, each a step below the last in priority and a doubling in ceiling. - let ladder = [(1_000_000, None), (2_000_000, Some(1_000_000)), (4_000_000, Some(2_000_000))]; + let ladder = [ + (1_000_000, None), + (2_000_000, Some(1_000_000)), + (4_000_000, Some(2_000_000)), + ]; let mut tracks = Vec::new(); let mut viewers = Vec::new(); let mut reservations = Vec::new(); @@ -1198,7 +1205,10 @@ mod tests { // 4.5 Mbps funds the lower two in full and leaves 1.5 Mbps, under the top // rendition's 2.67 Mbps boundary. estimate.set(Some(bps(4_500_000))).unwrap(); - assert_eq!(update(start), [on(bps(1_000_000)), on(bps(2_000_000)), Target::Disabled]); + assert_eq!( + update(start), + [on(bps(1_000_000)), on(bps(2_000_000)), Target::Disabled] + ); // Shrinking further takes the middle one next, still keeping the lowest whole. estimate.set(Some(bps(2_000_000))).unwrap(); diff --git a/rs/moq-net/MIGRATE.md b/rs/moq-net/MIGRATE.md index 9dea3a51305..9a6121658cd 100644 --- a/rs/moq-net/MIGRATE.md +++ b/rs/moq-net/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### An idle reservation reads the share it would get + +`bandwidth::Reservation::peek` and its `consumer()` report the slice the track would get back while nothing subscribes to it, instead of `None`; it still claims nothing from the other tracks. +`None` now means only that the connection has no estimate or the reservation is gone. +A sender that relied on `None` to hold its rate while unwatched should check its track's demand instead. + ### `stats::Refusal` is `stats::RefusalKind`, and `time::now` is gone (#6510) Rename `moq_net::stats::Refusal` to `moq_net::stats::RefusalKind`, so it no longer shares a name with `moq_auth::Refusal`; its variants and `Refusals` are unchanged. diff --git a/rs/moq-video/src/encode/producer.rs b/rs/moq-video/src/encode/producer.rs index 2037424571a..21347998c3d 100644 --- a/rs/moq-video/src/encode/producer.rs +++ b/rs/moq-video/src/encode/producer.rs @@ -600,11 +600,7 @@ async fn next_target(rate: &mut RateControl) -> Result, -) { +async fn apply_target(encoder: &mut Sink, rate: &mut RateControl, target: Result) { let Some(follower) = rate.as_mut() else { return }; let Ok(target) = target else { diff --git a/swift/Sources/Moq/Bandwidth.swift b/swift/Sources/Moq/Bandwidth.swift index 1bb855b52c5..910dafeed91 100644 --- a/swift/Sources/Moq/Bandwidth.swift +++ b/swift/Sources/Moq/Bandwidth.swift @@ -38,8 +38,9 @@ public final class Bandwidth: Sendable { /// One track's standing claim on a `Bandwidth`. /// -/// `grant()` is a snapshot: `nil` means no estimate or no demand, so hold the -/// current rate, and `0` is a real zero grant. +/// `grant()` is a snapshot: `nil` means no estimate, so hold the current rate, +/// and `0` is a real zero grant. While nothing subscribes it is the share the +/// track would get. public final class Reservation: Sendable { let ffi: MoqReservation diff --git a/swift/Sources/Moq/Video.swift b/swift/Sources/Moq/Video.swift index 26f1df4c9e1..876bde14a50 100644 --- a/swift/Sources/Moq/Video.swift +++ b/swift/Sources/Moq/Video.swift @@ -31,17 +31,23 @@ public enum Video { let ffi: MoqVideoProducer /// Open a video track on the catalog's broadcast and register its rendition. + /// + /// Pass `lower`, the next rendition down a simulcast ladder, to disable this + /// one while the bandwidth grant cannot fund it; it resumes once the grant + /// recovers. Needs `bandwidth` and a bitrate above the lower rendition's. public init( catalog: Media.CatalogProducer, input: EncoderInput, output: EncoderOutput, track: String? = nil, - bandwidth: Bandwidth? = nil + bandwidth: Bandwidth? = nil, + lower: Producer? = nil ) throws { ffi = try MoqVideoProducer( catalog: catalog.ffi, options: MoqVideoProducerOptions( - input: input, output: output, track: track, bandwidth: bandwidth?.ffi)) + input: input, output: output, track: track, bandwidth: bandwidth?.ffi, + lower: lower?.ffi)) } /// A watch-only handle to whether the video track has subscribers. From 1c4d279563dd1ab5616d7f7d497050a4fef763b9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 03:16:34 -0700 Subject: [PATCH 07/15] docs(obs): a missing grant now means only no estimate Co-Authored-By: Claude Opus 5.5 --- cpp/obs/src/moq-rate.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/obs/src/moq-rate.h b/cpp/obs/src/moq-rate.h index 89aaa8addcc..4b30ed207fe 100644 --- a/cpp/obs/src/moq-rate.h +++ b/cpp/obs/src/moq-rate.h @@ -22,8 +22,8 @@ class MoQRate { uint64_t Max() const { return max; } // Feed a grant, returning the new target when it moved enough to apply. - // A missing grant (no estimate, or no subscriber) holds the target: losing - // the estimate is not evidence the uplink got better. + // A missing grant (no estimate) holds the target: losing the estimate is not + // evidence the uplink got better. std::optional Update(std::optional grant, Clock::time_point now) { if (!grant) From 99f4ef142b8f4e25e06f8bb4bd160369d8337de6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 03:17:53 -0700 Subject: [PATCH 08/15] chore(quest): narrow simulcast-rung-disable to OBS and the shaped proof Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 2 +- quest/m1/simulcast-rung-disable.md | 103 +++++++++-------------------- quest/m2/probe/README.md | 2 +- 3 files changed, 34 insertions(+), 73 deletions(-) diff --git a/quest/m1/README.md b/quest/m1/README.md index 9ad0bfc1971..2eda36a34c1 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -133,7 +133,7 @@ Catalog HLS field. - [IETF takeover](/quest/m1/ietf-takeover.md) - carry the shared takeover contract through the negotiated IETF epoch extension - [Identity and recovery guide](/quest/m1/identity-guide.md) - explain pinned catalogs, lazy jobs, takeover, restart and version compatibility - [Skip ProbeRTT after a near-floor RTT](/quest/m1/bbr-probe-rtt-skip.md) - BBR3 skips ProbeRTT when an app-limited flow already saw a near-floor RTT, so media stops stalling every 5 s -- [Native rung disable](/quest/m1/simulcast-rung-disable.md) - Rust/FFI and OBS publishers stop encoding the top renditions their grant cannot fund and resume them once probing raises the estimate +- [Native rung disable](/quest/m1/simulcast-rung-disable.md) - OBS stops sending the top renditions its grant cannot fund, as the Rust/FFI publishers do, and a shaped uplink through moq-ffi proves the resume - [Audio jitter target](/quest/m1/audio-jitter-target/README.md) - the playout target is default in both languages; a manual browser proof and a native trace replay remain - [A/V clock](/quest/m1/av-clock.md) - the audio playhead drives Sync.reference while audio plays, through per-track sync handles - [Container reads drop backfill](/quest/m1/read-backfill.md) - the JS container consumer drops groups below its read cursor like Rust diff --git a/quest/m1/simulcast-rung-disable.md b/quest/m1/simulcast-rung-disable.md index cdfc90d3112..6baafc39928 100644 --- a/quest/m1/simulcast-rung-disable.md +++ b/quest/m1/simulcast-rung-disable.md @@ -1,79 +1,41 @@ -# [L] Native publishers disable simulcast rungs the grant cannot fund +# [M] OBS disables simulcast rungs the grant cannot fund, proven on a shaped uplink ## Goal -A publisher encoding several video renditions of one source stops encoding -its top renditions when its bandwidth grant cannot fund them, advertising -each as `enabled: false` in the catalog, and re-enables them with hysteresis -once the grant recovers. The lower renditions keep their share instead of -every rung degrading together. Covers the Rust and FFI publishers (the -`rs/moq-video` producer under the `rs/moq-mux` rate policy) and the OBS -plugin; the JS publisher is -[JS rung disable](/quest/m2/probe/rung-disable-js.md). `@moq/watch`, -`moq play`, `moqsrc`, WHEP, HLS and DASH export, and the moq-mux exports -already skip disabled renditions and follow one that flips; moq-ffi and moq-c -refuse to subscribe one and leave the fallback pick to the application. +The OBS plugin stops sending its top video renditions when their bandwidth +grant cannot fund them, advertising each as `enabled: false`, and re-enables +them with hysteresis once the grant recovers, as the Rust and FFI publishers +already do. One shaped-uplink run through moq-ffi proves the rule end to end +over real BBR3 probing, including resuming a rung after the grant fell below +the lowest rung. The JS publisher is +[JS rung disable](/quest/m2/probe/rung-disable-js.md). ## Plan -Today each video encoder reserves its configured ceiling and follows its own -grant down (`rs/moq-video/src/encode/producer.rs`), so a congested uplink -squeezes every rendition at once and a top rung starved of most of its share -keeps encoding. - -Decided with the maintainer on 2026-10-20: the native publishers moved to m1 -and require the session send target (a session probes up to its -`bandwidth::Allocator`'s reservations) and -native probing ([moq-dev/moq#6635](https://github.com/moq-dev/moq/pull/6635)), not padding, which is -browser-only. The JS publisher split into its own m2 quest, since it probes -with padding. - -- One rule, one home. Put the disable and re-enable decision in - `moq_mux::rate` beside `Control`, so audio - ([#2848](/quest/m2/2848-follow-the-bandwidth-grant-in-moq-audio-instead-of.md)) - and the transcode [ladder controller](/quest/m3/ladder/controller.md) reuse - it. Start from the ladder's band boundary - (`disable = (max + 2 * lower) / 3`, see the - [ladder questline](/quest/m3/ladder/README.md)) unless measurement argues - otherwise. The JS quest mirrors the same rule and constants. -- Catalog state follows what the encoder applied, not what the policy asked. - Never rewrite the advertised maximum bitrate or remove the rendition. -- Recovery must not depend on demand. Once viewers deselect a disabled rung - its demand, and so its grant, goes away; evaluate a hypothetical share - against the current estimate, as the ladder controller plans to. -- Decided with the maintainer on 2026-10-17 (the - [probe epic](/quest/m2/probe/README.md)): a disabled rung keeps its - reservation at its ceiling. The session's probe target sums every live - reservation, so it keeps probing for the stopped rung and the estimate can - rise to fund it. No padding handle or burst schedule is driven from `Rung`. -- The lowest rendition never disables. Native probing retransmits in-flight - data, so a session with every rung stopped has nothing to probe with once - its last packets are acknowledged, and could never re-enable a rung. -- Disable only on a session with a sender. Native probing needs moq-quic's - BBR3, so a session without it (iroh on noq, or moq-quic configured for - CUBIC or no congestion control) could never raise its estimate to resume a - stopped rung; there every rung keeps encoding and follows its grant down, - as today. Padding stays browser-only rather than covering these. A session - has a sender when its transport's `poll::Session::probe` returns one, which - moq-net keeps private until this quest exposes it to the publishers. -- Assign descending track priority down the renditions so the allocator - fills lower rungs first; the same number breaks ties in send order - between subscriptions at one subscriber priority. -- OBS: each OBS video encoder is its own rendition - (`video_tracks` in `cpp/obs/src/moq-output.cpp`), so apply the rule per - encoder through the generated C++ package, on top of the per-encoder - grant following (`MoQRate` in `cpp/obs/src/moq-rate.h`). - -Tests with mocked time: a shrinking grant disables the top rung first and -keeps the lower ones at their share; a recovering grant re-enables it only -past the hysteresis and after its viewers left; a jittering grant near the -boundary does not flap the catalog; a grant below every boundary keeps the -lowest rung encoding; a session without a sender never disables a rung. -Prove one shaped-uplink run through moq-ffi, including -a path that reopens after the grant fell below the lowest rung. - -Public API: the shared rule in `moq_mux::rate` and whatever knobs the -publishers expose for it. Wire: none; `enabled` already exists. +Landed in [moq-dev/moq#6781](https://github.com/moq-dev/moq/pull/6781): the +rule lives in `moq_mux::rate` (`Policy::lower` sets the band boundary +`(max + 2 * lower) / 3`, `Follower` yields `rate::Target::Disabled`), only +once a session probes for the reservation (`bandwidth::Consumer::probed`). +An idle reservation reads the share it would get, so recovery does not depend +on demand. The FFI video producer takes `lower` (the next rendition down), +sits one track priority below it, drains its encoder while disabled, and +reopens on a keyframe; `moq_encode_video` takes the same `lower` handle. + +- OBS: each OBS video encoder is its own rendition (`video_tracks` in + `cpp/obs/src/moq-output.cpp`) publishing pre-encoded packets through a + media track, and `MoQRate` (`cpp/obs/src/moq-rate.h`) mirrors + `rate::Control` without the rung rule. It needs, through the generated C++ + package: a way to flip a media track's catalog `enabled`, the reservation's + `probed` state (dropped from `MoqReservation` in #6781 for lack of a + consumer), and the lower encoder's ceiling and priority. OBS cannot stop an + encoder or force a keyframe per rung mid-stream, so decide between dropping + packets while disabled and resuming at the next natural keyframe, or + restarting the encoder. +- Shaped uplink: `moq-shaper` never steps its rate limit (queued datagrams + would reorder), so a run whose bottleneck opens back up needs either a + rate step the shaper can apply without reordering, or a second shaper the + first retargets to. Run it nightly with an `#[ignore]` test and a recipe in + `rs/justfile`, beside `quic-sim`. ## Related @@ -81,4 +43,3 @@ publishers expose for it. Wire: none; `enabled` already exists. - [Ladder controller](/quest/m3/ladder/controller.md) - the same rung disable for generated transcode ladders - [OBS multitrack](/quest/m2/obs-multitrack.md) - OBS's native multitrack encoders as the renditions this disables - [Viewer up-switch](/quest/m2/probe/viewer-upswitch.md) - the viewer side of moving between renditions as capacity changes -- [Closed rung disable](https://github.com/moq-dev/moq/pull/5850) - reference code from before the 2026-10-20 split, closed as superseded diff --git a/quest/m2/probe/README.md b/quest/m2/probe/README.md index 71c46f67c44..4202a55bca0 100644 --- a/quest/m2/probe/README.md +++ b/quest/m2/probe/README.md @@ -82,7 +82,7 @@ This README's own work, once the children land: ## Required -- [Native rung disable](/quest/m1/simulcast-rung-disable.md) - Rust/FFI and OBS publishers stop encoding the top renditions their grant cannot fund, advertise `enabled: false`, and re-enable with hysteresis +- [Native rung disable](/quest/m1/simulcast-rung-disable.md) - OBS stops sending the top renditions its grant cannot fund, as the Rust/FFI publishers do, and a shaped uplink through moq-ffi proves the resume - [Padding probes](/quest/m2/probe/padding.md) - a browser probes with a lowest-priority padding stream on an ALR schedule - [JS rung disable](/quest/m2/probe/rung-disable-js.md) - `@moq/publish` applies the native rung rule, recovering through padding - [Viewer targets](/quest/m2/probe/viewer-target.md) - a session sends its viewer handles as its PROBE target, probes to the peer's, splits the receive estimate into viewer grants, and advertises `Increase` where a sender exists From a82a81d598baa50b1873f1b209c737992659276a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 08:57:03 -0700 Subject: [PATCH 09/15] feat(c)!: moq_encode_video takes a moq_video_encode_options Replace the positional bandwidth and lower handles with a struct that holds both, validated before the encoder opens. NULL or zeroed keeps the unadapted behavior, and new fields append. The quest notes the BBR3 max_bw aging it depends on. Co-Authored-By: Claude Opus 5.5 --- doc/lib/c/index.md | 2 +- quest/m1/simulcast-rung-disable.md | 10 ++++++- rs/moq-c/MIGRATE.md | 5 ++-- rs/moq-c/c-tests/decode-output.c | 2 +- rs/moq-c/src/test.rs | 48 +++++++++++++++++------------- rs/moq-c/src/video.rs | 48 +++++++++++++++++++----------- 6 files changed, 72 insertions(+), 43 deletions(-) diff --git a/doc/lib/c/index.md b/doc/lib/c/index.md index 7e96a1382eb..1673c306423 100644 --- a/doc/lib/c/index.md +++ b/doc/lib/c/index.md @@ -51,7 +51,7 @@ if (session < 0) return fail(moq_error()); ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. For simulcast, give each rendition the next one down as `moq_encode_video`'s `lower` handle: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. For simulcast, give each rendition the next one down as the `lower` handle in its `moq_video_encode_options`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. ## Things to know diff --git a/quest/m1/simulcast-rung-disable.md b/quest/m1/simulcast-rung-disable.md index 6baafc39928..a2e156595b2 100644 --- a/quest/m1/simulcast-rung-disable.md +++ b/quest/m1/simulcast-rung-disable.md @@ -19,7 +19,15 @@ once a session probes for the reservation (`bandwidth::Consumer::probed`). An idle reservation reads the share it would get, so recovery does not depend on demand. The FFI video producer takes `lower` (the next rendition down), sits one track priority below it, drains its encoder while disabled, and -reopens on a keyframe; `moq_encode_video` takes the same `lower` handle. +reopens on a keyframe; `moq_encode_video` takes the same `lower` handle in +its `moq_video_encode_options`. + +Depends on BBR3 aging its estimate: the grant this rule reads can stay high +for 20 s or more after the path drops, since `update_max_bw` in +`rs/moq-congestion/src/bbr3/mod.rs` never ages an app-limited `max_bw` +(measured in [moq-dev/moq#6635](https://github.com/moq-dev/moq/pull/6635)), +so a rung keeps sending into a narrowed path until it does. A separate quest +fixes that; the shaped-uplink proof needs it to disable on time. - OBS: each OBS video encoder is its own rendition (`video_tracks` in `cpp/obs/src/moq-output.cpp`) publishing pre-encoded packets through a diff --git a/rs/moq-c/MIGRATE.md b/rs/moq-c/MIGRATE.md index a60f4c5329f..3695019bf1a 100644 --- a/rs/moq-c/MIGRATE.md +++ b/rs/moq-c/MIGRATE.md @@ -4,9 +4,10 @@ How to update your code for each breaking change, newest first. ## Unreleased -### `moq_encode_video` takes a lower rendition +### `moq_encode_video` takes a `moq_video_encode_options` -`moq_encode_video` takes a fifth `uint32_t lower` argument. Pass `0` to keep the old behavior, or the handle of the next lower rendition's encoder to disable this one while its bandwidth grant cannot fund it; that needs a `bandwidth` handle and a bitrate above the lower encoder's. +`moq_encode_video`'s fourth argument is a `const moq_video_encode_options *` instead of a `uint32_t bandwidth`: pass `NULL` where you passed `0`, or `&(moq_video_encode_options){.bandwidth = bandwidth}`. +Its `lower` field takes the handle of the next lower rendition's encoder, to disable this one while its bandwidth grant cannot fund it; that needs a `bandwidth` and a bitrate above the lower encoder's. `moq_reservation_grant` reports the slice the track would get back while nothing subscribes to it, instead of no grant. ### Sessions report a lifecycle snapshot diff --git a/rs/moq-c/c-tests/decode-output.c b/rs/moq-c/c-tests/decode-output.c index 3802b78c56b..d41ecd7f986 100644 --- a/rs/moq-c/c-tests/decode-output.c +++ b/rs/moq-c/c-tests/decode-output.c @@ -237,7 +237,7 @@ int main(void) { moq_video_encoder_input input = {MOQ_VIDEO_PIXEL_FORMAT_RGBA, WIDTH, HEIGHT, {30000, 1001}}; moq_video_encoder_output output = {MOQ_VIDEO_CODEC_H264, 0, 0, MOQ_VIDEO_ENCODER_KIND_SOFTWARE, NULL, 0}; - int32_t producer = moq_encode_video((uint32_t)broadcast, &input, &output, 0, 0); + int32_t producer = moq_encode_video((uint32_t)broadcast, &input, &output, NULL); if (producer <= 0) fail("error: moq_encode_video failed: %d (%s)\n", producer, moq_error()); if (moq_encode_video_cut((uint32_t)producer) < 0) diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index dcb46c37704..979563662f0 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -4187,7 +4187,7 @@ fn video_raw_publish_consume() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); let rgba = gray_rgba(320, 240); let publish = |index: u64| { @@ -4354,7 +4354,7 @@ fn decode_first_frame(output: &moq_video_decoder_output) -> (u32, u32, usize) { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &encode_output, 0, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &encode_output, std::ptr::null()) }); let rgba = gray_rgba(320, 240); let publish = |index: u64| { @@ -4472,7 +4472,7 @@ fn video_raw_publish_from_many_threads() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); // A fresh caller thread per frame, never the one that published. let rgba = std::sync::Arc::new(gray_rgba(320, 240)); @@ -4544,8 +4544,8 @@ fn a_stalled_encode_does_not_block_unrelated_calls() { encoder: std::ptr::null(), encoder_len: 0, }; - let stalled = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); - let other = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); + let stalled = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); + let other = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); // Hold the lock a publish takes for the duration of its encode. let handle = State::lock().video.producer(Id::try_from(stalled).unwrap()).unwrap(); @@ -4626,7 +4626,7 @@ fn video_raw_publish_rejects_frame_size_mismatch() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, 0, 0) }); + let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); // A 640x480 buffer against a 320x240 encoder: the frame carries no dimensions // of its own, so this is caught as a wrong-sized picture. @@ -4668,14 +4668,14 @@ fn video_raw_publish_rejects_invalid_config() { encoder_len: 0, }; - assert!(unsafe { moq_encode_video(broadcast, std::ptr::null(), &valid_output, 0, 0) } < 0); - assert!(unsafe { moq_encode_video(broadcast, &valid_input, std::ptr::null(), 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, std::ptr::null(), &valid_output, std::ptr::null()) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, std::ptr::null(), std::ptr::null()) } < 0); let bad_format = moq_video_encoder_input { format: 99, ..valid_input }; - assert!(unsafe { moq_encode_video(broadcast, &bad_format, &valid_output, 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &bad_format, &valid_output, std::ptr::null()) } < 0); let zero_framerate = moq_video_encoder_input { framerate: moq_video_frame_rate { @@ -4684,7 +4684,7 @@ fn video_raw_publish_rejects_invalid_config() { }, ..valid_input }; - assert!(unsafe { moq_encode_video(broadcast, &zero_framerate, &valid_output, 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &zero_framerate, &valid_output, std::ptr::null()) } < 0); let zero_denominator = moq_video_encoder_input { framerate: moq_video_frame_rate { numerator: 30_000, @@ -4692,7 +4692,7 @@ fn video_raw_publish_rejects_invalid_config() { }, ..valid_input }; - let refused = unsafe { moq_encode_video(broadcast, &zero_denominator, &valid_output, 0, 0) }; + let refused = unsafe { moq_encode_video(broadcast, &zero_denominator, &valid_output, std::ptr::null()) }; assert!(refused < 0 && refused != MOQ_ERROR_PANIC); // Regression: dimensions arrive as a raw `u32` pair, and their product used to @@ -4703,7 +4703,7 @@ fn video_raw_publish_rejects_invalid_config() { height: u32::MAX - 1, ..valid_input }; - let refused = unsafe { moq_encode_video(broadcast, &unrepresentable, &valid_output, 0, 0) }; + let refused = unsafe { moq_encode_video(broadcast, &unrepresentable, &valid_output, std::ptr::null()) }; assert!(refused < 0 && refused != MOQ_ERROR_PANIC, "{refused}"); // A size no encoder can take, but whose arithmetic is fine, is the backend's @@ -4718,7 +4718,7 @@ fn video_raw_publish_rejects_invalid_config() { height: 2, ..valid_input }; - let huge = unsafe { moq_encode_video(broadcast, &merely_huge, &valid_output, 0, 0) }; + let huge = unsafe { moq_encode_video(broadcast, &merely_huge, &valid_output, std::ptr::null()) }; if huge > 0 { assert_eq!(moq_encode_video_finish(id(huge)), 0); } else { @@ -4733,13 +4733,13 @@ fn video_raw_publish_rejects_invalid_config() { codec: 99, ..valid_output }; - assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_codec, 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_codec, std::ptr::null()) } < 0); let bad_kind = moq_video_encoder_output { kind: 99, ..valid_output }; - assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_kind, 0, 0) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &valid_input, &bad_kind, std::ptr::null()) } < 0); // Handles for a producer that was never created. assert!(moq_encode_video_cut(0) < 0); @@ -5640,7 +5640,14 @@ fn encode_video_bitrate_caps_the_reservation() { encoder: std::ptr::null(), encoder_len: 0, }; - let producer = id(unsafe { moq_encode_video(broadcast, &input, &output, bandwidth, 0) }); + let producer = id(unsafe { + moq_encode_video( + broadcast, + &input, + &output, + &moq_video_encode_options { bandwidth, lower: 0 }, + ) + }); let reservation = id(moq_encode_video_reservation(producer)); let (sub, cb) = subscribe_named_track(consume, b"0.avc3"); @@ -5691,10 +5698,11 @@ fn encode_video_rung_needs_a_grant_and_a_band() { encoder: std::ptr::null(), encoder_len: 0, }; - let low = id(unsafe { moq_encode_video(broadcast, &input, &output(500_000), bandwidth, 0) }); - assert!(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), 0, low) } < 0); - assert!(unsafe { moq_encode_video(broadcast, &input, &output(400_000), bandwidth, low) } < 0); - let high = id(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), bandwidth, low) }); + let options = |bandwidth, lower| moq_video_encode_options { bandwidth, lower }; + let low = id(unsafe { moq_encode_video(broadcast, &input, &output(500_000), &options(bandwidth, 0)) }); + assert!(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), &options(0, low)) } < 0); + assert!(unsafe { moq_encode_video(broadcast, &input, &output(400_000), &options(bandwidth, low)) } < 0); + let high = id(unsafe { moq_encode_video(broadcast, &input, &output(2_000_000), &options(bandwidth, low)) }); assert_eq!(moq_encode_video_finish(high), 0); assert_eq!(moq_encode_video_finish(low), 0); diff --git a/rs/moq-c/src/video.rs b/rs/moq-c/src/video.rs index aabe973d85b..9eeec698b7d 100644 --- a/rs/moq-c/src/video.rs +++ b/rs/moq-c/src/video.rs @@ -117,6 +117,26 @@ pub struct moq_video_encoder_output { pub encoder_len: usize, } +/// Rate control for [`moq_encode_video`]. +/// +/// NULL or zeroed holds the configured bitrate regardless of congestion. New fields +/// always append, so a zeroed struct keeps meaning the defaults. +#[repr(C)] +#[derive(Clone, Copy, Default)] +pub struct moq_video_encode_options { + /// A handle from [`crate::moq_session_bandwidth`] whose grant the encoder follows, + /// or 0 to hold the configured bitrate. + pub bandwidth: u32, + /// The [`moq_encode_video`] handle of the next lower rendition of the same source, + /// or 0. It makes this encoder a simulcast rung: while its grant cannot fund it, it + /// stops encoding (frames published to it are dropped) and advertises itself + /// disabled, so the lower renditions keep their share, then resumes on a keyframe + /// once the grant recovers. It takes the track priority one below `lower`'s, so the + /// allocator fills that one first. It needs a `bandwidth`, and a bitrate above + /// `lower`'s. + pub lower: u32, +} + /// One raw frame handed to [`moq_encode_video_frame`]. /// /// Pixel format and resolution are fixed by [`moq_video_encoder_input`] at @@ -847,27 +867,26 @@ unsafe fn encoder_kind(output: &moq_video_encoder_output) -> Resultencoder` must point to `output->encoder_len` bytes of UTF-8 when /// `output->kind` is `MOQ_VIDEO_ENCODER_KIND_NAMED`. -/// - `bandwidth` is a handle from [`crate::moq_session_bandwidth`], or 0 to hold the -/// configured bitrate regardless of congestion. -/// - `lower` is the handle of the next lower rendition of the same source from this -/// function, or 0. A non-zero `lower` makes this encoder a simulcast rung: when the -/// bandwidth grant can no longer fund it, it stops encoding (frames published to it -/// are dropped) and advertises itself disabled, so the lower renditions keep their -/// share, and it resumes on a keyframe once the grant recovers. It takes the track -/// priority one below `lower`'s, so the allocator fills that one first. It needs a -/// `bandwidth`, and a bitrate above `lower`'s. +/// - `options` may be NULL, or must point at a readable [`moq_video_encode_options`]. #[unsafe(no_mangle)] pub unsafe extern "C" fn moq_encode_video( broadcast: u32, input: *const moq_video_encoder_input, output: *const moq_video_encoder_output, - bandwidth: u32, - lower: u32, + options: *const moq_video_encode_options, ) -> i32 { ffi::enter(move || { let broadcast = ffi::parse_id(broadcast)?; let raw_input = unsafe { input.as_ref() }.ok_or(Error::InvalidPointer)?; let raw_output = unsafe { output.as_ref() }.ok_or(Error::InvalidPointer)?; + let options = unsafe { options.as_ref() }.copied().unwrap_or_default(); + let bandwidth = ffi::parse_id_optional(options.bandwidth)?; + let lower = ffi::parse_id_optional(options.lower)?; + if lower.is_some() && bandwidth.is_none() { + return Err(Error::InvalidConfig( + "a lower rendition needs a bandwidth allocator to follow".into(), + )); + } let format = pixel_format_from_u32(raw_input.format)?; @@ -891,13 +910,6 @@ pub unsafe extern "C" fn moq_encode_video( let rendition = block_on(config.probe())?; let encoder = block_on(moq_video::encode::Sink::open(&config))?; - let bandwidth = ffi::parse_id_optional(bandwidth)?; - let lower = ffi::parse_id_optional(lower)?; - if lower.is_some() && bandwidth.is_none() { - return Err(Error::InvalidConfig( - "a lower rendition needs a bandwidth allocator to follow".into(), - )); - } let mut state = State::lock(); let allocator = bandwidth.map(|id| state.bandwidth.allocator(id)).transpose()?; let State { publish, video, .. } = &mut *state; From 30c4025942b264e490085d30f2f3ec47dc2b618b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 09:58:59 -0700 Subject: [PATCH 10/15] fix(rate): a new lower rendition keeps the applied target Follower::set_policy reset the target to the ceiling on any policy change, but a lower rendition moving or finishing never retuned this sender, so a grant at the ceiling read as already applied and the encoder stayed below it. Only a new ceiling or floor starts over now. Co-Authored-By: Claude Opus 5.5 --- rs/moq-mux/src/rate.rs | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/rs/moq-mux/src/rate.rs b/rs/moq-mux/src/rate.rs index 65fb746f17f..9f6e6fce3db 100644 --- a/rs/moq-mux/src/rate.rs +++ b/rs/moq-mux/src/rate.rs @@ -422,17 +422,25 @@ impl Follower { &self.control.policy } - /// Follow under `policy` from now on. A different policy starts over at its - /// [`Policy::max`], as a fresh [`Follower`] would, and the next poll feeds it the - /// current grant. A disabled sender stays disabled until that grant resumes it - /// under the new policy, since nothing told it to start. + /// Follow under `policy` from now on, feeding it the current grant on the next poll. + /// + /// A new ceiling or floor starts over at its [`Policy::max`], as a fresh [`Follower`] + /// would, since the sender applied that ceiling itself. A new + /// [`lower`](Policy::lower) alone keeps the target and ramp, since nothing retuned + /// the sender. Either way a disabled sender stays disabled until that grant resumes + /// it under the new policy, since nothing told it to start. pub fn set_policy(&mut self, policy: Policy) { - if policy != self.control.policy { + if policy == self.control.policy { + return; + } + if policy.max == self.control.policy.max && policy.min == self.control.policy.min { + self.control.policy = policy; + } else { let enabled = self.control.enabled; self.control = Control::new(policy); self.control.enabled = enabled; - self.refeed = true; } + self.refeed = true; } /// Poll for the next target the sender should produce at, or the error that @@ -969,6 +977,28 @@ mod tests { assert_eq!(follower.next().await.unwrap(), on(bps(3_000_000))); } + /// A new lower rendition alone keeps the target the sender is actually at, so a + /// grant at the ceiling still ramps it there instead of reading as already applied. + #[tokio::test(start_paused = true)] + async fn a_new_lower_keeps_the_applied_target() { + let grant = bandwidth::Producer::new(); + let mut follower = Follower::new(grant.consume(), Policy::new(bps(4_000_000))); + grant.set(Some(bps(2_000_000))).unwrap(); + assert_eq!(follower.next().await.unwrap(), on(bps(2_000_000))); + + grant.set(Some(bps(4_000_000))).unwrap(); + let mut policy = Policy::new(bps(4_000_000)); + policy.lower = Some(bps(1_000_000)); + follower.set_policy(policy); + + let mut applied = Vec::new(); + while let Ok(target) = moq_clock::timeout(Duration::from_secs(60), follower.next()).await { + applied.push(target.unwrap()); + } + assert_eq!(applied.last(), Some(&on(bps(4_000_000))), "{applied:?}"); + assert!(applied.len() > 1, "a raise from 2 Mbps ramps: {applied:?}"); + } + /// A grant no session probes for, like a whole estimate, never disables a rendition, /// however far it falls under the band boundary. #[tokio::test(start_paused = true)] From f8a24df14ff2fad82dbdddbe422eb5d3370f65e3 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 10:08:56 -0700 Subject: [PATCH 11/15] fix(ffi,c): take back a target dropped for a moved policy When the policy moves between a follower's target and its apply, the target is dropped unapplied. Now that a moved lower rendition keeps the follower's state, it would count that target applied, so a disable could leave the encoder running. Roll it back before installing the policy. Co-Authored-By: Claude Opus 5.5 --- rs/moq-c/src/video.rs | 27 +++++++++++++++++++++++++++ rs/moq-ffi/src/video.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/rs/moq-c/src/video.rs b/rs/moq-c/src/video.rs index 9eeec698b7d..93ff6291a3f 100644 --- a/rs/moq-c/src/video.rs +++ b/rs/moq-c/src/video.rs @@ -317,6 +317,9 @@ fn retune( apply: impl FnOnce(moq_mux::rate::Target) -> Result<(), moq_video::Error>, ) -> Result, &'static str> { if *follower.policy() != policy { + // The target was never applied, so take it back first: a policy that keeps the + // follower's state (a moved lower rendition) would otherwise count it applied. + follower.failed(); follower.set_policy(policy); return Ok(None); } @@ -1297,6 +1300,30 @@ mod follow_tests { }) } + /// A lower rendition moving between a target and its apply drops that target, and the + /// follower asks for it again under the new policy rather than counting it applied. + #[tokio::test(start_paused = true)] + async fn a_moved_lower_retries_the_dropped_target() { + let policy = |lower| { + let mut policy = Policy::new(Rate::from_mbps(4)); + policy.lower = Some(lower); + policy + }; + let grant = moq_net::bandwidth::Producer::new(); + let mut follower = Follower::new(grant.consume(), policy(Rate::from_mbps(1))); + grant.set(Some(Rate::from_mbps(2))).unwrap(); + let dropped = follower.next().await.unwrap(); + let moved = retune(&mut follower, policy(Rate::from_kbps(900)), dropped, |_| { + unreachable!("a target under a stale policy is never applied") + }); + assert_eq!(moved, Ok(None)); + + let retried = moq_clock::timeout(Duration::from_secs(60), follower.next()) + .await + .expect("the dropped target is asked for again"); + assert_eq!(retried.unwrap(), Target::Enabled(Rate::from_mbps(2))); + } + fn bitrate(target: Target) -> Rate { let Target::Enabled(bitrate) = target else { unreachable!("no lower rendition") diff --git a/rs/moq-ffi/src/video.rs b/rs/moq-ffi/src/video.rs index 2488e90c34c..70d7e3ced67 100644 --- a/rs/moq-ffi/src/video.rs +++ b/rs/moq-ffi/src/video.rs @@ -917,6 +917,9 @@ fn retune( apply: impl FnOnce(moq_mux::rate::Target) -> Result<(), moq_video::Error>, ) -> Result, &'static str> { if *follower.policy() != policy { + // The target was never applied, so take it back first: a policy that keeps the + // follower's state (a moved lower rendition) would otherwise count it applied. + follower.failed(); follower.set_policy(policy); return Ok(None); } @@ -1405,6 +1408,30 @@ mod follow_tests { }) } + /// A lower rendition moving between a target and its apply drops that target, and the + /// follower asks for it again under the new policy rather than counting it applied. + #[tokio::test(start_paused = true)] + async fn a_moved_lower_retries_the_dropped_target() { + let policy = |lower| { + let mut policy = Policy::new(Rate::from_mbps(4)); + policy.lower = Some(lower); + policy + }; + let grant = moq_net::bandwidth::Producer::new(); + let mut follower = Follower::new(grant.consume(), policy(Rate::from_mbps(1))); + grant.set(Some(Rate::from_mbps(2))).unwrap(); + let dropped = follower.next().await.unwrap(); + let moved = retune(&mut follower, policy(Rate::from_kbps(900)), dropped, |_| { + unreachable!("a target under a stale policy is never applied") + }); + assert_eq!(moved, Ok(None)); + + let retried = moq_clock::timeout(Duration::from_secs(60), follower.next()) + .await + .expect("the dropped target is asked for again"); + assert_eq!(retried.unwrap(), Target::Enabled(Rate::from_mbps(2))); + } + fn bitrate(target: Target) -> Rate { let Target::Enabled(bitrate) = target else { unreachable!("no lower rendition") From 7b6e6c766d95784fc606f176367d24bfff7d410b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 16:34:36 -0700 Subject: [PATCH 12/15] fix(ffi): adopt typed durations in the rung tests after the main merge Co-Authored-By: Claude Opus 5.5 --- rs/moq-ffi/src/video.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/moq-ffi/src/video.rs b/rs/moq-ffi/src/video.rs index c4322dd3c58..6fe5ec816a2 100644 --- a/rs/moq-ffi/src/video.rs +++ b/rs/moq-ffi/src/video.rs @@ -1592,7 +1592,7 @@ mod rung_tests { .subscribe_track(track.into(), None) .await .unwrap(); - track.info().unwrap().priority + track.info().priority } /// A rung sits one priority below its lower rendition, so the allocator fills that first. @@ -1649,7 +1649,7 @@ mod rung_tests { "high".into(), Some(crate::consumer::MoqSubscription { priority: 0, - max_delay_us: None, + max_delay: None, ranges: None, order: None, }), From aee8421b4adbdcf9b8088ec17699025a8b9bc39e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 17:04:03 -0700 Subject: [PATCH 13/15] fix(rate): close the rung state holes and model-check Control A disabled rung whose failed resume falls back under the margin drops its retry deadline, which otherwise left the follower spinning on a fired timer. A rung whose applied target sits under its boundary (probing just started, or the lower rendition rose) disables on a grant below it instead of being held on by the deadband. A reservation is no longer probed while a zero target turns probing off. A seeded proptest replays random grants, probing flips, policy changes, refused changes, and time through Control the way Follower drives it, checking the rung's invariants after every wake. Follower::set_policy moves onto Control so the model drives the same code. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + rs/moq-mux/Cargo.toml | 1 + rs/moq-mux/src/rate.rs | 300 ++++++++++++++++++++++++++++-- rs/moq-net/src/model/bandwidth.rs | 21 ++- 4 files changed, 307 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f457cd227d..9992326e8ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5028,6 +5028,7 @@ dependencies = [ "mp4-atom", "mpeg2ts", "num_enum", + "proptest", "roxmltree", "scuffle-av1", "scuffle-h265", diff --git a/rs/moq-mux/Cargo.toml b/rs/moq-mux/Cargo.toml index 4359aa26a77..904d704ac32 100644 --- a/rs/moq-mux/Cargo.toml +++ b/rs/moq-mux/Cargo.toml @@ -47,6 +47,7 @@ webm-iterable = "0.7" criterion = { workspace = true } futures = { workspace = true } moq-clock = { workspace = true, features = ["sim"] } +proptest = { workspace = true } tokio = { workspace = true, features = ["test-util"] } tracing-test = { workspace = true, features = ["no-env-filter"] } diff --git a/rs/moq-mux/src/rate.rs b/rs/moq-mux/src/rate.rs index 9f6e6fce3db..376778eaf48 100644 --- a/rs/moq-mux/src/rate.rs +++ b/rs/moq-mux/src/rate.rs @@ -206,6 +206,23 @@ impl Control { self.probed = probed; } + /// Follow `policy` from the next [`update`](Self::update), returning whether it changed; + /// see [`Follower::set_policy`]. + fn set_policy(&mut self, policy: Policy) -> bool { + if policy == self.policy { + return false; + } + if policy.max == self.policy.max && policy.min == self.policy.min { + self.policy = policy; + } else { + let (enabled, probed) = (self.enabled, self.probed); + *self = Control::new(policy); + self.enabled = enabled; + self.probed = probed; + } + true + } + /// When to [`update`](Self::update) again with the latest estimate, changed or /// not, to continue a raise or retry a failed change; `None` once the target has /// settled. @@ -244,6 +261,8 @@ impl Control { .saturating_add(band(boundary).as_bps()) .min(self.policy.max.as_bps()); if desired.as_bps() < clear { + // A retry of a failed resume is no longer due either. + self.deadline = None; return None; } // Restart at the boundary and ramp: the estimate was measured while @@ -304,8 +323,15 @@ impl Control { // Drops keep the deadband either way: sitting a little above a falling // estimate is what it's for. That is also what keeps a grant jittering // around the band boundary from flapping the rendition off and on. + // + // The deadband only holds a rung on above a boundary that was already holding + // it. One whose applied target is under the boundary, because probing just + // started or the lower rendition rose, disables at once on a grant below it. + let disable = boundary.is_some_and(|boundary| desired < boundary); + let under = boundary.is_some_and(|boundary| self.target < boundary); let settling = next > self.target && next == self.policy.max; - if next == self.target || (!settling && next.abs_diff(self.target) < band(self.target)) { + let suppressed = next == self.target || (!settling && next.abs_diff(self.target) < band(self.target)); + if suppressed && !(disable && under) { self.deadline = self.raise(desired); return None; } @@ -316,7 +342,9 @@ impl Control { applied: self.applied, desired, }); - if boundary.is_some_and(|boundary| next < boundary) { + // On `desired` rather than `next`: a ramp still climbing from under the boundary + // toward a grant above it is funded, so it stays on. + if disable { // Only a new estimate clearing the boundary resumes it, so nothing is due. self.enabled = false; self.deadline = None; @@ -430,17 +458,9 @@ impl Follower { /// the sender. Either way a disabled sender stays disabled until that grant resumes /// it under the new policy, since nothing told it to start. pub fn set_policy(&mut self, policy: Policy) { - if policy == self.control.policy { - return; + if self.control.set_policy(policy) { + self.refeed = true; } - if policy.max == self.control.policy.max && policy.min == self.control.policy.min { - self.control.policy = policy; - } else { - let enabled = self.control.enabled; - self.control = Control::new(policy); - self.control.enabled = enabled; - } - self.refeed = true; } /// Poll for the next target the sender should produce at, or the error that @@ -1184,6 +1204,65 @@ mod tests { ); } + /// A resume the sender failed, whose grant then falls back under the margin, drops + /// its retry: an update at that deadline would change nothing, so a follower woken + /// by it would spin. + #[test] + fn a_failed_resume_under_the_margin_stops_retrying() { + let mut rung = rung(); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(3_000_000)), start), Some(Target::Disabled)); + let resumed = start + Duration::from_secs(1); + assert_eq!(rung.update(Some(bps(5_000_000)), resumed), Some(on(bps(3_333_333)))); + rung.failed(resumed); + assert_eq!(rung.deadline(), Some(resumed + RETRY)); + + assert_eq!(rung.update(Some(bps(3_000_000)), resumed + RETRY / 2), None); + assert_eq!(rung.target(), Target::Disabled); + assert_eq!(rung.deadline(), None); + } + + /// A rung following an unprobed grant under its boundary disables once probing starts, + /// though the grant never moves, unless the grant can fund it. + #[test] + fn a_rung_under_its_boundary_disables_when_probing_starts() { + let mut rung = rung(); + rung.set_probed(false); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(3_000_000)), start), Some(on(bps(3_000_000)))); + + let mut funded = rung.clone(); + rung.set_probed(true); + assert_eq!( + rung.update(Some(bps(3_000_000)), start + Duration::from_millis(100)), + Some(Target::Disabled) + ); + + // A grant over the boundary funds it, so it ramps up from under it instead. + funded.set_probed(true); + assert_eq!( + funded.update(Some(bps(5_000_000)), start + Duration::from_secs(1)), + Some(on(bps(3_750_000))) + ); + } + + /// The same when the lower rendition rises over the applied target. + #[test] + fn a_rung_under_a_raised_boundary_disables() { + let mut rung = rung(); + let start = moq_clock::now(); + assert_eq!(rung.update(Some(bps(3_500_000)), start), Some(on(bps(3_500_000)))); + + // The boundary moves from 3.33 Mbps to 3.67 Mbps, over the applied target. + let mut policy = rung.policy.clone(); + policy.lower = Some(bps(3_000_000)); + assert!(rung.set_policy(policy)); + assert_eq!( + rung.update(Some(bps(3_500_000)), start + Duration::from_millis(100)), + Some(Target::Disabled) + ); + } + /// End to end through the allocator: descending priority down the renditions fills /// the lower ones first, so a shrinking estimate disables the top rendition and leaves /// the lower ones at their full share. The top ones come back once the estimate @@ -1263,4 +1342,201 @@ mod tests { assert_eq!(recovered[1], on(bps(1_333_333))); assert_eq!(recovered[2], on(bps(2_666_666))); } + + /// Random walks of grants, probing, policy changes, refused changes, and time, + /// replayed through [`Control`] the way [`Follower`] drives it, checking the rung's + /// invariants after every wake. + mod model { + use proptest::prelude::*; + use proptest::test_runner::{Config, RngSeed, TestRunner}; + + use super::*; + + #[derive(Clone, Debug)] + enum Op { + Grant(Option), + /// Move the grant by this many per mille, around the deadband and the margins. + Jitter(i16), + Probed(bool), + Lower(Option), + Max(u64), + Min(u64), + Advance(Duration), + } + + fn op() -> impl Strategy { + let rate = || prop_oneof![0..12_000_000u64, 0..2_000u64]; + prop_oneof![ + 3 => proptest::option::weighted(0.9, rate()).prop_map(Op::Grant), + 3 => (-80i16..=80).prop_map(Op::Jitter), + 2 => any::().prop_map(Op::Probed), + 2 => proptest::option::weighted(0.8, rate()).prop_map(Op::Lower), + 1 => (1..10_000_000u64).prop_map(Op::Max), + 1 => rate().prop_map(Op::Min), + 4 => prop_oneof![0..400u64, 0..20_000u64].prop_map(|ms| Op::Advance(Duration::from_millis(ms))), + ] + } + + /// What the follower feeds the control, and when. + struct Model { + control: Control, + now: Instant, + grant: Option, + probed: bool, + } + + impl Model { + fn apply(&mut self, op: Op, fail: bool) { + let mut policy = self.control.policy.clone(); + match op { + Op::Grant(grant) => self.set_grant(grant.map(bps), fail), + Op::Jitter(per_mille) => { + let Some(grant) = self.grant else { return }; + let moved = i128::from(grant.as_bps()) * (1_000 + i128::from(per_mille)) / 1_000; + self.set_grant(Some(bps(moved as u64)), fail); + } + Op::Probed(probed) => { + // The grant reports a flip, and only a flip. + if probed != self.probed { + self.probed = probed; + self.wake(fail); + } + } + Op::Lower(lower) => { + policy.lower = lower.map(bps); + self.set_policy(policy, fail); + } + Op::Max(max) => { + policy.max = bps(max); + self.set_policy(policy, fail); + } + Op::Min(min) => { + policy.min = bps(min); + self.set_policy(policy, fail); + } + Op::Advance(elapsed) => { + let end = self.now + elapsed; + while let Some(at) = self.control.deadline().filter(|at| *at <= end) { + self.now = at; + self.wake(fail); + } + self.now = end; + } + } + } + + fn set_grant(&mut self, grant: Option, fail: bool) { + // The grant wakes the follower only when it moves. + if grant != self.grant { + self.grant = grant; + self.wake(fail); + } + } + + fn set_policy(&mut self, policy: Policy, fail: bool) { + if self.control.set_policy(policy) { + self.wake(fail); + } + } + + /// One update, as the follower makes on any wake, then the sender refusing what + /// it returned when `fail`. + fn wake(&mut self, fail: bool) { + let before = self.control.clone(); + self.control.set_probed(self.probed); + let change = self.control.update(self.grant, self.now); + self.check(&before, change); + if fail && change.is_some() { + self.control.failed(self.now); + } + // Anything already due would wake the follower again at once, for nothing. + if let Some(at) = self.control.deadline() { + assert!(at > self.now, "a deadline at {at:?} is already due at {:?}", self.now); + } + } + + fn check(&self, before: &Control, change: Option) { + let policy = &self.control.policy; + let min = policy.min.min(policy.max); + let boundary = policy.boundary(); + if let Target::Enabled(rate) = self.control.target() { + assert!(rate <= policy.max, "{rate:?} over {policy:?}"); + assert!(rate >= min, "{rate:?} under {policy:?}"); + } + + if change == Some(Target::Disabled) { + let boundary = boundary.expect("a policy without a band disabled the rung"); + assert!(self.probed, "an unprobed rung disabled"); + let desired = self + .grant + .expect("a missing estimate disabled the rung") + .clamp(min, policy.max); + assert!(desired < boundary, "{desired:?} is not under {boundary:?}"); + let Target::Enabled(was) = before.target() else { + panic!("a disabled rung disabled again") + }; + // Outside the deadband, unless the applied target was already under the boundary. + assert!( + was < boundary || was.as_bps() - desired.as_bps() >= band(was).as_bps(), + "{was:?} to {desired:?} is inside the deadband" + ); + } + + // A missing estimate holds whatever the rung was doing. + let Some(estimate) = self.grant else { return }; + let desired = estimate.clamp(min, policy.max); + match self.control.target() { + Target::Disabled => { + assert!(self.probed, "an unprobed rung stayed disabled"); + let boundary = boundary.expect("a policy without a band stayed disabled"); + let clear = (boundary.as_bps() + band(boundary).as_bps()).min(policy.max.as_bps()); + assert!( + desired.as_bps() < clear, + "{desired:?} clears {boundary:?} by the margin, yet the rung stayed disabled" + ); + } + Target::Enabled(rate) => { + if let Some(boundary) = boundary.filter(|boundary| self.probed && desired < *boundary) { + assert!(rate >= boundary, "{rate:?} stayed on under {boundary:?} at {desired:?}"); + assert!( + rate.as_bps() - desired.as_bps() < band(rate).as_bps(), + "{rate:?} stayed on past the deadband at {desired:?}" + ); + } + } + } + } + } + + #[test] + fn invariants_hold_on_random_walks() { + let mut runner = TestRunner::new(Config { + cases: 512, + rng_seed: RngSeed::Fixed(6781), + failure_persistence: None, + ..Default::default() + }); + let walk = ( + 1..10_000_000u64, + proptest::option::of(0..12_000_000u64), + proptest::collection::vec((op(), proptest::bool::weighted(0.1)), 1..200), + ); + runner + .run(&walk, |(max, lower, ops)| { + let mut policy = Policy::new(bps(max)); + policy.lower = lower.map(bps); + let mut model = Model { + control: Control::new(policy), + now: moq_clock::now(), + grant: None, + probed: false, + }; + for (op, fail) in ops { + model.apply(op, fail); + } + Ok(()) + }) + .unwrap(); + } + } } diff --git a/rs/moq-net/src/model/bandwidth.rs b/rs/moq-net/src/model/bandwidth.rs index b7d7daae558..0549e75f69c 100644 --- a/rs/moq-net/src/model/bandwidth.rs +++ b/rs/moq-net/src/model/bandwidth.rs @@ -457,7 +457,6 @@ impl Registry { } /// What the session probes up to. - #[cfg(any(feature = "async", test))] fn target(&self) -> Rate { self.target .unwrap_or_else(|| Rate::from_bps(u64::try_from(self.reserved).unwrap_or(u64::MAX))) @@ -565,9 +564,11 @@ impl Share { allocate(estimate, &wants, self.id) } - /// Whether a session probes for this share's registry. + /// Whether a session probes for this share's registry, toward a target above zero, + /// which turns probing off. fn probed(&self) -> bool { - self.registry.read().probes > 0 + let registry = self.registry.read(); + registry.probes > 0 && registry.target() > Rate::ZERO } /// This share's slice and whether a session probes for it, arming `waiter` for @@ -714,7 +715,8 @@ impl Consumer { /// Whether a session probes the connection for this slice's reservation, so the /// estimate can rise to fund it even while its sender is stopped. /// - /// Only a session whose transport has a sender probes. Without one the estimate + /// Only a session whose transport has a sender probes, and none does while + /// [`Allocator::set_target`] turns probing off. Without one the estimate /// stops learning once the connection is app-limited, so a sender that stops for /// lack of bandwidth could never see enough to resume. Always `false` for a whole /// estimate, which no reservation backs. [`poll_changed`](Self::poll_changed) @@ -1336,6 +1338,17 @@ mod tests { assert!(grant.probed()); assert!(grant.poll_changed(&waiter).is_pending()); + // A zero target turns probing off, though the session still follows it. + let (woken, waiter) = Woken::new(); + assert!(grant.poll_changed(&waiter).is_pending()); + allocator.set_target(Some(Rate::ZERO)); + assert!(woken.woken(), "turning probing off must wake the reader"); + assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); + assert!(!grant.probed()); + allocator.set_target(None); + assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); + assert!(grant.probed()); + drop(target); assert!(matches!(grant.poll_changed(&waiter), Poll::Ready(Ok(Some(rate))) if rate == bps(2_000_000))); assert!(!grant.probed()); From 80ab0740bb745f12c3aeda72cce2a3c746a5d573 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 17:39:52 -0700 Subject: [PATCH 14/15] fix(ffi,c): a rung watches its lower rendition's ceiling directly A rung learned that its lower rendition moved or finished only through the reservation the two share. A lower rendition with no allocator, or another session's, moved nothing this follower watched, so a disabled rung whose lower finished stayed disabled. Each video encoder now publishes its ceiling on a kio channel that closes when it finishes, and a rung's follow loop polls it. Co-Authored-By: Claude Opus 5.5 --- rs/moq-c/src/state.rs | 5 -- rs/moq-c/src/test.rs | 66 +++++++++++++++++++++++++ rs/moq-c/src/video.rs | 83 ++++++++++++++++++------------- rs/moq-ffi/src/video.rs | 106 ++++++++++++++++++++++++++++++---------- 4 files changed, 196 insertions(+), 64 deletions(-) diff --git a/rs/moq-c/src/state.rs b/rs/moq-c/src/state.rs index e2b99689598..ec7832ee658 100644 --- a/rs/moq-c/src/state.rs +++ b/rs/moq-c/src/state.rs @@ -63,11 +63,6 @@ impl Shared { self.0.lock().unwrap_or_else(PoisonError::into_inner) } - /// A handle that reads the resource without keeping it alive. - pub fn downgrade(&self) -> std::sync::Weak>> { - Arc::downgrade(&self.0) - } - /// Take the resource, for the call that ends it and needs to consume it. pub fn take(&self) -> Option { self.lock().take() diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 7b15023e526..6372e2e8776 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -5799,6 +5799,72 @@ fn encode_video_rung_needs_a_grant_and_a_band() { assert_eq!(moq_origin_close(origin), 0); } +/// A rung watches its lower rendition's own ceiling, which wakes it when that moves or +/// ends, so it notices even when the lower rendition reserves nothing it shares. +#[test] +fn encode_video_ceiling_wakes_a_rung_above() { + struct Flag(std::sync::atomic::AtomicBool); + impl std::task::Wake for Flag { + fn wake(self: std::sync::Arc) { + self.0.store(true, std::sync::atomic::Ordering::SeqCst); + } + } + + let origin = id(moq_origin_create()); + let broadcast = publish_broadcast(origin, b"rung"); + let input = moq_video_encoder_input { + format: moq_video_pixel_format::MOQ_VIDEO_PIXEL_FORMAT_RGBA as u32, + width: 320, + height: 240, + framerate: moq_video_frame_rate { + numerator: 30, + denominator: 1, + }, + }; + let output = moq_video_encoder_output { + codec: moq_video_codec::MOQ_VIDEO_CODEC_H264 as u32, + bitrate: 500_000, + gop: 0, + kind: moq_video_encoder_kind::MOQ_VIDEO_ENCODER_KIND_SOFTWARE as u32, + encoder: std::ptr::null(), + encoder_len: 0, + }; + // No allocator, so nothing it does moves a reservation a rung would share. + let low = id(unsafe { moq_encode_video(broadcast, &input, &output, std::ptr::null()) }); + let ceiling = State::lock() + .video + .producer(Id::try_from(low).unwrap()) + .unwrap() + .lock() + .as_ref() + .unwrap() + .ceiling + .consume(); + + let flag = std::sync::Arc::new(Flag(std::sync::atomic::AtomicBool::new(false))); + let waiter = kio::Waiter::new(std::task::Waker::from(flag.clone())); + let arm = || ceiling.poll(&waiter, |_| std::task::Poll::<()>::Pending).is_pending(); + assert!(arm()); + + assert_eq!(moq_encode_video_bitrate(low, 400_000), 0); + assert!( + flag.0.swap(false, std::sync::atomic::Ordering::SeqCst), + "a moved ceiling wakes the rung" + ); + assert_eq!(*ceiling.read(), moq_net::bandwidth::Rate::from_bps(400_000)); + assert!(arm()); + + assert_eq!(moq_encode_video_finish(low), 0); + assert!( + flag.0.load(std::sync::atomic::Ordering::SeqCst), + "an ended rendition wakes the rung" + ); + assert!(!arm(), "the ceiling closes with the rendition"); + + assert_eq!(moq_publish_close(broadcast), 0); + assert_eq!(moq_origin_close(origin), 0); +} + /// Listen on an ephemeral loopback port with a generated certificate. fn listen(cb: &Callback) -> (u32, String, String) { let bind = "127.0.0.1:0"; diff --git a/rs/moq-c/src/video.rs b/rs/moq-c/src/video.rs index 93ff6291a3f..536f8306bb0 100644 --- a/rs/moq-c/src/video.rs +++ b/rs/moq-c/src/video.rs @@ -254,16 +254,19 @@ fn video_policy(ceiling: moq_net::bandwidth::Rate, lower: Option>>; - /// The lower rendition's current ceiling, or `None` once it finished, which leaves -/// this one nothing to fall back to, so it never disables. -fn lower_ceiling(lower: &Option) -> Option { - let lower = lower.as_ref()?.upgrade()?; - let guard = lower.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - guard.as_ref().map(|encoder| encoder.ceiling) +/// this one nothing to fall back to, so it never disables. Arms `waiter` for either +/// change. +fn lower_ceiling( + lower: &Option>, + waiter: &kio::Waiter, +) -> Option { + let lower = lower.as_ref()?; + match lower.poll(waiter, |_| std::task::Poll::<()>::Pending) { + std::task::Poll::Pending => Some(*lower.read()), + // The closure never completes, so this is the channel closing. + std::task::Poll::Ready(_) => None, + } } /// Feed each grant through the same policy moq-video uses, and retune the live @@ -271,35 +274,37 @@ fn lower_ceiling(lower: &Option) -> Option { /// [`BitrateUnsupported`](moq_video::Error::BitrateUnsupported). async fn follow_reservation( inner: Shared, - lower: Option, + lower: Option>, consumer: moq_net::bandwidth::Consumer, ceiling: moq_net::bandwidth::Rate, ) { - let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower))); + let noop = kio::Waiter::noop(); + let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower, &noop))); loop { // Both ceilings are read on every wake, before the follower filters a grant against - // them: `moq_encode_video_bitrate` (here or on the lower rendition) moves a - // reservation after moving its ceiling, which wakes this poll. The lower one is - // read first, so no two encoder locks are ever held at once. + // them. This one's `moq_encode_video_bitrate` moves its reservation after moving its + // ceiling, which wakes this poll; the lower one's ceiling wakes it directly, wherever + // that rendition reserves. The lower one is read first, so no two encoder locks are + // ever held at once. let next = kio::wait(|waiter| { - let below = lower_ceiling(&lower); + let below = lower_ceiling(&lower, waiter); let guard = inner.lock(); let Some(producer) = guard.as_ref() else { return std::task::Poll::Ready(None); }; - follower.set_policy(video_policy(producer.ceiling, below)); + follower.set_policy(video_policy(*producer.ceiling.read(), below)); follower.poll_next(waiter).map(Some) }) .await; let Some(Ok(target)) = next else { return; }; - let below = lower_ceiling(&lower); + let below = lower_ceiling(&lower, &noop); let mut guard = inner.lock(); let Some(producer) = guard.as_mut() else { return; }; - let policy = video_policy(producer.ceiling, below); + let policy = video_policy(*producer.ceiling.read(), below); if retune(&mut follower, policy, target, |target| producer.apply(target)).is_err() { return; } @@ -360,8 +365,9 @@ pub(crate) struct VideoEncoder { config: moq_video::encode::Config, /// The track priority this rendition was published at; a higher rung takes one below. priority: i8, - /// The next lower rendition, set when this one is a simulcast rung. - lower: Option, + /// The next lower rendition's ceiling, set when this one is a simulcast rung. A + /// consumer, so a rung never keeps the rendition below it publishing. + lower: Option>, format: moq_video_pixel_format, /// The encoded resolution, from the publish config. Frames carry only pixels, /// so this is what says how to read them. @@ -371,8 +377,9 @@ pub(crate) struct VideoEncoder { followed: Option, /// The manual ceiling [`moq_encode_video_bitrate`] last applied, which caps every /// grant. Behind the encoder's lock, so the follower never reads an old, higher - /// one and retunes over a newer cap. - ceiling: moq_net::bandwidth::Rate, + /// one and retunes over a newer cap. A channel, so a rung above this rendition + /// wakes when it moves and when this rendition ends. + pub(crate) ceiling: kio::Producer, finished: crate::publish::Finished, } @@ -511,7 +518,11 @@ impl VideoEncoder { if let Some(encoder) = self.encoder.as_mut() { block_on(encoder.set_bitrate(bitrate))?; } - self.ceiling = bitrate; + let Ok(mut ceiling) = self.ceiling.write() else { + unreachable!("only dropping the encoder closes its ceiling") + }; + *ceiling = bitrate; + drop(ceiling); if let Some(reservation) = &self.reservation { reservation.update(bitrate); } @@ -544,10 +555,11 @@ impl VideoEncoder { return block_on(encoder.set_bitrate(bitrate)); } + let ceiling = *self.ceiling.read(); let mut config = self.config.clone(); - config.bitrate = Some(self.ceiling); + config.bitrate = Some(ceiling); let mut encoder = block_on(moq_video::encode::Sink::open(&config))?; - let retuned = match bitrate < self.ceiling { + let retuned = match bitrate < ceiling { true => block_on(encoder.set_bitrate(bitrate)), false => Ok(()), }; @@ -567,8 +579,11 @@ impl VideoEncoder { mut producer, followed, finished, + ceiling, .. } = self; + // Ended before the drain, so a rung above stops counting on this rendition now. + drop(ceiling); // Drain the codec into the track before ending it, so the last frames land // in it rather than being dropped with the encoder. let drained = match encoder { @@ -610,16 +625,15 @@ impl Video { let lower = self.producer(lower)?; let guard = lower.lock(); let below = guard.as_ref().ok_or(Error::MediaNotFound)?; - if below.ceiling >= ceiling { + let below_ceiling = *below.ceiling.read(); + if below_ceiling >= ceiling { return Err(Error::InvalidConfig(format!( "the lower rendition's {} bps is not below this one's {} bps", - below.ceiling.as_bps(), + below_ceiling.as_bps(), ceiling.as_bps() ))); } - let priority = below.priority.saturating_sub(1); - drop(guard); - (Some(lower.downgrade()), priority) + (Some(below.ceiling.consume()), below.priority.saturating_sub(1)) } None => (None, hang::catalog::PRIORITY.video), }; @@ -648,7 +662,7 @@ impl Video { reservation: None, follow: None, followed: None, - ceiling, + ceiling: kio::Producer::new(ceiling), finished, })) } @@ -659,9 +673,10 @@ impl Video { let (reservation, ceiling, lower) = { let mut guard = shared.lock(); let encoder = guard.as_mut().ok_or(Error::MediaNotFound)?; - let reservation = Arc::new(allocator.reserve(&encoder.producer.demand(), encoder.ceiling)); + let ceiling = *encoder.ceiling.read(); + let reservation = Arc::new(allocator.reserve(&encoder.producer.demand(), ceiling)); encoder.reservation = Some(reservation.clone()); - (reservation, encoder.ceiling, encoder.lower.clone()) + (reservation, ceiling, encoder.lower.clone()) }; let (close, closed) = oneshot::channel(); shared.lock().as_mut().ok_or(Error::MediaNotFound)?.follow = Some(close); @@ -1265,7 +1280,7 @@ mod tests { reservation: None, follow: None, followed: Some(followed), - ceiling: moq_net::bandwidth::Rate::from_mbps(1), + ceiling: kio::Producer::new(moq_net::bandwidth::Rate::from_mbps(1)), finished: Default::default(), }; diff --git a/rs/moq-ffi/src/video.rs b/rs/moq-ffi/src/video.rs index 6fe5ec816a2..8e16cdb4520 100644 --- a/rs/moq-ffi/src/video.rs +++ b/rs/moq-ffi/src/video.rs @@ -363,8 +363,9 @@ struct VideoProducer { size: moq_video::Size, /// The manual ceiling [`MoqVideoProducer::set_bitrate`] last applied, which caps /// every grant. Behind the encoder's lock, so the follower never reads an old, - /// higher one and retunes over a newer cap. - ceiling: moq_net::bandwidth::Rate, + /// higher one and retunes over a newer cap. A channel, so a rung above this + /// rendition wakes when it moves and when this rendition ends. + ceiling: kio::Producer, /// The track priority this rendition was published at; a higher rung takes one below. priority: i8, } @@ -453,10 +454,11 @@ impl VideoProducer { return block_on(encoder.set_bitrate(bitrate)); } + let ceiling = *self.ceiling.read(); let mut config = self.config.clone(); - config.bitrate = Some(self.ceiling); + config.bitrate = Some(ceiling); let mut encoder = block_on(moq_video::encode::Sink::open(&config))?; - let retuned = match bitrate < self.ceiling { + let retuned = match bitrate < ceiling { true => block_on(encoder.set_bitrate(bitrate)), false => Ok(()), }; @@ -472,8 +474,13 @@ impl VideoProducer { fn finish(self, finished: &crate::producer::Finished) -> Result<(), MoqError> { let Self { - encoder, mut producer, .. + encoder, + mut producer, + ceiling, + .. } = self; + // Ended before the drain, so a rung above stops counting on this rendition now. + drop(ceiling); // Drain the codec into the track before ending it, so the last frames land // in it rather than being dropped with the encoder. let drained = match encoder { @@ -599,7 +606,10 @@ impl MoqVideoProducer { block_on(encoder.set_bitrate(bitrate))?; } // Under the same lock as the retune, so the follower sees both or neither. - producer.ceiling = bitrate; + let Ok(mut ceiling) = producer.ceiling.write() else { + unreachable!("only dropping the producer closes its ceiling") + }; + *ceiling = bitrate; } if let Some(reservation) = self.reservation.lock().unwrap().as_ref() { reservation.update(bitrate); @@ -685,8 +695,8 @@ impl MoqVideoProducer { let guard = lower.inner.lock().map_err(|_| panic_error())?; let below = guard.as_ref().ok_or(MoqError::Closed)?; Some(( - Arc::downgrade(&lower.inner), - below.ceiling, + below.ceiling.consume(), + *below.ceiling.read(), below.priority.saturating_sub(1), )) } @@ -745,7 +755,7 @@ impl MoqVideoProducer { format: input.format, size: config.size(), config, - ceiling, + ceiling: kio::Producer::new(ceiling), priority, }))); let applied = Arc::new(AtomicU64::new(ceiling.as_bps())); @@ -801,9 +811,9 @@ fn video_ceiling( /// What the follow thread reads and retunes. struct Followed { inner: Arc>>, - /// The next lower rendition, whose ceiling sets this one's band. Weak, so a rung never - /// keeps the rendition below it publishing. - lower: Option>>>, + /// The next lower rendition's ceiling, which sets this one's band. A consumer, so a rung + /// never keeps the rendition below it publishing. + lower: Option>, consumer: moq_net::bandwidth::Consumer, ceiling: moq_net::bandwidth::Rate, applied: Arc, @@ -852,13 +862,18 @@ fn video_policy(ceiling: moq_net::bandwidth::Rate, lower: Option>>>, + lower: &Option>, + waiter: &kio::Waiter, ) -> Option { - let lower = lower.as_ref()?.upgrade()?; - let guard = lower.lock().ok()?; - guard.as_ref().map(|producer| producer.ceiling) + let lower = lower.as_ref()?; + match lower.poll(waiter, |_| std::task::Poll::<()>::Pending) { + std::task::Poll::Pending => Some(*lower.read()), + // The closure never completes, so this is the channel closing. + std::task::Poll::Ready(_) => None, + } } /// Feed each grant through the same policy moq-video uses, and retune the live @@ -872,31 +887,33 @@ async fn follow_reservation(followed: Followed) { ceiling, applied, } = followed; - let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower))); + let noop = kio::Waiter::noop(); + let mut follower = moq_mux::rate::Follower::new(consumer, video_policy(ceiling, lower_ceiling(&lower, &noop))); loop { // Both ceilings are read on every wake, before the follower filters a grant against - // them: `set_bitrate` (here or on the lower rendition) moves a reservation after - // moving its ceiling, which wakes this poll. The lower one is read first, so no - // two encoder locks are ever held at once. + // them. This one's `set_bitrate` moves its reservation after moving its ceiling, + // which wakes this poll; the lower one's ceiling wakes it directly, wherever that + // rendition reserves. The lower one is read first, so no two encoder locks are + // ever held at once. let next = kio::wait(|waiter| { - let below = lower_ceiling(&lower); + let below = lower_ceiling(&lower, waiter); let guard = inner.lock().unwrap(); let Some(producer) = guard.as_ref() else { return std::task::Poll::Ready(None); }; - follower.set_policy(video_policy(producer.ceiling, below)); + follower.set_policy(video_policy(*producer.ceiling.read(), below)); follower.poll_next(waiter).map(Some) }) .await; let Some(Ok(target)) = next else { return; }; - let below = lower_ceiling(&lower); + let below = lower_ceiling(&lower, &noop); let mut guard = inner.lock().unwrap(); let Some(producer) = guard.as_mut() else { return; }; - let policy = video_policy(producer.ceiling, below); + let policy = video_policy(*producer.ceiling.read(), below); match retune(&mut follower, policy, target, |target| producer.apply(target)) { Ok(Some(moq_mux::rate::Target::Enabled(bitrate))) => applied.store(bitrate.as_bps(), Ordering::SeqCst), Ok(Some(moq_mux::rate::Target::Disabled)) => applied.store(0, Ordering::SeqCst), @@ -1635,6 +1652,45 @@ mod rung_tests { assert!(matches!(ungranted, Err(MoqError::Video { .. }))); } + /// A rung's follower wakes on its lower rendition's own ceiling, whether that moves or + /// ends, so it notices even when the lower rendition reserves nothing it shares. + #[tokio::test] + async fn a_rung_wakes_when_its_unreserved_lower_moves_or_ends() { + struct Flag(AtomicBool); + impl std::task::Wake for Flag { + fn wake(self: Arc) { + self.0.store(true, Ordering::SeqCst); + } + } + + let broadcast = crate::producer::MoqBroadcastProducer::new().unwrap(); + let catalog = crate::media::MoqMediaCatalogProducer::new(broadcast.clone()).unwrap(); + let low = MoqVideoProducer::new( + &catalog, + MoqVideoProducerOptions { + input: input(), + output: output(500_000), + track: Some("low".into()), + bandwidth: None, + lower: None, + }, + ) + .unwrap(); + let lower = Some(low.inner.lock().unwrap().as_ref().unwrap().ceiling.consume()); + + let flag = Arc::new(Flag(AtomicBool::new(false))); + let waiter = kio::Waiter::new(std::task::Waker::from(flag.clone())); + assert_eq!(lower_ceiling(&lower, &waiter), Some(Rate::from_kbps(500))); + + low.set_bitrate(400_000).unwrap(); + assert!(flag.0.swap(false, Ordering::SeqCst), "a moved ceiling wakes the rung"); + assert_eq!(lower_ceiling(&lower, &waiter), Some(Rate::from_kbps(400))); + + low.finish().unwrap(); + assert!(flag.0.load(Ordering::SeqCst), "an ended lower rendition wakes the rung"); + assert_eq!(lower_ceiling(&lower, &waiter), None); + } + /// A disabled rung advertises itself disabled, drops the frames written to it, and /// resumes on a keyframe in a new group, its advertised maximum untouched throughout. #[tokio::test] From def2ee5d0046a4a0076deaaafaf54152996df72d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 25 Oct 2026 20:50:49 -0700 Subject: [PATCH 15/15] docs(lib): a simulcast rung only disables while the session probes Rate control holds a rung enabled unless a session probes for it, since nothing else could raise the estimate to resume it. Co-Authored-By: Claude Opus 5.5 --- doc/lib/c/index.md | 2 +- doc/lib/cpp/index.md | 2 +- doc/lib/dart/index.md | 2 +- doc/lib/go/index.md | 2 +- doc/lib/kt/index.md | 2 +- doc/lib/py/index.md | 2 +- doc/lib/swift/index.md | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/lib/c/index.md b/doc/lib/c/index.md index 66ecf84a6fc..04b424c9cc5 100644 --- a/doc/lib/c/index.md +++ b/doc/lib/c/index.md @@ -51,7 +51,7 @@ if (session < 0) return fail(moq_error()); ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. For simulcast, give each rendition the next one down as the `lower` handle in its `moq_video_encode_options`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Publication track names stay separate from codec settings. For simulcast, give each rendition the next one down as the `lower` handle in its `moq_video_encode_options`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know diff --git a/doc/lib/cpp/index.md b/doc/lib/cpp/index.md index 882798c9112..cd4b60dca6d 100644 --- a/doc/lib/cpp/index.md +++ b/doc/lib/cpp/index.md @@ -134,7 +134,7 @@ ok(broadcast->announce(route)); ok(broadcast->close()); // keep the producer alive while publishing, then close explicitly ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Futures diff --git a/doc/lib/dart/index.md b/doc/lib/dart/index.md index 5651f4d2fb6..3660f017a27 100644 --- a/doc/lib/dart/index.md +++ b/doc/lib/dart/index.md @@ -129,7 +129,7 @@ mine.close(); await moq.close(); ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know diff --git a/doc/lib/go/index.md b/doc/lib/go/index.md index d58828be3a6..ebb219f9a9c 100644 --- a/doc/lib/go/index.md +++ b/doc/lib/go/index.md @@ -117,7 +117,7 @@ _ = video.Finish() broadcast.Close() // keep the producer reachable while publishing, then close explicitly ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `Lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `Lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know diff --git a/doc/lib/kt/index.md b/doc/lib/kt/index.md index 5484031ec61..923da4c7392 100644 --- a/doc/lib/kt/index.md +++ b/doc/lib/kt/index.md @@ -89,7 +89,7 @@ Moq.connect("https://relay.example.com").use { moq -> } ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Producer options hold input, codec settings, track naming, and optional session bandwidth ownership; standalone encoders take only codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know diff --git a/doc/lib/py/index.md b/doc/lib/py/index.md index f3ca109fce5..5f0b59942a0 100644 --- a/doc/lib/py/index.md +++ b/doc/lib/py/index.md @@ -103,7 +103,7 @@ async def main(): asyncio.run(main()) ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know diff --git a/doc/lib/swift/index.md b/doc/lib/swift/index.md index e3b8d5df513..47231dfcb7e 100644 --- a/doc/lib/swift/index.md +++ b/doc/lib/swift/index.md @@ -90,7 +90,7 @@ session.shutdown() try await session.closed() ``` -Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. +Raw video encoders take an exact numerator/denominator frame rate, such as 30000/1001. Pass the publication track name to the producer, alongside its input and codec settings. For simulcast, give each rendition the next one down as `lower`: it stops encoding, advertised as disabled, while its bandwidth grant cannot fund it, and resumes once the grant recovers. A rung only disables while the session probes for bandwidth, which on native transports takes moq-quic with BBR3. ## Things to know