diff --git a/apps/web/__tests__/unit/video-frame-thumbnail.test.ts b/apps/web/__tests__/unit/video-frame-thumbnail.test.ts index 8644b8c7138..6d2264bda35 100644 --- a/apps/web/__tests__/unit/video-frame-thumbnail.test.ts +++ b/apps/web/__tests__/unit/video-frame-thumbnail.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { captureVideoFrameDataUrl } from "@/app/s/[videoId]/_components/video-frame-thumbnail"; +import { + captureVideoFrameDataUrl, + MAX_LIVE_THUMBNAIL_DRIFT_SECONDS, + shouldCaptureLiveThumbnail, +} from "@/app/s/[videoId]/_components/video-frame-thumbnail"; function createMockCanvas(options?: { contextReturnsNull?: boolean; @@ -189,3 +193,86 @@ describe("captureVideoFrameDataUrl", () => { expect(factory).toHaveBeenCalledTimes(1); }); }); + +describe("shouldCaptureLiveThumbnail", () => { + it("exports default drift threshold of 1.5 seconds", () => { + expect(MAX_LIVE_THUMBNAIL_DRIFT_SECONDS).toBe(1.5); + }); + + it("returns true when hoverTime is within drift threshold of currentTime", () => { + expect(shouldCaptureLiveThumbnail({ hoverTime: 10, currentTime: 10 })).toBe( + true, + ); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 11.2, currentTime: 10 }), + ).toBe(true); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 8.8, currentTime: 10 }), + ).toBe(true); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 11.5, currentTime: 10 }), + ).toBe(true); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 8.5, currentTime: 10 }), + ).toBe(true); + }); + + it("returns false when hoverTime drifts beyond drift threshold", () => { + expect( + shouldCaptureLiveThumbnail({ hoverTime: 11.51, currentTime: 10 }), + ).toBe(false); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 8.49, currentTime: 10 }), + ).toBe(false); + expect( + shouldCaptureLiveThumbnail({ hoverTime: 200, currentTime: 10 }), + ).toBe(false); + expect(shouldCaptureLiveThumbnail({ hoverTime: 0, currentTime: 10 })).toBe( + false, + ); + }); + + it("supports custom maxDriftSeconds", () => { + expect( + shouldCaptureLiveThumbnail({ + hoverTime: 13, + currentTime: 10, + maxDriftSeconds: 3, + }), + ).toBe(true); + expect( + shouldCaptureLiveThumbnail({ + hoverTime: 13.1, + currentTime: 10, + maxDriftSeconds: 3, + }), + ).toBe(false); + }); + + it("returns false for non-finite values", () => { + expect( + shouldCaptureLiveThumbnail({ + hoverTime: Number.NaN, + currentTime: 10, + }), + ).toBe(false); + expect( + shouldCaptureLiveThumbnail({ + hoverTime: 10, + currentTime: Number.NaN, + }), + ).toBe(false); + expect( + shouldCaptureLiveThumbnail({ + hoverTime: Number.POSITIVE_INFINITY, + currentTime: 10, + }), + ).toBe(false); + expect( + shouldCaptureLiveThumbnail({ + hoverTime: 10, + currentTime: Number.NEGATIVE_INFINITY, + }), + ).toBe(false); + }); +}); diff --git a/apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx b/apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx index cd8c6ccba2b..c5df67195b2 100644 --- a/apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx +++ b/apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx @@ -59,7 +59,10 @@ import { MediaPlayerVolumeIndicator, } from "./video/media-player"; import { Tooltip, TooltipContent, TooltipTrigger } from "./video/tooltip"; -import { captureVideoFrameDataUrl } from "./video-frame-thumbnail"; +import { + captureVideoFrameDataUrl, + shouldCaptureLiveThumbnail, +} from "./video-frame-thumbnail"; // Mounted only mid-upload; its RPC client drags the Effect runtime along, so // keeping it behind a dynamic import keeps that chunk off finished videos. @@ -629,14 +632,24 @@ export function CapVideoPlayer({ // The seek tooltip asks for a thumbnail on every render while the pointer is // over the bar (per animation frame). The capture snapshots the *displayed* - // frame, so re-encoding is pure waste until the video has actually moved; - // caching by currentTime (with a small tolerance while playing) bounds the - // canvas + JPEG work to a few captures per second at most. + // frame, so it is only accurate when hovering near the current playback position. + // For distant timestamps, returning undefined lets MediaPlayerSeek render the + // chapter title and timestamp without showing a misleading stale snapshot. + // Re-encoding is pure waste until the video has actually moved; caching by + // currentTime bounds the canvas + JPEG work to a few captures per second at most. const thumbnailCacheRef = useRef<{ time: number; url: string } | null>(null); const generateVideoFrameThumbnail = useCallback( - (_time: number): string | undefined => { + (time: number): string | undefined => { const video = videoRef.current; if (!video) return undefined; + if ( + !shouldCaptureLiveThumbnail({ + hoverTime: time, + currentTime: video.currentTime, + }) + ) { + return undefined; + } const cached = thumbnailCacheRef.current; if (cached && Math.abs(cached.time - video.currentTime) < 0.25) { return cached.url; diff --git a/apps/web/app/s/[videoId]/_components/video-frame-thumbnail.ts b/apps/web/app/s/[videoId]/_components/video-frame-thumbnail.ts index 886eae01628..05bc1ea8454 100644 --- a/apps/web/app/s/[videoId]/_components/video-frame-thumbnail.ts +++ b/apps/web/app/s/[videoId]/_components/video-frame-thumbnail.ts @@ -6,6 +6,31 @@ export interface CaptureVideoFrameDeps { quality?: number; } +export const MAX_LIVE_THUMBNAIL_DRIFT_SECONDS = 1.5; + +export interface ShouldCaptureLiveThumbnailParams { + hoverTime: number; + currentTime: number; + maxDriftSeconds?: number; +} + +/** + * Determines whether a live video element snapshot accurately reflects the hovered timeline time. + * Because HTMLVideoElement only holds decoded frame data for its active currentTime, + * snapshotting the element is only valid when hovering within close proximity of playback. + * Returning false for distant timestamps prevents displaying misleading frame snapshots. + */ +export function shouldCaptureLiveThumbnail({ + hoverTime, + currentTime, + maxDriftSeconds = MAX_LIVE_THUMBNAIL_DRIFT_SECONDS, +}: ShouldCaptureLiveThumbnailParams): boolean { + if (!Number.isFinite(hoverTime) || !Number.isFinite(currentTime)) { + return false; + } + return Math.abs(hoverTime - currentTime) <= maxDriftSeconds; +} + export function captureVideoFrameDataUrl( deps: CaptureVideoFrameDeps, ): string | undefined {