Ignore stco chunks with no samples to fill them

[kinetiknz]

Fixes bug 2026607, where videos on etoland.co.kr fail in Firefox but play in Chrome and Safari.

The file

The reported MP4 has one video track with an inconsistent sample table:

box	contents
stsz	sample_size=0, sample_count=112, sizes summing to 529972
stsc	one entry: first_chunk=1, samples_per_chunk=112
stco	entry_count=2: 0x90E (the mdat payload start) and 0xC000009D (3GB past the 532290-byte file)

The 112 sample sizes exactly fill the mdat, so the second chunk offset is garbage that no sample maps into.

The failure

mp4parse_new succeeds; mp4parse_get_indice_table returns Invalid. SampleToChunkIterator extends the last stsc entry across every chunk stco declares, yielding 2 × 112 = 224 samples. At sample 113 the stsz size iterator is exhausted, end_offset falls through to 0, and create_sample_table returns None, rejecting the whole track. Gecko surfaces that as NS_ERROR_DOM_MEDIA_METADATA_ERR.

The fix

Treat stsz as authoritative for the sample count and stop once every sample is placed. This is what ffmpeg's mov_build_index() does — it iterates chunks driven by stco just like we do, but bounds the inner loop by the stsz sample count and keeps the entries already built:

for (j = 0; j < sc->stsc_data[stsc_index].count; j++) {
    if (current_sample >= sc->sample_count) {
        av_log(mov->fc, AV_LOG_ERROR, "wrong sample count\n");
        return;                     /* stop, but keep the index built so far */
    }

sc->sample_count is assigned in mov_read_stsz before the if (sample_size) return 0; early-out, so it bounds both the size-table and constant-sample-size cases. SampleSizeBox now retains sample_count for the same reason: it was previously read and discarded when sample_size != 0.

This also fixes a quieter variant. With a constant sample size there is no size table to run out of, so the surplus chunk produced a phantom sample at the bogus offset instead of an error — confirmed on a synthetic file, where the old code emitted a second indice ending at 3221226371.

ffmpeg comparison

nb_read_frames for ffmpeg, indice-table result for mp4parse:

case	ffmpeg	before	after
A · the reported file	112 + wrong sample count	Invalid	112 ✅
B · synthetic surplus stco	3 + wrong sample count	Invalid	3 ✅
B0 · B's unmodified parent	3	3	3
C · const-size surplus stco	1 + wrong sample count	2 (phantom @3221226371)	1 ✅
C0 · C's unmodified parent	1	1	1
D · chunk_out_of_range.mp4	fatal	Invalid ✅	Invalid ✅
D0 · D with its flipped bit repaired	17	17	17
E · too few chunks (mirror case)	2	2	2
F · stsc mid-table overshoot	fatal	Invalid ✅	Invalid ✅
G · zero-sample track	opens, empty track	Invalid	Ok, 0 samples ✅

We diverged from ffmpeg in four rows before; we now match in all ten.

What stays strict

Tables inconsistent in the other direction — stsc naming a chunk stco doesn't have — remain an error. ffmpeg draws the same line in sanity_checks(), and deliberately distinguishes the two failures:

ret = sanity_checks(c->fc, sc, st->index);
if (ret)
    return ret > 1 ? AVERROR_INVALIDDATA : 0;

stsc_data[stsc_count - 1].first > chunk_count returns 2 and fails the whole header (cases D and F); a chunk-bearing track with no samples returns 1 and is kept as an empty track (case G).

The second commit documents this on test_chunk_out_of_range.rs. That test arrived in 7563263 as a regression test against indexing stco out of bounds, which used to panic before it was changed to use get() — the Invalid assertion was just what not panicking happened to produce, so it was worth recording that the strictness is now deliberate. That file's only defect is a single flipped bit: first_chunk is 16777217 (0x0100_0001) rather than 1. Repairing that bit makes it decode 17 frames everywhere (row D0).

Behaviour change beyond the reported case

Row G: a track with stsz sample_count=0 but a non-empty stco now yields an empty sample table instead of Invalid. This moves us toward ffmpeg, and Gecko already receives empty indice tables from fragmented init segments, so the shape is not new.

Testing

New fixture stco_extra_chunk.mp4 (1670 bytes), derived from the existing video_colr_nclx_hdr10.mp4 by adding one bogus stco entry — it has the same single-chunk stsc/stsz shape as the reported file. It returns Invalid without the fix and the same 3-sample table as its unmodified parent with it.

211 tests pass; cargo fmt --check and cargo clippy --all-targets clean.

🤖 Generated with Claude Code

[kinetiknz]

@alastor0325 Would you mind reviewing this please?