Merge pull request #18 from storytold/pr/never-crash-codecs

codecs: never crash (damaged MP4/TS headers; no expect/unreachable)

crates/codecs/src/gop.rs

@@ -240,8 +240,7 @@ impl GopCache {
         }
         // evict frames far from the most recent (keep a window around the working position)
         while st.bytes > budget && st.frames.len() > 2 {
-            let first = *st.frames.keys().next().expect("non-empty");
-            let last = *st.frames.keys().next_back().expect("non-empty");
+            let (Some(&first), Some(&last)) = (st.frames.keys().next(), st.frames.keys().next_back()) else { break };
             let victim = if pts - first > last - pts { first } else { last };
             if let Some(v) = st.frames.remove(&victim) {
                 st.drafts.remove(&victim);
@@ -323,7 +322,7 @@ impl GopCache {
             // every sample): step back to a sample the decoder can start from.
             while key > 0 {
                 let data = s.read(key)?;
-                if st.decoder.as_ref().expect("decoder").is_random_access(&data) != Some(false) {
+                if st.decoder.as_ref().and_then(|d| d.is_random_access(&data)) != Some(false) {
                     break;
                 }
                 key = s.sync_before(key - 1);
@@ -337,7 +336,10 @@ impl GopCache {
         }
         let limit = (i.max(st.next) + 64).min(n);
         let late = late_before.unwrap_or(i64::MIN).min(want_pts);
-        st.decoder.as_mut().expect("decoder").set_draft(draft);
+        let Some(decoder) = st.decoder.as_mut() else {
+            return Err(CodecError::Decode("no video decoder".into()));
+        };
+        decoder.set_draft(draft);
         while st.next < limit {
             if filmcraft_media::cancel::cancelled() {
                 // The decoder state stays consistent (`next`, `out_max`): a later request continues.
@@ -345,12 +347,15 @@ impl GopCache {
             }
             let k = st.next;
             let data = s.read(k)?;
-            if k != i && s.pts(k) < late && st.decoder.as_ref().expect("decoder").is_disposable(&data) {
+            if k != i && s.pts(k) < late && st.decoder.as_ref().is_some_and(|d| d.is_disposable(&data)) {
                 st.next += 1;
                 SKIPPED.fetch_add(1, Ordering::Relaxed);
                 continue;
             }
-            let out = timed(|| st.decoder.as_mut().expect("decoder").decode(&data, s.pts(k)))?;
+            let Some(decoder) = st.decoder.as_mut() else {
+                return Err(CodecError::Decode("no video decoder".into()));
+            };
+            let out = timed(|| decoder.decode(&data, s.pts(k)))?;
             st.next += 1;
             DECODED.fetch_add(1, Ordering::Relaxed);
             self.store_output(&mut st, out);
@@ -359,7 +364,7 @@ impl GopCache {
             }
         }
         if st.cached(want_pts, draft).is_none() {
-            let out = timed(|| st.decoder.as_mut().expect("decoder").flush());
+            let out = st.decoder.as_mut().map(|d| timed(|| d.flush())).unwrap_or_default();
             self.store_output(&mut st, out);
             st.next = usize::MAX;
         }

crates/codecs/src/lib.rs

@@ -16,6 +16,8 @@
 //! - [`AudioFileSource`]: standalone compressed audio files (MP3, FLAC, AIFF, …).
 //! - [`openers`]: the openers to register with the engine's media pool.
 
+#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
+
 pub mod audio;
 pub mod gop;
 pub mod mkv;

crates/codecs/src/mkv.rs

@@ -369,9 +369,9 @@ impl MkvSource {
         if let Some(p) = st.packets.get(&i) {
             return Ok(p.clone());
         }
-        let ti = self.atrack.expect("audio");
+        let ti = self.atrack.ok_or_else(|| CodecError::Unsupported("no audio track".into()))?;
         let track = &self.file.tracks[ti];
-        let ainfo = self.info.audio.as_ref().expect("audio info");
+        let ainfo = self.info.audio.as_ref().ok_or_else(|| CodecError::Unsupported("no audio track".into()))?;
         let data = self.read(ti, i)?;
         let decoded = match &track.codec {
             Codec::Pcm { float, big_endian, bits } => {
@@ -392,7 +392,7 @@ impl MkvSource {
                 // Non-sequential access: reset and prime with the preceding packets (codec pre-roll:
                 // one packet, or `SeekPreRoll` worth for Opus).
                 if st.last_decoded.is_none_or(|l| l + 1 != i) {
-                    let d = st.decoder.as_mut().expect("decoder");
+                    let d = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?;
                     d.reset();
                     let from = if self.audio_preroll > 0 {
                         self.audio_starts.partition_point(|&x| x <= self.audio_starts[i] - self.audio_preroll).saturating_sub(1)
@@ -405,7 +405,7 @@ impl MkvSource {
                         }
                     }
                 }
-                let r = st.decoder.as_mut().expect("decoder").decode(&data, self.audio_starts[i].max(0) as u64);
+                let r = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?.decode(&data, self.audio_starts[i].max(0) as u64);
                 st.last_decoded = Some(i);
                 r.unwrap_or_default()
             }

crates/codecs/src/mp4.rs

@@ -107,8 +107,11 @@ impl Mp4Source {
     pub fn open_reader(name: &str, reader: filmcraft_media::SharedReader) -> crate::Result<Self> {
         let bytes = crate::Src(reader);
         let file = filmcraft_isobmff::open(&bytes).map_err(|e| CodecError::Container(e.to_string()))?;
-        let vtrack = file.tracks.iter().position(|t| t.kind == TrackKind::Video && !t.samples.is_empty());
-        let atrack = file.tracks.iter().position(|t| t.kind == TrackKind::Audio && !t.samples.is_empty());
+        // A track needs samples, a sample description and a timescale to be playable (damaged
+        // files can lack them: indexing `entries[0]` or dividing by the timescale used to panic).
+        let playable = |t: &&filmcraft_isobmff::Track, kind| t.kind == kind && !t.samples.is_empty() && !t.entries.is_empty() && t.timescale > 0;
+        let vtrack = file.tracks.iter().position(|t| playable(&t, TrackKind::Video));
+        let atrack = file.tracks.iter().position(|t| playable(&t, TrackKind::Audio));
         if vtrack.is_none() && atrack.is_none() {
             return Err(CodecError::Unsupported("no playable tracks".into()));
         }
@@ -126,6 +129,8 @@ impl Mp4Source {
             durs.sort_unstable();
             let d = durs.get(durs.len() / 2).copied().unwrap_or(1).max(1);
             let rate = FrameRate::from_f64(t.timescale as f64 / d as f64);
+            // A rate that rounds to zero (one sample per huge duration) can't be divided by.
+            let rate = if rate.num > 0 && rate.den > 0 { rate } else { FrameRate::default() };
             color = color_from(entry, w, h);
             if entry
                 .video
@@ -264,9 +269,9 @@ impl Mp4Source {
         if let Some(p) = st.packets.get(&i) {
             return Ok(p.clone());
         }
-        let ti = self.atrack.expect("audio");
+        let ti = self.atrack.ok_or_else(|| CodecError::Unsupported("no audio track".into()))?;
         let track = &self.file.tracks[ti];
-        let entry = &track.entries[0];
+        let entry = track.entries.first().ok_or_else(|| CodecError::Container("audio track without a sample description".into()))?;
         let data = self.read(ti, i)?;
         let decoded = match &entry.codec {
             CodecConfig::Pcm(p) => decode_pcm(&data, p),
@@ -277,7 +282,7 @@ impl Mp4Source {
                 // Non-sequential access: reset and prime with the preceding packets (codec pre-roll:
                 // one packet, or `OPUS_PRE_ROLL` worth for Opus).
                 if st.last_decoded.is_none_or(|l| l + 1 != i) {
-                    let d = st.decoder.as_mut().expect("decoder");
+                    let d = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?;
                     d.reset();
                     let from = if self.audio_preroll > 0 {
                         self.audio_starts.partition_point(|&x| x <= self.audio_starts[i] - self.audio_preroll).saturating_sub(1)
@@ -290,7 +295,7 @@ impl Mp4Source {
                         }
                     }
                 }
-                let r = st.decoder.as_mut().expect("decoder").decode(&data, self.audio_starts[i].max(0) as u64);
+                let r = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?.decode(&data, self.audio_starts[i].max(0) as u64);
                 st.last_decoded = Some(i);
                 match r {
                     Ok(v) => v,
@@ -513,6 +518,33 @@ mod tests {
         b.into()
     }
 
+    /// Overwrite the big-endian u32 `skip` bytes after the first `fourcc` box type.
+    fn patch_u32(b: &mut [u8], fourcc: &[u8; 4], skip: usize, v: u32) {
+        let k = b.windows(4).position(|x| x == fourcc).expect("box") + 4 + skip;
+        b[k..k + 4].copy_from_slice(&v.to_be_bytes());
+    }
+
+    /// Damaged MOVs found by mutation fuzzing panicked on import: a track with timescale 0
+    /// ("attempt to divide by zero" in the duration) and one whose sample description list is
+    /// empty ("index out of bounds" on `entries[0]`). Both must open as an error, or as media
+    /// that answers requests without panicking.
+    #[test]
+    fn damaged_track_headers_never_panic() {
+        let identity = [0x10000, 0, 0, 0, 0x10000, 0, 0, 0, 0x4000_0000];
+        // mdhd v0: version/flags, creation, modification, then timescale
+        let mut zero_timescale = rotated_mov(identity).to_vec();
+        patch_u32(&mut zero_timescale, b"mdhd", 12, 0);
+        // stsd: version/flags, then entry_count
+        let mut no_entries = rotated_mov(identity).to_vec();
+        patch_u32(&mut no_entries, b"stsd", 4, 0);
+        for (name, b) in [("ts0.mov", zero_timescale), ("stsd0.mov", no_entries)] {
+            if let Ok(s) = Mp4Source::open(name, b.into()) {
+                let _ = s.video_frame(FrameRequest::full(Tick::ZERO));
+                let _ = s.audio(0, 1024, 48_000);
+            }
+        }
+    }
+
     /// Mean luma of the top and bottom halves of the decoded frame, and its size.
     fn halves(src: &Mp4Source) -> (u32, u32, u8, u8) {
         let f = src.video_frame(FrameRequest::full(Tick::ZERO)).expect("frame");

crates/codecs/src/mpeg.rs

@@ -123,6 +123,19 @@ fn audio_supported(c: &Codec) -> bool {
     matches!(c, Codec::MpegAudio | Codec::AacAdts | Codec::AacLatm | Codec::LpcmBluray | Codec::LpcmDvd | Codec::Ac3)
 }
 
+/// The coded rate unless the timestamps say otherwise (3:2 pulldown, field-rate coding). Never
+/// zero: damaged timestamps spread over a huge span give a rate that rounds to 0 fps, which the
+/// duration computation divided by.
+fn stream_rate(coded: Option<FrameRate>, from_pts: Option<f64>) -> FrameRate {
+    let rate = match (coded, from_pts) {
+        (Some(r), Some(p)) if (r.as_f64() - p).abs() / p > 0.01 => FrameRate::from_f64(p),
+        (Some(r), _) => r,
+        (None, Some(p)) => FrameRate::from_f64(p),
+        (None, None) => FrameRate::FPS_25,
+    };
+    if rate.num > 0 && rate.den > 0 { rate } else { FrameRate::FPS_25 }
+}
+
 /// Display-order keys of samples (decode order): the PTS when every unit has one, else (MPEG
 /// video) GOP start + temporal_reference, else the decode order.
 fn display_keys(units: &[&Unit], mpeg: bool) -> Vec<i64> {
@@ -242,8 +255,9 @@ impl MpegSource {
                     src.file = Some(file);
                     src.video = Some(track);
                     src.describe_video(rate_from_pts)?;
-                    let file = src.file.take().expect("file");
-                    src.setup_audio(file, astream, origin);
+                    if let Some(file) = src.file.take() {
+                        src.setup_audio(file, astream, origin);
+                    }
                 }
                 None => {
                     src.unsupported_video = Some(format!("{} video in {} (FilmCraft has no decoder for it)", st.codec.name(), file.format.name()));
@@ -369,14 +383,14 @@ impl MpegSource {
         if let Some((data, aus)) = &self.es {
             return Ok(data[aus[u].clone()].to_vec());
         }
-        let file = self.file.as_ref().expect("file");
+        let file = self.file.as_ref().ok_or_else(|| CodecError::Container("no stream file".into()))?;
         file.read_unit(&self.bytes, v.stream, u).map_err(|e| CodecError::Container(e.to_string()))
     }
 
     /// Fill in the video stream info from the first picture.
     fn describe_video(&mut self, rate_from_pts: Option<f64>) -> crate::Result<()> {
         let mut seq_header = None;
-        let v = self.video.as_ref().expect("video");
+        let v = self.video.as_ref().ok_or_else(|| CodecError::Unsupported("no video stream".into()))?;
         let first = self.read_unit(v, 0)?;
         let n = v.order.len();
         let (width, height, par, codec, pixel_format, mut color);
@@ -432,12 +446,7 @@ impl MpegSource {
             }
         }
         // the coded rate unless the timestamps say otherwise (3:2 pulldown, field-rate coding)
-        let rate = match (rate, rate_from_pts) {
-            (Some(r), Some(p)) if (r.as_f64() - p).abs() / p > 0.01 => FrameRate::from_f64(p),
-            (Some(r), _) => r,
-            (None, Some(p)) => FrameRate::from_f64(p),
-            (None, None) => FrameRate::FPS_25,
-        };
+        let rate = stream_rate(rate, rate_from_pts);
         let duration = rate.tick_of(n as i64);
         let total_bytes: u64 = match (&self.es, &self.file) {
             (Some((_, aus)), _) => v.units.iter().map(|&u| aus[u].len() as u64).sum(),
@@ -445,8 +454,10 @@ impl MpegSource {
             _ => 0,
         };
         let secs = duration.seconds();
-        if let Some(h) = seq_header {
-            self.video.as_mut().expect("video").header = h;
+        if let Some(h) = seq_header
+            && let Some(v) = self.video.as_mut()
+        {
+            v.header = h;
         }
         self.info.duration = duration;
         self.info.video = Some(VideoStreamInfo {
@@ -525,7 +536,7 @@ impl MpegSource {
                     return self.audio_unsupported(&codec_name, "DVD LPCM: no format header".into());
                 }
             },
-            _ => unreachable!("audio_supported"),
+            _ => return self.audio_unsupported(&codec_name, format!("{codec_name} audio")),
         };
         if codec_is_ac3_unsupported(codec) {
             return self.audio_unsupported(&codec_name, format!("{codec_name} audio (FilmCraft has no AC-3 decoder yet)"));
@@ -608,12 +619,12 @@ impl MpegSource {
         if let Some(p) = st.cache.get(&i) {
             return Ok(p.clone());
         }
-        let file = self.file.as_ref().expect("file");
+        let file = self.file.as_ref().ok_or_else(|| CodecError::Container("no stream file".into()))?;
         let read = |k: usize| file.read_unit(&self.bytes, a.stream, k).map_err(|e| CodecError::Container(e.to_string()));
         if st.dec.is_none() {
             st.dec = Some(AudioDecoder::new(a.codec, a.rate, a.channels, a.latm.clone())?);
         }
-        let dec = st.dec.as_mut().expect("decoder");
+        let dec = st.dec.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?;
         if st.last.is_none_or(|l| l + 1 != i) {
             dec.reset();
             if i > 0
@@ -840,6 +851,21 @@ pub fn reader_opener(name: &str, head: &[u8], reader: &filmcraft_media::SharedRe
 mod tests {
     use super::*;
 
+    /// A mutated transport stream whose PTS span gave ~0 fps panicked on open with "attempt to
+    /// divide by zero" (`FrameRate::tick_of`).
+    #[test]
+    fn damaged_timestamps_never_give_a_zero_rate() {
+        for (coded, p) in
+            [(None, Some(1e-9)), (Some(FrameRate::FPS_25), Some(1e-9)), (Some(FrameRate::new(0, 0)), None), (None, Some(0.0)), (None, Some(f64::NAN))]
+        {
+            let r = stream_rate(coded, p);
+            assert!(r.num > 0 && r.den > 0, "{coded:?} {p:?} -> {r:?}");
+            let _ = r.tick_of(1000);
+        }
+        assert_eq!(stream_rate(Some(FrameRate::FPS_25), Some(25.0)), FrameRate::FPS_25);
+        assert_eq!(stream_rate(None, Some(50.0)), FrameRate::from_f64(50.0));
+    }
+
     #[test]
     fn presentation_drops_leading_pictures_and_orders_by_key() {
         // decode order I(2) B(0) B(1) P(5) B(3) B(4): the leading B pictures are not presented

crates/codecs/src/mxf.rs

@@ -333,7 +333,7 @@ impl MxfSource {
             let mut channels = 0;
             for &ai in &atracks {
                 let t = &file.tracks[ai];
-                let si = t.sound.as_ref().expect("sound info");
+                let Some(si) = t.sound.as_ref() else { continue };
                 if si.sample_rate != rate {
                     continue;
                 }
@@ -424,7 +424,7 @@ impl VideoSamples for MxfVideo<'_> {
             return Err(CodecError::Unsupported(why.clone()));
         }
         let t = &self.src.file.tracks[self.track];
-        let v = self.src.info.video.as_ref().expect("video info");
+        let v = self.src.info.video.as_ref().ok_or_else(|| CodecError::Unsupported("no video track".into()))?;
         let (w, h) = (v.width.min(u16::MAX as u32) as u16, v.height.min(u16::MAX as u32) as u16);
         match t.codec {
             Codec::Avc { .. } => Ok(Box::new(crate::video::H264Decoder::annexb())),

crates/codecs/src/ogg.rs

@@ -133,7 +133,7 @@ impl OggSource {
         }
         // Non-sequential access: reset and decode the pre-roll (RFC 7845 §4.6; see OPUS_PRE_ROLL).
         if st.last_decoded.is_none_or(|l| l + 1 != i) {
-            let d = st.decoder.as_mut().expect("decoder");
+            let d = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?;
             d.reset();
             let from = timing.starts.partition_point(|&x| x <= timing.starts[i] - OPUS_PRE_ROLL as i64).saturating_sub(1);
             for j in from..i {
@@ -143,7 +143,7 @@ impl OggSource {
             }
         }
         let data = read(i)?;
-        let r = st.decoder.as_mut().expect("decoder").decode(&data, 0);
+        let r = st.decoder.as_mut().ok_or_else(|| CodecError::Decode("no audio decoder".into()))?.decode(&data, 0);
         st.last_decoded = Some(i);
         let p = Arc::new(r.unwrap_or_default());
         st.packets.insert(i, p.clone());

crates/codecs/src/video.rs

@@ -386,7 +386,8 @@ impl Vp9Decoder {
         }
     }
 
-    fn convert(&self, p: filmcraft_vp9::Picture) -> DecodedFrame {
+    /// `None` for a picture whose planes mix sample types (never produced by the decoder).
+    fn convert(&self, p: filmcraft_vp9::Picture) -> Option<DecodedFrame> {
         use filmcraft_color::{Matrix, Primaries, Range};
         use filmcraft_frame::{Chroma, PixelData};
         use filmcraft_vp9::Plane;
@@ -423,7 +424,7 @@ impl Vp9Decoder {
             }
             let mut frame = VideoFrame::rgba8(p.width, p.height, rgba);
             frame.color = filmcraft_color::ColorInfo { range: Range::Full, transfer: filmcraft_color::Transfer::Srgb, ..color };
-            return DecodedFrame { pts, frame, draft };
+            return Some(DecodedFrame { pts, frame, draft });
         }
         // 4:4:0 has no frame format of its own: chroma rows are repeated to 4:4:4.
         let (chroma, rows_440) = match (p.subsampling_x, p.subsampling_y) {
@@ -455,22 +456,22 @@ impl Vp9Decoder {
                 bits: p.bit_depth,
                 alpha: None,
             },
-            _ => unreachable!("VP9 planes share one sample type"),
+            _ => return None,
         };
         // render_size (the intended display size) is not applied: the container's display
         // dimensions / pixel aspect describe the presentation.
         let par = (1, 1);
-        DecodedFrame { pts, frame: VideoFrame { width: p.width, height: p.height, data, color, par, pts: filmcraft_time::Tick::ZERO }, draft }
+        Some(DecodedFrame { pts, frame: VideoFrame { width: p.width, height: p.height, data, color, par, pts: filmcraft_time::Tick::ZERO }, draft })
     }
 }
 
 impl VideoDecoder for Vp9Decoder {
     fn decode(&mut self, sample: &[u8], pts: i64) -> Result<Vec<DecodedFrame>> {
         let pics = self.dec.decode(sample, pts).map_err(|e| CodecError::Decode(e.to_string()))?;
-        Ok(pics.into_iter().map(|p| self.convert(p)).collect())
+        Ok(pics.into_iter().filter_map(|p| self.convert(p)).collect())
     }
     fn flush(&mut self) -> Vec<DecodedFrame> {
-        self.dec.flush().into_iter().map(|p| self.convert(p)).collect()
+        self.dec.flush().into_iter().filter_map(|p| self.convert(p)).collect()
     }
     fn reset(&mut self) {
         self.dec.reset();