From 2674aea0f14c96d6d88673834c3eb6aff2573479 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 07:44:09 -0700 Subject: [PATCH 1/7] quest: claim ts-video-content Co-Authored-By: Claude Opus 5.5 From 8f59fbbc7f5f5d308f3788d9cbac0cfeebd46bc1 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 08:23:54 -0700 Subject: [PATCH 2/7] fix(ts): start an export on a whole group and count every drop A live-edge export subscribes with a zero budget, so the publisher can reset the group it joined at (`Old`) before the raised budget reaches it, and the export emitted that group's cut-off start ahead of a hole (#5565). The jitter buffer now holds a track's start group whole: until a frame of the track has gone out, a loss (a source gap, or a frame it drops) takes everything the track holds, and it starts at its next sync frame. The TS export's tune-in follows the video's start when it moves. Drops are counted by cause: `dropped` (after a track's output began, including a new generation's acquisition, previously uncounted), `trimmed` (before it), and `gaps` (media skipped upstream, each logged with its span). The TS compliance harness prints them beside a failing decoded-content check. Co-Authored-By: Claude Opus 5.5 --- rs/moq-cli/src/subscribe.rs | 11 +- rs/moq-mux/src/container/consumer.rs | 5 + rs/moq-mux/src/container/source.rs | 9 +- rs/moq-mux/src/container/ts/export.rs | 82 +++++++++++-- rs/moq-mux/src/container/ts/export_test.rs | 82 ++++++++++++- rs/moq-mux/src/container/ts/stats.rs | 13 +- rs/moq-mux/src/jitter.rs | 132 +++++++++++++++++---- test/ts/README.md | 7 ++ test/ts/run.sh | 10 +- 9 files changed, 306 insertions(+), 45 deletions(-) diff --git a/rs/moq-cli/src/subscribe.rs b/rs/moq-cli/src/subscribe.rs index 9f9579d76f1..4d0fcd780ee 100644 --- a/rs/moq-cli/src/subscribe.rs +++ b/rs/moq-cli/src/subscribe.rs @@ -399,11 +399,13 @@ impl Subscribe { // the way `publish` reports one that stops arriving. let mut log = moq_mux::container::ts::stats::Log::default(); let mut sampled = tokio::time::Instant::now(); - // The jitter buffer's late drops and out-of-tolerance count, as last logged. - let mut reported = (0, 0); + // The export's drop, gap and out-of-tolerance counts, as last logged. + let mut reported = (0, 0, 0, 0); let release = |stats: &moq_mux::container::ts::stats::Export| { tracing::info!( dropped = stats.dropped, + trimmed = stats.trimmed, + gaps = stats.gaps, drift_ppm = ?stats.drift, out_of_tolerance = stats.out_of_tolerance, "TS export release clock" @@ -421,8 +423,9 @@ impl Subscribe { if sampled.elapsed() >= moq_mux::container::ts::stats::Log::INTERVAL { sampled = tokio::time::Instant::now(); let stats = ts.export().stats(); - if (stats.dropped, stats.out_of_tolerance) != reported { - reported = (stats.dropped, stats.out_of_tolerance); + let counts = (stats.dropped, stats.trimmed, stats.gaps, stats.out_of_tolerance); + if counts != reported { + reported = counts; release(&stats); } log.sample(stats.into()); diff --git a/rs/moq-mux/src/container/consumer.rs b/rs/moq-mux/src/container/consumer.rs index 243e5020b47..ed459e587eb 100644 --- a/rs/moq-mux/src/container/consumer.rs +++ b/rs/moq-mux/src/container/consumer.rs @@ -182,6 +182,11 @@ impl Consumer { self.group } + /// The track's name. + pub(crate) fn name(&self) -> &str { + self.track.name() + } + /// Read the next frame from the track. /// /// This method handles timestamp decoding, group ordering, and age management diff --git a/rs/moq-mux/src/container/source.rs b/rs/moq-mux/src/container/source.rs index dccea4b3bfc..dd1d5f9a06f 100644 --- a/rs/moq-mux/src/container/source.rs +++ b/rs/moq-mux/src/container/source.rs @@ -399,7 +399,14 @@ impl ExportSource { } frame } - Some(Item::Read(Read::Gap { .. })) => { + Some(Item::Read(Read::Gap { timestamp, duration })) => { + // The export never receives this media, so say what it loses and where. + tracing::warn!( + track = consumer.name(), + at = ?std::time::Duration::from(timestamp), + lost = ?std::time::Duration::from(duration), + "media skipped upstream; resuming at the next group" + ); self.finish_ends(); self.skips += 1; return Poll::Ready(Ok(Some(Event::GroupEnd))); diff --git a/rs/moq-mux/src/container/ts/export.rs b/rs/moq-mux/src/container/ts/export.rs index bbd4398298f..ef9a7165668 100644 --- a/rs/moq-mux/src/container/ts/export.rs +++ b/rs/moq-mux/src/container/ts/export.rs @@ -187,6 +187,12 @@ where /// up before it ever configures video. `None` until the tables are built, and for /// programs with no video track (nothing to align to). video_start: Option, + /// Whether a video frame has gone out since [`Self::video_start`] was set. Until one has, + /// what the other tracks release is dropped: a video track whose start group was cut + /// before it went out starts at its next keyframe instead ([`jitter::Buffer`]). + video_out: bool, + /// Frames dropped ahead of the first video keyframe, or of their codec config. + trimmed: u64, /// Each elementary stream's access units and silence on the PCR returned ([`Self::stats`]). liveness: super::import::Liveness, } @@ -272,6 +278,8 @@ struct Input<'a> { waiting_for_header: bool, video_start: Option, jitter: &'a mut jitter::Buffer, + /// Counts the frames dropped ahead of the tune-in point or their codec config. + trimmed: &'a mut u64, } impl Track { @@ -335,6 +343,7 @@ impl Track { match self.source.poll_read(waiter)? { Poll::Ready(Some(frame)) => { if input.waiting_for_header && !self.source.header_ready() { + *input.trimmed += 1; continue; } let pending = Pending { @@ -349,6 +358,7 @@ impl Track { && !is_video && pending.restart == self.restart && pending.frame.timestamp < start { + *input.trimmed += 1; continue; } if input.waiting_for_header { @@ -1046,6 +1056,8 @@ where queue: VecDeque::new(), liveness: Default::default(), video_start: None, + video_out: false, + trimmed: 0, mux_rate: None, mux_rate_override: None, } @@ -1200,6 +1212,7 @@ where && track.pending.as_ref().is_some_and(|p| p.frame.timestamp < start) { track.pending = None; + self.trimmed += 1; } } } @@ -1252,22 +1265,26 @@ where ready.generation >= self.generation, "the jitter buffer released an older generation after a newer one" ); - let name = self + let (name, is_video) = self .tracks .iter() .find(|(_, t)| t.pid == ready.track) - .map(|(name, _)| name.clone()) + .map(|(name, t)| (name.clone(), matches!(t.kind, Kind::Video(_)))) .context("frame for an unknown PID")?; - self.last_timestamp = Some(ready.item.frame.timestamp); - let decode = ready.item.decode.as_nanos(); - self.lay_before(ready.item.key.as_nanos())?; - self.mux(&name, ready.item)?; - if self.delay.is_zero() { - // Without a clock to lay slots on, a frame settles every slot before its own: a - // later frame decodes no earlier, so it can be due no earlier than this, less the - // longest drain of any track. - let drain = self.max_drain(); - self.lay(Some(schedule::slot(decode.saturating_sub(drain.as_nanos()))))?; + if self.leads_video(is_video, ready.item.frame.timestamp) { + self.trimmed += 1; + } else { + self.last_timestamp = Some(ready.item.frame.timestamp); + let decode = ready.item.decode.as_nanos(); + self.lay_before(ready.item.key.as_nanos())?; + self.mux(&name, ready.item)?; + if self.delay.is_zero() { + // Without a clock to lay slots on, a frame settles every slot before its own: a + // later frame decodes no earlier, so it can be due no earlier than this, less the + // longest drain of any track. + let drain = self.max_drain(); + self.lay(Some(schedule::slot(decode.saturating_sub(drain.as_nanos()))))?; + } } if self.queue.is_empty() { // Releasing a due frame opens room in its source. Read it before parking: @@ -1281,6 +1298,7 @@ where waiting_for_header: false, video_start: self.video_start, jitter: &mut self.jitter, + trimmed: &mut self.trimmed, }, )?; if !finished && track.finished { @@ -1395,6 +1413,7 @@ where waiting_for_header: self.psi.is_none(), video_start: self.video_start, jitter: &mut self.jitter, + trimmed: &mut self.trimmed, }; for (name, track) in &mut self.tracks { track.fill(name, waiter, &mut input)?; @@ -1666,6 +1685,15 @@ where pub fn stats(&self) -> super::stats::Export { let mut stats = super::stats::Export { dropped: self.jitter.dropped(), + trimmed: self.jitter.trimmed() + self.trimmed, + gaps: self + .tracks + .values() + .map(|track| { + let skips = track.base.1 + track.source.skips(); + skips - (track.base.0 + track.source.markers()) + }) + .sum(), drift: self.jitter.drift().map(|drift| drift * 1e6), out_of_tolerance: self.jitter.out_of_tolerance(), ..Default::default() @@ -1801,6 +1829,36 @@ where Ok(()) } + /// Whether a frame released at `timestamp` presents ahead of the video keyframe the stream + /// begins on ([`Self::video_start`]), and so is dropped. A video track whose start group + /// was cut before it went out starts at its next keyframe instead ([`jitter::Buffer`]), so + /// until video goes out the start is the earliest frame a video track holds. Video goes out + /// further ahead of its decode time than the other tracks, so what they release before it + /// presents ahead of it too. + fn leads_video(&mut self, is_video: bool, timestamp: Timestamp) -> bool { + let Some(start) = self.video_start else { + return false; + }; + if is_video { + if !self.video_out { + self.video_out = true; + self.video_start = Some(start.max(timestamp)); + } + return false; + } + if self.video_out { + return timestamp < start; + } + let next = self + .tracks + .values() + .filter(|track| matches!(track.kind, Kind::Video(_))) + .filter_map(|track| self.jitter.peek(&track.pid)) + .map(|queued| queued.frame.timestamp) + .min(); + timestamp < next.map_or(start, |next| next.max(start)) + } + /// Discard what has not gone out and restart the program clock. Every rendition /// joins the new generation: no track is fenced across a declared marker, since a /// latency skip on one track says nothing about another's timeline. The continuity diff --git a/rs/moq-mux/src/container/ts/export_test.rs b/rs/moq-mux/src/container/ts/export_test.rs index e77cd97475e..1cc0b6eb1e5 100644 --- a/rs/moq-mux/src/container/ts/export_test.rs +++ b/rs/moq-mux/src/container/ts/export_test.rs @@ -1464,7 +1464,9 @@ async fn export_skips_an_unconfirmed_ac3_sync_word_at_a_join() { } /// A declared marker breaks the timeline even where the timestamps run on, so a frame left -/// unfinished before it is dropped rather than completed with the first bytes after it. +/// unfinished before it is dropped rather than completed with the first bytes after it. A +/// data track declares the break by skipping a sequence, which reads as media skipped, and +/// here it comes before anything of the track went out, so the track starts after it. #[tokio::test(start_paused = true)] async fn export_drops_an_ac3_frame_cut_by_a_marker() { let stream: Vec = (0..4).flat_map(ac3_frame).collect(); @@ -1475,7 +1477,7 @@ async fn export_drops_an_ac3_frame_cut_by_a_marker() { ]) .await; let frames: Vec<&Vec> = timed.iter().map(|(_, bytes)| bytes).collect(); - assert_eq!(frames, [&ac3_frame(0), &ac3_frame(2), &ac3_frame(3)]); + assert_eq!(frames, [&ac3_frame(2), &ac3_frame(3)]); } /// AC-3 sync frames that run on share a PES, as many as DVB's 5,696-byte B holds with the @@ -5472,6 +5474,82 @@ async fn tune_in_does_not_wait_on_dropped_audio() { assert!(!out.is_empty(), "the tune-in waited on audio it had dropped"); } +/// A join whose first group the publisher cuts short, as a zero-budget live-edge subscribe +/// is cut when the next group lands before the raised budget reaches it (moq-dev/moq#5565), +/// starts at the next group: nothing of the cut-off start goes out, the next keyframe leads +/// the audio, and the rest is intact. +#[tokio::test(start_paused = true)] +async fn a_start_group_cut_short_starts_at_the_next() { + use crate::container::Container as _; + + let delay = Duration::from_secs(1); + let mut rig = Interleave::new().await; + let mut export = rig.export(delay).await; + let track = rig.video.track().clone(); + let wire = HangContainer::Legacy(crate::container::Kind::Video); + let picture = |tick: u64| { + let keyframe = tick.is_multiple_of(GOP); + let slice = vec![if keyframe { 0x65u8 } else { 0x41 }; 400]; + Frame { + timestamp: Timestamp::from_micros(tick * VIDEO_US).unwrap(), + duration: None, + payload: length_prefixed(&[&slice]), + keyframe, + } + }; + let mut out = Vec::new(); + + // The first group delivers five pictures before the next group lands and the publisher + // resets it. + let mut cut = track.create_group(moq_net::group::Info { sequence: 0 }).unwrap(); + for tick in 0..5 { + rig.at(tick * VIDEO_US).await; + wire.write(&mut cut, &[picture(tick)]).unwrap(); + out.extend(poll_frames(&mut export)); + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + let mut group = track.create_group(moq_net::group::Info { sequence: 1 }).unwrap(); + wire.write(&mut group, &[picture(GOP)]).unwrap(); + cut.abort(moq_net::Error::Old).unwrap(); + for tick in 5..GOP { + rig.at(tick * VIDEO_US).await; + out.extend(poll_frames(&mut export)); + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + for tick in GOP + 1..4 * GOP { + rig.at(tick * VIDEO_US).await; + if tick.is_multiple_of(GOP) { + group.finish().unwrap(); + group = track + .create_group(moq_net::group::Info { sequence: tick / GOP }) + .unwrap(); + } + wire.write(&mut group, &[picture(tick)]).unwrap(); + out.extend(poll_frames(&mut export)); + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + group.finish().unwrap(); + rig.finish(); + out.extend(drain_frames(&mut export).await); + + let ts: Vec = out.iter().flat_map(|f| f.payload.iter().copied()).collect(); + let (video, audio) = collect_pes_pts(&ts); + let pts = |tick: u64| tick * VIDEO_US * 90 / 1_000; + assert_eq!( + video, + (GOP..4 * GOP).map(pts).collect::>(), + "the export starts at the next group, whole" + ); + assert!( + audio.iter().all(|&p| p >= pts(GOP)), + "no audio leads the keyframe the export starts at: {audio:?}" + ); + let stats = export.stats(); + assert_eq!(stats.dropped, 0, "nothing that went out lost a frame"); + assert_eq!(stats.gaps, 1, "the cut is counted"); + assert!(stats.trimmed >= 5, "the cut-off start is counted: {}", stats.trimmed); +} + /// A section lost before the cycle wraps commits an observed subset; the next /// cycle must *converge* to the full set rather than flip-flop between subsets. /// The repetition fast-path skips sections already active, so without the diff --git a/rs/moq-mux/src/container/ts/stats.rs b/rs/moq-mux/src/container/ts/stats.rs index baf84a40af8..2cb9f4ee5df 100644 --- a/rs/moq-mux/src/container/ts/stats.rs +++ b/rs/moq-mux/src/container/ts/stats.rs @@ -174,9 +174,18 @@ impl Class { pub struct Export { /// One row per elementary stream PID the exporter writes, ordered by PID. pub streams: BTreeMap, - /// Frames dropped for arriving after their deadline, and the frames a video track then - /// dropped waiting for its next keyframe. + /// Frames dropped after their stream's first went out: for arriving after their deadline, + /// for falling due before a new timeline's clock was acquired, and the frames a video track + /// then dropped waiting for its next keyframe. pub dropped: u64, + /// Frames dropped before their stream's first went out, so the output starts on a whole + /// group led by the video keyframe: what a joiner is handed older than the delay, a start + /// group cut short upstream, and what precedes the first keyframe or the codec config. + /// Nothing is missing from the output for these. + pub trimmed: u64, + /// Times a source skipped media it will never deliver: a group lost or expired upstream, + /// or a resubscription. Each is logged with the span it lost. + pub gaps: u64, /// How fast the source's clock runs against ours, in parts per million (positive runs /// fast), as last measured. `None` until there is a measurement. pub drift: Option, diff --git a/rs/moq-mux/src/jitter.rs b/rs/moq-mux/src/jitter.rs index 6715d535bc7..b89c3eb983a 100644 --- a/rs/moq-mux/src/jitter.rs +++ b/rs/moq-mux/src/jitter.rs @@ -65,7 +65,14 @@ pub(crate) enum Push { /// ahead and audio just in time, and a clock anchored on the video would make every audio /// frame late. A joiner is handed the group it joined at once, already partly old, and /// anchoring on its first frame would carry that lag for the whole run. What the anchor -/// makes due before the acquisition ended is dropped instead, without counting as late. +/// makes due before the acquisition ended is dropped instead. +/// +/// A track starts on a whole group. Until a frame of it has gone out, a loss (a frame +/// dropped here, or media its source skipped) takes everything the track holds with it, and +/// the track starts at its next sync frame: a joiner whose zero-budget live-edge pick has the +/// publisher cut its group short starts at the next group rather than on the cut-off start. +/// What goes before a track's first frame out is [`Buffer::trimmed`], not lost; what drops +/// after it is [`Buffer::dropped`]. /// /// A skipped group leaves the timeline where it was, so it keeps the clock: frames it made /// late are dropped like any other. A source that restarts its timeline (a declared @@ -140,6 +147,7 @@ pub(crate) struct Buffer { leads: BTreeMap>, timer: Option>>, dropped: u64, + trimmed: u64, /// Steps at which the source's clock ran further off ours than the clock may follow. out_of_tolerance: u64, } @@ -300,17 +308,10 @@ struct Track { skip: u64, /// The generation of the last frame. generation: u64, - /// Frames are dropped until the next sync frame, and why. - waiting: Option, -} - -/// Why a track waits for a sync frame. -#[derive(Clone, Copy, PartialEq, Eq)] -enum Wait { - /// A frame missed its deadline: what it takes with it counts as dropped. - Late, - /// The acquisition dropped what was due before it ended, which is not a loss. - Acquired, + /// Frames are dropped until the next sync frame. + waiting: bool, + /// Whether a frame of it has gone out. Until one has, a loss takes everything it holds. + started: bool, } impl Buffer { @@ -329,6 +330,7 @@ impl Buffer { leads: BTreeMap::new(), timer: None, dropped: 0, + trimmed: 0, out_of_tolerance: 0, } } @@ -518,25 +520,36 @@ impl Buffer { } /// Queue a frame on `generation` due at `deadline`, unless it is `stale` or cannot make it. + /// + /// Until a frame of the track has gone out, a loss (a frame dropped here, or media its + /// source skipped) takes everything the track holds with it, so its output starts on a + /// whole group at its next sync frame rather than on a group's cut-off start. fn queue(&mut self, key: K, arrival: Arrival, generation: u64, deadline: Option, stale: bool) -> Push { let track = self.tracks.entry(key).or_insert_with(|| Track { queue: VecDeque::new(), restart: arrival.restart, skip: arrival.skip, generation, - waiting: None, + waiting: false, + started: false, }); + // A skip the source declared no restart for: media it will never deliver. + let gap = arrival.skip.wrapping_sub(arrival.restart) != track.skip.wrapping_sub(track.restart); + if gap && !track.started { + self.trimmed += track.queue.len() as u64; + track.queue.clear(); + } track.restart = arrival.restart; track.skip = arrival.skip; track.generation = generation; let push = match deadline { _ if stale => { - track.waiting = Some(Wait::Acquired); + track.waiting = true; Push::Waiting } - _ if track.waiting.is_some() && !arrival.sync => Push::Waiting, + _ if track.waiting && !arrival.sync => Push::Waiting, Some(deadline) if arrival.arrived <= deadline || self.delay.is_zero() => { - track.waiting = None; + track.waiting = false; track .queue .push_back((generation, arrival.decode, deadline, arrival.item)); @@ -545,12 +558,17 @@ impl Buffer { } // Past the deadline, or so far off the clock that no instant holds it. _ => { - track.waiting = Some(Wait::Late); + track.waiting = true; Push::Late } }; - if push != Push::Queued && track.waiting == Some(Wait::Late) { - self.dropped += 1; + match push { + Push::Queued => {} + _ if track.started => self.dropped += 1, + _ => { + self.trimmed += 1 + track.queue.len() as u64; + track.queue.clear(); + } } push } @@ -790,7 +808,9 @@ impl Buffer { if let Some((deadline, key)) = &front && (self.delay.is_zero() || now >= *deadline) { - let (generation, _, _, item) = self.tracks.get_mut(key).and_then(|t| t.queue.pop_front()).unwrap(); + let track = self.tracks.get_mut(key).expect("the front's track"); + let (generation, _, _, item) = track.queue.pop_front().expect("the front frame"); + track.started = true; self.released = true; // A previous-generation frame queued now would go out after this one. if self.clock.is_some_and(|clock| clock.generation == generation) { @@ -818,6 +838,12 @@ impl Buffer { } } + /// The frame `key` releases next, if it has one queued. + pub fn peek(&self, key: &K) -> Option<&T> { + let (_, _, _, item) = self.tracks.get(key)?.queue.front()?; + Some(item) + } + /// When the next queued frame is due, or the acquisition under way ends. #[cfg(test)] pub fn next_deadline(&self) -> Option { @@ -849,11 +875,18 @@ impl Buffer { self.acquire.is_none() && self.tracks.values().all(|track| track.queue.is_empty()) } - /// How many frames were dropped, late or waiting for a sync frame after one was. + /// How many frames were dropped after their track's first went out: late, due before a new + /// generation's acquisition ended, or waiting for a sync frame after one of those. pub fn dropped(&self) -> u64 { self.dropped } + /// How many frames were dropped before their track's first went out, so it starts on a whole + /// group: what a joiner is handed older than the delay, and a start group a loss cut short. + pub fn trimmed(&self) -> u64 { + self.trimmed + } + /// The source's clock rate off ours, as last measured, as a fraction: positive runs fast. pub fn drift(&self) -> Option { self.steer.source @@ -1028,6 +1061,8 @@ mod tests { let mut buffer = Buffer::new(DELAY).replay(); buffer.push(1, arrival(start, 0, "v0")).unwrap(); buffer.push(2, arrival(start, 0, "a0")).unwrap(); + tokio::time::advance(DELAY).await; + assert_eq!(due(&mut buffer), ["v0", "a0"]); let late = start + Duration::from_millis(200); let video = |decode, sync, item| Arrival { sync, @@ -1037,10 +1072,10 @@ mod tests { assert_eq!(buffer.push(1, video(200, false, "v200")).unwrap(), Push::Waiting); assert_eq!(buffer.push(1, video(240, true, "v240")).unwrap(), Push::Queued); assert_eq!(buffer.push(2, arrival(late, 180, "a180")).unwrap(), Push::Queued); - assert_eq!(buffer.dropped(), 2); + assert_eq!((buffer.dropped(), buffer.trimmed()), (2, 0)); tokio::time::advance(Duration::from_secs(1)).await; - assert_eq!(due(&mut buffer), ["v0", "a0", "a180", "v240"]); + assert_eq!(due(&mut buffer), ["a180", "v240"]); } /// A restart before anything went out keeps the clock, so the tracks stay on one. @@ -1698,6 +1733,57 @@ mod tests { tokio::time::advance(DELAY).await; assert_eq!(due(&mut buffer), ["v240"]); assert_eq!(buffer.dropped(), 0, "trimming the join is not a loss"); + assert_eq!(buffer.trimmed(), 6); + } + + /// A start group its source cuts short before anything of it went out is dropped whole, + /// so the track starts on the next group rather than on the cut-off start. + #[tokio::test(start_paused = true)] + async fn a_start_group_cut_by_a_gap_is_dropped_whole() { + let start = Instant::now(); + let mut buffer = Buffer::new(DELAY).replay(); + let video = |decode, sync, skip, item| Arrival { + sync, + skip, + ..arrival(start, decode, item) + }; + buffer.push(1, video(0, true, 0, "v0")).unwrap(); + buffer.push(1, video(40, false, 0, "v40")).unwrap(); + // The rest of the group never arrives: the source skips to the next. + assert_eq!(buffer.push(1, video(120, true, 1, "v120")).unwrap(), Push::Queued); + buffer.push(1, video(160, false, 1, "v160")).unwrap(); + tokio::time::advance(Duration::from_secs(1)).await; + assert_eq!(due(&mut buffer), ["v120", "v160"]); + assert_eq!((buffer.dropped(), buffer.trimmed()), (0, 2)); + + // Once a frame went out, a gap is a loss in the output, and the rest still goes out. + buffer.push(1, video(200, false, 1, "v200")).unwrap(); + buffer.push(1, video(280, true, 2, "v280")).unwrap(); + tokio::time::advance(Duration::from_secs(1)).await; + assert_eq!(due(&mut buffer), ["v200", "v280"]); + assert_eq!((buffer.dropped(), buffer.trimmed()), (0, 2)); + } + + /// The keyframe a joiner holds while the export starts is read early and passes, while the + /// rest of its group, read later, is older than the delay. The keyframe goes with them, + /// rather than out alone ahead of a hole. + #[tokio::test(start_paused = true)] + async fn a_join_drops_its_group_whole() { + let start = Instant::now(); + let mut buffer = Buffer::new(DELAY); + let video = |arrived, decode, sync, item| Arrival { + sync, + ..arrival(arrived, decode, item) + }; + buffer.push(1, video(start, 0, true, "v0")).unwrap(); + tokio::time::advance(Duration::from_millis(250)).await; + let later = start + Duration::from_millis(250); + for (decode, sync, item) in [(40, false, "v40"), (80, false, "v80"), (400, true, "v400")] { + buffer.push(1, video(later, decode, sync, item)).unwrap(); + } + tokio::time::advance(Duration::from_secs(1)).await; + assert_eq!(due(&mut buffer), ["v400"]); + assert_eq!((buffer.dropped(), buffer.trimmed()), (0, 3)); } /// A source that sends one track well ahead of its decode time and another just in time diff --git a/test/ts/README.md b/test/ts/README.md index b712aee170d..6c268c42579 100644 --- a/test/ts/README.md +++ b/test/ts/README.md @@ -614,6 +614,13 @@ the same access unit: AAC overlap and perceptual-noise state differ between a cold decoder and a decoder that saw earlier frames. No arbitrary warmup samples are discarded. PES boundaries and TS packetization may change. +The suffix is the exporter's to choose, and it starts on a whole group: a join +whose first group is cut short before anything of it went out starts at the next +one. Anything lost after that fails. When it does, the harness prints what the +exporter reported: the `TS export release clock` counts (`dropped` late, +`trimmed` before the output began, `gaps` skipped upstream) and each loss it +logged. + `just test ts-controls` exercises unchanged input, remuxing with new timestamps and PES sizes, timestamp quantization, and a cold join. Negative controls require the intended failure for missing audio, changed pictures, changed audio, a lost diff --git a/test/ts/run.sh b/test/ts/run.sh index 8756789e816..1f0b8807843 100755 --- a/test/ts/run.sh +++ b/test/ts/run.sh @@ -631,7 +631,15 @@ echo "### captured $(wc -c <"$SUB_TS" | tr -d ' ') bytes -> analyzing" # pair. Other arms retain their specific structural/model oracles until their # acquisition and decoder-priming contracts are covered by fixtures. if [[ -z "$SOURCE$OPEN_GOP$HRD$HEADROOM$LEAK$FULL" ]]; then - python3 "$DIR/decode.py" "$SRC_TS" "$SUB_TS" --output "$HARNESS_RUN/decode" + if ! python3 "$DIR/decode.py" "$SRC_TS" "$SUB_TS" --output "$HARNESS_RUN/decode"; then + # The exporter counts what it dropped (late), trimmed (before its output began), + # and lost upstream (gaps), and logs each loss, so the failure says which it was. + echo >&2 + echo "error: the decoded capture differs from the source; the exporter reported:" >&2 + grep -E 'TS export release clock|media skipped upstream|missed its deadline|group evicted' \ + "$HARNESS_RUN/sub.log" | sed 's/^/ sub: /' >&2 || true + exit 1 + fi else echo "decoded-content comparison: not covered for this profile" fi From 38f4c08693536e67b042ff60202723d4d8e258fb Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 08:25:00 -0700 Subject: [PATCH 3/7] quest: finish ts-video-content Co-Authored-By: Claude Opus 5.5 --- quest/m1/test-flakes-2/README.md | 1 - .../m1/test-flakes-2/ts-resume-late-frames.md | 7 +- quest/m1/test-flakes-2/ts-video-content.md | 153 ------------------ quest/m1/ts-drift-startup.md | 2 +- 4 files changed, 5 insertions(+), 158 deletions(-) delete mode 100644 quest/m1/test-flakes-2/ts-video-content.md diff --git a/quest/m1/test-flakes-2/README.md b/quest/m1/test-flakes-2/README.md index f95604acd21..63125be9fb4 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -35,7 +35,6 @@ Public API: none. Wire: none. ## Required -- [TS decoded content](/quest/m1/test-flakes-2/ts-video-content.md) - CI's TS compliance decoded-content checks fail only on real loss, and the exporter names every frame it drops - [TS resume late frames](/quest/m1/test-flakes-2/ts-resume-late-frames.md) - the same-instance TS resume test keeps its late frames, fixed in production or the fixture as the evidence says - [WebTransport close release](/quest/m1/test-flakes-2/interop-close-release.md) - a compatible published adapter contains the acknowledged-close fix - [Interop close code](/quest/m1/test-flakes-2/interop-close-code.md) - adopt that release so the browser reliably sees `unauthorized` for a refused session diff --git a/quest/m1/test-flakes-2/ts-resume-late-frames.md b/quest/m1/test-flakes-2/ts-resume-late-frames.md index a9fe5995df5..3c4b0076dd2 100644 --- a/quest/m1/test-flakes-2/ts-resume-late-frames.md +++ b/quest/m1/test-flakes-2/ts-resume-late-frames.md @@ -24,9 +24,10 @@ passed with no fixture change. Fixture: the test feeds the capture tail faster or later than a real publisher would, so pace it on the media clock or a mocked clock. - No retries, wider timeouts, or a looser new-content assertion. -- Compare with [TS decoded content](/quest/m1/test-flakes-2/ts-video-content.md): - its first hypothesis (a zero `max_delay` live subscription) may cause both. +- Compare with [#5713](https://github.com/moq-dev/moq/pull/5713): a zero + `max_delay` live subscription cut the TS export's start group, and the + export now counts `dropped`, `trimmed` and `gaps` separately. ## Related -- [TS decoded content](/quest/m1/test-flakes-2/ts-video-content.md) - the CI TS compliance flake, possibly the same cause +- [#5713](https://github.com/moq-dev/moq/pull/5713) - the CI TS compliance flake, possibly the same cause diff --git a/quest/m1/test-flakes-2/ts-video-content.md b/quest/m1/test-flakes-2/ts-video-content.md deleted file mode 100644 index d4d10232bc8..00000000000 --- a/quest/m1/test-flakes-2/ts-video-content.md +++ /dev/null @@ -1,153 +0,0 @@ -# [L] TS compliance fails only on real loss - -## Goal - -CI's `TS compliance` step (`just test ts`, `.github/workflows/ts.yml`) -gives the same verdict on every run of the same code. Its decoded-content -checks (`test/ts/decode.py`) fail only when the exporter really loses or -corrupts content, never because of how the host scheduled the run. Each -frame the exporter drops is counted and logged, so a failure says what was -dropped and why. The checks stay as strict: a captured suffix that loses or -duplicates a picture or an audio frame still fails. - -## Plan - -### Evidence - -CI: seen 2026-10-13 on #5471, #5481 and two unrelated branches within the -same hour, each passing on rerun. Then, from `gh run list --workflow ts.yml` -on 2026-10-14: since #5328 merged (2026-10-13 23:49 UTC), 4 of 41 completed -PR runs failed `video-content` ("decoded pictures are not an intact suffix of -the source"): #5446 at f6629aac8 -([38506130438](https://github.com/moq-dev/moq/actions/runs/38506130438)), -#5148 at 5adfd84a6 -([38502761750](https://github.com/moq-dev/moq/actions/runs/38502761750)), -`quest/m1/cli-duration-defaults` at e7e3eb1c4 -([38502673714](https://github.com/moq-dev/moq/actions/runs/38502673714)), and -#5481's branch `quest/m1/catalog-wall-clock-precision` at 312ac6260 -([38503267980](https://github.com/moq-dev/moq/actions/runs/38503267980)). -The first three do not touch TS; #5481 changed the shared clock this -pipeline reads, so keep it out of any argument that narrows the cause. -Only the default `just test ts` round failed; every later round passed in the -same runs. Main has no run yet: the workflow runs on matching PRs and a -nightly that had not fired. Failures on branches without #5471 or #5481 -point to an older cause, but check a failing capture against those diffs -before ruling them out. - -Every failing capture was 24531556 bytes against 23281544 in passing runs. -The difference, 1250012 bytes, is 1 s at the padded 10 Mb/s rate, so the -failing export started one 25-frame GOP earlier (group 1, not group 2). The -`--hrd` and `--headroom` rounds show the same one-GOP swing between runs but -do not run `decode.py`. Each failing run uploaded a `ts-failure` artifact -(`decode/*/frames.sha256`, `sub.log`), kept 7 days. - -Local, on 2026-10-14, `test/ts/run.sh` on a host at load ~28 on 24 cores -failed 3 of 4 runs, in two other ways: - -- `video-content`, once. The subscriber's `https://` race was won by - WebSocket, then upgraded to WebTransport; the old session "did not drain in - time" (GOAWAY timeout), and the exporter logged "current group evicted; - skipping to next buffered group" (`rs/moq-mux/src/container/consumer.rs`) - and one "frame missed its deadline". About 30 pictures across groups 10 and - 11 were missing, yet FFmpeg decoded the capture without an error. -- `audio-timing` ("capture has a gap or overlap in decoded audio"), twice, - over WebTransport with no upgrade and intact video. Two or three AAC frames - (43 to 64 ms) vanished mid-stream with no warning anywhere, and the - exporter's release clock reported `dropped=0`. - -Then [#5565](https://github.com/moq-dev/moq/issues/5565), on 2026-10-15: -4 of 51 `ts.yml` runs between 2026-10-14 19:57Z and 2026-10-15 03:55Z failed -`video-content` the same way, two on branches that change nothing in moq-net, -moq-relay or moq-mux's consumer: 38634848576 (`fix/tokio-windows-refused-dial`), -38657903232 (#5553), 38662560784 (`quest/m1/papercuts-rs`) and 38677047938 -(#5548's branch). Their artifacts expire about 2026-10-22. Each `sub.log` has -`current group evicted; skipping to next buffered group track="0.avc3" -group=1 error=Hang(Moq(Stream(Old)))`, 17 to 21 ms after a video subscribe -at about 1.98 s, and the capture keeps pictures 25 to about 33, then 50 -onward. #5548 moved the zero-budget conviction from the successor's first -frame to its arrival, essentially the same instant, so it should not change -the rate. Not reproduced locally: 0 of 15 on main (3abf0161c, macOS arm64). - -So in each case the checks saw real loss. The flake is what makes the -exporter lose content, and the loss goes unreported. - -2026-10-15: the same `video-content` failure blocked unrelated #5566 twice in -one run ([38725177110](https://github.com/moq-dev/moq/actions/runs/38725177110), -both attempts), and hit #5553 and #5148 again, so the rate is above the 4 in -41 estimate. In each of #5553's and #5566's `sub.log`, about 20 ms after the -video subscribe: `current group evicted ... group=1 -error=Hang(Moq(Stream(Old)))`, and `relay.log` and `pub.log` show -`max_delay: 0ns` on `0.avc3`. The publisher serves group 1, group 2's first -frame lands about 17 ms later, `poll_drifted` judges group 1 zero behind the -edge, and it resets group 1's stream `Old` before the raised budget reaches -it through the relay. That confirms hypothesis 1's first candidate (analysis -on #5565). Capture size does not separate the cases at a 1 s `--delay`: a -passing local run captured 25156468 bytes, like a failing CI run, so use the -log signature. - -### Hypotheses - -1. Startup truncation (CI). The importer holds the catalog for its 2 s PCR - measurement window (`rs/moq-mux/src/container/ts/mux_rate.rs`), so the - exporter subscribes around group 2's IDR, and whether group 1 is still - the newest is a wall-clock race. A group-1 join can then keep only a - prefix of group 1 before skipping to group 2. Two candidates: - - The live subscription starts with `max_delay = 0` and only relaxes to - the budget after the first frame (`rs/moq-mux/src/container/source.rs`), - so moq-net may expire group 1 partway through. - - The jitter buffer's acquisition judges staleness per frame against its - own read time (`judge`/`queue` in `rs/moq-mux/src/jitter.rs`). The held - IDR can pass while the frames read after the tables were built are - stale, and the track then waits for the next keyframe. Those drops are - `Wait::Acquired`, which is not counted in `dropped` or logged. -2. Transport upgrade (local mode 1). The WebSocket-to-QUIC upgrade's handover - evicts the current group. [#5484](https://github.com/moq-dev/moq/pull/5484) - (open) removes the upgrade. Check whether a GOAWAY drain still loses the - current group's tail, since drains remain. -3. Silent audio loss under load (local mode 2). Each AAC frame is its own - group (`rs/moq-mux/src/container/ts/import.rs`), so a group expired by the - latency budget, or skipped by the consumer, drops one frame with no count. - -Ruled out from reading the code: picture duplication, timestamp rebasing, and -pre-IDR content in group 0 (the importer opens a group only at a keyframe). - -### Steps - -- Reproduce first. Download a `ts-failure` artifact from one of the CI runs - above before it expires (about 2026-10-21) and map the captured pictures - onto the source to confirm hypothesis 1. Locally, loop `test/ts/run.sh` - beside a CPU hog, as above. -- Fail loud: every frame the exporter drops (acquisition, late, or a skipped - or expired group) is counted and logged, and the harness prints that count - beside a failing check. -- Fix each confirmed cause at its source. Recommended for hypothesis 1: an - export starts at a whole group, so acquisition admits or drops a group - entirely rather than emitting a prefix. Keep the zero-budget live-edge - pick (`ExportSource::live`): starting with the full budget would let a - join reach back to an older retained group, adding startup latency and - replay, a user-visible change. Make the group it picks atomic instead. - Each fix lands with a regression that fails without it, on a paused Tokio - clock or the controlled clock, never wall time. The jitter and export - tests in `rs/moq-mux` already run on mocked time. -- Keep the harness on its real-time path; this quest fixes the exporter, not - the grader. If #5484 has landed, rerun on top of it before chasing - hypothesis 2. Never loosen the comparison, widen a timeout or `--delay`, or - add a retry. -- Open question for the implementer to put to the maintainer, once the causes - are known: whether a frame dropped because the host starved the exporter - past its latency budget fails `decode.py`, or fails a separate named check - fed by the exporter's drop count. Recommended: a separate check, so - `video-content` and `audio-content` mean corruption only. - -Public API: none expected; the drop count may surface in the exporter's -summary. Wire: none. - -## Closes - -- [#5565](https://github.com/moq-dev/moq/issues/5565) - the TS compliance video-content flake - -## Related - -- [#5484](https://github.com/moq-dev/moq/pull/5484) - removes the WebSocket-to-QUIC upgrade behind local mode 1 -- [Loss and recovery tests](/quest/m1/ts-reliability/recovery.md) - deliberate MoQ content loss, which this flake hits by accident -- [Controlled time](/quest/m1/time/README.md) - the clock the regressions should run on diff --git a/quest/m1/ts-drift-startup.md b/quest/m1/ts-drift-startup.md index 960c1e25cbb..c45932857b7 100644 --- a/quest/m1/ts-drift-startup.md +++ b/quest/m1/ts-drift-startup.md @@ -53,4 +53,4 @@ Public API: none. Wire: none. ## Related - [Live TS clock validation](/quest/m1/ts-live-clock.md) - the grader's PCR-rate verdict on `--live`, a different layer -- [TS decoded content](/quest/m1/test-flakes-2/ts-video-content.md) - the other TS CI flake, on content loss +- [#5713](https://github.com/moq-dev/moq/pull/5713) - the other TS CI flake, on content loss: an export starts on a whole group From e9563d5939ff7867e2aeeffca08d51fd8ebb9d51 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 09:27:36 -0700 Subject: [PATCH 4/7] fix(ts): log the trim count beside a late frame A frame late before its track's first went out counts as trimmed, not dropped, so the warning carries both counts. Co-Authored-By: Claude Opus 5.5 --- rs/moq-mux/src/container/ts/export.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rs/moq-mux/src/container/ts/export.rs b/rs/moq-mux/src/container/ts/export.rs index ef9a7165668..618f1f0f069 100644 --- a/rs/moq-mux/src/container/ts/export.rs +++ b/rs/moq-mux/src/container/ts/export.rs @@ -632,7 +632,12 @@ impl Track { }, }; if jitter.push(self.pid, arrival)? == Push::Late { - tracing::warn!(track = %name, dropped = jitter.dropped(), "frame missed its deadline; dropped"); + tracing::warn!( + track = %name, + dropped = jitter.dropped(), + trimmed = jitter.trimmed(), + "frame missed its deadline; dropped" + ); } Ok(()) } From 765f99b2f5ca821f25dd597ee71c932ea43ba525 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 11:00:22 -0700 Subject: [PATCH 5/7] fix(ts): say a data track's declared break reads as a gap A data track marks a discontinuity by skipping a sequence, which the consumer reads as media skipped upstream. Document that in stats::Export::gaps and the warning, and add quests for telling the break apart on the wire and for audio leading a keyframe after a trimmed video start. Co-Authored-By: Claude Opus 5.5 --- quest/m1/ts-reliability/README.md | 2 ++ quest/m1/ts-reliability/audio-lead.md | 27 +++++++++++++++++++++++++ quest/m1/ts-reliability/data-break.md | 29 +++++++++++++++++++++++++++ rs/moq-mux/src/container/source.rs | 7 ++++--- rs/moq-mux/src/container/ts/stats.rs | 3 ++- 5 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 quest/m1/ts-reliability/audio-lead.md create mode 100644 quest/m1/ts-reliability/data-break.md diff --git a/quest/m1/ts-reliability/README.md b/quest/m1/ts-reliability/README.md index c70f555e76d..1a6a6278ee3 100644 --- a/quest/m1/ts-reliability/README.md +++ b/quest/m1/ts-reliability/README.md @@ -58,6 +58,8 @@ remain with their current quests. - [Plan: semantic fidelity](/quest/m1/ts-reliability/fidelity.md) - explicit profiles and PMT lifecycle contracts produce implementation quests - [Plan: PES timing and headers](/quest/m1/plan-ts-pes-untimed.md) - private PES semantics have a settled carriage and placement contract - [Loss and recovery tests](/quest/m1/ts-reliability/recovery.md) - actual MoQ omissions exercise reacquisition and unaffected tracks +- [Data-track breaks](/quest/m1/ts-reliability/data-break.md) - a data track's declared break reads as a break, not as media lost upstream +- [Audio behind the keyframe](/quest/m1/ts-reliability/audio-lead.md) - an export whose video start group is dropped still begins on a keyframe - [T-STD TS export](/quest/m1/tstd/README.md) - the semantic mux schedules the content it admits within its supported buffer models ## Related diff --git a/quest/m1/ts-reliability/audio-lead.md b/quest/m1/ts-reliability/audio-lead.md new file mode 100644 index 00000000000..f7befebe023 --- /dev/null +++ b/quest/m1/ts-reliability/audio-lead.md @@ -0,0 +1,27 @@ +# [S] A trimmed video start keeps audio behind the keyframe + +## Goal + +`moq export ts` begins on a video keyframe even when the video's start group +is dropped at acquisition and its next keyframe has not arrived yet. No audio +or data frame goes out ahead of the first video frame. + +## Plan + +Found reading [#5713](https://github.com/moq-dev/moq/pull/5713), not +reproduced. Until video goes out, `Export::leads_video` +(`rs/moq-mux/src/container/ts/export.rs`) drops non-video frames that present +before the earliest queued video frame. With no video frame queued it falls +back to `video_start`. When the acquisition drops the video's start group as +stale, the video queue is empty until the next keyframe arrives. Audio after +`video_start` then goes out first, by up to the remaining GOP less the delay. +The upstream-cut path that #5565 hit is not affected, because the next +group's keyframe arrives with the gap. + +Recommended fix, settled in #5713: while no video frame has gone out and none +is queued, drop non-video frames as trimmed. Land it only with a regression +that fails without it. Three paused-clock attempts on the `Interleave` rig did +not drop the start group, because a frame's `arrived` is when its source was +last found empty. A fixture likely needs the tables to wait on something +other than the video (a late catalog entry), so the held keyframe is read +well before the rest of its group. diff --git a/quest/m1/ts-reliability/data-break.md b/quest/m1/ts-reliability/data-break.md new file mode 100644 index 00000000000..ddfd1c77d3f --- /dev/null +++ b/quest/m1/ts-reliability/data-break.md @@ -0,0 +1,29 @@ +# [M] A data track's declared break is not a loss + +## Goal + +A consumer tells a data track's declared discontinuity from media lost +upstream. `moq export ts` stops counting a declared break on a verbatim PES +track (DVB AC-3, teletext, SCTE-35, ID3) in `stats::Export::gaps`, stops +warning "media skipped upstream" for it, and no longer trims what such a +track holds when the break lands before its first frame goes out. + +## Plan + +Found in [#5713](https://github.com/moq-dev/moq/pull/5713). Audio and video +mark a break with one empty frame, which the consumer reads as `Read::End`. +An empty payload is data on a data track, so `Producer::discontinuity` +(`rs/moq-mux/src/container/producer.rs`) skips a sequence with no object +instead. The consumer reads that skip as `Read::Gap`, the same as a lost +group, so the export's jitter buffer, gap count and warning treat it as loss. +#5713 documents the overlap rather than fixing it. + +Settle the carriage first: the marker must be visible on the wire, and an +empty group (zero objects) already means nothing. Any wire or format change +updates `drafts/` and `js/hang` in the same PR. The regression is +`export_drops_an_ac3_frame_cut_by_a_marker` (`rs/moq-mux/src/container/ts/export_test.rs`), +which should keep frame 0 again once the break is told apart. + +## Related + +- [Loss and recovery tests](/quest/m1/ts-reliability/recovery.md) - the loss reporting this keeps honest diff --git a/rs/moq-mux/src/container/source.rs b/rs/moq-mux/src/container/source.rs index dd1d5f9a06f..9e1a38dd53b 100644 --- a/rs/moq-mux/src/container/source.rs +++ b/rs/moq-mux/src/container/source.rs @@ -400,12 +400,13 @@ impl ExportSource { frame } Some(Item::Read(Read::Gap { timestamp, duration })) => { - // The export never receives this media, so say what it loses and where. + // Say what the export never receives and where. A data track declares a + // break by skipping a sequence, which reads the same though nothing is lost. tracing::warn!( track = consumer.name(), at = ?std::time::Duration::from(timestamp), - lost = ?std::time::Duration::from(duration), - "media skipped upstream; resuming at the next group" + skipped = ?std::time::Duration::from(duration), + "media skipped upstream, or a data track's declared break; resuming at the next group" ); self.finish_ends(); self.skips += 1; diff --git a/rs/moq-mux/src/container/ts/stats.rs b/rs/moq-mux/src/container/ts/stats.rs index 2cb9f4ee5df..1286781882e 100644 --- a/rs/moq-mux/src/container/ts/stats.rs +++ b/rs/moq-mux/src/container/ts/stats.rs @@ -184,7 +184,8 @@ pub struct Export { /// Nothing is missing from the output for these. pub trimmed: u64, /// Times a source skipped media it will never deliver: a group lost or expired upstream, - /// or a resubscription. Each is logged with the span it lost. + /// or a resubscription. A data track's declared break is a skipped sequence, so it counts + /// here too, though nothing of it is lost. Each is logged with the span it skipped. pub gaps: u64, /// How fast the source's clock runs against ours, in parts per million (positive runs /// fast), as last measured. `None` until there is a measurement. From cb64c6def1a69f079700c6408aa7292e22bc96e6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 11:15:15 -0700 Subject: [PATCH 6/7] fix(ts): leave a track unstarted when its first release is trimmed The jitter buffer marked a track started on release, before the export dropped the frame for leading the video keyframe. A later loss on that track then counted as dropped from the output and skipped the start trim. Buffer::discard now counts the dropped release and leaves a track whose first frame it was not started. Co-Authored-By: Claude Opus 5.5 --- rs/moq-mux/src/container/ts/export.rs | 5 +- rs/moq-mux/src/container/ts/export_test.rs | 76 ++++++++++++++++++++++ rs/moq-mux/src/jitter.rs | 18 ++++- 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/rs/moq-mux/src/container/ts/export.rs b/rs/moq-mux/src/container/ts/export.rs index 618f1f0f069..fc37b263e93 100644 --- a/rs/moq-mux/src/container/ts/export.rs +++ b/rs/moq-mux/src/container/ts/export.rs @@ -191,7 +191,8 @@ where /// what the other tracks release is dropped: a video track whose start group was cut /// before it went out starts at its next keyframe instead ([`jitter::Buffer`]). video_out: bool, - /// Frames dropped ahead of the first video keyframe, or of their codec config. + /// Frames dropped before the jitter buffer: ahead of the first video keyframe, or of their + /// codec config. trimmed: u64, /// Each elementary stream's access units and silence on the PCR returned ([`Self::stats`]). liveness: super::import::Liveness, @@ -1277,7 +1278,7 @@ where .map(|(name, t)| (name.clone(), matches!(t.kind, Kind::Video(_)))) .context("frame for an unknown PID")?; if self.leads_video(is_video, ready.item.frame.timestamp) { - self.trimmed += 1; + self.jitter.discard(&ready); } else { self.last_timestamp = Some(ready.item.frame.timestamp); let decode = ready.item.decode.as_nanos(); diff --git a/rs/moq-mux/src/container/ts/export_test.rs b/rs/moq-mux/src/container/ts/export_test.rs index 1cc0b6eb1e5..293684d0c4f 100644 --- a/rs/moq-mux/src/container/ts/export_test.rs +++ b/rs/moq-mux/src/container/ts/export_test.rs @@ -5550,6 +5550,82 @@ async fn a_start_group_cut_short_starts_at_the_next() { assert!(stats.trimmed >= 5, "the cut-off start is counted: {}", stats.trimmed); } +/// Audio released while the video's start moves to its next group is dropped ahead of the +/// keyframe, and its track has still sent nothing: audio that then misses its deadline is +/// trimmed with it, not counted as lost from the output. +#[tokio::test(start_paused = true)] +async fn audio_dropped_ahead_of_a_moved_start_is_not_started() { + use crate::container::Container as _; + + let delay = Duration::from_millis(200); + let mut rig = Interleave::new().await; + let mut export = rig.export(delay).await; + let track = rig.video.track().clone(); + let wire = HangContainer::Legacy(crate::container::Kind::Video); + let picture = |tick: u64| { + let keyframe = tick.is_multiple_of(GOP); + let slice = vec![if keyframe { 0x65u8 } else { 0x41 }; 400]; + Frame { + timestamp: Timestamp::from_micros(tick * VIDEO_US).unwrap(), + duration: None, + payload: length_prefixed(&[&slice]), + keyframe, + } + }; + let mut out = Vec::new(); + + // The first group delivers three pictures, then the next group lands, sent ahead, and the + // publisher resets the first. + let mut cut = track.create_group(moq_net::group::Info { sequence: 0 }).unwrap(); + for tick in 0..3 { + rig.at(tick * VIDEO_US).await; + wire.write(&mut cut, &[picture(tick)]).unwrap(); + out.extend(poll_frames(&mut export)); + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + let mut group = track.create_group(moq_net::group::Info { sequence: 1 }).unwrap(); + wire.write(&mut group, &[picture(GOP)]).unwrap(); + cut.abort(moq_net::Error::Old).unwrap(); + // The audio flows long enough for some of it to go out and be dropped ahead of the + // keyframe, then stalls past its deadline. + for tick in 3..10 { + rig.at(tick * VIDEO_US).await; + out.extend(poll_frames(&mut export)); + if tick < 5 { + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + } + for tick in 10..3 * GOP { + rig.at(tick * VIDEO_US).await; + if tick > GOP { + if tick.is_multiple_of(GOP) { + group.finish().unwrap(); + group = track + .create_group(moq_net::group::Info { sequence: tick / GOP }) + .unwrap(); + } + wire.write(&mut group, &[picture(tick)]).unwrap(); + } + out.extend(poll_frames(&mut export)); + rig.audio_until(tick * VIDEO_US, &mut export, &mut out); + } + group.finish().unwrap(); + rig.finish(); + out.extend(drain_frames(&mut export).await); + + let ts: Vec = out.iter().flat_map(|f| f.payload.iter().copied()).collect(); + let (video, audio) = collect_pes_pts(&ts); + let pts = |tick: u64| tick * VIDEO_US * 90 / 1_000; + assert_eq!(video.first(), Some(&pts(GOP)), "the export starts at the next group"); + assert!( + audio.iter().all(|&p| p >= pts(GOP)), + "no audio leads the keyframe: {audio:?}" + ); + let stats = export.stats(); + assert_eq!(stats.dropped, 0, "nothing was lost from the output"); + assert!(stats.trimmed > 0); +} + /// A section lost before the cycle wraps commits an observed subset; the next /// cycle must *converge* to the full set rather than flip-flop between subsets. /// The repetition fast-path skips sections already active, so without the diff --git a/rs/moq-mux/src/jitter.rs b/rs/moq-mux/src/jitter.rs index 8ea1b67aceb..fd244ea8a2b 100644 --- a/rs/moq-mux/src/jitter.rs +++ b/rs/moq-mux/src/jitter.rs @@ -36,6 +36,8 @@ pub(crate) struct Ready { /// frame of one generation goes out before any frame of the next. pub generation: u64, pub item: T, + /// Whether it is the first frame of its track to go out. + pub first: bool, } /// What [`Buffer::push`] did with a frame. @@ -823,7 +825,7 @@ impl Buffer { { let track = self.tracks.get_mut(key).expect("the front's track"); let (generation, _, _, item) = track.queue.pop_front().expect("the front frame"); - track.started = true; + let first = !std::mem::replace(&mut track.started, true); self.released = true; // A previous-generation frame queued now would go out after this one. if self.clock.is_some_and(|clock| clock.generation == generation) { @@ -833,6 +835,7 @@ impl Buffer { track: key.clone(), generation, item, + first, }); } let Some(wake) = front.map(|(deadline, _)| deadline).into_iter().chain(acquired).min() else { @@ -851,6 +854,19 @@ impl Buffer { } } + /// Count a frame the caller dropped after [`Self::poll_next`] released it. The first of its + /// track is trimmed and leaves the track not started; a later one is a loss. + pub fn discard(&mut self, ready: &Ready) { + if !ready.first { + self.dropped += 1; + return; + } + self.trimmed += 1; + if let Some(track) = self.tracks.get_mut(&ready.track) { + track.started = false; + } + } + /// The frame `key` releases next, if it has one queued. pub fn peek(&self, key: &K) -> Option<&T> { let (_, _, _, item) = self.tracks.get(key)?.queue.front()?; From a9bd0e618237c3517fe0076686089a48547fc7e2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 12:15:34 -0700 Subject: [PATCH 7/7] fix(ts): count leading pictures orphaned by a hole as dropped A delivery gap arms the leading floor, and the pictures below it were discarded before the jitter buffer saw them, so they counted nowhere. They are now dropped when the output holds frames ahead of the hole, and trimmed when they lead the track's first keyframe. Co-Authored-By: Claude Opus 5.5 --- rs/moq-mux/src/container/ts/export.rs | 18 ++++++++++++++---- rs/moq-mux/src/container/ts/export_test.rs | 19 +++++++++++++++++-- rs/moq-mux/src/container/ts/stats.rs | 6 ++++-- rs/moq-mux/src/jitter.rs | 9 +++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/rs/moq-mux/src/container/ts/export.rs b/rs/moq-mux/src/container/ts/export.rs index d03eb0c8f0d..4124b20a6a9 100644 --- a/rs/moq-mux/src/container/ts/export.rs +++ b/rs/moq-mux/src/container/ts/export.rs @@ -265,8 +265,12 @@ struct Track { clock: DecodeClock<(Pending, Option)>, /// Loss counter at the last read, excluding declared ends. recovery_gap: Option, - /// Leading pictures below the recovery keyframe have unavailable references. - leading_floor: Option, + /// Leading pictures below the recovery keyframe have unavailable references, and whether + /// the track queued frames ahead of that keyframe, so dropping them leaves a hole. + leading_floor: Option<(Timestamp, bool)>, + /// The source's cut count at the last frame queued. A cut since clears what a track that + /// has not started holds. + held: Option, /// The PTS of the last keyframe queued, which leading pictures present before. keyframe: Option, /// What the catalog and the SPS declare about the video's reordering. @@ -307,6 +311,7 @@ impl Track { clock: DecodeClock::default(), recovery_gap: None, leading_floor: None, + held: None, keyframe: None, timing: Timing::default(), buffer: None, @@ -338,11 +343,16 @@ impl Track { let gap = pending.skip.saturating_sub(pending.restart); let recover = self.recovery_gap.replace(gap).is_none_or(|previous| previous != gap); if pending.frame.keyframe { - self.leading_floor = recover.then_some(pending.frame.timestamp); + let ahead = self.held == Some(pending.cut); + self.leading_floor = recover.then_some((pending.frame.timestamp, ahead)); } - if self.leading_floor.is_some_and(|floor| pending.frame.timestamp < floor) { + if let Some((floor, ahead)) = self.leading_floor + && pending.frame.timestamp < floor + { + jitter.lose(&self.pid, ahead); return Ok(()); } + self.held = Some(pending.cut); if pending.frame.keyframe { self.timing.describe(stream_type, self.source.description(), name); } diff --git a/rs/moq-mux/src/container/ts/export_test.rs b/rs/moq-mux/src/container/ts/export_test.rs index ac9446086c5..e9816b2895e 100644 --- a/rs/moq-mux/src/container/ts/export_test.rs +++ b/rs/moq-mux/src/container/ts/export_test.rs @@ -1022,8 +1022,17 @@ async fn export_trims_leading_pictures_after_each_delivery_break() { group.finish().unwrap(); } track.finish().unwrap(); - let ts = drain(consumer).await; - let (video, _) = collect_pes_pts(&ts); + let mut export = open_export(crate::source::announced(&consumer).await) + .await + .unwrap() + .with_delay(RECORDING_MAX_AGE) + .with_replay(); + let bytes: Vec = drain_frames(&mut export) + .await + .iter() + .flat_map(|frame| frame.payload.iter().copied()) + .collect(); + let (video, _) = collect_pes_pts(&bytes); let expected: Vec<_> = [100, 110, 200, 180, 190, 210, 400, 410] .into_iter() .map(|ms| ms * 90) @@ -1032,6 +1041,12 @@ async fn export_trims_leading_pictures_after_each_delivery_break() { video, expected, "continuous leading pictures survive; only join and later-hole orphans are trimmed" ); + let stats = export.stats(); + assert_eq!( + (stats.dropped, stats.trimmed), + (2, 2), + "the join's orphans precede the output; the hole's are lost in it" + ); } /// A frame dropped for missing its deadline takes the leading pictures of the keyframe the diff --git a/rs/moq-mux/src/container/ts/stats.rs b/rs/moq-mux/src/container/ts/stats.rs index 4fb35bdeab1..b8b61981b76 100644 --- a/rs/moq-mux/src/container/ts/stats.rs +++ b/rs/moq-mux/src/container/ts/stats.rs @@ -176,11 +176,13 @@ pub struct Export { pub streams: BTreeMap, /// Frames dropped after their stream's first went out: for arriving after their deadline, /// for falling due before a new timeline's clock was acquired, and the frames a video track - /// then dropped waiting for its next keyframe or leading the keyframe it resumed at. + /// then dropped waiting for its next keyframe or leading the keyframe it resumed at, late + /// or after a hole upstream once the output held frames ahead of the hole. pub dropped: u64, /// Frames dropped before their stream's first went out, so the output starts on a whole /// group led by the video keyframe: what a joiner is handed older than the delay, a start - /// group cut short upstream, and what precedes the first keyframe or the codec config. + /// group cut short upstream, the first keyframe's leading pictures, and what precedes the + /// first keyframe or the codec config. /// Nothing is missing from the output for these. pub trimmed: u64, /// Times a source skipped media it will never deliver: a group lost or expired upstream, diff --git a/rs/moq-mux/src/jitter.rs b/rs/moq-mux/src/jitter.rs index 0e3ca4b23ca..79369f3fae3 100644 --- a/rs/moq-mux/src/jitter.rs +++ b/rs/moq-mux/src/jitter.rs @@ -886,6 +886,15 @@ impl Buffer { } } + /// Count a frame of `key` the caller dropped before queuing it: a loss once the track + /// started or when the caller queued frames `ahead` of it, else trimmed. + pub fn lose(&mut self, key: &K, ahead: bool) { + match ahead || self.tracks.get(key).is_some_and(|track| track.started) { + true => self.dropped += 1, + false => self.trimmed += 1, + } + } + /// The frame `key` releases next, if it has one queued. pub fn peek(&self, key: &K) -> Option<&T> { let (_, _, _, item) = self.tracks.get(key)?.queue.front()?;