performance improvement

sam-tool-tauri/src/modes/ExtractMode.tsx

@@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import { open as openDialog } from "@tauri-apps/plugin-dialog";
 import { listen } from "@tauri-apps/api/event";
 import { api } from "../ipc";
-import type { CocoShape, ImportedCoco, VideoInfo } from "../types";
+import type { CocoShape, ImportedCoco, Settings, VideoInfo } from "../types";
 import { FrameCanvas } from "../components/extract/FrameCanvas";
 import { drawOverlay } from "../components/extract/overlay";
 
@@ -11,6 +11,7 @@ const FRAME_BITMAP_CACHE_LIMIT = 24;
 
 interface Props {
   username: string;
+  settings: Settings | null;
 }
 
 interface BulkProgress {
@@ -19,7 +20,14 @@ interface BulkProgress {
   phase: "extract" | "undo";
 }
 
-export function ExtractMode({ username }: Props) {
+export function ExtractMode({ username, settings }: Props) {
+  // frame_skip ≥ 1 means: during playback advance by N frames per tick, and
+  // bulk-extract takes every Nth frame across [start, end]. Kept in a ref so
+  // the long-lived playback loop sees changes without re-running its effect.
+  const frameSkipRef = useRef<number>(Math.max(1, settings?.frame_skip ?? 1));
+  useEffect(() => {
+    frameSkipRef.current = Math.max(1, settings?.frame_skip ?? 1);
+  }, [settings?.frame_skip]);
   const [video, setVideo] = useState<VideoInfo | null>(null);
   const [frameIdx, setFrameIdx] = useState(0);
   const [playing, setPlaying] = useState(false);
@@ -53,9 +61,6 @@ export function ExtractMode({ username }: Props) {
   const frameMapRef = useRef<Map<number, CocoShape[]>>(new Map());
   const overlayVisibleRef = useRef(overlayVisible);
   const annotatedFrames = useRef<Set<number>>(new Set());
-  // Playback clock anchor — set when playback starts, rebased on seeks so a
-  // slider drag during play doesn't get overwritten by the tick loop.
-  const playAnchorRef = useRef<{ time: number; idx: number } | null>(null);
 
   useEffect(() => { frameIdxRef.current = frameIdx; }, [frameIdx]);
   useEffect(() => {
@@ -69,6 +74,12 @@ export function ExtractMode({ username }: Props) {
   const renderedIdx = useRef(-1);
   const decodingInFlight = useRef(false);
   const bitmapCacheRef = useRef<Map<number, ImageBitmap>>(new Map());
+  // Last decode/render error message. Set in `pump` on failure, cleared on
+  // any successful render, and observed by the playback loop so a mid-play
+  // failure stops playback with a visible banner instead of leaving the UI
+  // stuck in "playing" while the canvas freezes.
+  const decodeErrorRef = useRef<string | null>(null);
+  const [playbackError, setPlaybackError] = useState<string | null>(null);
 
   const clearBitmapCache = useCallback(() => {
     for (const bitmap of bitmapCacheRef.current.values()) {
@@ -105,31 +116,82 @@ export function ExtractMode({ username }: Props) {
   const drawBitmap = useCallback(
     (bitmap: ImageBitmap, atFrameIdx: number) => {
       const canvas = canvasRef.current;
-      if (!canvas) return;
+      if (!canvas || !video) return;
       const ctx = canvas.getContext("2d");
       if (!ctx) return;
-      ctx.clearRect(0, 0, canvas.width, canvas.height);
-      ctx.drawImage(bitmap, 0, 0, canvas.width, canvas.height);
+
+      // Backing store = displayed CSS rect × DPR, capped at video native
+      // resolution (anything above is upsampling for no benefit). Halves
+      // drawImage cost on 4K-into-1080p, which is the common slow-system
+      // case. Coords below are in image-pixel space — the transform handles
+      // the mapping to backing pixels, so overlay.ts is unchanged.
+      const rect = canvas.getBoundingClientRect();
+      const dpr = window.devicePixelRatio || 1;
+      const targetW = Math.max(
+        1,
+        Math.min(video.width, Math.round(rect.width * dpr))
+      );
+      const targetH = Math.max(
+        1,
+        Math.min(video.height, Math.round(rect.height * dpr))
+      );
+      if (canvas.width !== targetW) canvas.width = targetW;
+      if (canvas.height !== targetH) canvas.height = targetH;
+
+      ctx.setTransform(
+        targetW / video.width,
+        0,
+        0,
+        targetH / video.height,
+        0,
+        0
+      );
+      ctx.clearRect(0, 0, video.width, video.height);
+      ctx.drawImage(bitmap, 0, 0, video.width, video.height);
 
       if (overlayVisibleRef.current) {
         const shapes = frameMapRef.current.get(atFrameIdx);
         if (shapes && shapes.length > 0) {
-          drawOverlay(ctx, shapes, canvas.width, canvas.height);
+          drawOverlay(ctx, shapes, video.width, video.height);
         }
       }
+      ctx.setTransform(1, 0, 0, 1, 0, 0);
     },
-    []
+    [video]
   );
 
+  // Returns true if we drew, false if we were preempted by a newer request,
+  // and throws if bitmap creation failed — pump uses this to decide whether
+  // to mark the frame rendered or surface the error to the playback loop.
   const renderFrame = useCallback(
-    async (bytes: ArrayBuffer, atFrameIdx: number) => {
+    async (bytes: ArrayBuffer, atFrameIdx: number): Promise<boolean> => {
+      // Diagnostic timing log: set `window.SAM_FRAME_DEBUG = true` in devtools
+      // on a slow machine to see per-frame bitmap and draw timings — confirms
+      // where the bottleneck lives (decode vs createImageBitmap vs drawImage).
+      const debug = (window as unknown as { SAM_FRAME_DEBUG?: boolean })
+        .SAM_FRAME_DEBUG === true;
+      const t0 = debug ? performance.now() : 0;
       const blob = new Blob([bytes], { type: "image/jpeg" });
       let bitmap: ImageBitmap;
-      try { bitmap = await createImageBitmap(blob); }
-      catch (e) { console.error("createImageBitmap failed", e); return; }
+      try {
+        bitmap = await createImageBitmap(blob);
+      } catch (e) {
+        throw new Error(
+          `createImageBitmap failed: ${e instanceof Error ? e.message : String(e)}`
+        );
+      }
+      const t1 = debug ? performance.now() : 0;
       rememberBitmap(atFrameIdx, bitmap);
-      if (requestedIdx.current !== atFrameIdx) return;
+      if (requestedIdx.current !== atFrameIdx) return false;
       drawBitmap(bitmap, atFrameIdx);
+      if (debug) {
+        const t2 = performance.now();
+        // eslint-disable-next-line no-console
+        console.debug(
+          `[SAM] f${atFrameIdx} bytes=${bytes.byteLength} bitmap=${(t1 - t0).toFixed(0)}ms draw=${(t2 - t1).toFixed(0)}ms`
+        );
+      }
+      return true;
     },
     [drawBitmap, rememberBitmap]
   );
@@ -149,11 +211,17 @@ export function ExtractMode({ username }: Props) {
         try {
           const bytes = await api.decodeFrame(want);
           if (requestedIdx.current !== want) continue;
-          await renderFrame(bytes, want);
-          renderedIdx.current = want;
+          const drew = await renderFrame(bytes, want);
+          if (drew) {
+            renderedIdx.current = want;
+            decodeErrorRef.current = null;
+          }
+          // !drew means renderFrame was preempted by a newer request; loop
+          // continues and the new target gets a fresh decode.
         } catch (e) {
           if (requestedIdx.current !== want) continue;
-          console.error("decode_frame failed", e);
+          console.error("decode/render failed", e);
+          decodeErrorRef.current = e instanceof Error ? e.message : String(e);
           break;
         }
       }
@@ -181,47 +249,114 @@ export function ExtractMode({ username }: Props) {
 
   useEffect(() => clearBitmapCache, [clearBitmapCache]);
 
+  // Redraw on canvas resize: backing-store is sized from CSS rect × DPR, so
+  // a window resize/maximize would otherwise leave the old backing dimensions
+  // until the next decode landed. Cheap — just re-runs drawBitmap on the
+  // already-cached frame.
+  useEffect(() => {
+    const canvas = canvasRef.current;
+    if (!canvas || !video) return;
+    let scheduled = false;
+    const ro = new ResizeObserver(() => {
+      if (scheduled) return;
+      scheduled = true;
+      requestAnimationFrame(() => {
+        scheduled = false;
+        const idx = frameIdxRef.current;
+        const cached = getCachedBitmap(idx);
+        if (cached) drawBitmap(cached, idx);
+        else { renderedIdx.current = -1; pump(); }
+      });
+    });
+    ro.observe(canvas);
+    return () => ro.disconnect();
+  }, [video, drawBitmap, getCachedBitmap, pump]);
+
   // --- playback loop ---------------------------------------------------
+  // Render-paced: advance ONLY after the previous frame actually drew. On a
+  // fast box this hits target fps; on a slow box it plays smoothly at the
+  // achievable decode/render rate instead of dropping 9-of-10 frames at the
+  // canvas while the wall-clock UI counter races ahead.
   useEffect(() => {
     if (!playing || !video) return;
-    playAnchorRef.current = {
-      time: performance.now(),
-      idx: frameIdxRef.current,
-    };
+    let cancelled = false;
+    let nextDueAt = performance.now();
 
-    let raf = 0;
-    const tick = () => {
-      const anchor = playAnchorRef.current;
-      if (!anchor) return;
-      const elapsed = (performance.now() - anchor.time) / 1000;
-      const want = Math.min(
-        video.total_frames - 1,
-        anchor.idx + Math.floor(elapsed * video.fps * speed)
-      );
-      // Only trigger the review-mode auto-pause once we've actually advanced
-      // past the resume point. Otherwise pressing play while parked on an
-      // annotated frame would re-pause on the very first tick, making the
-      // play button look broken — the user would have to step a frame
-      // forward manually before play could "stick".
-      if (
-        reviewMode &&
-        want > anchor.idx &&
-        annotatedFrames.current.has(want)
-      ) {
-        setFrameIdx(want);
-        setPlaying(false);
-        return;
+    const waitForFrame = (target: number) =>
+      new Promise<void>((resolve) => {
+        const check = () => {
+          if (
+            cancelled ||
+            renderedIdx.current === target ||
+            decodeErrorRef.current !== null
+          ) {
+            resolve();
+            return;
+          }
+          requestAnimationFrame(check);
+        };
+        check();
+      });
+
+    const loop = async () => {
+      while (!cancelled && playing) {
+        const skip = Math.max(1, frameSkipRef.current | 0);
+        // Real-time pacing regardless of skip: skip controls density,
+        // `speed` controls speed. Without the skip factor here, skip=3 would
+        // play at 3× wall-clock — which is what the speed slider is for.
+        const interval = (skip * 1000) / (video.fps * speed);
+        const current = frameIdxRef.current;
+        let target = Math.min(video.total_frames - 1, current + skip);
+        if (target <= current) {
+          setPlaying(false);
+          return;
+        }
+        // Review-mode scan: with skip > 1 a naive +skip jump can leap past
+        // annotations between current+1 and current+skip. Walk that window
+        // and clamp `target` down to the first annotated frame.
+        if (reviewMode) {
+          for (let i = current + 1; i <= target; i++) {
+            if (annotatedFrames.current.has(i)) {
+              target = i;
+              break;
+            }
+          }
+        }
+        requestedIdx.current = target;
+        pump();
+        await waitForFrame(target);
+        if (cancelled) return;
+        // Decode failed mid-play: stop and surface so the UI doesn't claim to
+        // be playing while the canvas is frozen on the last good frame.
+        if (decodeErrorRef.current) {
+          setPlaybackError(decodeErrorRef.current);
+          setPlaying(false);
+          return;
+        }
+        setFrameIdx(target);
+        // Pause AFTER rendering so the user actually sees the annotated frame.
+        if (reviewMode && annotatedFrames.current.has(target)) {
+          setPlaying(false);
+          return;
+        }
+
+        const now = performance.now();
+        const due = nextDueAt + interval;
+        if (due > now) {
+          await new Promise((r) => setTimeout(r, due - now));
+          nextDueAt = due;
+        } else {
+          // Behind real-time — don't accumulate debt and then "catch up" by
+          // skipping frames; just reset the clock to now.
+          nextDueAt = now;
+        }
       }
-      setFrameIdx(want);
-      if (want >= video.total_frames - 1) { setPlaying(false); return; }
-      raf = requestAnimationFrame(tick);
     };
-    raf = requestAnimationFrame(tick);
+    loop();
     return () => {
-      cancelAnimationFrame(raf);
-      playAnchorRef.current = null;
+      cancelled = true;
     };
-  }, [playing, video, speed, reviewMode]);
+  }, [playing, video, speed, reviewMode, pump]);
 
   // --- navigation helpers ----------------------------------------------
   const seekToFrame = useCallback((idx: number) => {
@@ -237,11 +372,6 @@ export function ExtractMode({ username }: Props) {
       pump();
     }
     setFrameIdx((prev) => (prev === clamped ? prev : clamped));
-    // If a playback tick is active, rebase its anchor so the loop continues
-    // from the new position instead of snapping back.
-    if (playAnchorRef.current) {
-      playAnchorRef.current = { time: performance.now(), idx: clamped };
-    }
   }, [drawBitmap, getCachedBitmap, pump, video]);
 
   const stepFrames = useCallback((delta: number) => {
@@ -252,7 +382,15 @@ export function ExtractMode({ username }: Props) {
 
   const togglePlay = useCallback(() => {
     if (!video) return;
-    setPlaying((p) => !p);
+    setPlaying((p) => {
+      if (!p) {
+        // Fresh play attempt — clear any stale decode error so the loop can
+        // re-arm. If it fails again, the banner will reappear.
+        decodeErrorRef.current = null;
+        setPlaybackError(null);
+      }
+      return !p;
+    });
   }, [video]);
 
   const bumpSpeed = useCallback((dir: 1 | -1) => {
@@ -326,22 +464,31 @@ export function ExtractMode({ username }: Props) {
     };
   }, [bulkStart, bulkEnd]);
 
+  const bulkCount = useMemo(() => {
+    if (!bulkRange) return 0;
+    const skip = Math.max(1, settings?.frame_skip ?? 1);
+    return Math.floor((bulkRange.end - bulkRange.start) / skip) + 1;
+  }, [bulkRange, settings?.frame_skip]);
+
   async function runBulkExtract() {
     if (!outputFolder || !bulkRange) return;
     setPlaying(false);
     setBulkError(null);
-    setBulkProgress({ done: 0, total: bulkRange.end - bulkRange.start + 1, phase: "extract" });
+    const skip = Math.max(1, settings?.frame_skip ?? 1);
+    const total = Math.floor((bulkRange.end - bulkRange.start) / skip) + 1;
+    setBulkProgress({ done: 0, total, phase: "extract" });
     const unlisten = await listen<{ done: number; total: number }>(
       "bulk-extract-progress",
       (e) => setBulkProgress({ ...e.payload, phase: "extract" })
     );
+    // Only include annotations for frames we'll actually emit; backend will
+    // step start..end by `skip` and shapesByFrame is keyed by absolute idx.
     const shapesByFrame: Record<string, CocoShape[]> = {};
     if (coco) {
-      for (const [key, shapes] of Object.entries(coco.frames)) {
-        const idx = Number(key);
-        if (idx >= bulkRange.start && idx <= bulkRange.end) {
-          shapesByFrame[key] = shapes;
-        }
+      for (let idx = bulkRange.start; idx <= bulkRange.end; idx += skip) {
+        const key = String(idx);
+        const shapes = coco.frames[key];
+        if (shapes) shapesByFrame[key] = shapes;
       }
     }
     try {
@@ -350,7 +497,8 @@ export function ExtractMode({ username }: Props) {
         bulkRange.end,
         outputFolder,
         shapesByFrame,
-        username
+        username,
+        skip
       );
       const existing = await api.listExtractedFrames(outputFolder);
       setExtractedFrames(new Set(existing));
@@ -401,6 +549,16 @@ export function ExtractMode({ username }: Props) {
       const tag = (e.target as HTMLElement | null)?.tagName;
       if (tag === "INPUT" || tag === "TEXTAREA") return;
       const step = e.ctrlKey || e.metaKey ? 10 : 1;
+      // Adaptive throttle: drop OS key-repeats when the previous step is
+      // still decoding. On fast systems this is a no-op; on slow systems it
+      // prevents a held arrow from queueing 30 seeks/sec the pump can't keep
+      // up with, which is what causes the "huge gap between keypress and
+      // frame" feel.
+      const isHeldRepeat =
+        e.repeat &&
+        (e.key === "ArrowRight" || e.key === "ArrowLeft") &&
+        requestedIdx.current !== renderedIdx.current;
+      if (isHeldRepeat) { e.preventDefault(); return; }
       switch (e.key) {
         case " ":
           e.preventDefault(); togglePlay(); break;
@@ -463,6 +621,8 @@ export function ExtractMode({ username }: Props) {
       setPlaying(false);
       setSpeed(1);
       renderedIdx.current = -1;
+      decodeErrorRef.current = null;
+      setPlaybackError(null);
       // Reset per-video extraction context, then auto-adopt the per-video
       // default folder. If the user already extracted frames into it in a
       // prior session, listExtractedFrames rehydrates them immediately.
@@ -590,7 +750,7 @@ export function ExtractMode({ username }: Props) {
             onClick={runBulkExtract}
             disabled={!outputFolder || !bulkRange || !!bulkProgress}
           >
-            Bulk Extract{bulkRange && ` (${bulkRange.end - bulkRange.start + 1})`}
+            Bulk Extract{bulkRange && ` (${bulkCount})`}
           </button>
           <button
             onClick={runBulkUndo}
@@ -683,6 +843,22 @@ export function ExtractMode({ username }: Props) {
         </button>
         <span className="dim shortcuts">space · ← → · ctrl+← → · ↑ ↓ · H · E · U · [ ]</span>
       </div>
+
+      {playbackError && (
+        <p className="error" role="alert">
+          Playback stopped: {playbackError}
+          {" "}
+          <button
+            className="link"
+            onClick={() => {
+              decodeErrorRef.current = null;
+              setPlaybackError(null);
+            }}
+          >
+            dismiss
+          </button>
+        </p>
+      )}
     </div>
   );
 }