address review: a frame too large to analyse is a distinct error for scene detection and auto-trace

crates/engine/src/analysis/tracking.rs

@@ -199,9 +199,27 @@ pub(crate) struct Work {
 /// Most pixels a source frame is brought to for analysis (2 GiB as float RGBA).
 const MAX_SOURCE_PIXELS: u64 = 1 << 27;
 
-/// The layer's source frame at comp time `t` at 100 % (layer pixels + offset = image pixels).
-/// Footage at its native size is used straight from the footage source (no copy unless its
-/// Cineon Settings convert it).
+/// Why a layer's source frame could not be had.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) enum FrameError {
+    /// The layer has no pixels at that time.
+    Missing,
+    /// The frame at 100 % would be `width` x `height`, more than analysis holds ([`MAX_SOURCE_PIXELS`]).
+    TooLarge { width: u32, height: u32 },
+}
+
+impl std::fmt::Display for FrameError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            FrameError::Missing => f.write_str("the layer has no pixels"),
+            FrameError::TooLarge { width, height } => {
+                write!(f, "the layer's frame is {width} x {height} pixels at 100 %, more than the {MAX_SOURCE_PIXELS} pixels analysis works on")
+            }
+        }
+    }
+}
+
+/// [`source_frame_checked`] without the reason: `None` when there is no frame to use.
 pub(crate) fn source_frame(
     r: &Renderer,
     project: &Project,
@@ -211,6 +229,22 @@ pub(crate) fn source_frame(
     t: Tick,
     expr: Option<&dyn ExprHost>,
 ) -> Option<(Arc<Image>, [f64; 2])> {
+    source_frame_checked(r, project, cid, comp, layer, t, expr).ok()
+}
+
+/// The layer's source frame at comp time `t` at 100 % (layer pixels + offset = image pixels).
+/// Footage at its native size is used straight from the footage source (no copy unless its
+/// Cineon Settings convert it). A frame too large to bring to 100 % is [`FrameError::TooLarge`],
+/// not an absent frame.
+pub(crate) fn source_frame_checked(
+    r: &Renderer,
+    project: &Project,
+    cid: ItemId,
+    comp: &Comp,
+    layer: &Layer,
+    t: Tick,
+    expr: Option<&dyn ExprHost>,
+) -> Result<(Arc<Image>, [f64; 2]), FrameError> {
     let ctx = EvalCtx { project, comp_id: cid, comp, time: t, expr, footage: None, property_overrides: &[], media_overrides: &[] };
     if let effectcraft_project::LayerSource::Footage { item } = &layer.source
         && let Some(effectcraft_project::ItemKind::Footage(f)) = project.item(*item).map(|i| &i.kind)
@@ -219,9 +253,9 @@ pub(crate) fn source_frame(
         && img.width == f.width
         && img.height == f.height
     {
-        return Some((effectcraft_render::cineon_frame(img, f), [0.0; 2]));
+        return Ok((effectcraft_render::cineon_frame(img, f), [0.0; 2]));
     }
-    let buf = r.layer_source(&ctx, layer)?;
+    let buf = r.layer_source(&ctx, layer).ok_or(FrameError::Missing)?;
     if (buf.scale - 1.0).abs() > 1e-9 && buf.scale > 0.0 {
         // Footage decoded at another size: bring it to layer pixels.
         let w = (buf.img.width as f64 / buf.scale).round().max(1.0) as u32;
@@ -229,12 +263,12 @@ pub(crate) fn source_frame(
         // The scale comes from the footage's saved width alone: a decoded frame of another
         // aspect would give a height far beyond anything the layer shows.
         if u64::from(w) * u64::from(h) > MAX_SOURCE_PIXELS {
-            return None;
+            return Err(FrameError::TooLarge { width: w, height: h });
         }
         let off = [buf.offset[0] / buf.scale, buf.offset[1] / buf.scale];
-        return Some((Arc::new(effectcraft_raster::resample(&buf.img, w, h)), off));
+        return Ok((Arc::new(effectcraft_raster::resample(&buf.img, w, h)), off));
     }
-    Some((Arc::new(buf.img), buf.offset))
+    Ok((Arc::new(buf.img), buf.offset))
 }
 
 pub(crate) async fn run_work(w: Work, shared: &TrackShared) {
@@ -636,3 +670,17 @@ mod tests {
         }
     }
 }
+
+#[cfg(test)]
+mod frame_error_tests {
+    use super::FrameError;
+
+    #[test]
+    fn a_frame_too_large_to_analyse_is_told_apart_from_a_missing_one() {
+        let big = FrameError::TooLarge { width: 160_000, height: 160_000 };
+        assert_ne!(big, FrameError::Missing);
+        let text = big.to_string();
+        assert!(text.contains("160000 x 160000") && text.contains("analysis"), "{text}");
+        assert_eq!(FrameError::Missing.to_string(), "the layer has no pixels");
+    }
+}

crates/engine/src/cmd/autotrace.rs

@@ -112,13 +112,15 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
         let r = &r;
         let (project, expr) = (&s.project, s.expr.as_deref());
         use rayon::prelude::*;
-        times
+        let traced: std::result::Result<Vec<Vec<Traced>>, String> = times
             .par_iter()
-            .map(|t| match crate::tracking::source_frame(r, project, cid, &comp, &layer, *t, expr) {
-                Some((img, off)) => trace_frame(&img, off, ch, invert, blur, &o),
-                None => vec![],
+            .map(|t| match crate::tracking::source_frame_checked(r, project, cid, &comp, &layer, *t, expr) {
+                Ok((img, off)) => Ok(trace_frame(&img, off, ch, invert, blur, &o)),
+                Err(crate::tracking::FrameError::Missing) => Ok(vec![]),
+                Err(e) => Err(format!("{e} (at {:.3} s)", t.seconds())),
             })
-            .collect()
+            .collect();
+        traced.map_err(|e| bad(c, e))?
     };
     let n_masks = frames.iter().map(Vec::len).max().unwrap_or(0);
     if n_masks == 0 {

crates/engine/src/cmd/scene_detect.rs

@@ -31,14 +31,17 @@ impl LayerFrames {
         LayerFrames { project: s.project.clone(), footage: s.footage.clone(), expr: s.expr.clone(), cache: s.layer_cache.clone(), comp, layer }
     }
     /// Run `f` with a frame getter: comp time → (layer source pixels, offset in layer space).
-    pub fn with<R>(&self, f: impl FnOnce(&(dyn Fn(Tick) -> Option<(Arc<effectcraft_raster::Image>, [f64; 2])> + Sync)) -> R) -> Option<R> {
+    pub fn with<R>(
+        &self,
+        f: impl FnOnce(&(dyn Fn(Tick) -> std::result::Result<(Arc<effectcraft_raster::Image>, [f64; 2]), crate::tracking::FrameError> + Sync)) -> R,
+    ) -> Option<R> {
         let comp = self.project.comp(self.comp)?;
         let layer = comp.layer(self.layer)?;
         let mut r = Renderer::new(&self.project, self.footage.as_ref(), RenderOpts::default());
         r.expr = self.expr.as_deref();
         r.cache = Some(&self.cache);
         let r = &r;
-        let get = |t: Tick| crate::tracking::source_frame(r, &self.project, self.comp, comp, layer, t, self.expr.as_deref());
+        let get = |t: Tick| crate::tracking::source_frame_checked(r, &self.project, self.comp, comp, layer, t, self.expr.as_deref());
         Some(f(&get))
     }
 }
@@ -62,8 +65,17 @@ pub(crate) fn analyse(
         // Signatures in batches (parallel), with progress and cancel between batches.
         for (k, chunk) in times.chunks(8).enumerate() {
             use rayon::prelude::*;
-            let batch: Vec<cuts::Signature> = chunk.par_iter().map(|t| get(*t).map(|(img, _)| cuts::signature(&img)).unwrap_or_default()).collect();
-            sigs.extend(batch);
+            // No pixels at a time is an empty signature (as before); a frame too large to analyse
+            // fails the detection rather than reading as "no cut".
+            let batch: std::result::Result<Vec<cuts::Signature>, String> = chunk
+                .par_iter()
+                .map(|t| match get(*t) {
+                    Ok((img, _)) => Ok(cuts::signature(&img)),
+                    Err(crate::tracking::FrameError::Missing) => Ok(cuts::Signature::default()),
+                    Err(e) => Err(format!("{e} (at {:.3} s)", t.seconds())),
+                })
+                .collect();
+            sigs.extend(batch?);
             if !progress(((k + 1) * 8).min(times.len()) as u64, times.len() as u64) {
                 return Err(crate::render_queue::CANCELLED.to_string());
             }