Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/components/video-editor/VideoEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,16 @@ export default function VideoEditor() {
video.currentTime = time;
}

// Reads the live playhead position directly off the <video> element, bypassing
// `currentTime` React state entirely. The timeline's playhead subscribes to this
// via its own rAF loop so it stays in lockstep with actual playback regardless of
// how long this (large) component's own re-render takes — see issue #111.
const getPlaybackTimeMs = useCallback(() => {
const video = videoPlaybackRef.current?.video;
if (video) return video.currentTime * 1000;
return currentTimeRef.current * 1000;
}, []);

const handleSelectZoom = useCallback((id: string | null) => {
setSelectedZoomId(id);
if (id) {
Expand Down Expand Up @@ -3218,6 +3228,7 @@ export default function VideoEditor() {
<TimelineEditor
videoDuration={duration}
currentTime={currentTime}
getPlaybackTimeMs={getPlaybackTimeMs}
onSeek={handleSeek}
zoomRegions={zoomRegions}
onZoomAdded={handleZoomAdded}
Expand Down
40 changes: 21 additions & 19 deletions src/components/video-editor/VideoPlayback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1308,25 +1308,26 @@ const VideoPlayback = forwardRef<VideoPlaybackRef, VideoPlaybackProps>(
layoutVideoContentRef.current?.();
video.pause();

const { handlePlay, handlePause, handleSeeked, handleSeeking } = createVideoEventHandlers({
video,
isSeekingRef,
isPlayingRef,
allowPlaybackRef,
currentTimeRef,
timeUpdateAnimationRef,
onPlayStateChange: (playing) => onPlayStateChangeRef.current(playing),
onTimeUpdate: (time) => onTimeUpdateRef.current(time),
trimRegionsRef,
speedRegionsRef,
isScrubbingRef,
scrubEndTimerRef,
onScrubChange: (scrubbing) => setIsScrubbing(scrubbing),
seekSteppingRef,
stepVirtualSecRef,
stepLastTsRef,
stepPrevMutedRef,
});
const { handlePlay, handlePause, handleSeeked, handleSeeking, dispose } =
createVideoEventHandlers({
video,
isSeekingRef,
isPlayingRef,
allowPlaybackRef,
currentTimeRef,
timeUpdateAnimationRef,
onPlayStateChange: (playing) => onPlayStateChangeRef.current(playing),
onTimeUpdate: (time) => onTimeUpdateRef.current(time),
trimRegionsRef,
speedRegionsRef,
isScrubbingRef,
scrubEndTimerRef,
onScrubChange: (scrubbing) => setIsScrubbing(scrubbing),
seekSteppingRef,
stepVirtualSecRef,
stepLastTsRef,
stepPrevMutedRef,
});

video.addEventListener("play", handlePlay);
video.addEventListener("pause", handlePause);
Expand All @@ -1344,6 +1345,7 @@ const VideoPlayback = forwardRef<VideoPlaybackRef, VideoPlaybackProps>(
if (timeUpdateAnimationRef.current) {
cancelAnimationFrame(timeUpdateAnimationRef.current);
}
dispose();

if (videoSprite) {
videoContainer.removeChild(videoSprite);
Expand Down
49 changes: 47 additions & 2 deletions src/components/video-editor/timeline/TimelineEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ interface TimelineEditorProps {
videoDuration: number;
hasVideoSource?: boolean;
currentTime: number;
/**
* Reads the live playhead position (ms) directly, bypassing `currentTime` React
* state. The playhead subscribes to this via its own rAF loop so it stays in sync
* with actual playback regardless of how long an ancestor's re-render takes.
*/
getPlaybackTimeMs?: () => number;
onSeek?: (time: number) => void;
zoomRegions: ZoomRegion[];
onZoomAdded: (span: Span) => void;
Expand Down Expand Up @@ -284,13 +290,15 @@ function shouldStartTimelineScrub(target: EventTarget | null, timelineElement: H
function PlaybackCursor({
currentTimeMs,
videoDurationMs,
getPlaybackTimeMs,
onSeek,
onRangeChange,
timelineRef,
keyframes = [],
}: {
currentTimeMs: number;
videoDurationMs: number;
getPlaybackTimeMs?: () => number;
onSeek?: (time: number) => void;
onRangeChange?: (updater: (previous: Range) => Range) => void;
timelineRef: React.RefObject<HTMLDivElement>;
Expand All @@ -301,6 +309,38 @@ function PlaybackCursor({
const [isDragging, setIsDragging] = useState(false);
const [dragPreviewTimeMs, setDragPreviewTimeMs] = useState<number | null>(null);

// Drives the playhead position from the video element every animation frame,
// isolated from the rest of the tree: this component's own `useState` call only
// re-renders this small subtree, not TimelineEditor/VideoEditor above it. That
// decoupling is what keeps the playhead glued to actual playback even while an
// ancestor's (much heavier) re-render is in flight — see issue #111.
const [liveTimeMs, setLiveTimeMs] = useState(currentTimeMs);
const getPlaybackTimeMsRef = useRef(getPlaybackTimeMs);
useEffect(() => {
getPlaybackTimeMsRef.current = getPlaybackTimeMs;
}, [getPlaybackTimeMs]);

useEffect(() => {
if (!getPlaybackTimeMsRef.current) {
return;
}

let rafId: number;
const tick = () => {
const getter = getPlaybackTimeMsRef.current;
if (getter) {
const latest = getter();
setLiveTimeMs((previous) => (previous === latest ? previous : latest));
}
rafId = requestAnimationFrame(tick);
};
rafId = requestAnimationFrame(tick);

return () => cancelAnimationFrame(rafId);
}, []);

const resolvedTimeMs = getPlaybackTimeMs ? liveTimeMs : currentTimeMs;

useEffect(() => {
if (!isDragging) return;

Expand Down Expand Up @@ -399,7 +439,7 @@ function PlaybackCursor({
]);

const displayTimeMs =
isDragging && dragPreviewTimeMs !== null ? dragPreviewTimeMs : currentTimeMs;
isDragging && dragPreviewTimeMs !== null ? dragPreviewTimeMs : resolvedTimeMs;

if (videoDurationMs <= 0 || displayTimeMs < 0) {
return null;
Expand Down Expand Up @@ -428,7 +468,7 @@ function PlaybackCursor({
}}
onMouseDown={(e) => {
e.stopPropagation(); // Prevent timeline click
setDragPreviewTimeMs(currentTimeMs);
setDragPreviewTimeMs(resolvedTimeMs);
setIsDragging(true);
}}
>
Expand Down Expand Up @@ -571,6 +611,7 @@ function Timeline({
items,
videoDurationMs,
currentTimeMs,
getPlaybackTimeMs,
onSeek,
onRangeChange,
onSelectZoom,
Expand All @@ -592,6 +633,7 @@ function Timeline({
items: TimelineRenderItem[];
videoDurationMs: number;
currentTimeMs: number;
getPlaybackTimeMs?: () => number;
onSeek?: (time: number) => void;
onRangeChange?: (updater: (previous: Range) => Range) => void;
onSelectZoom?: (id: string | null) => void;
Expand Down Expand Up @@ -797,6 +839,7 @@ function Timeline({
<PlaybackCursor
currentTimeMs={currentTimeMs}
videoDurationMs={videoDurationMs}
getPlaybackTimeMs={getPlaybackTimeMs}
onSeek={onSeek}
onRangeChange={onRangeChange}
timelineRef={localTimelineRef}
Expand Down Expand Up @@ -934,6 +977,7 @@ export default function TimelineEditor({
videoDuration,
hasVideoSource = false,
currentTime,
getPlaybackTimeMs,
onSeek,
zoomRegions,
onZoomAdded,
Expand Down Expand Up @@ -1803,6 +1847,7 @@ export default function TimelineEditor({
items={timelineItems}
videoDurationMs={totalMs}
currentTimeMs={currentTimeMs}
getPlaybackTimeMs={getPlaybackTimeMs}
onSeek={onSeek}
onRangeChange={setRange}
onSelectZoom={onSelectZoom}
Expand Down
97 changes: 97 additions & 0 deletions src/components/video-editor/videoPlayback/rafCoalescer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createRafCoalescer } from "./rafCoalescer";

describe("createRafCoalescer", () => {
let rafCallbacks: FrameRequestCallback[];
let nextRafId: number;

beforeEach(() => {
rafCallbacks = [];
nextRafId = 1;
vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => {
rafCallbacks.push(cb);
return nextRafId++;
});
vi.stubGlobal("cancelAnimationFrame", (id: number) => {
// Mark the callback as a no-op instead of removing it, to mirror the
// browser's index-stable semantics.
const index = id - 1;
if (rafCallbacks[index]) {
rafCallbacks[index] = () => {
// Cancelled: no-op.
};
}
});
});

afterEach(() => {
vi.unstubAllGlobals();
});

function flushOneFrame(timeMs = 0) {
const callbacks = rafCallbacks;
rafCallbacks = [];
for (const cb of callbacks) cb(timeMs);
}

it("does not call flush until an animation frame runs", () => {
const flush = vi.fn();
const coalescer = createRafCoalescer<number>(flush);

coalescer.schedule(1);

expect(flush).not.toHaveBeenCalled();
});

it("collapses multiple schedule calls within the same frame into a single flush", () => {
const flush = vi.fn();
const coalescer = createRafCoalescer<number>(flush);

coalescer.schedule(1);
coalescer.schedule(2);
coalescer.schedule(3);

flushOneFrame();

expect(flush).toHaveBeenCalledTimes(1);
expect(flush).toHaveBeenCalledWith(3);
});

it("schedules a fresh frame for values reported after the previous frame flushed", () => {
const flush = vi.fn();
const coalescer = createRafCoalescer<number>(flush);

coalescer.schedule(1);
flushOneFrame();
coalescer.schedule(2);
flushOneFrame();

expect(flush).toHaveBeenCalledTimes(2);
expect(flush).toHaveBeenNthCalledWith(1, 1);
expect(flush).toHaveBeenNthCalledWith(2, 2);
});

it("cancel() prevents a pending flush from firing", () => {
const flush = vi.fn();
const coalescer = createRafCoalescer<number>(flush);

coalescer.schedule(1);
coalescer.cancel();
flushOneFrame();

expect(flush).not.toHaveBeenCalled();
});

it("a schedule() call after cancel() still flushes on the next frame", () => {
const flush = vi.fn();
const coalescer = createRafCoalescer<number>(flush);

coalescer.schedule(1);
coalescer.cancel();
coalescer.schedule(2);
flushOneFrame();

expect(flush).toHaveBeenCalledTimes(1);
expect(flush).toHaveBeenCalledWith(2);
});
});
41 changes: 41 additions & 0 deletions src/components/video-editor/videoPlayback/rafCoalescer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Coalesces rapid-fire calls into at most one flush per animation frame.
*
* Used so a ref-backed value can be updated synchronously on every call (cheap),
* while an expensive side effect — e.g. a React state commit — is deferred to
* once per frame. Without this, a burst of calls within the same frame (many
* `seeking` events fired while dragging the timeline playhead) would otherwise
* force one state commit per call, saturating the main thread and making both
* the drag and the playhead's own rendering feel laggy.
*/
export function createRafCoalescer<T>(flush: (value: T) => void) {
let pendingValue: T | undefined;
let hasPending = false;
let rafId: number | null = null;

const schedule = (value: T) => {
pendingValue = value;
hasPending = true;
if (rafId !== null) return;
rafId = requestAnimationFrame(() => {
rafId = null;
if (hasPending) {
hasPending = false;
const valueToFlush = pendingValue as T;
pendingValue = undefined;
flush(valueToFlush);
}
});
};

const cancel = () => {
if (rafId !== null) {
cancelAnimationFrame(rafId);
rafId = null;
}
hasPending = false;
pendingValue = undefined;
};

return { schedule, cancel };
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type React from "react";
import { MAX_NATIVE_PLAYBACK_RATE, type SpeedRegion, type TrimRegion } from "../types";
import { createRafCoalescer } from "./rafCoalescer";

// Keep "scrub mode" on for a brief tail after `seeked`: rapid drag-scrubbing fires
// `seeking`/`seeked` dozens of times a second and toggling effects each time would flicker.
Expand Down Expand Up @@ -77,9 +78,16 @@ export function createVideoEventHandlers(params: VideoEventHandlersParams) {
}
};

// currentTimeRef is updated synchronously on every call (cheap; other imperative
// consumers like the Pixi renderer read it directly). The React state commit
// (`onTimeUpdate`) is coalesced to at most once per animation frame so a burst of
// `seeking` events (fast timeline drag) or the per-frame rAF playback loop can't
// force more than one parent re-render per frame.
const timeUpdateCoalescer = createRafCoalescer<number>(onTimeUpdate);

const emitTime = (timeValue: number) => {
currentTimeRef.current = timeValue * 1000;
onTimeUpdate(timeValue);
timeUpdateCoalescer.schedule(timeValue);
};

const findActiveTrimRegion = (currentTimeMs: number): TrimRegion | null => {
Expand Down Expand Up @@ -331,5 +339,6 @@ export function createVideoEventHandlers(params: VideoEventHandlersParams) {
handlePause,
handleSeeked,
handleSeeking,
dispose: () => timeUpdateCoalescer.cancel(),
};
}
Loading