Skip to content

Commit 5b24c5f

Browse files
authored
fix(ThumbnailStrip): replace abortRef with run ID to eliminate concurrency race (magic-peach#893)
* fix(ffmpeg): guard buildAudioFilter against zero or negative speed When speed is zero or negative, the while loop dividing remaining by 0.5 never converges: 0 / 0.5 == 0, causing an infinite loop that blocks the browser thread entirely. Return an empty filter string immediately for any speed <= 0, which is consistent with the existing behavior of omitting atempo filters when no adjustment is needed. Closes magic-peach#862 * fix(ThumbnailStrip): replace abortRef with run ID to eliminate concurrency race The previous abortRef approach had a race window: the useEffect cleanup sets abortRef.current = true, but the new effect immediately invokes generateThumbnails which resets it to false. Any in-flight iteration from the prior run reads the newly-reset false and continues executing concurrently with the new run, causing duplicate offscreen video elements, interleaved thumbnail sequences, and DOM/memory leaks. Replace abortRef with a lastRunIdRef integer counter. Each invocation captures its own runId via ++lastRunIdRef.current. Every await checkpoint and every state-mutating code path checks lastRunIdRef.current === runId before proceeding. The cleanup function simply increments the counter, which immediately invalidates any in-flight run without a shared boolean that can be reset underneath it. Wrap the video element lifecycle in try/finally to guarantee video.src is cleared and offscreenVideoRef is released even when the function exits early (stale run detected, ctx unavailable, or unhandled rejection). Closes magic-peach#863
1 parent 309feda commit 5b24c5f

1 file changed

Lines changed: 74 additions & 58 deletions

File tree

src/components/ThumbnailStrip.tsx

Lines changed: 74 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export default function ThumbnailStrip({
3333
const [hoveredIndex, setHoveredIndex] = useState<number | null>(null);
3434
const stripRef = useRef<HTMLDivElement>(null);
3535
const offscreenVideoRef = useRef<HTMLVideoElement | null>(null);
36-
const abortRef = useRef(false);
36+
const lastRunIdRef = useRef(0);
3737
const objectUrlsRef = useRef<string[]>([]);
3838

3939
const effectiveTrimEnd = trimEnd ?? duration;
@@ -46,89 +46,105 @@ export default function ThumbnailStrip({
4646
const generateThumbnails = useCallback(async () => {
4747
if (!videoSrc || duration <= 0) return;
4848

49-
abortRef.current = false;
49+
const runId = ++lastRunIdRef.current;
5050
setIsGenerating(true);
5151
revokeAllObjectUrls();
5252
setThumbnails([]);
5353
setProgress(0);
5454

5555
const video = document.createElement("video");
56-
video.src = videoSrc;
57-
video.crossOrigin = "anonymous";
58-
video.muted = true;
59-
video.preload = "auto";
6056
offscreenVideoRef.current = video;
6157

62-
await new Promise<void>((resolve, reject) => {
63-
video.onloadedmetadata = () => resolve();
64-
video.onerror = () => reject(new Error("Video load failed"));
65-
video.load();
66-
});
58+
try {
59+
video.src = videoSrc;
60+
video.crossOrigin = "anonymous";
61+
video.muted = true;
62+
video.preload = "auto";
6763

68-
const canvas = document.createElement("canvas");
69-
const ctx = canvas.getContext("2d");
70-
if (!ctx) return;
64+
await new Promise<void>((resolve, reject) => {
65+
video.onloadedmetadata = () => resolve();
66+
video.onerror = () => reject(new Error("Video load failed"));
67+
video.load();
68+
});
7169

72-
const thumbW = 160;
73-
const thumbH = 90;
74-
canvas.width = thumbW;
75-
canvas.height = thumbH;
70+
if (lastRunIdRef.current !== runId) return;
7671

77-
const times: number[] = [];
78-
for (let t = 0; t <= duration; t += intervalSeconds) {
79-
times.push(Math.min(t, duration - 0.1));
80-
}
81-
if ((times[times.length - 1] ?? 0) < duration - 0.5) {
82-
times.push(duration - 0.1);
83-
}
72+
const canvas = document.createElement("canvas");
73+
const ctx = canvas.getContext("2d");
74+
if (!ctx) return;
8475

85-
const captured: Thumbnail[] = [];
76+
const thumbW = 160;
77+
const thumbH = 90;
78+
canvas.width = thumbW;
79+
canvas.height = thumbH;
8680

87-
for (let i = 0; i < times.length; i++) {
88-
if (abortRef.current) break;
81+
const times: number[] = [];
82+
for (let t = 0; t <= duration; t += intervalSeconds) {
83+
times.push(Math.min(t, duration - 0.1));
84+
}
85+
if ((times[times.length - 1] ?? 0) < duration - 0.5) {
86+
times.push(duration - 0.1);
87+
}
8988

90-
const time = times[i] ?? 0;
91-
await new Promise<void>((resolve) => {
92-
const onSeeked = async () => {
93-
video.removeEventListener("seeked", onSeeked);
94-
ctx.drawImage(video, 0, 0, thumbW, thumbH);
89+
const captured: Thumbnail[] = [];
9590

96-
try {
97-
const blob = await new Promise<Blob | null>((blobResolve) => {
98-
canvas.toBlob((b) => blobResolve(b), "image/jpeg", 0.7);
99-
});
100-
if (blob && !abortRef.current) {
101-
const url = URL.createObjectURL(blob);
102-
objectUrlsRef.current.push(url);
103-
captured.push({ time, dataUrl: url });
91+
for (let i = 0; i < times.length; i++) {
92+
if (lastRunIdRef.current !== runId) break;
93+
94+
const time = times[i] ?? 0;
95+
await new Promise<void>((resolve) => {
96+
const onSeeked = async () => {
97+
video.removeEventListener("seeked", onSeeked);
98+
99+
if (lastRunIdRef.current !== runId) {
100+
resolve();
101+
return;
102+
}
104103

105-
if (i === times.length - 1 || captured.length % 5 === 0) {
106-
setThumbnails([...captured]);
104+
ctx.drawImage(video, 0, 0, thumbW, thumbH);
105+
106+
try {
107+
const blob = await new Promise<Blob | null>((blobResolve) => {
108+
canvas.toBlob((b) => blobResolve(b), "image/jpeg", 0.7);
109+
});
110+
if (blob && lastRunIdRef.current === runId) {
111+
const url = URL.createObjectURL(blob);
112+
objectUrlsRef.current.push(url);
113+
captured.push({ time, dataUrl: url });
114+
115+
if (i === times.length - 1 || captured.length % 5 === 0) {
116+
setThumbnails([...captured]);
117+
}
107118
}
119+
} catch (err) {
120+
console.error("Failed to generate thumbnail blob", err);
108121
}
109-
} catch (err) {
110-
console.error("Failed to generate thumbnail blob", err);
111-
}
112-
113-
setProgress(Math.round(((i + 1) / times.length) * 100));
114-
resolve();
115-
};
116-
video.addEventListener("seeked", onSeeked);
117-
video.currentTime = time;
118-
});
119-
}
120122

121-
video.src = "";
122-
offscreenVideoRef.current = null;
123-
setIsGenerating(false);
123+
setProgress(Math.round(((i + 1) / times.length) * 100));
124+
resolve();
125+
};
126+
video.addEventListener("seeked", onSeeked);
127+
video.currentTime = time;
128+
});
129+
}
130+
131+
if (lastRunIdRef.current === runId) {
132+
setIsGenerating(false);
133+
}
134+
} finally {
135+
video.src = "";
136+
if (offscreenVideoRef.current === video) {
137+
offscreenVideoRef.current = null;
138+
}
139+
}
124140
}, [videoSrc, duration, intervalSeconds, revokeAllObjectUrls]);
125141

126142
useEffect(() => {
127143
if (videoSrc && duration > 0) {
128144
generateThumbnails();
129145
}
130146
return () => {
131-
abortRef.current = true;
147+
lastRunIdRef.current++;
132148
revokeAllObjectUrls();
133149
};
134150
}, [generateThumbnails, revokeAllObjectUrls, videoSrc, duration]);

0 commit comments

Comments
 (0)