From ce5257e78612035ea78604bb4a195ecdd304f465 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 14:47:43 -0700 Subject: [PATCH 1/3] fix(hls): an estimated framerate keeps the rendition Co-Authored-By: Claude Opus 5.5 --- rs/moq-hls/src/export/mod.rs | 30 +++++++++++++++++++++++++++++ rs/moq-hls/src/export/rendition.rs | 10 +++++----- rs/moq-hls/src/export/renditions.rs | 2 +- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/rs/moq-hls/src/export/mod.rs b/rs/moq-hls/src/export/mod.rs index 1d3cc5df92..b9887d452a 100644 --- a/rs/moq-hls/src/export/mod.rs +++ b/rs/moq-hls/src/export/mod.rs @@ -708,6 +708,36 @@ mod tests { assert_eq!(video.bandwidth(), 2_000_000); } + // The estimator also measures the framerate and publishes it once it settles, so an + // imported source gains one about a second in. Rebuilding for it would re-time the fMP4 + // (its timescale derives from the framerate) and churn every recording cursor mid-stream. + #[tokio::test] + async fn estimated_framerate_keeps_the_rendition() { + let origin = produce_origin(); + let _broadcast = origin.create_broadcast("live").expect("publish allowed"); + _broadcast.announce(Default::default()).expect("publish allowed"); + settle().await; + let upstream = empty_upstream(&origin, "live").await; + + let mut catalog = catalog_with_both(); + catalog.video.renditions.get_mut("video0").unwrap().framerate = None; + let renditions = renditions::Producer::new(Config::default().window); + let mut cursor = renditions.subscribe(); + renditions.sync(&upstream, &catalog); + let video = renditions.get(Kind::Video, "video0").expect("video rendition"); + while next_event(&mut cursor).await.is_some() {} + + for framerate in [Some(30.0), Some(60.0)] { + catalog.video.renditions.get_mut("video0").unwrap().framerate = framerate; + renditions.sync(&upstream, &catalog); + assert!( + Arc::ptr_eq(&video, &renditions.get(Kind::Video, "video0").unwrap()), + "the video rendition survives a framerate estimate of {framerate:?}" + ); + assert!(next_event(&mut cursor).await.is_none(), "no rendition churn"); + } + } + #[tokio::test] async fn estimate_before_first_init_reaches_aac_descriptor() { let origin = produce_origin(); diff --git a/rs/moq-hls/src/export/rendition.rs b/rs/moq-hls/src/export/rendition.rs index 7d30ee5f32..75c9905ba8 100644 --- a/rs/moq-hls/src/export/rendition.rs +++ b/rs/moq-hls/src/export/rendition.rs @@ -192,9 +192,9 @@ fn normalize_video(config: &VideoConfig) -> VideoConfig { config.delay = None; config.label = None; config.stalled = None; - // The muxer ignores a non-finite framerate, and NaN never equals itself, so a catalog - // carrying one would otherwise look different from itself on every republish. - config.framerate = config.framerate.filter(|fps| fps.is_finite()); + // Publishers estimate it and republish once it settles. The rendition keeps the framerate, + // and the fMP4 timescale derived from it, that it was built with: the media is unchanged. + config.framerate = None; config } @@ -250,8 +250,8 @@ pub struct Rendition { impl Rendition { /// Whether `config` describes the same media this rendition is already serving, i.e. whether /// it decodes and muxes identically. Fields the publisher revises without touching the media - /// (its bitrate and jitter estimates, a label) are ignored here and picked up by - /// [`refresh`](Self::refresh) instead. + /// (its bitrate, jitter, and framerate estimates, a label) are ignored here, and the bitrate + /// is picked up by [`refresh`](Self::refresh) instead. pub(crate) fn matches_video(&self, config: &VideoConfig) -> bool { let Config::Video(current) = &self.config else { return false; diff --git a/rs/moq-hls/src/export/renditions.rs b/rs/moq-hls/src/export/renditions.rs index 63815fed7c..ab1037ab6c 100644 --- a/rs/moq-hls/src/export/renditions.rs +++ b/rs/moq-hls/src/export/renditions.rs @@ -352,7 +352,7 @@ impl Producer { /// "Reconfigured" means the media itself changed (see [`Rendition::matches_video`]). A /// rendition whose entry only carries a revised estimate is kept as-is and just takes the /// new advertised bitrate, since the publisher republishes the catalog every time its - /// measured bitrate or jitter moves. + /// measured bitrate, jitter, or framerate moves. /// /// Renditions are only servable when the catalog advertises the broadcast's timeline (its /// root `archive` entry): without one there is nothing to render playlists from, so the From 58c54b1191267569376f8cb9bee12cf1fd7df8c0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 17:13:19 -0700 Subject: [PATCH 2/3] fix(hls): refresh the fallback framerate and DASH frameRate in place Co-Authored-By: Claude Opus 5.5 --- rs/moq-hls/src/export/mod.rs | 20 ++++++++++ rs/moq-hls/src/export/rendition.rs | 31 +++++++++++----- rs/moq-hls/src/export/renditions.rs | 6 +-- rs/moq-mux/src/container/fmp4/muxer.rs | 51 ++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/rs/moq-hls/src/export/mod.rs b/rs/moq-hls/src/export/mod.rs index b9887d452a..67c1fd4e2c 100644 --- a/rs/moq-hls/src/export/mod.rs +++ b/rs/moq-hls/src/export/mod.rs @@ -736,6 +736,26 @@ mod tests { ); assert!(next_event(&mut cursor).await.is_none(), "no rendition churn"); } + + // The settled estimate still reaches the fallback cadence of a duration-less frame, + // at the timescale the rendition was built with (90 kHz without a framerate). + let muxer = video.muxer().unwrap(); + assert_eq!(muxer.timescale().as_u64(), 90_000); + let fragment = muxer.fragment(0, &[vp8_frame(0, true)]).unwrap(); + assert_eq!(first_sample_duration(&fragment), Some(1_500), "16.7 ms at 90 kHz"); + } + + /// The `trun` duration of a fragment's first sample. + fn first_sample_duration(fragment: &bytes::Bytes) -> Option { + use moq_mux::container::fmp4::mp4_atom::{self, DecodeMaybe}; + + let mut cursor = std::io::Cursor::new(fragment.as_ref()); + while let Some(atom) = mp4_atom::Any::decode_maybe(&mut cursor).unwrap() { + if let mp4_atom::Any::Moof(moof) = atom { + return moof.traf[0].trun[0].entries[0].duration; + } + } + panic!("no moof in fragment"); } #[tokio::test] diff --git a/rs/moq-hls/src/export/rendition.rs b/rs/moq-hls/src/export/rendition.rs index 75c9905ba8..af468b35cc 100644 --- a/rs/moq-hls/src/export/rendition.rs +++ b/rs/moq-hls/src/export/rendition.rs @@ -192,8 +192,8 @@ fn normalize_video(config: &VideoConfig) -> VideoConfig { config.delay = None; config.label = None; config.stalled = None; - // Publishers estimate it and republish once it settles. The rendition keeps the framerate, - // and the fMP4 timescale derived from it, that it was built with: the media is unchanged. + // Publishers estimate it and republish once it settles. The rendition keeps the fMP4 + // timescale it was built with, and the framerate is refreshed in place like the bitrate. config.framerate = None; config } @@ -218,6 +218,9 @@ pub struct Rendition { /// Advertised bitrate for the master playlist `BANDWIDTH` attribute; read through /// [`bandwidth`](Self::bandwidth), refreshed in place by [`refresh`](Self::refresh). bitrate: RwLock>, + /// The latest catalog framerate (video only), refreshed in place by [`refresh`](Self::refresh): + /// the muxer's fallback sample cadence and the DASH `frameRate`. + framerate: RwLock>, /// Coded width, for the master playlist `RESOLUTION` (video only). pub width: Option, /// Coded height, for the master playlist `RESOLUTION` (video only). @@ -251,7 +254,7 @@ impl Rendition { /// Whether `config` describes the same media this rendition is already serving, i.e. whether /// it decodes and muxes identically. Fields the publisher revises without touching the media /// (its bitrate, jitter, and framerate estimates, a label) are ignored here, and the bitrate - /// is picked up by [`refresh`](Self::refresh) instead. + /// and framerate are picked up by [`refresh`](Self::refresh) instead. pub(crate) fn matches_video(&self, config: &VideoConfig) -> bool { let Config::Video(current) = &self.config else { return false; @@ -273,14 +276,15 @@ impl Rendition { self.bitrate().unwrap_or(default_bandwidth(self.kind)) } - /// Take the advertised bitrate from a catalog update that - /// [`matches`](Self::matches_video) this rendition. + /// Take the advertised bitrate and framerate from a catalog update that + /// [`matches`](Self::matches_video) this rendition (audio passes no framerate). /// /// The publisher's estimator republishes the catalog whenever its measured bitrate moves, so /// this is the common update by far: rebuilding the rendition for it would reset the playlist /// window, the cached init segment, and `EXT-X-MEDIA-SEQUENCE` several times a minute. - pub(crate) fn refresh(&self, bitrate: Option) { + pub(crate) fn refresh(&self, bitrate: Option, framerate: Option) { *self.bitrate.write().expect("bitrate lock poisoned") = bitrate; + *self.framerate.write().expect("framerate lock poisoned") = framerate; } /// The latest catalog bitrate, preserving `None` for muxer-specific fallback behavior. @@ -288,6 +292,11 @@ impl Rendition { *self.bitrate.read().expect("bitrate lock poisoned") } + /// The latest catalog framerate. + fn framerate(&self) -> Option { + *self.framerate.read().expect("framerate lock poisoned") + } + /// Build a video rendition over the broadcast's timeline `section`, timed by the /// catalog root `clock`. /// @@ -304,6 +313,7 @@ impl Rendition { name, kind: Kind::Video, bitrate: RwLock::new(config.bitrate), + framerate: RwLock::new(config.framerate), width: config.coded_width, height: config.coded_height, codec: config.codec.to_string(), @@ -330,6 +340,7 @@ impl Rendition { name, kind: Kind::Audio, bitrate: RwLock::new(config.bitrate), + framerate: RwLock::new(None), width: None, height: None, codec: config.codec.to_string(), @@ -565,7 +576,7 @@ impl Rendition { }) .collect(); let (framerate, sample_rate, channel_count) = match &self.config { - Config::Video(config) => (config.framerate, None, None), + Config::Video(_) => (self.framerate(), None, None), Config::Audio(config) => (None, Some(config.sample_rate), Some(config.channel_count)), }; Some(mpd::Representation { @@ -611,13 +622,15 @@ impl Rendition { clock.wall_clock(pts).ok() } - fn muxer(&self) -> Result { + /// A muxer at the timescale this rendition's config chose, with the latest bitrate and + /// fallback framerate. + pub(crate) fn muxer(&self) -> Result { let bitrate = self.bitrate(); Ok(match &self.config { Config::Video(config) => { let mut config = config.clone(); config.bitrate = bitrate; - Muxer::video(&config)? + Muxer::video(&config)?.with_framerate(self.framerate()) } Config::Audio(config) => { let mut config = config.clone(); diff --git a/rs/moq-hls/src/export/renditions.rs b/rs/moq-hls/src/export/renditions.rs index ab1037ab6c..757b9657df 100644 --- a/rs/moq-hls/src/export/renditions.rs +++ b/rs/moq-hls/src/export/renditions.rs @@ -405,8 +405,8 @@ impl Producer { let key = (Kind::Video, name.clone()); if let Some(rendition) = current.get(&key) { // Survived the stale pass, so it decodes the same: keep its window, its cached - // init segment and its media sequence, and just take the new advertised bitrate. - rendition.refresh(video.bitrate); + // init segment and its media sequence, and just take the new bitrate and framerate. + rendition.refresh(video.bitrate, video.framerate); continue; } let rendition = match Rendition::video(name.clone(), video, upstream, section.clone(), clock) { @@ -422,7 +422,7 @@ impl Producer { for (name, audio) in &catalog.audio.renditions { let key = (Kind::Audio, name.clone()); if let Some(rendition) = current.get(&key) { - rendition.refresh(audio.bitrate); + rendition.refresh(audio.bitrate, None); continue; } let rendition = match Rendition::audio(name.clone(), audio, upstream, section.clone(), clock) { diff --git a/rs/moq-mux/src/container/fmp4/muxer.rs b/rs/moq-mux/src/container/fmp4/muxer.rs index 373e260236..0f9d1d698c 100644 --- a/rs/moq-mux/src/container/fmp4/muxer.rs +++ b/rs/moq-mux/src/container/fmp4/muxer.rs @@ -125,6 +125,22 @@ impl Muxer { Ok(self) } + /// Synthesize missing video sample durations at `framerate`, a newer estimate than the + /// config this muxer was built from. + /// + /// Only the fallback cadence follows it: the timescale, and so the init segment, stays the + /// one the original config chose. Ignored for audio, for a missing, non-finite or + /// non-positive rate, and for one whose frame doesn't fit `trun`'s 32-bit sample duration. + pub fn with_framerate(mut self, framerate: Option) -> Self { + if matches!(self.kind, Kind::Video(_)) + && let Some(fps) = framerate.filter(|fps| fps.is_finite() && *fps > 0.0) + && self.timescale.as_u64() as f64 / fps <= u32::MAX as f64 + { + self.default_frame = Duration::from_secs_f64(1.0 / fps); + } + self + } + /// The rendition's catalog container, whichever kind of track this is. fn catalog_container(&self) -> &CatalogContainer { match &self.kind { @@ -582,6 +598,41 @@ mod tests { assert_eq!(decoded[0].timestamp.as_micros(), 33_333); } + // A rendition built before the publisher's framerate estimate settled keeps its 90 kHz + // timescale, but a duration-less frame follows the newer cadence. + #[test] + fn with_framerate_updates_the_fallback_without_the_timescale() { + let muxer = video_muxer_without_framerate().with_framerate(Some(60.0)); + assert_eq!(muxer.timescale().as_u64(), 90_000); + let fragment = muxer.fragment(0, &[frame(0, true)]).unwrap(); + assert_eq!(super::super::sample_durations(&fragment), vec![Some(1_500)]); + } + + #[test] + fn with_framerate_ignores_an_unusable_rate() { + // 90 kHz at 1e-6 fps is 9e10 ticks per frame, past trun's 32 bits. + for fps in [ + None, + Some(0.0), + Some(-30.0), + Some(f64::NAN), + Some(f64::INFINITY), + Some(1e-6), + ] { + let muxer = video_muxer_without_framerate().with_framerate(fps); + let fragment = muxer.fragment(0, &[frame(0, true)]).unwrap(); + // The 30 fps fallback at 90 kHz. + assert_eq!(super::super::sample_durations(&fragment), vec![Some(3_000)], "{fps:?}"); + } + } + + fn video_muxer_without_framerate() -> Muxer { + let mut config = VideoConfig::new(VideoCodec::VP8); + config.coded_width = Some(320); + config.coded_height = Some(240); + Muxer::video(&config).unwrap() + } + #[test] fn with_timescale_recomputes_the_fallback_frame_duration() { let timescale = moq_net::Timescale::new(90_000).unwrap(); From 3341eb7b903adf2dd4ab083fbc0b7a25e18906f6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 30 Sep 2026 17:32:08 -0700 Subject: [PATCH 3/3] fix(mux): snap a refreshed fallback cadence to whole ticks Co-Authored-By: Claude Opus 5.5 --- rs/moq-hls/src/export/mod.rs | 6 ++++ rs/moq-mux/src/container/fmp4/muxer.rs | 50 +++++++++++++++++++++----- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/rs/moq-hls/src/export/mod.rs b/rs/moq-hls/src/export/mod.rs index 67c1fd4e2c..058eca36c6 100644 --- a/rs/moq-hls/src/export/mod.rs +++ b/rs/moq-hls/src/export/mod.rs @@ -743,6 +743,12 @@ mod tests { assert_eq!(muxer.timescale().as_u64(), 90_000); let fragment = muxer.fragment(0, &[vp8_frame(0, true)]).unwrap(); assert_eq!(first_sample_duration(&fragment), Some(1_500), "16.7 ms at 90 kHz"); + + // An NTSC estimate is 1501.5 ticks at 90 kHz: it snaps to whole ticks and still muxes. + catalog.video.renditions.get_mut("video0").unwrap().framerate = Some(60_000.0 / 1001.0); + renditions.sync(&upstream, &catalog); + let fragment = video.muxer().unwrap().fragment(0, &[vp8_frame(0, true)]).unwrap(); + assert_eq!(first_sample_duration(&fragment), Some(1_502)); } /// The `trun` duration of a fragment's first sample. diff --git a/rs/moq-mux/src/container/fmp4/muxer.rs b/rs/moq-mux/src/container/fmp4/muxer.rs index 0f9d1d698c..c220b7ddb5 100644 --- a/rs/moq-mux/src/container/fmp4/muxer.rs +++ b/rs/moq-mux/src/container/fmp4/muxer.rs @@ -129,14 +129,28 @@ impl Muxer { /// config this muxer was built from. /// /// Only the fallback cadence follows it: the timescale, and so the init segment, stays the - /// one the original config chose. Ignored for audio, for a missing, non-finite or - /// non-positive rate, and for one whose frame doesn't fit `trun`'s 32-bit sample duration. + /// one the original config chose. The cadence snaps to whole ticks at that timescale (an NTSC + /// rate is fractional ticks at 90 kHz, which would otherwise fail to mux as inexact). Ignored + /// for audio, for a missing, non-finite or non-positive rate, and for one whose frame rounds + /// to zero ticks or past `trun`'s 32-bit sample duration. pub fn with_framerate(mut self, framerate: Option) -> Self { - if matches!(self.kind, Kind::Video(_)) - && let Some(fps) = framerate.filter(|fps| fps.is_finite() && *fps > 0.0) - && self.timescale.as_u64() as f64 / fps <= u32::MAX as f64 - { - self.default_frame = Duration::from_secs_f64(1.0 / fps); + if !matches!(self.kind, Kind::Video(_)) { + return self; + } + let Some(fps) = framerate.filter(|fps| fps.is_finite() && *fps > 0.0) else { + return self; + }; + let timescale = self.timescale.as_u64(); + let ticks = (timescale as f64 / fps).round(); + if !(1.0..=f64::from(u32::MAX)).contains(&ticks) { + return self; + } + // Exactly `ticks / timescale`, rounded to the nanosecond, so the fallback snaps back to + // `ticks` within the muxer's one-nanosecond tolerance. + let timescale = u128::from(timescale); + let nanos = (ticks as u128 * 1_000_000_000 + timescale / 2) / timescale; + if let Ok(nanos) = u64::try_from(nanos) { + self.default_frame = Duration::from_nanos(nanos); } self } @@ -608,9 +622,28 @@ mod tests { assert_eq!(super::super::sample_durations(&fragment), vec![Some(1_500)]); } + // An NTSC rate is fractional ticks at a timescale that doesn't divide it (1501.5 at 90 kHz), + // so the cadence snaps to whole ticks rather than failing the fragment as inexact. + #[test] + fn with_framerate_snaps_a_fractional_cadence_to_whole_ticks() { + for (fps, ticks) in [(60_000.0 / 1001.0, 1_502), (24_000.0 / 1001.0, 3_754)] { + let muxer = video_muxer_without_framerate().with_framerate(Some(fps)); + let fragment = muxer.fragment(0, &[frame(0, true)]).unwrap(); + assert_eq!(super::super::sample_durations(&fragment), vec![Some(ticks)], "{fps}"); + } + + // 30 kHz doesn't divide a second's nanoseconds either: 1251.25 ticks rounds to 1251. + let mut config = VideoConfig::new(VideoCodec::VP8); + config.framerate = Some(30.0); + let muxer = Muxer::video(&config).unwrap().with_framerate(Some(24_000.0 / 1001.0)); + assert_eq!(muxer.timescale().as_u64(), 30_000); + let fragment = muxer.fragment(0, &[frame(0, true)]).unwrap(); + assert_eq!(super::super::sample_durations(&fragment), vec![Some(1_251)]); + } + #[test] fn with_framerate_ignores_an_unusable_rate() { - // 90 kHz at 1e-6 fps is 9e10 ticks per frame, past trun's 32 bits. + // 90 kHz at 1e-6 fps is 9e10 ticks per frame, past trun's 32 bits; at 1e9 fps it is 0. for fps in [ None, Some(0.0), @@ -618,6 +651,7 @@ mod tests { Some(f64::NAN), Some(f64::INFINITY), Some(1e-6), + Some(1e9), ] { let muxer = video_muxer_without_framerate().with_framerate(fps); let fragment = muxer.fragment(0, &[frame(0, true)]).unwrap();