Merge codeaudit/b08-mkv (f72fbc2)

src/mux/ebml.rs

@@ -549,6 +549,8 @@ pub const BLOCK_ADD_ID_EXTRA_DATA: u32 = 0x41ED;
 /// to select this BlockAdditionMapping. Values ≥ 2 (1 is the default plain
 /// BlockAdditional). Used by the MVC (`mvcC`) mapping for Blu-ray 3D.
 pub const BLOCK_ADD_ID_VALUE: u32 = 0x41F0;
+/// MaxBlockAdditionID (RFC 9559 5.1.4.1.16): highest BlockAddID used on the track.
+pub const MAX_BLOCK_ADDITION_ID: u32 = 0x55EE;
 
 // Block additions inside a BlockGroup — per-frame side data. For Blu-ray 3D (MVC),
 // dependent (right-eye) NAL units ride as a BlockAdditional under `mvcC` (RFC 9559
@@ -610,6 +612,9 @@ pub const CHAPTER_UID: u32 = 0x73C4;
 pub const CHAPTER_TIME_START: u32 = 0x91;
 pub const CHAPTER_DISPLAY: u32 = 0x80;
 pub const CHAP_STRING: u32 = 0x85;
+pub const EDITION_FLAG_DEFAULT: u32 = 0xDB;
+pub const CHAPTER_FLAG_HIDDEN: u32 = 0x98;
+pub const CHAPTER_FLAG_ENABLED: u32 = 0x4598;
 pub const CHAP_LANGUAGE: u32 = 0x437C;
 
 // Track types

src/mux/meta.rs

@@ -425,6 +425,9 @@ pub fn read_header(r: &mut impl Read) -> io::Result<Option<M2tsMeta>> {
     let mut meta: M2tsMeta =
         serde_json::from_slice(&json_buf).map_err(|_| crate::error::Error::NoMetadata)?;
     meta.frame_padding = magic[VERSION_BYTE] >= 2;
+    // The wire track is a u8: cap untrusted timings so timing() stays O(256).
+    meta.timings.retain(|t| t.track < 256);
+    meta.timings.truncate(256);
 
     // Skip padding to next 192-byte boundary (at most BD_SOURCE_PACKET_BYTES-1 bytes →
     // a stack buffer, no heap allocation).
@@ -888,4 +891,21 @@ mod tests {
             assert_eq!(back.timing(0), timings.first().copied().unwrap_or_default());
         }
     }
+
+    #[test]
+    fn read_header_caps_untrusted_timings() {
+        let t = video_title(HdrFormat::Sdr, ColorSpace::Bt709);
+        let mut meta = M2tsMeta::from_title(&t);
+        meta.timings = (0..5000)
+            .map(|track| MetaTiming {
+                track,
+                codec_delay_ns: 1,
+                seek_preroll_ns: 1,
+            })
+            .collect();
+        let mut buf = Vec::new();
+        write_header(&mut buf, &meta).unwrap();
+        let back = read_header(&mut io::Cursor::new(&buf)).unwrap().unwrap();
+        assert!(back.timings.len() <= 256);
+    }
 }

src/mux/mkv.rs

@@ -332,8 +332,8 @@ impl MkvTrack {
             // (see the audio counterpart — no error channel exists here either).
             _ => ebml::CODEC_MPEG2,
         };
-        // Unknown resolution -> `pixels()` reports (0, 0) (it no longer fabricates a
-        // 1920x1080 default), and the writer omits the optional PixelWidth/PixelHeight
+        // Unknown resolution -> `pixels()` reports (0, 0) (no default is fabricated),
+        // and the writer omits the optional PixelWidth/PixelHeight
         // on 0 per RFC 9559 5.1.4.1.28-29.
         let (w, h) = v.resolution.pixels().unwrap_or((0, 0));
         let (num, den) = v.frame_rate.as_fraction();
@@ -402,8 +402,8 @@ impl MkvTrack {
     /// maps to the single registered `A_DTS` ID (see the note below).
     pub fn audio(a: &AudioStream) -> Self {
         // `A_DTS` is the sole registered codec ID for the whole DTS family; players
-        // distinguish core/HD-HRA/HD-MA from the bitstream itself. The old `A_DTS/MA`
-        // and `A_DTS/HR` suffixes aren't registered and some strict parsers reject them.
+        // distinguish core/HD-HRA/HD-MA from the bitstream itself. `A_DTS/MA` and
+        // `A_DTS/HR` aren't registered and some strict parsers reject them.
         let codec_id = match a.codec {
             Codec::Ac3 => ebml::CODEC_AC3,
             Codec::Ac3Plus => ebml::CODEC_EAC3,
@@ -421,8 +421,7 @@ impl MkvTrack {
             _ => ebml::CODEC_AC3,
         };
         // Unknown sample rate/channels -> accessors return 0, so the serializer omits
-        // SamplingFrequency/Channels rather than writing a fabricated 48kHz/6ch value
-        // (they used to fabricate that default, forcing every caller to guard it).
+        // SamplingFrequency/Channels rather than writing a fabricated 48kHz/6ch value.
         let sr = a.sample_rate.hz();
         let ch = a.channels.count();
 
@@ -774,6 +773,11 @@ const CLUSTER_DURATION_TICKS: i64 = 2_000 * 1_000_000 / TIMESTAMP_SCALE_NS;
 // Maximum block-relative timestamp in the signed 16-bit SimpleBlock/Block field; a frame
 // outside this forces a new cluster so `as i16` never wraps.
 const MAX_BLOCK_REL: i64 = i16::MAX as i64;
+
+/// Nanoseconds to timestamp ticks, at least 1 so a duration never truncates to 0.
+fn ns_to_ticks(ns: u64) -> u64 {
+    (ns as i64 / TIMESTAMP_SCALE_NS).max(1) as u64
+}
 /// Minimum block-relative timestamp expressible in the signed 16-bit field.
 const MIN_BLOCK_REL: i64 = i16::MIN as i64;
 
@@ -962,6 +966,13 @@ impl<W: Write + Seek> MkvMuxer<W> {
                 }
             }
 
+            if mvc_record.is_some() {
+                ebml::write_uint(
+                    &mut writer,
+                    ebml::MAX_BLOCK_ADDITION_ID,
+                    BLOCK_ADD_ID_VALUE_MVC,
+                )?;
+            }
             ebml::write_uint(&mut writer, ebml::FLAG_LACING, 0)?;
             ebml::write_string(&mut writer, ebml::CODEC_ID, track.codec_id)?;
             ebml::write_string(&mut writer, ebml::LANGUAGE, &track.language)?;
@@ -1314,14 +1325,14 @@ impl<W: Write + Seek> MkvMuxer<W> {
     /// first ~100 coded frames per track to `<output>.opening.bin` and logs a
     /// per-frame summary, so opening-GOP / menu issues are diagnosable from a
     /// log + side file without the disc. `None` is a no-op (normal runs).
-    pub fn set_opening_capture(&mut self, capture: Option<crate::diag::OpeningCapture>) {
+    pub(crate) fn set_opening_capture(&mut self, capture: Option<crate::diag::OpeningCapture>) {
         self.opening_capture = capture;
     }
     /// Drive seam correction from the title's PlayItem marks instead of inferring it from PTS
     /// jumps. Each clip is placed at the sum of the earlier clips' durations, so output runs
     /// exactly as long as the playlist says. No-op for fewer than two clips or without usable
     /// marks — DVD, HD-DVD and file sources keep the inference path.
-    pub fn set_clips(
+    pub(crate) fn set_clips(
         &mut self,
         clips: &[crate::disc::Clip],
         content_format: crate::disc::ContentFormat,
@@ -1410,13 +1421,22 @@ impl<W: Write + Seek> MkvMuxer<W> {
             cap.record(track_idx, pts_ns, keyframe, data);
         }
 
+        // A block for an undeclared TrackNumber would make the file inconsistent.
+        if track_idx >= self.track_uids.len() {
+            return Err(crate::error::Error::MuxTrackRange {
+                track: track_idx,
+                tracks: self.track_uids.len(),
+            }
+            .into());
+        }
+
         // Is this a video track? Used for the monotonic block-timestamp nudge
         // below, which must exempt EVERY video track (incl. a Dolby Vision EL).
         let is_video = self.track_is_video.get(track_idx).copied().unwrap_or(false);
 
         // Clip-boundary epochs are driven by the PRIMARY video track only (not literal
         // index 0 — M2TS/PMT can list audio first): a Dolby Vision EL's overlapping PTS
-        // would false-trigger a reset every GOP otherwise (once inflated a 1-clip title to ~7h).
+        // would false-trigger a reset every GOP, inflating a 1-clip title to ~7h.
         let drives_epoch = Some(track_idx) == self.primary_video_track;
 
         // Map the raw PES PTS onto the continuous timeline FIRST: source PTS jumps
@@ -1507,13 +1527,15 @@ impl<W: Write + Seek> MkvMuxer<W> {
             if !(MIN_BLOCK_REL..=MAX_BLOCK_REL).contains(&rel) {
                 // Block-relative timestamp is signed 16-bit (~±3.27s at 0.1ms scale); a
                 // long GOP/audio stretch or PTS back-jump can wrap it, so force a fresh
-                // cluster with its own Cue entry, or seeking lands in multi-second gaps.
+                // cluster, with a Cue entry if this is a keyframe.
                 self.start_cluster(pts_ticks)?;
-                self.cues.push(CuePoint {
-                    timestamp_ticks: pts_ticks,
-                    track: track_idx + 1,
-                    cluster_pos: self.cluster_pos - self.segment_start,
-                });
+                if keyframe {
+                    self.cues.push(CuePoint {
+                        timestamp_ticks: pts_ticks,
+                        track: track_idx + 1,
+                        cluster_pos: self.cluster_pos - self.segment_start,
+                    });
+                }
             }
         }
 
@@ -1523,13 +1545,11 @@ impl<W: Write + Seek> MkvMuxer<W> {
         // Track the highest block END (start + duration when known), not just start,
         // so a back-patched Segment Duration (for a missing source duration) covers
         // the final frame's full presentation instead of understating the runtime.
-        let block_end_ticks =
-            pts_ticks + duration_ns.map_or(0, |d| (d as i64 / TIMESTAMP_SCALE_NS).max(1));
+        let block_end_ticks = pts_ticks + duration_ns.map_or(0, |d| ns_to_ticks(d) as i64);
         self.max_block_ticks = self.max_block_ticks.max(block_end_ticks);
 
         let relative_ts = (pts_ticks - self.cluster_ts_ticks) as i16;
-        let duration_ticks =
-            duration_ns.map(|dur_ns| (dur_ns as i64 / TIMESTAMP_SCALE_NS).max(1) as u64);
+        let duration_ticks = duration_ns.map(ns_to_ticks);
         // Defense in depth (issue #52): a SUBTITLE block must never be a bare
         // SimpleBlock (no DefaultDuration => unbounded cue, ffmpeg "Timestamps are
         // unset"). Substitute a minimum fallback so it takes the BlockGroup arm.
@@ -1551,7 +1571,7 @@ impl<W: Write + Seek> MkvMuxer<W> {
         let duration_ticks = match duration_ticks {
             Some(dt) => Some(dt),
             None if is_vobsub => match vobsub_display_ns(data).filter(|&ns| ns > 0) {
-                Some(ns) => Some((ns as i64 / TIMESTAMP_SCALE_NS).max(1) as u64),
+                Some(ns) => Some(ns_to_ticks(ns)),
                 None => {
                     vobsub_open = true;
                     Some(VOBSUB_OPEN_END_TICKS)
@@ -1742,7 +1762,7 @@ impl<W: Write + Seek> MkvMuxer<W> {
         Ok(())
     }
 
-    pub fn finish(mut self) -> io::Result<()> {
+    pub(crate) fn finish(mut self) -> io::Result<()> {
         // An MP2 track shorter than the tracker's look settles on its base count now.
         let pending: Vec<(usize, u64, u8, u8)> = self
             .channel_fixups
@@ -1784,7 +1804,7 @@ impl<W: Write + Seek> MkvMuxer<W> {
         let seam_dropped = self.continuity.dropped_total();
         // Counting isn't bounding: if marks don't line up with the PES clock, the plan
         // can discard most of a title, and the only other gate (zero-frame check) is a
-        // skippable stub — a title emitting seconds of a feature exiting 0 shipped once.
+        // skippable stub — a title emitting seconds of a feature must not exit 0.
         if seam_dropped > self.frame_count {
             return Err(crate::error::Error::SeamPlanDroppedMost {
                 dropped: seam_dropped,
@@ -4486,7 +4506,7 @@ mod tests {
         // `mux_to_bytes` muxes with `duration_secs = 0.0` (as an HD-DVD title
         // does), so DURATION is reserved as a placeholder and must be
         // back-patched from the muxed timeline at finish() — not left 0/absent.
-        let tracks = [make_video_track()];
+        let tracks = [make_video_track(), make_audio_track()];
         let frames = frames_for(5.0, 1.0); // ~5 s of 24 fps video
         let (data, _) = mux_to_bytes(&tracks, &[], &frames);
         let dur = find_duration_ticks(&data).expect("DURATION element present");
@@ -4529,7 +4549,7 @@ mod tests {
         // A title that declares its runtime keeps that value in the header even
         // when fewer seconds were muxed; only the Cues reflect the real timeline.
         let shared = Arc::new(Mutex::new(Cursor::new(Vec::new())));
-        let tracks = [make_video_track()];
+        let tracks = [make_video_track(), make_audio_track()];
         let mut muxer =
             MkvMuxer::new(SharedWriter(shared.clone()), &tracks, None, 100.0, &[]).unwrap();
         for (t, pts, kf, d) in frames_for(5.0, 1.0) {
@@ -4567,7 +4587,7 @@ mod tests {
 
     #[test]
     fn seekhead_omits_chapters_when_empty() {
-        let tracks = [make_video_track()];
+        let tracks = [make_video_track(), make_audio_track()];
         let (data, _) = mux_to_bytes(&tracks, &[], &frames_for(5.0, 1.0));
         let entries = parse_seekhead(&data);
         assert_eq!(
@@ -6776,7 +6796,7 @@ mod tests {
     // back-patched too.
     #[test]
     fn seekhead_chapters_entry_resolves_to_the_chapters_element() {
-        let tracks = [make_video_track()];
+        let tracks = [make_video_track(), make_audio_track()];
         let chapters = vec![
             Chapter {
                 time_secs: 0.0,
@@ -7054,4 +7074,66 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn mvc_track_declares_max_block_addition_id() {
+        let mut v = make_video_track();
+        v.mvc_params = Some((
+            vec![0x6F, 0x80, 0x00, 0x33, 0x11, 0x22],
+            vec![0x68, 0xEE, 0x3C],
+        ));
+        let muxer = MkvMuxer::new(Cursor::new(Vec::new()), &[v], None, 0.0, &[]).unwrap();
+        let data = muxer.writer.into_inner();
+        assert!(
+            find_id(&data, ebml::MAX_BLOCK_ADDITION_ID).is_some(),
+            "MVC mapping needs MaxBlockAdditionID"
+        );
+        let muxer = MkvMuxer::new(
+            Cursor::new(Vec::new()),
+            &[make_video_track()],
+            None,
+            0.0,
+            &[],
+        )
+        .unwrap();
+        let data = muxer.writer.into_inner();
+        assert!(find_id(&data, ebml::MAX_BLOCK_ADDITION_ID).is_none());
+    }
+
+    #[test]
+    fn write_frame_rejects_undeclared_track() {
+        let mut muxer = MkvMuxer::new(
+            Cursor::new(Vec::new()),
+            &[make_video_track()],
+            None,
+            0.0,
+            &[],
+        )
+        .unwrap();
+        let err = muxer
+            .write_frame(3, 0, true, &[0x65, 0x01], None, None)
+            .unwrap_err();
+        let inner = err
+            .get_ref()
+            .and_then(|e| e.downcast_ref::<crate::error::Error>());
+        assert!(matches!(
+            inner,
+            Some(crate::error::Error::MuxTrackRange {
+                track: 3,
+                tracks: 1
+            })
+        ));
+    }
+
+    #[test]
+    fn forced_split_on_non_key_video_frame_adds_no_cue() {
+        // One keyframe, then P frames 1 s apart: the i16 guard splits the cluster on
+        // a non-key frame, which must not become a seek point.
+        let frames: Vec<(usize, i64, bool, Vec<u8>)> = (0..10)
+            .map(|i| (0, i * 1_000_000_000, i == 0, vec![0xAB; 16]))
+            .collect();
+        let (data, _) = mux_to_bytes(&[make_video_track()], &[], &frames);
+        assert!(find_clusters(&data).len() >= 2, "split expected");
+        assert_eq!(parse_cues(&data).len(), 1, "only the keyframe is a cue");
+    }
 }

src/mux/mkvstream.rs

@@ -67,9 +67,10 @@ impl MkvTrackKind {
 }
 
 /// Read the EBML header, Segment Info and Tracks of a Matroska file and stop:
-/// no cluster is touched, so this is cheap enough to run over a whole library.
+/// no cluster, Chapters, Attachments or Tags is touched, so this is cheap
+/// enough to run over a whole library.
 pub fn probe_mkv(mut r: impl Read) -> io::Result<MkvProbe> {
-    Ok(parse_mkv_header(&mut r)?.probe)
+    Ok(parse_mkv_header(&mut r, false)?.probe)
 }
 
 /// [`probe_mkv`] plus `last_cue_secs`: follows the SeekHead to the Cues element
@@ -77,7 +78,7 @@ pub fn probe_mkv(mut r: impl Read) -> io::Result<MkvProbe> {
 /// when the file has no Cues.
 pub fn probe_mkv_with_cues(mut r: impl Read + io::Seek) -> io::Result<MkvProbe> {
     r.seek(io::SeekFrom::Start(0))?;
-    let mut probe = parse_mkv_header(&mut r)?.probe;
+    let mut probe = parse_mkv_header(&mut r, false)?.probe;
     r.seek(io::SeekFrom::Start(0))?;
     let last_cue_ticks = last_cue_ticks(&mut r)?;
     probe.last_cue_secs =
@@ -891,7 +892,7 @@ impl MkvStream {
             ts_scale_ns,
             tracks,
             probe: _,
-        } = parse_mkv_header(&mut reader)?;
+        } = parse_mkv_header(&mut reader, true)?;
         let mut stream = Self {
             disc_title,
             mvc: None,
@@ -1228,6 +1229,20 @@ impl MkvStream {
                             _ => skip_bytes(&mut rs.reader, cs)?,
                         }
                     }
+                    if block.is_none() {
+                        // Block is mandatory (RFC 9559); count the unit as lost, never silent.
+                        if rs.additions_dropped == 0 {
+                            tracing::warn!(
+                                target: "mux",
+                                bytes = size,
+                                "mkv read-back: dropping a BlockGroup with no Block; \
+                                 counted in lost_bytes/errors."
+                            );
+                        }
+                        rs.additions_dropped = rs.additions_dropped.saturating_add(1);
+                        rs.additions_dropped_bytes =
+                            rs.additions_dropped_bytes.saturating_add(size);
+                    }
                     if let Some(block) = block {
                         // BLOCK_DURATION is TimestampScale ticks, not ms — scale by
                         // ts_scale_ns (1_000_000 for our own 1ms scale, non-default
@@ -1454,8 +1469,8 @@ impl crate::pes::Stream for MkvStream {
         true // MKV has all headers upfront in the EBML header
     }
 
-    // Count of `BlockAdditions` subtrees dropped on read-back (e.g. a 3D MVC
-    // dependent-view AU the PES frame model can't carry). Reported like a
+    // Count of `BlockAdditions` subtrees (e.g. a 3D MVC dependent-view AU the PES
+    // frame model can't carry) and Block-less BlockGroups dropped on read-back. Reported like a
     // disc-read skip: `0` for write side / sources with no `BlockAdditions`.
     fn errors(&self) -> u64 {
         match self.mode {
@@ -1464,7 +1479,7 @@ impl crate::pes::Stream for MkvStream {
         }
     }
 
-    // Cumulative `BlockAdditions` bytes dropped on read-back; counts the whole
+    // Cumulative bytes of the units counted by `errors()`; counts the whole
     // skipped subtree (payload + EBML framing), an upper bound on the payload.
     fn lost_bytes(&self) -> u64 {
         match self.mode {
@@ -1476,8 +1491,9 @@ impl crate::pes::Stream for MkvStream {
 
 // ── MKV header parsing (read side) ────────────────────────────
 
-// Reads the EBML header, then the Segment's Info + Tracks, stopping at the first Cluster.
-fn parse_mkv_header(r: &mut impl Read) -> io::Result<MkvHeader> {
+// Reads the EBML header, then the Segment's Info + Tracks. With `want_chapters` it runs on to
+// the first Cluster to find Chapters (best effort); without, it stops once Info + Tracks are read.
+fn parse_mkv_header(r: &mut impl Read, want_chapters: bool) -> io::Result<MkvHeader> {
     let mut title = String::new();
     // EBML `DURATION` is a float expressed in TimestampScale ticks, not
     // milliseconds (Matroska spec). Converted to seconds as ticks * ts_scale_ns / 1e9.
@@ -1503,10 +1519,11 @@ fn parse_mkv_header(r: &mut impl Read) -> io::Result<MkvHeader> {
         return Err(crate::error::Error::MkvSourceInvalid.into());
     }
 
+    let mut chapters: Vec<Chapter> = Vec::new();
     let (mut got_info, mut got_tracks) = (false, false);
 
     loop {
-        if got_info && got_tracks {
+        if !want_chapters && got_info && got_tracks {
             break;
         }
         let (id, size, _) = match ebml::read_element_header(r) {
@@ -1593,10 +1610,26 @@ fn parse_mkv_header(r: &mut impl Read) -> io::Result<MkvHeader> {
                 }
                 got_tracks = true;
             }
-            ebml::CLUSTER => break,
-            _ if size != u64::MAX => {
-                skip_bytes(r, size)?;
+            ebml::CHAPTERS if want_chapters && size != u64::MAX => {
+                // Chapters are optional metadata: a bad or truncated element is dropped.
+                match read_chapters_buf(r, size) {
+                    Ok(buf) => match parse_chapters(&buf) {
+                        Ok(c) => chapters = c,
+                        Err(e) => {
+                            tracing::warn!(target: "mux", error = %e, "ignoring malformed MKV Chapters element")
+                        }
+                    },
+                    Err(e) if is_truncation(&e) => break,
+                    Err(e) => return Err(e),
+                }
             }
+            ebml::CLUSTER => break,
+            _ if size != u64::MAX => match skip_bytes(r, size) {
+                Ok(()) => {}
+                // Truncated Attachments/Tags before the first Cluster: stop scanning.
+                Err(e) if want_chapters && is_truncation(&e) => break,
+                Err(e) => return Err(e),
+            },
             _ => break,
         }
     }
@@ -1615,6 +1648,7 @@ fn parse_mkv_header(r: &mut impl Read) -> io::Result<MkvHeader> {
         playlist: title,
         duration_secs: duration_secs.unwrap_or(0.0),
         streams,
+        chapters,
         ..DiscTitle::empty()
     };
     // Clamp the (untrusted) scale to a positive i64 for the tick→ns multiply on
@@ -1633,6 +1667,95 @@ fn parse_mkv_header(r: &mut impl Read) -> io::Result<MkvHeader> {
     })
 }
 
+/// Most chapter marks kept from an untrusted Chapters element.
+const MAX_CHAPTERS: usize = 4096;
+/// Largest Chapters body that is parsed; a bigger one is skipped.
+const MAX_CHAPTERS_BYTES: u64 = 4 * 1024 * 1024;
+
+// A short read / bad element: the data is truncated or malformed, not an I/O failure.
+fn is_truncation(e: &io::Error) -> bool {
+    matches!(
+        e.kind(),
+        io::ErrorKind::UnexpectedEof | io::ErrorKind::InvalidData
+    )
+}
+
+// Read a Chapters body of `size` bytes into memory; over the cap it is skipped (empty).
+fn read_chapters_buf(r: &mut impl Read, size: u64) -> io::Result<Vec<u8>> {
+    if size > MAX_CHAPTERS_BYTES {
+        skip_bytes(r, size)?;
+        return Ok(Vec::new());
+    }
+    let mut buf = Vec::new();
+    r.take(size).read_to_end(&mut buf)?;
+    if buf.len() as u64 != size {
+        return Err(crate::error::Error::MkvSourceInvalid.into());
+    }
+    Ok(buf)
+}
+
+/// Read the default EditionEntry's (else the first) top-level ChapterAtoms
+/// (start time + first ChapString, else a 1-based index) from a Chapters
+/// body, skipping hidden and disabled atoms.
+fn parse_chapters(body: &[u8]) -> io::Result<Vec<Chapter>> {
+    let mut editions: Vec<(bool, Vec<Chapter>)> = Vec::new();
+    for_each_child(&mut &body[..], body.len() as u64, |r, cid, cs| {
+        if cid != ebml::EDITION_ENTRY {
+            return skip_bytes(r, cs);
+        }
+        let (mut is_default, mut out) = (false, Vec::<Chapter>::new());
+        for_each_child(r, cs, |r, cid, cs| match cid {
+            ebml::EDITION_FLAG_DEFAULT => {
+                is_default = read_uint_bounded(r, cs)? != 0;
+                Ok(())
+            }
+            ebml::CHAPTER_ATOM if out.len() < MAX_CHAPTERS => {
+                let (mut start_ns, mut name) = (0u64, None);
+                let (mut hidden, mut enabled) = (false, true);
+                for_each_child(r, cs, |r, cid, cs| match cid {
+                    ebml::CHAPTER_TIME_START => {
+                        start_ns = read_uint_bounded(r, cs)?;
+                        Ok(())
+                    }
+                    ebml::CHAPTER_FLAG_HIDDEN => {
+                        hidden = read_uint_bounded(r, cs)? != 0;
+                        Ok(())
+                    }
+                    ebml::CHAPTER_FLAG_ENABLED => {
+                        enabled = read_uint_bounded(r, cs)? != 0;
+                        Ok(())
+                    }
+                    ebml::CHAPTER_DISPLAY => for_each_child(r, cs, |r, cid, cs| {
+                        if cid == ebml::CHAP_STRING && name.is_none() {
+                            name = Some(read_string_bounded(r, cs)?);
+                            Ok(())
+                        } else {
+                            skip_bytes(r, cs)
+                        }
+                    }),
+                    _ => skip_bytes(r, cs),
+                })?;
+                if !hidden && enabled {
+                    out.push(Chapter {
+                        time_secs: start_ns as f64 / 1_000_000_000.0,
+                        name: name.unwrap_or_else(|| (out.len() + 1).to_string()),
+                    });
+                }
+                Ok(())
+            }
+            _ => skip_bytes(r, cs),
+        })?;
+        editions.push((is_default, out));
+        Ok(())
+    })?;
+    let pick = editions.iter().position(|e| e.0).unwrap_or(0);
+    Ok(editions
+        .into_iter()
+        .nth(pick)
+        .map(|e| e.1)
+        .unwrap_or_default())
+}
+
 /// Largest valid 13-bit MPEG-TS PID.
 const MAX_TS_PID: u32 = 0x1FFF;
 
@@ -1919,6 +2042,10 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result<ParsedTrack> {
         Codec::Vc1
     } else if cid == ebml::CODEC_MPEG2 {
         Codec::Mpeg2
+    } else if cid == ebml::CODEC_MPEG1 {
+        Codec::Mpeg1
+    } else if cid == ebml::CODEC_AV1 {
+        Codec::Av1
     } else if cid == ebml::CODEC_AC3 {
         Codec::Ac3
     } else if cid == ebml::CODEC_EAC3 {
@@ -1959,9 +2086,8 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result<ParsedTrack> {
     } else if sr >= 48000.0 {
         SampleRate::S48
     } else {
-        // Below the lowest mapped rate is UNKNOWN, not 48kHz: the ladder's final
-        // `else` used to be S48, so a legal 32kHz AC-3/DTS track was misrecorded as
-        // 48kHz. This float ladder exists only for tolerance vs. `SampleRate::from_hz`.
+        // Below the lowest mapped rate is UNKNOWN, not 48kHz (a legal 32kHz
+        // AC-3/DTS track must not be recorded as 48kHz). This float ladder exists only for tolerance vs. `SampleRate::from_hz`.
         SampleRate::Unknown
     };
 
@@ -6389,4 +6515,186 @@ mod tests {
         );
         assert_eq!(parse_freemkv_version("mkvmerge v80.0 ('x') 64-bit"), None);
     }
+
+    // Info + Tracks (one entry with `codec_id`) + optional Chapters + one Cluster
+    // holding `cluster_body`, as raw EBML.
+    fn synthetic_mkv(codec_id: &str, chapters: &[u8], cluster_body: &[u8]) -> Vec<u8> {
+        let mut entry = Vec::new();
+        ebml::write_uint(&mut entry, ebml::TRACK_NUMBER, 1).unwrap();
+        ebml::write_uint(&mut entry, ebml::TRACK_TYPE, 1).unwrap();
+        ebml::write_string(&mut entry, ebml::CODEC_ID, codec_id).unwrap();
+        let mut tracks = Vec::new();
+        ebml::write_id(&mut tracks, ebml::TRACK_ENTRY).unwrap();
+        ebml::write_size(&mut tracks, entry.len() as u64).unwrap();
+        tracks.extend_from_slice(&entry);
+        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();
+        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(chapters);
+        ebml::write_id(&mut out, ebml::CLUSTER).unwrap();
+        ebml::write_unknown_size(&mut out).unwrap();
+        out.extend_from_slice(cluster_body);
+        out
+    }
+
+    #[test]
+    fn read_back_keeps_chapters() {
+        let mut atoms = Vec::new();
+        for (t, name) in [(0u64, "1"), (5_000_000_000, "Two")] {
+            let mut body = Vec::new();
+            ebml::write_uint(&mut body, ebml::CHAPTER_TIME_START, t).unwrap();
+            let mut disp = Vec::new();
+            ebml::write_string(&mut disp, ebml::CHAP_STRING, name).unwrap();
+            ebml::write_id(&mut body, ebml::CHAPTER_DISPLAY).unwrap();
+            ebml::write_size(&mut body, disp.len() as u64).unwrap();
+            body.extend_from_slice(&disp);
+            ebml::write_id(&mut atoms, ebml::CHAPTER_ATOM).unwrap();
+            ebml::write_size(&mut atoms, body.len() as u64).unwrap();
+            atoms.extend_from_slice(&body);
+        }
+        let mut edition = Vec::new();
+        ebml::write_id(&mut edition, ebml::EDITION_ENTRY).unwrap();
+        ebml::write_size(&mut edition, atoms.len() as u64).unwrap();
+        edition.extend_from_slice(&atoms);
+        let mut chapters = Vec::new();
+        ebml::write_id(&mut chapters, ebml::CHAPTERS).unwrap();
+        ebml::write_size(&mut chapters, edition.len() as u64).unwrap();
+        chapters.extend_from_slice(&edition);
+
+        let stream =
+            MkvStream::open(Cursor::new(synthetic_mkv("V_MPEG2", &chapters, &[]))).unwrap();
+        let got: Vec<(f64, String)> = crate::pes::Stream::info(&stream)
+            .chapters
+            .iter()
+            .map(|c| (c.time_secs, c.name.clone()))
+            .collect();
+        assert_eq!(got, vec![(0.0, "1".to_string()), (5.0, "Two".to_string())]);
+    }
+
+    fn el(id: u32, body: &[u8]) -> Vec<u8> {
+        let mut v = Vec::new();
+        ebml::write_id(&mut v, id).unwrap();
+        ebml::write_size(&mut v, body.len() as u64).unwrap();
+        v.extend_from_slice(body);
+        v
+    }
+
+    fn atom(t: u64, name: &str, flags: &[(u32, u64)]) -> Vec<u8> {
+        let mut b = Vec::new();
+        ebml::write_uint(&mut b, ebml::CHAPTER_TIME_START, t).unwrap();
+        for &(id, v) in flags {
+            ebml::write_uint(&mut b, id, v).unwrap();
+        }
+        let mut d = Vec::new();
+        ebml::write_string(&mut d, ebml::CHAP_STRING, name).unwrap();
+        b.extend(el(ebml::CHAPTER_DISPLAY, &d));
+        el(ebml::CHAPTER_ATOM, &b)
+    }
+
+    fn chapter_names(chapters: &[u8]) -> Vec<String> {
+        let s = MkvStream::open(Cursor::new(synthetic_mkv("V_MPEG2", chapters, &[]))).unwrap();
+        crate::pes::Stream::info(&s)
+            .chapters
+            .iter()
+            .map(|c| c.name.clone())
+            .collect()
+    }
+
+    #[test]
+    fn chapters_use_default_edition_and_skip_hidden_disabled() {
+        let mut e1 = atom(0, "first-ed", &[]);
+        e1.extend(atom(1, "x", &[]));
+        let mut e2 = Vec::new();
+        ebml::write_uint(&mut e2, ebml::EDITION_FLAG_DEFAULT, 1).unwrap();
+        e2.extend(atom(0, "keep", &[]));
+        e2.extend(atom(1, "hid", &[(ebml::CHAPTER_FLAG_HIDDEN, 1)]));
+        e2.extend(atom(2, "off", &[(ebml::CHAPTER_FLAG_ENABLED, 0)]));
+        e2.extend(atom(3, "keep2", &[(ebml::CHAPTER_FLAG_ENABLED, 1)]));
+        let mut body = el(ebml::EDITION_ENTRY, &e1);
+        body.extend(el(ebml::EDITION_ENTRY, &e2));
+        let ch = el(ebml::CHAPTERS, &body);
+        assert_eq!(chapter_names(&ch), vec!["keep", "keep2"]);
+    }
+
+    #[test]
+    fn malformed_chapters_still_open_and_mux() {
+        let mut bad_name = Vec::new();
+        ebml::write_id(&mut bad_name, ebml::CHAP_STRING).unwrap();
+        ebml::write_size(&mut bad_name, 2).unwrap();
+        bad_name.extend_from_slice(&[0xFF, 0xFE]);
+        let mut a = Vec::new();
+        ebml::write_uint(&mut a, ebml::CHAPTER_TIME_START, 0).unwrap();
+        a.extend(el(ebml::CHAPTER_DISPLAY, &bad_name));
+        let ch = el(
+            ebml::CHAPTERS,
+            &el(ebml::EDITION_ENTRY, &el(ebml::CHAPTER_ATOM, &a)),
+        );
+        // Oversized ChapString (> 64 KiB) inside otherwise well-formed elements.
+        let mut big = Vec::new();
+        ebml::write_id(&mut big, ebml::CHAP_STRING).unwrap();
+        ebml::write_size(&mut big, MAX_STRING_LEN + 1).unwrap();
+        big.resize(big.len() + MAX_STRING_LEN as usize + 1, b'a');
+        let mut a2 = Vec::new();
+        a2.extend(el(ebml::CHAPTER_DISPLAY, &big));
+        let ch2 = el(
+            ebml::CHAPTERS,
+            &el(ebml::EDITION_ENTRY, &el(ebml::CHAPTER_ATOM, &a2)),
+        );
+        for c in [ch, ch2] {
+            let bytes = synthetic_mkv("V_MPEG2", &c, &[]);
+            assert!(chapter_names(&c).is_empty());
+            assert!(probe_mkv(Cursor::new(bytes.clone())).is_ok());
+            let mut s = MkvStream::open(Cursor::new(bytes)).unwrap();
+            assert!(crate::pes::Stream::read(&mut s).unwrap().is_none());
+        }
+    }
+
+    #[test]
+    fn truncated_tags_before_cluster_still_opens() {
+        let mut tags = Vec::new();
+        ebml::write_id(&mut tags, ebml::TAGS).unwrap();
+        ebml::write_size(&mut tags, 1000).unwrap();
+        tags.extend_from_slice(&[0u8; 10]);
+        // Tags claims 1000 bytes but the file ends: no Cluster follows.
+        let mut bytes = synthetic_mkv("V_MPEG2", &[], &[]);
+        let cut = bytes.len() - 12; // drop the Cluster header (ID 4 + unknown size 8)
+        bytes.truncate(cut);
+        bytes.extend_from_slice(&tags);
+        assert!(probe_mkv(Cursor::new(bytes.clone())).is_ok());
+        let s = MkvStream::open(Cursor::new(bytes)).unwrap();
+        assert_eq!(crate::pes::Stream::info(&s).streams.len(), 1);
+    }
+
+    #[test]
+    fn read_back_maps_mpeg1_and_av1_codec_ids() {
+        for (id, want) in [("V_MPEG1", Codec::Mpeg1), ("V_AV1", Codec::Av1)] {
+            let stream = MkvStream::open(Cursor::new(synthetic_mkv(id, &[], &[]))).unwrap();
+            match &crate::pes::Stream::info(&stream).streams[0] {
+                crate::disc::Stream::Video(v) => assert_eq!(v.codec, want, "{id}"),
+                _ => panic!("expected a video stream"),
+            }
+        }
+    }
+
+    #[test]
+    fn blockless_block_group_is_counted_not_silent() {
+        let mut cluster = Vec::new();
+        ebml::write_id(&mut cluster, ebml::BLOCK_GROUP).unwrap();
+        let mut body = Vec::new();
+        ebml::write_uint(&mut body, ebml::BLOCK_DURATION, 40).unwrap();
+        ebml::write_size(&mut cluster, body.len() as u64).unwrap();
+        cluster.extend_from_slice(&body);
+        let mut stream =
+            MkvStream::open(Cursor::new(synthetic_mkv("V_MPEG2", &[], &cluster))).unwrap();
+        assert!(crate::pes::Stream::read(&mut stream).unwrap().is_none());
+        assert_eq!(crate::pes::Stream::errors(&stream), 1);
+        assert!(crate::pes::Stream::lost_bytes(&stream) > 0);
+    }
 }

src/mux/mp4/audio.rs

@@ -125,7 +125,7 @@ fn parse_ac3(f: &[u8]) -> Option<DolbyConfig> {
         lfeon,
         bit_rate_code: frmsizecod >> 1,
         data_rate_kbps: 0,
-        sample_rate: FSCOD_RATES.get(fscod as usize).copied().unwrap_or(48_000),
+        sample_rate: *FSCOD_RATES.get(fscod as usize)?,
         channels: DolbyConfig::channel_count(acmod, lfeon),
     })
 }
@@ -145,6 +145,9 @@ fn parse_eac3(f: &[u8]) -> Option<DolbyConfig> {
 
     let (sample_rate, blocks) = if fscod == 0x03 {
         let fscod2 = (f[4] >> 4) & 0x03; // shares bits with numblkscod when fscod==3
+        if fscod2 == 0x03 {
+            return None; // reserved
+        }
         (EAC3_REDUCED_RATES[fscod2 as usize], 6u32)
     } else {
         let blocks = [1u32, 2, 3, 6][numblkscod as usize];
@@ -244,10 +247,11 @@ pub(super) fn audio_sample_entry(
 
 // ── DTS (dtsc/dtsh + ddts) ───────────────────────────────────────────────────
 
-/// DTS core `SFREQ` (4-bit) → sample rate (Hz). Reserved indices → 48 kHz.
+/// DTS core `SFREQ` (4-bit) → sample rate (Hz); 0 marks an invalid code
+/// (0, 4, 5, 9, 10). Codes 14 and 15 map to 96 kHz / 192 kHz.
 const DTS_SFREQ: [u32; 16] = [
-    48_000, 8_000, 16_000, 32_000, 48_000, 48_000, 11_025, 22_050, 44_100, 48_000, 48_000, 12_000,
-    24_000, 48_000, 96_000, 192_000,
+    0, 8_000, 16_000, 32_000, 0, 0, 11_025, 22_050, 44_100, 0, 0, 12_000, 24_000, 48_000, 96_000,
+    192_000,
 ];
 // DTS core base channel count per AMODE (ETSI TS 102 114 §5.3.1); matches the decodability
 // gate's DTS_AMODE_COUNT in dts.rs.
@@ -288,6 +292,9 @@ fn parse_dts(frame: &[u8]) -> Option<DtsConfig> {
     let lfe = lff == 1 || lff == 2;
 
     let sample_rate = DTS_SFREQ[sfreq];
+    if sample_rate == 0 {
+        return None;
+    }
     // AMODE is a 6-bit field, so 16..=63 are reachable but RESERVED (ETSI TS 102 114) —
     // no channel count or speaker mask is known for them. Refuse rather than guess: the
     // old `unwrap_or(6)` invented a count the speaker mask could not match.
@@ -419,8 +426,8 @@ fn dts_entry_rate(max_rate: u32) -> u32 {
     }
 }
 
-/// The MP4 fourcc + config box for an audio frame, or `None` if the codec has no
-/// MP4 mapping here. Together with [`audio_fits`] this is the fit oracle for
+/// The complete MP4 AudioSampleEntry (Dolby and DTS) for an audio frame, or `None`
+/// if the codec has no MP4 mapping here. Together with [`audio_fits`] this is the fit oracle for
 /// audio: only what returns `Some` is muxable.
 /// `stream_hz` is the title's rate for the track (0 if unknown).
 pub(super) fn dolby_sample_entry(
@@ -747,7 +754,7 @@ mod tests {
         // fsize=8 → core_size=9; the EXSS sync sits exactly at byte 9 (right after
         // the core) and MUST be detected → dtsh. Guards the off-by-4 boundary.
         let f = vec![
-            0x7F, 0xFE, 0x80, 0x01, 0x00, 0x00, 0x00, 0x80, 0x00, 0x64, 0x58, 0x20, 0x25,
+            0x7F, 0xFE, 0x80, 0x01, 0x00, 0x00, 0x00, 0x80, 0x34, 0x64, 0x58, 0x20, 0x25,
         ];
         let c = parse_dts(&f).expect("parses");
         assert_eq!(c.core_size, 9);
@@ -817,7 +824,7 @@ mod tests {
         // extension substream is still ONE asset, so deriving the flag from "EXSS
         // follows the core" told a parser to select a nonexistent second asset — 0 is honest.
         let f = vec![
-            0x7F, 0xFE, 0x80, 0x01, 0x00, 0x00, 0x00, 0x80, 0x00, 0x64, 0x58, 0x20, 0x25,
+            0x7F, 0xFE, 0x80, 0x01, 0x00, 0x00, 0x00, 0x80, 0x34, 0x64, 0x58, 0x20, 0x25,
         ];
         let c = parse_dts(&f).expect("parses");
         assert!(c.has_extension, "fixture must exercise the extension path");
@@ -1264,4 +1271,20 @@ mod tests {
         // high 16 bits = the integer rate.
         assert_eq!(&e[32..34], &[0xFF, 0xFF], "capped to 65535, not wrapped");
     }
+
+    #[test]
+    fn reserved_sample_rate_codes_are_refused() {
+        let mut f = eac3_frame_5_1();
+        f[4] = 0b1111_1111; // fscod=3, fscod2=3 (reserved)
+        assert!(parse_dolby(&f).is_none());
+        let mut f = ac3_frame_5_1();
+        f[4] = 0b11_010110; // fscod=3 (reserved)
+        assert!(parse_dolby(&f).is_none());
+        let mut f = vec![
+            0x7F, 0xFE, 0x80, 0x01, 0x00, 0x3C, 0x05, 0xF2, 0x77, 0x00, 0x02, 0x00,
+        ];
+        assert!(parse_dts(&f).is_some());
+        f[8] = 0x43; // SFREQ 0 is invalid
+        assert!(parse_dts(&f).is_none());
+    }
 }