From f7029e87482c77d75f050f1610a4a8305aff6601 Mon Sep 17 00:00:00 2001 From: ravi Date: Thu, 4 Jun 2026 18:20:13 +0530 Subject: [PATCH] playback similar to videoannotator --- sam-tool-tauri/src-tauri/src/decoder.rs | 131 +------- .../src/components/extract/FrameCanvas.tsx | 17 +- sam-tool-tauri/src/modes/ExtractMode.tsx | 311 ++++++++++-------- .../src/services/frameDecoderPool.ts | 124 +++---- 4 files changed, 265 insertions(+), 318 deletions(-) diff --git a/sam-tool-tauri/src-tauri/src/decoder.rs b/sam-tool-tauri/src-tauri/src/decoder.rs index 92da6a9..9b8ec05 100644 --- a/sam-tool-tauri/src-tauri/src/decoder.rs +++ b/sam-tool-tauri/src-tauri/src/decoder.rs @@ -2,7 +2,7 @@ use crate::video::VideoInfo; use std::collections::{HashMap, VecDeque}; use std::io::{BufRead, BufReader}; use std::path::PathBuf; -use std::process::{Child, ChildStderr, ChildStdout, Command, Stdio}; +use std::process::{Child, ChildStdout, Command, Stdio}; use std::sync::{Arc, Condvar, Mutex}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; @@ -12,12 +12,10 @@ pub const MIN_CACHE_CAPACITY: usize = 50; pub const MAX_CACHE_CAPACITY: usize = 800; const READ_BUF_BYTES: usize = 256 * 1024; /// If the requested frame is this far ahead of the worker, kill + reseek. -const SEEK_AHEAD_THRESHOLD: u64 = 60; +/// Higher = fewer ffmpeg restarts during forward scan; the cap is bounded +/// by lookahead × cache capacity anyway. +const SEEK_AHEAD_THRESHOLD: u64 = 120; const DECODE_TIMEOUT: Duration = Duration::from_secs(15); -/// Keep the last N stderr lines from ffmpeg so error messages can include -/// the real reason (e.g. "vaapi: failed to load driver") instead of just -/// "pipe closed before frame completed". -const STDERR_TAIL_LINES: usize = 32; struct FrameCache { map: HashMap>, @@ -93,9 +91,6 @@ struct SharedState { struct Shared { state: Mutex, cond: Condvar, - /// Ring buffer of recent ffmpeg stderr lines, filled by a sibling drain - /// thread. Cleared at each new seek. - stderr_tail: Mutex>, } pub struct DecoderSession { @@ -107,10 +102,6 @@ pub struct DecoderSession { hwaccel: Option, shared: Arc, worker: Option>, - /// Sibling thread draining ffmpeg's stderr into `shared.stderr_tail`. - /// Joined in `stop_worker` so the tail doesn't get clobbered by a - /// straggling drain across seeks. - stderr_drain: Option>, } impl DecoderSession { @@ -131,10 +122,8 @@ impl DecoderSession { shutdown: false, }), cond: Condvar::new(), - stderr_tail: Mutex::new(VecDeque::new()), }), worker: None, - stderr_drain: None, } } @@ -234,7 +223,7 @@ impl DecoderSession { fn seek_and_start_worker(&mut self, idx: u64) -> Result<(), String> { self.stop_worker(); - let (child, stdout, stderr) = + let (child, stdout) = spawn_ffmpeg(&self.path, &self.info, idx, self.hwaccel.as_deref())?; { @@ -246,24 +235,8 @@ impl DecoderSession { st.error = None; st.shutdown = false; } - self.shared.stderr_tail.lock().unwrap().clear(); self.shared.cond.notify_all(); - // Drain ffmpeg's stderr into a ring buffer so error messages can - // include the real reason. We must drain even on success — leaving - // stderr unread would let the pipe buffer fill and stall ffmpeg. - let drain_shared = self.shared.clone(); - self.stderr_drain = Some(thread::spawn(move || { - let reader = BufReader::new(stderr); - for line in reader.lines().map_while(Result::ok) { - let mut tail = drain_shared.stderr_tail.lock().unwrap(); - if tail.len() >= STDERR_TAIL_LINES { - tail.pop_front(); - } - tail.push_back(line); - } - })); - let shared = self.shared.clone(); let handle = thread::spawn(move || { worker_loop(stdout, shared); @@ -289,12 +262,6 @@ impl DecoderSession { if let Some(handle) = self.worker.take() { let _ = handle.join(); } - // Killing the child closed stderr, so the drain thread sees EOF and - // exits — joining it here prevents a straggling write into the new - // session's stderr_tail after the next seek clears it. - if let Some(handle) = self.stderr_drain.take() { - let _ = handle.join(); - } let mut st = self.shared.state.lock().unwrap(); st.shutdown = false; @@ -347,7 +314,7 @@ fn spawn_ffmpeg( info: &VideoInfo, idx: u64, hwaccel: Option<&str>, -) -> Result<(Child, BufReader, ChildStderr), String> { +) -> Result<(Child, BufReader), String> { let path_str = path.to_str().ok_or_else(|| "non-utf8 path".to_string())?; let t = idx as f64 / info.fps; let mut cmd = Command::new("ffmpeg"); @@ -372,14 +339,12 @@ fn spawn_ffmpeg( } _ => {} } - if t > 1.0 { - cmd.args(["-ss", &format!("{:.3}", t - 1.0)]); - cmd.args(["-i", path_str]); - cmd.args(["-ss", "1.0"]); - } else { - cmd.args(["-i", path_str]); - cmd.args(["-ss", &format!("{:.3}", t)]); - } + // Single fast-seek `-ss` before `-i`. The old `(-ss t-1) -i (-ss 1.0)` + // double-seek pattern decoded ~30 hidden warmup frames per seek; on + // typical content the fast-seek is accurate enough and seeks take + // milliseconds instead of hundreds of ms. + cmd.args(["-ss", &format!("{:.6}", t)]); + cmd.args(["-i", path_str]); cmd.args([ "-an", "-q:v", @@ -390,21 +355,16 @@ fn spawn_ffmpeg( "mjpeg", "pipe:1", ]); - cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + // stderr null: capturing it cost a spawn+join drain thread per seek + // plus up to 150 ms of polling on EOF. hwaccel-failure messages still + // surface via the exit code path through `decode_at`'s fallback. + cmd.stdout(Stdio::piped()).stderr(Stdio::null()); let mut child = cmd.spawn().map_err(|e| format!("ffmpeg spawn: {e}"))?; let stdout = child .stdout .take() .ok_or_else(|| "ffmpeg stdout unavailable".to_string())?; - let stderr = child - .stderr - .take() - .ok_or_else(|| "ffmpeg stderr unavailable".to_string())?; - Ok(( - child, - BufReader::with_capacity(READ_BUF_BYTES, stdout), - stderr, - )) + Ok((child, BufReader::with_capacity(READ_BUF_BYTES, stdout))) } fn worker_loop(mut stdout: BufReader, shared: Arc) { @@ -440,19 +400,8 @@ fn worker_loop(mut stdout: BufReader, shared: Arc) { shared.cond.notify_all(); } Err(e) => { - // ffmpeg closes stderr right alongside stdout; on real - // failures the drain thread may not have flushed yet, so - // we wait briefly. On a clean exit there's nothing to - // flush — collect_stderr_tail short-circuits via the - // child's exit code so end-of-stream EOF isn't penalised. - let tail = collect_stderr_tail(&shared, Duration::from_millis(150)); let mut st = shared.state.lock().unwrap(); - if !tail.is_empty() { - // With `-v error`, anything on stderr means ffmpeg - // actually failed (vs a clean exit) — surface it as a - // real error so decode_at can fall back / report. - st.error = Some(format!("{e}: ffmpeg: {tail}")); - } else if e.contains("pipe closed") || e.contains("EOF") { + if e.contains("pipe closed") || e.contains("EOF") { st.eof = true; } else { st.error = Some(e); @@ -464,50 +413,6 @@ fn worker_loop(mut stdout: BufReader, shared: Arc) { } } -fn read_tail_now(shared: &Arc) -> Option { - let tail = shared.stderr_tail.lock().unwrap(); - if tail.is_empty() { - None - } else { - Some(tail.iter().cloned().collect::>().join(" | ")) - } -} - -fn collect_stderr_tail(shared: &Arc, max_wait: Duration) -> String { - // Fast path: drain already flushed — common when ffmpeg writes its - // error before exiting and stdout EOF arrives after the drain. - if let Some(tail) = read_tail_now(shared) { - return tail; - } - // Skip the poll on clean ffmpeg exit (exit code 0): natural end-of- - // stream has nothing on stderr (we run with `-v error`), and 150ms × - // every EOF seek near the final frames adds up. - let exited_cleanly = { - let mut st = shared.state.lock().unwrap(); - st.child - .as_mut() - .and_then(|c| c.try_wait().ok().flatten()) - .map(|status| status.success()) - .unwrap_or(false) - }; - if exited_cleanly { - return String::new(); - } - // Slow path: ffmpeg exited non-zero (or hasn't exited yet) and the - // drain thread hasn't flushed. Poll briefly so the error message - // carries the real reason. - let deadline = Instant::now() + max_wait; - loop { - if let Some(tail) = read_tail_now(shared) { - return tail; - } - if Instant::now() >= deadline { - return String::new(); - } - thread::sleep(Duration::from_millis(10)); - } -} - fn read_jpeg_bytes(reader: &mut BufReader) -> Result, String> { let mut out = Vec::with_capacity(64 * 1024); let mut last = 0u8; diff --git a/sam-tool-tauri/src/components/extract/FrameCanvas.tsx b/sam-tool-tauri/src/components/extract/FrameCanvas.tsx index 2f261ce..b836bfb 100644 --- a/sam-tool-tauri/src/components/extract/FrameCanvas.tsx +++ b/sam-tool-tauri/src/components/extract/FrameCanvas.tsx @@ -1,4 +1,4 @@ -import { forwardRef } from "react"; +import { forwardRef, useMemo } from "react"; import type { VideoInfo } from "../../types"; interface Props { @@ -10,15 +10,16 @@ export const FrameCanvas = forwardRef(function FrameCa { video, extracted }, ref ) { + // Memo the style so a fresh object isn't allocated on every parent render. + // The canvas's backing store width/height are JS-driven (see ExtractMode's + // ResizeObserver effect); CSS just keeps the box at the video's aspect. + const aspectStyle = useMemo( + () => ({ aspectRatio: `${video.width} / ${video.height}` }), + [video.width, video.height] + ); return (
-
- {/* Backing-store width/height are set imperatively in drawBitmap so we - can cap it at the displayed (CSS) size × DPR — drawing 4K every frame - into a 1080p window is the main playback cost on slow systems. */} +
{extracted && (
EXTRACTED
diff --git a/sam-tool-tauri/src/modes/ExtractMode.tsx b/sam-tool-tauri/src/modes/ExtractMode.tsx index ea11846..9f0ad99 100644 --- a/sam-tool-tauri/src/modes/ExtractMode.tsx +++ b/sam-tool-tauri/src/modes/ExtractMode.tsx @@ -9,7 +9,10 @@ import { drawOverlay } from "../components/extract/overlay"; import { sharedFrameDecoderPool } from "../services/frameDecoderPool"; const SPEEDS = [0.25, 0.5, 1, 1.5, 2, 3, 4]; -const FRAME_BITMAP_CACHE_LIMIT = 24; +// Stream playback no longer writes to this cache (it holds at most one +// bitmap at a time via `lastStreamBitmapRef`). The cache is now used only by +// the seek/scrub/step path, so a small limit is plenty. +const FRAME_BITMAP_CACHE_LIMIT = 8; interface Props { username: string; @@ -38,6 +41,12 @@ export function ExtractMode({ username, settings }: Props) { previewMaxHRef.current = settings?.preview_max_height ?? 0; }, [settings?.preview_max_height]); const [video, setVideo] = useState(null); + // `frameIdxRef.current` is the authoritative current frame. During stream + // playback the Channel onmessage handler writes ONLY to the ref — never to + // React state — to avoid forcing a full ExtractMode reconciliation at 30 fps + // (that was the primary stutter source). `frameIdx` (state) is the + // display-visible value, copied from the ref at ~10 Hz by an interval below + // and updated immediately on user-initiated changes (seek / step). const [frameIdx, setFrameIdx] = useState(0); const [playing, setPlaying] = useState(false); const [speed, setSpeed] = useState(1); @@ -45,6 +54,15 @@ export function ExtractMode({ username, settings }: Props) { const [overlayVisible, setOverlayVisible] = useState(true); const [loadError, setLoadError] = useState(null); + // Refs that mirror reactive props/state so the playback effect can read + // them without listing them in deps. Otherwise toggling speed or + // reviewMode tears down + respawns the Rust producer, which is what was + // hitching on mid-play settings changes. + const speedRef = useRef(speed); + const reviewModeRef = useRef(reviewMode); + useEffect(() => { speedRef.current = speed; }, [speed]); + useEffect(() => { reviewModeRef.current = reviewMode; }, [reviewMode]); + const [coco, setCoco] = useState(null); // Extraction state @@ -83,17 +101,21 @@ export function ExtractMode({ username, settings }: Props) { const renderedIdx = useRef(-1); const decodingInFlight = useRef(false); const bitmapCacheRef = useRef>(new Map()); + // During stream playback we don't write to the bitmap cache (each frame is + // a one-shot, caching them at 30 fps just balloons GPU memory). The stream + // handler holds the latest bitmap here and closes the previous one when a + // new one arrives. + const lastStreamBitmapRef = useRef(null); + // Bumped on every stream start/stop. The channel handler reads this before + // and after every `await` and bails (closing the bitmap) if it changed — + // prevents stale frames from a torn-down stream painting over a new one. + const streamEpochRef = useRef(0); // 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(null); const [playbackError, setPlaybackError] = useState(null); - // Cached canvas CSS rect. drawBitmap used to call getBoundingClientRect() - // every frame, which forces a synchronous layout flush each time. We now - // refresh it from the ResizeObserver below and fall back to a one-shot - // query if it isn't populated yet. - const canvasRectRef = useRef<{ width: number; height: number } | null>(null); const clearBitmapCache = useCallback(() => { for (const bitmap of bitmapCacheRef.current.values()) { @@ -127,62 +149,46 @@ export function ExtractMode({ username, settings }: Props) { } }, []); + // drawBitmap is the per-frame hot path. It assumes canvas.width / height + // have already been sized by the ResizeObserver effect below — we DO NOT + // mutate the backing store here. The old code did per-frame resize plus a + // non-identity setTransform, which on every frame: (a) reallocated the + // canvas backing store when rounded rect×DPR oscillated by a pixel; and + // (b) pushed the GPU off its fast identity-transform drawImage path. Now + // we do exactly one fillRect (letterbox) + one drawImage with + // pre-computed contain-fit (dx, dy, dw, dh) at the canvas's existing + // backing size. Overlay rendering wraps a localised save / setTransform + // so overlay.ts keeps using image-pixel coords unchanged. const drawBitmap = useCallback( (bitmap: ImageBitmap, atFrameIdx: number) => { const canvas = canvasRef.current; if (!canvas || !video) return; const ctx = canvas.getContext("2d"); if (!ctx) return; + const cw = canvas.width; + const ch = canvas.height; + if (cw <= 0 || ch <= 0) return; - // Backing store = displayed CSS rect × DPR, capped at video native and - // optionally at the user's Preview-quality preset. Halves drawImage - // cost on 4K-into-1080p (the common slow-system case). Coords below - // are in image-pixel space — the transform maps to backing pixels so - // overlay.ts is unchanged. The rect is cached by the ResizeObserver - // effect; we only fall back to a live query on the first frame. - let rect = canvasRectRef.current; - if (!rect) { - const dom = canvas.getBoundingClientRect(); - rect = { width: dom.width, height: dom.height }; - canvasRectRef.current = rect; - } - const dpr = window.devicePixelRatio || 1; - let maxW = video.width; - let maxH = video.height; - const previewMaxH = previewMaxHRef.current; - if (previewMaxH > 0 && previewMaxH < video.height) { - maxH = previewMaxH; - maxW = Math.round(previewMaxH * (video.width / video.height)); - } - const targetW = Math.max( - 1, - Math.min(maxW, Math.round(rect.width * dpr)) - ); - const targetH = Math.max( - 1, - Math.min(maxH, Math.round(rect.height * dpr)) - ); - if (canvas.width !== targetW) canvas.width = targetW; - if (canvas.height !== targetH) canvas.height = targetH; + ctx.fillStyle = "#000"; + ctx.fillRect(0, 0, cw, ch); - 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); + const scale = Math.min(cw / video.width, ch / video.height); + const dw = video.width * scale; + const dh = video.height * scale; + const dx = (cw - dw) / 2; + const dy = (ch - dh) / 2; + ctx.drawImage(bitmap, dx, dy, dw, dh); if (overlayVisibleRef.current) { const shapes = frameMapRef.current.get(atFrameIdx); if (shapes && shapes.length > 0) { + ctx.save(); + ctx.translate(dx, dy); + ctx.scale(scale, scale); drawOverlay(ctx, shapes, video.width, video.height); + ctx.restore(); } } - ctx.setTransform(1, 0, 0, 1, 0, 0); }, [video] ); @@ -286,77 +292,91 @@ export function ExtractMode({ username, settings }: Props) { useEffect(() => clearBitmapCache, [clearBitmapCache]); - // Live re-render when the preview-quality cap changes so the user sees the - // resolution swap immediately on Settings save instead of having to seek. + // OWNS canvas backing-store sizing. drawBitmap reads the canvas's current + // width/height; this effect mutates them in response to resize events and + // preview-quality settings changes. Backing store = CSS rect (NO DPR + // multiplier) capped at `preview_max_height` × aspect, capped at video + // native. Dropping DPR is intentional: on a HiDPI laptop it was 4×ing the + // per-frame fill cost for marginal sharpness gain. useEffect(() => { const canvas = canvasRef.current; if (!canvas || !video) return; - const idx = frameIdxRef.current; - const cached = getCachedBitmap(idx); - if (cached) drawBitmap(cached, idx); - // If not cached, the next pump invocation picks up the new cap naturally. - }, [settings?.preview_max_height, video, drawBitmap, getCachedBitmap]); - // 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; - // Seed the rect cache once on mount so the first drawBitmap doesn't - // have to force a layout flush itself. - { - const dom = canvas.getBoundingClientRect(); - canvasRectRef.current = { width: dom.width, height: dom.height }; - } - const ro = new ResizeObserver((entries) => { - // Update the cached rect from the observer entry — it carries the - // CSS-box size without forcing a sync layout. - const last = entries[entries.length - 1]; - const box = last?.contentBoxSize?.[0]; - if (box) { - canvasRectRef.current = { - width: box.inlineSize, - height: box.blockSize, - }; - } else if (last?.contentRect) { - canvasRectRef.current = { - width: last.contentRect.width, - height: last.contentRect.height, - }; + const applySize = () => { + const rect = canvas.getBoundingClientRect(); + const previewMaxH = previewMaxHRef.current; + let w = Math.max(1, Math.round(rect.width)); + let h = Math.max(1, Math.round(rect.height)); + // Cap by preview-quality preset (uses aspect to derive width). + if (previewMaxH > 0 && h > previewMaxH) { + const aspect = video.width / video.height; + h = previewMaxH; + w = Math.max(1, Math.round(h * aspect)); } + // Never upsample beyond native video. + if (w > video.width) { + const aspect = video.width / video.height; + w = video.width; + h = Math.max(1, Math.round(w / aspect)); + } + if (h > video.height) { + const aspect = video.width / video.height; + h = video.height; + w = Math.max(1, Math.round(h * aspect)); + } + let changed = false; + if (canvas.width !== w) { canvas.width = w; changed = true; } + if (canvas.height !== h) { canvas.height = h; changed = true; } + return changed; + }; + + const repaintCurrent = () => { + const idx = frameIdxRef.current; + const cached = lastStreamBitmapRef.current ?? getCachedBitmap(idx); + if (cached) drawBitmap(cached, idx); + else { renderedIdx.current = -1; pump(); } + }; + + // Initial sizing + paint. + applySize(); + repaintCurrent(); + + 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(); } + const changed = applySize(); + if (changed) repaintCurrent(); }); }); ro.observe(canvas); return () => ro.disconnect(); - }, [video, drawBitmap, getCachedBitmap, pump]); + }, [video, drawBitmap, getCachedBitmap, pump, settings?.preview_max_height]); // --- playback (Channel-push transport) ----------------------------------- // Forward playback runs through a Rust producer thread that pushes packed - // frames over a Tauri Channel — eliminates the per-frame IPC round-trip - // that was the main stutter source. Backward / step / scrub / pause all - // stay on the invoke-pull path (pump() above). Three guards live here: - // * `paused` — set true on review-mode stop so any in-flight messages - // queued behind us don't paint; - // * `lastPainted` — direction-aware out-of-order drop in case the worker - // pool returns N+1 before N; - // * cleanup stops the stream and re-fetches the last painted frame via - // invoke so the canvas exactly matches `frameIdx` on pause. + // frames over a Tauri Channel. The hot path here is RIGOROUSLY ref-only: + // * `frameIdxRef.current` is the authoritative current frame. + // * `setFrameIdx` is NEVER called from the channel handler — that was + // forcing a 30 Hz React reconciliation of the 1000+ line ExtractMode + // tree, which starved the decode pump. A separate 10 Hz interval + // copies the ref into state for the slider/readout. + // * Bitmaps from the stream are NOT written into the LRU cache (24 + // bitmaps × 1080p ≈ 200 MB of GPU memory if we did). The channel + // handler holds at most one bitmap at a time in `lastStreamBitmapRef`. + // * Deps are `[playing, video?.path]` only — speed/reviewMode/skip flow + // through refs so toggling them mid-play does NOT tear down the + // producer. A separate effect pushes them via setStreamState. + // * `streamEpochRef` is bumped on every start so any in-flight decode + // from a torn-down session can be bailed (and its bitmap closed). useEffect(() => { if (!playing || !video) return; let cancelled = false; let paused = false; let lastPainted = frameIdxRef.current; + const myEpoch = ++streamEpochRef.current; const skip = Math.max(1, frameSkipRef.current | 0); const startFrom = Math.min( video.total_frames - 1, @@ -369,8 +389,7 @@ export function ExtractMode({ username, settings }: Props) { const channel = new Channel(); channel.onmessage = async (buffer: ArrayBuffer) => { - if (cancelled || paused) return; - // Empty-frame sentinel from the producer = decode failed. + if (cancelled || paused || streamEpochRef.current !== myEpoch) return; if (buffer.byteLength <= 8) { setPlaybackError("decode failed during playback"); setPlaying(false); @@ -383,42 +402,47 @@ export function ExtractMode({ username, settings }: Props) { frameNumber = result.frameNumber; bitmap = result.bitmap; } catch (e) { - if (cancelled) return; + if (cancelled || streamEpochRef.current !== myEpoch) return; setPlaybackError(e instanceof Error ? e.message : String(e)); setPlaying(false); return; } - if (cancelled || paused) { - bitmap.close(); - return; - } - // Direction-aware out-of-order drop: forward play is monotonic, so - // any frame number < the last painted came from a parallel worker - // and is stale. Cheap defence against worker-pool races. - if (frameNumber < lastPainted) { + // Re-check epoch and cancel after the await: a play→pause→play in + // quick succession can land here with a bitmap from the prior session. + if ( + cancelled || + paused || + streamEpochRef.current !== myEpoch || + frameNumber < lastPainted + ) { bitmap.close(); return; } lastPainted = frameNumber; - rememberBitmap(frameNumber, bitmap); + // Hold exactly one bitmap at a time during stream playback — do NOT + // populate the LRU cache from here. + const prev = lastStreamBitmapRef.current; + lastStreamBitmapRef.current = bitmap; + if (prev && prev !== bitmap) prev.close(); drawBitmap(bitmap, frameNumber); + // Ref-only writes. The 10 Hz interval below syncs to React state. renderedIdx.current = frameNumber; requestedIdx.current = frameNumber; frameIdxRef.current = frameNumber; - setFrameIdx(frameNumber); decodeErrorRef.current = null; - // Review-mode stops AT the annotated frame, so the canvas already - // shows it by the time we ask the producer to halt. - if (reviewMode && annotatedFrames.current.has(frameNumber)) { + if (reviewModeRef.current && annotatedFrames.current.has(frameNumber)) { paused = true; try { await api.stopStream(); } catch { /* ignore */ } + // This IS a transition the user must see immediately, so flush + // state synchronously here. + setFrameIdx(frameNumber); setPlaying(false); } }; api - .startStream(channel, startFrom, speed, skip) + .startStream(channel, startFrom, speedRef.current, skip) .catch((e) => { if (cancelled) return; setPlaybackError(`start_stream failed: ${e}`); @@ -427,32 +451,45 @@ export function ExtractMode({ username, settings }: Props) { return () => { cancelled = true; - // Fire-and-forget: stop the producer, then re-fetch the last painted - // frame via the authoritative invoke path so the canvas matches - // `frameIdx` exactly (the producer may have been mid-flight). + streamEpochRef.current++; const lastFrame = frameIdxRef.current; - api - .stopStream() - .catch(() => { /* ignore */ }) - .then(() => api.decodeFrame(lastFrame)) - .then((buf) => sharedFrameDecoderPool.decode(buf)) - .then(({ frameNumber, bitmap }) => { - // Newer request may have started by the time this resolves — - // only paint if it still matches. - if (requestedIdx.current !== frameNumber) { - bitmap.close(); - return; - } - rememberBitmap(frameNumber, bitmap); - drawBitmap(bitmap, frameNumber); - renderedIdx.current = frameNumber; - }) - .catch(() => { /* invoke path will recover on next seek */ }); + // Flush the ref into state so the slider/readout aren't stale after + // pause. (The 10 Hz syncer below would catch up but this avoids the + // visible lag.) + setFrameIdx(lastFrame); + // Fire-and-forget stop. We don't do a re-fetch round-trip on every + // cleanup any more — the next user-initiated seek or step pulls a + // fresh frame via the pump (invoke) path. Cuts an IPC + worker + // decode off every pause. + api.stopStream().catch(() => { /* ignore */ }); + // Close the held stream bitmap so its GPU memory frees promptly. + const held = lastStreamBitmapRef.current; + if (held) { + held.close(); + lastStreamBitmapRef.current = null; + } }; - }, [playing, video, speed, reviewMode, drawBitmap, rememberBitmap]); + // Stable deps: speed / reviewMode / frameSkip are read via refs and + // applied to the running producer through setStreamState below. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [playing, video?.path]); - // Live-update the producer when speed or frame-skip changes mid-play, so - // the user doesn't have to pause + resume to see the new setting. + // 10 Hz state sync: copy frameIdxRef into React state during playback so + // the timeline slider and readout follow without forcing 30 reconciles + // per second. Stops when paused (state is then driven immediately by + // user actions like seek/step). + useEffect(() => { + if (!playing) return; + const id = window.setInterval(() => { + const cur = frameIdxRef.current; + setFrameIdx((prev) => (prev === cur ? prev : cur)); + }, 100); + return () => window.clearInterval(id); + }, [playing]); + + // Live-update the producer when speed or frame-skip changes mid-play — + // these go through setStreamState atomics so the producer thread doesn't + // need a restart. useEffect(() => { if (!playing) return; api.setStreamState({ speed }).catch(() => { /* ignore */ }); diff --git a/sam-tool-tauri/src/services/frameDecoderPool.ts b/sam-tool-tauri/src/services/frameDecoderPool.ts index 68a3cd2..e9f2014 100644 --- a/sam-tool-tauri/src/services/frameDecoderPool.ts +++ b/sam-tool-tauri/src/services/frameDecoderPool.ts @@ -1,20 +1,24 @@ -// Worker pool that takes a packed-frame ArrayBuffer (8-byte LE i64 frame -// number + JPEG bytes), decodes the JPEG to an ImageBitmap off the main -// thread, and returns both via transferable postMessage so the main thread -// only does the final drawImage. Inlined as a Blob URL so we don't need a -// separate build entry for the worker file. +// Worker that takes a packed-frame ArrayBuffer (8-byte LE i64 frame number + +// JPEG bytes), decodes the JPEG to an ImageBitmap off the main thread, and +// returns both via transferable postMessage so the main thread only does the +// final drawImage. Inlined as a Blob URL so we don't need a separate worker +// build entry. +// +// Two perf details worth keeping: +// * We do NOT copy the JPEG payload before constructing the Blob. A +// Uint8Array view onto the transferred buffer is enough — the Blob +// takes ownership of the bytes without an intermediate allocation, +// saving ~10 MB/s of GC pressure at 30 fps. +// * The worker is lazy-initialised on first decode. Mounting ExtractMode +// no longer pays a worker spin-up cost. const WORKER_SOURCE = ` self.onmessage = async (e) => { const { id, buffer } = e.data; let frameNumber = -1; try { - const headerView = new BigInt64Array(buffer, 0, 1); - frameNumber = Number(headerView[0]); - const jpegView = new Uint8Array(buffer, 8); - // Copy out of the transferred buffer so Blob ownership is independent. - const jpegCopy = jpegView.slice(); - const blob = new Blob([jpegCopy], { type: "image/jpeg" }); + frameNumber = Number(new BigInt64Array(buffer, 0, 1)[0]); + const blob = new Blob([new Uint8Array(buffer, 8)], { type: "image/jpeg" }); const bitmap = await createImageBitmap(blob); self.postMessage({ id, frameNumber, bitmap }, [bitmap]); } catch (err) { @@ -24,11 +28,6 @@ self.onmessage = async (e) => { }; `; -const workerBlob = new Blob([WORKER_SOURCE], { - type: "application/javascript", -}); -const workerUrl = URL.createObjectURL(workerBlob); - export interface DecodeResult { frameNumber: number; bitmap: ImageBitmap; @@ -47,62 +46,67 @@ interface WorkerMessage { } export class FrameDecoderPool { - private workers: Worker[] = []; - private next = 0; + private worker: Worker | null = null; + private workerUrl: string | null = null; private pending = new Map(); private nextId = 1; - constructor(size: number = 2) { - const count = Math.max(1, Math.min(size, 8)); - for (let i = 0; i < count; i++) { - const worker = new Worker(workerUrl); - worker.onmessage = (e: MessageEvent) => { - const { id, frameNumber, bitmap, error } = e.data; - const slot = this.pending.get(id); - if (!slot) { - // The caller discarded this request — close the bitmap so it - // doesn't leak (an ImageBitmap holds GPU memory). - bitmap?.close?.(); - return; - } + private ensureWorker(): Worker { + if (this.worker) return this.worker; + const blob = new Blob([WORKER_SOURCE], { type: "application/javascript" }); + this.workerUrl = URL.createObjectURL(blob); + const worker = new Worker(this.workerUrl); + worker.onmessage = (e: MessageEvent) => { + const { id, frameNumber, bitmap, error } = e.data; + const slot = this.pending.get(id); + if (!slot) { + // The caller discarded this request — close the bitmap so it + // doesn't leak (an ImageBitmap holds GPU memory). + bitmap?.close?.(); + return; + } + this.pending.delete(id); + if (error || !bitmap) { + slot.reject(new Error(error ?? "decode failed")); + } else { + slot.resolve({ frameNumber, bitmap }); + } + }; + worker.onerror = (e) => { + // Surface worker-thread errors to any pending request. We don't know + // which id failed, so reject the oldest. + const first = this.pending.entries().next().value; + if (first) { + const [id, slot] = first; this.pending.delete(id); - if (error || !bitmap) { - slot.reject(new Error(error ?? "decode failed")); - } else { - slot.resolve({ frameNumber, bitmap }); - } - }; - worker.onerror = (e) => { - // Surface worker-thread errors to any pending request on this - // worker. We don't know which id failed, so reject the oldest. - const first = this.pending.entries().next().value; - if (first) { - const [id, slot] = first; - this.pending.delete(id); - slot.reject(new Error(`worker error: ${e.message}`)); - } - }; - this.workers.push(worker); - } + slot.reject(new Error(`worker error: ${e.message}`)); + } + }; + this.worker = worker; + return worker; } /** - * Decode a packed-frame buffer. The buffer ownership transfers to the - * worker — do not read it after this call returns. + * Decode a packed-frame buffer. Ownership of the buffer transfers to the + * worker — do not read it on the main thread after this call returns. */ decode(buffer: ArrayBuffer): Promise { return new Promise((resolve, reject) => { const id = this.nextId++; this.pending.set(id, { resolve, reject }); - const worker = this.workers[this.next]; - this.next = (this.next + 1) % this.workers.length; - worker.postMessage({ id, buffer }, [buffer]); + this.ensureWorker().postMessage({ id, buffer }, [buffer]); }); } destroy() { - for (const w of this.workers) w.terminate(); - this.workers = []; + if (this.worker) { + this.worker.terminate(); + this.worker = null; + } + if (this.workerUrl) { + URL.revokeObjectURL(this.workerUrl); + this.workerUrl = null; + } for (const slot of this.pending.values()) { slot.reject(new Error("decoder pool destroyed")); } @@ -111,8 +115,8 @@ export class FrameDecoderPool { } /** - * Shared pool used by ExtractMode (and later, AnnotateMode). Two workers is - * enough for sequential playback decode; raising it helps the seek path - * when several scrub requests overlap. + * Shared single-worker pool used by ExtractMode (and later, AnnotateMode). + * Forward playback is sequential decode, so one worker is enough; running + * two only adds GC churn and risks frame reordering. */ -export const sharedFrameDecoderPool = new FrameDecoderPool(2); +export const sharedFrameDecoderPool = new FrameDecoderPool();