diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53b7b3e1..08980d6d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -214,12 +214,8 @@ jobs: freemkv-i18n = { path = "../freemkv-i18n" } EOF done - # Since freemkv 1.7.6 the `freemkv` consumer's Linux target compiles a - # GTK4 + libadwaita desktop shell, so `cargo test` on it needs the - # pkg-config dev headers the `gtk4-sys`/`glib-sys` build.rs scripts - # probe. Kept local (not in the shared reusable) — only freemkv links - # against GTK; adding the deps to every consumer would tax the six - # that don't. + # freemkv's app build (`--features gui`) compiles a GTK4 + libadwaita + # shell, so testing it needs these pkg-config dev headers. - name: Install GTK4 + libadwaita development headers (for freemkv) run: | sudo apt-get update @@ -242,6 +238,8 @@ jobs: working-directory: freemkv-engine - run: cargo test --tests working-directory: freemkv + - run: cargo test --tests --features gui + working-directory: freemkv - run: cargo test --tests working-directory: autorip - run: cargo test --tests diff --git a/src/drive/mod.rs b/src/drive/mod.rs index e975fb72..6bf2c0cd 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -1244,8 +1244,8 @@ impl SectorSource for Drive { /// /// Opens each candidate in enumeration order and returns the first reporting /// [`DriveStatus::DiscPresent`] via [`Drive::drive_status`]; falls back to the first drive that -/// opened if none report a disc. For just listing drives without opening, use -/// `scsi::list_drives()` instead. +/// opened if none report a disc. Only one drive is held open at a time. For +/// just listing drives without opening, use `scsi::list_drives()` instead. pub fn find_drive() -> Option { #[cfg(target_os = "macos")] let candidates = crate::scsi::list_drives() @@ -1259,36 +1259,34 @@ pub fn find_drive() -> Option { .map(|(path, _)| path) .collect::>(); - let opened = - candidates - .into_iter() - .filter_map(|path| match Drive::open(std::path::Path::new(&path)) { - Ok(drive) - if !drive.drive_id.raw_inquiry.is_empty() - && (drive.drive_id.raw_inquiry[0] & 0x1F) == 0x05 => - { - Some(drive) - } - Ok(drive) => { - tracing::debug!( - target: "freemkv::drive", - path, - "skipping non-optical SCSI device during drive selection" - ); - drop(drive); - None - } - Err(error) => { - tracing::warn!( - target: "freemkv::drive", - path, - error = %error, - "drive open failed during autodetection" - ); - None - } - }); - select_drive_with_media(opened) + select_drive_with_media(candidates, |path| { + match Drive::open(std::path::Path::new(path)) { + Ok(drive) + if !drive.drive_id.raw_inquiry.is_empty() + && (drive.drive_id.raw_inquiry[0] & 0x1F) == 0x05 => + { + Some(drive) + } + Ok(drive) => { + tracing::debug!( + target: "freemkv::drive", + path, + "skipping non-optical SCSI device during drive selection" + ); + drop(drive); + None + } + Err(error) => { + tracing::warn!( + target: "freemkv::drive", + path, + error = %error, + "drive open failed during autodetection" + ); + None + } + } + }) } #[cfg(not(target_os = "macos"))] @@ -1296,21 +1294,35 @@ fn discover_drives() -> Vec<(String, DriveId)> { platform::find_drives() } -// Pick a drive preferring DiscPresent status, falling back to the first -// yielded. Split from find_drive so it's unit-testable without hardware. -fn select_drive_with_media(drives: impl Iterator) -> Option { - let mut fallback: Option = None; - for mut drive in drives { +// Prefer a drive with media, else the first that opened. Only one drive is open +// at a time (macOS allows one live handle): the fallback is released before the +// next open and reopened by path. Split from find_drive for unit tests. +fn select_drive_with_media

( + candidates: impl IntoIterator, + mut open: impl FnMut(&P) -> Option, +) -> Option { + let mut fallback: Option<(P, Option)> = None; + for path in candidates { + if let Some((_, held)) = fallback.as_mut() { + *held = None; + } + let Some(mut drive) = open(&path) else { + continue; + }; if drive.drive_status() == DriveStatus::DiscPresent { return Some(drive); } - // Remember the first drive that opened as the no-media fallback so - // single-drive / status-unavailable setups still get a drive. if fallback.is_none() { - fallback = Some(drive); + fallback = Some((path, Some(drive))); } } - fallback + match fallback { + Some((_, Some(drive))) => Some(drive), + // Accepted edge: if the reopen fails (e.g. another process took the + // device in the gap) there is no drive to return. + Some((path, None)) => open(&path), + None => None, + } } // MODE SENSE(10) Error Recovery page -> MODE SELECT(10) payload enabling @@ -2512,9 +2524,8 @@ mod command_tests { // Drive #1 has no disc (0x00), drive #2 has a disc (0x02). The // selection must skip the empty first drive and pick the one with // media — the Windows multi-drive bug fix. - let drives = vec![drive_with_media_byte(0x00), drive_with_media_byte(0x02)]; - let picked = select_drive_with_media(drives.into_iter()).expect("a drive"); - let mut picked = picked; + let picked = select_drive_with_media([0x00u8, 0x02], |m| Some(drive_with_media_byte(*m))); + let mut picked = picked.expect("a drive"); assert_eq!( picked.drive_status(), DriveStatus::DiscPresent, @@ -2527,8 +2538,8 @@ mod command_tests { // No drive reports a disc → fall back to the FIRST opened drive so single-drive // / quirky setups still get one (historical behavior). Tag drive #1 distinctly // (TrayOpen 0x01) and confirm it, not #2 (NoDisc 0x00), is returned. - let drives = vec![drive_with_media_byte(0x01), drive_with_media_byte(0x00)]; - let mut picked = select_drive_with_media(drives.into_iter()).expect("a fallback drive"); + let picked = select_drive_with_media([0x01u8, 0x00], |m| Some(drive_with_media_byte(*m))); + let mut picked = picked.expect("a fallback drive"); assert_eq!( picked.drive_status(), DriveStatus::TrayOpen, @@ -2539,8 +2550,65 @@ mod command_tests { #[test] fn select_drive_none_when_no_drives() { // No candidates at all → None. - let empty: Vec = Vec::new(); - assert!(select_drive_with_media(empty.into_iter()).is_none()); + let empty: [u8; 0] = []; + assert!(select_drive_with_media(empty, |m| Some(drive_with_media_byte(*m))).is_none()); + } + + // Models macOS `MacScsiTransport`: only one instance may be alive per + // process; construction fails while another is open, Drop releases it. + struct ExclusiveTransport { + inner: FixedTransport, + live: std::sync::Arc, + } + + impl ScsiTransport for ExclusiveTransport { + fn execute( + &mut self, + cdb: &[u8], + direction: DataDirection, + data: &mut [u8], + timeout_ms: u32, + ) -> Result { + self.inner.execute(cdb, direction, data, timeout_ms) + } + } + + impl Drop for ExclusiveTransport { + fn drop(&mut self) { + self.live.store(false, std::sync::atomic::Ordering::SeqCst); + } + } + + fn open_exclusive( + live: &std::sync::Arc, + media_status: u8, + ) -> Option { + if live.swap(true, std::sync::atomic::Ordering::SeqCst) { + return None; // DeviceLocked: another handle is still open + } + Some(Drive::from_transport_for_test(Box::new( + ExclusiveTransport { + inner: FixedTransport { + payload: media_event_reply(media_status), + }, + live: live.clone(), + }, + ))) + } + + #[test] + fn select_drive_finds_media_behind_empty_drive_with_single_open_transport() { + // Two drives, first empty (TrayOpen), second has a disc, on a + // transport that allows one live handle. The empty drive must not + // be held open while the second is tried. + let live = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let picked = select_drive_with_media([0x01u8, 0x02], |m| open_exclusive(&live, *m)); + let mut picked = picked.expect("a drive"); + assert_eq!( + picked.drive_status(), + DriveStatus::DiscPresent, + "must open the second drive and pick its disc" + ); } // ── drive_status branch coverage (GET EVENT STATUS byte 5) ────── diff --git a/src/mux/codec/adts.rs b/src/mux/codec/adts.rs index c6764a5d..9e2ee42c 100644 --- a/src/mux/codec/adts.rs +++ b/src/mux/codec/adts.rs @@ -53,6 +53,7 @@ fn adts_verdict(data: &[u8]) -> AdtsVerdict { pub struct AdtsParser { frames: AudioFrames, config: Option>, + config_changes: u64, } impl Default for AdtsParser { @@ -66,6 +67,7 @@ impl AdtsParser { Self { frames: AudioFrames::new("aac"), config: None, + config_changes: 0, } } pub fn dropped_frames(&self) -> u64 { @@ -79,6 +81,8 @@ impl AdtsParser { impl CodecParser for AdtsParser { fn parse(&mut self, pes: &PesPacket) -> Vec { let config = &mut self.config; + let changes = &mut self.config_changes; + let pid = pes.pid; self.frames.parse(pes, 7, |data| { if !matches!(adts_verdict(data), AdtsVerdict::Valid) { return None; @@ -86,14 +90,23 @@ impl CodecParser for AdtsParser { let rate_index = (data[2] >> 2) & 15; let object_type = (data[2] >> 6) + 1; let channels = ((data[2] & 1) << 2) | (data[3] >> 6); - // MPEG-4 AudioSpecificConfig replaces the ADTS transport header in - // Matroska. The raw AAC payload must not retain that header or CRC. - config.get_or_insert_with(|| { - vec![ - (object_type << 3) | (rate_index >> 1), - (rate_index << 7) | (channels << 3), - ] - }); + // The ASC replaces the ADTS header (payload carries no header/CRC). + // CodecPrivate is fixed per track: a later config change keeps the + // first ASC and is counted. + let asc = [ + (object_type << 3) | (rate_index >> 1), + (rate_index << 7) | (channels << 3), + ]; + match config { + None => *config = Some(asc.to_vec()), + Some(first) if first[..] != asc => { + if *changes == 0 { + tracing::warn!(target: "mux", pid, "AAC config changed mid-stream; keeping the first"); + } + *changes += 1; + } + Some(_) => {} + } Some(Header { bytes: (usize::from(data[3] & 3) << 11) | (usize::from(data[4]) << 3) @@ -111,6 +124,9 @@ impl CodecParser for AdtsParser { fn codec_private(&self) -> Option> { self.config.clone() } + fn config_changes(&self) -> u64 { + self.config_changes + } } #[cfg(test)] @@ -330,6 +346,27 @@ mod tests { assert_eq!(p.codec_private(), Some(vec![0x12, 0x10])); } + // Matroska CodecPrivate is fixed per track: a later ADTS header with a + // different config keeps the first ASC and is counted, not silently adopted. + #[test] + fn mid_stream_config_change_keeps_first_config_and_is_counted() { + let stereo = adts_frame(20); + let mut surround = adts_frame(20); + surround[2] = (surround[2] & !1) | 1; // channel_configuration = 6 (5.1) + surround[3] = (surround[3] & 0x3F) | (2 << 6); + let mut p = AdtsParser::new(); + p.parse(&make_pes(stereo.clone(), Some(0))); + assert_eq!(p.config_changes(), 0); + let f = p.parse(&make_pes(surround.clone(), Some(9000))); + assert_eq!(f.len(), 1, "the frame is still emitted"); + assert_eq!(p.codec_private(), Some(vec![0x12, 0x10]), "first ASC kept"); + assert_eq!(p.config_changes(), 1); + p.parse(&make_pes(surround, Some(18000))); + assert_eq!(p.config_changes(), 2, "every mismatching frame counts"); + p.parse(&make_pes(stereo, Some(27000))); + assert_eq!(p.config_changes(), 2); + } + #[test] fn crc_bytes_are_removed_with_transport_header() { let mut data = adts_frame(20); diff --git a/src/mux/codec/audio_frames.rs b/src/mux/codec/audio_frames.rs index 096c43de..2d31c10d 100644 --- a/src/mux/codec/audio_frames.rs +++ b/src/mux/codec/audio_frames.rs @@ -41,7 +41,7 @@ impl AudioFrames { &mut self, pes: &PesPacket, min_header: usize, - mut header: impl FnMut(&[u8]) -> Option

, + header: impl FnMut(&[u8]) -> Option
, ) -> Vec { if pes.data.is_empty() { return Vec::new(); @@ -89,6 +89,16 @@ impl AudioFrames { return Vec::new(); } self.buf.push(pes); + self.frame_buffered(min_header, header) + } + + // Frames every complete unit at the buffer front; a header callback asks to wait by + // returning `bytes` beyond what is buffered. + fn frame_buffered( + &mut self, + min_header: usize, + mut header: impl FnMut(&[u8]) -> Option
, + ) -> Vec { let mut frames = Vec::new(); let mut consumed = 0; while self.buf.len() - consumed >= min_header { @@ -130,9 +140,35 @@ impl AudioFrames { } pub fn flush(&mut self) -> Vec { - // A trailing partial syncframe is not decodable; never manufacture one. + self.flush_with(0, |_| None) + } + + // Before a discontinuity clears the buffer: frame what `header` can size with no more + // data coming (as at EOS), then drop the rest. + pub fn drain_before_gap( + &mut self, + min_header: usize, + header: impl FnMut(&[u8]) -> Option
, + ) -> Vec { + let frames = self.frame_buffered(min_header, header); + self.buf.clear(); + frames + } + + // EOS: frame what `header` can size now that no more data is coming, then drop the + // rest. A trailing partial syncframe is not decodable; never manufacture one. + pub fn flush_with( + &mut self, + min_header: usize, + header: impl FnMut(&[u8]) -> Option
, + ) -> Vec { + let frames = if min_header > 0 { + self.frame_buffered(min_header, header) + } else { + Vec::new() + }; self.buf.clear(); self.tally.log_summary(); - Vec::new() + frames } } diff --git a/src/mux/codec/dts.rs b/src/mux/codec/dts.rs index 3dfad201..85952f84 100644 --- a/src/mux/codec/dts.rs +++ b/src/mux/codec/dts.rs @@ -147,6 +147,8 @@ impl DtsParser { /// Drop `n` bytes from the front, rebasing attribution onto the new front. fn drain_front(&mut self, n: usize) { + #[cfg(test)] + DRAIN_CALLS.with(|c| c.set(c.get() + 1)); self.acc.drain(n); } @@ -166,6 +168,12 @@ impl DtsParser { } } +// Test-only count of `drain_front` calls, proving a bogus-sync resync is one drain. +#[cfg(test)] +thread_local! { + static DRAIN_CALLS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Hard cap on a buffered access unit (core + all its extension substreams). /// A DTS-HD MA frame is at most a few tens of KB; if the buffer grows past /// this without a clean boundary we resync rather than stall or balloon. @@ -187,6 +195,13 @@ const PTS_UNSET: i64 = -1; impl CodecParser for DtsParser { fn parse(&mut self, pes: &PesPacket) -> Vec { + // A PTS-less PES after a gap continues from the pre-gap projection, not 0. Best + // effort: the lost span is unknown, so such a PES lands early by the gap's length. + let gap_carry = if self.next_pts_ns != PTS_UNSET { + self.next_pts_ns + } else { + self.continuation_base_ns() + }; // B1: a concealed/lost gap means the buffered DTS AU is TRUNCATED; splicing // post-gap bytes onto it corrupts the framing. Drop the partial AU and its // PTS marks (DTS frames independently resync, unlike TrueHD/MLP). @@ -205,7 +220,13 @@ impl CodecParser for DtsParser { // timeline to 0 — continue from the most recent known base. let pts_ns = super::pesbuf::PesFacts::of(pes) .presentation_ns() - .unwrap_or_else(|| self.continuation_base_ns()); + .unwrap_or_else(|| { + if pes.discontinuity { + gap_carry + } else { + self.continuation_base_ns() + } + }); // Blu-ray DTS-HD MA/HRA: core frame + extension substreams (lossless data) // arrive in SEPARATE, later PES packets, so assemble core-to-next-core @@ -224,9 +245,10 @@ impl CodecParser for DtsParser { let mut frames = Vec::new(); loop { - // Resync to the first core sync; drop any leading junk. - let Some(start) = find_sync(self.acc.as_slice(), &DTS_CORE_SYNC) else { - // No core sync at all yet — keep at most a 3-byte tail so a + // Resync to the first candidate core sync; drop leading junk and any run of + // bogus (implausibly sized) syncs in ONE drain rather than 4 bytes at a time. + let Some(start) = first_core_candidate(self.acc.as_slice(), 0) else { + // No candidate core sync yet — keep at most a 3-byte tail so a // sync split across PES packets can still be found next time. if self.acc.len() > 3 { let tail = self.acc.len() - 3; @@ -250,15 +272,8 @@ impl CodecParser for DtsParser { if self.acc.len() < CORE_HEADER_MIN_BYTES { break; } + // Plausible by construction: `first_core_candidate` skipped bogus sizes. let core_size = dts_core_frame_size(self.acc.as_slice()); - // A real core frame is at least MIN_CORE_FRAME_BYTES (96, ETSI floor); - // a smaller size came from a false sync, so reject it (drain 4 bytes, - // resync) rather than close the AU at a junk boundary. - if !(MIN_CORE_FRAME_BYTES..=MAX_AU_BYTES).contains(&core_size) { - // Bogus core sync — skip past it and resync. - self.drain_front(4); - continue; - } if self.acc.len() < core_size { break; // core frame not fully buffered yet — wait } @@ -378,37 +393,80 @@ impl DtsParser { } } -/// Offset where the FINAL access unit ends at end-of-stream, dropping a truncated trailing -/// core. Trailing DTS-HD extension substreams belong to this AU and are skipped precisely -/// (kept); a trailing CORE sync — complete or truncated — begins a NEW AU that never closed -/// during `parse()`, so the AU ends there. A tail too short to identify, or non-sync garbage, -/// is kept with the AU. +/// Offset where the FINAL access unit ends at end-of-stream, mirroring `next_core_boundary`. +/// Complete extension substreams are kept; a plausible or truncated trailing core begins a +/// NEW AU that never closed, so the AU ends there; garbage or a truncated extension leaves +/// the core alone. fn final_au_end(buf: &[u8], core_size: usize) -> usize { - let mut pos = core_size; + let mut pos = exss_start(buf, core_size); loop { - if buf.len().saturating_sub(pos) < SYNCWORD_BYTES { - // Fewer than a syncword of trailing bytes: nothing identifiable — - // keep them with this AU (the same 3-byte-tail tolerance as parse). + let tail = &buf[pos..]; + if tail.is_empty() { return buf.len(); } + if tail.len() < SYNCWORD_BYTES { + // A core-sync prefix is a truncated new AU (dropped); anything else is a + // truncated extension or garbage, which leaves the core alone. + return if DTS_CORE_SYNC.starts_with(tail) { + pos + } else { + core_size + }; + } if buf[pos..].starts_with(&DTS_HD_EXT_SYNC) { match exss_frame_size(&buf[pos..]) { // A fully-buffered extension substream belongs to this AU. Some(sz) if sz >= SYNCWORD_BYTES && buf.len() >= pos + sz => pos += sz, - // Truncated/unsizeable extension at EOS: still part of this AU. - _ => return buf.len(), + // Sized but truncated at EOS: a decoder would read past it; core only. + Some(sz) if sz >= SYNCWORD_BYTES => return core_size, + // Unsizeable: scan for the next core, as parse() does. + _ => return first_core_candidate(buf, pos).unwrap_or(buf.len()), } } else if buf[pos..].starts_with(&DTS_CORE_SYNC) { - // A trailing core (truncated — a complete one would have closed the - // AU during parse) is a new AU that never completed: drop it. - return pos; + // A trailing core (truncated — a complete one would have closed the AU during + // parse) is a new AU that never completed: drop it. Plausibility as in + // `next_core_boundary`: a false sync stays with this AU. + return first_core_candidate(buf, pos).unwrap_or(buf.len()); } else { - // Non-sync garbage after the core is not a further AU; keep it. - return buf.len(); + // Garbage at the extension boundary: parse() emits the core alone. + return core_size; } } } +// Where extensions after a core begin: an EXSS is 4-byte aligned after a core whose size is +// not a multiple of 4 (as ffmpeg's dcadec frames it), so skip up to 3 padding bytes to it. +fn exss_start(buf: &[u8], core_size: usize) -> usize { + let aligned = core_size.next_multiple_of(4); + let padded_exss = buf + .get(aligned..) + .is_some_and(|t| t.starts_with(&DTS_HD_EXT_SYNC)); + let direct = buf + .get(core_size..) + .is_some_and(|t| t.starts_with(&DTS_CORE_SYNC) || t.starts_with(&DTS_HD_EXT_SYNC)); + if padded_exss && !direct { + aligned + } else { + core_size + } +} + +// First core sync at or after `from` whose header is truncated (undecidable) or whose size is +// plausible; bogus syncs before it are skipped in one pass. +fn first_core_candidate(buf: &[u8], from: usize) -> Option { + let mut from = from; + while let Some(rel) = find_sync(&buf[from..], &DTS_CORE_SYNC) { + let pos = from + rel; + if buf.len() - pos < CORE_HEADER_MIN_BYTES + || (MIN_CORE_FRAME_BYTES..=MAX_AU_BYTES).contains(&dts_core_frame_size(&buf[pos..])) + { + return Some(pos); + } + from = pos + SYNCWORD_BYTES; + } + None +} + fn find_sync(data: &[u8], pattern: &[u8; 4]) -> Option { if data.len() < 4 { return None; @@ -483,7 +541,11 @@ fn exss_frame_size(buf: &[u8]) -> Option { // skipped PRECISELY by declared size, so a false core sync in XLL payload can't be mistaken for // the boundary. fn next_core_boundary(buf: &[u8], core_size: usize) -> NextCore { - let mut pos = core_size; + // Up to 3 alignment bytes may precede the EXSS; decide only once they are buffered. + if buf.len() < core_size.next_multiple_of(4) + SYNCWORD_BYTES { + return NextCore::NeedMore; + } + let mut pos = exss_start(buf, core_size); loop { if buf.len() < pos + SYNCWORD_BYTES { return NextCore::NeedMore; // need a syncword to identify the next chunk @@ -935,6 +997,131 @@ mod tests { assert_eq!(tail[0].data.len(), 512); } + // A fully-buffered trailing core sync whose size is implausible is not a new AU in + // parse() (next_core_boundary rejects it), so EOS must keep it with the AU too. + #[test] + fn implausible_trailing_core_sync_is_kept_at_eos() { + let mut buf = make_dts_core(512); + buf.extend_from_slice(&make_exss(64, None)); + let mut fake = make_dts_core(17); // fsize decodes below MIN_CORE_FRAME_BYTES + fake.resize(40, 0x11); + buf.extend_from_slice(&fake); + assert!(matches!(next_core_boundary(&buf, 512), NextCore::None)); + assert_eq!(final_au_end(&buf, 512), buf.len()); + } + + // An unsizeable extension (declared size < a syncword) makes parse() fall back to + // scanning for the next core; EOS must do the same and drop a truncated next core. + #[test] + fn truncated_core_after_unsizeable_ext_is_dropped_at_eos() { + let mut ext = make_exss(64, None); + ext[6] &= 0xE0; // nuExtSSFsize = 0 -> size 1: unsizeable + ext[7] = 0; + ext[8] = 0; + let mut buf = make_dts_core(512); + buf.extend_from_slice(&ext); + let trunc = buf.len(); + buf.extend_from_slice(&DTS_CORE_SYNC); + buf.extend_from_slice(&[0x00, 0x00]); + assert_eq!(final_au_end(&buf, 512), trunc); + } + + // Garbage at the extension boundary: parse() emits the core alone, and so must EOS. + #[test] + fn garbage_after_core_is_dropped_at_eos() { + let mut buf = make_dts_core(512); + buf.extend(std::iter::repeat_n(0xAB, 300)); + assert!(matches!( + next_core_boundary(&buf, 512), + NextCore::None + | NextCore::Found { + ext_clean: false, + .. + } + )); + assert_eq!(final_au_end(&buf, 512), 512); + } + + // A run of bogus core syncs must be skipped in one drain, not one memmove per sync. + #[test] + fn bogus_sync_run_is_skipped_in_one_drain() { + let mut data = Vec::new(); + for _ in 0..200 { + data.extend_from_slice(&make_dts_core(17)); // size 17 < MIN: bogus + } + data.extend_from_slice(&make_dts_core(512)); + data.extend_from_slice(&make_dts_core(512)); + let mut parser = DtsParser::new(); + DRAIN_CALLS.with(|c| c.set(0)); + let frames = parser.parse(&make_pes(data, Some(90_000))); + assert_eq!(frames.len(), 1); + let drains = DRAIN_CALLS.with(|c| c.get()); + assert!(drains <= 3, "{drains} drains for one resync"); + } + + // Through parse(): a PTS-less PES after a discontinuity continues from the + // pre-gap projection instead of restarting the timeline at 0. + #[test] + fn pts_less_pes_after_gap_continues_the_timeline() { + let mut parser = DtsParser::new(); + let mut first = make_dts_core(512); + first.extend_from_slice(&make_dts_core(512)); + assert_eq!(parser.parse(&make_pes(first, Some(90_000))).len(), 1); + let gap = PesPacket { + discontinuity: true, + ..make_pes(make_dts_core(512), None) + }; + assert!(parser.parse(&gap).is_empty()); + let tail = parser.flush(); + assert_eq!(tail.len(), 1); + assert_eq!(tail[0].pts_ns, pts_to_ns(90_000) + DTS_CORE_DUR_NS); + } + + // A core whose size is not a multiple of 4 is padded to a 4-byte boundary before its + // EXSS (ffmpeg dcadec aligns the same way); the extension must still be kept. + #[test] + fn exss_after_unaligned_core_is_kept() { + let core = make_dts_core(2013); + let mut au = core.clone(); + au.extend_from_slice(&[0, 0, 0]); // pad to 2016 + au.extend_from_slice(&make_exss(64, None)); + let mut buf = au.clone(); + buf.extend_from_slice(&make_dts_core(512)); + assert!(matches!( + next_core_boundary(&buf, 2013), + NextCore::Found { + end: 2080, + ext_clean: true + } + )); + assert_eq!( + final_au_end(&au, 2013), + au.len(), + "EOS keeps the extension too" + ); + let f = DtsParser::new().parse(&make_pes(buf, Some(0))); + assert_eq!(f.len(), 1); + assert_eq!(f[0].data, au, "parse() keeps pad + EXSS with the core"); + } + + // At EOS a truncated extension (sized or 1-3 unidentified bytes) leaves the core alone, + // like garbage; a 1-3 byte core-sync prefix is a truncated new AU and is dropped. + #[test] + fn eos_truncated_tail_is_handled_consistently() { + let core = make_dts_core(512); + let exss = make_exss(64, None); + let mut buf = core.clone(); + buf.extend_from_slice(&exss[..30]); + assert_eq!(final_au_end(&buf, 512), 512, "truncated sized EXSS dropped"); + let mut buf = core.clone(); + buf.extend_from_slice(&[0xAB, 0xCD]); + assert_eq!(final_au_end(&buf, 512), 512, "short garbage tail dropped"); + let mut buf = core.clone(); + buf.extend_from_slice(&exss); + buf.extend_from_slice(&DTS_CORE_SYNC[..2]); + assert_eq!(final_au_end(&buf, 512), 576, "truncated next core dropped"); + } + #[test] fn truncated_trailing_core_is_dropped_at_eos() { // A complete core is held awaiting a next core, but the stream cuts off mid-way diff --git a/src/mux/codec/h264.rs b/src/mux/codec/h264.rs index 89776c8c..c69f05ee 100644 --- a/src/mux/codec/h264.rs +++ b/src/mux/codec/h264.rs @@ -206,11 +206,12 @@ impl CodecParser for H264Parser { let mut keyframe = false; // Picture coding type, MEASURED from the first coded slice's header. let mut coding_type: Option = None; - // Open-GOP promotion needs EVERY VCL slice intra, not just the first: a picture - // whose first slice is I but later slices are P/B is not a clean random-access - // point. `all_vcl_intra` holds only while every slice header parses AND codes as I/SI; `saw_vcl` guards against promoting a param-set-only access unit. + // Open-GOP promotion needs every slice of the FIRST picture intra. A later slice with + // first_mb_in_slice == 0 starts the next picture (the P second field of a 1080i + // anchor), which doesn't veto it. `saw_vcl` guards a param-set-only access unit. let mut all_vcl_intra = true; let mut saw_vcl = false; + let mut first_pic_done = false; // Did this access unit already carry each param-set type in-band? let mut emitted_sps = false; let mut emitted_pps = false; @@ -246,26 +247,32 @@ impl CodecParser for H264Parser { if nal_type == NAL_SLICE_IDR { keyframe = true; } - // Measure coding type from each slice header (§7.3.3: first_mb_in_slice, - // slice_type, both ue(v)) after unescaping EBSP (§7.3.1) — large - // first_mb_in_slice needs escaped 0x00 0x00. The FIRST slice sets the picture's `coding_type`; EVERY slice feeds `all_vcl_intra` for the open-GOP promotion below. - if nal_type == NAL_SLICE_NON_IDR || nal_type == NAL_SLICE_IDR { - saw_vcl = true; + // Slice header (§7.3.3: first_mb_in_slice, slice_type, both ue(v)), read + // after unescaping EBSP (§7.3.1). The first slice sets `coding_type`; each + // slice of the first picture feeds `all_vcl_intra`. + let is_slice = nal_type == NAL_SLICE_NON_IDR || nal_type == NAL_SLICE_IDR; + // Once the first picture is over (or already non-intra with its coding + // type known), no later slice can change the outcome: skip the parse. + let settled = first_pic_done || (coding_type.is_some() && !all_vcl_intra); + if is_slice && !settled { let header = unescape_ebsp_prefix(&nal[1..]); let mut br = BitReader::new(&header); - if let (Some(_first_mb), Some(slice_type)) = (br.read_ue(), br.read_ue()) { - let ct = h264_slice_coding_type(slice_type); - if coding_type.is_none() { - coding_type = ct; + match (br.read_ue(), br.read_ue()) { + (Some(0), _) if saw_vcl => first_pic_done = true, + (Some(_first_mb), Some(slice_type)) => { + let ct = h264_slice_coding_type(slice_type); + if coding_type.is_none() { + coding_type = ct; + } + if ct != Some(CodingType::I) { + all_vcl_intra = false; + } } - if ct != Some(CodingType::I) { - all_vcl_intra = false; - } - } else { // An unparseable slice header cannot be proven intra. - all_vcl_intra = false; + _ => all_vcl_intra = false, } } + saw_vcl |= is_slice; // A NAL longer than u32::MAX can't be length-prefixed in the // 4-byte field; skip it rather than mis-frame the output. // Unreachable in practice (no real AU is >4 GiB). @@ -282,9 +289,9 @@ impl CodecParser for H264Parser { return Vec::new(); } - // Open-GOP resync anchor: BD titles use open GOPs whose random-access point is a - // non-IDR I-frame. H.264 signals it with a recovery_point SEI, which this parser - // does NOT read — it infers the anchor from the slices. Without treating it as a keyframe the resync gate (`mux/resync.rs`) can miss it and drop frames to EOF. Promote only when EVERY VCL slice is intra (a picture with a P/B slice is not a random-access point) and only on the base view (`!mvc`): a dependent MVC view is never an independent anchor. + // Open-GOP anchor heuristic (recovery_point SEI §D.2.8 is authoritative, not read): + // promote when every slice of the first picture is intra, a following P field + // allowed, base view only. Else the resync gate can drop frames to EOF. if saw_vcl && all_vcl_intra && !mvc { keyframe = true; } @@ -613,15 +620,15 @@ mod tests { } // A picture whose FIRST slice is I but whose LATER slice is P/B is not a - // clean random-access point: the open-GOP promotion must require EVERY VCL - // slice to be intra, not trust the first slice alone. + // clean random-access point: the open-GOP promotion must require EVERY slice + // of the picture to be intra, not trust the first slice alone. #[test] fn a_mixed_slice_picture_with_a_p_slice_is_not_promoted() { - // AU with two coded slices: slice 1 is I (0x88, slice_type=7), slice 2 - // is P (0xC0, slice_type=0). Both NAL type 1, nal_ref_idc 3 -> 0x61. + // One picture, two slices: I at first_mb 0 (0x88, slice_type=7), then P at + // first_mb 1 (ue '010', slice_type '1', stop '1' -> 0x58). NAL type 1 -> 0x61. let au = vec![ - 0x00, 0x00, 0x01, 0x61, 0x88, // I slice - 0x00, 0x00, 0x01, 0x61, 0xC0, // P slice + 0x00, 0x00, 0x01, 0x61, 0x88, // I slice, first_mb 0 + 0x00, 0x00, 0x01, 0x61, 0x58, // P slice, first_mb 1 (same picture) 0x00, ]; let mut parser = H264Parser::new(); @@ -638,6 +645,45 @@ mod tests { ); } + // 1080i open-GOP anchor (heuristic, not spec-guaranteed; recovery_point SEI §D.2.8 is + // authoritative): an I first field with a P second field is promoted. + #[test] + fn field_coded_i_then_p_pair_is_a_keyframe() { + // I field: slices at first_mb 0 (0x88) and 1 ('010'+'0001000'+stop -> 0x42 0x20); + // P field: slices at first_mb 0 (0xC0) and 1 (0x58). NAL type 1 -> 0x61. + let au = vec![ + 0x00, 0x00, 0x01, 0x61, 0x88, // I field, first_mb 0 + 0x00, 0x00, 0x01, 0x61, 0x42, 0x20, // I field, first_mb 1 + 0x00, 0x00, 0x01, 0x61, 0xC0, // P field, first_mb 0 + 0x00, 0x00, 0x01, 0x61, 0x58, // P field, first_mb 1 + 0x00, + ]; + let frames = H264Parser::new().parse(&make_pes(au, Some(0))); + assert_eq!(frames.len(), 1); + assert_eq!( + frames[0].coding.map(|c| c.coding_type()), + Some(CodingType::I) + ); + assert!( + frames[0].keyframe, + "an I/P field pair is an open-GOP anchor" + ); + } + + // A second-field slice whose slice_type is truncated still starts a new picture + // (first_mb 0), so it must not veto the I first field. + #[test] + fn truncated_second_field_slice_type_does_not_block_promotion() { + // I field (0x88), then first_mb 0 with nothing after it (0x80: '1', then zeros). + let au = vec![0x00, 0x00, 0x01, 0x61, 0x88, 0x00, 0x00, 0x01, 0x61, 0x80]; + let frames = H264Parser::new().parse(&make_pes(au, Some(0))); + assert_eq!(frames.len(), 1); + assert!( + frames[0].keyframe, + "second-field header truncation is not a veto" + ); + } + // MVC dependent-view (Blu-ray 3D right-eye) passthrough: an intra dependent // view is not an independent random-access anchor, so it must NOT be // promoted to a keyframe even when every slice is intra. diff --git a/src/mux/codec/hevc.rs b/src/mux/codec/hevc.rs index 7e17575f..70ae43d8 100644 --- a/src/mux/codec/hevc.rs +++ b/src/mux/codec/hevc.rs @@ -60,6 +60,9 @@ fn hevc_slice_coding_type(slice_type: u32) -> Option { } } +// Number of PPS ids a stream may use: `pps_pic_parameter_set_id` is 0..=63 (H.265 §7.4.3.3). +const HEVC_MAX_PPS_COUNT: usize = 64; + // The `pps_pic_parameter_set_id` (first ue(v)) of a PPS NAL — the key under which each PPS is // stored so a slice's referenced PPS is resolved by its own id. `None` if the PPS is too short. fn hevc_pps_id(pps_nal: &[u8]) -> Option { @@ -110,11 +113,10 @@ pub struct HevcParser { cur_vps: Option>, cur_sps: Option>, cur_pps: Option>, - /// Every PPS seen, keyed by its `pps_pic_parameter_set_id`, so a slice's - /// coding type is measured with the `num_extra_slice_header_bits` of the PPS - /// it REFERENCES rather than the last-active one (multiple PPS with differing - /// values would otherwise shift the slice_type bit offset). - pps_by_id: std::collections::HashMap>, + /// `num_extra_slice_header_bits` of each PPS, indexed by `pps_pic_parameter_set_id`, so a + /// slice's coding type uses the PPS it REFERENCES, not the last-active one. Fixed size: + /// ids are 0..=63 (H.265 §7.4.3.3); an out-of-range id is ignored. + pps_num_extra: [Option; HEVC_MAX_PPS_COUNT], // Splice-aware CRA→BLA rewrite for a non-seamless BD clip boundary (first // CRA_NUT -> BLA_W_LP so NoRaslOutput discards dangling RASL). Armed by // `mark_clip_boundary` AND PTS-backstep auto-detect in `parse` — not dead code. @@ -190,6 +192,8 @@ thread_local! { #[cfg(test)] thread_local! { static FORCE_FRAMING_DESYNC: std::cell::Cell = const { std::cell::Cell::new(false) }; + // Count of access units the self-check dropped; real input must never move it. + static GUARD_DROPS: std::cell::Cell = const { std::cell::Cell::new(0) }; } // 33-bit 90kHz PTS wraps at 2^33 ticks (~26.5h). A backward step of ~2^33 is a @@ -213,7 +217,7 @@ impl HevcParser { cur_vps: None, cur_sps: None, cur_pps: None, - pps_by_id: std::collections::HashMap::new(), + pps_num_extra: [None; HEVC_MAX_PPS_COUNT], pending_clip_boundary: false, high_pts: None, pts_wrap_offset: 0, @@ -261,49 +265,44 @@ impl HevcParser { // Scans an SEI NAL for the two HDR10 payload types, capturing each the FIRST time it // appears. fn scan_sei(&mut self, nal: &[u8]) { - // Both HDR10 messages are sticky (first wins); once both are captured, - // return before `strip_emulation_prevention` (alloc+copy) — an HDR10 UHD - // stream carries a prefix SEI per AU, saving ~200k discarded copies per title. + // Both HDR10 messages are sticky (first wins); once both are captured, skip the + // walk entirely — an HDR10 UHD stream carries a prefix SEI per AU. if self.sei_mastering.is_some() && self.sei_content_light.is_some() { return; } let Some(raw) = nal.get(2..) else { return; }; - let rbsp = strip_emulation_prevention(raw); - let mut i = 0usize; - // payloadType: sum of 0xFF run + final byte. Exhausting the RBSP ends the - // walk; the remaining `let ... else break` arms below handle a TRUNCATED - // message, which is a different condition from a clean end. - while let Some(payload_type) = read_sei_ff_value(&rbsp, &mut i) { - // payloadSize: same ff-extension coding. - let Some(payload_size) = read_sei_ff_value(&rbsp, &mut i) else { + // Unescape on the fly: only a payload being captured is copied, so a stream + // lacking one HDR10 message costs no per-AU RBSP copy. + let mut rbsp = RbspBytes::new(raw); + // payloadType/payloadSize use ff-extension coding. A clean end of the RBSP ends + // the walk; a trailing 0x80 is a bogus type whose size read fails. + while let Some(payload_type) = read_sei_ff_value(&mut rbsp) { + let Some(payload_size) = read_sei_ff_value(&mut rbsp) else { break; }; let payload_size = payload_size as usize; - let Some(payload) = rbsp.get(i..i.saturating_add(payload_size)) else { - break; // truncated / malformed payload length — stop scanning + let wanted = match payload_type { + SEI_MASTERING_DISPLAY_COLOUR_VOLUME => self.sei_mastering.is_none(), + SEI_CONTENT_LIGHT_LEVEL_INFO => self.sei_content_light.is_none(), + _ => false, }; - match payload_type { - SEI_MASTERING_DISPLAY_COLOUR_VOLUME if self.sei_mastering.is_none() => { - if let Some(m) = parse_mastering_display(payload) { - self.sei_mastering = Some(m); - } + if !wanted { + if rbsp.by_ref().take(payload_size).count() < payload_size { + break; // truncated payload — stop scanning } - SEI_CONTENT_LIGHT_LEVEL_INFO if self.sei_content_light.is_none() => { - if let Some(c) = parse_content_light_level(payload) { - self.sei_content_light = Some(c); - } - } - _ => {} + continue; } - i += payload_size; - // An RBSP trailing byte (0x80) or padding zeros after the last - // message aren't another payloadType. `read_sei_ff_value` returning - // None handles end-of-buffer; a lone 0x80 is a bogus type failing the size read. - if i >= rbsp.len() { + let payload: Vec = rbsp.by_ref().take(payload_size).collect(); + if payload.len() < payload_size { break; } + if payload_type == SEI_MASTERING_DISPLAY_COLOUR_VOLUME { + self.sei_mastering = parse_mastering_display(&payload); + } else { + self.sei_content_light = parse_content_light_level(&payload); + } } } @@ -485,16 +484,11 @@ impl CodecParser for HevcParser { if coding_type.is_none() && nal_type <= NAL_VCL_MAX { // Resolve num_extra from the PPS the slice REFERENCES (by // its slice_pic_parameter_set_id), not the last-active PPS. - let pps_by_id = &self.pps_by_id; + let pps_num_extra = &self.pps_num_extra; coding_type = hevc_first_slice_coding_type( &data[nal_start..end], nal_type, - |pps_id| { - pps_by_id - .get(&pps_id) - .map(Vec::as_slice) - .and_then(hevc_num_extra_slice_header_bits) - }, + |pps_id| *pps_num_extra.get(usize::try_from(pps_id).ok()?)?, ); } @@ -519,8 +513,12 @@ impl CodecParser for HevcParser { // Store the PPS under its own id so a later slice's // coding type resolves num_extra from the PPS it // references, not merely the last-active one. - if let Some(id) = hevc_pps_id(&data[nal_start..end]) { - self.pps_by_id.insert(id, data[nal_start..end].to_vec()); + let pps = &data[nal_start..end]; + if let Some(slot) = hevc_pps_id(pps) + .and_then(|id| usize::try_from(id).ok()) + .and_then(|id| self.pps_num_extra.get_mut(id)) + { + *slot = hevc_num_extra_slice_header_bits(pps); } emitted_pps |= handle_param_set( &mut self.pps, @@ -610,6 +608,8 @@ impl CodecParser for HevcParser { // construction, but a desynced buffer would surface as "Invalid NAL unit // size". Drop it (log src) rather than emit a mis-framed access unit. if !length_prefix_tiles(&frame_data) { + #[cfg(test)] + GUARD_DROPS.with(|c| c.set(c.get() + 1)); tracing::warn!( target: "freemkv::mux::hevc", src = ?pes.source, @@ -792,13 +792,46 @@ fn strip_emulation_prevention(rbsp: &[u8]) -> Vec { out } +// RBSP bytes of an EBSP with emulation-prevention bytes dropped, yielded without copying. +// Same rule as `strip_emulation_prevention`. +struct RbspBytes<'a> { + src: &'a [u8], + pos: usize, + zeros: usize, +} + +impl<'a> RbspBytes<'a> { + fn new(src: &'a [u8]) -> Self { + Self { + src, + pos: 0, + zeros: 0, + } + } +} + +impl Iterator for RbspBytes<'_> { + type Item = u8; + fn next(&mut self) -> Option { + loop { + let b = *self.src.get(self.pos)?; + self.pos += 1; + if self.zeros >= 2 && b == 0x03 { + self.zeros = 0; + continue; + } + self.zeros = if b == 0x00 { self.zeros + 1 } else { 0 }; + return Some(b); + } + } +} + // Reads an SEI payloadType/payloadSize value (H.265 D.2 ff-extension coding: // a run of 0xFF bytes plus one final byte < 0xFF). `None` at end-of-buffer. -fn read_sei_ff_value(rbsp: &[u8], i: &mut usize) -> Option { +fn read_sei_ff_value(rbsp: &mut impl Iterator) -> Option { let mut value: u32 = 0; loop { - let b = *rbsp.get(*i)?; - *i += 1; + let b = rbsp.next()?; value = value.checked_add(b as u32)?; if b != 0xFF { return Some(value); @@ -1145,7 +1178,7 @@ mod tests { ); } - // MEASURED: `scan_sei` must stop copying the RBSP once both HDR10 messages are captured. + // MEASURED: `scan_sei` makes no RBSP copy once both HDR10 messages are captured. #[test] fn scan_sei_stops_copying_once_both_hdr10_messages_are_captured() { let pps = { @@ -1175,7 +1208,7 @@ mod tests { }; let mut parser = HevcParser::new(); - // First AU: both messages captured, so this one legitimately copies. + // First AU captures both messages. parser.parse(&make_pes(au(), Some(0))); assert!( parser.sei_mastering.is_some() && parser.sei_content_light.is_some(), @@ -1200,6 +1233,26 @@ mod tests { ); } + // A stream carrying only ONE HDR10 message never completes the pair, so the SEI + // scan runs every AU; it must still not copy the RBSP each time. + #[test] + fn scan_sei_does_not_copy_when_one_hdr10_message_is_absent() { + let mut au = nal_bytes(NAL_PPS, &[0xC0]); + au.extend_from_slice(&sei_nal(&[sei_message( + SEI_MASTERING_DISPLAY_COLOUR_VOLUME, + &mastering_payload([1, 2, 3], [4, 5, 6], 7, 8, 9, 10), + )])); + au.extend_from_slice(&nal_bytes(19, &[0xEC])); + let mut parser = HevcParser::new(); + parser.parse(&make_pes(au.clone(), Some(0))); + assert!(parser.sei_mastering.is_some() && parser.sei_content_light.is_none()); + RBSP_COPIES.with(|c| c.set(0)); + for i in 0..50 { + parser.parse(&make_pes(au.clone(), Some(3750 * (i + 1)))); + } + assert_eq!(RBSP_COPIES.with(|c| c.get()), 0); + } + /// Only the mastering-display SEI (no content-light SEI) → metadata is NOT /// surfaced. HDR10 requires BOTH; a half-populated record is never emitted. #[test] @@ -1449,6 +1502,40 @@ mod tests { ); } + // PPS ids are 0..=63 (H.265 7.4.3.3): an out-of-range id is never stored, so a slice + // naming it declines a coding type; id 63 still resolves. + #[test] + #[allow(clippy::unusual_byte_groupings)] + fn pps_id_outside_spec_range_is_ignored() { + use super::super::coding::CodingType; + // PPS id=64: ue '0000001000001', sps_id '1', flags '00', num_extra '000', stop '1'. + let pps64 = [0b0000_0010, 0b0000_1100, 0b0001_0000]; + // TRAIL_R slice: first '1', pps_id ue(64), slice_type ue '011' (I), stop '1'. + let slice64 = [0b1000_0001, 0b0000_0101, 0b1100_0000]; + let mut data = nal_bytes(NAL_PPS, &pps64); + data.extend_from_slice(&nal_bytes(1, &slice64)); + let frames = HevcParser::new().parse(&make_pes(data, Some(0))); + assert_eq!(frames.len(), 1); + assert_eq!( + frames[0].coding.map(|c| c.coding_type()), + None, + "pps id 64 is out of range" + ); + + // PPS id=63: ue '0000001000000', sps_id '1', flags '00', num_extra '000', stop '1'. + let pps63 = [0b0000_0010, 0b0000_0100, 0b0001_0000]; + // Slice: first '1', pps_id ue(63), slice_type '011', stop '1'. + let slice63 = [0b1000_0001, 0b0000_0001, 0b1100_0000]; + assert_eq!(hevc_pps_id(&[0x44, 0x01, pps63[0], pps63[1]]), Some(63)); + let mut data = nal_bytes(NAL_PPS, &pps63); + data.extend_from_slice(&nal_bytes(1, &slice63)); + let frames = HevcParser::new().parse(&make_pes(data, Some(0))); + assert_eq!( + frames[0].coding.map(|c| c.coding_type()), + Some(CodingType::I) + ); + } + /// Build an Annex-B NAL (start code + 2-byte header + body bytes). fn nal_bytes(nal_type: u8, body: &[u8]) -> Vec { let mut v = vec![0x00, 0x00, 0x01]; @@ -2851,6 +2938,7 @@ mod tests { assert_eq!(frames.len(), 1, "a well-formed access unit is emitted"); // Same AU, framing forced out of sync: the guard drops it. + GUARD_DROPS.with(|c| c.set(0)); FORCE_FRAMING_DESYNC.with(|c| c.set(true)); let mut parser2 = HevcParser::new(); let dropped = parser2.parse(&make_pes(annex_b_access_unit(), Some(0))); @@ -2859,6 +2947,48 @@ mod tests { dropped.is_empty(), "a desynced access unit is dropped by parse(), not emitted" ); + assert_eq!(GUARD_DROPS.with(|c| c.get()), 1); + } + + // The hook above is the only way to fire the guard: over arbitrary Annex-B input + // (random NAL types, zero runs, param-set churn, empty NALs) every emitted frame + // tiles and the guard never drops, so it stays pure defense in depth. + #[test] + fn guard_never_fires_on_arbitrary_annex_b_input() { + let mut seed = 0x2545_F491_4F6C_DD1Du64; + let mut next = move || { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + seed + }; + GUARD_DROPS.with(|c| c.set(0)); + let mut parser = HevcParser::new(); + for au in 0..2000 { + let mut data = Vec::new(); + for _ in 0..(next() % 6) { + data.extend_from_slice(if next() % 2 == 0 { + &[0, 0, 1] + } else { + &[0, 0, 0, 1] + }); + let r = next(); + let nal_type = + [NAL_VPS, NAL_SPS, NAL_PPS, NAL_AUD, 1, 19, 21, 39][(r % 8) as usize]; + data.extend_from_slice(&hevc_nal_header(nal_type)); + for _ in 0..(next() % 12) { + data.push([0x00, 0x03, 0xFF, (next() & 0xFF) as u8][(next() % 4) as usize]); + } + } + for f in parser.parse(&make_pes(data, Some(au * 3750))) { + assert!(length_prefix_tiles(&f.data), "au {au} does not tile"); + } + } + assert_eq!( + GUARD_DROPS.with(|c| c.get()), + 0, + "the guard fired on real input" + ); } #[test] diff --git a/src/mux/codec/mod.rs b/src/mux/codec/mod.rs index 3a8dea3a..796fe3bc 100644 --- a/src/mux/codec/mod.rs +++ b/src/mux/codec/mod.rs @@ -14,10 +14,9 @@ pub mod adts; mod audio_frames; /// Codec-agnostic per-picture coding carrier (`PictureInfo` + accessors). pub mod coding; -/// DTS / DTS-HD elementary-stream parser. pub(crate) mod crc; pub(crate) mod dropgate; - +/// DTS / DTS-HD elementary-stream parser. pub mod dts; /// DVD bitmap subtitle (VobSub) parser. pub mod dvdsub; @@ -119,6 +118,11 @@ pub trait CodecParser: Send { /// Get codec initialization data (e.g., SPS+PPS for H.264). /// Returns None until enough data has been seen. fn codec_private(&self) -> Option>; + + /// Frames whose in-band config differs from the kept first config. + fn config_changes(&self) -> u64 { + 0 + } } /// Passthrough parser — treats each PES as one frame, no parsing. @@ -268,12 +272,10 @@ mod tests { // interchangeable. #[test] fn parsers_that_derive_no_config_report_absent_never_an_empty_codec_private() { - // Codecs whose parsers do no configuration extraction, paired with a - // payload that is a REAL frame of that codec so the gate takes its - // keep-path (a rejected frame proves nothing about the config answer). - let cases: [(Codec, Vec); 5] = [ - // ADTS: syncword FFF1, MPEG-4 AAC-LC, 44.1 kHz, stereo, 7-byte frame. - (Codec::Aac, vec![0xFF, 0xF1, 0x50, 0x80, 0x00, 0xBF, 0xFC]), + // Parsers with no config extraction, each fed a REAL frame so the gate + // keeps it (a rejected frame proves nothing). AAC derives an ASC, so + // it is tested separately below. + let cases: [(Codec, Vec); 4] = [ // MPEG-1 Layer II, 44.1 kHz, 128 kbit/s, stereo. (Codec::Mp2, vec![0xFF, 0xFD, 0x70, 0x00, 0x00, 0x00]), // MPEG-1 Layer III, 44.1 kHz, 128 kbit/s, stereo. @@ -307,6 +309,20 @@ mod tests { } } + // A valid ADTS frame (AAC-LC, 44.1 kHz, stereo, frame_length 7) must yield + // its AudioSpecificConfig as soon as the frame is parsed. + #[test] + fn aac_parser_derives_audio_specific_config_from_first_frame() { + let mut parser = parser_for_codec(Codec::Aac, None, false); + assert_eq!(parser.codec_private(), None); + let frames = parser.parse(&pes( + Some(0), + vec![0xFF, 0xF1, 0x50, 0x80, 0x00, 0xFF, 0xFC], + )); + assert_eq!(frames.len(), 1, "the frame is valid, not dropped"); + assert_eq!(parser.codec_private(), Some(vec![0x12, 0x10])); + } + #[test] fn audio_codecs_emit_keyframe_frames() { // PES = frame audio: every frame is independently decodable → keyframe. @@ -336,6 +352,7 @@ mod provenance_guard { ("mod.rs", include_str!("mod.rs")), ("ac3.rs", include_str!("ac3.rs")), ("adts.rs", include_str!("adts.rs")), + ("audio_frames.rs", include_str!("audio_frames.rs")), ("dts.rs", include_str!("dts.rs")), ("dvdsub.rs", include_str!("dvdsub.rs")), ("flac.rs", include_str!("flac.rs")), @@ -443,9 +460,9 @@ mod provenance_guard { .lines() .filter_map(|l| { let l = l.trim(); - let rest = l - .strip_prefix("pub mod ") - .or_else(|| l.strip_prefix("pub(crate) mod "))?; + let rest = ["pub mod ", "pub(crate) mod ", "mod "] + .iter() + .find_map(|p| l.strip_prefix(p))?; rest.strip_suffix(';') }) .collect(); diff --git a/src/mux/codec/mpegaudio.rs b/src/mux/codec/mpegaudio.rs index a0998311..66543687 100644 --- a/src/mux/codec/mpegaudio.rs +++ b/src/mux/codec/mpegaudio.rs @@ -58,7 +58,59 @@ const MPEG2_L1: [u32; 15] = [ ]; const MPEG2_L23: [u32; 15] = [0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160]; -fn frame_header(data: &[u8]) -> Option
{ +// Free-format search ceilings in bytes (a practical bound from the frame syntax, not a spec +// limit): Layer I/II never usefully exceed their raw 16-bit stereo PCM (samples x 4); Layer III +// is header + CRC + side info + max part2_3_length for every granule/channel + reservoir. +const MAX_FREE_L1_BYTES: usize = 384 * 4; +const MAX_FREE_L2_BYTES: usize = 1152 * 4; +const MAX_FREE_L3_MPEG1_BYTES: usize = 4 + 2 + 32 + 2 * 2 * 4095 / 8 + 1 + 511; +const MAX_FREE_L3_MPEG2_BYTES: usize = 4 + 2 + 17 + 2 * 4095 / 8 + 1 + 255; + +// Does a header continuing this free-format stream (same version/layer/bitrate/rate, +// padding and private bits ignored) start at `i`? +fn free_format_next_at(data: &[u8], i: usize) -> bool { + data.get(i..i + 3) + .is_some_and(|h| h[0] == 0xFF && h[1] == data[1] && h[2] & 0xFC == data[2] & 0xFC) +} + +// Size of the free-format frame at `data[0]` (its size is the spacing to the next header), +// or `bytes > data.len()` to wait. `free_size` is the learned size without padding, trusted +// only while the next header is where it predicts. At `eos`, a lone last frame is emitted. +fn free_format_bytes( + data: &[u8], + free_size: &mut Option, + pad: usize, + max: usize, + eos: bool, +) -> Option { + let wait = data.len().saturating_add(1); + if let Some(n) = *free_size { + let b = n + pad; + // At EOS the learned size frames the last unit; trailing junk (ID3v1) stays out. + if free_format_next_at(data, b) || (eos && data.len() >= b) { + return Some(b); + } + if data.len() < b + 3 { + return Some(wait); + } + // No header where predicted: relearn below (the old size stands until replaced). + } + let last = max.min(data.len().saturating_sub(3)); + if let Some(d) = (pad + 5..=last).find(|&i| free_format_next_at(data, i)) { + *free_size = Some(d - pad); + return Some(d); + } + if data.len() >= max + 3 { + return None; // no successor within the largest legal frame: not a real header + } + Some(if eos && data.len() > pad + 4 { + data.len() + } else { + wait + }) +} + +fn frame_header(data: &[u8], free_size: &mut Option, eos: bool) -> Option
{ if !matches!(mpa_verdict(data), MpaVerdict::Valid) { return None; } @@ -84,13 +136,24 @@ fn frame_header(data: &[u8]) -> Option
{ 1 if version != 3 => 576, _ => 1152, }; + let size = |bitrate: u32, padding: u32| -> usize { + if layer == 3 { + ((12 * bitrate / rate + padding) * 4) as usize + } else { + ((samples / 8) * bitrate / rate + padding) as usize + } + }; let bytes = if bitrate == 0 { - // Free-format has no signalled size; retain PES-granular passthrough. - data.len() - } else if layer == 3 { - ((12 * bitrate / rate + padding) * 4) as usize + let pad = size(0, padding); + let max = match layer { + 3 => MAX_FREE_L1_BYTES, + 2 => MAX_FREE_L2_BYTES, + _ if version == 3 => MAX_FREE_L3_MPEG1_BYTES, + _ => MAX_FREE_L3_MPEG2_BYTES, + }; + free_format_bytes(data, free_size, pad, max, eos)? } else { - ((samples / 8) * bitrate / rate + padding) as usize + size(bitrate, padding) }; Some(Header { bytes, @@ -102,6 +165,8 @@ fn frame_header(data: &[u8]) -> Option
{ pub struct MpegAudioParser { frames: AudioFrames, + // Free-format frame size (without padding), learned from the first sync spacing. + free_size: Option, } impl Default for MpegAudioParser { fn default() -> Self { @@ -112,6 +177,7 @@ impl MpegAudioParser { pub fn new() -> Self { Self { frames: AudioFrames::new("mpegaudio"), + free_size: None, } } pub fn dropped_frames(&self) -> u64 { @@ -123,10 +189,26 @@ impl MpegAudioParser { } impl CodecParser for MpegAudioParser { fn parse(&mut self, pes: &PesPacket) -> Vec { - self.frames.parse(pes, 4, frame_header) + let mut out = Vec::new(); + if pes.discontinuity { + // Emit the complete frame still awaiting its successor before the gap clears it. + let free_size = &mut self.free_size; + out = self + .frames + .drain_before_gap(4, |d| frame_header(d, free_size, true)); + self.free_size = None; + } + let free_size = &mut self.free_size; + out.extend( + self.frames + .parse(pes, 4, |d| frame_header(d, free_size, false)), + ); + out } fn flush(&mut self) -> Vec { - self.frames.flush() + let free_size = &mut self.free_size; + self.frames + .flush_with(4, |d| frame_header(d, free_size, true)) } fn codec_private(&self) -> Option> { None @@ -150,17 +232,28 @@ mod tests { /// A valid MPEG-1 Layer III header: sync 0xFFF, version MPEG-1 (11), layer /// III (01), bitrate_index 9, sample-rate 0 (44.1 kHz), no CRC. Bytes: - /// 0xFF 0xFB 0x90 0x00 — the canonical MP3 frame header. - fn mp3_frame(_payload: usize) -> Vec { + /// 0xFF 0xFB 0x90 0x00 — the canonical MP3 frame header. The header fixes the + /// frame at 417 bytes (128 kbit/s, 44.1 kHz), so there is no size argument. + const MP3_FRAME_BYTES: usize = 417; + fn mp3_frame() -> Vec { let mut f = vec![0xFF, 0xFB, 0x90, 0x00]; - f.extend(std::iter::repeat_n(0xAA, 413)); + f.resize(MP3_FRAME_BYTES, 0xAA); f } + #[test] + fn mp3_frame_fixture_is_exactly_one_frame() { + let f = mp3_frame(); + assert_eq!( + frame_header(&f, &mut None, false).map(|h| h.bytes), + Some(f.len()) + ); + } + #[test] fn valid_header_is_kept() { let mut p = MpegAudioParser::new(); - let f = p.parse(&make_pes(mp3_frame(400), Some(90000))); + let f = p.parse(&make_pes(mp3_frame(), Some(90000))); assert_eq!(f.len(), 1); assert_eq!(f[0].pts_ns, pts_to_ns(90000)); assert_eq!(p.dropped_frames(), 0); @@ -172,8 +265,8 @@ mod tests { // advance from the last timestamp by sample count — resetting to 0 would corrupt // A/V sync. Mirrors the adts.rs guard test. let mut p = MpegAudioParser::new(); - p.parse(&make_pes(mp3_frame(400), Some(90000))); - let f = p.parse(&make_pes(mp3_frame(400), None)); + p.parse(&make_pes(mp3_frame(), Some(90000))); + let f = p.parse(&make_pes(mp3_frame(), None)); assert_eq!(f.len(), 1); assert_eq!( f[0].pts_ns, @@ -189,7 +282,7 @@ mod tests { fn dropped_frames_are_counted_but_their_duration_is_not_invented() { let mut parser = MpegAudioParser::new(); // Reserved layer field (00) — rejected per ISO/IEC 11172-3. - let mut bad = mp3_frame(32); + let mut bad = mp3_frame(); bad[1] &= !0b0000_0110; for i in 0..3 { let out = parser.parse(&make_pes(bad.clone(), Some(i * 90_000))); @@ -209,7 +302,7 @@ mod tests { // version field = 01 (reserved) → rejected. byte1 = 111_01_01_1 = 0xEB // keeps the 11-bit sync (0xFF + top 3 bits 111) but sets version bits to 01. let mut p = MpegAudioParser::new(); - let mut frame = mp3_frame(400); + let mut frame = mp3_frame(); frame[1] = 0xEB; let f = p.parse(&make_pes(frame, Some(90000))); assert!(f.is_empty(), "reserved version dropped"); @@ -221,7 +314,7 @@ mod tests { // Sync present but sample-rate field = 3 (reserved) → rejected per spec. // 0xFF 0xFB then byte2 with bits 11..10 = 11: 0x9C. let mut p = MpegAudioParser::new(); - let mut frame = mp3_frame(400); + let mut frame = mp3_frame(); frame[2] = 0x9C; // freq field = 3 let f = p.parse(&make_pes(frame, Some(90000))); assert!(f.is_empty(), "reserved sample rate dropped"); @@ -233,7 +326,7 @@ mod tests { // Layer field 00 (reserved). byte1 bits 2..1 = 00 → 0xF9 keeps sync // (0xFFF needs byte1 top 3 bits set) and sets layer=00. let mut p = MpegAudioParser::new(); - let mut frame = mp3_frame(400); + let mut frame = mp3_frame(); frame[1] = 0xF9; // 1111_1001: sync ok (top 3 =111), version 11, layer 00 let f = p.parse(&make_pes(frame, Some(0))); assert!(f.is_empty(), "reserved layer dropped"); @@ -243,7 +336,7 @@ mod tests { #[test] fn bad_bitrate_index_15_is_dropped() { let mut p = MpegAudioParser::new(); - let mut frame = mp3_frame(400); + let mut frame = mp3_frame(); frame[2] = 0xF0; // bitrate_index = 1111 assert!(p.parse(&make_pes(frame, Some(0))).is_empty()); assert_eq!(p.dropped_frames(), 1); @@ -254,13 +347,169 @@ mod tests { // Free format (bitrate_index == 0) is legal and decodable — it must NOT // be dropped (that would be a false positive on a clean stream). let mut p = MpegAudioParser::new(); - let mut frame = mp3_frame(400); + let mut frame = mp3_frame(); frame[2] = 0x00; // bitrate_index = 0000 (free format); sync/layer/rate ok - let f = p.parse(&make_pes(frame, Some(0))); - assert_eq!(f.len(), 1, "free-format frame kept"); + // Its size is the spacing to the next header: the last frame waits for EOS. + let mut f = p.parse(&make_pes(frame.repeat(2), Some(0))); + f.extend(p.flush()); + assert_eq!(f.len(), 2, "free-format frames kept"); assert_eq!(p.dropped_frames(), 0); } + // Free-format frame: bitrate_index 0, 44.1 kHz, no padding, 300 bytes total. + fn free_frame() -> Vec { + let mut f = vec![0xFF, 0xFB, 0x00, 0x00]; + f.resize(300, 0xAA); + f + } + + // Free-format size comes from sync spacing, so several frames in one PES each get + // one frame's duration instead of one AU swallowing the rest of the buffer. + #[test] + fn free_format_frames_in_one_pes_are_split_with_distinct_pts() { + let mut p = MpegAudioParser::new(); + let mut f = p.parse(&make_pes(free_frame().repeat(3), Some(0))); + assert_eq!(f.len(), 2, "the third awaits a confirming header"); + f.extend(p.flush()); + assert_eq!(f.len(), 3, "EOS emits the known-size third"); + for (i, fr) in f.iter().enumerate() { + assert_eq!(fr.data, free_frame()); + assert_eq!(fr.pts_ns, i as i64 * (1152 * 1_000_000_000i64 / 44100)); + } + } + + #[test] + fn free_format_frame_split_across_pes_reassembles() { + let data = free_frame().repeat(2); + for split in [1, 4, 150, 299, 300, 301, 450] { + let mut p = MpegAudioParser::new(); + let mut f = p.parse(&make_pes(data[..split].to_vec(), Some(0))); + f.extend(p.parse(&make_pes(data[split..].to_vec(), None))); + f.extend(p.parse(&make_pes(free_frame(), None))); + f.extend(p.flush()); + assert_eq!(f.len(), 3, "split {split}"); + assert!(f.iter().all(|fr| fr.data == free_frame()), "split {split}"); + } + } + + // A false free-format header in a CBR stream never finds a matching successor; the + // wait is capped at the largest legal free-format frame, so CBR framing resumes. + #[test] + fn false_free_format_header_does_not_stall_cbr() { + let mut p = MpegAudioParser::new(); + let mut first = vec![0xFF, 0xFB, 0x00, 0x00]; + first.extend_from_slice(&mp3_frame()); + let mut n = p.parse(&make_pes(first, Some(0))).len(); + for _ in 0..9 { + n += p.parse(&make_pes(mp3_frame(), None)).len(); + } + assert_eq!( + n, 10, + "every CBR frame emitted without waiting for the buffer cap" + ); + } + + // A matching header closer than a minimal frame is not the spacing; keep looking. + #[test] + fn too_close_free_format_match_is_skipped() { + let mut data = vec![0xFF, 0xFB, 0x00, 0x00]; + data.extend_from_slice(&free_frame().repeat(3)); + let f = MpegAudioParser::new().parse(&make_pes(data, Some(0))); + assert!(f.len() >= 2, "framing proceeds, got {} frames", f.len()); + } + + // The learned size is re-checked against the next header: a new stream after a gap + // (or a size change) is relearned rather than misframed. + #[test] + fn free_format_size_is_relearned() { + let short = |n: usize| { + let mut f = free_frame(); + f.truncate(n); + f + }; + let mut p = MpegAudioParser::new(); + assert!( + !p.parse(&make_pes(free_frame().repeat(3), Some(0))) + .is_empty() + ); + let gap = PesPacket { + discontinuity: true, + ..make_pes(short(200).repeat(3), Some(900_000)) + }; + let f = p.parse(&gap); + assert_eq!( + f[0].data.len(), + 300, + "the pending pre-gap frame is emitted first" + ); + assert!(f.len() > 1); + assert!( + f[1..].iter().all(|fr| fr.data.len() == 200), + "relearned after the gap" + ); + let mut p = MpegAudioParser::new(); + let mut data = free_frame().repeat(2); + data.extend_from_slice(&short(200).repeat(3)); + let f = p.parse(&make_pes(data, Some(0))); + let sizes: Vec = f.iter().map(|fr| fr.data.len()).collect(); + assert_eq!(sizes, [300, 300, 200, 200], "relearned on a size change"); + } + + // A lone free-format frame has no successor to size it; EOS emits it. + #[test] + fn lone_free_format_frame_is_emitted_at_flush() { + let mut p = MpegAudioParser::new(); + assert!(p.parse(&make_pes(free_frame(), Some(0))).is_empty()); + let f = p.flush(); + assert_eq!(f.len(), 1); + assert_eq!(f[0].data, free_frame()); + } + + // Layer II free format at 768 kbit/s, 48 kHz: 2304-byte frames, above any table rate. + #[test] + fn high_rate_layer2_free_format_frames() { + let mut frame = vec![0xFF, 0xFD, 0x04, 0x00]; + frame.resize(2304, 0xAA); + let mut p = MpegAudioParser::new(); + let mut f = p.parse(&make_pes(frame.repeat(3), Some(0))); + f.extend(p.flush()); + assert_eq!(f.len(), 3); + assert!(f.iter().all(|fr| fr.data == frame)); + } + + // The complete free-format frame awaiting its successor is emitted before a gap + // clears the buffer, not silently lost. + #[test] + fn pending_free_format_frame_survives_a_discontinuity() { + let mut p = MpegAudioParser::new(); + let mut f = p.parse(&make_pes(free_frame().repeat(2), Some(0))); + let gap = PesPacket { + discontinuity: true, + ..make_pes(free_frame().repeat(2), Some(900_000)) + }; + f.extend(p.parse(&gap)); + assert_eq!( + f.len(), + 3, + "both pre-gap frames plus the first post-gap one" + ); + assert_eq!(p.dropped_frames(), 0); + } + + // At EOS the learned size frames the last unit; trailing ID3v1 "TAG" bytes are not + // absorbed into it. + #[test] + fn eos_uses_learned_size_not_trailing_junk() { + let mut data = free_frame().repeat(2); + data.extend_from_slice(b"TAG"); + data.resize(data.len() + 125, b' '); + let mut p = MpegAudioParser::new(); + let mut f = p.parse(&make_pes(data, Some(0))); + f.extend(p.flush()); + assert_eq!(f.len(), 2); + assert!(f.iter().all(|fr| fr.data == free_frame())); + } + #[test] fn non_sync_packet_passes_through() { // No 11-bit sync → not a validatable frame → keep (conservative). @@ -273,10 +522,10 @@ mod tests { #[test] fn drop_preserves_sync_via_own_pts() { let mut p = MpegAudioParser::new(); - let mut bad = mp3_frame(400); + let mut bad = mp3_frame(); bad[2] = 0x9C; // reserved sample rate assert!(p.parse(&make_pes(bad, Some(90000))).is_empty()); - let f = p.parse(&make_pes(mp3_frame(400), Some(96000))); + let f = p.parse(&make_pes(mp3_frame(), Some(96000))); assert_eq!(f.len(), 1); assert_eq!( f[0].pts_ns, @@ -291,8 +540,8 @@ mod tests { fn flush_adds_no_phantom_frame_after_the_last_real_packet() { let mut p = MpegAudioParser::new(); let mut emitted = Vec::new(); - emitted.extend(p.parse(&make_pes(mp3_frame(400), Some(90_000)))); - emitted.extend(p.parse(&make_pes(mp3_frame(400), Some(180_000)))); + emitted.extend(p.parse(&make_pes(mp3_frame(), Some(90_000)))); + emitted.extend(p.parse(&make_pes(mp3_frame(), Some(180_000)))); // An invalid header (version field 01 = reserved) is dropped, not buffered. emitted.extend(p.parse(&make_pes(vec![0xFF, 0xEB, 0x90, 0x00, 0xAA], Some(270_000)))); assert_eq!(emitted.len(), 2, "two valid packets out, one dropped"); @@ -317,7 +566,7 @@ mod tests { #[test] fn an_emitted_frame_carries_the_packets_source_offset() { let mut p = MpegAudioParser::new(); - let mut pes = make_pes(mp3_frame(400), Some(90_000)); + let mut pes = make_pes(mp3_frame(), Some(90_000)); pes.source = Some(crate::pes::SourcePos::at_byte(7_777)); let f = p.parse(&pes); assert!(!f.is_empty(), "the frame is emitted"); @@ -325,7 +574,7 @@ mod tests { } #[test] fn every_pes_split_reassembles_a_complete_mpeg_frame() { - let data = mp3_frame(413); + let data = mp3_frame(); for split in 1..data.len() { let mut p = MpegAudioParser::new(); assert!( @@ -342,7 +591,7 @@ mod tests { #[test] fn multiple_mpeg_frames_in_one_pes_get_distinct_timestamps() { - let data = mp3_frame(413); + let data = mp3_frame(); let mut p = MpegAudioParser::new(); let frames = p.parse(&make_pes(data.repeat(3), Some(0))); assert_eq!(frames.len(), 3); @@ -362,7 +611,7 @@ mod tests { ([0xff, 0xf3, 0x80, 0], 208, 576, 22050), ([0xff, 0xe3, 0x80, 0], 417, 576, 11025), ] { - let h = frame_header(&header).unwrap(); + let h = frame_header(&header, &mut None, false).unwrap(); assert_eq!((h.bytes, h.samples, h.rate), (bytes, samples, rate)); } } diff --git a/src/mux/disc.rs b/src/mux/disc.rs index 87726428..e4844712 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -197,6 +197,8 @@ pub struct DiscStream { /// Whether each stream (by track index) is video — audio/subtitle frames are /// independent and always admit. is_video: Vec, + /// Bounds the header pump's wait for in-band codec configs (AAC). + header_gate: super::header_gate::HeaderGate, } impl DiscStream { @@ -311,6 +313,7 @@ impl DiscStream { fed_bytes: 0, resync, is_video, + header_gate: super::header_gate::HeaderGate::default(), }) } @@ -667,8 +670,8 @@ fn prof_tick(stage: &str, ns: u128, bytes: u64) { }); } -impl crate::pes::Stream for DiscStream { - fn read(&mut self) -> io::Result> { +impl DiscStream { + fn read_frame(&mut self) -> io::Result> { if let Some(frame) = self.pending_frames.pop_front() { return Ok(Some(frame)); } @@ -954,6 +957,18 @@ impl crate::pes::Stream for DiscStream { } } } +} + +impl crate::pes::Stream for DiscStream { + fn read(&mut self) -> io::Result> { + let read = self.read_frame(); + match &read { + Ok(Some(frame)) => self.header_gate.observe(frame), + Ok(None) => self.header_gate.expire(), + Err(_) => {} + } + read + } fn write(&mut self, _frame: &crate::pes::PesFrame) -> io::Result<()> { Err(crate::error::Error::StreamReadOnly.into()) @@ -979,6 +994,17 @@ impl crate::pes::Stream for DiscStream { .and_then(|(_, parser)| parser.codec_private()) } + fn config_changes(&self) -> Vec<(usize, u64)> { + self.pid_to_track + .iter() + .filter_map(|&(pid, track)| { + let (_, parser) = self.parsers.iter().find(|(p, _)| *p == pid)?; + let n = parser.config_changes(); + (n > 0).then_some((track, n)) + }) + .collect() + } + fn headers_ready(&self) -> bool { // FREEMKV_SKIP_PARSE bypasses codec parsers entirely, so // codec_private is never populated — pretend headers are ready @@ -986,15 +1012,8 @@ impl crate::pes::Stream for DiscStream { if self.skip_parse { return true; } - for (idx, s) in self.title.streams.iter().enumerate() { - if let crate::disc::Stream::Video(v) = s - && !v.secondary - && self.codec_private(idx).is_none() - { - return false; - } - } - true + self.header_gate + .ready(&self.title, |idx| self.codec_private(idx)) } fn errors(&self) -> u64 { @@ -2336,6 +2355,135 @@ mod tests { assert!(PesStream::codec_private(&s, 7).is_none()); } + // A resolved video config must not finalise headers while an AAC track + // still lacks its AudioSpecificConfig (frames are ADTS-stripped). + #[test] + fn headers_ready_also_waits_for_the_aac_config() { + use crate::disc::{AudioChannels, AudioStream, Codec, LabelPurpose, SampleRate}; + let mut title = audio_then_video_title(); + title.content_format = ContentFormat::BdTs; + title.streams[1] = crate::disc::Stream::Video(match &title.streams[1] { + crate::disc::Stream::Video(v) => crate::disc::VideoStream { + pid: 0x1011, + ..v.clone() + }, + _ => unreachable!(), + }); + title.streams.push(crate::disc::Stream::Audio(AudioStream { + pid: 0x1100, + codec: Codec::Aac, + channels: AudioChannels::Stereo, + language: "eng".to_string(), + sample_rate: SampleRate::S44_1, + secondary: false, + purpose: LabelPurpose::Normal, + label: String::new(), + })); + let mut s = DiscStream::new( + Box::new(ZeroReader { capacity: 8 }), + title, + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::BdTs, + false, + None, + ) + .unwrap(); + let pes = |pid: u16, data: Vec| crate::mux::ts::PesPacket { + source: None, + pid, + pts: Some(0), + dts: None, + data, + discontinuity: false, + }; + fn parser( + s: &mut DiscStream, + pid: u16, + ) -> &mut Box { + let i = s.parsers.iter().position(|(p, _)| *p == pid).unwrap(); + &mut s.parsers[i].1 + } + let mut au = seq_header(720, 480); + au.extend_from_slice(&picture_header(1)); + au.extend_from_slice(&[0xAA; 16]); + let _ = parser(&mut s, 0x1011).parse(&pes(0x1011, au)); + let _ = parser(&mut s, 0x1011).parse(&pes(0x1011, picture_header(3))); + assert!(PesStream::codec_private(&s, 1).is_some()); + assert!( + !PesStream::headers_ready(&s), + "AAC track 2 has no AudioSpecificConfig yet" + ); + + let mut adts = vec![0xFF, 0xF1, 0x50, 0x80, 0x02, 0x9F, 0xFC]; + adts.extend_from_slice(&[0u8; 13]); // frame_length = 20 + let _ = parser(&mut s, 0x1100).parse(&pes(0x1100, adts)); + assert_eq!(PesStream::codec_private(&s, 2), Some(vec![0x12, 0x10])); + assert!(PesStream::headers_ready(&s)); + } + + fn aac_only_stream() -> DiscStream { + use crate::disc::{AudioChannels, AudioStream, Codec, LabelPurpose, SampleRate}; + let mut title = audio_then_video_title(); + title.content_format = ContentFormat::BdTs; + title.streams = vec![crate::disc::Stream::Audio(AudioStream { + pid: 0x1100, + codec: Codec::Aac, + channels: AudioChannels::Stereo, + language: "eng".to_string(), + sample_rate: SampleRate::S48, + secondary: false, + purpose: LabelPurpose::Normal, + label: String::new(), + })]; + DiscStream::new( + Box::new(ZeroReader { capacity: 8 }), + title, + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::BdTs, + false, + None, + ) + .unwrap() + } + + // read() must feed the header gate: EOF releases a silent AAC track. + #[test] + fn read_eof_releases_a_silent_aac_track() { + let mut s = aac_only_stream(); + assert!(!PesStream::headers_ready(&s)); + assert!( + PesStream::read(&mut s).unwrap().is_none(), + "all-zero source" + ); + assert!(PesStream::headers_ready(&s), "EOF expires the AAC wait"); + } + + // read() must feed the header gate every frame it returns. + #[test] + fn read_frames_advance_the_aac_wait() { + let mut s = aac_only_stream(); + for sec in 0..=5i64 { + s.pending_frames.push_back(crate::pes::PesFrame { + discard_padding_ns: 0, + coding: None, + source: None, + track: 0, + pts: sec * 1_000_000_000, + keyframe: true, + data: vec![0; 4], + duration_ns: None, + }); + } + for sec in 0..5 { + assert!(PesStream::read(&mut s).unwrap().is_some()); + assert!(!PesStream::headers_ready(&s), "{sec} s: still waiting"); + } + assert!(PesStream::read(&mut s).unwrap().is_some()); + assert!(PesStream::headers_ready(&s), "5 s of source time elapsed"); + } + // ── on_event() / emit() ─────────────────────────────────────────── /// `on_event()` installs the sink and `emit()` feeds it. If either is a diff --git a/src/mux/driver.rs b/src/mux/driver.rs index 156730a3..12978c9a 100644 --- a/src/mux/driver.rs +++ b/src/mux/driver.rs @@ -52,7 +52,7 @@ fn effective_send_deadline(send_deadline: Option) -> Duration { // Ceiling on bytes buffered while waiting for `headers_ready()`, so a damaged title whose // `codec_private` never resolves fails fast instead of OOM-killing the process. -const HEADER_BUFFER_CAP_BYTES: usize = 512 * 1024 * 1024; +pub(crate) const HEADER_BUFFER_CAP_BYTES: usize = 512 * 1024 * 1024; /// Where [`mux_stream`] reads its PES frames from. The driver owns the /// construction of the underlying [`Stream`] so consumers stop hand-rolling @@ -552,8 +552,8 @@ fn drive_mux( }); } - // ── Header pump ── Buffer frames until every video track's codec_private - // has resolved; MKV can't write a track header without codec init data. + // ── Header pump ── Buffer frames until every video (and AAC) track's + // codec_private has resolved; MKV can't write a track header without codec init data. // The loop breaks on EOF/None too, so the gate below re-checks. let mut buffered: Vec = Vec::new(); let mut buffered_bytes: usize = 0; @@ -599,9 +599,9 @@ fn drive_mux( target: "mux", buffered_bytes, cap = HEADER_BUFFER_CAP_BYTES, - "header buffer cap exceeded: the title keeps yielding frames but no \ - video track's codec_private ever resolved; refusing rather than \ - buffering the whole stream into RAM" + "header buffer cap exceeded: the title keeps yielding frames but a \ + required codec_private (video or AAC) never resolved; refusing \ + rather than buffering the whole stream into RAM" ); return Err(Error::MuxHeaderBufferExceeded { bytes: buffered_bytes as u64, @@ -613,24 +613,24 @@ fn drive_mux( } } - // ── Header gate ── The pump can break on EOF without headers resolving. - // Finalising then would write a track header with no CODEC_PRIVATE — a - // structurally-invalid MKV the zero-output guard does not catch. Refuse. + // ── Header gate ── Halt first, whatever headers_ready() says: on the + // highway path a halt can end the stream as `Ok(None)`, and that EOF also + // releases the AAC wait, so a ready gate does not mean the pump finished. + if halt.is_cancelled() { + return Ok(MuxOutcome { + completed: false, + output_opened: false, + bytes_written: 0, + errors: stream.errors(), + lost_bytes: stream.lost_bytes(), + streams: 0, + undelivered_streams: Vec::new(), + }); + } + // The pump can break on EOF without headers resolving. Finalising then would + // write a track header with no CODEC_PRIVATE — a structurally-invalid MKV the + // zero-output guard does not catch. Refuse. if !stream.headers_ready() { - // Re-check halt FIRST: on the highway path a halt can end the stream as - // `Ok(None)` rather than `Err(Halted)`, breaking the loop with headers - // unresolved through no fault of the data — not a malformed disc. - if halt.is_cancelled() { - return Ok(MuxOutcome { - completed: false, - output_opened: false, - bytes_written: 0, - errors: stream.errors(), - lost_bytes: stream.lost_bytes(), - streams: 0, - undelivered_streams: Vec::new(), - }); - } return Err(Error::MkvInvalid.into()); } @@ -641,6 +641,22 @@ fn drive_mux( out_title.codec_privates = (0..info.streams.len()) .map(|i| stream.codec_private(i)) .collect(); + // AAC tracks whose ASC was unknown at header time: handed to the sink when + // it resolves (a seekable MKV backpatches it). + let mut late_aac: Vec = info + .streams + .iter() + .enumerate() + .filter(|(track, s)| { + matches!(s, crate::disc::Stream::Audio(a) if matches!(a.codec, crate::disc::Codec::Aac)) + && out_title + .codec_privates + .get(*track) + .is_none_or(Option::is_none) + }) + .map(|(track, _)| track) + .collect(); + let late_configs: LateConfigs = Arc::default(); let total_bytes = info.size_bytes; let num_streams = info.streams.len(); @@ -659,6 +675,7 @@ fn drive_mux( let sink = WriteSink { output: output_stream, bytes: bytes.clone(), + late_configs: late_configs.clone(), }; let pipe = Pipeline::spawn_named("freemkv-mux-consumer", WRITE_PIPELINE_DEPTH, sink) .map_err(std::io::Error::from)?; @@ -690,6 +707,10 @@ fn drive_mux( } match stream.read() { Ok(Some(frame)) => { + // Queued before the frame is sent, so the sink applies it first. + if !late_aac.is_empty() { + collect_late_configs(&*stream, &mut late_aac, &late_configs); + } if pipe.send_with_halt(frame, halt, deadline).is_err() { interrupted = true; break; @@ -734,6 +755,13 @@ fn drive_mux( } } + if !late_aac.is_empty() { + collect_late_configs(&*stream, &mut late_aac, &late_configs); + if !late_aac.is_empty() { + tracing::debug!(target: "mux", tracks = ?late_aac, "AAC tracks never yielded an AudioSpecificConfig"); + } + } + // ── Finish ── Drop the producer, join the consumer; `close()` finalises // the container. On halt/wedge this returns an error variant, translated // to `completed = false` rather than a hard failure. @@ -753,6 +781,15 @@ fn drive_mux( ); } + for (track, frames) in stream.config_changes() { + tracing::warn!( + target: "mux", + track, + frames, + "in-band codec config changed mid-track; those frames keep the first config" + ); + } + if !mux_run_completed(interrupted, finalize_failed, halt.is_cancelled()) { return Ok(MuxOutcome { completed: false, @@ -783,18 +820,55 @@ fn drive_mux( }) } +// Late `(track, codec_private)` pairs from the reader, drained by the sink. +type LateConfigs = Arc)>>>; + +fn lock_late(q: &LateConfigs) -> std::sync::MutexGuard<'_, Vec<(usize, Vec)>> { + q.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + +// Move tracks whose codec_private has now resolved from `pending` to the queue. +fn collect_late_configs(stream: &dyn Stream, pending: &mut Vec, out: &LateConfigs) { + pending.retain(|&track| match stream.codec_private(track) { + Some(cp) => { + lock_late(out).push((track, cp)); + false + } + None => true, + }); +} + // Write-side `Sink`: applies each frame to the counting output stream and // finalises the container on close. `close()` returns the payload-byte count // plus any undelivered streams (see `MuxOutcome::undelivered_streams`). struct WriteSink { output: CountingStream, bytes: Arc, + late_configs: LateConfigs, +} + +impl WriteSink { + fn apply_late_configs(&mut self) -> Result<(), Error> { + let late = std::mem::take(&mut *lock_late(&self.late_configs)); + for (track, cp) in late { + if !self + .output + .set_codec_private(track, &cp) + .map_err(Error::from)? + { + // Most such sinks (m2ts/stdio/network) carry config in-band anyway. + tracing::debug!(target: "mux", track, "sink does not record a late codec_private"); + } + } + Ok(()) + } } impl Sink for WriteSink { type Output = (u64, Vec); fn apply(&mut self, frame: PesFrame) -> Result { + self.apply_late_configs()?; self.output.write(&frame).map_err(Error::from)?; self.bytes .store(self.output.bytes_written(), Ordering::Relaxed); @@ -802,6 +876,7 @@ impl Sink for WriteSink { } fn close(mut self) -> Result<(u64, Vec), Error> { + self.apply_late_configs()?; self.output.finish().map_err(Error::from)?; // Sample AFTER finish(): the mp4 sink decides its drops there. Ok(( @@ -838,6 +913,9 @@ mod tests { /// value — simulating a halt landing DURING a blocking `fill_extents` read /// (the common operator-stop case). halt_err_at_read: Option, + /// If set, `headers_ready` also flips once `read()` has returned `None`. + ready_on_eof: bool, + eof_seen: bool, } fn audio_stream() -> crate::disc::Stream { @@ -868,6 +946,8 @@ mod tests { cancel_halt: None, read_observer: None, halt_err_at_read: None, + ready_on_eof: false, + eof_seen: false, } } /// After `after` successful reads, the next `read()` returns @@ -915,6 +995,7 @@ mod tests { return Err(crate::error::Error::Halted.into()); } let f = self.frames.pop_front(); + self.eof_seen |= f.is_none(); if f.is_some() { self.reads += 1; if let Some(obs) = &self.read_observer { @@ -936,7 +1017,7 @@ mod tests { self.codec_private_ready.then(|| vec![1, 2, 3]) } fn headers_ready(&self) -> bool { - self.reads >= self.headers_ready_after + self.reads >= self.headers_ready_after || (self.ready_on_eof && self.eof_seen) } } @@ -1134,6 +1215,30 @@ mod tests { assert!(out.output_opened, "the sink was opened before the halt"); } + // A halt that ends the stream as Ok(None) can also release the header gate + // (EOF expires the AAC wait); it must still stop before the sink opens. + #[test] + fn halt_ending_the_header_pump_never_opens_the_output() { + let halt = Halt::new(); + let mut fs = FakeStream::new(1).with_frames(1).cancels(halt.clone(), 1); + fs.headers_ready_after = usize::MAX; + fs.ready_on_eof = true; + let events = SpyEvents::new(); + let out = drive_mux( + Box::new(fs), + "null://", + &halt, + &events, + None, + Duration::from_secs(60), + None, + ) + .expect("halt is a clean stop"); + assert!(!out.completed); + assert!(!out.output_opened, "no sink may be opened after a halt"); + assert!(!events.opened.load(Ordering::SeqCst)); + } + // ── A halt landing mid-read (Err(Halted), the common operator-stop case) // is a clean stop, NOT a failure — both pumps must yield completed=false. // Mutation: propagating the read-arm Err instead of mapping it panics. ── @@ -1887,6 +1992,233 @@ mod tests { assert!(map_none.is_none(), "CSS/clear must NOT resolve an AACS map"); } + // Video + a secondary AAC track whose first frame (and so its ASC) arrives at + // 10 s, long after headers were finalised. + struct LateAacStream { + info: DiscTitle, + frames: std::collections::VecDeque, + aac_seen: bool, + } + + impl LateAacStream { + fn new() -> Self { + use crate::disc::{ + AudioChannels, AudioStream, Codec, ColorSpace, FrameRate, HdrFormat, LabelPurpose, + Resolution, SampleRate, Stream as S, VideoStream, + }; + let mut info = DiscTitle::empty(); + info.streams = vec![ + S::Video(VideoStream { + pid: 0x1011, + codec: Codec::H264, + resolution: Resolution::R1080p, + frame_rate: FrameRate::F25, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt709, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + }), + S::Audio(AudioStream { + pid: 0x1100, + codec: Codec::Aac, + channels: AudioChannels::Stereo, + language: "eng".into(), + sample_rate: SampleRate::S44_1, + secondary: true, + purpose: LabelPurpose::Commentary, + label: String::new(), + }), + ]; + let frame = |track: usize, pts: i64| PesFrame { + discard_padding_ns: 0, + track, + pts, + keyframe: true, + data: vec![0x11; 32], + duration_ns: None, + source: None, + coding: None, + }; + let mut frames = std::collections::VecDeque::new(); + for i in 0..300i64 { + let pts = i * 40_000_000; + frames.push_back(frame(0, pts)); + if pts >= 10_000_000_000 { + frames.push_back(frame(1, pts)); + } + } + LateAacStream { + info, + frames, + aac_seen: false, + } + } + } + + impl Stream for LateAacStream { + fn read(&mut self) -> std::io::Result> { + let f = self.frames.pop_front(); + self.aac_seen |= f.as_ref().is_some_and(|f| f.track == 1); + Ok(f) + } + fn write(&mut self, _frame: &PesFrame) -> std::io::Result<()> { + Ok(()) + } + fn finish(&mut self) -> std::io::Result<()> { + Ok(()) + } + fn info(&self) -> &DiscTitle { + &self.info + } + fn codec_private(&self, track: usize) -> Option> { + match track { + 0 => Some(vec![1, 0x64, 0, 0x28, 0xFF, 0xE0, 0, 0]), + _ => self.aac_seen.then(|| vec![0x12, 0x10]), + } + } + } + + // A seekable MKV must end up with the late AAC track's CodecPrivate. + #[test] + fn late_aac_config_is_backpatched_into_a_seekable_mkv() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("late.mkv"); + let halt = Halt::new(); + let out = drive_mux( + Box::new(LateAacStream::new()), + &format!("mkv://{}", path.display()), + &halt, + &NoopEvents, + None, + Duration::from_secs(60), + None, + ) + .expect("mux succeeds"); + assert!(out.completed); + let back = crate::mux::mkvstream::MkvStream::open( + std::fs::File::open(&path).expect("output exists"), + ) + .expect("output parses"); + assert_eq!( + back.codec_private(1), + Some(vec![0x12, 0x10]), + "late AudioSpecificConfig must reach the track header" + ); + } + + // Reserve fill covers every remainder shape (Void, none, the 1-byte case + // absorbed by a wider size VINT); an unfilled reserve stays a valid Void. + #[test] + fn mkv_late_codec_private_fills_every_reserve_shape() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("shapes.mkv"); + let mut title = LateAacStream::new().info; + let aac = title.streams[1].clone(); + title + .streams + .extend([aac.clone(), aac.clone(), aac.clone(), aac]); + title.codec_privates = vec![Some(vec![1, 0x64, 0, 0x28, 0xFF, 0xE0, 0, 0])]; + let file = std::fs::File::create(&path).expect("create"); + let mut mkv = crate::mux::mkvstream::MkvStream::create(Box::new(file), &title, None) + .expect("create mkv"); + let frame = |track: usize| PesFrame { + discard_padding_ns: 0, + track, + pts: 0, + keyframe: true, + data: vec![0x11; 32], + duration_ns: None, + source: None, + coding: None, + }; + // Before activation the config lands in the pending track header. + assert!(mkv.set_codec_private(5, &[0x11, 0x90]).expect("pending")); + mkv.write(&frame(0)).expect("video activates the muxer"); + let cps: [Vec; 3] = [vec![0x12, 0x10], (0..12).collect(), (0..13).collect()]; + for (i, cp) in cps.iter().enumerate() { + assert!(mkv.set_codec_private(i + 1, cp).expect("patch")); + assert!(!mkv.set_codec_private(i + 1, cp).expect("second patch")); + } + assert!(!mkv.set_codec_private(1, &[0; 14]).expect("too big")); + mkv.write(&frame(1)).expect("audio frame"); + mkv.finish().expect("finish"); + let back = crate::mux::mkvstream::MkvStream::open(std::fs::File::open(&path).unwrap()) + .expect("parses"); + for (i, cp) in cps.iter().enumerate() { + assert_eq!( + back.codec_private(i + 1).as_ref(), + Some(cp), + "track {}", + i + 1 + ); + } + assert_eq!(back.codec_private(4), None, "unfilled reserve is a Void"); + assert_eq!(back.codec_private(5), Some(vec![0x11, 0x90])); + assert_eq!(back.info().streams.len(), 6); + } + + // MVC folds the dependent view into the base track, shifting later stream + // indices down: a late config must land on the remapped AAC track. + #[test] + fn mkv_late_codec_private_follows_the_mvc_track_remap() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("mvc.mkv"); + let mut title = LateAacStream::new().info; + let crate::disc::Stream::Video(base) = title.streams[0].clone() else { + unreachable!() + }; + let dep = crate::disc::VideoStream { + pid: 0x1012, + label: crate::disc::MVC_DEPENDENT_LABEL.to_string(), + ..base + }; + title.streams.insert(1, crate::disc::Stream::Video(dep)); + title.codec_privates = vec![Some(vec![1, 0x64, 0, 0x28, 0xFF, 0xE0, 0, 0])]; + let file = std::fs::File::create(&path).expect("create"); + let mut mkv = crate::mux::mkvstream::MkvStream::create(Box::new(file), &title, None) + .expect("create mkv"); + assert!( + !mkv.set_codec_private(1, &[1]) + .expect("dependent has no track") + ); + assert!(mkv.set_codec_private(2, &[0x12, 0x10]).expect("aac")); + mkv.write(&PesFrame { + discard_padding_ns: 0, + track: 0, + pts: 0, + keyframe: true, + data: vec![0x11; 32], + duration_ns: None, + source: None, + coding: None, + }) + .expect("video"); + mkv.finish().expect("finish"); + let back = crate::mux::mkvstream::MkvStream::open(std::fs::File::open(&path).unwrap()) + .expect("parses"); + assert_eq!(back.info().streams.len(), 2, "dependent folded into base"); + assert_eq!(back.codec_private(1), Some(vec![0x12, 0x10])); + } + + // Non-seekable/other sinks keep the old behaviour: no late patch, no error. + #[test] + fn late_aac_config_is_harmless_on_a_sink_without_backpatch() { + let halt = Halt::new(); + let out = drive_mux( + Box::new(LateAacStream::new()), + "null://", + &halt, + &NoopEvents, + None, + Duration::from_secs(60), + None, + ) + .expect("mux succeeds"); + assert!(out.completed); + } + /// A sink that accepts every frame but reports one stream it could not put in /// the finished container — the `mp4://` shape (an audio track dropped at /// `finish()` because no frame yielded a parseable sample entry). @@ -1922,6 +2254,7 @@ mod tests { info: DiscTitle::empty(), })), bytes: Arc::new(AtomicU64::new(0)), + late_configs: LateConfigs::default(), }; let (bytes, undelivered) = sink.close().expect("close succeeds"); assert_eq!(bytes, 0); diff --git a/src/mux/header_gate.rs b/src/mux/header_gate.rs new file mode 100644 index 00000000..0373f66d --- /dev/null +++ b/src/mux/header_gate.rs @@ -0,0 +1,243 @@ +//! Shared `headers_ready` rule for the PES read streams. + +use crate::disc::{Codec, DiscTitle, Stream}; +use crate::pes::PesFrame; + +// Source time an AAC track may take to deliver its first frame (the only source +// of its AudioSpecificConfig) before headers are finalised without it. +const AAC_CONFIG_WAIT_NS: i64 = 5_000_000_000; +// Larger PTS steps are discontinuities/wraps, not elapsed time. +const MAX_PTS_STEP_NS: i64 = 1_000_000_000; +// Backstops, well inside the driver's cap: per-track frames that did not +// advance PTS (a stuck clock), and total buffered bytes. +const AAC_CONFIG_WAIT_FRAMES: u32 = 2048; +const AAC_CONFIG_WAIT_BYTES: usize = super::driver::HEADER_BUFFER_CAP_BYTES / 2; + +// One track's view of source time, so interleaved tracks far apart in PTS +// neither rebase nor double-count each other. +#[derive(Debug)] +struct TrackClock { + track: usize, + max_pts: i64, + elapsed_ns: i64, + stalled: u32, +} + +/// Tracks how long the header pump has waited on in-band codec configs. +#[derive(Debug, Default)] +pub(crate) struct HeaderGate { + clocks: Vec, + bytes: usize, + expired: bool, +} + +impl HeaderGate { + /// Account for a frame handed to the caller. + pub(crate) fn observe(&mut self, frame: &PesFrame) { + self.account(frame.track, frame.pts, frame.data.len()); + } + + // Per track, elapsed time sums plausible forward PTS steps past the highest + // PTS seen: small backward steps are reordering; large steps either way rebase. + fn account(&mut self, track: usize, pts: i64, bytes: usize) { + self.bytes = self.bytes.saturating_add(bytes); + let clock = match self.clocks.iter().position(|c| c.track == track) { + Some(i) => &mut self.clocks[i], + None => { + self.clocks.push(TrackClock { + track, + max_pts: pts, + elapsed_ns: 0, + stalled: 0, + }); + let last = self.clocks.len() - 1; + &mut self.clocks[last] + } + }; + let step = pts.saturating_sub(clock.max_pts); + if step > 0 && step <= MAX_PTS_STEP_NS { + clock.elapsed_ns = clock.elapsed_ns.saturating_add(step); + clock.max_pts = pts; + } else { + clock.stalled = clock.stalled.saturating_add(1); + if step.saturating_abs() > MAX_PTS_STEP_NS { + clock.max_pts = pts; + } + } + if clock.stalled >= AAC_CONFIG_WAIT_FRAMES + || clock.elapsed_ns >= AAC_CONFIG_WAIT_NS + || self.bytes >= AAC_CONFIG_WAIT_BYTES + { + self.expired = true; + } + } + + /// End of stream: no further frame can supply a missing config. + pub(crate) fn expire(&mut self) { + self.expired = true; + } + + /// Primary video always needs its config; AAC needs it until the wait expires + /// (Matroska A_AAC frames are ADTS-stripped, so the ASC is mandatory). + pub(crate) fn ready( + &self, + title: &DiscTitle, + codec_private: impl Fn(usize) -> Option>, + ) -> bool { + title.streams.iter().enumerate().all(|(idx, s)| { + let needs = match s { + Stream::Video(v) => !v.secondary, + // Secondary (commentary) AAC is not exempt: without its ASC it is + // undecodable too, and the wait is bounded anyway. + Stream::Audio(a) => matches!(a.codec, Codec::Aac) && !self.expired, + Stream::Subtitle(_) => false, + }; + !needs || codec_private(idx).is_some() + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const S: i64 = 1_000_000_000; + + fn aac_title() -> DiscTitle { + aac_title_with(false) + } + + fn aac_title_with(secondary: bool) -> DiscTitle { + let mut t = DiscTitle::empty(); + t.streams.push(Stream::Audio(crate::disc::AudioStream { + pid: 0x1100, + codec: Codec::Aac, + channels: crate::disc::AudioChannels::Stereo, + language: "eng".into(), + sample_rate: crate::disc::SampleRate::S48, + secondary, + purpose: crate::disc::LabelPurpose::Normal, + label: String::new(), + })); + t + } + + fn waiting(g: &HeaderGate) -> bool { + !g.ready(&aac_title(), |_| None) + } + + #[test] + fn frame_count_backstop_expires_when_pts_never_advances() { + let mut g = HeaderGate::default(); + for _ in 0..AAC_CONFIG_WAIT_FRAMES - 1 { + g.account(0, 0, 0); + } + assert!(waiting(&g)); + g.account(0, 0, 0); + assert!(!waiting(&g), "backstop reached"); + } + + // A PTS wrap/backstep must not reset the clock: elapsed is the sum of + // plausible forward steps, so 3 s before + 2 s after the wrap is 5 s. + #[test] + fn elapsed_time_survives_a_pts_backstep() { + let mut g = HeaderGate::default(); + for pts in [0, S, 2 * S, 3 * S, S / 100, S + S / 100, 2 * S + S / 100] { + g.account(0, pts, 0); + } + assert!(!waiting(&g)); + } + + // A forward jump (discontinuity) is not elapsed time. + #[test] + fn a_forward_pts_jump_is_not_counted_as_waiting_time() { + let mut g = HeaderGate::default(); + for pts in [0, S, 100 * S, 101 * S] { + g.account(0, pts, 0); + } + assert!(waiting(&g)); + } + + // Decode-order reordering (B-frames) must not inflate elapsed time. + #[test] + fn reordered_pts_count_once() { + let mut g = HeaderGate::default(); + let mut pts = Vec::new(); + for gop in 0..4 { + let b = gop * 3 * (S / 3); + pts.extend([b + S, b + S / 3, b + 2 * S / 3]); + } + for p in pts { + g.account(0, p, 0); + } + assert!(waiting(&g), "PTS 1 s -> 4 s is 3 s of content, not 5+"); + } + + // Two dense tracks 1.2 s apart in PTS, muxed in bursts: 3 s of content + // must count as 3 s, not the sum over both tracks. + #[test] + fn interleaved_tracks_far_apart_in_pts_do_not_double_count() { + let mut g = HeaderGate::default(); + let step = S / 25; + for burst in 0..7i64 { + for i in 0..10 { + let t = (burst * 10 + i) * step; + g.account(0, S + S / 5 + t, 0); + } + for i in 0..10 { + let t = (burst * 10 + i) * step; + g.account(1, t, 0); + } + } + assert!(waiting(&g), "about 2.8 s of content per track"); + } + + // A high-rate track (TrueHD, ~1200 frames/s) advancing normally is not a + // stuck-PTS source: 2 s of it must not trip the frame backstop. + #[test] + fn high_frame_rate_track_does_not_trip_the_frame_backstop() { + let mut g = HeaderGate::default(); + let step = S / 1200; + for i in 0..2400 { + g.account(0, i * step, 0); + } + assert!(waiting(&g)); + } + + #[test] + fn byte_bound_expires_before_the_driver_cap() { + let mut g = HeaderGate::default(); + g.account(0, 0, AAC_CONFIG_WAIT_BYTES - 1); + assert!(waiting(&g)); + g.account(0, 0, 1); + assert!(!waiting(&g)); + } + + #[test] + fn secondary_aac_track_is_waited_for() { + let t = aac_title_with(true); + let g = HeaderGate::default(); + assert!(!g.ready(&t, |_| None), "commentary AAC still needs its ASC"); + assert!(g.ready(&t, |_| Some(vec![0x12, 0x10]))); + } + + #[test] + fn video_gate_is_never_expired() { + let mut g = HeaderGate::default(); + g.expire(); + let mut t = DiscTitle::empty(); + t.streams.push(Stream::Video(crate::disc::VideoStream { + pid: 0x1011, + codec: Codec::Hevc, + resolution: crate::disc::Resolution::R2160p, + frame_rate: crate::disc::FrameRate::F23_976, + hdr: crate::disc::HdrFormat::Hdr10, + color_space: crate::disc::ColorSpace::Bt2020, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + })); + assert!(!g.ready(&t, |_| None)); + } +} diff --git a/src/mux/interop_tests.rs b/src/mux/interop_tests.rs index 3c0906f1..21621b96 100644 --- a/src/mux/interop_tests.rs +++ b/src/mux/interop_tests.rs @@ -399,6 +399,7 @@ fn validate_audio_parser(encoder: &str, format: &str, codec: crate::disc::Codec) _ => Box::new(Ac3Parser::new()), }; let mut frames = Vec::new(); + let mut codec_private = None; // Deliberately split headers and bodies across PES boundaries. Only the // first chunk has PTS; subsequent frame times must come from codec cadence. for (index, data) in std::fs::read(&input).unwrap().chunks(137).enumerate() { @@ -410,6 +411,10 @@ fn validate_audio_parser(encoder: &str, format: &str, codec: crate::disc::Codec) source: None, discontinuity: false, })); + // Snapshot when the first frame appears, as the header pump does. + if codec_private.is_none() && !frames.is_empty() { + codec_private = Some(parser.codec_private()); + } } frames.extend(parser.flush()); let mut track = MkvTrack::audio(&AudioStream { @@ -422,7 +427,7 @@ fn validate_audio_parser(encoder: &str, format: &str, codec: crate::disc::Codec) purpose: LabelPurpose::Normal, label: String::new(), }); - track.codec_private = parser.codec_private(); + track.codec_private = codec_private.flatten(); let output = dir.path().join("output.mkv"); let mut muxer = MkvMuxer::new( std::fs::File::create(&output).unwrap(), diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index 6d9a14a5..bbd3e9ba 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -634,8 +634,15 @@ pub struct MkvMuxer { /// diagnosable from a future log without the disc. `None` on normal runs /// (diag off) — the muxer pays nothing. opening_capture: Option, + /// Per track: file offset of a Void reserved for a CodecPrivate that was not + /// known at header time (AAC), filled by [`Self::set_codec_private`]. + codec_private_reserves: std::collections::HashMap, } +// Bytes reserved for a late AAC CodecPrivate: ID (2) + size (1-2) + an ASC of +// up to 12 bytes, all inside one Void so the Tracks size never changes. +const CODEC_PRIVATE_RESERVE: usize = 16; + /// Deferred AC-3 channel-count correction: the track header's `Channels` byte /// is written up-front from the (unreliable) IFO count; on the first AC-3 frame /// for the track the value is rewritten from the bitstream `acmod`. @@ -864,6 +871,8 @@ impl MkvMuxer { std::collections::HashMap::new(); let mut pgs_forced_fixups: std::collections::HashMap = std::collections::HashMap::new(); + let mut codec_private_reserves: std::collections::HashMap = + std::collections::HashMap::new(); let mut flag_interlaced_fixups: std::collections::HashMap = std::collections::HashMap::new(); // Per track: whether it emitted a conforming `mvcC` BlockAdditionMapping. @@ -936,6 +945,9 @@ impl MkvMuxer { // Non-MVC (2D/UHD/audio/…): write codec_private verbatim, unchanged. None => ebml::write_binary(&mut writer, ebml::CODEC_PRIVATE, cp)?, } + } else if track.codec_id == ebml::CODEC_AAC { + codec_private_reserves.insert(i, writer.stream_position()?); + writer.write_all(&ebml::void_element(CODEC_PRIVATE_RESERVE))?; } // Pre-0.13's deferred codecPrivate path was removed as dead code. @@ -1173,9 +1185,41 @@ impl MkvMuxer { pgs_forced_fixups, flag_interlaced_fixups, opening_capture: None, + codec_private_reserves, }) } + /// Fill a CodecPrivate reserved at header time. `Ok(false)` when the track + /// had no reservation (already set, or not AAC) or `cp` does not fit. + pub(crate) fn set_codec_private(&mut self, track: usize, cp: &[u8]) -> io::Result { + let Some(&pos) = self.codec_private_reserves.get(&track) else { + return Ok(false); + }; + let mut rest = match CODEC_PRIVATE_RESERVE.checked_sub(3 + cp.len()) { + Some(r) => r, + None => return Ok(false), + }; + let mut el = Vec::with_capacity(CODEC_PRIVATE_RESERVE); + ebml::write_id(&mut el, ebml::CODEC_PRIVATE)?; + // A 1-byte remainder cannot hold a Void, so widen the size VINT instead. + if rest == 1 { + el.extend_from_slice(&[0x40, cp.len() as u8]); + rest = 0; + } else { + ebml::write_size(&mut el, cp.len() as u64)?; + } + el.extend_from_slice(cp); + if rest >= 2 { + el.extend_from_slice(&ebml::void_element(rest)); + } + let here = self.writer.stream_position()?; + self.writer.seek(std::io::SeekFrom::Start(pos))?; + self.writer.write_all(&el)?; + self.writer.seek(std::io::SeekFrom::Start(here))?; + self.codec_private_reserves.remove(&track); + Ok(true) + } + /// Attach an opening-frame capture (`--log-level 3`). The capture writes the /// first ~100 coded frames per track to `.opening.bin` and logs a /// per-frame summary, so opening-GOP / menu issues are diagnosable from a diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 2c514565..a4041157 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -912,6 +912,27 @@ impl crate::pes::Stream for MkvStream { } } + fn set_codec_private(&mut self, track: usize, data: &[u8]) -> io::Result { + let track = match &self.mvc { + Some(m) => match m.stream_to_track.get(track).copied().flatten() { + Some(t) => t, + None => return Ok(false), + }, + None => track, + }; + match &mut self.mode { + Mode::Write(WriteMode::Pending(p)) => match p.tracks.get_mut(track) { + Some(t) if t.codec_private.is_none() => { + t.codec_private = Some(data.to_vec()); + Ok(true) + } + _ => Ok(false), + }, + Mode::Write(WriteMode::Active(m)) => m.set_codec_private(track, data), + _ => Ok(false), + } + } + fn codec_private(&self, track: usize) -> Option> { if let Mode::Read(ref rs) = self.mode { // `track` is a stream index but `codec_privates` is keyed by Matroska diff --git a/src/mux/mod.rs b/src/mux/mod.rs index 19844e11..6d7518ed 100644 --- a/src/mux/mod.rs +++ b/src/mux/mod.rs @@ -41,6 +41,7 @@ pub(crate) mod ebml; /// `fvi://` sink — freemkv's native per-picture video index. A write-only PES sink that emits /// one JSON-Lines record per coded picture; reuses the pure-data [`videomap`] model. pub(crate) mod fvi_sink; +pub(crate) mod header_gate; pub(crate) mod m2ts; /// FMKV metadata header (used by `M2tsStream` / `NetworkStream` / `StdioStream` /// to round-trip codec_privates that don't fit inside the underlying format). diff --git a/src/mux/pipelined_stream.rs b/src/mux/pipelined_stream.rs index 7c9e6075..01cbfc4d 100644 --- a/src/mux/pipelined_stream.rs +++ b/src/mux/pipelined_stream.rs @@ -57,6 +57,8 @@ pub struct PipelinedPesStream { /// stream index. (TS titles are AU-complete already, so this is a passthrough /// there too — `consume_ts` does not use it.) au_asm: Vec, + /// Bounds the header pump's wait for in-band codec configs (AAC). + header_gate: super::header_gate::HeaderGate, } /// The `Codec` of a stream, for configuring its [`AuAssembler`](crate::mux::au_assembly::AuAssembler). @@ -106,6 +108,7 @@ impl PipelinedPesStream { resync, is_video, au_asm, + header_gate: super::header_gate::HeaderGate::default(), } } @@ -266,8 +269,8 @@ impl PipelinedPesStream { } } -impl Stream for PipelinedPesStream { - fn read(&mut self) -> io::Result> { +impl PipelinedPesStream { + fn read_frame(&mut self) -> io::Result> { if let Some(frame) = self.pending_frames.pop_front() { return Ok(Some(frame)); } @@ -352,6 +355,18 @@ impl Stream for PipelinedPesStream { } } } +} + +impl Stream for PipelinedPesStream { + fn read(&mut self) -> io::Result> { + let read = self.read_frame(); + match &read { + Ok(Some(frame)) => self.header_gate.observe(frame), + Ok(None) => self.header_gate.expire(), + Err(_) => {} + } + read + } fn write(&mut self, _: &PesFrame) -> io::Result<()> { Err(crate::error::Error::StreamReadOnly.into()) @@ -365,6 +380,17 @@ impl Stream for PipelinedPesStream { &self.title } + fn config_changes(&self) -> Vec<(usize, u64)> { + self.pid_to_track + .iter() + .filter_map(|&(pid, track)| { + let (_, parser) = self.parsers.iter().find(|(p, _)| *p == pid)?; + let n = parser.config_changes(); + (n > 0).then_some((track, n)) + }) + .collect() + } + fn headers_ready(&self) -> bool { // Match DiscStream semantics: video tracks need codec_private before the // consumer can write the container header. FREEMKV_SKIP_PARSE forces ready @@ -372,15 +398,8 @@ impl Stream for PipelinedPesStream { if self.skip_parse { return true; } - for (idx, s) in self.title.streams.iter().enumerate() { - if let crate::disc::Stream::Video(v) = s - && !v.secondary - && self.codec_private(idx).is_none() - { - return false; - } - } - true + self.header_gate + .ready(&self.title, |idx| self.codec_private(idx)) } fn codec_private(&self, track: usize) -> Option> { @@ -1046,6 +1065,152 @@ mod tests { assert!(stream.finish().is_ok()); } + // --- AAC AudioSpecificConfig must exist before headers are finalised --- + + fn aac_audio(pid: u16) -> crate::disc::Stream { + crate::disc::Stream::Audio(AudioStream { + pid, + codec: Codec::Aac, + channels: AudioChannels::Stereo, + language: "eng".into(), + sample_rate: SampleRate::S44_1, + secondary: false, + purpose: LabelPurpose::Normal, + label: String::new(), + }) + } + + /// Valid AAC-LC 44.1 kHz stereo ADTS frame (ASC = 0x12 0x10). + fn adts(payload: usize) -> Vec { + let len = 7 + payload; + let mut f = vec![0u8; len]; + f[..3].copy_from_slice(&[0xFF, 0xF1, 0x50]); + f[3] = 0x80 | ((len >> 11) & 3) as u8; + f[4] = ((len >> 3) & 0xFF) as u8; + f[5] = (((len & 7) << 5) as u8) | 0x1F; + f[6] = 0xFC; + f + } + + fn pes_at(pid: u16, data: Vec, pts: i64) -> PesPacket { + PesPacket { + pts: Some(pts), + ..ts_pes(pid, data) + } + } + + fn video_plus_aac() -> (PipelinedPesStream, Sender) { + let mut title = video_title(false); + title.streams.push(aac_audio(0x1100)); + let parsers: Vec<(u16, Box)> = vec![ + ( + 0x1011, + Box::new(CountingParser { + per_pes: 1, + flush_n: 0, + cp: Some(vec![1]), + }), + ), + ( + 0x1100, + Box::new(super::super::codec::adts::AdtsParser::new()), + ), + ]; + make_stream(title, parsers, vec![(0x1011, 0), (0x1100, 1)]) + } + + // Video-first TS: the video config resolves before any AAC frame. Finalising + // then wrote A_AAC with no CodecPrivate over header-stripped frames. + #[test] + fn headers_wait_for_aac_config_when_video_arrives_first() { + let (mut stream, tx) = video_plus_aac(); + tx.send(DemuxBatch::Ts(vec![pes_at(0x1011, vec![0; 8], 90_000)])) + .unwrap(); + assert_eq!(stream.read().unwrap().map(|f| f.track), Some(0)); + assert!( + !stream.headers_ready(), + "AAC track has no AudioSpecificConfig yet" + ); + tx.send(DemuxBatch::Ts(vec![pes_at(0x1100, adts(16), 90_000)])) + .unwrap(); + assert_eq!(stream.read().unwrap().map(|f| f.track), Some(1)); + assert!(stream.headers_ready()); + assert_eq!(stream.codec_private(1), Some(vec![0x12, 0x10])); + } + + // A mid-stream AAC config change must be visible per track, not only in + // the parser. + #[test] + fn mid_stream_aac_config_change_is_reported_per_track() { + let (mut stream, tx) = video_plus_aac(); + let mut surround = adts(16); + surround[2] |= 1; // channel_configuration = 6 + surround[3] = (surround[3] & 0x3F) | (2 << 6); + tx.send(DemuxBatch::Ts(vec![ + pes_at(0x1100, adts(16), 0), + pes_at(0x1100, surround, 1920), + ])) + .unwrap(); + assert!(stream.read().unwrap().is_some()); + assert!(stream.read().unwrap().is_some()); + assert_eq!(stream.config_changes(), vec![(1, 1)]); + } + + #[test] + fn audio_only_aac_title_waits_for_config() { + let mut title = DiscTitle::empty(); + title.streams.push(aac_audio(0x1100)); + let parsers: Vec<(u16, Box)> = vec![( + 0x1100, + Box::new(super::super::codec::adts::AdtsParser::new()), + )]; + let (mut stream, tx) = make_stream(title, parsers, vec![(0x1100, 0)]); + assert!(!stream.headers_ready(), "no AAC frame yet → no ASC"); + tx.send(DemuxBatch::Ts(vec![pes_at(0x1100, adts(16), 0)])) + .unwrap(); + assert!(stream.read().unwrap().is_some()); + assert!(stream.headers_ready()); + } + + // An AAC track that never yields a frame must not stall the header pump + // until the buffer cap: the wait is bounded by source time and by EOF. + #[test] + fn silent_aac_track_stops_blocking_headers_after_the_wait_bound() { + let (mut stream, tx) = video_plus_aac(); + // CountingParser passes PES pts through as ns: 1 s steps. + for sec in 0..5i64 { + tx.send(DemuxBatch::Ts(vec![pes_at( + 0x1011, + vec![0; 8], + sec * 1_000_000_000, + )])) + .unwrap(); + stream.read().unwrap(); + assert!(!stream.headers_ready(), "{sec} s: still waiting"); + } + tx.send(DemuxBatch::Ts(vec![pes_at( + 0x1011, + vec![0; 8], + 5_000_000_000, + )])) + .unwrap(); + stream.read().unwrap(); + assert!(stream.headers_ready(), "wait bound reached"); + assert_eq!(stream.codec_private(1), None); + } + + #[test] + fn silent_aac_track_does_not_block_headers_at_eof() { + let (mut stream, tx) = video_plus_aac(); + tx.send(DemuxBatch::Ts(vec![pes_at(0x1011, vec![0; 8], 0)])) + .unwrap(); + tx.send(DemuxBatch::Eof).unwrap(); + stream.read().unwrap(); + assert!(!stream.headers_ready()); + assert!(stream.read().unwrap().is_none()); + assert!(stream.headers_ready(), "EOF: no AAC frame will ever come"); + } + // --- DVD highway: keyframe flag must survive the PS → parser → frame path --- /// Build a minimal MPEG-2 720x480/29.97 sequence header. diff --git a/src/mux/resolve.rs b/src/mux/resolve.rs index deeb072d..51805900 100644 --- a/src/mux/resolve.rs +++ b/src/mux/resolve.rs @@ -2239,7 +2239,7 @@ mod tests { let mut t = DiscTitle::empty(); t.streams.push(Stream::Audio(AudioStream { pid, - codec: Codec::Aac, // → all-keyframe PassthroughParser (1 PES = 1 frame) + codec: Codec::Aac, // → AdtsParser channels: AudioChannels::Stereo, language: "eng".into(), sample_rate: SampleRate::S48, @@ -2510,7 +2510,7 @@ mod tests { .read() .expect("read ok") .expect("one frame emitted from the single PES"); - // PassthroughParser routes the audio stream (PID 0x1100) to track 0. + // AdtsParser passes the unsynced ES through; PID 0x1100 routes to track 0. assert_eq!(frame.track, 0); // TS PesAssembler delivers every byte after the 9-byte PES header to // the end of the 184-byte TS payload (PES is closed by next PUSI or diff --git a/src/pes.rs b/src/pes.rs index b0d43ad4..5419d579 100644 --- a/src/pes.rs +++ b/src/pes.rs @@ -238,13 +238,27 @@ pub trait Stream: Send { None } - /// True when `codec_private` is available for every video track — + /// True when `codec_private` is available for every primary video track + /// and every AAC track (the latter bounded by a short wait and EOF) — /// callers buffer input frames until this flips, since some output /// formats (MKV) can't write frames without codec init data. fn headers_ready(&self) -> bool { true } + /// `(track, frames)` for tracks whose in-band codec config changed after + /// the track header was fixed (e.g. AAC stereo↔5.1); those frames keep the + /// first config. Empty when none. + fn config_changes(&self) -> Vec<(usize, u64)> { + Vec::new() + } + + /// Supply a codec_private that resolved after the header was written (late + /// AAC config). `Ok(true)` when the sink recorded it; default `Ok(false)`. + fn set_codec_private(&mut self, _track: usize, _data: &[u8]) -> std::io::Result { + Ok(false) + } + /// Cumulative count of read errors the stream skipped past (e.g. /// zero-filled bad sectors on a live drive). Default `0` for /// streams that don't have a notion of skip-on-error (file ISO, @@ -344,6 +358,14 @@ impl Stream for CountingStream { self.inner.headers_ready() } + fn config_changes(&self) -> Vec<(usize, u64)> { + self.inner.config_changes() + } + + fn set_codec_private(&mut self, track: usize, data: &[u8]) -> std::io::Result { + self.inner.set_codec_private(track, data) + } + fn errors(&self) -> u64 { self.inner.errors() } diff --git a/src/udf.rs b/src/udf.rs index 8f7ad307..88091eb2 100644 --- a/src/udf.rs +++ b/src/udf.rs @@ -42,13 +42,49 @@ pub struct UdfFs { pub volume_id: String, /// Physical partition start (absolute sector) partition_start: u32, - /// Metadata partition start (absolute sector) + /// Metadata partition block → absolute sector map. /// For UDF 2.50 discs, all file/directory references use metadata-relative LBAs - metadata_start: u32, - /// Metadata partition size in sectors + meta: MetaMap, + /// Sectors in the metadata partition's first extent metadata_sectors: u32, } +/// Metadata-partition block → absolute sector, through the Metadata File's extents +/// `(abs_start, sectors)` in file order; the last extent is open-ended. +#[derive(Debug, Clone)] +pub(crate) struct MetaMap(Vec<(u32, u32)>); + +impl MetaMap { + /// One open-ended extent: the metadata partition is contiguous from `start`. + pub(crate) fn contiguous(start: u32) -> Self { + Self(vec![(start, 0)]) + } + + fn start(&self) -> u32 { + self.0.first().map_or(0, |e| e.0) + } + + fn to_abs(&self, rel: u32) -> Result { + let mut rel = rel; + let last = self.0.len().saturating_sub(1); + for (i, &(start, n)) in self.0.iter().enumerate() { + if rel < n || i == last { + return start.checked_add(rel).ok_or(Error::DiscRead { + sector: start as u64, + status: None, + sense: None, + }); + } + rel -= n; + } + Err(Error::DiscRead { + sector: 0, + status: None, + sense: None, + }) + } +} + /// One allocation extent of a file, as recorded in its ICB. /// /// `recorded` distinguishes the two extent types (ECMA-167 4/14.14.1.1): @@ -104,7 +140,7 @@ impl UdfFs { /// Metadata partition start sector. pub fn metadata_start(&self) -> u32 { - self.metadata_start + self.meta.start() } /// Metadata partition size in sectors. @@ -339,8 +375,10 @@ impl UdfFs { // UDF structure: sector 0 through end of metadata partition // Covers AVDP, VDS, partition descriptor, metadata ICB, FSD, all directories - let meta_end = self.metadata_start.saturating_add(self.metadata_sectors); + let meta_end = self.meta.start().saturating_add(self.metadata_sectors); ranges.push((0, meta_end)); + // A fragmented Metadata File's later extents live elsewhere on disc. + ranges.extend(self.meta.0.iter().skip(1).copied()); // Walk tree, collect ranges for each metadata file self.collect_file_ranges(reader, &self.root, &mut ranges)?; @@ -399,13 +437,7 @@ impl UdfFs { /// `meta_lba` is disc-controlled, so the sum is checked to avoid a /// wrap-to-wrong-sector on a crafted ICB. fn meta_to_abs(&self, meta_lba: u32) -> Result { - self.metadata_start - .checked_add(meta_lba) - .ok_or(Error::DiscRead { - sector: self.metadata_start as u64, - status: None, - sense: None, - }) + self.meta.to_abs(meta_lba) } // Returns the file's first RECORDED extent (not extents.first(), which may be an unrecorded @@ -561,6 +593,7 @@ impl UdfFs { // of hops instead leaves `extents` describing only PART of the file, which is // worse than failing — see the error's own documentation. let mut chain_ended = false; + let mut seen_aeds: Vec = Vec::new(); for _ in 0..MAX_AD_BLOCKS { let num_descriptors = ad_bytes / ad_size; @@ -591,11 +624,12 @@ impl UdfFs { block[lba_off + 3], ]); + // Any AD whose 30-bit length is 0 ends the list (ECMA-167 4/12), whatever + // its type; continuation blocks' trailing zero padding stops here too. + if data_len == 0 { + break; + } match extent_type { - // Recorded and allocated. A zero-length type-0 descriptor is the - // AD-list terminator: continuation blocks are scanned to the end of - // the sector, so trailing zero padding must not be read as extents. - 0 if data_len == 0 => break, 0 => extents.push(IcbExtent { lba: data_lba, len: data_len, @@ -627,14 +661,8 @@ impl UdfFs { match next_block { Some(cont_lba) => { - read_sector(reader, self.meta_to_abs(cont_lba)?, &mut block)?; - // A continuation block starts with a 24-byte Allocation Extent - // Descriptor (ECMA-167 4/14.5, tag 258); real ADs start at offset - // 24 (length @20) — reading from 0 truncates fragmented files (e.g. BD3D). - let aed_l_ad = - u32::from_le_bytes([block[20], block[21], block[22], block[23]]) as usize; - ad_start = 24; - ad_bytes = aed_l_ad.min(block.len().saturating_sub(24)); + let abs = self.meta_to_abs(cont_lba)?; + (ad_start, ad_bytes) = read_aed(reader, abs, &mut block, &mut seen_aeds)?; } None => { chain_ended = true; @@ -813,6 +841,7 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { let mut partition_start: u32 = 0; let mut num_partition_maps: u32 = 0; let mut lvd_sector: Option = None; + let mut fsd_block: Option = None; let mut volume_id = String::new(); let mut metadata_size_bytes: u32 = 0; // A read fault inside a sweep must not abort before the next candidate is @@ -844,6 +873,11 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { num_partition_maps = u32::from_le_bytes([desc[268], desc[269], desc[270], desc[271]]); lvd_sector = Some(i); + // Logical Volume Contents Use: FSD long_ad (length @248, block @252). + fsd_block = Some(u32::from_le_bytes([ + desc[252], desc[253], desc[254], desc[255], + ])) + .filter(|&b| b != 0); } // Terminating Descriptor — end of VDS 8 => break, @@ -868,6 +902,7 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { // Step 3: Parse partition maps from LVD to find metadata partition // BD-ROM discs (UDF 2.50) use a metadata partition (Type 2 map with "*UDF Metadata Partition") // The metadata file is stored at lba=0 of the physical partition + let mut meta_extents: Vec<(u32, u32)> = Vec::new(); let metadata_start = if num_partition_maps >= 2 { let lvd_sec = lvd_sector.ok_or(Error::DiscRead { sector: 0, @@ -923,40 +958,12 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { } if meta_tag == 266 { - // Extended File Entry — get allocation extent - let l_ea = u32::from_le_bytes([ - meta_icb[208], - meta_icb[209], - meta_icb[210], - meta_icb[211], - ]) as usize; - let ad_off = 216 + l_ea; - if ad_off + 8 > meta_icb.len() { - return Err(Error::DiscRead { - sector: meta_file_lba as u64, - status: None, - sense: None, - }); - } - let ad_len = u32::from_le_bytes([ - meta_icb[ad_off], - meta_icb[ad_off + 1], - meta_icb[ad_off + 2], - meta_icb[ad_off + 3], - ]) & 0x3FFF_FFFF; - metadata_size_bytes = ad_len; - let ad_pos = u32::from_le_bytes([ - meta_icb[ad_off + 4], - meta_icb[ad_off + 5], - meta_icb[ad_off + 6], - meta_icb[ad_off + 7], - ]); - // Metadata content starts at partition_start + ad_pos - partition_start.checked_add(ad_pos).ok_or(Error::DiscRead { - sector: partition_start as u64, - status: None, - sense: None, - })? + let (extents, first_bytes) = + metadata_file_extents(&meta_icb, partition_start, meta_file_lba)?; + metadata_size_bytes = first_bytes; + let start = extents.first().map_or(partition_start, |e| e.0); + meta_extents = extents; + start } else { // Fallback: no metadata partition, use physical partition directly partition_start @@ -972,16 +979,33 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { partition_start }; - // Step 4: Read File Set Descriptor from metadata partition - // FSD is at metadata-relative lba 0 (first sector of metadata content) - let mut fsd = [0u8; 2048]; - read_sector(reader, metadata_start, &mut fsd)?; + let meta = if meta_extents.is_empty() { + MetaMap::contiguous(metadata_start) + } else { + MetaMap(meta_extents) + }; - let fsd_tag = u16::from_le_bytes([fsd[0], fsd[1]]); - if fsd_tag != 256 { - // The metadata sector read fine but carries no File Set Descriptor: - // structurally not a UDF disc, not a transient read fault. - return Err(Error::UdfNotFilesystem); + // Step 4: File Set Descriptor at the block the LVD records (ECMA-167 3/10.6.13), + // falling back to metadata block 0 (the customary location). + let mut fsd = [0u8; 2048]; + let mut fsd_found = false; + let mut fsd_err = None; + for cand in fsd_block.into_iter().chain(std::iter::once(0)) { + match meta + .to_abs(cand) + .and_then(|abs| read_sector(reader, abs, &mut fsd)) + { + Ok(()) if u16::from_le_bytes([fsd[0], fsd[1]]) == 256 => { + fsd_found = true; + break; + } + Ok(()) => {} + Err(e) => fsd_err = Some(e), + } + } + if !fsd_found { + // A read fault is transient; "read fine, no FSD" is structurally not UDF. + return Err(fsd_err.unwrap_or(Error::UdfNotFilesystem)); } // Root Directory ICB: long_ad at FSD offset 400 @@ -991,12 +1015,12 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { // Step 5: Read root directory and build file tree. // Pre-seed visited with the root ICB so that any FID pointing back to // root_lba is detected as a cycle immediately. - let root_icb_key = ((metadata_start as u64) << 32) | root_lba as u64; + let root_icb_key = ((meta.start() as u64) << 32) | root_lba as u64; let mut visited: HashSet = HashSet::from([root_icb_key]); let root = read_directory( reader, partition_start, - metadata_start, + &meta, root_lba, "", 0, @@ -1010,11 +1034,74 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result { root, volume_id, partition_start, - metadata_start, + meta, metadata_sectors, }) } +// The Metadata File's recorded extents `(abs_start, sectors)` from its File Entry, plus the +// first extent's byte length. Only recorded (type 0) ADs can hold metadata blocks. +fn metadata_file_extents( + fe: &[u8; 2048], + partition_start: u32, + fe_lba: u32, +) -> Result<(Vec<(u32, u32)>, u32)> { + let bad = Error::DiscRead { + sector: fe_lba as u64, + status: None, + sense: None, + }; + let l_ea = u32::from_le_bytes([fe[208], fe[209], fe[210], fe[211]]) as usize; + let l_ad = u32::from_le_bytes([fe[212], fe[213], fe[214], fe[215]]) as usize; + let ad_size = match fe[34] & 0x07 { + 1 => 16, + 2 => 20, + 3 => return Err(bad), // embedded: no extents at all + _ => 8, + }; + let ad_off = 216 + l_ea; + if ad_off + ad_size > fe.len() { + return Err(bad); + } + // At least the first AD, as before L_AD was honoured. + let n = (l_ad / ad_size).clamp(1, (fe.len() - ad_off) / ad_size); + let mut extents = Vec::new(); + let mut first_bytes = 0; + for i in 0..n { + let o = ad_off + i * ad_size; + let raw = u32::from_le_bytes([fe[o], fe[o + 1], fe[o + 2], fe[o + 3]]); + let len = raw & 0x3FFF_FFFF; + // Metadata lives only in recorded (type 0) ADs: a bad first AD is fatal, + // a later one ends the map. + if raw >> 30 != 0 { + if i == 0 { + return Err(bad); + } + break; + } + if len == 0 && i > 0 { + break; + } + let lba_off = if ad_size == 20 { o + 12 } else { o + 4 }; + let pos = u32::from_le_bytes([ + fe[lba_off], + fe[lba_off + 1], + fe[lba_off + 2], + fe[lba_off + 3], + ]); + let abs = partition_start.checked_add(pos).ok_or(Error::DiscRead { + sector: partition_start as u64, + status: None, + sense: None, + })?; + if i == 0 { + first_bytes = len; + } + extents.push((abs, len.div_ceil(2048))); + } + Ok((extents, first_bytes)) +} + // UDF 2.50 2.2.10 Metadata File Location: partition-relative block of the Metadata File's File // Entry, at offset 40 of the Type 2 map at `map`. None if the map doesn't fit, or isn't "*UDF // Metadata Partition". @@ -1053,7 +1140,7 @@ const MAX_TOTAL_DIR_ENTRIES: usize = 100_000; fn read_directory( reader: &mut dyn SectorSource, part_start: u32, - meta_start: u32, + meta: &MetaMap, meta_lba: u32, name: &str, depth: u32, @@ -1061,11 +1148,7 @@ fn read_directory( visited: &mut HashSet, ) -> Result { // Read ICB for this directory - let icb_abs = meta_start.checked_add(meta_lba).ok_or(Error::DiscRead { - sector: meta_start as u64, - status: None, - sense: None, - })?; + let icb_abs = meta.to_abs(meta_lba)?; let mut icb = [0u8; 2048]; read_sector(reader, icb_abs, &mut icb)?; @@ -1115,9 +1198,8 @@ fn read_directory( } (icb[ad_off..ad_off + l_ad].to_vec(), l_ad as u32) } else { - // Out-of-line: the FID list may span MULTIPLE ADs (only extent_type 0 is FID data); - // reading just the first AD truncated dirs whose FIDs cross an extent boundary, or - // misread a sparse/type-3 descriptor's (length,LBA) as FIDs. Walk the list, gathering every extent's sectors into one buffer, following type-3 continuations like `read_icb_extents`. + // Out-of-line: the FID list may span several ADs (only type 0 holds FIDs). Gather + // every recorded extent into one buffer, following type-3 continuations. let ad_size: usize = match ad_type { 0 => 8, // short_ad 1 => 16, // long_ad @@ -1137,10 +1219,11 @@ fn read_directory( }); } let mut dir_data: Vec = Vec::new(); - // Declared FID-list byte length (sum of the recorded extents' lengths), - // kept apart from the sector-rounded buffer so `size` and the FID-loop - // bound stay the honest declared length, never the padded buffer. + // Declared FID-list byte length (sum of the recorded extents' lengths). let mut total_len: u32 = 0; + let mut sectors_read: u32 = 0; + let mut seen_aeds: Vec = Vec::new(); + let mut chain_ended = false; let mut block = icb; let mut ad_start = ad_off; let mut ad_bytes = l_ad; @@ -1181,69 +1264,64 @@ fn read_directory( } break; } - // Types 0/1/2 are allocated extents whose sectors hold FID data - // (the 2-bit extent type is masked out of `data_len` above). A - // zero-length descriptor terminates the AD list. + // A zero-length descriptor terminates the AD list. if data_len == 0 { + chain_ended = true; break 'chain; } - // Bound the cumulative allocation before growing: `data_len` is a - // disc-controlled 30-bit value, and multiple extents could - // otherwise sum to a huge buffer. + // Types 1/2 are unrecorded (4/14.14.1.1): no FID data at data_lba. + if extent_type != 0 { + continue; + } + // Bound total sectors READ (not bytes kept): each tiny extent still + // costs a whole sector, and `data_len` is a disc-controlled 30-bit value. let sector_count = data_len.div_ceil(2048); let add = sector_count as usize * 2048; - if dir_data.len().saturating_add(add) > MAX_DIR_BYTES as usize { + sectors_read = sectors_read.saturating_add(sector_count); + if sectors_read > MAX_DIR_BYTES / 2048 { return Err(Error::DiscRead { - sector: meta_start as u64, + sector: meta.start() as u64, status: None, sense: None, }); } - let dir_abs = meta_start.checked_add(data_lba).ok_or(Error::DiscRead { - sector: meta_start as u64, - status: None, - sense: None, - })?; let base = dir_data.len(); dir_data.resize(base + add, 0); for s in 0..sector_count { - let abs = dir_abs.checked_add(s).ok_or(Error::DiscRead { - sector: dir_abs as u64, + let rel = data_lba.checked_add(s).ok_or(Error::DiscRead { + sector: meta.start() as u64, status: None, sense: None, })?; + let abs = meta.to_abs(rel)?; let o = base + s as usize * 2048; read_sector(reader, abs, &mut dir_data[o..o + 2048])?; } + // Drop the sector padding so the next extent's FIDs follow directly. + dir_data.truncate(base + data_len as usize); total_len = total_len.saturating_add(data_len); } match next_block { Some(cont_lba) => { - let abs = meta_start.checked_add(cont_lba).ok_or(Error::DiscRead { - sector: meta_start as u64, - status: None, - sense: None, - })?; - read_sector(reader, abs, &mut block)?; - // A continuation block leads with a 24-byte Allocation Extent - // Descriptor (tag 258); its L_AD is at offset 20 and real ADs - // start at offset 24. - let aed_l_ad = - u32::from_le_bytes([block[20], block[21], block[22], block[23]]) as usize; - ad_start = 24; - ad_bytes = aed_l_ad.min(block.len().saturating_sub(24)); + let abs = meta.to_abs(cont_lba)?; + (ad_start, ad_bytes) = read_aed(reader, abs, &mut block, &mut seen_aeds)?; + } + None => { + chain_ended = true; + break; } - None => break, } } + if !chain_ended { + return Err(Error::UdfAdChainTooLong); + } (dir_data, total_len) }; - // Parse File Identifier Descriptors. The declared FID-list length — never - // the sector-rounded buffer — bounds the walk: trailing padding past it is - // not directory content and must not be read as FIDs. - let dir_end = dir_data.len().min(ad_len as usize); + // Parse File Identifier Descriptors. `dir_data` holds exactly the declared + // FID bytes (padding truncated above), so its length bounds the walk. + let dir_end = dir_data.len(); let mut entries = Vec::new(); let mut pos = 0; @@ -1290,7 +1368,7 @@ fn read_directory( *budget = budget.saturating_add(1); if *budget > MAX_TOTAL_DIR_ENTRIES { return Err(Error::DiscRead { - sector: meta_start as u64, + sector: meta.start() as u64, status: None, sense: None, }); @@ -1299,12 +1377,12 @@ fn read_directory( // Read failures must propagate, not become size 0 (indistinguishable // from a real empty file). `read_file_size` still returns Ok(0) for // ICBs that aren't File/Extended File Entry tags (261/266) — genuine zero. - let file_size = read_file_size(reader, meta_start, icb_lba)?; + let file_size = read_file_size(reader, meta, icb_lba)?; if is_dir && depth < MAX_DIR_DEPTH { // Cycle guard: skip any ICB LBA we have already opened as // a directory (self-referential or cross-linked dirs). - let icb_key = ((meta_start as u64) << 32) | icb_lba as u64; + let icb_key = ((meta.start() as u64) << 32) | icb_lba as u64; if visited.contains(&icb_key) { // Emit as a leaf so the name is preserved but don't // recurse into the cycle. @@ -1320,16 +1398,29 @@ fn read_directory( // Recurse into subdirectory. The depth cap guards against pathological // nesting on a corrupt disc while covering real BD-ROM nesting // (e.g. BDMV/BACKUP/BDJO/*.bdjo is 3 levels deep). - let subdir = read_directory( + let subdir = match read_directory( reader, part_start, - meta_start, + meta, icb_lba, &entry_name, depth + 1, budget, visited, - )?; + ) { + // Unfollowable AD chain: lose only this dir, as Linux/libudfread do. + Err(e @ Error::UdfAdChainTooLong) => { + tracing::warn!(target: "freemkv::udf", code = e.code(), icb_lba, "subdirectory listed empty"); + DirEntry { + name: entry_name, + is_dir: true, + meta_lba: icb_lba, + size: file_size, + entries: Vec::new(), + } + } + other => other?, + }; entries.push(subdir); } } else { @@ -1359,12 +1450,8 @@ fn read_directory( } /// Read file size (info_length) from an Extended File Entry ICB. -fn read_file_size(reader: &mut dyn SectorSource, meta_start: u32, meta_lba: u32) -> Result { - let abs = meta_start.checked_add(meta_lba).ok_or(Error::DiscRead { - sector: meta_start as u64, - status: None, - sense: None, - })?; +fn read_file_size(reader: &mut dyn SectorSource, meta: &MetaMap, meta_lba: u32) -> Result { + let abs = meta.to_abs(meta_lba)?; let mut icb = [0u8; 2048]; read_sector(reader, abs, &mut icb)?; @@ -1412,12 +1499,12 @@ pub(crate) fn parse_udf_name(data: &[u8]) -> String { /// first. Shared range utility — also used to build the disc's encrypted-content /// extent map (see `Disc::encrypted_content_ranges`). pub(crate) fn merge_ranges(ranges: &[(u32, u32)]) -> Vec<(u32, u32)> { - if ranges.is_empty() { - return Vec::new(); - } - let mut result = vec![ranges[0]]; - for &(start, count) in &ranges[1..] { - let last = result.last_mut().unwrap(); + let mut result: Vec<(u32, u32)> = Vec::new(); + for &(start, count) in ranges { + let Some(last) = result.last_mut() else { + result.push((start, count)); + continue; + }; // Saturating arithmetic: ranges derive from disc-controlled ICB // LBAs/lengths, so a corrupt disc could otherwise overflow u32 // (panic in debug, wrap in release). @@ -1440,10 +1527,10 @@ pub(crate) fn merge_ranges(ranges: &[(u32, u32)]) -> Vec<(u32, u32)> { /// Used for Volume Identifier and other UDF descriptor strings. /// The first byte of content is a compression ID: 8 = ASCII, 16 = UTF-16BE. fn parse_dstring(data: &[u8]) -> String { - if data.is_empty() { + let Some(&len) = data.last() else { return String::new(); - } - let len = *data.last().unwrap() as usize; + }; + let len = len as usize; if len == 0 || len > data.len() { return String::new(); } @@ -1621,6 +1708,9 @@ impl SectorSource for BufferedSectorReader<'_> { return Ok(2048); } let block = self.batch; + // Invalidate before the buffer is touched: a failed read below would + // otherwise leave the old window pointing at shrunk/overwritten bytes. + self.cache_sectors = 0; self.cache.resize(block as usize * 2048, 0); match self.inner.read_sectors(lba, block, &mut self.cache, true) { Ok(_) => { @@ -1653,6 +1743,27 @@ fn read_sector(reader: &mut dyn SectorSource, lba: u32, buf: &mut [u8]) -> Resul Ok(()) } +// Load the continuation block a type-3 AD points at: an Allocation Extent Descriptor +// (ECMA-167 4/14.5, tag 258, L_AD @20, ADs from 24). Returns (ad_start, ad_bytes). +// A revisited, non-AED or overrunning block is an unfollowable chain (UdfAdChainTooLong). +fn read_aed( + reader: &mut dyn SectorSource, + abs: u32, + block: &mut [u8; 2048], + seen: &mut Vec, +) -> Result<(usize, usize)> { + if seen.contains(&abs) { + return Err(Error::UdfAdChainTooLong); + } + seen.push(abs); + read_sector(reader, abs, block)?; + let l_ad = u32::from_le_bytes([block[20], block[21], block[22], block[23]]) as usize; + if u16::from_le_bytes([block[0], block[1]]) != 258 || l_ad > block.len() - 24 { + return Err(Error::UdfAdChainTooLong); + } + Ok((24, l_ad)) +} + #[cfg(test)] mod tests { use super::*; @@ -1754,6 +1865,7 @@ mod tests { // prev_allocation_extent_location (@16) and length_of_allocation_descriptors // (@20); ADs begin at offset 24, which the parser skips past. let mut s = [0u8; 2048]; + s[0..2].copy_from_slice(&258u16.to_le_bytes()); let l_ad = (ads.len() * 8) as u32; s[20..24].copy_from_slice(&l_ad.to_le_bytes()); let mut off = 24usize; @@ -1935,7 +2047,7 @@ mod tests { root, volume_id: String::new(), partition_start: part_start, - metadata_start: meta_start, + meta: MetaMap::contiguous(meta_start), metadata_sectors: 0, } } @@ -2294,7 +2406,7 @@ mod tests { }, volume_id: String::new(), partition_start: 0, - metadata_start: 0, + meta: MetaMap::contiguous(0), metadata_sectors: 0, } } @@ -2391,8 +2503,17 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, icb); // directory ICB at meta_start(0) + meta_lba(5) - let err = read_directory(&mut reader, 0, 0, 5, "DIR", 0, &mut 0, &mut HashSet::new()) - .unwrap_err(); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "DIR", + 0, + &mut 0, + &mut HashSet::new(), + ) + .unwrap_err(); assert!(matches!(err, Error::DiscRead { .. })); } @@ -2404,8 +2525,17 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, icb); // directory data at meta_start(0) + ad_pos(50) = 50 reads as zeros. - let dir = read_directory(&mut reader, 0, 0, 5, "DIR", 0, &mut 0, &mut HashSet::new()) - .expect("small dir parses"); + let dir = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "DIR", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("small dir parses"); assert!(dir.entries.is_empty()); assert!(dir.is_dir); } @@ -2545,53 +2675,28 @@ mod tests { ); } - // A ZERO-LENGTH unrecorded descriptor must not cost the file its plan: it displaces - // nothing, so refusing on `!recorded` alone would silently drop whole titles that ripped - // fine before. + // ECMA-167 4/12: an AD whose 30-bit length is 0 ends the list whatever its type + // (Linux udf_get_fileshortad); a zero-length type-1 AD is not a skippable hole. #[test] - fn file_extents_accepts_a_zero_length_unrecorded_extent() { - // A zero-length type-1 hole, then the file's real 4096 bytes. - let icb = build_efe(4096, &[(1, 0, 4999), (0, 4096, 5000)]); - let mut reader = MapReader::new(); - reader.put(5, icb); - let fs = fs_with( - 0, - 0, - DirEntry { - name: String::new(), - is_dir: true, - meta_lba: 0, - size: 0, - entries: vec![file_entry("ZL", 5, 4096)], - }, - ); - - // Fixture check: the hole really is present and really is zero-length, - // or this test proves nothing. - assert_eq!( - fs.extents_abs_at(&mut reader, 5).expect("extents"), - vec![ - AbsExtent { - lba: 4999, - len: 0, - recorded: false - }, - AbsExtent { + fn icb_extents_stop_at_a_zero_length_unrecorded_extent() { + for ext_type in [1u32, 2] { + let icb = build_efe( + 4096, + &[(0, 2048, 5000), (ext_type, 0, 4999), (0, 2048, 6000)], + ); + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with(0, 0, file_entry("ZL", 5, 4096)); + assert_eq!( + fs.extents_abs_at(&mut reader, 5).expect("extents"), + vec![AbsExtent { lba: 5000, - len: 4096, + len: 2048, recorded: true - }, - ], - "fixture must present a ZERO-LENGTH unrecorded extent" - ); - - let plan = fs - .file_extents(&mut reader, "/ZL") - .expect("a zero-length hole displaces nothing, so the file is readable"); - assert!( - plan.iter().any(|&(lba, n)| lba == 5000 && n == 2), - "the real 4096 bytes must still be in the read plan; got {plan:?}" - ); + }], + "type {ext_type}: zero-length AD must terminate the list" + ); + } } #[test] @@ -2816,8 +2921,17 @@ mod tests { // Sanity: with every sector readable the entry parses and carries its // real size, so the failure below is the ICB read and nothing else. - let ok = read_directory(&mut inner, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses when every sector reads"); + let ok = read_directory( + &mut inner, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses when every sector reads"); assert_eq!(ok.entries.len(), 1); assert_eq!(ok.entries[0].size, 123); @@ -2829,8 +2943,17 @@ mod tests { reader.inner.put(60, dir); reader.inner.put(7, build_efe_icb(123, 2048, 0)); - let err = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect_err("an unreadable entry ICB must fail the scan, not report size 0"); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect_err("an unreadable entry ICB must fail the scan, not report size 0"); assert!( matches!(err, Error::DiscRead { sector: 7, .. }), "the propagated error must name the sector that failed, got {err:?}" @@ -2862,8 +2985,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(123, 2048, 0)); // child size ICB - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert_eq!(parsed.entries.len(), 1, "exactly one FID entry"); assert_eq!( parsed.entries[0].name, "CLPI", @@ -2894,8 +3026,17 @@ mod tests { reader.put(60, dir); reader.put(9, build_efe_icb(0, 2048, 0)); // child size ICB - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert!( parsed.entries.is_empty(), "the parent (..) FID must not be emitted even with a valid name" @@ -3341,7 +3482,7 @@ mod tests { }, volume_id: String::new(), partition_start: PART, - metadata_start: PART, + meta: MetaMap::contiguous(PART), metadata_sectors: 4, }; @@ -3496,7 +3637,7 @@ mod tests { let err = read_directory( &mut reader, 0, - 0, + &MetaMap::contiguous(0), 5, "ROOT", 0, @@ -3537,8 +3678,17 @@ mod tests { let root_key: u64 = 5u64; // meta_start=0 → key = (0 << 32) | 5 visited.insert(root_key); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut visited) - .expect("cycle must not blow up"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut visited, + ) + .expect("cycle must not blow up"); // The cyclic entry is emitted as a leaf (no children), not recursed into. assert_eq!(parsed.entries.len(), 1); @@ -3560,8 +3710,17 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, icb); - let err = read_directory(&mut reader, 0, 0, 5, "BDMV", 0, &mut 0, &mut HashSet::new()) - .expect_err("a non-File-Entry directory ICB must fail, not read as an empty directory"); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "BDMV", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect_err("a non-File-Entry directory ICB must fail, not read as an empty directory"); assert!( matches!(err, Error::DiscRead { sector: 5, .. }), "the error must name the ICB sector that carried the bad tag, got {err:?}" @@ -3575,7 +3734,7 @@ mod tests { let dir = read_directory( &mut ok_reader, 0, - 0, + &MetaMap::contiguous(0), 5, "BDMV", 0, @@ -4362,8 +4521,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(4096, 4096, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a File Entry (261) directory ICB is valid and must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a File Entry (261) directory ICB is valid and must be readable"); assert_eq!(child_names(&parsed), vec!["VIDEO_TS.IFO".to_string()]); assert_eq!( parsed.entries[0].size, 4096, @@ -4398,17 +4566,25 @@ mod tests { reader.put(5, build_dir_icb_flagged(3, &fids)); reader.put(7, build_efe_icb(1024, 1024, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("an embedded (AD type 3) directory is valid and must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("an embedded (AD type 3) directory is valid and must be readable"); assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); assert_eq!(parsed.entries[0].size, 1024); } #[test] fn read_directory_reads_fids_spanning_multiple_allocation_descriptors() { - // A directory's FID list can span MORE than one AD (ECMA-167 4/14.14.1); reading - // only the first AD truncated it at the extent boundary, so every recorded extent - // must be gathered and walked. Extent 0 (sector 60) is packed to exactly 2048 bytes so the walk flows into extent 1 (sector 61), where the second file's FID lives. + // A FID list can span several ADs (ECMA-167 4/14.14.1). Extent 0 (sector 60) fills + // exactly 2048 bytes so the walk flows into extent 1 (sector 61) for the 2nd FID. let mut e0 = Vec::new(); push_fid_iu(&mut e0, "", 5, true, true, 0); // parent (..), 40 bytes // Pad FIRST.CLPI's implementation-use area so e0 ends exactly at 2048. @@ -4436,8 +4612,17 @@ mod tests { reader.put(7, build_efe_icb(11, 2048, 0)); reader.put(8, build_efe_icb(22, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a multi-extent directory must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a multi-extent directory must be readable"); assert_eq!( child_names(&parsed), vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], @@ -4482,8 +4667,17 @@ mod tests { reader.put(7, build_efe_icb(11, 2048, 0)); reader.put(8, build_efe_icb(22, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a directory with a continuation AD must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a directory with a continuation AD must be readable"); assert_eq!( child_names(&parsed), vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], @@ -4526,8 +4720,17 @@ mod tests { reader.put(7, build_efe_icb(11, 2048, 0)); reader.put(8, build_efe_icb(22, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("continuation with an AED header must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("continuation with an AED header must be readable"); assert_eq!( child_names(&parsed), vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], @@ -4535,11 +4738,10 @@ mod tests { ); } - // Hostile input: a continuation pointer that cycles to its own block. The - // MAX_AD_BLOCKS budget must bound the walk so it TERMINATES (returns the FIDs - // gathered before the cycle) instead of looping forever or panicking. + // Hostile input: a continuation pointer that cycles to its own block must + // error (not loop, and not re-read the cycle's data extents as duplicates). #[test] - fn read_directory_self_cycling_continuation_terminates() { + fn read_directory_self_cycling_continuation_is_an_error() { let mut e0 = Vec::new(); push_fid_iu(&mut e0, "", 5, true, true, 0); push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 1959); @@ -4555,17 +4757,25 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, build_dir_icb_flagged(0, &body)); reader.put(60, sec0); - // Continuation block 50 points at ITSELF (type-3 → lba 50). - reader.put(50, build_cont_block(&[(3, 2048, 50)])); + // Continuation block 50 holds a data extent, then points at ITSELF. + let (sec1, len1) = fid_sector("SECOND.CLPI", 8); + reader.put(50, build_cont_block(&[(0, len1, 61), (3, 2048, 50)])); + reader.put(61, sec1); reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(8, build_efe_icb(22, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a self-cycling continuation must terminate, not hang or error"); - assert_eq!( - child_names(&parsed), - vec!["FIRST.CLPI".to_string()], - "FIDs before the cycle are returned; the MAX_AD_BLOCKS budget bounds the walk" - ); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect_err("a cyclic AD chain is corruption, like read_icb_extents"); + assert!(matches!(err, Error::UdfAdChainTooLong), "got {err:?}"); } #[test] @@ -4590,8 +4800,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(1024, 1024, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("an extended_ad directory must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("an extended_ad directory must be readable"); assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); } @@ -4616,42 +4835,59 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(1024, 1024, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a long_ad directory must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a long_ad directory must be readable"); assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); assert_eq!(parsed.size, fids.len() as u64); } #[test] - fn read_directory_masks_the_extent_type_bits_out_of_the_ad_length() { - // ECMA-167 4/14.14.1.1: a short_ad's head 32-bit field is a 30-bit length plus a 2-bit - // extent TYPE (bits 30..31), which must be masked off before use as a byte count; - // folding them in yields >= 1 GiB, tripping MAX_DIR_BYTES. Encoded here as type 1. + fn read_directory_masks_type_bits_so_a_zero_length_typed_ad_terminates() { + // ECMA-167 4/14.14.1.1: masking + the zero-length terminator only. A type-1 AD of + // length 0 ends the list; unmasked it reads non-zero and the walk reaches GHOST. + // (The type 1/2 skip: read_directory_does_not_read_unrecorded_extents_as_fids.) let mut fids = Vec::new(); push_fid_iu(&mut fids, "", 5, true, true, 0); - push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 0); + push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 1959); + assert_eq!(fids.len(), 2048); let mut dir = [0u8; 2048]; - dir[..fids.len()].copy_from_slice(&fids); - let raw = 0x4000_0000u32 | fids.len() as u32; + dir.copy_from_slice(&fids); + let (ghost, ghost_len) = fid_sector("GHOST.CLPI", 9); - for tag in [261u16, 266u16] { - let mut reader = MemReader::new(); - reader.put(5, build_dir_icb_tagged(tag, 0, raw, 60)); - reader.put(60, dir); - reader.put(7, build_efe_icb(1024, 1024, 0)); - - let parsed = - read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .unwrap_or_else(|e| { - panic!("tag {tag}: extent-type bits must not inflate the length: {e:?}") - }); - assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); - assert_eq!( - parsed.size, - fids.len() as u64, - "tag {tag}: the reported directory size is the 30-bit length only" - ); + let mut body = Vec::new(); + for (raw, lba) in [(2048u32, 60u32), (1 << 30, 0), (ghost_len, 61)] { + body.extend_from_slice(&raw.to_le_bytes()); + body.extend_from_slice(&lba.to_le_bytes()); } + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, dir); + reader.put(61, ghost); + reader.put(7, build_efe_icb(1024, 1024, 0)); + reader.put(9, build_efe_icb(1024, 1024, 0)); + + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("directory parses"); + assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); + assert_eq!(parsed.size, 2048); } #[test] @@ -4676,8 +4912,17 @@ mod tests { reader.put(7, build_efe_icb(11, 2048, 0)); reader.put(8, build_efe_icb(22, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert_eq!( child_names(&parsed), vec!["A".to_string()], @@ -4709,8 +4954,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(1, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert_eq!( child_names(&parsed), vec!["REAL.MPLS".to_string()], @@ -4736,8 +4990,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(99, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert_eq!(child_names(&parsed), vec!["ENDSATEND".to_string()]); assert_eq!(parsed.entries[0].size, 99); } @@ -4763,8 +5026,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(5, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("an out-of-range File Identifier must end the scan, not fail the read"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("an out-of-range File Identifier must end the scan, not fail the read"); assert_eq!( child_names(&parsed), vec!["OK.MPLS".to_string()], @@ -4791,7 +5063,7 @@ mod tests { let parsed = read_directory( &mut reader, 0, - 0, + &MetaMap::contiguous(0), 5, "ROOT", 0, @@ -4852,7 +5124,7 @@ mod tests { let parsed = read_directory( &mut reader, 0, - 0, + &MetaMap::contiguous(0), 100, "ROOT", 0, @@ -4904,8 +5176,17 @@ mod tests { reader.put(7, build_efe_icb(1, 2048, 0)); reader.put(8, build_efe_icb(2, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("dir parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("dir parses"); assert_eq!( child_names(&parsed), vec!["AAAAA".to_string(), "BBBBB".to_string()], @@ -4930,9 +5211,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(3, 2048, 0)); - let parsed = - read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .unwrap_or_else(|e| panic!("tag {tag}: exact-fit AD must be read: {e:?}")); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .unwrap_or_else(|e| panic!("tag {tag}: exact-fit AD must be read: {e:?}")); assert_eq!(child_names(&parsed), vec!["TIGHT.CLPI".to_string()]); } } @@ -4947,8 +5236,17 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, build_dir_icb_tagged(tag, l_ea, 2048, 60)); - let err = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect_err("an AD past the end of the ICB sector must fail"); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect_err("an AD past the end of the ICB sector must fail"); assert!( matches!(err, Error::DiscRead { sector: 5, .. }), "tag {tag}: the error must name the ICB sector, got {err:?}" @@ -4972,9 +5270,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(6, 2048, 0)); - let parsed = - read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .unwrap_or_else(|e| panic!("tag {tag}: L_EA offset must be honoured: {e:?}")); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .unwrap_or_else(|e| panic!("tag {tag}: L_EA offset must be honoured: {e:?}")); assert_eq!(child_names(&parsed), vec!["WITHEA.MPLS".to_string()]); } } @@ -5000,8 +5306,17 @@ mod tests { reader.put(60, dir); reader.put(7, build_efe_icb(64, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a deleted FID must not fail the enumeration of its directory"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a deleted FID must not fail the enumeration of its directory"); assert_eq!( child_names(&parsed), vec!["LIVE.MPLS".to_string()], @@ -5027,9 +5342,17 @@ mod tests { reader.put(ad_pos, dir); reader.put(7, build_efe_icb(12, 2048, 0)); - let parsed = - read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .unwrap_or_else(|e| panic!("tag {tag}: short_ad must be decoded: {e:?}")); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .unwrap_or_else(|e| panic!("tag {tag}: short_ad must be decoded: {e:?}")); assert_eq!( parsed.size, ad_len as u64, "tag {tag}: every byte of the ExtentLength field is significant" @@ -5051,8 +5374,17 @@ mod tests { let mut reader = MemReader::new(); reader.put(5, build_dir_icb_tagged(tag, 0, 0x0100_0000, 60)); - let err = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect_err("a 16 MiB directory length must be refused, not read as empty"); + let err = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect_err("a 16 MiB directory length must be refused, not read as empty"); assert!( matches!(err, Error::DiscRead { .. }), "tag {tag}: got {err:?}" @@ -5075,8 +5407,17 @@ mod tests { reader.put(60, dir); // remaining sectors read as zeros → scan ends reader.put(7, build_efe_icb(4, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a directory exactly at the size ceiling must be readable"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a directory exactly at the size ceiling must be readable"); assert_eq!(child_names(&parsed), vec!["00000.M2TS".to_string()]); } @@ -5096,10 +5437,399 @@ mod tests { reader.put(61, [0u8; 2048]); reader.put(7, build_efe_icb(8, 2048, 0)); - let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) - .expect("a two-sector directory parses"); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a two-sector directory parses"); assert_eq!(child_names(&parsed), vec!["FIRST.CLPI".to_string()]); } + + /// Serves `CountReader::expected` data, except reads touching LBA >= `bad` + /// scribble 0xEE into the buffer and fail (a scratched region). + struct FailingTailReader { + bad: u32, + } + + impl SectorSource for FailingTailReader { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + _recovery: bool, + ) -> Result { + let need = count as usize * 2048; + if buf.len() < need { + return Err(Error::UdfBufferTooSmall); + } + if lba.saturating_add(count as u32) > self.bad { + buf[..need].fill(0xEE); + return Err(Error::UdfBufferTooSmall); + } + for i in 0..count as u32 { + let off = i as usize * 2048; + buf[off..off + 2048].copy_from_slice(&CountReader::expected(lba + i)); + } + Ok(need) + } + } + + // A failed miss (batch AND single retry fail) must not leave the old window + // marked valid over a shrunk/overwritten buffer: the next hit panicked slicing + // past the 2048-byte cache, or served the failed read's bytes. + #[test] + fn buffered_reader_failed_miss_invalidates_the_window() { + let mut inner = FailingTailReader { bad: 100 }; + let mut br = BufferedSectorReader::new(&mut inner, 8); + let mut buf = [0u8; 2048]; + br.read_sectors(0, 1, &mut buf, true) + .expect("seed window 0..8"); + assert!(br.read_sectors(100, 1, &mut buf, true).is_err()); + br.read_sectors(5, 1, &mut buf, true) + .expect("sector 5 is readable"); + assert_eq!(buf, CountReader::expected(5), "stale window served"); + } + + // Offset 0 of the old window stays in bounds after the shrink, so a failed + // miss served the failed read's junk instead of panicking. + #[test] + fn buffered_reader_failed_miss_does_not_serve_its_junk_from_the_old_window() { + let mut inner = FailingTailReader { bad: 100 }; + let mut br = BufferedSectorReader::new(&mut inner, 8); + let mut buf = [0u8; 2048]; + br.read_sectors(0, 1, &mut buf, true) + .expect("seed window 0..8"); + assert!(br.read_sectors(100, 1, &mut buf, true).is_err()); + br.read_sectors(0, 1, &mut buf, true).expect("sector 0"); + assert_eq!(buf, CountReader::expected(0), "failed read's bytes served"); + } + + // Build a sector holding one FID for `name` whose ICB is at `icb`. + fn fid_sector(name: &str, icb: u32) -> ([u8; 2048], u32) { + let mut v = Vec::new(); + push_fid_iu(&mut v, name, icb, false, false, 0); + let mut s = [0u8; 2048]; + s[..v.len()].copy_from_slice(&v); + (s, v.len() as u32) + } + + // ECMA-167 4/14.14.1.1: extent types 1 (allocated, not recorded) and 2 (not + // allocated) hold no FID data; their LBA must not be read as directory content. + #[test] + fn read_directory_does_not_read_unrecorded_extents_as_fids() { + for ext_type in [1u32, 2] { + let mut e0 = Vec::new(); + push_fid_iu(&mut e0, "", 5, true, true, 0); + push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 1959); + assert_eq!(e0.len(), 2048); + let mut sec0 = [0u8; 2048]; + sec0.copy_from_slice(&e0); + let (ghost, _) = fid_sector("GHOST.CLPI", 9); + let (sec1, len1) = fid_sector("SECOND.CLPI", 8); + + let mut body = Vec::new(); + for (raw, lba) in [(2048u32, 60u32), ((ext_type << 30) | 2048, 62), (len1, 61)] { + body.extend_from_slice(&raw.to_le_bytes()); + body.extend_from_slice(&lba.to_le_bytes()); + } + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, sec0); + reader.put(61, sec1); + reader.put(62, ghost); + reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(8, build_efe_icb(22, 2048, 0)); + reader.put(9, build_efe_icb(33, 2048, 0)); + + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("directory with an unrecorded extent parses"); + assert_eq!( + child_names(&parsed), + vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], + "type {ext_type} extent was read as FID data" + ); + assert_eq!(parsed.size, 2048 + len1 as u64); + } + } + + // A non-final extent shorter than a sector must not leave sector padding + // between it and the next extent's FIDs (the walk stopped at the padding). + #[test] + fn read_directory_joins_a_short_non_final_extent_without_padding() { + let mut e0 = Vec::new(); + push_fid_iu(&mut e0, "", 5, true, true, 0); + push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 0); + let len0 = e0.len() as u32; + let mut sec0 = [0u8; 2048]; + sec0[..e0.len()].copy_from_slice(&e0); + let (sec1, len1) = fid_sector("SECOND.CLPI", 8); + + let mut body = Vec::new(); + for (raw, lba) in [(len0, 60u32), (len1, 61)] { + body.extend_from_slice(&raw.to_le_bytes()); + body.extend_from_slice(&lba.to_le_bytes()); + } + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, sec0); + reader.put(61, sec1); + reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(8, build_efe_icb(22, 2048, 0)); + + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("multi-extent directory parses"); + assert_eq!( + child_names(&parsed), + vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()] + ); + assert_eq!(parsed.size, (len0 + len1) as u64); + } + + /// `n` short ADs of (type, len, lba), then an optional type-3 pointer. + fn short_ads(ads: &[(u32, u32, u32)]) -> Vec { + let mut v = Vec::new(); + for &(t, len, lba) in ads { + v.extend_from_slice(&((t << 30) | len).to_le_bytes()); + v.extend_from_slice(&lba.to_le_bytes()); + } + v + } + + // Each 1-byte extent costs a full sector read; without a cap on sectors read, + // hundreds of tiny extents bypass MAX_DIR_BYTES (which counts only bytes kept). + #[test] + fn read_directory_caps_sector_reads_across_many_tiny_extents() { + let mut icb_ads = vec![(0u32, 1u32, 60u32); 228]; + icb_ads.push((3, 2048, 50)); + let mut cont_a = vec![(0u32, 1u32, 60u32); 252]; + cont_a.push((3, 2048, 51)); + let cont_b = vec![(0u32, 1u32, 60u32); 252]; + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &short_ads(&icb_ads))); + reader.put(50, build_cont_block(&cont_a)); + reader.put(51, build_cont_block(&cont_b)); + let res = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ); + assert!( + res.is_err(), + "732 one-byte extents read 732 sectors past the 512-sector budget" + ); + } + + // ECMA-167 4/14.5: a continuation block must be an Allocation Extent Descriptor. + #[test] + fn read_directory_rejects_a_continuation_block_that_is_not_an_aed() { + let (sec1, len1) = fid_sector("SECOND.CLPI", 8); + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &short_ads(&[(3, 2048, 50)]))); + let mut cont = build_cont_block(&[(0, len1, 61)]); + cont[0..2].copy_from_slice(&0u16.to_le_bytes()); + reader.put(50, cont); + reader.put(61, sec1); + reader.put(8, build_efe_icb(22, 2048, 0)); + let res = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ); + assert!(res.is_err(), "non-AED continuation accepted: {res:?}"); + } + + #[test] + fn icb_extents_reject_a_continuation_block_that_is_not_an_aed() { + let icb = build_efe(4096, &[(0, 2048, 10), (3, 2048, 50)]); + let mut cont = build_cont_block(&[(0, 2048, 20)]); + cont[0..2].copy_from_slice(&0u16.to_le_bytes()); + let mut reader = MapReader::new(); + reader.put(5, icb); + reader.put(50, cont); + let fs = fs_with(0, 0, file_entry("F", 5, 4096)); + let res = fs.read_icb_extents(&mut reader, 5); + assert!(res.is_err(), "non-AED continuation accepted: {res:?}"); + } + + /// Rewrite the Metadata File FE of a `build_meta_vol` volume with `ads`. + fn put_meta_fe(disc: &mut fixture::MemDisc, spec: &MetaVol, ads: &[(u32, u32, u32)]) { + let body = short_ads(ads); + let mut fe = vec![0u8; 2048]; + fe[0..2].copy_from_slice(&266u16.to_le_bytes()); + fe[208..212].copy_from_slice(&(spec.l_ea as u32).to_le_bytes()); + fe[212..216].copy_from_slice(&(body.len() as u32).to_le_bytes()); + let ad = 216 + spec.l_ea; + fe[ad..ad + body.len()].copy_from_slice(&body); + disc.put_bytes(fixture::PART_START + spec.meta_fe_at, &fe); + } + + // The Metadata File's first AD defines metadata_start; an unrecorded (1/2) or + // continuation (3) AD holds no metadata there, so it must not be mounted. + #[test] + fn read_filesystem_rejects_a_metadata_file_whose_first_extent_is_not_recorded() { + for t in [1u32, 2, 3] { + let spec = conformant_meta_vol(); + let (mut disc, _) = build_meta_vol(&spec); + put_meta_fe(&mut disc, &spec, &[(t, spec.meta_bytes, spec.meta_pos)]); + let res = super::read_filesystem(&mut disc); + assert!(res.is_err(), "type {t} first metadata AD mounted"); + } + } + + // A fragmented Metadata File: metadata blocks past the first extent live in + // the second extent, not contiguously after the first. + #[test] + fn read_filesystem_maps_metadata_blocks_through_every_metadata_file_extent() { + use fixture::PART_START; + let spec = conformant_meta_vol(); + let (mut disc, meta_start) = build_meta_vol(&spec); + // Extent 0 = metadata blocks 0..4 (FSD, root ICB); extent 1 = blocks 4.. at pos2. + let pos2 = spec.meta_pos + 100_000; + let mut fids = Vec::new(); + push_fid_iu(&mut fids, "", MV_ROOT_ICB, true, true, 0); + push_fid_iu(&mut fids, "INDEX.BDMV", MV_FILE_ICB, false, false, 0); + disc.put_bytes(meta_start + MV_ROOT_DATA, &[0u8; 2048]); + disc.put_bytes(meta_start + MV_FILE_ICB, &[0u8; 2048]); + disc.put_bytes(PART_START + pos2, &fids); + disc.put_bytes( + PART_START + pos2 + 1, + &build_efe_icb(MV_FILE_SIZE, MV_FILE_SIZE as u32, 20), + ); + put_meta_fe( + &mut disc, + &spec, + &[(0, 4 * 2048, spec.meta_pos), (0, 4 * 2048, pos2)], + ); + let fs = super::read_filesystem(&mut disc).expect("fragmented metadata file mounts"); + assert_eq!(child_names(&fs.root), vec!["INDEX.BDMV".to_string()]); + assert_eq!(fs.root.entries[0].size, MV_FILE_SIZE); + } + + // ECMA-167 3/10.6.13: the LVD's Logical Volume Contents Use (long_ad @248) + // records where the File Set Descriptor is; it need not be block 0. + #[test] + fn read_filesystem_reads_the_fsd_where_the_lvd_records_it() { + let spec = conformant_meta_vol(); + let (mut disc, meta_start) = build_meta_vol(&spec); + let mut fsd = vec![0u8; 2048]; + fsd[0..2].copy_from_slice(&256u16.to_le_bytes()); + fsd[404..408].copy_from_slice(&MV_ROOT_ICB.to_le_bytes()); + disc.put_bytes(meta_start, &[0u8; 2048]); + disc.put_bytes(meta_start + 2, &fsd); + let mut lvd = vec![0u8; 2048]; + let mut one = [0u8; 2048]; + disc.read_sectors(34, 1, &mut one, true).expect("lvd"); + lvd.copy_from_slice(&one); + lvd[248..252].copy_from_slice(&2048u32.to_le_bytes()); + lvd[252..256].copy_from_slice(&2u32.to_le_bytes()); + lvd[256..258].copy_from_slice(&1u16.to_le_bytes()); + disc.put_bytes(34, &lvd); + let fs = super::read_filesystem(&mut disc).expect("FSD at LVD-recorded block mounts"); + assert_eq!(child_names(&fs.root), vec!["INDEX.BDMV".to_string()]); + } + + // Only the FIRST Metadata File AD must be recorded; a later unrecorded AD ends + // the map rather than failing a mount that worked before extents were honoured. + #[test] + fn read_filesystem_mounts_a_metadata_file_with_a_trailing_unrecorded_extent() { + let spec = conformant_meta_vol(); + let (mut disc, _) = build_meta_vol(&spec); + put_meta_fe( + &mut disc, + &spec, + &[(0, spec.meta_bytes, spec.meta_pos), (1, 2048, 5)], + ); + let fs = super::read_filesystem(&mut disc).expect("trailing type-1 AD mounts"); + assert_eq!(child_names(&fs.root), vec!["INDEX.BDMV".to_string()]); + } + + // A subdirectory whose AD chain can't be followed is listed empty (as Linux and + // libudfread lose only that dir), so e.g. a broken BACKUP can't block the mount. + #[test] + fn read_directory_lists_a_subdirectory_with_a_broken_ad_chain_as_empty() { + let mut fids = Vec::new(); + push_fid_iu(&mut fids, "", 5, true, true, 0); + push_fid_iu(&mut fids, "BACKUP", 9, true, false, 0); + push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 0); + let mut sec = [0u8; 2048]; + sec[..fids.len()].copy_from_slice(&fids); + let mut reader = MemReader::new(); + reader.put( + 5, + build_dir_icb_flagged(0, &short_ads(&[(0, fids.len() as u32, 60)])), + ); + reader.put(60, sec); + reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(9, build_dir_icb_flagged(0, &short_ads(&[(3, 2048, 50)]))); + let mut not_aed = build_cont_block(&[(0, 2048, 61)]); + not_aed[0..2].copy_from_slice(&0u16.to_le_bytes()); + reader.put(50, not_aed); + let parsed = read_directory( + &mut reader, + 0, + &MetaMap::contiguous(0), + 5, + "ROOT", + 0, + &mut 0, + &mut HashSet::new(), + ) + .expect("a broken subdirectory must not fail the parent"); + assert_eq!( + child_names(&parsed), + vec!["BACKUP".to_string(), "INDEX.BDMV".to_string()] + ); + assert!(parsed.entries[0].is_dir && parsed.entries[0].entries.is_empty()); + } + + // An AED whose L_AD runs past its block is corrupt (Linux/libudfread reject it). + #[test] + fn icb_extents_reject_an_aed_whose_l_ad_overruns_the_block() { + let icb = build_efe(4096, &[(0, 2048, 10), (3, 2048, 50)]); + let mut cont = build_cont_block(&[(0, 2048, 20)]); + cont[20..24].copy_from_slice(&2025u32.to_le_bytes()); + let mut reader = MapReader::new(); + reader.put(5, icb); + reader.put(50, cont); + let fs = fs_with(0, 0, file_entry("F", 5, 4096)); + let res = fs.read_icb_extents(&mut reader, 5); + assert!(res.is_err(), "overrunning L_AD accepted: {res:?}"); + } } // Shared UDF image fixtures for tests across the disc::* format scanners: