mux: harden audio discontinuity handling (audit follow-up)

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.
This commit is contained in:
Matthew Jackson
2026-06-29 12:50:52 -07:00
parent be08e3938b
commit 9e6af4a729
3 changed files with 92 additions and 13 deletions
+63 -5
View File
@@ -60,10 +60,6 @@ impl Ac3Parser {
impl CodecParser for Ac3Parser { impl CodecParser for Ac3Parser {
fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> { fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> {
if pes.data.is_empty() {
return Vec::new();
}
// B1: a concealed/lost gap means the bytes held in `buf` are a TRUNCATED // 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. Appending the post-gap bytes would splice them into one corrupt
// frame (wrong frame_size, bad CRC → "exponent out of range" / garbage). // 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 // gap instead of a frankenstein frame. (The video parsers carry this via
// the ResyncGate; audio has no inter-frame refs, so dropping the spliced // the ResyncGate; audio has no inter-frame refs, so dropping the spliced
// partial is the whole fix.) // 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 { if pes.discontinuity {
self.buf.clear(); self.buf.clear();
} }
if pes.data.is_empty() {
return Vec::new();
}
// Base PTS for the FIRST frame emitted from this call. Each subsequent // 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 // 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 // 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 // PTS per frame instead of the same PES timestamp on all of them (which
// collapses their timecodes and drifts A/V). // 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 // Prepend leftover from previous PES
self.buf.extend_from_slice(&pes.data); 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] #[test]
fn skip_garbage_before_sync() { fn skip_garbage_before_sync() {
let mut parser = Ac3Parser::new(); let mut parser = Ac3Parser::new();
+22 -4
View File
@@ -123,21 +123,39 @@ const PTS_UNSET: i64 = -1;
impl CodecParser for DtsParser { impl CodecParser for DtsParser {
fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> { fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> {
if pes.data.is_empty() {
return Vec::new();
}
// B1: a concealed/lost gap means the buffered DTS access unit is // B1: a concealed/lost gap means the buffered DTS access unit is
// TRUNCATED. Splicing post-gap bytes onto it corrupts the core/extension // TRUNCATED. Splicing post-gap bytes onto it corrupts the core/extension
// framing (→ "Failed to decode block code(s)" / "Invalid data found"). // 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 // 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 // unit. (Audio has no inter-frame refs — dropping the spliced partial is
// the whole fix; the video ResyncGate handles video.) // 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 { if pes.discontinuity {
self.buf.clear(); self.buf.clear();
self.pts_marks.clear(); self.pts_marks.clear();
self.pending_pts = PTS_UNSET; 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 // 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 // (sync 0x7FFE8001) followed by one or more DTS extension substreams
+7 -4
View File
@@ -126,19 +126,22 @@ enum Ac3Size {
impl CodecParser for TrueHdParser { impl CodecParser for TrueHdParser {
fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> { fn parse(&mut self, pes: &PesPacket) -> Vec<Frame> {
if pes.data.is_empty() {
return Vec::new();
}
// B1: a concealed/lost gap means the buffered TrueHD AU is TRUNCATED. // B1: a concealed/lost gap means the buffered TrueHD AU is TRUNCATED.
// Splicing post-gap bytes onto it corrupts the AU framing (→ "Invalid // Splicing post-gap bytes onto it corrupts the AU framing (→ "Invalid
// data found") and strands the PTS cadence (the non-monotonic audio-DTS // 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 // 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 // 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.) // 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 { if pes.discontinuity {
self.buf.clear(); 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 // 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 // is mid-assembly in `buf`. TrueHD access units span PES packets; a PES