diff --git a/mp4parse/src/lib.rs b/mp4parse/src/lib.rs index 7cb41924..7116e6af 100644 --- a/mp4parse/src/lib.rs +++ b/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, } @@ -4984,6 +4987,7 @@ fn read_stsz(src: &mut BMFFBox) -> Result { Ok(SampleSizeBox { sample_size, + sample_count, sample_sizes, }) } diff --git a/mp4parse/src/unstable.rs b/mp4parse/src/unstable.rs index a1b12a5d..032acf27 100644 --- a/mp4parse/src/unstable.rs +++ b/mp4parse/src/unstable.rs @@ -150,6 +150,13 @@ 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. @@ -157,10 +164,11 @@ pub fn create_sample_table( // 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) { @@ -168,6 +176,14 @@ pub fn create_sample_table( _ => 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)?, diff --git a/mp4parse_capi/tests/stco_extra_chunk.mp4 b/mp4parse_capi/tests/stco_extra_chunk.mp4 new file mode 100644 index 00000000..5b8ccacc Binary files /dev/null and b/mp4parse_capi/tests/stco_extra_chunk.mp4 differ diff --git a/mp4parse_capi/tests/test_chunk_out_of_range.rs b/mp4parse_capi/tests/test_chunk_out_of_range.rs index 26c2f506..00a126f6 100644 --- a/mp4parse_capi/tests/test_chunk_out_of_range.rs +++ b/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"); diff --git a/mp4parse_capi/tests/test_sample_table.rs b/mp4parse_capi/tests/test_sample_table.rs index 0558ee0b..49037a3d 100644 --- a/mp4parse_capi/tests/test_sample_table.rs +++ b/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); + } +}