Ignore stco chunks with no samples to fill them

mp4parse/src/lib.rs

@@ -1044,6 +1044,9 @@ pub struct SampleToChunk {
 #[derive(Debug)]
 pub struct SampleSizeBox {
     pub sample_size: u32,
+    /// The number of samples in the track. When `sample_size` is zero, this
+    /// is also the length of `sample_sizes`.
+    pub sample_count: u32,
     pub sample_sizes: TryVec<u32>,
 }
 
@@ -4984,6 +4987,7 @@ fn read_stsz<T: Read>(src: &mut BMFFBox<T>) -> Result<SampleSizeBox> {
 
     Ok(SampleSizeBox {
         sample_size,
+        sample_count,
         sample_sizes,
     })
 }

mp4parse/src/unstable.rs

@@ -150,24 +150,40 @@ pub fn create_sample_table(
 
     let mut sample_size_iter = stsz.sample_sizes.iter();
 
+    // 'stsz' is authoritative for the number of samples in the track. Some
+    // files declare more chunks in 'stco' than the samples described by
+    // 'stsz' can fill, leaving trailing chunks that no sample maps into.
+    // Those chunks are unreachable, so stop once every sample is accounted
+    // for rather than rejecting the track.
+    let stsz_sample_count = stsz.sample_count.to_usize();
+
     // Get 'stsc' iterator for (chunk_id, chunk_sample_count) and calculate the sample
     // offset address.
 
     // With large numbers of samples, the cost of many allocations dominates,
     // so it's worth iterating twice to allocate sample_table just once.
     let total_sample_count = sample_to_chunk_iter(&stsc.samples, &stco.offsets)
         .map(|(_, sample_counts)| sample_counts.to_usize())
-        .try_fold(0usize, usize::checked_add)?;
+        .try_fold(0usize, usize::checked_add)?
+        .min(stsz_sample_count);
     let mut sample_table = TryVec::with_capacity(total_sample_count).ok()?;
 
-    for i in sample_to_chunk_iter(&stsc.samples, &stco.offsets) {
+    'chunks: for i in sample_to_chunk_iter(&stsc.samples, &stco.offsets) {
         let chunk_id = i.0 as usize;
         let sample_counts = i.1;
         let mut cur_position = match stco.offsets.get(chunk_id) {
             Some(&i) => i.into(),
             _ => return None,
         };
         for _ in 0..sample_counts {
+            if sample_table.len() >= stsz_sample_count {
+                debug!(
+                    "track {}: 'stco' declares more chunks than the {} samples \
+                     in 'stsz' can fill, ignoring the surplus",
+                    track.id, stsz_sample_count
+                );
+                break 'chunks;
+            }
             let start_offset = cur_position;
             let end_offset = match (stsz.sample_size, sample_size_iter.next()) {
                 (_, Some(t)) => (start_offset + *t)?,

mp4parse_capi/tests/test_chunk_out_of_range.rs

@@ -10,6 +10,16 @@ extern "C" fn buf_read(buf: *mut u8, size: usize, userdata: *mut std::os::raw::c
     }
 }
 
+/// This file's only defect is a single flipped bit in 'stsc': first_chunk is
+/// 16777217 (0x0100_0001) rather than 1, so it names a chunk 'stco' doesn't
+/// have. Unlike a surplus 'stco' entry that no sample maps into (see
+/// parse_sample_table_with_surplus_stco_entry), there's no sound way to place
+/// the samples, so this stays an error. ffmpeg rejects it the same way, in
+/// sanity_checks(): stsc_data[stsc_count - 1].first > chunk_count gives
+/// "contradictionary STSC and STCO" and fails the whole header.
+///
+/// Note the assertion below is also a regression test against indexing 'stco'
+/// out of bounds, which used to panic before it was changed to use get().
 #[test]
 fn parse_out_of_chunk_range() {
     let mut file = std::fs::File::open("tests/chunk_out_of_range.mp4").expect("Unknown file");

mp4parse_capi/tests/test_sample_table.rs

@@ -327,3 +327,63 @@ fn parse_sample_table_with_negative_ctts() {
         mp4parse_free(parser);
     }
 }
+
+/// A file whose 'stco' declares more chunks than the samples in 'stsz' can
+/// fill leaves trailing chunks that no sample maps into. The surplus chunk
+/// offset is bogus (past the end of the file), but since nothing references
+/// it the track is still decodable, so parse it rather than rejecting it.
+/// See https://bugzilla.mozilla.org/show_bug.cgi?id=2026607
+#[test]
+fn parse_sample_table_with_surplus_stco_entry() {
+    let mut file = std::fs::File::open("tests/stco_extra_chunk.mp4").expect("Unknown file");
+    let io = Mp4parseIo {
+        read: Some(buf_read),
+        userdata: &mut file as *mut _ as *mut std::os::raw::c_void,
+    };
+
+    unsafe {
+        let mut parser = std::ptr::null_mut();
+        let rv = mp4parse_new(&io, &mut parser);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+        assert!(!parser.is_null());
+
+        let mut track_info = Mp4parseTrackInfo::default();
+        let rv = mp4parse_get_track_info(parser, 0, &mut track_info);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+        assert_eq!(track_info.track_type, Mp4parseTrackType::Video);
+
+        let mut indice = Mp4parseByteData::default();
+        let rv = mp4parse_get_indice_table(parser, track_info.track_id, &mut indice);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+
+        // 'stsc' maps 3 samples per chunk and 'stco' declares 2 chunks, but
+        // 'stsz' only describes 3 samples, so only the first chunk is used.
+        // The table matches the one from the well formed video_colr_nclx_hdr10.mp4
+        // this file was derived from.
+        assert_eq!(indice.length, 3);
+        assert_eq!(
+            *indice.indices.offset(0),
+            Indice {
+                start_offset: 48.into(),
+                end_offset: 757.into(),
+                start_composition: 0.into(),
+                end_composition: 512.into(),
+                start_decode: 0.into(),
+                sync: true,
+            }
+        );
+        assert_eq!(
+            *indice.indices.offset(2),
+            Indice {
+                start_offset: 769.into(),
+                end_offset: 781.into(),
+                start_composition: 512.into(),
+                end_composition: 1024.into(),
+                start_decode: 1024.into(),
+                sync: false,
+            }
+        );
+
+        mp4parse_free(parser);
+    }
+}