From 19786a826c55e0efbb111437315225712eb54eed Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Thu, 27 Aug 2026 08:58:30 +1200 Subject: [PATCH 1/2] mp4parse: ignore stco chunks with no samples to fill them Some files declare more chunks in 'stco' than the samples in 'stsz' can fill, and the surplus chunk offsets are bogus. Treat 'stsz' as authoritative for the sample count and stop once every sample is placed, rather than rejecting the track. Matches ffmpeg's mov_build_index(). Fixes bug 2026607. --- mp4parse/src/lib.rs | 4 ++ mp4parse/src/unstable.rs | 20 +++++++- mp4parse_capi/tests/stco_extra_chunk.mp4 | Bin 0 -> 1670 bytes mp4parse_capi/tests/test_sample_table.rs | 60 +++++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 mp4parse_capi/tests/stco_extra_chunk.mp4 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 0000000000000000000000000000000000000000..5b8ccacc7779e2735edd13a2e4efe9a3f6dd24c3 GIT binary patch literal 1670 zcmZuy&1)n@6t78kgDwj$7-7K(<+_50ok@339Fsu9#<-Y+f*1F&2u*c$^>j1+QK_mV zGka24B!7ViK@mJD;=zk2*9hw(cozd+Tu~H7Z?YG6{Jox;%@};7>iynly{h+m!WiSJ z8|mEEC1VSWI94F`rC;hbpE34GYLtTQ=Q0rv{L`hgI^dUygS^f9J~g{P>wa z{~89pHQwg3Hj4Xzrt5p_yz47}Bi(>urvr!8S6{pKa(j($UEhXhq9hF4wH_5Jbv*FA zZaeUTE|i&bdUJL4;NYOMpC_s=MAfNHf0a(@WUegWQ)`#kmEGi0#6pHXH!2OgJW+8` z%R%UGdYc{>l_*A5h2F5c=?#6q%S#nzYRK(g4DFBFg9^ zA~K@ttI+RpnVGs25kh_Lj4F!U0<90%6X^itZW($gK_oZpN`--c#rJtCtc$cARvgYzN$B1`(8~sOy2qAVeoRTjUb8%oHB4Oml|JOW~-FymHDE0#4A!#hwwP zNYA~fA1j^bQcIwUu$xz~Zp`Lz;=6a1Ju zyv4fX2Y>%@`)qIdo3Ci7x8A)}y!hpVBf_osw?^m2civ)baa7jzK6FL7pC#aPwD>E8 zA!{-EJE8vwP@j^oo_PKrTEGYOA>L&G;~Q?!oVhsXILkJi(@o&aGBcc#uaX|wo5k2= zl{gD&MHLnwaYGb=yl190FFaW$xqyzB$?3ZXP%p99?=*4kEGbNbu+ir^!^E%C=ei@R zq)_D3x{HRT0L%MjlXSk!%gMyHl3*$*7)18-d#2zM+UR)EAa1movz>Uy?_^aOg|+PW3yizkHI(KwcSeCx$%!|r4Rneaddci zbmP|HnGY}>jN=)$?!ENcQHw2QzyI{@rK5lD->3M;WnGv`7DM(ZyMP6{poP`oR2{8l z4EFTtZ{K5#7Q1L&(-y$S2d$@`v1Qzr+C?;re@>qG(uhEh)-`--%A42~#7a_tX0cO$ z#$mfTF*Q4#_chS@=|VlSp4usycf{Cx;PGDK1o(|oH5`yVJwFB*9V4`+PCcY{KAE*= ZKXNrCeG30SC Date: Thu, 27 Aug 2026 08:58:30 +1200 Subject: [PATCH 2/2] Document why chunk_out_of_range.mp4 stays an error --- mp4parse_capi/tests/test_chunk_out_of_range.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) 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");