Merge pull request #6280 from remotion-dev/bugfix/web-renderer-onprogress

Author: JonnyBurger

packages/web-renderer/src/render-media-on-web.tsx

@@ -279,6 +279,9 @@ const internalRenderMediaOnWeb = async <
 		target,
 	});
 
+	using throttledProgress = createThrottledProgressCallback(onProgress);
+	const throttledOnProgress = throttledProgress?.throttled ?? null;
+
 	try {
 		if (signal?.aborted) {
 			throw new Error('renderMediaOnWeb() was cancelled');
@@ -337,8 +340,6 @@ const internalRenderMediaOnWeb = async <
 			encodedFrames: 0,
 		};
 
-		const throttledOnProgress = createThrottledProgressCallback(onProgress);
-
 		for (let frame = realFrameRange[0]; frame <= realFrameRange[1]; frame++) {
 			if (signal?.aborted) {
 				throw new Error('renderMediaOnWeb() was cancelled');

packages/web-renderer/src/test/render-media.test.tsx

@@ -98,3 +98,43 @@ test('should throttle onProgress callback to 250ms', {retry: 2}, async (t) => {
 		}
 	}
 });
+
+test('should not fire stale progress callbacks after render completes', async (t) => {
+	if (t.task.file.projectName === 'webkit') {
+		t.skip();
+		return;
+	}
+
+	let renderCompleted = false;
+	let staleCallbackReceived = false;
+	let callbackCallCount = 0;
+
+	const Component: React.FC = () => null;
+
+	await renderMediaOnWeb({
+		composition: {
+			component: Component,
+			id: 'stale-progress-test',
+			width: 100,
+			height: 100,
+			fps: 30,
+			durationInFrames: 60,
+		},
+		inputProps: {},
+		onProgress: () => {
+			callbackCallCount++;
+			if (renderCompleted) {
+				staleCallbackReceived = true;
+			}
+		},
+		licenseKey: 'free-license',
+	});
+
+	renderCompleted = true;
+
+	// eslint-disable-next-line
+	await new Promise((resolve) => setTimeout(resolve, 500));
+
+	expect(staleCallbackReceived).toBe(false);
+	expect(callbackCallCount).toBeGreaterThan(0);
+});

packages/web-renderer/src/throttle-progress.ts

@@ -2,14 +2,19 @@ import type {RenderMediaOnWebProgressCallback} from './render-media-on-web';
 
 const DEFAULT_THROTTLE_MS = 250;
 
+export type ThrottledProgressResult = {
+	throttled: RenderMediaOnWebProgressCallback;
+	[Symbol.dispose]: () => void;
+};
+
 /**
  * Creates a throttled version of a progress callback that ensures it's not called
  * more frequently than the specified interval (default: 250ms)
  */
 export const createThrottledProgressCallback = (
 	callback: RenderMediaOnWebProgressCallback | null,
 	throttleMs: number = DEFAULT_THROTTLE_MS,
-): RenderMediaOnWebProgressCallback | null => {
+): ThrottledProgressResult | null => {
 	if (!callback) {
 		return null;
 	}
@@ -52,5 +57,14 @@ export const createThrottledProgressCallback = (
 		}
 	};
 
-	return throttled;
+	const cleanup = () => {
+		if (timeoutId !== null) {
+			clearTimeout(timeoutId);
+			timeoutId = null;
+		}
+
+		pendingUpdate = null;
+	};
+
+	return {throttled, [Symbol.dispose]: cleanup};
 };