From 4fcd28b487847c109ee00de09a5c4e92bd341024 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:53:19 -0700 Subject: [PATCH] Parse MKV lacing, route by real TrackNumber, honour NAL length size and edit lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four conformance defects in the read paths, two of them silent corruption. **Lacing was ignored entirely.** RFC 9559 §10.2 defines Xiph, EBML and fixed-size lacing, where one Block carries several frames; the reader took the Block payload verbatim, so a laced Block became a single "frame" consisting of a lacing header followed by concatenated frames — garbage to the codec parser, no error. Audio tracks from other muxers commonly use lacing, so an ordinary foreign MKV was silently mangled. All three modes are now parsed: Xiph 255-run sizes including the trailing-zero rule for exact multiples of 255, EBML unsigned first size plus SIGNED VINT deltas with the 2^((7*n)-1)-1 bias of §10.3.3, and fixed-size even division, with the last frame's size deduced from the remainder. Laced timestamps follow §10.3.5: the first frame takes the Block timestamp and the rest are spaced by the track's DefaultDuration, else BlockDuration/count, else shared with a warn. Parsing was chosen over refusing because refusal would leave freemkv unable to remux common foreign audio at all, and each mode is about fifteen lines. A malformed lacing header now raises a NEW code, E_MKV_LACING_INVALID = 9052, deliberately NOT MkvInvalid — because is_skippable_title_stub classifies MkvInvalid as a skippable nav stub, so reusing it would have recreated the exact conflation that is still open as a separate finding. A test asserts the new code is not skippable. **TrackNumber was assumed to be 1..N in TrackEntry order.** RFC 9559 §5.1.4.1.1 only requires it to be non-zero and unique, so sparse or unordered numbers are legal. Block routing and codec_private both computed track + 1. A real TrackNumber map is now built, recorded only for TrackEntries that yield a stream so dropped track types no longer shift the mapping. Verified red here independently, and the failure mode is worse than mis-routing: with track + 1 restored, a buttons track's payload was attributed to the AUDIO stream — wrong payload into the wrong codec parser. **The NAL length prefix was hardcoded to 4 bytes.** lengthSizeMinusOne lives in avcC byte 4 and hvcC byte 21 (ISO/IEC 14496-15 §5.3.3.1.2, §8.3.3.1.2) and was never read, so a source declaring 1- or 2-byte prefixes had its raw prefixed bytes emitted verbatim with no start codes. All four conversion sites now derive the width from the track's own configuration record. **Edit lists were ignored.** No edts/elst was parsed, so the presentation timeline an edit list defines (ISO/IEC 14496-12 §8.6.5/§8.6.6) was dropped — which is how encoder delay is normally expressed. Leading empty edits and the first media edit's media_time are now applied to both dts and pts, with the movie vs media timescale distinction respected. A list needing more than a constant shift applies the leading edit and warns rather than presenting the result as faithful. 17 tests. I reproduced the lacing mutant independently: returning the body whole kills five of them, including the exact-payload and malformed-header cases. Still open and deliberately untouched: the MkvInvalid / is_skippable_title_stub conflation across ~20 reader raise sites. It is a cross-cutting error.rs change and E_MKV_LACING_INVALID is the template for it. --- src/error.rs | 19 + src/mux/demux_sink.rs | 55 ++- src/mux/hevc/mod.rs | 181 ++++++++- src/mux/m2ts_mux/mod.rs | 9 +- src/mux/mkvstream.rs | 848 ++++++++++++++++++++++++++++++++++++---- src/mux/mp4/read.rs | 309 ++++++++++++++- src/mux/tsmux.rs | 14 +- 7 files changed, 1347 insertions(+), 88 deletions(-) diff --git a/src/error.rs b/src/error.rs index 0579fb1..c81e05f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -156,6 +156,15 @@ pub const E_MUX_EMPTY: u16 = 9023; /// [`is_skippable_title_stub`] treats as a skippable empty nav/menu stub — a /// cap-overflow is a real title and must never be silently skipped. pub const E_MUX_HEADER_BUFFER_EXCEEDED: u16 = 9051; +/// An `mkv://` SOURCE Block declared lacing (RFC 9559 §10.3) whose header does +/// not describe its own payload, so the frame boundaries inside the Block are +/// unknowable. Deliberately NOT [`E_MKV_INVALID`], which +/// [`is_skippable_title_stub`] treats as a skippable empty nav/menu stub: a +/// laced Block belongs to a track with real media in it, and mis-reporting the +/// rejection as a stub would drop that media from a run that then exits +/// successfully — the same conflation [`E_MUX_HEADER_BUFFER_EXCEEDED`] exists to +/// avoid. +pub const E_MKV_LACING_INVALID: u16 = 9052; pub const E_EXTENT_NOT_UNIT_ALIGNED: u16 = 9030; /// `mp4://` output but the title has no (primary) video track to carry. pub const E_MP4_NO_VIDEO_TRACK: u16 = 9048; @@ -297,6 +306,10 @@ pub enum Error { }, IfoParse, MkvInvalid, + /// An `mkv://` source Block's lacing header does not describe its payload — + /// the frames packed into that Block cannot be separated. NOT + /// [`Error::MkvInvalid`]: see [`E_MKV_LACING_INVALID`]. + MkvLacingInvalid, NoStreams, /// A [`crate::StreamSelection`] listed a PID that does not exist in the /// title's declared streams — a caller bug (e.g. a stale scan), reported @@ -604,6 +617,7 @@ impl Error { Error::DiscTitleRange { .. } => E_DISC_TITLE_RANGE, Error::IfoParse => E_IFO_PARSE, Error::MkvInvalid => E_MKV_INVALID, + Error::MkvLacingInvalid => E_MKV_LACING_INVALID, Error::NoStreams => E_NO_STREAMS, Error::SelectionPidUnknown { .. } => E_SELECTION_PID_UNKNOWN, Error::MapfileInvalid { .. } => E_MAPFILE_INVALID, @@ -895,6 +909,9 @@ impl From for std::io::Error { // 9051 MuxHeaderBufferExceeded: the source kept yielding frames but // never its codec init data — the input is unusable as declared. E_MUX_HEADER_BUFFER_EXCEEDED => std::io::ErrorKind::InvalidData, + // 9052 MkvLacingInvalid: a source Block's lacing header does not + // describe its own payload — malformed input data. + E_MKV_LACING_INVALID => std::io::ErrorKind::InvalidData, // mp4:// demux errors: a malformed/truncated source file // (E_MP4_INVALID), or a source whose tracks the mux can't use — no // video track / missing codec-private config. All are invalid data. @@ -1391,6 +1408,7 @@ mod tests { E_NETWORK_ADDR_BLOCKED, E_MUX_EMPTY, E_MUX_HEADER_BUFFER_EXCEEDED, + E_MKV_LACING_INVALID, E_MP4_NO_VIDEO_TRACK, E_MP4_INVALID, E_MP4_MISSING_CODEC_PRIVATE, @@ -1485,6 +1503,7 @@ mod tests { Error::MuxHeaderBufferExceeded { bytes: 0 }, E_MUX_HEADER_BUFFER_EXCEEDED, ), + (Error::MkvLacingInvalid, E_MKV_LACING_INVALID), (Error::Mp4NoVideoTrack, E_MP4_NO_VIDEO_TRACK), (Error::Mp4Invalid, E_MP4_INVALID), (Error::Mp4MissingCodecPrivate, E_MP4_MISSING_CODEC_PRIVATE), diff --git a/src/mux/demux_sink.rs b/src/mux/demux_sink.rs index 223e9f8..a68de31 100644 --- a/src/mux/demux_sink.rs +++ b/src/mux/demux_sink.rs @@ -23,7 +23,9 @@ //! The sink does NOT touch the MKV mux path; it is purely additive. use crate::disc::{Chapter, Codec, DiscTitle, Stream as DiscStream}; -use crate::mux::hevc::{append_length_prefixed_as_annex_b, avcc_to_annex_b, hvcc_to_annex_b}; +use crate::mux::hevc::{ + append_length_prefixed_as_annex_b_sized, avcc_to_annex_b, hvcc_to_annex_b, nal_length_size, +}; use crate::mux::timeline::TimelineContinuity; use crate::pes::{PesFrame, Stream}; use std::fs::File; @@ -207,12 +209,17 @@ impl EsWriter for PassthroughWriter { } } -/// HEVC/H.264 writer: reframes 4-byte-length-prefixed NALs (the hvcC/avcC form -/// the parsers emit) into Annex-B, prepending the parameter sets once. +/// HEVC/H.264 writer: reframes length-prefixed NALs (the hvcC/avcC form the +/// parsers emit) into Annex-B, prepending the parameter sets once. struct AnnexBWriter { /// Annex-B-framed VPS/SPS/PPS (or SPS/PPS), parsed from the hvcC/avcC. params: Vec, wrote_params: bool, + /// Octets per NAL length prefix, from the configuration record's + /// `lengthSizeMinusOne` (ISO/IEC 14496-15). NOT assumed to be 4: a legal + /// avcC/hvcC may declare 1 or 2, and reading those as u32-BE parses no NALs + /// at all, so the raw prefixed bytes would be emitted as if already Annex B. + length_size: usize, } impl AnnexBWriter { @@ -223,6 +230,7 @@ impl AnnexBWriter { Self { params, wrote_params: false, + length_size: nal_length_size(codec, codec_private), } } } @@ -244,7 +252,7 @@ impl EsWriter for AnnexBWriter { // zero-length NALs and drops a truncated trailing NAL without panicking, // rather than `break`ing on the first zero-length NAL. let mut scratch = Vec::with_capacity(f.data.len() + (f.data.len() / 32) + 4); - append_length_prefixed_as_annex_b(&mut scratch, &f.data); + append_length_prefixed_as_annex_b_sized(&mut scratch, &f.data, self.length_size); w.write_all(&scratch)?; n += scratch.len(); Ok(n) @@ -982,6 +990,45 @@ mod tests { ); } + /// Regression (silent corruption): the sink reframed every frame as if the + /// NAL length prefixes were 4 octets wide. ISO/IEC 14496-15 §5.3.3.1.2 lets + /// an avcC declare `lengthSizeMinusOne = 1` (2-octet prefixes); reading those + /// as one u32-BE gives an absurd length, so nothing parsed and the raw + /// prefixed bytes were written to the `demux://` ES verbatim — no start + /// codes, undecodable video, and no error anywhere. + #[test] + fn annexb_writer_honours_the_records_declared_nal_length_size() { + // avcC with byte 4 = 0xFD → lengthSizeMinusOne 1 → 2-octet prefixes. + // numSPS = 1 (0xE1), SPS len 2 = [0x67 0x42], numPPS = 1, PPS len 1. + let rec = [ + 1, 0x42, 0x00, 0x1F, 0xFD, 0xE1, 0, 2, 0x67, 0x42, 1, 0, 1, 0x68, + ]; + assert_eq!(nal_length_size(Codec::H264, Some(&rec)), 2); + let mut w = AnnexBWriter::new(Codec::H264, Some(&rec)); + let mut out = Vec::new(); + let f = PesFrame { + coding: None, + source: None, + track: 0, + pts: 0, + keyframe: true, + // Two NALs with 2-octet length prefixes. + data: vec![0, 2, 0xAA, 0xBB, 0, 3, 0x01, 0x02, 0x03], + duration_ns: None, + }; + w.write_frame(&mut out, &f, 0).unwrap(); + assert_eq!( + out, + vec![ + 0, 0, 0, 1, 0x67, 0x42, // SPS + 0, 0, 0, 1, 0x68, // PPS + 0, 0, 0, 1, 0xAA, 0xBB, // frame NAL #1 + 0, 0, 0, 1, 0x01, 0x02, 0x03, // frame NAL #2 + ], + "2-octet-prefixed NALs must reach the ES as Annex B" + ); + } + #[test] fn annexb_writer_prepends_params_once() { let rec = [ diff --git a/src/mux/hevc/mod.rs b/src/mux/hevc/mod.rs index ef0c760..44c9ea4 100644 --- a/src/mux/hevc/mod.rs +++ b/src/mux/hevc/mod.rs @@ -96,7 +96,17 @@ impl HevcMux { } } } - let annex_b = length_prefixed_to_annex_b(data); + // The hvcC declares the NAL length-prefix width (ISO/IEC 14496-15 + // §8.3.3.1.2 `lengthSizeMinusOne + 1`). Assuming 4 for a source that + // declares 1 or 2 emits the raw prefixed bytes with no start codes. + let length_size = nal_length_size(crate::disc::Codec::Hevc, self.codec_private.as_deref()); + let annex_b = if starts_with_start_code(data) { + data.to_vec() + } else { + let mut out = Vec::with_capacity(data.len() + (data.len() / 32)); + append_length_prefixed_as_annex_b_sized(&mut out, data, length_size); + out + }; self.writer.write_all(&annex_b) } @@ -190,6 +200,44 @@ pub(crate) fn length_prefixed_to_annex_b(data: &[u8]) -> Vec { out } +/// The NAL length-prefix width this crate's own parsers emit, and the width +/// ISO/IEC 14496-15 records declare as `lengthSizeMinusOne = 3`. +pub(crate) const DEFAULT_NAL_LENGTH_SIZE: usize = 4; + +/// Number of octets each NAL length prefix occupies in the elementary data of a +/// track described by `record` — the `lengthSizeMinusOne + 1` field of the +/// decoder configuration record (ISO/IEC 14496-15). +/// +/// * avcC (`AVCDecoderConfigurationRecord`, §5.3.3.1.2): byte 4 is +/// `bit(6) reserved | unsigned int(2) lengthSizeMinusOne`. +/// * hvcC (`HEVCDecoderConfigurationRecord`, §8.3.3.1.2): byte 21 is +/// `constantFrameRate(2) | numTemporalLayers(3) | temporalIdNested(1) | +/// lengthSizeMinusOne(2)`. +/// +/// The spec permits only 1, 2 or 4 octets (`lengthSizeMinusOne` of 0, 1 or 3); +/// a declared 3 is non-conformant but is decoded rather than rejected, since +/// reading N octets is the same operation for every N. A record too short to +/// carry the field, or a codec with no such record, falls back to +/// [`DEFAULT_NAL_LENGTH_SIZE`] — the width every freemkv parser emits. +/// +/// This exists because assuming 4 is silent corruption for a legal source: +/// reading a 2-octet-prefixed frame as u32-BE yields an absurd first length, the +/// conversion loop bails with nothing parsed, and the raw length-prefixed bytes +/// are passed through as though they were already Annex B — a stream with no +/// start codes at all, and no error anywhere. +pub(crate) fn nal_length_size(codec: crate::disc::Codec, record: Option<&[u8]>) -> usize { + use crate::disc::Codec; + let field_offset = match codec { + Codec::H264 => 4, + Codec::Hevc => 21, + _ => return DEFAULT_NAL_LENGTH_SIZE, + }; + match record.and_then(|r| r.get(field_offset)) { + Some(&b) => (b & 0x03) as usize + 1, + None => DEFAULT_NAL_LENGTH_SIZE, + } +} + /// Append the Annex B form of `data` (length-prefixed NALs) into `out`. /// /// Same conversion as [`length_prefixed_to_annex_b`] but writes directly @@ -198,20 +246,40 @@ pub(crate) fn length_prefixed_to_annex_b(data: &[u8]) -> Vec { /// length-prefixed (no NALs extracted), it's appended unchanged on the /// assumption it's already Annex B. pub(crate) fn append_length_prefixed_as_annex_b(out: &mut Vec, data: &[u8]) { + append_length_prefixed_as_annex_b_sized(out, data, DEFAULT_NAL_LENGTH_SIZE); +} + +/// [`append_length_prefixed_as_annex_b`] for a source whose NAL length prefixes +/// are `length_size` octets wide rather than the 4 this crate's own parsers +/// emit. Derive `length_size` from the track's configuration record with +/// [`nal_length_size`] — ISO/IEC 14496-15 lets a legal avcC/hvcC declare 1 or 2 +/// octet prefixes, and reading those as u32-BE mangles the frame. +/// +/// `length_size` outside `1..=4` is clamped to [`DEFAULT_NAL_LENGTH_SIZE`]; the +/// field it comes from is 2 bits wide, so that is unreachable from real input. +pub(crate) fn append_length_prefixed_as_annex_b_sized( + out: &mut Vec, + data: &[u8], + length_size: usize, +) { + let length_size = if (1..=4).contains(&length_size) { + length_size + } else { + DEFAULT_NAL_LENGTH_SIZE + }; let mut offset = 0; // True once we've consumed at least one well-formed length prefix // (even a zero-length one). Distinguishes "parsed as length-prefixed, // all NALs empty" (emit nothing) from "not length-prefixed at all" // (pass through as already-Annex B). let mut parsed_any = false; - while offset + 4 <= data.len() { - let len = u32::from_be_bytes([ - data[offset], - data[offset + 1], - data[offset + 2], - data[offset + 3], - ]) as usize; - offset += 4; + while offset + length_size <= data.len() { + // Big-endian over exactly `length_size` octets (ISO/IEC 14496-15: the + // prefix is an unsigned integer of `lengthSizeMinusOne + 1` bytes). + let len = data[offset..offset + length_size] + .iter() + .fold(0usize, |acc, &b| (acc << 8) | b as usize); + offset += length_size; if offset + len > data.len() { // Mid-NAL truncation (e.g. a NAL cut by a bad disc sector) — // drop the truncated trailing NAL and emit only the valid @@ -637,6 +705,101 @@ mod tests { assert_eq!(out, want); } + /// ISO/IEC 14496-15 §5.3.3.1.2 (avcC byte 4) and §8.3.3.1.2 (hvcC byte 21) + /// each carry `lengthSizeMinusOne` in the low 2 bits. Nothing in the crate + /// read it, so every conversion assumed a 4-octet prefix. + #[test] + fn nal_length_size_is_read_from_the_configuration_record() { + use crate::disc::Codec; + // avcC: byte 4 = 0xFF → lengthSizeMinusOne 3 → 4-octet prefixes. + let mut avcc = vec![0x01, 0x64, 0x00, 0x28, 0xFF, 0xE1]; + assert_eq!(nal_length_size(Codec::H264, Some(&avcc)), 4); + // 0xFD → lengthSizeMinusOne 1 → 2-octet prefixes (legal per §5.3.3.1.2). + avcc[4] = 0xFD; + assert_eq!(nal_length_size(Codec::H264, Some(&avcc)), 2); + // 0xFC → lengthSizeMinusOne 0 → 1-octet prefixes. + avcc[4] = 0xFC; + assert_eq!(nal_length_size(Codec::H264, Some(&avcc)), 1); + + // hvcC: the field is byte 21, not byte 4. + let mut hvcc = vec![0u8; 23]; + hvcc[21] = 0xFF; + assert_eq!(nal_length_size(Codec::Hevc, Some(&hvcc)), 4); + hvcc[21] = 0xFD; + assert_eq!(nal_length_size(Codec::Hevc, Some(&hvcc)), 2); + + // Absent / too-short record, or a non-NAL codec → the crate's own width. + assert_eq!(nal_length_size(Codec::Hevc, None), DEFAULT_NAL_LENGTH_SIZE); + assert_eq!( + nal_length_size(Codec::Hevc, Some(&hvcc[..8])), + DEFAULT_NAL_LENGTH_SIZE + ); + assert_eq!( + nal_length_size(Codec::Mpeg2, Some(&avcc)), + DEFAULT_NAL_LENGTH_SIZE + ); + } + + /// Regression (silent corruption): a source whose avcC declares 2-octet NAL + /// lengths was reframed by reading the first FOUR octets as one u32-BE + /// length. That value is absurd, the loop breaks with `parsed_any == false`, + /// and the whole frame is passed through verbatim — raw length-prefixed + /// bytes in a stream that is supposed to be Annex B, with no start codes, + /// no NALs and no error. + #[test] + fn two_octet_length_prefixes_convert_instead_of_leaking_raw_bytes() { + // Two NALs with 2-octet prefixes: [0x00 0x03][3 bytes][0x00 0x02][2 bytes] + let data = [ + 0x00, 0x03, 0x67, 0x42, 0x1E, // NAL 1 + 0x00, 0x02, 0x68, 0xCE, // NAL 2 + ]; + let mut want = START_CODE.to_vec(); + want.extend_from_slice(&[0x67, 0x42, 0x1E]); + want.extend_from_slice(&START_CODE); + want.extend_from_slice(&[0x68, 0xCE]); + + let mut got = Vec::new(); + append_length_prefixed_as_annex_b_sized(&mut got, &data, 2); + assert_eq!(got, want, "2-octet prefixes must be reframed to Annex B"); + + // What the 4-octet assumption produced: the raw bytes, verbatim, with no + // start code anywhere. + let mut assumed_four = Vec::new(); + append_length_prefixed_as_annex_b(&mut assumed_four, &data); + assert_eq!( + assumed_four, + data.to_vec(), + "the 4-octet assumption leaks the source bytes unconverted" + ); + assert!( + !assumed_four.starts_with(&START_CODE), + "no start code at all — the video cannot decode" + ); + } + + /// A 1-octet prefix width works the same way, and an out-of-range width + /// falls back to the crate's own 4 rather than panicking or looping. + #[test] + fn one_octet_length_prefixes_and_out_of_range_width() { + let data = [0x02, 0x40, 0x01, 0x01, 0x09]; + let mut got = Vec::new(); + append_length_prefixed_as_annex_b_sized(&mut got, &data, 1); + let mut want = START_CODE.to_vec(); + want.extend_from_slice(&[0x40, 0x01]); + want.extend_from_slice(&START_CODE); + want.extend_from_slice(&[0x09]); + assert_eq!(got, want); + + // width 0 and width 9 both clamp to DEFAULT_NAL_LENGTH_SIZE. + let mut four = Vec::new(); + append_length_prefixed_as_annex_b(&mut four, &data); + for bad in [0usize, 9] { + let mut clamped = Vec::new(); + append_length_prefixed_as_annex_b_sized(&mut clamped, &data, bad); + assert_eq!(clamped, four, "an impossible width clamps to 4"); + } + } + #[test] fn starts_with_start_code_detects_both_forms() { assert!(starts_with_start_code(&[0x00, 0x00, 0x00, 0x01, 0x42])); diff --git a/src/mux/m2ts_mux/mod.rs b/src/mux/m2ts_mux/mod.rs index 34d5319..b4b9593 100644 --- a/src/mux/m2ts_mux/mod.rs +++ b/src/mux/m2ts_mux/mod.rs @@ -242,7 +242,14 @@ impl M2tsMux { } // Append the Annex-B form directly into the pre-sized `es` // buffer rather than materializing an intermediate Vec. - super::hevc::append_length_prefixed_as_annex_b(&mut es, data); + // The source's hvcC declares the NAL length-prefix width (ISO/IEC + // 14496-15 §8.3.3.1.2 `lengthSizeMinusOne + 1`); assuming 4 mangles a + // legal 1- or 2-octet-prefixed source into a start-code-free stream. + let length_size = super::hevc::nal_length_size( + crate::disc::Codec::Hevc, + self.video_codec_private.as_deref(), + ); + super::hevc::append_length_prefixed_as_annex_b_sized(&mut es, data, length_size); let pes = build_video_pes(pts_90k, &es); self.write_pes(PID_VIDEO, &pes, Some(pcr), keyframe) diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 330dcad..0a487c6 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -6,9 +6,10 @@ use super::mkv::{MkvMuxer, MkvTrack}; use super::{WriteSeek, ebml}; -/// (title, codec_privates, ts_scale_ns) — `ts_scale_ns` is the -/// TimestampScale in nanoseconds per tick, threaded into the frame read path. -type MkvHeaderResult = io::Result<(crate::disc::DiscTitle, Vec<(u16, Vec)>, i64)>; +/// (title, codec_privates, ts_scale_ns, track_table) — `ts_scale_ns` is the +/// TimestampScale in nanoseconds per tick, threaded into the frame read path; +/// `track_table` maps Matroska TrackNumbers onto `DiscTitle::streams` indices. +type MkvHeaderResult = io::Result<(crate::disc::DiscTitle, Vec<(u16, Vec)>, i64, TrackTable)>; /// Skip `n` bytes on a forward-only reader (no Seek required). fn skip_bytes(r: &mut impl Read, n: u64) -> io::Result<()> { @@ -74,6 +75,14 @@ struct ReadState { ts_scale_ns: i64, /// Codec private data per track (track_number, hvcC/avcC bytes). codec_privates: Vec<(u16, Vec)>, + /// TrackNumber → stream-index map (and per-track DefaultDuration). MKV + /// TrackNumbers are not required to be `1..=N` in TrackEntry order, so this + /// is the only permitted translation between the two spaces. + tracks: TrackTable, + /// Frames decoded from a LACED Block that have not been handed out yet. One + /// Block can carry many frames (RFC 9559 §10.3) while `Stream::read` yields + /// one at a time, so the surplus waits here. + pending: std::collections::VecDeque, /// Number of `BlockAdditions` subtrees skipped on read-back (see /// `MkvStream`'s `Stream::lost_bytes`). Each one is a per-frame side payload — for a /// Blu-ray 3D rip written by this crate, one MVC dependent-view (right-eye) @@ -550,7 +559,7 @@ impl MkvStream { /// Open an MKV file for reading → PES frames. pub fn open(mut reader: impl Read + Send + 'static) -> io::Result { - let (disc_title, codec_privates, ts_scale_ns) = parse_mkv_header(&mut reader)?; + let (disc_title, codec_privates, ts_scale_ns, tracks) = parse_mkv_header(&mut reader)?; Ok(Self { disc_title, mvc: None, @@ -559,6 +568,8 @@ impl MkvStream { cluster_ts_ticks: 0, ts_scale_ns, codec_privates, + tracks, + pending: std::collections::VecDeque::new(), additions_dropped: 0, additions_dropped_bytes: 0, }), @@ -628,12 +639,17 @@ fn apply_coding_to_track( impl crate::pes::Stream for MkvStream { fn read(&mut self) -> io::Result> { - let streams_len = self.disc_title.streams.len(); let rs = match self.mode { Mode::Read(ref mut rs) => rs, Mode::Write(_) => return Err(crate::error::Error::StreamWriteOnly.into()), }; + // Frames still owed from the last LACED Block come out before any new + // element is read, so a lace is never truncated by the next Block. + if let Some(frame) = rs.pending.pop_front() { + return Ok(Some(frame)); + } + loop { let (id, size, _) = match ebml::read_element_header(&mut rs.reader) { Ok(h) => h, @@ -662,13 +678,15 @@ impl crate::pes::Stream for MkvStream { ebml::SIMPLE_BLOCK => { let block = ebml::read_binary_val(&mut rs.reader, checked_size(size, MAX_BLOCK_SIZE)?)?; - if let Some(frame) = parse_block( + let frames = parse_block( &block, rs.cluster_ts_ticks, rs.ts_scale_ns, - streams_len, + &rs.tracks, None, - ) { + )?; + rs.pending.extend(frames); + if let Some(frame) = rs.pending.pop_front() { return Ok(Some(frame)); } continue; @@ -773,17 +791,21 @@ impl crate::pes::Stream for MkvStream { // in foreign MKVs) — same scaling PTS uses. let dur_ns = duration_ms.map(|ticks| ticks.saturating_mul(rs.ts_scale_ns as u64)); - if let Some(mut frame) = parse_block( + let frames = parse_block( &block, rs.cluster_ts_ticks, rs.ts_scale_ns, - streams_len, + &rs.tracks, dur_ns, - ) { - // Override the flag-bit guess from `parse_block` - // (meaningful for SimpleBlock only) with the - // BlockGroup's authoritative signal. - frame.keyframe = !has_reference; + )?; + // Override the flag-bit guess from `parse_block` + // (meaningful for SimpleBlock only) with the + // BlockGroup's authoritative signal. + rs.pending.extend(frames.into_iter().map(|mut f| { + f.keyframe = !has_reference; + f + })); + if let Some(frame) = rs.pending.pop_front() { return Ok(Some(frame)); } } @@ -859,8 +881,12 @@ impl crate::pes::Stream for MkvStream { } fn codec_private(&self, track: usize) -> Option> { - let track_num = (track + 1) as u16; // MKV tracks are 1-based if let Mode::Read(ref rs) = self.mode { + // `track` is a stream index; `codec_privates` is keyed by Matroska + // TrackNumber. Those are NOT the same space (RFC 9559 §5.1.4.1.1 + // requires only a non-zero TrackNumber), so translate through the + // real map instead of assuming `track + 1`. + let track_num = rs.tracks.num_of(track)?; rs.codec_privates .iter() .find(|(tn, _)| *tn == track_num) @@ -914,6 +940,7 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { let mut ts_scale: u64 = 1_000_000; let mut streams: Vec = Vec::new(); let mut codec_privates: Vec<(u16, Vec)> = Vec::new(); + let mut tracks = TrackTable::default(); let (id, size, _) = ebml::read_element_header(r)?; if id != ebml::EBML { @@ -982,9 +1009,13 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { } remaining = remaining.saturating_sub(hlen as u64 + cs); if cid == ebml::TRACK_ENTRY { - let (stream, tnum, cp) = parse_track(r, cs)?; + let (stream, tnum, cp, default_dur) = parse_track(r, cs)?; if let Some(s) = stream { + // Record the TrackNumber alongside the stream it maps + // to, in the SAME order, so block routing never has to + // guess that TrackNumbers are 1..=N. streams.push(s); + tracks.push(tnum, default_dur); } if let Some(cp) = cp { codec_privates.push((tnum, cp)); @@ -1016,7 +1047,7 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { } else { ts_scale as i64 }; - Ok((disc_title, codec_privates, ts_scale_ns)) + Ok((disc_title, codec_privates, ts_scale_ns, tracks)) } /// Largest valid 13-bit MPEG-TS PID. @@ -1043,12 +1074,24 @@ fn ts_pid_for_track(tnum: u16) -> io::Result { Ok(pid as u16) } -/// Returns (stream, track_number, codec_private_bytes) -fn parse_track( - r: &mut impl Read, - size: u64, -) -> io::Result<(Option, u16, Option>)> { +/// (stream, track_number, codec_private_bytes, default_duration_ns) — one +/// decoded `TrackEntry`. `stream` is `None` for a TrackType this crate does not +/// carry, in which case the TrackNumber gets no stream index at all. +type ParsedTrack = ( + Option, + u16, + Option>, + Option, +); + +/// Returns (stream, track_number, codec_private_bytes, default_duration_ns) +fn parse_track(r: &mut impl Read, size: u64) -> io::Result { let (mut ttype, mut tnum) = (0u64, 0u16); + /// RFC 9559 §5.1.4.1.13 gives DefaultDuration as nanoseconds per frame with + /// "range: not 0". A value this large is nonsense for a frame period and + /// would only skew laced-frame spacing, so treat it as absent. + const MAX_DEFAULT_DURATION_NS: u64 = 60 * 1_000_000_000; + let mut default_dur: Option = None; let (mut codec_id, mut lang, mut name) = (String::new(), String::from("und"), String::new()); let (mut ph, mut sr, mut ch, mut forced) = (0u32, 0.0f64, 0u8, false); let mut codec_priv: Option> = None; @@ -1072,6 +1115,10 @@ fn parse_track( tnum = n as u16; } ebml::TRACK_TYPE => ttype = read_uint_bounded(r, cs)?, + ebml::DEFAULT_DURATION => { + let ns = read_uint_bounded(r, cs)?; + default_dur = (ns > 0 && ns <= MAX_DEFAULT_DURATION_NS).then_some(ns); + } ebml::CODEC_ID => codec_id = read_string_bounded(r, cs)?, ebml::CODEC_PRIVATE => { codec_priv = Some(ebml::read_binary_val( @@ -1212,65 +1259,322 @@ fn parse_track( })), _ => None, }; - Ok((stream, tnum, codec_priv)) + Ok((stream, tnum, codec_priv, default_dur)) } -/// Parse a (Simple)Block payload into a PesFrame, or `None` if it should be -/// skipped (too short, track 0, or a track index out of range). +/// Read-side map from Matroska TrackNumber to the index of the corresponding +/// entry in `DiscTitle::streams`. +/// +/// RFC 9559 §5.1.4.1.1 constrains TrackNumber only to be non-zero ("range: not +/// 0"); NOTHING in the specification requires the numbers to be `1..=N`, to be +/// contiguous, or to appear in ascending TrackEntry order. `parse_track` also +/// DROPS every TrackEntry whose TrackType this crate cannot carry (anything but +/// 1/2/17 — e.g. a TrackType 18 buttons track), so the TrackNumber space and the +/// stream vector diverge for perfectly legal inputs. +/// +/// The reader used to derive the stream index as `TrackNumber - 1`, which routes +/// blocks to the WRONG stream (parsed by the wrong codec parser) or drops them +/// entirely. This table records the real TrackNumber for each retained stream, +/// in stream order, and is the only thing allowed to translate between the two. +#[derive(Default)] +struct TrackTable { + /// Matroska TrackNumber of `DiscTitle::streams[i]`, indexed by `i`. + nums: Vec, + /// TrackEntry `DefaultDuration` (RFC 9559 §5.1.4.1.13 — nanoseconds per + /// frame, already in Matroska Ticks = ns), per stream index, when declared. + /// Used to space the frames of a LACED Block, whose second and later frames + /// carry an "underdetermined" timestamp per RFC 9559 §10.3.5. + default_durations: Vec>, +} + +impl TrackTable { + fn push(&mut self, num: u16, default_duration_ns: Option) { + self.nums.push(num); + self.default_durations.push(default_duration_ns); + } + + /// Stream index carrying blocks with this TrackNumber, or `None` when the + /// file has no such (retained) track. + fn index_of(&self, num: u64) -> Option { + if num == 0 || num > u16::MAX as u64 { + return None; + } + let num = num as u16; + self.nums.iter().position(|&n| n == num) + } + + /// TrackNumber of a stream index (the inverse of `index_of`). + fn num_of(&self, idx: usize) -> Option { + self.nums.get(idx).copied() + } + + /// TrackNumbers `1..=n` in stream order — the layout this crate's own writer + /// emits, and the shape the unit tests exercise. + #[cfg(test)] + fn contiguous(n: usize) -> Self { + Self { + nums: (1..=n as u16).collect(), + default_durations: vec![None; n], + } + } +} + +/// Lacing mode from the 2-bit LACING field of a (Simple)Block flags byte +/// (RFC 9559 §10.1/§10.2: `KEY | Rsvrd | INV | LACING(2) | DIS`, bit 0 = MSB, +/// so the field is `flags & 0x06` shifted right by 1). +const LACING_MASK: u8 = 0x06; +const LACING_NONE: u8 = 0b00; +const LACING_XIPH: u8 = 0b01; +const LACING_FIXED: u8 = 0b10; +const LACING_EBML: u8 = 0b11; + +/// Read one unsigned EBML VINT (RFC 8794 §4.4) from the head of `d`, returning +/// `(value, octet width)`. `None` when the first octet has no VINT_MARKER (a +/// width above 8 octets, which Matroska lacing never uses) or the value is +/// truncated. +/// +/// Distinct from `block_vint`: that one is the *track number* decoder and caps +/// at 4 octets with a "treat as track 0" fallback, whereas lacing sizes need the +/// full 1..=8 range and must be able to report malformedness. +fn lace_vint(d: &[u8]) -> Option<(u64, usize)> { + let first = *d.first()?; + if first == 0 { + return None; // width > 8 octets — not representable here + } + let width = first.leading_zeros() as usize + 1; // 1..=8 + if d.len() < width { + return None; + } + // Strip the VINT_MARKER bit, then fold in the remaining octets big-endian. + let mut v = (first as u64) & (0xFFu64 >> width); + for &b in &d[1..width] { + v = (v << 8) | b as u64; + } + Some((v, width)) +} + +/// Read one SIGNED EBML lacing VINT. Per RFC 9559 §10.3.3 the signed value is +/// the unsigned VINT value minus `2^((7*n)-1) - 1`, where `n` is the octet width. +fn lace_svint(d: &[u8]) -> Option<(i64, usize)> { + let (v, width) = lace_vint(d)?; + // width <= 8 → 7*8-1 = 55, so both the bias and `v` (at most 2^56-1) are + // exactly representable in i64; no overflow is possible here. + let bias = (1i64 << (7 * width as u32 - 1)) - 1; + Some(((v as i64) - bias, width)) +} + +/// Split the body of a LACED (Simple)Block — the bytes after the flags octet, +/// beginning with the Lacing Head — into its individual frame payloads, per +/// RFC 9559 §10.3. +/// +/// `lacing` is the 2-bit LACING field value (`LACING_XIPH`, `LACING_EBML` or +/// `LACING_FIXED`). Returns `None` when the lacing header is malformed — the +/// frame boundaries are then unknown, and the caller MUST reject the block +/// rather than hand a concatenation of frames plus lacing header downstream as +/// though it were one frame (which is exactly the silent corruption this +/// function exists to end). +/// +/// The Lacing Head is "number of frames in the lace minus 1" on one octet, so +/// the frame count is bounded by 256 and no allocation here is attacker-scaled. +fn split_lacing(lacing: u8, body: &[u8]) -> Option> { + let (&count_minus_one, rest) = body.split_first()?; + let n = count_minus_one as usize + 1; + + // Sizes of the first n-1 frames; the last frame's size is deduced from what + // remains in the Block (RFC 9559 §10.3.2/§10.3.3). + let mut sizes: Vec = Vec::with_capacity(n); + let mut pos = 0usize; + match lacing { + LACING_FIXED => { + // §10.3.4: no sizes are stored; every frame MUST have the same size, + // deduced from the Block's total size. A body that does not divide + // evenly is malformed. + if rest.len() % n != 0 { + return None; + } + let each = rest.len() / n; + return Some(rest.chunks(each.max(1)).take(n).collect()); + } + LACING_XIPH => { + // §10.3.2: each size is a run of 0xFF octets (255 each) terminated + // by an octet below 255 (which may itself be 0). + for _ in 0..n - 1 { + let mut sz = 0usize; + loop { + let b = *rest.get(pos)?; + pos += 1; + sz = sz.checked_add(b as usize)?; + if b != 0xFF { + break; + } + } + sizes.push(sz); + } + } + LACING_EBML => { + // §10.3.3: the first size is an unsigned VINT; each later size is a + // SIGNED VINT holding the difference from the previous size. + if n >= 2 { + let (first, w) = lace_vint(rest.get(pos..)?)?; + pos += w; + let mut prev = i64::try_from(first).ok()?; + sizes.push(usize::try_from(prev).ok()?); + for _ in 0..n - 2 { + let (delta, w) = lace_svint(rest.get(pos..)?)?; + pos += w; + prev = prev.checked_add(delta)?; + sizes.push(usize::try_from(prev).ok()?); + } + } + } + _ => return None, + } + + // Carve the frames out of the bytes after the size table. The declared sizes + // must fit inside what remains, with the remainder going to the last frame. + let payload = rest.get(pos..)?; + let declared: usize = sizes.iter().try_fold(0usize, |a, &s| a.checked_add(s))?; + let last = payload.len().checked_sub(declared)?; + sizes.push(last); + + let mut out = Vec::with_capacity(n); + let mut at = 0usize; + for sz in sizes { + let end = at.checked_add(sz)?; + out.push(payload.get(at..end)?); + at = end; + } + Some(out) +} + +/// Parse a (Simple)Block payload into zero or more PesFrames. +/// +/// Zero frames means the block was SKIPPED — too short, track 0, or a +/// TrackNumber this file does not (retainedly) declare. More than one frame +/// means the Block was LACED (RFC 9559 §10.3): one Block legitimately carries +/// several frames, and handing the raw payload downstream as a single frame +/// feeds the codec parser a concatenation of frames plus lacing header. An +/// `Err` means the lacing header is malformed, so the frame boundaries are +/// unknowable — the block is rejected rather than mangled. /// /// `cluster_ts_ticks` is the open cluster's timestamp in TimestampScale ticks /// and `ts_scale_ns` is that scale (ns per tick); the block PTS is computed as /// `(cluster_ts_ticks + rel_ts) * ts_scale_ns` so foreign MKVs whose scale /// isn't 1 ms are honoured (freemkv's own output uses 1_000_000 and round-trips -/// unchanged). `streams_len` bounds the resolved track index; `duration_ns` is -/// propagated for BlockGroup blocks (None for SimpleBlock). +/// unchanged). `tracks` resolves the TrackNumber to a stream index; `duration_ns` +/// is propagated for BlockGroup blocks (None for SimpleBlock). fn parse_block( block: &[u8], cluster_ts_ticks: i64, ts_scale_ns: i64, - streams_len: usize, + tracks: &TrackTable, duration_ns: Option, -) -> Option { +) -> io::Result> { if block.len() < 4 { - return None; + return Ok(Vec::new()); } let (track, vl) = block_vint(block); if vl + 3 > block.len() { - return None; + return Ok(Vec::new()); } - // Track 0 is invalid (MKV track numbers are 1-based). block_vint also + // Track 0 is invalid (RFC 9559 §5.1.4.1.1: "range: not 0"). block_vint also // returns 0 for an unsupported 5+ byte VINT, so a corrupt/zero-track block // must be skipped rather than attributed to the first stream. if track == 0 { - return None; + return Ok(Vec::new()); } let rel_ts = i16::from_be_bytes([block[vl], block[vl + 1]]); - let keyframe = block[vl + 2] & 0x80 != 0; - let data = block[vl + 3..].to_vec(); + let flags = block[vl + 2]; + let keyframe = flags & 0x80 != 0; + let body = &block[vl + 3..]; // saturating_add: a hostile CLUSTER_TIMESTAMP near i64::MAX plus a positive // rel_ts would overflow this add (panic in debug/test, wrap to a large // negative PTS in release) — one operation BEFORE the saturating_mul below. // rel_ts as i64 is exact, so this fully bounds the sum on adversarial input. let pts_ticks = cluster_ts_ticks.saturating_add(rel_ts as i64); - let track_idx = (track as usize) - 1; // track >= 1 checked above + // saturating_mul: a hostile CLUSTER_TIMESTAMP could push pts_ticks near + // i64::MAX, where ticks→ns would overflow and panic in debug builds. + let base_pts = pts_ticks.saturating_mul(ts_scale_ns); - // Skip blocks for non-existent tracks. - if track_idx >= streams_len { - return None; + // Blocks for tracks this file does not declare (or whose TrackType this + // reader dropped) are skipped. Resolved through the real TrackNumber→index + // map, NOT `TrackNumber - 1`. + let Some(track_idx) = tracks.index_of(track) else { + return Ok(Vec::new()); + }; + + let lacing = (flags & LACING_MASK) >> 1; + if lacing == LACING_NONE { + return Ok(vec![crate::pes::PesFrame { + coding: None, + source: None, + track: track_idx, + pts: base_pts, + keyframe, + data: body.to_vec(), + duration_ns, + }]); } - Some(crate::pes::PesFrame { - coding: None, - source: None, - track: track_idx, - // saturating_mul: a hostile CLUSTER_TIMESTAMP could push pts_ticks near - // i64::MAX, where ticks→ns would overflow and panic in debug builds. - pts: pts_ticks.saturating_mul(ts_scale_ns), - keyframe, - data, - duration_ns, - }) + let Some(laced) = split_lacing(lacing, body) else { + tracing::warn!( + target: "mux", + track_number = track, + lacing, + body_len = body.len(), + "mkv read-back: malformed lacing header in a (Simple)Block; the frame \ + boundaries are unknowable, so the block is rejected rather than passed \ + downstream as one mangled frame" + ); + // NOT MkvInvalid: `error::is_skippable_title_stub` classifies that code + // as an empty nav/menu stub, so a real track with unseparable frames + // would be dropped by the caller and the run would still report success. + return Err(crate::error::Error::MkvLacingInvalid.into()); + }; + + // RFC 9559 §10.3.5: a Block carries a single timestamp, which applies to the + // FIRST frame of the lace; every later frame has an "underdetermined" + // timestamp but MUST be contiguous with its predecessor. Recover the spacing + // from the track's DefaultDuration (§5.1.4.1.13, already nanoseconds) when + // declared, else by dividing this Block's BlockDuration across the lace. + let count = laced.len().max(1) as u64; + let per_frame_ns = tracks + .default_durations + .get(track_idx) + .copied() + .flatten() + .or_else(|| duration_ns.map(|d| d / count)); + if per_frame_ns.is_none() && laced.len() > 1 { + tracing::warn!( + target: "mux", + track_number = track, + frames = laced.len(), + "mkv read-back: laced Block on a track with neither DefaultDuration nor \ + BlockDuration; the laced frames share one timestamp because the source \ + declares nothing to derive their spacing from (RFC 9559 §10.3.5)" + ); + } + + let mut out = Vec::with_capacity(laced.len()); + for (i, data) in laced.into_iter().enumerate() { + let step = per_frame_ns + .unwrap_or(0) + .saturating_mul(i as u64) + .min(i64::MAX as u64) as i64; + out.push(crate::pes::PesFrame { + coding: None, + source: None, + track: track_idx, + pts: base_pts.saturating_add(step), + keyframe, + data: data.to_vec(), + // A laced Block's BlockDuration covers the WHOLE lace, so the + // per-frame duration is the derived spacing, not the block's. + duration_ns: per_frame_ns, + }); + } + Ok(out) } fn block_vint(d: &[u8]) -> (u64, usize) { @@ -1674,9 +1978,13 @@ mod tests { } fn is_mkv_invalid(e: &io::Error) -> bool { - e.kind() == io::ErrorKind::InvalidData - && e.to_string() - .starts_with(&format!("E{}", crate::error::E_MKV_INVALID)) + has_code(e, crate::error::E_MKV_INVALID) + } + + /// Whether an error carries the given numeric code (the crate's errors + /// render as `E` with no English text). + fn has_code(e: &io::Error, code: u16) -> bool { + e.kind() == io::ErrorKind::InvalidData && e.to_string().starts_with(&format!("E{code}")) } #[test] @@ -2301,17 +2609,42 @@ mod tests { } // ============================================================ - // parse_block — turns a (Simple)Block payload into a PesFrame. + // parse_block — turns a (Simple)Block payload into PesFrames. // Layout: [track VINT][rel_ts i16 BE][flags u8][data...]. - // Guards: len<4 → None; vl+3 > len → None; track 0 → None; - // track_idx >= streams_len → None. + // Guards: len<4 → none; vl+3 > len → none; track 0 → none; + // unknown TrackNumber → none. // ============================================================ + /// `parse_block` for an UNLACED block on a file whose TrackNumbers are + /// `1..=streams_len` (the layout this crate's own writer emits): the single + /// frame, or `None` when the block was skipped. + fn parse_block_one( + block: &[u8], + cluster_ts_ticks: i64, + ts_scale_ns: i64, + streams_len: usize, + duration_ns: Option, + ) -> Option { + let frames = parse_block( + block, + cluster_ts_ticks, + ts_scale_ns, + &TrackTable::contiguous(streams_len), + duration_ns, + ) + .expect("unlaced block never errors"); + assert!( + frames.len() <= 1, + "an unlaced block yields at most one frame" + ); + frames.into_iter().next() + } + #[test] fn parse_block_too_short_is_none() { // Fewer than 4 bytes can't hold vint(1)+ts(2)+flags(1); must be None. - assert!(parse_block(&[0x81, 0x00, 0x00], 0, 1_000_000, 1, None).is_none()); - assert!(parse_block(&[], 0, 1_000_000, 1, None).is_none()); + assert!(parse_block_one(&[0x81, 0x00, 0x00], 0, 1_000_000, 1, None).is_none()); + assert!(parse_block_one(&[], 0, 1_000_000, 1, None).is_none()); } #[test] @@ -2319,7 +2652,7 @@ mod tests { // A 2-byte track VINT (0x40 0x01) needs vl(2)+3 = 5 bytes minimum, but // only 4 are supplied → vl+3 > len → None (no OOB index of data slice). let block = [0x40u8, 0x01, 0x00, 0x00]; // len 4, vl 2 → 2+3=5 > 4 - assert!(parse_block(&block, 0, 1_000_000, 2, None).is_none()); + assert!(parse_block_one(&block, 0, 1_000_000, 2, None).is_none()); } #[test] @@ -2327,9 +2660,9 @@ mod tests { // track 2 → index 1, but only 1 stream exists → must skip (None), // never index past the streams slice. let block = [0x82u8, 0x00, 0x00, 0x80, 0xAA]; // track 2 - assert!(parse_block(&block, 0, 1_000_000, 1, None).is_none()); + assert!(parse_block_one(&block, 0, 1_000_000, 1, None).is_none()); // With 2 streams it resolves to index 1. - let f = parse_block(&block, 0, 1_000_000, 2, None).unwrap(); + let f = parse_block_one(&block, 0, 1_000_000, 2, None).unwrap(); assert_eq!(f.track, 1); } @@ -2339,11 +2672,11 @@ mod tests { // the result must scale accordingly (foreign MKVs). rel_ts = 10 here. let block = [0x81u8, 0x00, 0x0A, 0x80, 0xAA]; // track 1, rel 10, kf // ts_scale 1_000_000 (1ms): cluster 100 + rel 10 = 110 ticks → 110ms. - let f = parse_block(&block, 100, 1_000_000, 1, None).unwrap(); + let f = parse_block_one(&block, 100, 1_000_000, 1, None).unwrap(); assert_eq!(f.pts, 110 * 1_000_000); assert!(f.keyframe); // ts_scale 90_000 (90kHz): (100+10) * 90_000. - let f = parse_block(&block, 100, 90_000, 1, None).unwrap(); + let f = parse_block_one(&block, 100, 90_000, 1, None).unwrap(); assert_eq!(f.pts, 110 * 90_000); } @@ -2352,7 +2685,7 @@ mod tests { // rel_ts is a SIGNED 16-bit big-endian value. 0xFFFF = -1. The pts must // go DOWN from the cluster timestamp, not jump to +65535. let block = [0x81u8, 0xFF, 0xFF, 0x80, 0xAA]; // rel_ts = -1 - let f = parse_block(&block, 100, 1_000_000, 1, None).unwrap(); + let f = parse_block_one(&block, 100, 1_000_000, 1, None).unwrap(); assert_eq!(f.pts, 99 * 1_000_000, "rel_ts -1 must subtract one tick"); } @@ -2362,9 +2695,17 @@ mod tests { // passed through unchanged (BlockGroup path supplies it). let kf = [0x81u8, 0x00, 0x00, 0x80, 0xAA]; let nkf = [0x81u8, 0x00, 0x00, 0x00, 0xAA]; - assert!(parse_block(&kf, 0, 1_000_000, 1, None).unwrap().keyframe); - assert!(!parse_block(&nkf, 0, 1_000_000, 1, None).unwrap().keyframe); - let f = parse_block(&kf, 0, 1_000_000, 1, Some(40_000_000)).unwrap(); + assert!( + parse_block_one(&kf, 0, 1_000_000, 1, None) + .unwrap() + .keyframe + ); + assert!( + !parse_block_one(&nkf, 0, 1_000_000, 1, None) + .unwrap() + .keyframe + ); + let f = parse_block_one(&kf, 0, 1_000_000, 1, Some(40_000_000)).unwrap(); assert_eq!(f.duration_ns, Some(40_000_000)); } @@ -2374,7 +2715,7 @@ mod tests { // ticks→ns multiply; saturating_mul caps it. (Guards the debug-build // overflow the source comment calls out.) let block = [0x81u8, 0x00, 0x00, 0x80, 0xAA]; - let f = parse_block(&block, i64::MAX, 1_000_000, 1, None).unwrap(); + let f = parse_block_one(&block, i64::MAX, 1_000_000, 1, None).unwrap(); assert_eq!(f.pts, i64::MAX, "ticks→ns must saturate, not wrap/panic"); } @@ -2386,7 +2727,7 @@ mod tests { // checks on) and silently wraps to a large negative PTS in release. // rel_ts = +0x7FFF = 32767 (max positive signed 16-bit). let block = [0x81u8, 0x7F, 0xFF, 0x80, 0xAA]; - let f = parse_block(&block, i64::MAX, 1_000_000, 1, None).unwrap(); + let f = parse_block_one(&block, i64::MAX, 1_000_000, 1, None).unwrap(); // The add saturates at i64::MAX, then the mul saturates too. assert_eq!( f.pts, @@ -2594,4 +2935,371 @@ mod tests { assert!(stream.read().unwrap().is_some(), "first frame"); assert!(stream.read().unwrap().is_none(), "clean EOF → None"); } + + // ============================================================ + // Block LACING (RFC 9559 §10.3) and TrackNumber→stream routing + // (RFC 9559 §5.1.4.1.1). + // ============================================================ + + /// One TrackEntry description for `mkv_with_tracks_and_cluster`: + /// (TrackNumber, TrackType, DefaultDuration ns, CodecPrivate). + struct TrackSpec { + tnum: u64, + ttype: u64, + default_duration_ns: Option, + codec_private: Option>, + } + + impl TrackSpec { + fn new(tnum: u64, ttype: u64) -> Self { + Self { + tnum, + ttype, + default_duration_ns: None, + codec_private: None, + } + } + fn with_default_duration(mut self, ns: u64) -> Self { + self.default_duration_ns = Some(ns); + self + } + fn with_codec_private(mut self, cp: &[u8]) -> Self { + self.codec_private = Some(cp.to_vec()); + self + } + } + + /// Build an MKV header with an arbitrary set of TrackEntries — arbitrary + /// TrackNumbers, in arbitrary order — followed by `cluster_body`. + fn mkv_with_tracks_and_cluster(specs: &[TrackSpec], cluster_body: &[u8]) -> Vec { + let mut out = Vec::new(); + ebml::write_id(&mut out, ebml::EBML).unwrap(); + ebml::write_size(&mut out, 0).unwrap(); + ebml::write_id(&mut out, ebml::SEGMENT).unwrap(); + ebml::write_unknown_size(&mut out).unwrap(); + ebml::write_id(&mut out, ebml::INFO).unwrap(); + ebml::write_size(&mut out, 0).unwrap(); + + let mut tracks = Vec::new(); + for s in specs { + let mut entry = Vec::new(); + ebml::write_uint(&mut entry, ebml::TRACK_NUMBER, s.tnum).unwrap(); + ebml::write_uint(&mut entry, ebml::TRACK_TYPE, s.ttype).unwrap(); + if let Some(ns) = s.default_duration_ns { + ebml::write_uint(&mut entry, ebml::DEFAULT_DURATION, ns).unwrap(); + } + if let Some(cp) = &s.codec_private { + ebml::write_binary(&mut entry, ebml::CODEC_PRIVATE, cp).unwrap(); + } + ebml::write_id(&mut tracks, ebml::TRACK_ENTRY).unwrap(); + ebml::write_size(&mut tracks, entry.len() as u64).unwrap(); + tracks.extend_from_slice(&entry); + } + ebml::write_id(&mut out, ebml::TRACKS).unwrap(); + ebml::write_size(&mut out, tracks.len() as u64).unwrap(); + out.extend_from_slice(&tracks); + out.extend_from_slice(cluster_body); + out + } + + /// Wrap one raw (Simple)Block payload in a Cluster with the given timestamp. + fn cluster_with_simple_block(cluster_ts: u64, block: &[u8]) -> Vec { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + ebml::write_uint(&mut cluster, ebml::CLUSTER_TIMESTAMP, cluster_ts).unwrap(); + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, block.len() as u64).unwrap(); + cluster.extend_from_slice(block); + cluster + } + + /// Drain every frame a reader will yield. + fn drain(stream: &mut MkvStream) -> Vec { + let mut out = Vec::new(); + while let Some(f) = stream.read().expect("no read error") { + out.push(f); + } + out + } + + /// EBML lacing (RFC 9559 §10.3.3): the Lacing Head, the first frame's size as + /// an unsigned VINT, then each later size as a SIGNED VINT difference from + /// the previous one. Three frames of 3/4/5 octets must come out as THREE + /// frames with byte-exact payloads. + /// + /// Regression (silent corruption): the reader took the Block payload verbatim + /// and never looked at the LACING bits, so this Block became ONE 15-byte frame + /// whose first three bytes are the lacing header — garbage handed to the codec + /// parser with no error, and one timestamp for three frames. + #[test] + fn ebml_laced_block_yields_every_frame_with_exact_payloads() { + // size 3 → 0x83 (VINT, value 3). size delta 4-3 = +1 → unsigned 1 + bias + // (2^6-1 = 63) = 64 → 0xC0 with the VINT_MARKER. + let mut block = vec![ + 0x81, // TrackNumber 1 + 0x00, 0x00, // rel_ts 0 + 0x86, // KEY | LACING = 11b (EBML) + 0x02, // Lacing Head: 3 frames minus 1 + 0x83, // first frame size = 3 + 0xC0, // second frame size = previous + 1 = 4 + ]; + block.extend_from_slice(&[0xAA; 3]); + block.extend_from_slice(&[0xBB; 4]); + block.extend_from_slice(&[0xCC; 5]); + + // DefaultDuration 24 ms/frame is what §10.3.5 leaves the reader to space + // the second and later frames by. + let specs = [TrackSpec::new(1, 2).with_default_duration(24_000_000)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster_with_simple_block(100, &block)); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let frames = drain(&mut stream); + + assert_eq!(frames.len(), 3, "one laced Block carries three frames"); + assert_eq!(frames[0].data, vec![0xAA; 3], "frame 1 payload byte-exact"); + assert_eq!(frames[1].data, vec![0xBB; 4], "frame 2 payload byte-exact"); + assert_eq!(frames[2].data, vec![0xCC; 5], "frame 3 payload byte-exact"); + for f in &frames { + assert_eq!(f.track, 0); + assert!(f.keyframe, "the KEY flag covers the whole lace"); + assert_eq!(f.duration_ns, Some(24_000_000)); + } + // The Block timestamp applies to the FIRST frame; the rest are spaced by + // DefaultDuration (§10.3.5). + assert_eq!(frames[0].pts, 100 * 1_000_000); + assert_eq!(frames[1].pts, 100 * 1_000_000 + 24_000_000); + assert_eq!(frames[2].pts, 100 * 1_000_000 + 48_000_000); + } + + /// Xiph lacing (RFC 9559 §10.3.2): sizes are runs of 0xFF octets terminated + /// by an octet below 255, and a size that is a multiple of 255 ends in a 0. + #[test] + fn xiph_laced_block_splits_on_255_coded_sizes() { + let mut block = vec![ + 0x81, // TrackNumber 1 + 0x00, 0x00, // rel_ts 0 + 0x82, // KEY | LACING = 01b (Xiph) + 0x01, // Lacing Head: 2 frames minus 1 + 0xFF, 0x00, // first frame size = 255 (a multiple of 255 → trailing 0) + ]; + block.extend_from_slice(&[0xAA; 255]); + block.extend_from_slice(&[0xBB; 2]); + + let specs = [TrackSpec::new(1, 2)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster_with_simple_block(0, &block)); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let frames = drain(&mut stream); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].data, vec![0xAA; 255]); + assert_eq!( + frames[1].data, + vec![0xBB; 2], + "the last frame's size is the Block remainder" + ); + } + + /// Fixed-size lacing (RFC 9559 §10.3.4): no sizes are stored; every frame is + /// the Block remainder divided by the frame count. + #[test] + fn fixed_size_laced_block_splits_evenly() { + let mut block = vec![ + 0x81, // TrackNumber 1 + 0x00, 0x00, // rel_ts 0 + 0x84, // KEY | LACING = 10b (fixed-size) + 0x02, // Lacing Head: 3 frames minus 1 + ]; + block.extend_from_slice(&[0x11, 0x11, 0x22, 0x22, 0x33, 0x33]); + + let specs = [TrackSpec::new(1, 2)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster_with_simple_block(0, &block)); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let frames = drain(&mut stream); + assert_eq!(frames.len(), 3); + assert_eq!(frames[0].data, vec![0x11, 0x11]); + assert_eq!(frames[1].data, vec![0x22, 0x22]); + assert_eq!(frames[2].data, vec![0x33, 0x33]); + } + + /// A lacing header whose declared sizes do not fit in the Block leaves the + /// frame boundaries unknowable. That MUST be an error, never a pass-through + /// of the raw payload as one frame. + #[test] + fn malformed_lacing_header_is_rejected_not_passed_through() { + // Xiph, 2 frames, first size declared as 200 but only 4 payload octets + // follow → the remainder for the last frame underflows. + let block = [ + 0x81, 0x00, 0x00, 0x82, // KEY | Xiph lacing + 0x01, // 2 frames + 0xC8, // first frame size = 200 + 0xAA, 0xBB, 0xCC, 0xDD, + ]; + let specs = [TrackSpec::new(1, 2)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster_with_simple_block(0, &block)); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let e = stream.read().unwrap_err(); + assert!( + has_code(&e, crate::error::E_MKV_LACING_INVALID), + "malformed lacing must be rejected" + ); + assert!( + !crate::error::is_skippable_title_stub(&e), + "a track whose frames cannot be separated is NOT an empty nav stub" + ); + + // Fixed-size lacing whose body does not divide evenly by the frame count. + let block = [ + 0x81, 0x00, 0x00, 0x84, // KEY | fixed-size lacing + 0x02, // 3 frames + 0xAA, 0xBB, 0xCC, 0xDD, // 4 octets — not divisible by 3 + ]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster_with_simple_block(0, &block)); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + assert!(has_code( + &stream.read().unwrap_err(), + crate::error::E_MKV_LACING_INVALID + )); + } + + /// A laced Block on a track with no DefaultDuration falls back to spreading + /// the BlockGroup's BlockDuration across the lace: the Block's duration covers + /// the WHOLE lace (RFC 9559 §5.1.3.5), not each frame. + #[test] + fn laced_block_duration_is_divided_across_the_lace() { + let mut block = vec![0x81u8, 0x00, 0x00, 0x04, 0x01]; // fixed-size, 2 frames, no KEY + block.extend_from_slice(&[0x11, 0x22]); + let mut bg_body = Vec::new(); + ebml::write_id(&mut bg_body, ebml::BLOCK).unwrap(); + ebml::write_size(&mut bg_body, block.len() as u64).unwrap(); + bg_body.extend_from_slice(&block); + // 48 ms for the pair → 24 ms per frame. + ebml::write_uint(&mut bg_body, ebml::BLOCK_DURATION, 48).unwrap(); + + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + ebml::write_id(&mut cluster, ebml::BLOCK_GROUP).unwrap(); + ebml::write_size(&mut cluster, bg_body.len() as u64).unwrap(); + cluster.extend_from_slice(&bg_body); + + let specs = [TrackSpec::new(1, 2)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let frames = drain(&mut stream); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].data, vec![0x11]); + assert_eq!(frames[1].data, vec![0x22]); + assert_eq!(frames[0].duration_ns, Some(24_000_000)); + assert_eq!(frames[1].pts, 24_000_000, "spaced by the derived duration"); + } + + /// RFC 9559 §5.1.4.1.1 constrains TrackNumber only to be non-zero — nothing + /// requires `1..=N` in TrackEntry order. A file with a TrackType this reader + /// drops (18 = buttons) between two carried tracks makes the TrackNumber + /// space and the stream vector diverge. + /// + /// Regression (silent corruption): the reader computed the stream index as + /// `TrackNumber - 1`, so the audio blocks of TrackNumber 3 resolved to index + /// 2 in a 2-stream title and were DISCARDED — a remux with no audio, reported + /// as success. + #[test] + fn sparse_track_numbers_route_to_the_right_stream() { + let video = [0x81u8, 0x00, 0x00, 0x80, 0x11]; // TrackNumber 1 + let audio = [0x83u8, 0x00, 0x0A, 0x80, 0x22]; // TrackNumber 3, rel_ts 10 + let buttons = [0x82u8, 0x00, 0x00, 0x80, 0x33]; // TrackNumber 2 — dropped track + + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + for b in [video.as_slice(), buttons.as_slice(), audio.as_slice()] { + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, b.len() as u64).unwrap(); + cluster.extend_from_slice(b); + } + + let specs = [ + TrackSpec::new(1, 1), // video → stream 0 + TrackSpec::new(2, 18), // buttons → dropped, no stream + TrackSpec::new(3, 2), // audio → stream 1 + ]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + assert_eq!( + stream.info().streams.len(), + 2, + "the buttons track is dropped" + ); + let frames = drain(&mut stream); + assert_eq!( + frames.len(), + 2, + "the video and audio blocks both survive; only the dropped track's do not" + ); + assert_eq!(frames[0].track, 0, "TrackNumber 1 → stream 0"); + assert_eq!(frames[0].data, vec![0x11]); + assert_eq!(frames[1].track, 1, "TrackNumber 3 → stream 1, not dropped"); + assert_eq!(frames[1].data, vec![0x22]); + } + + /// A descending TrackEntry order is legal too: the map is by number, not by + /// position, and a block must never be attributed to the wrong codec parser. + #[test] + fn descending_track_numbers_route_by_number_not_position() { + let first = [0x87u8, 0x00, 0x00, 0x80, 0xAA]; // TrackNumber 7 + let second = [0x84u8, 0x00, 0x00, 0x80, 0xBB]; // TrackNumber 4 + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + for b in [first.as_slice(), second.as_slice()] { + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, b.len() as u64).unwrap(); + cluster.extend_from_slice(b); + } + let specs = [TrackSpec::new(7, 1), TrackSpec::new(4, 2)]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let frames = drain(&mut stream); + assert_eq!(frames.len(), 2); + assert_eq!(frames[0].track, 0, "TrackNumber 7 is the FIRST TrackEntry"); + assert_eq!(frames[1].track, 1, "TrackNumber 4 is the second"); + } + + /// `codec_private(stream_idx)` is keyed by TrackNumber internally, so it must + /// translate through the same map — not assume `stream_idx + 1`. + #[test] + fn codec_private_resolves_through_the_track_number_map() { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + let specs = [ + TrackSpec::new(1, 1).with_codec_private(&[0x01, 0x02]), + TrackSpec::new(2, 18).with_codec_private(&[0xDE, 0xAD]), + TrackSpec::new(3, 2).with_codec_private(&[0x03, 0x04]), + ]; + let bytes = mkv_with_tracks_and_cluster(&specs, &cluster); + let stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + assert_eq!(stream.codec_private(0), Some(vec![0x01, 0x02])); + assert_eq!( + stream.codec_private(1), + Some(vec![0x03, 0x04]), + "stream 1 is TrackNumber 3 — not TrackNumber 2 (the dropped track)" + ); + assert_eq!(stream.codec_private(2), None, "no third stream"); + } + + /// The signed-VINT bias of RFC 9559 §10.3.3 exactly as the spec's own EBML + /// lacing example encodes it (800 then 500 → a delta of -300). + #[test] + fn lace_vint_matches_the_spec_worked_example() { + // 800 = 0x320, encoded as a 2-octet VINT: 0x43 0x20. + assert_eq!(lace_vint(&[0x43, 0x20]), Some((800, 2))); + // -300 as a 2-octet signed VINT: 0x5E 0xD3 (value 0x1ED3 minus bias 8191). + assert_eq!(lace_svint(&[0x5E, 0xD3]), Some((-300, 2))); + // 1-octet forms: 0x81 → 1; signed 0x80 → -(2^6-1) = -63. + assert_eq!(lace_vint(&[0x81]), Some((1, 1))); + assert_eq!(lace_svint(&[0x80]), Some((-63, 1))); + // A first octet of 0 has no VINT_MARKER within 8 octets → unrepresentable. + assert!(lace_vint(&[0x00, 0x01]).is_none()); + // Truncated: a 2-octet marker with only one octet available. + assert!(lace_vint(&[0x43]).is_none()); + } } diff --git a/src/mux/mp4/read.rs b/src/mux/mp4/read.rs index 4d62cad..c9d4fa3 100644 --- a/src/mux/mp4/read.rs +++ b/src/mux/mp4/read.rs @@ -99,6 +99,13 @@ impl Mp4Reader { let mut title = DiscTitle::empty(); title.playlist = name; + // Movie timescale (ISO/IEC 14496-12 §8.2.2). An edit list's + // `segment_duration` is expressed in it, while its `media_time` is in the + // track's own media timescale, so both are needed to place an edit. + let movie_timescale = find_box(&moov, b"mvhd") + .and_then(mvhd_timescale) + .filter(|&t| t != 0); + let mut samples: Vec = Vec::new(); let mut codec_privates: Vec>> = Vec::new(); let mut track_idx = 0usize; @@ -288,12 +295,27 @@ impl Mp4Reader { (ticks as i128 * NS / timescale as i128).clamp(i64::MIN as i128, i64::MAX as i128) as i64 }; + // Edit list (ISO/IEC 14496-12 §8.6.5 `edts` / §8.6.6 `elst`): the + // presentation timeline is NOT the media timeline. Ignoring it — which + // this reader did — starts every track at media time 0, so a track + // carrying the standard encoder-delay/A-V-offset edit ends up shifted + // against its siblings for the whole title, silently. + let edit_offset_ticks = find_box(trak, b"edts") + .and_then(|edts| find_box(edts, b"elst")) + .map(|elst| { + let entries = parse_elst(elst); + elst_offset_ticks(&entries, movie_timescale, timescale, track_idx) + }) + .unwrap_or(0); + let mut decode_ticks: i64 = 0; for (i, &size) in sizes.iter().enumerate() { let dur = durations.get(i).copied().unwrap_or(0); let comp = ctts.get(i).copied().unwrap_or(0); - let dts_ns = to_ns(decode_ticks); - let pts_ticks = decode_ticks.saturating_add(comp as i64); + let dts_ns = to_ns(decode_ticks.saturating_add(edit_offset_ticks)); + let pts_ticks = decode_ticks + .saturating_add(comp as i64) + .saturating_add(edit_offset_ticks); let pts_ns = to_ns(pts_ticks); decode_ticks = decode_ticks.saturating_add(dur as i64); let keyframe = match &sync { @@ -474,6 +496,152 @@ fn be16(b: &[u8], o: usize) -> u16 { u16::from_be_bytes([b[o], b[o + 1]]) } +/// mvhd (version 0/1) → movie timescale (ISO/IEC 14496-12 §8.2.2). +fn mvhd_timescale(b: &[u8]) -> Option { + let version = b.first().copied()?; + if version == 1 { + // version(1)+flags(3) creation(8) modification(8) timescale(4) ... + (b.len() >= 24).then(|| be32(b, 20)) + } else { + // version(1)+flags(3) creation(4) modification(4) timescale(4) ... + (b.len() >= 16).then(|| be32(b, 12)) + } +} + +/// Upper bound on parsed `elst` entries. Only the leading empty edits and the +/// FIRST non-empty edit shape the offset applied below, so a longer list buys +/// nothing but allocation; the cap keeps a crafted 256 MiB `moov` from turning +/// a box into a larger Vec than the box itself. +const MAX_ELST_ENTRIES: usize = 1024; + +/// One `elst` entry: `(segment_duration, media_time, media_rate_integer)`. +type EditListEntry = (u64, i64, i16); + +/// Parse an `elst` payload (ISO/IEC 14496-12 §8.6.6). Entry count is clamped +/// both by the box's own bytes and by [`MAX_ELST_ENTRIES`]. +/// +/// Version 1 entries are `segment_duration:u64, media_time:i64, +/// media_rate_integer:i16, media_rate_fraction:i16` (20 bytes); version 0 uses +/// 32-bit duration/time (12 bytes). +fn parse_elst(b: &[u8]) -> Vec { + if b.len() < 8 { + return Vec::new(); + } + let version = b[0]; + let entry_size = if version == 1 { 20 } else { 12 }; + let declared = be32(b, 4) as usize; + let available = (b.len() - 8) / entry_size; + let n = declared.min(available).min(MAX_ELST_ENTRIES); + let mut out = Vec::with_capacity(n); + for i in 0..n { + let o = 8 + i * entry_size; + let (seg, media_time, rate_off) = if version == 1 { + let seg = u64::from_be_bytes([ + b[o], + b[o + 1], + b[o + 2], + b[o + 3], + b[o + 4], + b[o + 5], + b[o + 6], + b[o + 7], + ]); + let mt = i64::from_be_bytes([ + b[o + 8], + b[o + 9], + b[o + 10], + b[o + 11], + b[o + 12], + b[o + 13], + b[o + 14], + b[o + 15], + ]); + (seg, mt, 16) + } else { + (be32(b, o) as u64, be32(b, o + 4) as i32 as i64, 8) + }; + let rate = be16(b, o + rate_off) as i16; + out.push((seg, media_time, rate)); + } + out +} + +/// Presentation-time offset an edit list imposes on a track's samples, in the +/// track's MEDIA timescale ticks (ISO/IEC 14496-12 §8.6.5-§8.6.6). +/// +/// Two constructs cover essentially every real edit list, and both reduce to a +/// constant shift of the whole track: +/// * an EMPTY edit (`media_time == -1`) before the media edit, whose +/// `segment_duration` — in MOVIE timescale ticks — delays presentation; +/// * a non-empty edit whose `media_time` trims that much media off the front. +/// +/// So the offset is `(sum of leading empty segment_durations) - media_time`. +/// A list with several non-empty edits, or a non-empty edit at a rate other than +/// 1, describes a timeline this frame model cannot express (it would need samples +/// dropped, reordered or repeated); the leading edit is still honoured, and the +/// part that is not is LOGGED rather than passed off as a faithful copy. +fn elst_offset_ticks( + entries: &[EditListEntry], + movie_timescale: Option, + media_timescale: u32, + track_idx: usize, +) -> i64 { + let mut empty_movie_ticks: u64 = 0; + let mut trim_media_ticks: i64 = 0; + let mut media_edits = 0usize; + let mut odd_rate = false; + + for &(segment_duration, media_time, rate) in entries { + if media_time < 0 { + // Empty edit: blank presentation time. Only the ones BEFORE the first + // media edit shift this track's start. + if media_edits == 0 { + empty_movie_ticks = empty_movie_ticks.saturating_add(segment_duration); + } + continue; + } + media_edits += 1; + if media_edits == 1 { + trim_media_ticks = media_time; + odd_rate = rate != 1; + } + } + + if media_edits > 1 || odd_rate { + tracing::warn!( + track = track_idx, + media_edits, + odd_rate, + "mp4: edit list describes a timeline richer than a constant shift \ + (several media edits, or a rate other than 1); only the leading edit \ + is applied and the remainder of the presentation timeline is not" + ); + } + + // An empty edit's duration is in MOVIE ticks; convert to media ticks before + // subtracting the media-timescale trim. i128 so neither product overflows. + let delay_media_ticks = match movie_timescale { + Some(mts) if empty_movie_ticks > 0 => { + ((empty_movie_ticks as i128 * media_timescale as i128) / mts as i128) + .clamp(0, i64::MAX as i128) as i64 + } + Some(_) => 0, + None => { + if empty_movie_ticks > 0 { + tracing::warn!( + track = track_idx, + "mp4: edit list has an empty edit but the movie timescale is \ + absent or zero, so its delay cannot be converted to media \ + ticks; the delay is not applied" + ); + } + 0 + } + }; + + delay_media_ticks.saturating_sub(trim_media_ticks) +} + /// mdhd (version 0/1) → media timescale. fn mdhd_timescale(b: &[u8]) -> Option { let version = b.first().copied()?; @@ -1329,6 +1497,143 @@ mod tests { ); } + // ============================================================ + // Edit lists — ISO/IEC 14496-12 §8.6.5 (`edts`) / §8.6.6 (`elst`). + // ============================================================ + + /// A `mvhd` payload declaring the movie timescale (ISO/IEC 14496-12 §8.2.2). + fn mvhd_box(timescale: u32) -> Vec { + // v0: version+flags(4) creation(4) modification(4) timescale(4) duration(4) … + let mut p = vec![0u8; 100]; + p[12..16].copy_from_slice(×cale.to_be_bytes()); + mp4_box(b"mvhd", &p) + } + + /// A version-0 `elst` payload: `(segment_duration, media_time)` per entry, + /// each at media_rate 1. + fn elst_v0(entries: &[(u32, i32)]) -> Vec { + let mut p = vec![0u8, 0, 0, 0]; // version 0 + flags + p.extend_from_slice(&(entries.len() as u32).to_be_bytes()); + for &(seg, media_time) in entries { + p.extend_from_slice(&seg.to_be_bytes()); + p.extend_from_slice(&media_time.to_be_bytes()); + p.extend_from_slice(&1i16.to_be_bytes()); // media_rate_integer + p.extend_from_slice(&0i16.to_be_bytes()); // media_rate_fraction + } + p + } + + /// Insert an `edts > elst` into an existing `trak` box. + fn trak_with_elst(trak: &[u8], elst_payload: &[u8]) -> Vec { + let mut payload = mp4_box(b"edts", &mp4_box(b"elst", elst_payload)); + payload.extend_from_slice(&trak[8..]); // the original trak's children + mp4_box(b"trak", &payload) + } + + /// Regression (silent A/V desync): the sample timeline was built purely from + /// stts/ctts starting at tick 0 and no `edts`/`elst` was ever parsed, so the + /// presentation timeline an edit list defines was discarded. A non-empty edit + /// with `media_time = 1024` — the standard way encoder delay is expressed — + /// must move the track's presentation, not be ignored. + #[test] + fn edit_list_media_time_shifts_the_presentation_timeline() { + use std::io::Cursor; + let trak = trak_with_elst(&audio_trak(48_000), &elst_v0(&[(0, 1024)])); + let moov = mp4_box(b"moov", &trak); + let rd = Mp4Reader::from_reader(Cursor::new(moov), "elst".into()).unwrap(); + assert_eq!(rd.samples.len(), 1); + // media_time 1024 at 48 kHz trims 1024 ticks off the front, so the first + // sample sits 1024 ticks BEFORE the presentation origin. + let want = -(1024i128 * NS / 48_000) as i64; + assert_eq!(rd.samples[0].pts_ns, want, "media_time must shift the pts"); + assert_eq!(rd.samples[0].dts_ns, want, "and the dts with it"); + assert_ne!(want, 0, "the shift is observable"); + } + + /// An EMPTY edit (`media_time == -1`) delays presentation by its + /// `segment_duration`, which is in MOVIE timescale ticks and must be + /// converted to the track's media timescale before it is applied. + #[test] + fn empty_edit_delays_presentation_in_movie_timescale() { + use std::io::Cursor; + // Movie timescale 1000 → segment_duration 40 = 40 ms of blank leader, + // then the media edit itself. + let trak = trak_with_elst(&audio_trak(48_000), &elst_v0(&[(40, -1), (0, 0)])); + let mut moov_payload = mvhd_box(1000); + moov_payload.extend_from_slice(&trak); + let moov = mp4_box(b"moov", &moov_payload); + let rd = Mp4Reader::from_reader(Cursor::new(moov), "empty-edit".into()).unwrap(); + assert_eq!(rd.samples.len(), 1); + assert_eq!( + rd.samples[0].pts_ns, 40_000_000, + "a 40 ms empty edit delays the track by 40 ms" + ); + } + + /// A track with no `edts` is untouched — the shift only ever comes from a + /// declared edit list. + #[test] + fn no_edit_list_leaves_the_timeline_at_zero() { + use std::io::Cursor; + let moov = mp4_box(b"moov", &audio_trak(48_000)); + let rd = Mp4Reader::from_reader(Cursor::new(moov), "no-elst".into()).unwrap(); + assert_eq!(rd.samples[0].pts_ns, 0); + assert_eq!(rd.samples[0].dts_ns, 0); + } + + /// `elst` decoding: both versions, the entry count bounded by the box's own + /// bytes, and the offset arithmetic in isolation. + #[test] + fn parse_elst_and_offset_arithmetic() { + // Version 0, two entries: an empty edit then a media edit. + let v0 = elst_v0(&[(40, -1), (0, 1024)]); + let entries = parse_elst(&v0); + assert_eq!(entries, vec![(40, -1, 1), (0, 1024, 1)]); + + // Version 1: 64-bit segment_duration and media_time. + let mut v1 = vec![1u8, 0, 0, 0]; + v1.extend_from_slice(&1u32.to_be_bytes()); + v1.extend_from_slice(&5_000u64.to_be_bytes()); + v1.extend_from_slice(&(-1i64).to_be_bytes()); + v1.extend_from_slice(&1i16.to_be_bytes()); + v1.extend_from_slice(&0i16.to_be_bytes()); + assert_eq!(parse_elst(&v1), vec![(5_000, -1, 1)]); + + // A declared count larger than the box can hold is clamped by the bytes. + let mut lying = elst_v0(&[(0, 0)]); + lying[4..8].copy_from_slice(&9_999u32.to_be_bytes()); + assert_eq!(parse_elst(&lying).len(), 1, "bounded by the box bytes"); + // Too short to hold even the header → no entries, no panic. + assert!(parse_elst(&[0, 0, 0, 0]).is_empty()); + + // Offset: the empty edit's 40 movie ticks at movie timescale 1000 is + // 40 ms = 1920 ticks at 48 kHz, minus a media_time trim of 1024. + assert_eq!( + elst_offset_ticks(&[(40, -1, 1), (0, 1024, 1)], Some(1000), 48_000, 0), + 1920 - 1024 + ); + // No movie timescale → the empty edit's delay cannot be converted, so + // only the trim applies (and it is logged, not silently dropped). + assert_eq!( + elst_offset_ticks(&[(40, -1, 1), (0, 1024, 1)], None, 48_000, 0), + -1024 + ); + // An empty list, or a single identity edit, shifts nothing. + assert_eq!(elst_offset_ticks(&[], Some(1000), 48_000, 0), 0); + assert_eq!(elst_offset_ticks(&[(1000, 0, 1)], Some(1000), 48_000, 0), 0); + // Only the FIRST media edit's media_time is applied; trailing empty + // edits do not add to the leading delay. + assert_eq!( + elst_offset_ticks(&[(0, 512, 1), (40, -1, 1)], Some(1000), 48_000, 0), + -512 + ); + // A hostile segment_duration cannot overflow the tick conversion. + assert_eq!( + elst_offset_ticks(&[(u64::MAX, -1, 1)], Some(1), 48_000, 0), + i64::MAX, + ); + } + #[test] fn trak_loop_stops_at_max_tracks() { use std::io::Cursor; diff --git a/src/mux/tsmux.rs b/src/mux/tsmux.rs index 2debbbe..70b2243 100644 --- a/src/mux/tsmux.rs +++ b/src/mux/tsmux.rs @@ -4,7 +4,9 @@ //! packets. Each frame is wrapped in a PES header, split into TS packets, //! and prepended with the 4-byte TP_extra_header. -use super::hevc::{append_length_prefixed_as_annex_b, avcc_to_annex_b, hvcc_to_annex_b}; +use super::hevc::{ + append_length_prefixed_as_annex_b_sized, avcc_to_annex_b, hvcc_to_annex_b, nal_length_size, +}; use crate::disc::Codec; use std::io::{self, Write}; @@ -239,7 +241,15 @@ impl TsMuxer { // and we then copied it in, so every video frame cost two full-frame // allocations and two full-frame copies. At ~200k frames averaging // ~310 KB of ES on a UHD, that is ~124 GB of pointless memcpy. - append_length_prefixed_as_annex_b(&mut annex_b, data); + // The prefix width is whatever the source's avcC/hvcC declares + // (ISO/IEC 14496-15 `lengthSizeMinusOne + 1`), NOT an assumed 4: + // a 1- or 2-octet-prefixed source read as u32-BE parses no NALs at + // all and its raw bytes are passed through with no start codes. + let length_size = nal_length_size( + self.video_codec[track], + self.codec_privates[track].as_deref(), + ); + append_length_prefixed_as_annex_b_sized(&mut annex_b, data, length_size); } else if is_video { self.params_written[track] = true; }