fix(video): treat decoded frames as unpremultiplied

crates/rustmotion-components/src/gif.rs

@@ -455,9 +455,9 @@ mod tests {
     /// own video is, so passing generous `max_w`/`max_h` here isolates the
     /// byte-budget check from the video-dimensions check exercised by the
     /// next test. 144 MiB is deliberately far short of the 65535×65535
-    /// (~17 GiB) header a real crafted file could declare — large enough to
-    /// prove the budget check fires, small enough that running this test
-    /// never risks the allocation it is asserting never happens.
+    /// (~17 GiB) header the audit's own crafted file could declare — large
+    /// enough to prove the budget check fires, small enough that running
+    /// this test never risks the allocation it is asserting never happens.
     #[test]
     fn a_canvas_over_the_byte_budget_is_rejected_without_allocating_it() {
         let path = std::env::temp_dir().join(format!(

crates/rustmotion-components/src/video.rs

@@ -95,10 +95,10 @@ fn draw_fitted(canvas: &Canvas, img: skia_safe::Image, fit: &ImageFit, layout: &
 /// The source clip's own duration, probed via `ffprobe` and memoized per
 /// `src` for the life of the process — `effective_source_time` below is
 /// called once per painted frame, and re-probing on every one of them would
-/// mean one subprocess spawn per frame for any looping video. `None` on a
-/// probe failure (no ffprobe on `PATH`, or the source can't be read) is
-/// memoized too, so a broken source fails fast on every subsequent frame
-/// instead of retrying the same failing probe.
+/// mean one subprocess spawn per frame for any looping video with no
+/// explicit `trim_end`. `None` on a probe failure (no ffprobe on `PATH`, or
+/// the source can't be read) is memoized too, so a broken source fails fast
+/// on every subsequent frame instead of retrying the same failing probe.
 fn video_duration_secs(src: &str) -> Option<f64> {
     static CACHE: std::sync::OnceLock<
         std::sync::Mutex<std::collections::HashMap<String, Option<f64>>>,
@@ -122,15 +122,29 @@ fn video_duration_secs(src: &str) -> Option<f64> {
 
 impl Video {
     /// The timestamp to sample from the source clip for a given scene time.
-    /// When `loop_video` is set, playback wraps within the source's own
-    /// probed duration instead of running past it and holding on
-    /// whatever the last extractable frame happens to be.
+    ///
+    /// Honours `trim_end` on the picture the same way the audio track
+    /// already does: past `trim_end`, playback holds on the last in-window
+    /// frame instead of continuing to draw whatever the source contains
+    /// beyond the intended trim point. When `loop_video` is set, playback
+    /// wraps within `[trim_start, trim_end)` instead of clamping — falling
+    /// back to the source's own probed duration as the loop window only
+    /// when `trim_end` is absent, since that is the only case where the
+    /// window cannot otherwise be known at all.
     fn effective_source_time(&self, ctx_time: f64) -> f64 {
         let rate = self.playback_rate.unwrap_or(1.0);
         let trim_start = self.trim_start.unwrap_or(0.0);
         let raw = trim_start + ctx_time * rate;
 
-        if self.loop_video == Some(true) {
+        if let Some(end) = self.trim_end {
+            if end > trim_start {
+                return if self.loop_video == Some(true) {
+                    trim_start + (raw - trim_start).rem_euclid(end - trim_start)
+                } else {
+                    raw.min(end)
+                };
+            }
+        } else if self.loop_video == Some(true) {
             if let Some(duration) = video_duration_secs(&self.src) {
                 if duration > trim_start {
                     return trim_start + (raw - trim_start).rem_euclid(duration - trim_start);
@@ -162,7 +176,7 @@ impl Painter for Video {
                 let img_info = ImageInfo::new(
                     (fw as i32, fh as i32),
                     ColorType::RGBA8888,
-                    skia_safe::AlphaType::Premul,
+                    skia_safe::AlphaType::Unpremul,
                     None,
                 );
                 let row_bytes = fw as usize * 4;

crates/rustmotion-components/tests/audit_ws_d.rs

@@ -186,3 +186,138 @@ fn fill_fit_still_stretches_to_the_whole_box() {
         "fill must cover every corner"
     );
 }
+
+// ─── `trim_end` was honoured only on the extracted audio ───────────────────
+
+/// Frames beyond `trim_end` sit in the cache (simulating a preextraction
+/// window, or a direct extraction, wider than the intended trim), so the
+/// picture path must never pick one of them once `trim_end` is set: past
+/// `trim_end`, playback holds on the last in-window frame. Before the fix,
+/// `source_time` had no upper bound at all — querying past `trim_end` on a
+/// cache/source that extends further would draw whatever sits further
+/// along the source, not the frame at the trim boundary.
+#[test]
+fn trim_end_clamps_playback_instead_of_running_past_it() {
+    let src = unique_src("trimend");
+    let cache_key = format!("{src}:20x20");
+    let frames = vec![
+        (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2),
+        (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2),
+        (1.0, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2),
+        (1.5, solid_rgba([128, 0, 128, 255], 2, 2), 2, 2),
+        (2.0, solid_rgba([255, 165, 0, 255], 2, 2), 2, 2),
+    ];
+    video_frame_cache().insert(cache_key, Arc::new(frames));
+
+    let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), None);
+    let ctx = ctx_at(1.8);
+    let pixels = paint_and_read(&v, &ctx, 20, 20, false);
+
+    assert_eq!(
+        px(&pixels, 20, 10, 10),
+        [0, 255, 0, 255],
+        "past trim_end, playback must clamp to the frame at trim_end (green), not the frame \
+         nearest the unclamped query time (orange)"
+    );
+}
+
+// ─── `loop_video` made neither the picture nor the audio loop ─────────────
+
+/// Cache frames only cover `[0.0, 1.0)`; `trim_end: Some(1.0)` gives
+/// `loop_video` a window to wrap within without needing a real source file
+/// to probe. Querying at `ctx.time = 2.1` (raw source time 2.1s, i.e. "2
+/// full loops plus 0.1s") must land near 0.1s once wrapped — nearest to
+/// that among `{0.0, 0.25, 0.5, 0.75}` is red. Before this fix, the same
+/// query — with the trim-end clamp from the previous test already in place
+/// but no loop branch yet — clamped to `min(2.1, 1.0) = 1.0`, whose nearest
+/// cached frame is yellow: a clearly different pixel, which is what proves
+/// this test is exercising the loop path and not being masked by the clamp.
+#[test]
+fn loop_video_wraps_playback_within_the_trim_window() {
+    let src = unique_src("loop");
+    let cache_key = format!("{src}:20x20");
+    let frames = vec![
+        (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2),
+        (0.25, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2),
+        (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2),
+        (0.75, solid_rgba([255, 255, 0, 255], 2, 2), 2, 2),
+    ];
+    video_frame_cache().insert(cache_key, Arc::new(frames));
+
+    let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), Some(true));
+    let ctx = ctx_at(2.1);
+    let pixels = paint_and_read(&v, &ctx, 20, 20, false);
+
+    assert_eq!(
+        px(&pixels, 20, 10, 10),
+        [255, 0, 0, 255],
+        "looping must wrap the query time back into the window (nearest: red), not clamp to \
+         the window's own end (nearest: yellow)"
+    );
+}
+
+/// Without `loop_video`, a `trim_end`-bounded video must still clamp
+/// (unaffected by the loop branch existing) rather than wrap — the same
+/// scenario as the wrap test above, minus the flag.
+#[test]
+fn without_loop_video_playback_still_clamps_not_wraps() {
+    let src = unique_src("no-loop");
+    let cache_key = format!("{src}:20x20");
+    let frames = vec![
+        (0.0, solid_rgba([255, 0, 0, 255], 2, 2), 2, 2),
+        (0.25, solid_rgba([0, 255, 0, 255], 2, 2), 2, 2),
+        (0.5, solid_rgba([0, 0, 255, 255], 2, 2), 2, 2),
+        (0.75, solid_rgba([255, 255, 0, 255], 2, 2), 2, 2),
+    ];
+    video_frame_cache().insert(cache_key, Arc::new(frames));
+
+    let v = video(&src, ImageFit::Fill, Some(0.0), Some(1.0), None);
+    let ctx = ctx_at(2.1);
+    let pixels = paint_and_read(&v, &ctx, 20, 20, false);
+
+    assert_eq!(
+        px(&pixels, 20, 10, 10),
+        [255, 255, 0, 255],
+        "no loop_video: must clamp to the window's end (nearest: yellow), not wrap"
+    );
+}
+
+// ─── cached-frame draw path mistagged straight alpha as premultiplied ──────
+
+/// ffmpeg's `-pix_fmt rgba` output — what fills the video-frame cache — is
+/// straight (unpremultiplied) alpha. Tagging that buffer `AlphaType::Premul`
+/// makes Skia treat the RGB channels as already scaled by alpha instead of
+/// scaling them itself, which brightens (here: doubles) every
+/// semi-transparent pixel's channels once composited.
+///
+/// A straight-alpha (200, 100, 50, 128) pixel, composited over black:
+/// correctly tagged `Unpremul`, Skia premultiplies it to
+/// (200×128/255, 100×128/255, 50×128/255) ≈ (100, 50, 25) before compositing
+/// over black, landing there almost exactly (the `(1 - alpha) * 0` background
+/// term vanishes either way). Mistagged `Premul`, Skia uses the raw channel
+/// values directly as if already scaled — (200, 100, 50) — composited over
+/// black with no further scaling, landing at roughly double the correct
+/// result.
+#[test]
+fn cached_frame_straight_alpha_composites_correctly_not_doubled() {
+    let src = unique_src("alpha");
+    let cache_key = format!("{src}:10x10");
+    let straight = [200u8, 100, 50, 128];
+    video_frame_cache().insert(
+        cache_key,
+        Arc::new(vec![(0.0, solid_rgba(straight, 2, 2), 2, 2)]),
+    );
+
+    let v = video(&src, ImageFit::Fill, None, None, None);
+    let ctx = ctx_at(0.0);
+    let pixels = paint_and_read(&v, &ctx, 10, 10, false);
+    let composited = px(&pixels, 10, 5, 5);
+
+    let close = |actual: u8, expected: u8| (actual as i16 - expected as i16).abs() <= 4;
+    assert!(
+        close(composited[0], 100) && close(composited[1], 50) && close(composited[2], 25),
+        "straight-alpha (200,100,50,128) over black must composite to roughly (100,50,25), \
+         got {composited:?} — a value near (200,100,50) means the buffer is still mistagged \
+         as premultiplied and its channels are being used unscaled"
+    );
+}