From 9e6af4a7294ee4a6463311044beeb097516d34da Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Mon, 29 Jun 2026 12:50:52 -0700 Subject: [PATCH] mux: harden audio discontinuity handling (audit follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defensive hardenings from the post-fix audit (vs FFmpeg/GStreamer): 1. Move the `pes.discontinuity` partial-drop ABOVE the empty-data guard in all three audio parsers (ac3/dts/truehd), so a discontinuity signal can never be stranded by an empty post-gap PES. The demuxer only emits non-empty PES today; this is defense-in-depth for any future caller. 2. A PES with no PTS must not reset the timeline to 0. ac3 now carries `flush_pts_ns`, dts continues from the most recent known base; truehd already kept its running cadence on a None PTS. Matches OSS behavior (PTS rebases off the next PES that actually carries a PTS). Adds an ac3 regression test (empty-payload discontinuity PES still drops the stranded partial). Loss accounting was reviewed: TS-demux CC-gaps are NOT counted toward lost_video_secs / abort (that is sector-based via DiscStream::errors / mapfile bytes_unreadable), so a source splice never inflates loss — no gating needed there. --- src/mux/codec/ac3.rs | 68 ++++++++++++++++++++++++++++++++++++++--- src/mux/codec/dts.rs | 26 +++++++++++++--- src/mux/codec/truehd.rs | 11 ++++--- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/mux/codec/ac3.rs b/src/mux/codec/ac3.rs index 7aca241..c7d8e2c 100644 --- a/src/mux/codec/ac3.rs +++ b/src/mux/codec/ac3.rs @@ -60,10 +60,6 @@ impl Ac3Parser { impl CodecParser for Ac3Parser { fn parse(&mut self, pes: &PesPacket) -> Vec { - if pes.data.is_empty() { - return Vec::new(); - } - // B1: a concealed/lost gap means the bytes held in `buf` are a TRUNCATED // frame. Appending the post-gap bytes would splice them into one corrupt // frame (wrong frame_size, bad CRC → "exponent out of range" / garbage). @@ -71,16 +67,29 @@ impl CodecParser for Ac3Parser { // gap instead of a frankenstein frame. (The video parsers carry this via // the ResyncGate; audio has no inter-frame refs, so dropping the spliced // partial is the whole fix.) + // + // Handle the discontinuity BEFORE the empty-data guard so the signal can + // never be stranded by an empty post-gap PES (the demuxer only emits + // non-empty PES today; this is defensive for any future caller). if pes.discontinuity { self.buf.clear(); } + if pes.data.is_empty() { + return Vec::new(); + } // Base PTS for the FIRST frame emitted from this call. Each subsequent // frame in the same call advances by the previous frame's duration, so a // PES that carries several AC-3 frames stamps a monotonically increasing // PTS per frame instead of the same PES timestamp on all of them (which // collapses their timecodes and drifts A/V). - let base_pts_ns = pes.pts.map(pts_to_ns).unwrap_or(0); + // + // A PES with no PTS (rare for audio, but legal — and the case OSS demuxers + // guard at a post-gap continuation) must NOT reset the timeline to 0; + // carry the running cadence (`flush_pts_ns` tracks the retained partial / + // last frame's PTS). The discontinuity-carrying PES is a PUSI with a PTS + // in practice, so this is defense-in-depth. + let base_pts_ns = pes.pts.map(pts_to_ns).unwrap_or(self.flush_pts_ns); // Prepend leftover from previous PES self.buf.extend_from_slice(&pes.data); @@ -559,6 +568,55 @@ mod tests { ); } + #[test] + fn empty_discontinuity_pes_still_drops_partial() { + // Defensive ordering: the discontinuity clear runs BEFORE the empty-data + // guard, so even an empty-payload discontinuity PES drops the stranded + // partial instead of leaking the signal and splicing on the next PES. + let mut parser = Ac3Parser::new(); + let frame_data = make_ac3_frame(0, 2); // 160 bytes + + // Partial first half buffered. + let pes1 = PesPacket { + source: None, + pid: 0, + pts: Some(90000), + dts: None, + data: frame_data[..80].to_vec(), + discontinuity: false, + }; + assert!(parser.parse(&pes1).is_empty()); + + // Empty-payload discontinuity PES: must still clear the partial. + let gap = PesPacket { + source: None, + pid: 0, + pts: None, + dts: None, + data: vec![], + discontinuity: true, + }; + assert!(parser.parse(&gap).is_empty(), "empty PES emits nothing"); + + // A fresh whole frame (no discontinuity now): if the partial had leaked, + // this would splice into a frankenstein; instead it emits cleanly. + let fresh = make_ac3_frame(0, 2); + let pes2 = PesPacket { + source: None, + pid: 0, + pts: Some(99000), + dts: None, + data: fresh.clone(), + discontinuity: false, + }; + let frames = parser.parse(&pes2); + assert_eq!(frames.len(), 1, "one clean frame, partial was dropped"); + assert_eq!( + frames[0].data, fresh, + "no splice — partial did not leak past the empty gap PES" + ); + } + #[test] fn skip_garbage_before_sync() { let mut parser = Ac3Parser::new(); diff --git a/src/mux/codec/dts.rs b/src/mux/codec/dts.rs index 8c06479..ecd1da7 100644 --- a/src/mux/codec/dts.rs +++ b/src/mux/codec/dts.rs @@ -123,21 +123,39 @@ const PTS_UNSET: i64 = -1; impl CodecParser for DtsParser { fn parse(&mut self, pes: &PesPacket) -> Vec { - if pes.data.is_empty() { - return Vec::new(); - } // B1: a concealed/lost gap means the buffered DTS access unit is // TRUNCATED. Splicing post-gap bytes onto it corrupts the core/extension // framing (→ "Failed to decode block code(s)" / "Invalid data found"). // Drop the partial AU and its PTS marks; the next PES re-bases a fresh // unit. (Audio has no inter-frame refs — dropping the spliced partial is // the whole fix; the video ResyncGate handles video.) + // + // Handle the discontinuity BEFORE the empty-data guard so the signal can + // never be stranded by an empty post-gap PES (defensive; the demuxer only + // emits non-empty PES today). if pes.discontinuity { self.buf.clear(); self.pts_marks.clear(); self.pending_pts = PTS_UNSET; } - let pts_ns = pes.pts.map(pts_to_ns).unwrap_or(0); + if pes.data.is_empty() { + return Vec::new(); + } + // A PES with no PTS (rare for audio, but legal — the case OSS demuxers + // guard at a post-gap continuation) must NOT reset the timeline to 0; + // continue from the most recent known base. Defense-in-depth: the + // discontinuity-carrying PES is a PUSI with a PTS in practice. + let pts_ns = pes.pts.map(pts_to_ns).unwrap_or_else(|| { + self.pts_marks + .last() + .map(|&(_, p)| p) + .filter(|&p| p >= 0) + .unwrap_or(if self.pending_pts >= 0 { + self.pending_pts + } else { + 0 + }) + }); // On Blu-ray, a DTS-HD MA/HRA access unit is a DTS core frame // (sync 0x7FFE8001) followed by one or more DTS extension substreams diff --git a/src/mux/codec/truehd.rs b/src/mux/codec/truehd.rs index eea2a7d..fa84c52 100644 --- a/src/mux/codec/truehd.rs +++ b/src/mux/codec/truehd.rs @@ -126,19 +126,22 @@ enum Ac3Size { impl CodecParser for TrueHdParser { fn parse(&mut self, pes: &PesPacket) -> Vec { - if pes.data.is_empty() { - return Vec::new(); - } - // B1: a concealed/lost gap means the buffered TrueHD AU is TRUNCATED. // Splicing post-gap bytes onto it corrupts the AU framing (→ "Invalid // data found") and strands the PTS cadence (the non-monotonic audio-DTS // band at gaps). Drop the partial; with `buf` now empty the PTS-base block // below re-seeds the cadence from the post-gap PES, monotonic across the // gap. (Audio has no inter-frame refs — this is the whole audio fix.) + // + // Handle the discontinuity BEFORE the empty-data guard so the signal can + // never be stranded by an empty post-gap PES (defensive; the demuxer only + // emits non-empty PES today). if pes.discontinuity { self.buf.clear(); } + if pes.data.is_empty() { + return Vec::new(); + } // Capture the PTS base ONLY at an access-unit boundary, i.e. when no AU // is mid-assembly in `buf`. TrueHD access units span PES packets; a PES