From 9de88969caf7f018f70983cc2a3d7d84aa2d2359 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:13:33 -0700 Subject: [PATCH] test: constrain the DiscStream loss surface and the empty-title guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second mutation pass over src/mux/. 26 survivors killed, no production change. Verified on HEAD before landing: each mutation below passes all 1,237 mux tests unmutated-suite. The priority item was the honest-loss-reporting surface. Both DiscStream::errors and DiscStream::lost_bytes could return a constant with nothing failing — a rip that lost sectors would report zero loss to the caller. This project has already shipped one defect of that shape (a total decryption failure reported as an empty title, exit 0). Driven now through two short-read fills so both land on values that are neither 0 nor 1 and differ from each other; no constant and no field swap survives. MkvStream::finish -> Ok(()) also survived. MkvMuxer::finish has the zero-frame MkvInvalid guard and two tests cover it, but the Stream wrapper above it could return Ok unconditionally and bypass the guard entirely — the empty-title defence was one layer thinner than it looked. au_assembly: pinned au_opener_from behaviourally to the normative byte values for all four modes, with negative cases for codes that are explicitly not openers (MPEG-2 slice 0x01..0xAF, user data 0xB2, extension 0xB5, sequence end 0xB7 per 13818-2 Table 6-1; VC-1 0x0A/0x0B/0x0C; H.264 SPS/PPS/IDR-slice). au_assembly and codec/ hold independent copies of these constants; they agree today, and comparing constants would not catch logic drifting apart, so both sides are now pinned to the spec instead of to each other. demux_sink::sanitize: every filename component demux:// writes comes from disc-controlled text, so the path-separator arm is a traversal guard. Deleting it now fails, including an end-to-end case where base = "../evil/Title" must produce exactly one file inside the chosen directory. stts_and_ctts_expand renamed to stts_expands_runs_to_per_sample_deltas_in_order and given runs with distinct deltas AND distinct lengths. Its old name claimed ctts coverage it never had, which is why the composition-time chain went unconstrained for eight rounds; the doc comment now points at the tests that do cover ctts. Correction to the previous pass: codec/truehd.rs flush -> vec![] IS equivalent. Applied it, full mux suite green. TrueHD buffers across PES but parse emits every complete unit immediately, so a residual buffer at EOF is a truncated access unit and is correctly discarded. The vec![Default::default()] variants are genuinely different and are killed. Deliberately not constrained: mkv::set_opening_capture (diagnostics behind a process-global tracing check, flaky under the parallel runner), and the three stdio.rs header paths (StdioStream holds concrete io::Stdin/Stdout and cannot be driven without a production refactor to injectable Read/Write). --- src/mux/au_assembly.rs | 233 +++++++++++++++++++ src/mux/codec/flac.rs | 30 +++ src/mux/codec/hevc.rs | 96 ++++++++ src/mux/codec/mpegaudio.rs | 32 +++ src/mux/demux_sink.rs | 161 ++++++++++++++ src/mux/disc.rs | 445 +++++++++++++++++++++++++++++++++++++ src/mux/hevc/mod.rs | 54 +++++ src/mux/m2ts_mux/packet.rs | 52 +++++ src/mux/meta_sink.rs | 49 ++++ src/mux/mkvstream.rs | 200 +++++++++++++++++ src/mux/mp4/read.rs | 42 +++- 11 files changed, 1387 insertions(+), 7 deletions(-) diff --git a/src/mux/au_assembly.rs b/src/mux/au_assembly.rs index 2bce747..70493aa 100644 --- a/src/mux/au_assembly.rs +++ b/src/mux/au_assembly.rs @@ -1001,4 +1001,237 @@ mod tests { after a large one: {hits} copies over 20 access units" ); } + + // ── AU-opener detection: the per-mode start-code rule ───────────────── + // + // `au_opener_from` is the SECOND implementation of a rule each codec parser + // also encodes (h264 `NAL_AUD`, hevc `NAL_AUD`, vc1 `SC_*`, mpeg2 + // `PICTURE_CODE`/`SEQ_HEADER_CODE`/`GOP_CODE`). Two independent copies of one + // rule drift; these cases pin this copy to the normative byte values and to + // the codes that are explicitly NOT openers, so a drift shows up here. + + /// The opener offset must be the position of the real start code, never a + /// fixed 0. A constant `Some(0)` makes every pre-sync run of junk bytes look + /// like the head of an access unit, so the first AU of every stream that does + /// not begin exactly on a start code is emitted with junk glued to its front. + #[test] + fn au_opener_from_locates_the_real_start_code_per_codec() { + // Junk that contains a start-code PREFIX but no opener suffix, so a + // scanner that stopped at `00 00 01` alone would answer wrongly. + let junk: &[u8] = &[0xFF, 0x00, 0x00, 0x01, 0x67, 0xAA]; + let cases: &[(Mode, u8, &str)] = &[ + // ISO/IEC 14496-10 §7.4.1: nal_unit_type 9 = access unit delimiter, + // and nal_ref_idc shall be 0 for it, so the header byte is 0x09. + (Mode::StartCode(0x09), 0x09, "H.264 AUD"), + // ITU-T H.265 §7.4.2.2: nal_unit_type 35 = AUD_NUT. The first NAL + // header byte is forbidden_zero_bit(1) | nal_unit_type(6) | + // nuh_layer_id MSB(1) = (35 << 1) = 0x46 on the base layer. + (Mode::StartCode(0x46), 0x46, "HEVC AUD"), + // SMPTE 421M Annex E BDU types. + (Mode::Vc1, VC1_SEQ, "VC-1 sequence header"), + (Mode::Vc1, VC1_ENTRY, "VC-1 entry point"), + (Mode::Vc1, VC1_FRAME, "VC-1 frame"), + // ISO/IEC 13818-2 §6.2.1 Table 6-1 start code values. + (Mode::Mpeg2, MP2_PICTURE, "MPEG-2 picture"), + (Mode::Mpeg2, MP2_SEQ, "MPEG-2 sequence header"), + (Mode::Mpeg2, MP2_GOP, "MPEG-2 GOP header"), + ]; + for &(mode, code, what) in cases { + let mut buf = junk.to_vec(); + buf.extend_from_slice(&[0x00, 0x00, 0x01, code, 0x5A]); + assert_eq!( + au_opener_from(mode, &buf, 0), + Some(junk.len()), + "{what}: opener must be found at the start code, not at 0" + ); + // `from` must actually skip: searching past the only opener finds none. + assert_eq!( + au_opener_from(mode, &buf, junk.len() + 1), + None, + "{what}: the resume cursor must be honoured" + ); + } + } + + /// Start codes that are NOT access-unit openers must not be reported as one. + /// Treating a slice or an extension header as an AU start splits one coded + /// picture into several frames, each missing its picture header. + #[test] + fn non_opening_start_codes_are_not_au_openers() { + // ISO/IEC 13818-2 Table 6-1: slice (0x01..=0xAF), user data (0xB2), + // extension (0xB5), sequence end (0xB7) all appear INSIDE an access unit. + for code in [0x01u8, 0xAF, 0xB2, 0xB5, 0xB7] { + let buf = [0x00, 0x00, 0x01, code, 0x11, 0x22]; + assert_eq!( + au_opener_from(Mode::Mpeg2, &buf, 0), + None, + "MPEG-2 start code {code:#04x} must not open an access unit" + ); + } + // SMPTE 421M: slice (0x0B) and field (0x0C) BDUs belong to the frame + // already in progress; end-of-sequence (0x0A) opens nothing. + for code in [0x0Au8, 0x0B, 0x0C] { + let buf = [0x00, 0x00, 0x01, code, 0x11, 0x22]; + assert_eq!( + au_opener_from(Mode::Vc1, &buf, 0), + None, + "VC-1 BDU {code:#04x} must not open an access unit" + ); + } + // H.264: an SPS (7) / PPS (8) / IDR slice (5) is not the AU DELIMITER the + // StartCode mode splits on. + for code in [0x05u8, 0x67, 0x68] { + let buf = [0x00, 0x00, 0x01, code, 0x11, 0x22]; + assert_eq!(au_opener_from(Mode::StartCode(0x09), &buf, 0), None); + } + // Passthrough never frames — the codec self-frames. + assert_eq!( + au_opener_from(Mode::Passthrough, &[0, 0, 1, 0x09, 0xAA], 0), + None + ); + } + + /// `au_opener_resumable` must return the true offset AND advance + /// `opener_pos` only over bytes that cannot hide a straddling start code. + /// A constant `Some(0)` short-circuits both. + #[test] + fn au_opener_resumable_reports_the_real_offset_and_resumes_safely() { + let mut a = AuAssembler::for_codec(Codec::H264); + + // A junk run with no opener: None, and the cursor parks 3 bytes back so a + // start code split across the append boundary is still found. + a.buf.extend_from_slice(&[0xFFu8; 32]); + assert_eq!(a.au_opener_resumable(), None, "no opener in a junk run"); + assert_eq!( + a.opener_pos, 29, + "resume 3 bytes back for a straddling code" + ); + + // Now append a start code that STRADDLES the previous end: the first three + // bytes of `00 00 01 09` land at offsets 29..32. + a.buf.truncate(29); + a.buf.extend_from_slice(&[0x00, 0x00, 0x01, 0x09, 0x77]); + assert_eq!( + a.au_opener_resumable(), + Some(29), + "a start code straddling the previous scan end must still be found" + ); + } + + /// After the pre-sync bytes are discarded, the emitted AU must take the + /// timing of the fragment that ACTUALLY opened it. `drop_marks_before` is + /// what retires the discarded fragment's marks; a no-op there stamps the + /// first real access unit with the PTS and source of bytes that were thrown + /// away — a whole-title A/V sync offset, since every later frame is timed + /// relative to it. + #[test] + fn discarded_pre_sync_marks_do_not_time_the_first_access_unit() { + let src = |b: u64| SourcePos { + byte: b, + ..Default::default() + }; + let mut a = AuAssembler::for_codec(Codec::H264); + + // Fragment 1: pre-sync junk, no start code. Carries its own PTS/source. + assert!( + a.push(&[0xFFu8; 24], Some(1_000), Some(900), Some(src(11)), false) + .is_empty() + ); + // Fragment 2: the first real AU opener, with the timing that belongs to it. + assert!( + a.push( + &au(0x33, 40), + Some(2_000), + Some(1_900), + Some(src(22)), + false + ) + .is_empty() + ); + // Fragment 3: a second AU, closing the first. + let out = a.push( + &au(0x44, 40), + Some(3_000), + Some(2_900), + Some(src(33)), + false, + ); + + assert_eq!(out.len(), 1, "the first AU closes on the second opener"); + assert_eq!(out[0].data, au(0x33, 40), "junk discarded, AU intact"); + assert_eq!( + out[0].pts, + Some(2_000), + "the AU must take the opening fragment's PTS, not the discarded junk's" + ); + assert_eq!(out[0].dts, Some(1_900), "same for DTS"); + assert_eq!( + out[0].source.map(|s| s.byte), + Some(22), + "same for the source position used by the recovery map" + ); + + let tail = a.flush(); + assert_eq!(tail.len(), 1); + assert_eq!(tail[0].pts, Some(3_000), "the second AU keeps its own PTS"); + } + + /// `for_codec` is the dispatch that decides whether a stream is REASSEMBLED + /// across PES fragments or passed straight through. Getting it wrong is + /// silent: an H.264/HEVC/VC-1 stream routed to `Passthrough` on a program + /// source emits one "frame" per PES fragment — a few hundred bytes of a + /// coded picture, framed as a whole access unit — and the output plays as + /// corruption, not as an error. + /// + /// Each mode is identified BEHAVIOURALLY (feed a two-AU stream in two halves + /// and see whether it reassembles), so the case cannot pass by matching a + /// constant. + #[test] + fn for_codec_routes_each_video_codec_to_its_reassembly_mode() { + // Buffering codecs: a stream split mid-AU must NOT emit until the second + // AU's opener arrives, and must then emit the FIRST AU whole. + let buffering: &[(Codec, u8)] = &[ + (Codec::H264, 0x09), // ISO/IEC 14496-10 §7.4.1 AUD + (Codec::Hevc, 0x46), // ITU-T H.265 §7.4.2.2 AUD_NUT, (35 << 1) + ]; + for &(codec, marker) in buffering { + let mut a = AuAssembler::for_codec(codec); + let mut unit = vec![0x00, 0x00, 0x01, marker]; + unit.extend(std::iter::repeat_n(0x5Au8, 30)); + // First half of AU 1: nothing complete yet. + assert!( + a.push(&unit[..20], Some(1), None, None, false).is_empty(), + "{codec:?} must buffer a partial access unit, not emit it" + ); + assert!( + a.push(&unit[20..], None, None, None, false).is_empty(), + "{codec:?} must hold AU 1 until the next opener" + ); + // AU 2's opener closes AU 1. + let out = a.push(&unit, Some(2), None, None, false); + assert_eq!(out.len(), 1, "{codec:?} emits exactly one AU"); + assert_eq!(out[0].data, unit, "{codec:?} reassembles AU 1 whole"); + assert_eq!(out[0].pts, Some(1), "{codec:?} carries the AU-start PTS"); + } + + // VC-1 buffers too, on its own boundary rule (no single AU delimiter). + let mut a = AuAssembler::for_codec(Codec::Vc1); + let frame = bdu(VC1_FRAME, 0x77, 30); + assert!(a.push(&frame, Some(1), None, None, false).is_empty()); + assert_eq!( + a.push(&frame, Some(2), None, None, false).len(), + 1, + "VC-1 emits AU 1 when the next frame BDU opens AU 2" + ); + + // Self-framing codecs pass each fragment through immediately — the same + // half-AU input that the buffering modes held back comes straight out. + for codec in [Codec::Mpeg2, Codec::Ac3, Codec::TrueHd, Codec::Pgs] { + let mut a = AuAssembler::for_codec(codec); + let out = a.push(&[0x00, 0x00, 0x01, 0x09, 0xAA], Some(7), None, None, false); + assert_eq!(out.len(), 1, "{codec:?} must pass through, not buffer"); + assert_eq!(out[0].pts, Some(7)); + assert!(a.flush().is_empty(), "{codec:?} buffers nothing at EOF"); + } + } } diff --git a/src/mux/codec/flac.rs b/src/mux/codec/flac.rs index 9f58ede..b903859 100644 --- a/src/mux/codec/flac.rs +++ b/src/mux/codec/flac.rs @@ -241,4 +241,34 @@ mod tests { let mut p = FlacParser::new(); assert!(p.parse(&make_pes(Vec::new(), Some(0))).is_empty()); } + + /// FLAC packets are self-framing: `parse` emits or drops each one on the + /// spot and buffers nothing, so `flush` has nothing to deliver. A + /// manufactured tail frame would be a zero-length block at PTS 0 appended + /// after the track's real end — a backwards timestamp (RFC 9559 §5.1.3.2) + /// carrying no decodable FLAC frame. + #[test] + fn flush_adds_no_phantom_frame_after_the_last_real_packet() { + let mut p = FlacParser::new(); + let mut emitted = Vec::new(); + emitted.extend(p.parse(&make_pes(make_flac_frame(100), Some(90_000)))); + emitted.extend(p.parse(&make_pes(make_flac_frame(120), Some(180_000)))); + // A frame whose CRC-16 residue is nonzero is dropped, not buffered. + let mut corrupt = make_flac_frame(100); + let last = corrupt.len() - 1; + corrupt[last] ^= 0xFF; + emitted.extend(p.parse(&make_pes(corrupt, Some(270_000)))); + assert_eq!(emitted.len(), 2, "two valid frames out, one dropped"); + assert_eq!(p.dropped_frames(), 1); + + let tail = p.flush(); + assert!( + tail.is_empty(), + "nothing is buffered past the last packet; flush produced {:?}", + tail.iter() + .map(|f| (f.pts_ns, f.data.len())) + .collect::>() + ); + assert_eq!(emitted.len() + tail.len(), 2); + } } diff --git a/src/mux/codec/hevc.rs b/src/mux/codec/hevc.rs index fe67874..56381a6 100644 --- a/src/mux/codec/hevc.rs +++ b/src/mux/codec/hevc.rs @@ -3233,4 +3233,100 @@ mod tests { "oversized param set must not produce a (truncated) hvcC" ); } + + /// HEVC counterpart of `h264_ps_reorder_reconstructs_distinct_display_pts`. + /// + /// A DVD/HD-DVD program stream stamps a PTS only on each GOP anchor, so the + /// parser must reconstruct display-order timestamps for the rest. Two things + /// are pinned here that nothing else constrained: + /// + /// * `with_ps_reorder(true)` must actually INSTALL the reorderer (a builder + /// that returned a default-constructed parser would silently leave the + /// transport-stream path in place on every DVD title), and + /// * `flush()` must drain the reorderer's real buffered frames at EOF — + /// not nothing (the tail of every title lost) and not a manufactured + /// empty frame at PTS 0 (a zero-length Block whose timestamp jumps back + /// behind every cluster already written, RFC 9559 §5.1.3.2). + /// + /// Slice bodies follow H.265 §7.3.6.1: `first_slice_segment_in_pic_flag`, + /// the IRAP-only `no_output_of_prior_pics_flag`, `ue(pps_id)`, `ue(slice_type)` + /// (Table 7-7: 0 = B, 1 = P, 2 = I). PPS body 0xC0 sets + /// `num_extra_slice_header_bits = 0`. + #[test] + fn hevc_ps_reorder_is_installed_and_flush_drains_its_real_frames() { + const NAL_IDR_W_RADL: u8 = 19; // IRAP → keyframe / GOP anchor + const NAL_TRAIL_R: u8 = 1; // non-IRAP coded slice + let nal = |t: u8, body: u8| { + let mut v = vec![0x00, 0x00, 0x01]; + v.extend_from_slice(&hevc_nal_header(t)); + v.push(body); + v + }; + // One access unit = active PPS + one coded slice, in one PES. + let au = |t: u8, body: u8| { + let mut d = nal(NAL_PPS, 0xC0); + d.extend_from_slice(&nal(t, body)); + d + }; + // Decode order of a classic single-B GOP: I P B P B. + let gop = |anchor: Option| { + vec![ + (au(NAL_IDR_W_RADL, 0xAC), anchor), // I, IRAP anchor + (au(NAL_TRAIL_R, 0xD0), None), // P + (au(NAL_TRAIL_R, 0xE0), None), // B + (au(NAL_TRAIL_R, 0xD0), None), // P + (au(NAL_TRAIL_R, 0xE0), None), // B + ] + }; + + let feed = |reorder: bool| -> (Vec, Vec) { + let mut p = HevcParser::new().with_ps_reorder(reorder); + let mut during = Vec::new(); + // Two GOPs; the second anchor is 5 frames later (90 kHz: 5 × 3750). + for (data, pts) in gop(Some(0)).into_iter().chain(gop(Some(18_750))) { + during.extend(p.parse(&make_pes(data, pts))); + } + let tail = p.flush(); + (during, tail) + }; + + let (during, tail) = feed(true); + assert!( + !tail.is_empty(), + "the reorderer holds frames back; flush must release them" + ); + for f in &tail { + assert!( + !f.data.is_empty(), + "a flushed frame carries real coded bytes, never a manufactured empty one" + ); + } + let all: Vec<&Frame> = during.iter().chain(tail.iter()).collect(); + assert_eq!(all.len(), 10, "every access unit is emitted exactly once"); + let mut pts: Vec = all.iter().map(|f| f.pts_ns).collect(); + let n = pts.len(); + pts.sort_unstable(); + pts.dedup(); + assert_eq!( + pts.len(), + n, + "reconstructed PTS are all distinct (no DTS collision)" + ); + + // With reorder OFF (transport-stream behaviour) the sparse-PTS frames + // collapse onto the anchor's timestamp and nothing is buffered, so flush + // is empty. This is the discriminator that proves `with_ps_reorder(true)` + // really changed the parser rather than the two paths being identical. + let (raw_during, raw_tail) = feed(false); + assert!( + raw_tail.is_empty(), + "no reorderer installed → nothing buffered at EOF" + ); + assert_eq!(raw_during.len(), 10); + let collisions = raw_during.iter().filter(|f| f.pts_ns == 0).count(); + assert!( + collisions >= 4, + "without reorder the sparse-PTS frames collide on 0 (got {collisions})" + ); + } } diff --git a/src/mux/codec/mpegaudio.rs b/src/mux/codec/mpegaudio.rs index 4b32475..db48e45 100644 --- a/src/mux/codec/mpegaudio.rs +++ b/src/mux/codec/mpegaudio.rs @@ -287,4 +287,36 @@ mod tests { "next frame keeps its own PTS" ); } + + /// This parser is self-framing at PES granularity: `parse` emits (or drops) + /// every packet immediately and buffers nothing, so end-of-stream has + /// nothing left to hand over. A `flush` that manufactured a frame would + /// append a zero-length block at PTS 0 AFTER a track that has already run to + /// its real end — a Matroska Block whose timestamp jumps backwards past every + /// cluster before it (RFC 9559 §5.1.3.2 Blocks are relative to their + /// cluster's timestamp; a phantom 0 lands in the wrong cluster entirely) and + /// an empty audio frame no decoder can consume. + #[test] + 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)))); + // 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"); + assert_eq!(p.dropped_frames(), 1); + + let tail = p.flush(); + assert!( + tail.is_empty(), + "nothing is buffered past the last packet; flush produced {:?}", + tail.iter() + .map(|f| (f.pts_ns, f.data.len())) + .collect::>() + ); + // Total frame count over the whole stream equals the valid input count — + // a manufactured tail frame would break this even if it were non-empty. + assert_eq!(emitted.len() + tail.len(), 2); + } } diff --git a/src/mux/demux_sink.rs b/src/mux/demux_sink.rs index 8719444..b99be43 100644 --- a/src/mux/demux_sink.rs +++ b/src/mux/demux_sink.rs @@ -1628,4 +1628,165 @@ mod tests { std::fs::create_dir_all(&p).unwrap(); p } + + /// `demux://` exports the title's chapters as side files at `finish()`. + /// A `write_chapters` that returned `Ok(())` without writing produces a + /// demux run that reports complete success while the chapter files simply + /// do not exist — the caller has no way to tell an intentionally + /// chapterless title from a lost export. + /// + /// Both formats are requested at once and BOTH files are checked, with + /// distinct chapter names and a non-zero timestamp, so a writer that emitted + /// one format, an empty file, or the wrong chapter list cannot pass. + #[test] + fn finish_exports_both_chapter_formats_with_real_content() { + let dir = tempdir(); + let mut title = title_with(vec![video_stream(Codec::Mpeg2)], vec![None]); + title.chapters = vec![ + crate::disc::Chapter { + time_secs: 0.0, + name: "Opening".into(), + }, + crate::disc::Chapter { + time_secs: 62.5, + name: "Second".into(), + }, + ]; + let opts = DemuxOptions { + base: "ChapTitle".into(), + export_chapters: true, + chapters_fmt: ChaptersFmt::Both, + ..Default::default() + }; + let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); + sink.finish().unwrap(); + + let xml = std::fs::read_to_string(dir.join("ChapTitle chapters.xml")) + .expect("chapters.xml must exist after finish"); + let ogm = std::fs::read_to_string(dir.join("ChapTitle chapters.txt")) + .expect("chapters.txt must exist after finish"); + + // Content, not merely existence: both chapters, both names, and the + // 62.5 s timestamp formatted per its format. + assert!( + xml.contains("Opening") && xml.contains("Second"), + "xml: {xml}" + ); + assert!( + xml.contains("00:01:02.500"), + "xml must carry the real chapter time: {xml}" + ); + assert!( + ogm.contains("CHAPTER01=") && ogm.contains("CHAPTER02="), + "ogm: {ogm}" + ); + assert!( + ogm.contains("Opening") && ogm.contains("Second"), + "ogm names: {ogm}" + ); + assert!( + ogm.contains("00:01:02.500"), + "ogm must carry the real chapter time: {ogm}" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// The other side of the gate: with the export switched off, no chapter + /// file is written at all. Without this the test above would also pass for + /// a `write_chapters` that ignored `opts.export_chapters`. + #[test] + fn chapters_are_not_exported_when_the_option_is_off() { + let dir = tempdir(); + let mut title = title_with(vec![video_stream(Codec::Mpeg2)], vec![None]); + title.chapters = vec![crate::disc::Chapter { + time_secs: 0.0, + name: "Opening".into(), + }]; + let opts = DemuxOptions { + base: "ChapTitle".into(), + export_chapters: false, + chapters_fmt: ChaptersFmt::Both, + ..Default::default() + }; + let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); + sink.finish().unwrap(); + assert!(!dir.join("ChapTitle chapters.xml").exists()); + assert!(!dir.join("ChapTitle chapters.txt").exists()); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Every filename component `demux://` writes is built from DISC-CONTROLLED + /// text — the volume label / playlist name (`opts.base`) and the track label. + /// `sanitize` is the only thing standing between that text and the + /// filesystem: a surviving `/` (or `\` on Windows) makes the sink write + /// OUTSIDE the output directory the caller chose, and `:` / `?` / `*` / `"` / + /// `<` / `>` / `|` make the create fail outright on Windows and on SMB/exFAT + /// shares, which are the normal targets for a rip. + #[test] + fn sanitize_neutralises_every_path_hostile_character() { + for c in ['/', '\\', ':', '*', '?', '"', '<', '>', '|'] { + let got = sanitize(&format!("a{c}b")); + assert_eq!(got, "a_b", "{c:?} must be replaced, got {got:?}"); + } + // A traversal attempt in a disc label cannot escape the output directory: + // no separator survives, so the whole thing stays ONE component. + let escaped = sanitize("../../etc/passwd"); + assert_eq!(escaped, ".._.._etc_passwd"); + assert!( + !std::path::Path::new(&escaped) + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)), + "the sanitized name must not decompose into a parent-directory hop" + ); + // Ordinary characters — including spaces, dots, unicode and other + // punctuation — are preserved, so the replacement is targeted, not a + // blanket scrub that would mangle real titles. + assert_eq!( + sanitize("Amélie (2001) - Chapter 1.5 [Director's Cut]"), + "Amélie (2001) - Chapter 1.5 [Director's Cut]" + ); + } + + /// End-to-end witness that `sanitize` is actually applied on the write path: + /// a disc label containing a separator must produce ONE file inside the + /// chosen directory, never a write into a sibling/parent path. + #[test] + fn a_disc_label_with_a_separator_cannot_write_outside_the_output_directory() { + let dir = tempdir(); + let title = title_with(vec![video_stream(Codec::Mpeg2)], vec![None]); + let opts = DemuxOptions { + base: "../evil/Title".into(), + export_chapters: false, + ..Default::default() + }; + let mut sink = DemuxSink::create(&dir, &title, &opts).unwrap(); + sink.write(&PesFrame { + coding: None, + source: None, + track: 0, + pts: 0, + keyframe: true, + data: vec![0x00, 0x00, 0x01, 0xB3, 0xAA], + duration_ns: None, + }) + .unwrap(); + sink.finish().unwrap(); + + let names: Vec = std::fs::read_dir(&dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + assert_eq!(names.len(), 1, "exactly one output file; got {names:?}"); + assert!( + names[0].starts_with(".._evil_Title"), + "the separators must be neutralised in the real filename; got {names:?}" + ); + assert!( + !dir.parent().unwrap().join("evil").exists(), + "nothing may be created outside the output directory" + ); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/src/mux/disc.rs b/src/mux/disc.rs index e49c131..9060d35 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -2056,4 +2056,449 @@ mod tests { "raw single-pass must construct without cracking, even on scrambled-uncrackable input" ); } + + mod stream_surface_tests { + //! The `impl crate::pes::Stream for DiscStream` surface plus the adaptive + //! batch sizer. These are the accessors the CLI's abort gate and the + //! header-wait loop read; a constant in any of them hides read loss or + //! stalls the mux, so each is constrained against a value that is neither + //! the mutation constant nor the initial state. + use super::*; + use crate::pes::Stream as PesStream; + + // ── errors() / lost_bytes(): honest loss reporting ───────────────── + + /// `Stream::errors()` and `Stream::lost_bytes()` are the ONLY channel by + /// which a caller learns that bytes went missing (the abort gate and the + /// "N sectors skipped" report both read them). A constant there reports a + /// lossy rip as clean. + /// + /// Two short-read fills are driven so both counters land on values that are + /// neither `0` nor `1` and are distinct from each other — a single fill + /// would leave `errors == 1`, indistinguishable from a stuck constant, and + /// equal counters would not prove the two accessors read different fields. + #[test] + fn errors_and_lost_bytes_report_real_short_read_loss_through_the_trait() { + let mut s = short_read_stream(true); + + // Before any read the stream is clean — establishes the accessors are + // not simply echoing a preloaded value. + assert_eq!(PesStream::errors(&s), 0); + assert_eq!(PesStream::lost_bytes(&s), 0); + + for _ in 0..2 { + assert!( + s.fill_extents() + .expect("skip_errors absorbs the short read") + ); + } + + // Two 8-sector (16384 B) requests, 2048 B delivered each: two skip + // events, 2 * (16384 - 2048) = 28672 bytes lost. + assert_eq!( + PesStream::errors(&s), + 2, + "errors() must report BOTH short reads, not a constant" + ); + assert_eq!( + PesStream::lost_bytes(&s), + 28_672, + "lost_bytes() must report the byte total, not an event count or a constant" + ); + assert_ne!( + PesStream::errors(&s), + PesStream::lost_bytes(&s), + "the two accessors must read different fields" + ); + } + + // ── write(): DiscStream is read-only ────────────────────────────── + + /// `DiscStream` is the tree's only read-only `Stream`. `write()` returning + /// `Ok(())` would make a caller that muxed INTO a disc stream believe every + /// frame landed, producing a silent no-op rip. It must refuse with the + /// numeric code `E_STREAM_READ_ONLY`. + #[test] + fn write_refuses_with_the_read_only_code() { + let mut s = short_read_stream(false); + let frame = crate::pes::PesFrame { + coding: None, + source: None, + track: 0, + pts: 0, + keyframe: true, + data: vec![0u8; 4], + duration_ns: None, + }; + let err = PesStream::write(&mut s, &frame) + .expect_err("a read-only stream must never accept a frame"); + assert_eq!(err.kind(), std::io::ErrorKind::Unsupported); + let code = format!("E{}", crate::error::Error::StreamReadOnly.code()); + assert!( + err.to_string().contains(&code), + "expected the read-only code {code}, got {err}" + ); + } + + // ── codec_private() / headers_ready() ───────────────────────────── + + /// MPEG-2 sequence header (ISO/IEC 13818-2 §6.2.2.1) — start code `0x000001B3` + /// followed by 12-bit horizontal_size, 12-bit vertical_size, then + /// aspect_ratio_information / frame_rate_code and the bit-rate/VBV tail. + fn seq_header(width: u16, height: u16) -> Vec { + let mut h = vec![0x00, 0x00, 0x01, 0xB3]; + h.push((width >> 4) as u8); + h.push((((width & 0x0F) as u8) << 4) | ((height >> 8) & 0x0F) as u8); + h.push((height & 0xFF) as u8); + h.push((3 << 4) | 4); // aspect_ratio_information=3, frame_rate_code=4 + h.extend_from_slice(&[0xFF, 0xFF, 0xFF, 0x00]); + h + } + + /// MPEG-2 picture header (ISO/IEC 13818-2 §6.2.3), coding type in bits 3..5 + /// of the sixth byte. Type 1 = I-picture. + fn picture_header(coding_type: u8) -> Vec { + vec![ + 0x00, + 0x00, + 0x01, + 0x00, + 0x00, + (coding_type & 0x07) << 3, + 0, + 0, + ] + } + + /// One video (MPEG-2, track 1) behind one audio (AC-3, track 0). The video + /// is deliberately NOT track 0 so a `codec_private` that ignored its `track` + /// argument, or read the pid map in the wrong direction, would answer with + /// the audio parser (which has no codec private) and fail. + fn audio_then_video_title() -> DiscTitle { + use crate::disc::{ + AudioChannels, AudioStream, Codec, ColorSpace, FrameRate, HdrFormat, LabelPurpose, + Resolution, SampleRate, VideoStream, + }; + let mut t = DiscTitle { + extents: vec![crate::disc::Extent { + start_lba: 0, + sector_count: 8, + }], + ..DiscTitle::empty() + }; + t.content_format = ContentFormat::MpegPs; + t.streams = vec![ + crate::disc::Stream::Audio(AudioStream { + pid: 0x00BD, + codec: Codec::Ac3, + channels: AudioChannels::Stereo, + language: "eng".to_string(), + sample_rate: SampleRate::S48, + secondary: false, + purpose: LabelPurpose::Normal, + label: String::new(), + }), + crate::disc::Stream::Video(VideoStream { + pid: 0x00E0, + codec: Codec::Mpeg2, + resolution: Resolution::R480i, + frame_rate: FrameRate::F29_97, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Smpte170m, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + }), + ]; + t + } + + fn mixed_stream() -> DiscStream { + DiscStream::new( + Box::new(ZeroReader { capacity: 8 }), + audio_then_video_title(), + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::MpegPs, + false, + None, + ) + .unwrap() + } + + /// `headers_ready()` gates the CLI's "wait for codec private" loop: a + /// constant `true` starts the mux before the video's extradata exists + /// (an MKV with an empty CodecPrivate — unplayable video, RFC 9559 §5.1.4.1.5 + /// requires it for V_MPEG2), and a constant `false` hangs forever. + /// + /// Both directions are pinned in one case: not ready before the video + /// parser has seen a sequence header, ready after. + #[test] + fn headers_ready_follows_the_video_codec_private_and_flips_both_ways() { + let mut s = mixed_stream(); + + assert!( + PesStream::codec_private(&s, 1).is_none(), + "no sequence header parsed yet" + ); + assert!( + !PesStream::headers_ready(&s), + "a non-secondary video track without codec private must NOT be reported ready" + ); + + // Feed the video parser a complete access unit: sequence header + + // I-picture, closed by a following picture so the AU boundary is hit. + let vpid = 0x00E0u16; + let (_, parser) = s + .parsers + .iter_mut() + .find(|(p, _)| *p == vpid) + .expect("video parser present"); + let mut au = seq_header(720, 480); + au.extend_from_slice(&picture_header(1)); + au.extend_from_slice(&[0xAA; 16]); + let pes = |data: Vec, pts: Option| crate::mux::ts::PesPacket { + source: None, + pid: vpid, + pts, + dts: None, + data, + discontinuity: false, + }; + let _ = parser.parse(&pes(au, Some(0))); + let mut next = picture_header(3); + next.extend_from_slice(&[0xBB; 16]); + let _ = parser.parse(&pes(next, None)); + + let cp = PesStream::codec_private(&s, 1) + .expect("codec_private must reach the VIDEO track's parser, not track 0's"); + assert_eq!( + &cp[..4], + &[0x00, 0x00, 0x01, 0xB3], + "codec private is the MPEG-2 sequence header" + ); + assert_eq!(&cp[4..7], &[0x2D, 0x01, 0xE0], "720x480 as authored"); + + assert!( + PesStream::headers_ready(&s), + "with the video's codec private present the mux may start" + ); + // The AC-3 track genuinely has none — so the Some() above is a real + // per-track lookup, not a fixed answer. + assert!(PesStream::codec_private(&s, 0).is_none()); + // And a track index past the end of the pid map has none either. + assert!(PesStream::codec_private(&s, 7).is_none()); + } + + // ── on_event() / emit() ─────────────────────────────────────────── + + /// `on_event()` installs the sink and `emit()` feeds it. If either is a + /// no-op the CLI's progress bar never moves and skipped sectors are never + /// reported — the rip looks clean and stalled at 0 %. + #[test] + fn installed_event_sink_receives_skip_and_progress_events() { + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::::new())); + let sink = log.clone(); + let mut s = short_read_stream(true); + s.on_event(move |e| { + let tag = match e.kind { + EventKind::SectorSkipped { sector } => format!("skip:{sector}"), + EventKind::BytesRead { bytes, total } => format!("bytes:{bytes}/{total}"), + other => format!("other:{other:?}"), + }; + sink.lock().unwrap().push(tag); + }); + + assert!(s.fill_extents().expect("short read absorbed")); + + let got = log.lock().unwrap().clone(); + assert!( + got.contains(&"skip:0".to_string()), + "the skipped unit at LBA 0 must reach the installed sink; got {got:?}" + ); + // 2048 B delivered out of a 64-sector (131072 B) title. + assert!( + got.contains(&"bytes:2048/131072".to_string()), + "progress must report DELIVERED bytes against the extent total; got {got:?}" + ); + } + + // ── set_raw() ───────────────────────────────────────────────────── + + /// `set_raw()` must flip BOTH key holders — the metadata mirror read by + /// `info()`-side callers and the wrapped reader that actually decrypts. A + /// no-op leaves an AACS stream decrypting when the caller asked for + /// ciphertext (the `--raw` forensic path), silently returning plaintext. + #[test] + fn set_raw_clears_both_the_mirror_and_the_readers_keys() { + let mut s = DiscStream::new( + Box::new(ZeroReader { capacity: 8 }), + synthetic_title(8), + crate::decrypt::DecryptKeys::Aacs { + unit_keys: vec![(0, [0x11u8; 16])], + read_data_key: None, + format: ContentFormat::BdTs, + }, + 3, + ContentFormat::BdTs, + false, + None, + ) + .unwrap(); + assert!( + s.decrypt_keys.is_encrypted(), + "fixture must start encrypted or the test proves nothing" + ); + + // Behavioural witness for the WRAPPED reader's keys: while AACS keys + // are installed the decorator refuses a read that does not begin on a + // 6144-byte aligned unit (it would mis-decrypt every following unit), + // so a mid-unit LBA hard-fails. + let mut buf = vec![0u8; 2048]; + let before = s.reader.read_sectors(1, 1, &mut buf, false); + assert!( + matches!(before, Err(crate::error::Error::DecryptFailed)), + "the encrypted fixture must reject a mid-unit read; got {before:?}" + ); + + s.set_raw(); + + assert!( + !s.decrypt_keys.is_encrypted(), + "the metadata mirror must report the stream as raw" + ); + let after = s.reader.read_sectors(1, 1, &mut buf, false); + assert_eq!( + after.expect("raw mode must pass ciphertext straight through"), + 2048, + "the wrapped reader must stop decrypting, not just the mirror" + ); + } + + // ── AdaptiveBatch ───────────────────────────────────────────────── + + /// AACS decrypts whole 3-sector (6144 B) units, so every batch size at or + /// above one doubled unit must stay a multiple of 3 — an unaligned size + /// makes the next read straddle a unit boundary and mis-decrypt the rest of + /// the title. Below 6 the ladder descends 3 → 1 with no unaligned rungs. + #[test] + fn halve_batch_size_keeps_unit_alignment_and_bottoms_out_at_one() { + assert_eq!( + halve_batch_size(64), + 30, + "32 rounded down to a unit multiple" + ); + assert_eq!(halve_batch_size(30), 15); + assert_eq!(halve_batch_size(12), 6, "6 is already unit-aligned"); + assert_eq!(halve_batch_size(11), 5, "below 6: no alignment rounding"); + assert_eq!(halve_batch_size(6), 3); + assert_eq!(halve_batch_size(3), 1); + assert_eq!(halve_batch_size(2), 1); + assert_eq!( + halve_batch_size(1), + 1, + "must never reach 0 — a 0-sector read" + ); + for size in 1u16..=4096 { + let h = halve_batch_size(size); + assert!(h >= 1, "halve({size}) must never be 0"); + assert!(h <= size, "halve({size}) = {h} must not grow"); + assert!(h < 6 || h % 3 == 0, "halve({size}) = {h} is unit-unaligned"); + } + } + + #[test] + fn double_batch_size_grows_toward_preferred_without_breaking_alignment() { + assert_eq!(double_batch_size(30, 64), 60); + assert_eq!( + double_batch_size(4, 64), + 6, + "8 rounded down to a unit multiple" + ); + assert_eq!( + double_batch_size(1, 64), + 2, + "below 6: no alignment rounding" + ); + assert_eq!( + double_batch_size(60, 64), + 63, + "clamped to preferred, then aligned" + ); + for size in 1u16..=2048 { + let d = double_batch_size(size, 4096); + assert!(d >= size, "double({size}) = {d} must not shrink"); + assert!( + d < 6 || d % 3 == 0, + "double({size}) = {d} is unit-unaligned" + ); + } + } + + /// The sizer must actually probe back up: after a failure drops the batch, + /// a sustained clean run has to return a `BatchSizeChanged{Probed}` event + /// AND raise `current`. Never probing locks a rip at the reduced size for + /// the rest of the disc (the whole point of the amortised descent). + #[test] + fn on_success_probes_up_after_a_sustained_clean_run_and_resets_the_streak() { + let mut b = AdaptiveBatch::new(64); + assert!( + matches!( + b.on_failure(), + Some(EventKind::BatchSizeChanged { + new_size: 30, + reason: BatchSizeReason::Shrunk + }) + ), + "a failure must shrink 64 -> 30" + ); + assert_eq!(b.current(), 30); + + // Just under the 51200-sector probe threshold: still silent. + let mut fed = 0u32; + while fed + 30 < PROBE_THRESHOLD_SECTORS { + assert!( + b.on_success(30).is_none(), + "no probe before {PROBE_THRESHOLD_SECTORS} clean sectors (at {fed})" + ); + fed += 30; + } + assert_eq!(b.current(), 30, "still at the reduced size"); + + // The read that crosses the threshold probes up. + let ev = b + .on_success(30) + .expect("a sustained clean run must probe the batch size back up"); + assert!( + matches!( + ev, + EventKind::BatchSizeChanged { + new_size: 60, + reason: BatchSizeReason::Probed + } + ), + "expected a Probed grow to 60, got {ev:?}" + ); + assert_eq!( + b.current(), + 60, + "the sizer must actually adopt the new size" + ); + assert_eq!( + b.streak_sectors, 0, + "the streak resets so the next probe needs a fresh clean run" + ); + + // At the preferred size a clean run must NOT keep firing events. + let mut b = AdaptiveBatch::new(64); + for _ in 0..(PROBE_THRESHOLD_SECTORS / 64 + 2) { + assert!( + b.on_success(64).is_none(), + "no probe is possible when already at the preferred size" + ); + } + assert_eq!(b.current(), 64); + } + } } diff --git a/src/mux/hevc/mod.rs b/src/mux/hevc/mod.rs index 44c9ea4..ff5f814 100644 --- a/src/mux/hevc/mod.rs +++ b/src/mux/hevc/mod.rs @@ -902,4 +902,58 @@ mod tests { assert_eq!(sink[11], 0xAA); assert_eq!(sink[17], 0xAA); } + + /// A sink that records only what actually reaches it, so "was flush called" + /// is MEASURED rather than assumed. Its own `flush` is a no-op — the whole + /// point is that the intermediate `BufWriter` must be told to hand its bytes + /// over. + #[derive(Clone, Default)] + struct SharedSink(std::sync::Arc>>); + + impl SharedSink { + fn bytes(&self) -> Vec { + self.0.lock().unwrap().clone() + } + } + + impl Write for SharedSink { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + /// `finish()` is the ONLY thing that pushes a buffered sink's tail to the + /// file. Real sinks (`LocalFileSink`, `SocketSink`) buffer, and this muxer + /// deliberately adds none of its own — so a `finish` that skipped the flush + /// truncates every `.hevc` output by up to a whole sink buffer of trailing + /// NAL units, producing a file whose last GOP simply is not there. + #[test] + fn finish_flushes_the_buffered_sink_or_the_stream_tail_is_lost() { + let sink = SharedSink::default(); + let mut mux = HevcMux::new(io::BufWriter::new(sink.clone())); + // One length-prefixed NAL: [len:u32-BE][NAL bytes] → Annex B. + let nal = [0x26u8, 0x01, 0xAA, 0xBB]; // (0x26 >> 1) = 19 = IDR_W_RADL + let mut frame = (nal.len() as u32).to_be_bytes().to_vec(); + frame.extend_from_slice(&nal); + mux.write_frame(0, &frame).unwrap(); + + assert!( + sink.bytes().is_empty(), + "the fixture must actually buffer, or this test proves nothing" + ); + + mux.finish().unwrap(); + + let out = sink.bytes(); + let mut expected = START_CODE.to_vec(); + expected.extend_from_slice(&nal); + assert_eq!( + out, expected, + "finish must deliver the whole Annex B stream to the sink" + ); + } } diff --git a/src/mux/m2ts_mux/packet.rs b/src/mux/m2ts_mux/packet.rs index b2dd812..1bff445 100644 --- a/src/mux/m2ts_mux/packet.rs +++ b/src/mux/m2ts_mux/packet.rs @@ -350,4 +350,56 @@ mod tests { let pid = u16::from_be_bytes([p.bytes()[1] & 0x1F, p.bytes()[2]]); assert_eq!(pid, 0xE100 & 0x1FFF, "PID masked to 13 bits"); } + + /// A sink that records only what actually reaches it, so "was flush called" + /// is MEASURED rather than assumed. Its own `flush` is a no-op — the whole + /// point is that the intermediate `BufWriter` must be told to hand its bytes + /// over. + #[derive(Clone, Default)] + struct SharedSink(std::sync::Arc>>); + + impl SharedSink { + fn bytes(&self) -> Vec { + self.0.lock().unwrap().clone() + } + } + + impl Write for SharedSink { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + /// `PacketWriter` adds no buffering of its own, so the sink's buffer is the + /// only one — and `flush()` is the only thing that empties it. Skipping it + /// truncates the transport stream mid-packet: the final 188-byte packets + /// never reach the file, so the last PES of the title is lost and the stream + /// ends on a partial packet (ISO/IEC 13818-1 §2.4.3.2 requires whole + /// 188-byte packets). + #[test] + fn flush_delivers_the_buffered_packets_to_the_sink() { + let sink = SharedSink::default(); + let mut w = PacketWriter::new(io::BufWriter::new(sink.clone())); + let mut p = Packet::new(); + p.set_header(0x1011, true, true, false, 3); + p.append_payload(&[0x11, 0x22, 0x33]).unwrap(); + p.pad_to_188(); + w.write_packet(&p).unwrap(); + + assert!( + sink.bytes().is_empty(), + "the fixture must actually buffer, or this test proves nothing" + ); + + w.flush().unwrap(); + + let out = sink.bytes(); + assert_eq!(out.len(), TS_PACKET_BYTES, "one whole 188-byte TS packet"); + assert_eq!(out[0], SYNC_BYTE, "ISO/IEC 13818-1 sync_byte 0x47"); + assert_eq!(out, p.bytes(), "the bytes written are the packet's own"); + } } diff --git a/src/mux/meta_sink.rs b/src/mux/meta_sink.rs index 16ee186..71f86b6 100644 --- a/src/mux/meta_sink.rs +++ b/src/mux/meta_sink.rs @@ -428,4 +428,53 @@ mod tests { assert_eq!(vid["hdr"], "hdr10"); assert_eq!(vid["color_space"], "bt2020"); } + + fn temp_path(name: &str) -> std::path::PathBuf { + use std::sync::atomic::{AtomicU64, Ordering}; + static N: AtomicU64 = AtomicU64::new(0); + let n = N.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!("fmkv_meta_sink_{}_{n}_{name}", std::process::id())) + } + + fn sink_title() -> crate::disc::DiscTitle { + let mut t = crate::disc::DiscTitle::empty(); + t.playlist = "MAIN".into(); + t.chapters = chaps(); + t + } + + /// `chapters://` and `json://` are WRITE-ONLY sinks: the whole file is + /// emitted at `create()` and there is nothing to demux back. `read()` + /// returning `Ok(None)` instead of the write-only error makes a caller that + /// pointed a mux INPUT at one of these URLs see a clean empty stream — the + /// exact shape of the shipped "empty title, exit code 0" defect. It must + /// refuse with the numeric code `E_STREAM_WRITE_ONLY`. + #[test] + fn metadata_sinks_refuse_to_be_read_from() { + let code = format!("E{}", crate::error::Error::StreamWriteOnly.code()); + + let cpath = temp_path("chapters.xml"); + let mut c = ChaptersSink::create(&cpath, &sink_title()).unwrap(); + let err = c + .read() + .expect_err("chapters:// is write-only; read must not report a clean EOF"); + assert_eq!(err.kind(), io::ErrorKind::Unsupported); + assert!( + err.to_string().contains(&code), + "expected {code}, got {err}" + ); + let _ = std::fs::remove_file(&cpath); + + let jpath = temp_path("meta.json"); + let mut j = JsonSink::create(&jpath, &sink_title()).unwrap(); + let err = j + .read() + .expect_err("json:// is write-only; read must not report a clean EOF"); + assert_eq!(err.kind(), io::ErrorKind::Unsupported); + assert!( + err.to_string().contains(&code), + "expected {code}, got {err}" + ); + let _ = std::fs::remove_file(&jpath); + } } diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 5819886..0687320 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -3465,4 +3465,204 @@ mod tests { // Truncated: a 2-octet marker with only one octet available. assert!(lace_vint(&[0x43]).is_none()); } + + // ── finish(): the only thing that produces a valid file ─────────────── + + /// A `Cursor>` the test still owns after `MkvStream` takes it, so the + /// bytes the writer actually produced can be inspected (and re-opened). + #[derive(Clone)] + struct SharedOut(std::sync::Arc>>>); + + impl SharedOut { + fn new() -> Self { + Self(std::sync::Arc::new(std::sync::Mutex::new(Cursor::new( + Vec::new(), + )))) + } + fn bytes(&self) -> Vec { + self.0.lock().unwrap().get_ref().clone() + } + } + + impl io::Write for SharedOut { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.lock().unwrap().write(buf) + } + fn flush(&mut self) -> io::Result<()> { + self.0.lock().unwrap().flush() + } + } + + impl io::Seek for SharedOut { + fn seek(&mut self, pos: io::SeekFrom) -> io::Result { + self.0.lock().unwrap().seek(pos) + } + } + + fn h264_title() -> crate::disc::DiscTitle { + use crate::disc::{ + Codec, ColorSpace, DiscTitle, FrameRate, HdrFormat, Resolution, Stream, VideoStream, + }; + let mut t = DiscTitle { + streams: vec![Stream::Video(VideoStream { + pid: 0x1011, + codec: Codec::H264, + resolution: Resolution::R1080p, + frame_rate: FrameRate::F24, + hdr: HdrFormat::Sdr, + color_space: ColorSpace::Bt709, + display_aspect: None, + secondary: false, + label: String::new(), + measured_cicp: None, + })], + ..DiscTitle::empty() + }; + t.playlist = "FinishTitle".into(); + // A minimal avcC so the written TrackEntry carries a CodecPrivate. + t.codec_privates = vec![Some(vec![0x01, 0x64, 0x00, 0x1F, 0xFF, 0xE1])]; + t + } + + /// `finish()` is what turns a stream of frames into a FILE. It activates a + /// still-pending muxer (writing EBML header, Segment, Info, Tracks), then + /// finalizes it (Cues, SeekHead, the backpatched Segment size). A `finish` + /// that returned `Ok(())` without doing any of that leaves the caller with a + /// zero-byte or truncated `.mkv` and an exit code of 0 — a rip that reports + /// success and produced nothing. + /// + /// Proven by reading the output back through this crate's own MKV reader: + /// the frames must come out in order, with their real payloads, timestamps + /// and keyframe flags. + #[test] + fn finish_produces_a_readable_mkv_with_every_written_frame() { + let out = SharedOut::new(); + let title = h264_title(); + let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); + + let frames = [ + (0i64, true, vec![0xA1u8; 48]), + (41_708_333i64, false, vec![0xB2u8; 24]), + (83_416_666i64, false, vec![0xC3u8; 96]), + ]; + for (pts, keyframe, data) in &frames { + s.write(&crate::pes::PesFrame { + coding: None, + source: None, + track: 0, + pts: *pts, + keyframe: *keyframe, + data: data.clone(), + duration_ns: None, + }) + .unwrap(); + } + s.finish().unwrap(); + + let bytes = out.bytes(); + assert!(!bytes.is_empty(), "finish must have produced a file"); + + let mut back = MkvStream::open(Cursor::new(bytes)).unwrap(); + let mut got = Vec::new(); + while let Some(f) = back.read().unwrap() { + got.push(f); + } + assert_eq!( + got.len(), + frames.len(), + "every frame survives the round trip" + ); + for (i, (pts, keyframe, data)) in frames.iter().enumerate() { + assert_eq!(&got[i].data, data, "frame {i} payload"); + assert_eq!(got[i].keyframe, *keyframe, "frame {i} keyframe flag"); + // Matroska block timestamps are milliseconds at the default + // TimestampScale (RFC 9559 §5.1.2.6), so the ns PTS round-trips to + // the nearest ms. + assert_eq!( + got[i].pts / 1_000_000, + pts / 1_000_000, + "frame {i} timestamp" + ); + } + assert_eq!( + back.info().playlist, + "FinishTitle", + "the Segment Title written at finish survives" + ); + assert_eq!( + back.codec_private(0).as_deref(), + Some(&[0x01u8, 0x64, 0x00, 0x1F, 0xFF, 0xE1][..]), + "the TrackEntry CodecPrivate written at finish survives" + ); + } + + /// A title that produced NO frames must NOT finish successfully. `finish()` + /// activates the still-pending muxer (so the header/Tracks are written) and + /// then hands off to `MkvMuxer::finish`, whose zero-frame guard raises + /// `Error::MkvInvalid` (E6008) rather than emitting a structurally valid but + /// clusterless MKV. + /// + /// A `finish` that returned `Ok(())` would report a completed rip for a + /// title that muxed nothing — precisely the "empty title, exit code 0" + /// outcome the guard exists to prevent — and `error::is_skippable_title_stub` + /// would never get the code it classifies on. + #[test] + fn finish_refuses_a_zero_frame_title_instead_of_reporting_success() { + let out = SharedOut::new(); + let title = h264_title(); + let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); + + let err = s + .finish() + .expect_err("a title that muxed no frames must not finish successfully"); + assert_eq!(err.kind(), io::ErrorKind::InvalidData); + let code = format!("E{}", crate::error::Error::MkvInvalid.code()); + assert!( + err.to_string().contains(&code), + "expected the empty-mux code {code}, got {err}" + ); + assert!( + crate::error::is_skippable_title_stub(&err), + "the code raised must be the one the title loop classifies as a stub" + ); + } + + /// `headers_ready()` gates the CLI's wait-for-codec-private loop. For + /// Matroska it is unconditionally true because RFC 9559 §5.1 places the + /// Tracks element (carrying every CodecPrivate) in the Segment header, + /// ahead of the first Cluster — `MkvStream::open` has therefore already + /// parsed them by the time it returns. Returning `false` would hang the + /// mux forever on a source whose headers are, by construction, present. + /// + /// Pinned as an implication rather than a bare constant: readiness is + /// asserted TOGETHER with the codec private actually being retrievable, on + /// a freshly opened stream that has read no frame yet. + #[test] + fn headers_are_ready_at_open_because_matroska_front_loads_them() { + let out = SharedOut::new(); + let title = h264_title(); + let mut s = MkvStream::create(Box::new(out.clone()), &title, None).unwrap(); + s.write(&crate::pes::PesFrame { + coding: None, + source: None, + track: 0, + pts: 0, + keyframe: true, + data: vec![0xA1; 48], + duration_ns: None, + }) + .unwrap(); + s.finish().unwrap(); + + let back = MkvStream::open(Cursor::new(out.bytes())).unwrap(); + assert!( + back.headers_ready(), + "Matroska carries Tracks before the first Cluster; open() already has them" + ); + assert!( + back.codec_private(0).is_some(), + "and the readiness claim is honest: the codec private IS available \ + before any frame has been read" + ); + } } diff --git a/src/mux/mp4/read.rs b/src/mux/mp4/read.rs index ae51a14..fec7d7e 100644 --- a/src/mux/mp4/read.rs +++ b/src/mux/mp4/read.rs @@ -1186,15 +1186,43 @@ mod tests { assert_eq!(auds[0].1, ac3.len()); } + /// `stts` run-length expansion — ISO/IEC 14496-12 §8.6.1.2. The box stores + /// `(sample_count, sample_delta)` runs; the reader must expand them back to + /// one delta PER SAMPLE, in order, or every sample after the first run lands + /// on the wrong decode time. + /// + /// (This test used to be called `stts_and_ctts_expand` while touching no + /// `ctts` box at all. The composition-offset half now lives in + /// `ctts_build_and_parse_are_exact_inverses_over_signed_offsets` and + /// `b_frame_presentation_order_survives_the_mp4_round_trip`.) #[test] - fn stts_and_ctts_expand() { - // stts: 3 samples × 1001 ticks. + fn stts_expands_runs_to_per_sample_deltas_in_order() { + // Three runs with DISTINCT deltas and distinct lengths, so a parser that + // dropped a run, reused the first delta, or emitted the runs in the + // wrong order cannot agree. A trailing 0-length run must contribute + // nothing (legal: §8.6.1.2 places no lower bound on sample_count). let mut stts = Vec::new(); - stts.extend_from_slice(&[0, 0, 0, 0]); // version+flags - stts.extend_from_slice(&1u32.to_be_bytes()); // entry_count - stts.extend_from_slice(&3u32.to_be_bytes()); - stts.extend_from_slice(&1001u32.to_be_bytes()); - assert_eq!(parse_stts(&stts, MAX_SAMPLE_COUNT), vec![1001, 1001, 1001]); + stts.extend_from_slice(&[0, 0, 0, 0]); // version + flags + stts.extend_from_slice(&4u32.to_be_bytes()); // entry_count + for (n, delta) in [(3u32, 1001u32), (1, 2002), (0, 7777), (2, 1002)] { + stts.extend_from_slice(&n.to_be_bytes()); + stts.extend_from_slice(&delta.to_be_bytes()); + } + assert_eq!( + parse_stts(&stts, MAX_SAMPLE_COUNT), + vec![1001, 1001, 1001, 2002, 1002, 1002], + ); + + // A truncated box (entry_count claims more runs than the bytes hold) + // must yield the runs actually present, never read past the end. + let truncated = &stts[..stts.len() - 6]; + assert_eq!( + parse_stts(truncated, MAX_SAMPLE_COUNT), + vec![1001, 1001, 1001, 2002], + ); + + // Too short to hold version/flags + entry_count → no samples. + assert!(parse_stts(&stts[..7], MAX_SAMPLE_COUNT).is_empty()); } // ── Composition offsets (`ctts`) — ISO/IEC 14496-12 §8.6.1.3.