audit: clamp BD format fallback, running GOP byte counter, O(1) DTS marks
Round-8 findings from the 10-phase release audit: - detect_disc_format's BDMV fallback passed detect_format's result through unchanged, so an SD bonus/menu title could tag a BD-tree disc as DVD (mis-sizing the ECC sweep) — violating its own "never below Blu-ray" invariant. Clamp anything but UHD up to Blu-ray. - Track the MPEG-2 GOP byte total incrementally instead of re-summing the whole gop_buf on every pushed picture (was O(pictures²) on any MPEG-2 disc, not just adversarial input). - Back the DTS pts_marks deque with a VecDeque so the over-cap prune is an O(1) pop_front, not an O(n) Vec::remove(0). - Add a test exercising parse_stream_id_extension's PTS/DTS skip branches (the real AU-opening 0xFD video PES path) — previously untested.
This commit is contained in:
@@ -37,7 +37,7 @@ pub struct DtsParser {
|
||||
/// timestamp instead of the later PES's. Offsets are kept relative to the
|
||||
/// current `buf` start and rebased whenever bytes are drained from the
|
||||
/// front.
|
||||
pts_marks: Vec<(usize, i64)>,
|
||||
pts_marks: std::collections::VecDeque<(usize, i64)>,
|
||||
/// The `front_pts` of the PREVIOUS emitted access unit. When the current
|
||||
/// AU's `front_pts` differs, it began a new PES → re-base to it. When it is
|
||||
/// unchanged, this AU shares the previous AU's PES → advance one frame
|
||||
@@ -62,7 +62,7 @@ impl DtsParser {
|
||||
Self {
|
||||
buf: Vec::with_capacity(32768),
|
||||
pending_pts: 0,
|
||||
pts_marks: Vec::new(),
|
||||
pts_marks: std::collections::VecDeque::new(),
|
||||
last_front_pts: PTS_UNSET,
|
||||
next_pts_ns: PTS_UNSET,
|
||||
}
|
||||
@@ -200,7 +200,7 @@ impl CodecParser for DtsParser {
|
||||
// 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()
|
||||
.back()
|
||||
.map(|&(_, p)| p)
|
||||
.filter(|&p| p >= 0)
|
||||
.unwrap_or(if self.pending_pts >= 0 {
|
||||
@@ -236,14 +236,14 @@ impl CodecParser for DtsParser {
|
||||
// (see `front_pts`), so an AU whose core arrived in an earlier PES keeps
|
||||
// that core's timestamp even when its extensions / the following core
|
||||
// arrive (with a later PTS) in this same parse() call.
|
||||
self.pts_marks.push((self.buf.len(), pts_ns));
|
||||
self.pts_marks.push_back((self.buf.len(), pts_ns));
|
||||
// Backstop: a run of zero-length (sub-header-only) PES packets that each
|
||||
// carry a PTS grows no buffer bytes, so `drain_front` (which prunes marks)
|
||||
// never runs. Bound the deque directly — drop the oldest, which belongs to
|
||||
// an already-emitted or lost AU — so hostile PS input can't accumulate
|
||||
// marks without bound.
|
||||
if self.pts_marks.len() > MAX_PTS_MARKS {
|
||||
self.pts_marks.remove(0);
|
||||
self.pts_marks.pop_front();
|
||||
}
|
||||
self.buf.extend_from_slice(&pes.data);
|
||||
|
||||
|
||||
@@ -110,6 +110,10 @@ pub struct Mpeg2Parser {
|
||||
/// without ever reordering emitted blocks (B-frames keep decode order; only
|
||||
/// their PTS is lower).
|
||||
gop_buf: Vec<BufferedPicture>,
|
||||
/// Running total of `data` bytes buffered in `gop_buf` — the byte-cap counter,
|
||||
/// incremented on each push and reset when the GOP flushes. Avoids re-summing
|
||||
/// the whole buffer per picture (which would be O(pictures²)).
|
||||
gop_bytes: usize,
|
||||
/// Total field-display periods of all frames already emitted, in display
|
||||
/// order — the running base for each new frame's display time.
|
||||
emitted_fields: u64,
|
||||
@@ -148,6 +152,7 @@ impl Mpeg2Parser {
|
||||
frame_duration_ns: 0,
|
||||
progressive_sequence: false,
|
||||
gop_buf: Vec::new(),
|
||||
gop_bytes: 0,
|
||||
emitted_fields: 0,
|
||||
origin_pts_ns: None,
|
||||
}
|
||||
@@ -241,6 +246,7 @@ impl Mpeg2Parser {
|
||||
if gop_boundary && !self.gop_buf.is_empty() {
|
||||
self.flush_gop(out);
|
||||
}
|
||||
self.gop_bytes += data.len();
|
||||
self.gop_buf.push(BufferedPicture {
|
||||
tr,
|
||||
info,
|
||||
@@ -262,8 +268,7 @@ impl Mpeg2Parser {
|
||||
// unbounded. Force-flush a pathologically long run as its own GOP —
|
||||
// bounded by BOTH the frame count and the total buffered bytes, so a
|
||||
// crafted stream of few-but-huge pictures cannot over-allocate either.
|
||||
let gop_bytes: usize = self.gop_buf.iter().map(|p| p.frame.data.len()).sum();
|
||||
if self.gop_buf.len() >= MAX_PENDING_FRAMES || gop_bytes >= MAX_PENDING_BYTES {
|
||||
if self.gop_buf.len() >= MAX_PENDING_FRAMES || self.gop_bytes >= MAX_PENDING_BYTES {
|
||||
self.flush_gop(out);
|
||||
}
|
||||
}
|
||||
@@ -281,6 +286,8 @@ impl Mpeg2Parser {
|
||||
if n == 0 {
|
||||
return;
|
||||
}
|
||||
// The GOP is fully drained below; reset the running byte counter.
|
||||
self.gop_bytes = 0;
|
||||
let field_period = self.frame_duration_ns / 2;
|
||||
if field_period <= 0 {
|
||||
// No sequence header / frame rate yet (malformed lead-in): emit in
|
||||
|
||||
@@ -980,6 +980,44 @@ mod tests {
|
||||
assert_eq!(parsed.dvd_pid(), Some(0xFD55));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_extended_stream_id_skips_pts_and_dts_before_the_extension() {
|
||||
// The common real case: an AU-opening 0xFD VC-1 video PES carries a PTS
|
||||
// (and often DTS) in the optional-header region, which the parser must
|
||||
// SKIP (PTS +5, DTS +5) to reach the PES_extension → stream_id_extension.
|
||||
// Both branches were previously untested (flags2 there was 0x01, skipping
|
||||
// everything), so an off-by-one in the skip would silently misroute video.
|
||||
let build = |flags2: u8, skip: usize| {
|
||||
let mut pkt = vec![0x00, 0x00, 0x01, EXTENDED_STREAM_ID];
|
||||
// optional region: `skip` bytes (PTS/DTS placeholders) then
|
||||
// ext_flags=0x01, field_len=0x81, stream_id_extension=0x55.
|
||||
let mut opt = vec![0xFFu8; skip];
|
||||
opt.extend_from_slice(&[0x01, 0x81, 0x55]);
|
||||
let es = [0xDEu8, 0xAD];
|
||||
let len = (3 + opt.len() + es.len()) as u16;
|
||||
pkt.extend_from_slice(&len.to_be_bytes());
|
||||
// flags1=0x80, flags2, header_data_length = optional region length.
|
||||
pkt.extend_from_slice(&[0x80, flags2, opt.len() as u8]);
|
||||
pkt.extend_from_slice(&opt);
|
||||
pkt.extend_from_slice(&es);
|
||||
pkt
|
||||
};
|
||||
// PTS present (pts_dts bits = 10 → flags2 0x80) + PES_extension (0x01).
|
||||
let pts_only = parse_pes_packet(&build(0x81, 5)).expect("parses");
|
||||
assert_eq!(
|
||||
pts_only.sub_stream_id,
|
||||
Some(0x55),
|
||||
"extension found after skipping a 5-byte PTS"
|
||||
);
|
||||
// PTS+DTS present (pts_dts bits = 11 → flags2 0xC0) + PES_extension.
|
||||
let pts_dts = parse_pes_packet(&build(0xC1, 10)).expect("parses");
|
||||
assert_eq!(
|
||||
pts_dts.sub_stream_id,
|
||||
Some(0x55),
|
||||
"extension found after skipping a 10-byte PTS+DTS"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_extended_stream_id_without_extension_yields_no_sub_id() {
|
||||
// A 0xFD PES that declares no PES_extension (flags2=0x00) can't carry a
|
||||
|
||||
Reference in New Issue
Block a user