Bug 1448762 - P2. Ignore sctc invalid entries. r?kinetik draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 28 Mar 2018 19:36:27 +0200
changeset 773887 4575edb472218bd00da4fbfe6ce7735ebb6630c7
parent 773886 a7b0ede9394e3c959084610b608c0819b83d00d6
push id104345
push userbmo:jyavenard@mozilla.com
push dateWed, 28 Mar 2018 17:36:48 +0000
reviewerskinetik
bugs1448762
milestone61.0a1
Bug 1448762 - P2. Ignore sctc invalid entries. r?kinetik Some file have a valid stco (Chunk Offset Box) while having an invalid stsc (Sample To Chunk Box). Those entries in stsc all have the same first_chunk value. stagefright would skip over all those entries and ultimately would end up using the stco only which in this case would be sufficient to properly retrieve the sample table. We implement the same behaviour in the rust parser. While searching on the problem, I found that a particular encoding tool used to create those files which was corrected about 3 years ago. Seeing that Chrome, quicktime and other players play it, as well as firefox prior 58, we allow to play this particular invalid file. MozReview-Commit-ID: 4MvVfnpKAqh
media/mp4parse-rust/mp4parse_capi/src/lib.rs
--- a/media/mp4parse-rust/mp4parse_capi/src/lib.rs
+++ b/media/mp4parse-rust/mp4parse_capi/src/lib.rs
@@ -821,43 +821,58 @@ struct SampleToChunkIterator<'a> {
 }
 
 impl<'a> Iterator for SampleToChunkIterator<'a> {
     type Item = (u32, u32);
 
     fn next(&mut self) -> Option<(u32, u32)> {
         let has_chunk = self.chunks.next()
             .or_else(|| {
-                self.chunks = match (self.stsc_peek_iter.next(), self.stsc_peek_iter.peek()) {
-                    (Some(next), Some(peek)) if next.first_chunk > 0 && peek.first_chunk > 0 => {
-                        self.sample_count = next.samples_per_chunk;
-                        ((next.first_chunk - 1) .. (peek.first_chunk - 1))
-                    },
-                    (Some(next), None) if next.first_chunk > 0 => {
-                        self.sample_count = next.samples_per_chunk;
-                        // Total chunk number in 'stsc' could be different to 'stco',
-                        // there could be more chunks at the last 'stsc' record.
-                        match next.first_chunk.checked_add(self.remain_chunk_count) {
-                            Some(r) => ((next.first_chunk - 1) .. r - 1),
-                            _ => (0 .. 0),
-                        }
-                    },
-                    _ => (0 .. 0),
-                };
-
+                self.chunks = self.locate();
                 self.remain_chunk_count.checked_sub(self.chunks.len() as u32).and_then(|res| {
                     self.remain_chunk_count = res;
                     self.chunks.next()
                 })
             });
 
         has_chunk.map_or(None, |id| { Some((id, self.sample_count)) })
     }
 }
 
+impl<'a> SampleToChunkIterator<'a> {
+    fn locate(&mut self) -> std::ops::Range<u32> {
+        loop {
+            let r = match (self.stsc_peek_iter.next(), self.stsc_peek_iter.peek()) {
+                (Some(next), Some(peek)) if next.first_chunk == peek.first_chunk => {
+                    // Invalid entry, skip it and will continue searching at
+                    // next loop iteration.
+                    None
+                },
+                (Some(next), Some(peek)) if next.first_chunk > 0 && peek.first_chunk > 0 => {
+                    self.sample_count = next.samples_per_chunk;
+                    Some((next.first_chunk - 1) .. (peek.first_chunk - 1))
+                },
+                (Some(next), None) if next.first_chunk > 0 => {
+                    self.sample_count = next.samples_per_chunk;
+                    // Total chunk number in 'stsc' could be different to 'stco',
+                    // there could be more chunks at the last 'stsc' record.
+                    match next.first_chunk.checked_add(self.remain_chunk_count) {
+                        Some(r) => Some((next.first_chunk - 1) .. r - 1),
+                        _ => Some(0 .. 0),
+                    }
+                },
+                _ => Some(0 .. 0)
+            };
+            if r.is_some() {
+                return r.unwrap();
+            }
+        };
+    }
+}
+
 fn create_sample_table(track: &Track, track_offset_time: i64) -> Option<Vec<Mp4parseIndice>> {
     let timescale = match track.timescale {
         Some(ref t) => TrackTimeScale::<i64>(t.0 as i64, t.1),
         _ => TrackTimeScale::<i64>(0, 0),
     };
 
     let (stsc, stco, stsz, stts) =
         match (&track.stsc, &track.stco, &track.stsz, &track.stts) {