From eca7dc7bcfa49addfdd5f126f8d594bdf3ae0120 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 17:04:33 -0700 Subject: [PATCH 1/2] quest: claim ts-reliability/audio-lead From 145abe4d99ef2fa5464be1c71cecc481fbd271c9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 16 Oct 2026 17:24:35 -0700 Subject: [PATCH 2/2] fix(ts): hold audio behind a stale start group's next keyframe A joiner's video start group older than the delay goes stale at acquisition, leaving the video queue empty until its next keyframe. leads_video fell back to the dropped keyframe's timestamp, so audio and data went out ahead of the first video frame. Drop them as trimmed until a video frame is queued. Co-Authored-By: Claude Opus 5.5 --- quest/m1/ts-reliability/README.md | 1 - quest/m1/ts-reliability/audio-lead.md | 27 -------- rs/moq-mux/src/container/ts/export.rs | 11 ++-- rs/moq-mux/src/container/ts/export_test.rs | 76 ++++++++++++++++++---- rs/moq-srt/src/ts.rs | 8 ++- 5 files changed, 78 insertions(+), 45 deletions(-) delete mode 100644 quest/m1/ts-reliability/audio-lead.md diff --git a/quest/m1/ts-reliability/README.md b/quest/m1/ts-reliability/README.md index c391f65c98..2036f92e05 100644 --- a/quest/m1/ts-reliability/README.md +++ b/quest/m1/ts-reliability/README.md @@ -106,7 +106,6 @@ remain with their current quests. - [Section placement](/quest/m1/ts-reliability/section-placement.md) - sections share the PCR placement clock instead of the video PTS - [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 deleted file mode 100644 index f7befebe02..0000000000 --- a/quest/m1/ts-reliability/audio-lead.md +++ /dev/null @@ -1,27 +0,0 @@ -# [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/rs/moq-mux/src/container/ts/export.rs b/rs/moq-mux/src/container/ts/export.rs index 4124b20a6a..b1c028da39 100644 --- a/rs/moq-mux/src/container/ts/export.rs +++ b/rs/moq-mux/src/container/ts/export.rs @@ -1881,10 +1881,11 @@ where /// 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. + /// was cut or went stale 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, and with none held, everything leads the keyframe yet to arrive. 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; @@ -1906,7 +1907,7 @@ where .filter_map(|track| self.jitter.peek(&track.pid)) .map(|queued| queued.frame.timestamp) .min(); - timestamp < next.map_or(start, |next| next.max(start)) + next.is_none_or(|next| timestamp < next.max(start)) } /// Discard what has not gone out and restart the program clock. Every rendition diff --git a/rs/moq-mux/src/container/ts/export_test.rs b/rs/moq-mux/src/container/ts/export_test.rs index 97cb158f29..1d9d9c59b8 100644 --- a/rs/moq-mux/src/container/ts/export_test.rs +++ b/rs/moq-mux/src/container/ts/export_test.rs @@ -5257,6 +5257,8 @@ struct Interleave { source: crate::Source, /// The next audio frame to write. audio_index: u64, + /// Audio frames per group. + audio_group: u64, /// The paused clock's instant at media time zero, set by the first [`Self::at`]. start: Option, } @@ -5313,6 +5315,7 @@ impl Interleave { video, audio, audio_index: 0, + audio_group: AUDIO_GROUP, start: None, } } @@ -5351,20 +5354,25 @@ impl Interleave { /// polling `export` after each. fn audio_until(&mut self, until: u64, export: &mut Export, out: &mut Vec) { while self.audio_index * AUDIO_US < until { - let index = self.audio_index; - self.audio - .write(Frame { - timestamp: Timestamp::from_micros(index * AUDIO_US).unwrap(), - duration: None, - payload: Bytes::from_iter((0..180u16).map(|i| (i ^ index as u16) as u8)), - keyframe: index.is_multiple_of(AUDIO_GROUP), - }) - .unwrap(); - self.audio_index += 1; + self.audio_frame(); out.extend(poll_frames(export)); } } + /// Write the next audio frame. + fn audio_frame(&mut self) { + let index = self.audio_index; + self.audio + .write(Frame { + timestamp: Timestamp::from_micros(index * AUDIO_US).unwrap(), + duration: None, + payload: Bytes::from_iter((0..180u16).map(|i| (i ^ index as u16) as u8)), + keyframe: index.is_multiple_of(self.audio_group), + }) + .unwrap(); + self.audio_index += 1; + } + fn finish(&mut self) { self.video.finish().unwrap(); self.audio.finish().unwrap(); @@ -5605,7 +5613,9 @@ async fn tune_in_does_not_wait_on_dropped_audio() { let mut export = rig.export(delay).await; let mut out = Vec::new(); rig.audio_until(3 * GOP * VIDEO_US, &mut export, &mut out); - for tick in GOP..2 * GOP { + // Under a delay's worth of the group, read at once: the whole group would leave its + // keyframe stale, and the audio would wait on the next. + for tick in GOP..GOP + 6 { rig.video(tick); } out.extend(poll_frames(&mut export)); @@ -5768,6 +5778,50 @@ async fn audio_dropped_ahead_of_a_moved_start_is_not_started() { assert!(stats.trimmed > 0); } +/// A joiner's start group older than the delay goes stale at acquisition and is dropped +/// whole, so the video holds nothing until its next keyframe lands. The audio due meanwhile +/// presents after the dropped keyframe but before the next one, and is dropped rather than +/// leading it. +#[tokio::test(start_paused = true)] +async fn a_stale_start_group_holds_the_audio_behind_the_next_keyframe() { + let delay = Duration::from_millis(100); + let mut rig = Interleave::new().await; + // A group per audio frame, so the joiner's audio starts at the live edge, fresher than the + // video's start group. + rig.audio_group = 1; + // Join 160 ms into the first group: older than the delay, with most of it still to come. + let join = 4; + for tick in 0..join { + rig.at(tick * VIDEO_US).await; + rig.video(tick); + while rig.audio_index * AUDIO_US < (tick + 1) * VIDEO_US { + rig.audio_frame(); + } + } + let mut export = open_export(rig.source.clone()).await.unwrap().with_delay(delay); + let mut out = Vec::new(); + for tick in join..3 * GOP { + rig.at(tick * VIDEO_US).await; + rig.video(tick); + out.extend(poll_frames(&mut export)); + rig.audio_until((tick + 1) * VIDEO_US, &mut export, &mut out); + } + 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 stale start group is dropped"); + 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 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-srt/src/ts.rs b/rs/moq-srt/src/ts.rs index 7ccaa59ace..361ae67a05 100644 --- a/rs/moq-srt/src/ts.rs +++ b/rs/moq-srt/src/ts.rs @@ -658,7 +658,13 @@ mod tests { let expected_cue = cue.payload; assert_eq!(expected_cue[0], 0xFC, "a verbatim splice_info_section (table_id 0xFC)"); - let mut subscriber = Subscriber::new(&origin.consume(), "ingest", Options::default()) + // The egress joins the clip's last group, read at once and two seconds long. Under the + // default delay its keyframe is stale, and the egress waits on the next one. + let options = Options { + latency: Duration::from_secs(3), + ..Default::default() + }; + let mut subscriber = Subscriber::new(&origin.consume(), "ingest", options) .await .unwrap() .expect("the ingest broadcast is available for SRT egress");