web-sdui: add frame-by-frame VideoEncoder export using WebCodecs#45
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds browser-side video export using the WebCodecs VideoEncoder: the app captures preview frames via a ref, encodes them as VP8 into EncodedVideoChunks, assembles an IVF file, and provides a UI button to trigger download with encoding status. ChangesVideoEncoder Integration
Sequence DiagramsequenceDiagram
participant UI_Button as "Generate Video Button"
participant App as "App.handleGenerateVideo"
participant FrameWaiter as "frameWaiterRef"
participant Preview as "previewRef.current (DOM)"
participant CanvasRenderer as "renderNodeToCanvas"
participant VideoEncoder as "VideoEncoder (VP8)"
participant IVFBuilder as "IVF assembler"
participant Downloader as "downloadIVF"
UI_Button->>App: click -> handleGenerateVideo()
App->>FrameWaiter: setFrameAndWaitForRender(frame)
FrameWaiter->>Preview: wait until currentFrame rendered
App->>CanvasRenderer: renderNodeToCanvas(previewRef.current)
CanvasRenderer-->>App: canvas frame
App->>VideoEncoder: encode(canvas frame) (keyframe cadence)
VideoEncoder-->>App: encoded chunks (EncodedVideoChunk)
App->>IVFBuilder: appendEncodedChunk(chunk, timestamp)
App->>VideoEncoder: flush()
App->>IVFBuilder: finalize IVF
App->>Downloader: downloadIVF(ivfBlob)
Downloader-->>UI_Button: saved video.ivf
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adds WebCodecs-based video export functionality to the SDUI preview. The implementation successfully introduces frame-by-frame encoding using VideoEncoder with VP8 codec and IVF container format.
Critical Issues Identified (5)
Memory Leaks & Resource Management:
- Blob URLs not properly released in error paths of
renderNodeToCanvas - Missing cleanup when canvas context creation fails
Crash Prevention:
- Encoder error callback doesn't halt the encoding loop, causing continued operations on failed encoder
encoder.close()in finally block may throw if encoder is in error state- Missing error state checks before encode operations
All identified issues have been marked with specific code suggestions to fix the defects. Please address these critical issues before merging to prevent memory leaks and runtime crashes during video generation.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| const encoder = new VideoEncoder({ | ||
| output: (chunk) => chunks.push(chunk), | ||
| error: (error) => setEncodeStatus(`Encoding error: ${error.message}`) | ||
| }); |
There was a problem hiding this comment.
🛑 Crash Risk: The encoder error callback only updates UI state but doesn't halt the encoding loop. When an encoding error occurs, the for-loop at line 327 continues encoding frames with a failed encoder, which will throw exceptions. Add error tracking and break the loop when encoder errors occur.
| const encoder = new VideoEncoder({ | |
| output: (chunk) => chunks.push(chunk), | |
| error: (error) => setEncodeStatus(`Encoding error: ${error.message}`) | |
| }); | |
| let encodingError: Error | null = null; | |
| const encoder = new VideoEncoder({ | |
| output: (chunk) => chunks.push(chunk), | |
| error: (error) => { | |
| encodingError = error; | |
| setEncodeStatus(`Encoding error: ${error.message}`); | |
| } | |
| }); |
| await new Promise<void>((resolve, reject) => { | ||
| img.onload = () => resolve(); | ||
| img.onerror = () => reject(new Error('Failed to render frame.')); | ||
| img.src = url; | ||
| }); |
There was a problem hiding this comment.
🛑 Memory Leak: The blob URL is not revoked when the image fails to load, causing a memory leak. If img.onerror triggers at line 257, execution jumps to the rejection path without calling URL.revokeObjectURL(url), leaving the blob URL unreleased in memory.
| await new Promise<void>((resolve, reject) => { | |
| img.onload = () => resolve(); | |
| img.onerror = () => reject(new Error('Failed to render frame.')); | |
| img.src = url; | |
| }); | |
| await new Promise<void>((resolve, reject) => { | |
| img.onload = () => { | |
| URL.revokeObjectURL(url); | |
| resolve(); | |
| }; | |
| img.onerror = () => { | |
| URL.revokeObjectURL(url); | |
| reject(new Error('Failed to render frame.')); | |
| }; | |
| img.src = url; | |
| }); |
| const ctx = canvas.getContext('2d'); | ||
| if (!ctx) { | ||
| URL.revokeObjectURL(url); | ||
| throw new Error('Could not create canvas context.'); | ||
| } | ||
| ctx.clearRect(0, 0, width, height); | ||
| ctx.drawImage(img, 0, 0, width, height); | ||
| URL.revokeObjectURL(url); |
There was a problem hiding this comment.
🛑 Logic Error: The URL revocation at line 267 executes before the canvas drawing completes when ctx is null, but the error is thrown after. Move URL revocation into the finally block or after the drawImage call to ensure it always executes, preventing the memory leak when context creation fails.
| const ctx = canvas.getContext('2d'); | |
| if (!ctx) { | |
| URL.revokeObjectURL(url); | |
| throw new Error('Could not create canvas context.'); | |
| } | |
| ctx.clearRect(0, 0, width, height); | |
| ctx.drawImage(img, 0, 0, width, height); | |
| URL.revokeObjectURL(url); | |
| const ctx = canvas.getContext('2d'); | |
| if (!ctx) { | |
| throw new Error('Could not create canvas context.'); | |
| } | |
| ctx.clearRect(0, 0, width, height); | |
| ctx.drawImage(img, 0, 0, width, height); |
| for (let frame = 0; frame <= maxFrames; frame += 1) { | ||
| setCurrentFrame(frame); | ||
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | ||
| await waitForNextPaint(); | ||
| await renderNodeToCanvas(previewRef.current, exportCanvas, width, height); | ||
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | ||
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | ||
| videoFrame.close(); | ||
| } |
There was a problem hiding this comment.
Check encodingError before each encode operation to prevent encoding with a failed encoder. Without this check, the loop continues after encoder failures, causing crashes.
| for (let frame = 0; frame <= maxFrames; frame += 1) { | |
| setCurrentFrame(frame); | |
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | |
| await waitForNextPaint(); | |
| await renderNodeToCanvas(previewRef.current, exportCanvas, width, height); | |
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | |
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | |
| videoFrame.close(); | |
| } | |
| for (let frame = 0; frame <= maxFrames; frame += 1) { | |
| if (encodingError) { | |
| throw encodingError; | |
| } | |
| setCurrentFrame(frame); | |
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | |
| await waitForNextPaint(); | |
| await renderNodeToCanvas(previewRef.current, exportCanvas, width, height); | |
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | |
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | |
| videoFrame.close(); | |
| } |
| } finally { | ||
| encoder.close(); | ||
| setIsEncoding(false); | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Calling encoder.close() may throw if the encoder is already in an error state or closed. Wrap in a try-catch to prevent unhandled exceptions during cleanup.
| } finally { | |
| encoder.close(); | |
| setIsEncoding(false); | |
| } | |
| } finally { | |
| try { | |
| encoder.close(); | |
| } catch (closeError) { | |
| // Ignore close errors as encoder may already be closed or in error state | |
| } | |
| setIsEncoding(false); | |
| } |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces video export functionality using the Web VideoEncoder API, allowing users to generate and download IVF video files from the UI preview. The implementation includes a frame-by-frame rendering pipeline from HTML to canvas and a custom IVF packager. Feedback identifies significant limitations in the current rasterization method regarding CSS and external resource capture, a bug in the IVF timestamp calculation that affects playback timing, and a suggestion to dynamically scale the video bitrate based on configuration.
| const serialized = new XMLSerializer().serializeToString(node); | ||
| const svg = ` | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="${width}" height="${height}"> | ||
| <foreignObject x="0" y="0" width="100%" height="100%">${serialized}</foreignObject> | ||
| </svg> | ||
| `; |
There was a problem hiding this comment.
The current rasterization method using XMLSerializer and foreignObject has significant limitations that will likely result in the exported video looking different from the preview:
- Styles: External stylesheets and
<style>tags in the document head are not captured byserializeToString(node). Only inline styles or styles within the node are preserved. This will cause the rendered frames to lose most of their CSS. - External Resources: When an SVG is loaded into an
Imageobject (as done on line 258), it is strictly sandboxed. It cannot load external resources like remote images (e.g., the YouTube avatar indefaultSDUI) or web fonts. These will be missing in the output.
To improve this, you should inject the document's styles into the SVG and ensure all images are converted to Data URLs before serialization.
| const frameHeader = new ArrayBuffer(12); | ||
| const fh = new DataView(frameHeader); | ||
| fh.setUint32(0, chunk.byteLength, true); | ||
| const timestamp = BigInt(Math.round(chunk.timestamp / 1000)); |
There was a problem hiding this comment.
The IVF frame timestamp calculation is inconsistent with the file header. The header (lines 279-280) defines the timebase as 1 / fps seconds per unit. Therefore, the timestamp for each frame should be its integer index (0, 1, 2...). Currently, it is calculated as milliseconds, which will cause players to interpret the video timing incorrectly.
| const timestamp = BigInt(Math.round(chunk.timestamp / 1000)); | |
| const timestamp = BigInt(Math.round((chunk.timestamp * fps) / 1000000)); |
| error: (error) => setEncodeStatus(`Encoding error: ${error.message}`) | ||
| }); | ||
| try { | ||
| encoder.configure({ codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps }); |
There was a problem hiding this comment.
The bitrate is currently hardcoded to 4Mbps. For varying resolutions and frame rates, it would be better to scale this based on the outputQuality configuration or the video dimensions to ensure consistent visual quality.
| encoder.configure({ codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps }); | |
| encoder.configure({ codec: 'vp8', width, height, bitrate: (sdui?.config?.outputQuality || 100) * 50000, framerate: fps }); |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
sdui | c02873a | Commit Preview URL Branch Preview URL |
May 13 2026, 09:49 AM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lyrics | c02873a | Commit Preview URL Branch Preview URL |
May 13 2026, 09:50 AM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-sdui/src/App.tsx`:
- Around line 270-301: The downloadIvf helper writes IVF frame pts using
chunk.timestamp/1000 (ms) which mismatches the header timebase (1/fps), so
change pts to the frame index (e.g., use an incrementing frame counter or
compute Math.round(frameIndex) where header fps is used) so pts counts frames in
units of 1/fps; also ensure you write pts as a 64-bit little-endian split into
low/high via fh.setUint32(4, low, true) and fh.setUint32(8, high, true) as done
now. For the flaky download, append the created anchor element to document.body
before calling a.click(), and defer URL.revokeObjectURL(a.href) (e.g., via
setTimeout or queueMicrotask) after the click and then remove the anchor from
the DOM; update symbols: downloadIvf, header/fh/frameHeader, a.href, a.click(),
URL.revokeObjectURL to implement these changes.
- Around line 303-345: handleGenerateVideo currently ignores encoder runtime
errors and overwrites them during the frame loop; add a boolean flag (e.g.,
encoderError) scoped near the VideoEncoder construction and set it inside the
encoder.error callback (use error: unknown and narrow with instanceof Error) so
the loop checks that flag after each encode and breaks early if set, preventing
further encoder.encode calls; before calling encoder.configure call await
VideoEncoder.isConfigSupported({ codec: 'vp8', width, height, framerate: fps })
and throw/set status if unsupported; also replace catch(error: any) with
catch(error: unknown) and narrow to extract a safe message for setEncodeStatus.
- Around line 244-268: renderNodeToCanvas currently serializes a DOM into an SVG
<foreignObject> and draws it via an <img>, which taints the canvas (causing
VideoFrame construction to throw SecurityError) and also misses external
styles/fonts/images; fix by inlining external resources before serialization:
convert all <img> srcs (e.g., yt3.googleusercontent.com) to data: URLs or remove
them, copy computed styles into the cloned DOM's style attributes, embed
webfonts/stylesheets as <style> blocks, and set img.crossOrigin='anonymous'
where appropriate; alternatively replace renderNodeToCanvas with a proven
library (e.g., html-to-image or dom-to-image-more) that handles resource
inlining, or change the export path to render directly to a
canvas/OffscreenCanvas/captureStream to avoid SVG foreignObject—ensure the
VideoFrame creation uses a guaranteed origin-clean canvas.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 002360fa-6ab0-46d2-98bf-5b0b6d11566d
📒 Files selected for processing (2)
web-sdui/src/App.tsxweb-sdui/src/components/MotionCanvas.tsx
| const downloadIvf = (chunks: EncodedVideoChunk[], width: number, height: number, fps: number) => { | ||
| const fourcc = 'VP80'; | ||
| const frameCount = chunks.length; | ||
| const header = new ArrayBuffer(32); | ||
| const dv = new DataView(header); | ||
| dv.setUint8(0, 'D'.charCodeAt(0)); dv.setUint8(1, 'K'.charCodeAt(0)); dv.setUint8(2, 'I'.charCodeAt(0)); dv.setUint8(3, 'F'.charCodeAt(0)); | ||
| dv.setUint16(4, 0, true); dv.setUint16(6, 32, true); | ||
| for (let i = 0; i < 4; i += 1) dv.setUint8(8 + i, fourcc.charCodeAt(i)); | ||
| dv.setUint16(12, width, true); dv.setUint16(14, height, true); | ||
| dv.setUint32(16, fps, true); dv.setUint32(20, 1, true); | ||
| dv.setUint32(24, frameCount, true); dv.setUint32(28, 0, true); | ||
|
|
||
| const parts: BlobPart[] = [header]; | ||
| for (const chunk of chunks) { | ||
| const frameHeader = new ArrayBuffer(12); | ||
| const fh = new DataView(frameHeader); | ||
| fh.setUint32(0, chunk.byteLength, true); | ||
| const timestamp = BigInt(Math.round(chunk.timestamp / 1000)); | ||
| fh.setUint32(4, Number(timestamp & BigInt(0xffffffff)), true); | ||
| fh.setUint32(8, Number((timestamp >> BigInt(32)) & BigInt(0xffffffff)), true); | ||
| const chunkData = new Uint8Array(chunk.byteLength); | ||
| chunk.copyTo(chunkData); | ||
| parts.push(frameHeader, chunkData); | ||
| } | ||
|
|
||
| const blob = new Blob(parts, { type: 'video/x-ivf' }); | ||
| const a = document.createElement('a'); | ||
| a.href = URL.createObjectURL(blob); | ||
| a.download = `motion-${Date.now()}.ivf`; | ||
| a.click(); | ||
| URL.revokeObjectURL(a.href); | ||
| }; |
There was a problem hiding this comment.
IVF frame pts doesn't match the declared timebase; download trigger can also be flaky.
Two issues in this helper:
-
Timebase/pts mismatch. The header declares timebase = framerate_den/framerate_num =
1/fps, so each frame's pts is measured in units of1/fpsseconds. For a 24 fps stream, frame N should havepts = N(decodes to N/24 s). The code instead writesround(chunk.timestamp / 1000)— i.e. milliseconds — so frame N gets pts ≈N * 1000 / fps. With timebase 1/24 that decodes to≈ N * 41.7 / 24 ≈ N * 1.74seconds per frame, ~42× too slow in any player that respects pts (ffmpeg, pion's ivfreader, etc.). Some players ignore pts and fall back to the header framerate, which is why a playable file may still come out — but the file is not a valid 24 fps stream.Pick one consistent scheme, e.g. keep timebase =
1/fpsand write the frame index as pts:♻️ Suggested fix
- const parts: BlobPart[] = [header]; - for (const chunk of chunks) { + const parts: BlobPart[] = [header]; + chunks.forEach((chunk, index) => { const frameHeader = new ArrayBuffer(12); const fh = new DataView(frameHeader); fh.setUint32(0, chunk.byteLength, true); - const timestamp = BigInt(Math.round(chunk.timestamp / 1000)); + // pts is in units of timebase (den/num = 1/fps) → frame index. + const timestamp = BigInt(index); fh.setUint32(4, Number(timestamp & BigInt(0xffffffff)), true); fh.setUint32(8, Number((timestamp >> BigInt(32)) & BigInt(0xffffffff)), true); const chunkData = new Uint8Array(chunk.byteLength); chunk.copyTo(chunkData); parts.push(frameHeader, chunkData); - } + });
-
Download anchor isn't in the DOM and URL is revoked too eagerly.
a.click()on a detached anchor is not reliable across browsers (notably Firefox), and callingURL.revokeObjectURL(a.href)synchronously afterclick()can interrupt the download before the browser has read the blob. Append the anchor, click it, then revoke on a microtask/timeout:♻️ Suggested fix
- const blob = new Blob(parts, { type: 'video/x-ivf' }); - const a = document.createElement('a'); - a.href = URL.createObjectURL(blob); - a.download = `motion-${Date.now()}.ivf`; - a.click(); - URL.revokeObjectURL(a.href); + const blob = new Blob(parts, { type: 'video/x-ivf' }); + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = `motion-${Date.now()}.ivf`; + document.body.appendChild(a); + a.click(); + a.remove(); + setTimeout(() => URL.revokeObjectURL(url), 0);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-sdui/src/App.tsx` around lines 270 - 301, The downloadIvf helper writes
IVF frame pts using chunk.timestamp/1000 (ms) which mismatches the header
timebase (1/fps), so change pts to the frame index (e.g., use an incrementing
frame counter or compute Math.round(frameIndex) where header fps is used) so pts
counts frames in units of 1/fps; also ensure you write pts as a 64-bit
little-endian split into low/high via fh.setUint32(4, low, true) and
fh.setUint32(8, high, true) as done now. For the flaky download, append the
created anchor element to document.body before calling a.click(), and defer
URL.revokeObjectURL(a.href) (e.g., via setTimeout or queueMicrotask) after the
click and then remove the anchor from the DOM; update symbols: downloadIvf,
header/fh/frameHeader, a.href, a.click(), URL.revokeObjectURL to implement these
changes.
| const handleGenerateVideo = async () => { | ||
| if (!('VideoEncoder' in window)) { | ||
| setEncodeStatus('VideoEncoder is not supported in this browser.'); | ||
| return; | ||
| } | ||
| if (!previewRef.current) { | ||
| setEncodeStatus('Preview element is not available.'); | ||
| return; | ||
| } | ||
| setIsEncoding(true); | ||
| setEncodeStatus('Starting encoding...'); | ||
| const fps = sdui?.config?.fps || 24; | ||
| const width = sdui?.config?.aspectRatio?.width || 480; | ||
| const height = sdui?.config?.aspectRatio?.height || 854; | ||
| const exportCanvas = document.createElement('canvas'); | ||
| exportCanvas.width = width; | ||
| exportCanvas.height = height; | ||
| const chunks: EncodedVideoChunk[] = []; | ||
| const encoder = new VideoEncoder({ | ||
| output: (chunk) => chunks.push(chunk), | ||
| error: (error) => setEncodeStatus(`Encoding error: ${error.message}`) | ||
| }); | ||
| try { | ||
| encoder.configure({ codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps }); | ||
| for (let frame = 0; frame <= maxFrames; frame += 1) { | ||
| setCurrentFrame(frame); | ||
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | ||
| await waitForNextPaint(); | ||
| await renderNodeToCanvas(previewRef.current, exportCanvas, width, height); | ||
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | ||
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | ||
| videoFrame.close(); | ||
| } | ||
| await encoder.flush(); | ||
| downloadIvf(chunks, width, height, fps); | ||
| setEncodeStatus(`Done. Downloaded ${chunks.length} encoded frames as IVF.`); | ||
| } catch (error: any) { | ||
| setEncodeStatus(`Failed: ${error?.message || 'Unknown error'}`); | ||
| } finally { | ||
| encoder.close(); | ||
| setIsEncoding(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WebCodecs VideoEncoder isConfigSupported vp8 error callback state
💡 Result:
WebCodecs VideoEncoder + VP8 + “isConfigSupported” error callback state: 1) What “isConfigSupported” does (and what it doesn’t) - VideoEncoder.isConfigSupported(config) checks whether the browser can successfully configure the encoder with the given config, and it resolves to { supported, config } when the config is structurally valid; it throws TypeError only if the provided config is invalid (e.g., empty codec, negative width). [1][2] - If you get supported === false, this indicates the config is valid but not supported; in that case you should not proceed to configure/encode with it. (The API is intended as a best-effort pre-check.) [1][3] 2) How the “error callback” and encoder state are related - The error callback is invoked when an encoding error occurs, and when an error occurs the encoder transitions permanently to the "closed" state; you must create a new VideoEncoder instance afterward. [4] - MDN’s WebCodecs usage guide also notes that configure failures are reported via the error callback provided to the VideoEncoder constructor when the encoder can’t be initialized with the specified parameters. [5] 3) Interpreting “vp8 isConfigSupported error callback state” (the likely meaning) - If your flow is: a) await VideoEncoder.isConfigSupported({ codec: "vp8",... }) b) supported is false, or you still call configure anyway c) you observe your encoder error callback firing and/or encoder.state becoming "closed" then the expected behavior is: - For structurally valid but unsupported configs, you should see supported === false from isConfigSupported. [1][2] - If you proceed anyway (or if you hit an implementation-specific failure after configure), the encoder can report an error through the error callback and transition to "closed" permanently. [5][4] 4) Code pattern to avoid ending up in "closed" - Pre-check with isConfigSupported; only construct/configure the encoder if supported is true. [6][1] Example: const config = { codec: "vp8", width: 640, height: 480, bitrate: 2_000_000, framerate: 30 }; const { supported } = await VideoEncoder.isConfigSupported(config); if (!supported) { // fall back / choose another codec or parameters return; } const encoder = new VideoEncoder({ output: (chunk, meta) => { /* mux/store chunk */ }, error: (e) => { // When called, the encoder transitions permanently to "closed" console.error(e); }, }); encoder.configure(config); (That “error => encoder is closed” rule is per WebCodecs behavior described by MDN.) [4][5] 5) Edge case: “closed” can also happen even if isConfigSupported looked okay - Even with a supported config, WebCodecs can still error during encoding (e.g., resource reclamation when tab is switched), and on error the encoder transitions permanently to "closed". [4] If you paste your exact config object (codec string + width/height + framerate/bitrate + any hardwareAcceleration value) and the console error text, I can map it to the likely class of failure (unsupported config vs runtime encoder failure) and what to change (e.g., fall back to prefer-software vs lower resolution).
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/VideoEncoder/isConfigSupported_static
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/api/videoencoder/isconfigsupported_static/index.md
- 3: Spec promise<bool> IsConfigSupported() w3c/webcodecs#98
- 4: https://developer.mozilla.org/en-US/docs/Web/API/WebCodecs_API/Using_the_WebCodecs_API
- 5: https://developer.mozilla.org/en-US/docs/Web/API/VideoEncoder/configure
- 6: https://developer.chrome.com/docs/web-platform/best-practices/webcodecs
Encoder errors don't abort the loop and the original message is overwritten; also pre-check codec support and avoid any.
Problems in order of impact:
-
Loop doesn't honor encoder errors. The
errorcallback at line 323 only setsencodeStatus. When the encoder encounters an error and transitions to the closed state, the loop continues callingencoder.encode(...)for remaining frames and keeps overwritingencodeStatuswithEncoding frame N/M…, so the user never sees the actual error. Eventuallyencoder.flush()rejects and the catch block replaces the message withFailed: <flush error>, hiding the real cause. Use a flag to track encoder errors and break early. -
No
isConfigSupportedpre-check. Although VP8 is widely supported, you should queryVideoEncoder.isConfigSupported()before configuring. This gives you a synchronous signal to fail fast with an actionable error instead of relying on the async error callback, which may fire mid-loop and get buried by subsequent status updates. -
error: any(line 339) — flagged by@typescript-eslint/no-explicit-any. Useunknownand narrow withinstanceof Error.
♻️ Suggested fix
setIsEncoding(true);
setEncodeStatus('Starting encoding...');
const fps = sdui?.config?.fps || 24;
const width = sdui?.config?.aspectRatio?.width || 480;
const height = sdui?.config?.aspectRatio?.height || 854;
+ const config: VideoEncoderConfig = { codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps };
+
+ const support = await VideoEncoder.isConfigSupported(config);
+ if (!support.supported) {
+ setEncodeStatus('VP8 encoding is not supported in this browser.');
+ setIsEncoding(false);
+ return;
+ }
+
const exportCanvas = document.createElement('canvas');
exportCanvas.width = width;
exportCanvas.height = height;
const chunks: EncodedVideoChunk[] = [];
+ let encoderError: Error | null = null;
const encoder = new VideoEncoder({
output: (chunk) => chunks.push(chunk),
- error: (error) => setEncodeStatus(`Encoding error: ${error.message}`)
+ error: (error) => { encoderError = error; setEncodeStatus(`Encoding error: ${error.message}`); },
});
try {
- encoder.configure({ codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps });
+ encoder.configure(config);
for (let frame = 0; frame <= maxFrames; frame += 1) {
+ if (encoderError) throw encoderError;
setCurrentFrame(frame);
setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`);
await waitForNextPaint();
- await renderNodeToCanvas(previewRef.current, exportCanvas, width, height);
+ await renderNodeToCanvas(previewRef.current!, exportCanvas, width, height);
const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) });
encoder.encode(videoFrame, { keyFrame: frame % fps === 0 });
videoFrame.close();
}
await encoder.flush();
+ if (encoderError) throw encoderError;
downloadIvf(chunks, width, height, fps);
setEncodeStatus(`Done. Downloaded ${chunks.length} encoded frames as IVF.`);
- } catch (error: any) {
- setEncodeStatus(`Failed: ${error?.message || 'Unknown error'}`);
+ } catch (error: unknown) {
+ const message = error instanceof Error ? error.message : 'Unknown error';
+ setEncodeStatus(`Failed: ${message}`);
} finally {
+ if (encoder.state !== 'closed') encoder.close();
setIsEncoding(false);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleGenerateVideo = async () => { | |
| if (!('VideoEncoder' in window)) { | |
| setEncodeStatus('VideoEncoder is not supported in this browser.'); | |
| return; | |
| } | |
| if (!previewRef.current) { | |
| setEncodeStatus('Preview element is not available.'); | |
| return; | |
| } | |
| setIsEncoding(true); | |
| setEncodeStatus('Starting encoding...'); | |
| const fps = sdui?.config?.fps || 24; | |
| const width = sdui?.config?.aspectRatio?.width || 480; | |
| const height = sdui?.config?.aspectRatio?.height || 854; | |
| const exportCanvas = document.createElement('canvas'); | |
| exportCanvas.width = width; | |
| exportCanvas.height = height; | |
| const chunks: EncodedVideoChunk[] = []; | |
| const encoder = new VideoEncoder({ | |
| output: (chunk) => chunks.push(chunk), | |
| error: (error) => setEncodeStatus(`Encoding error: ${error.message}`) | |
| }); | |
| try { | |
| encoder.configure({ codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps }); | |
| for (let frame = 0; frame <= maxFrames; frame += 1) { | |
| setCurrentFrame(frame); | |
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | |
| await waitForNextPaint(); | |
| await renderNodeToCanvas(previewRef.current, exportCanvas, width, height); | |
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | |
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | |
| videoFrame.close(); | |
| } | |
| await encoder.flush(); | |
| downloadIvf(chunks, width, height, fps); | |
| setEncodeStatus(`Done. Downloaded ${chunks.length} encoded frames as IVF.`); | |
| } catch (error: any) { | |
| setEncodeStatus(`Failed: ${error?.message || 'Unknown error'}`); | |
| } finally { | |
| encoder.close(); | |
| setIsEncoding(false); | |
| } | |
| }; | |
| const handleGenerateVideo = async () => { | |
| if (!('VideoEncoder' in window)) { | |
| setEncodeStatus('VideoEncoder is not supported in this browser.'); | |
| return; | |
| } | |
| if (!previewRef.current) { | |
| setEncodeStatus('Preview element is not available.'); | |
| return; | |
| } | |
| setIsEncoding(true); | |
| setEncodeStatus('Starting encoding...'); | |
| const fps = sdui?.config?.fps || 24; | |
| const width = sdui?.config?.aspectRatio?.width || 480; | |
| const height = sdui?.config?.aspectRatio?.height || 854; | |
| const config: VideoEncoderConfig = { codec: 'vp8', width, height, bitrate: 4_000_000, framerate: fps }; | |
| const support = await VideoEncoder.isConfigSupported(config); | |
| if (!support.supported) { | |
| setEncodeStatus('VP8 encoding is not supported in this browser.'); | |
| setIsEncoding(false); | |
| return; | |
| } | |
| const exportCanvas = document.createElement('canvas'); | |
| exportCanvas.width = width; | |
| exportCanvas.height = height; | |
| const chunks: EncodedVideoChunk[] = []; | |
| let encoderError: Error | null = null; | |
| const encoder = new VideoEncoder({ | |
| output: (chunk) => chunks.push(chunk), | |
| error: (error) => { encoderError = error; setEncodeStatus(`Encoding error: ${error.message}`); }, | |
| }); | |
| try { | |
| encoder.configure(config); | |
| for (let frame = 0; frame <= maxFrames; frame += 1) { | |
| if (encoderError) throw encoderError; | |
| setCurrentFrame(frame); | |
| setEncodeStatus(`Encoding frame ${frame + 1}/${maxFrames + 1}...`); | |
| await waitForNextPaint(); | |
| await renderNodeToCanvas(previewRef.current!, exportCanvas, width, height); | |
| const videoFrame = new VideoFrame(exportCanvas, { timestamp: Math.round((frame / fps) * 1_000_000) }); | |
| encoder.encode(videoFrame, { keyFrame: frame % fps === 0 }); | |
| videoFrame.close(); | |
| } | |
| await encoder.flush(); | |
| if (encoderError) throw encoderError; | |
| downloadIvf(chunks, width, height, fps); | |
| setEncodeStatus(`Done. Downloaded ${chunks.length} encoded frames as IVF.`); | |
| } catch (error: unknown) { | |
| const message = error instanceof Error ? error.message : 'Unknown error'; | |
| setEncodeStatus(`Failed: ${message}`); | |
| } finally { | |
| if (encoder.state !== 'closed') encoder.close(); | |
| setIsEncoding(false); | |
| } | |
| }; |
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 339-339: web-sdui/src/App.tsx#L339
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-sdui/src/App.tsx` around lines 303 - 345, handleGenerateVideo currently
ignores encoder runtime errors and overwrites them during the frame loop; add a
boolean flag (e.g., encoderError) scoped near the VideoEncoder construction and
set it inside the encoder.error callback (use error: unknown and narrow with
instanceof Error) so the loop checks that flag after each encode and breaks
early if set, preventing further encoder.encode calls; before calling
encoder.configure call await VideoEncoder.isConfigSupported({ codec: 'vp8',
width, height, framerate: fps }) and throw/set status if unsupported; also
replace catch(error: any) with catch(error: unknown) and narrow to extract a
safe message for setEncodeStatus.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-sdui/src/App.tsx`:
- Around line 242-253: There is a race because user-driven calls to
setCurrentFrame (from the play/pause handler and seek bar) can resolve
frameWaiterRef while encoding; add an isEncoding boolean state (e.g.,
useState(false)) and set it true/false around the encoding loop, then: 1)
disable the play/pause button and seek bar UI when isEncoding is true so they
cannot trigger setCurrentFrame during encoding, and 2) guard any external
setters (the play/pause handler and the seek change handler) to be no-ops when
isEncoding is true so setCurrentFrame and setFrameAndWaitForRender only come
from the encoder while encoding. Ensure you update references to frameWaiterRef,
setFrameAndWaitForRender, currentFrame, and the play/seek handlers accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web-sdui/src/App.tsx (1)
395-412: 💤 Low value
previewRef.currentmay benullinside the loop per TypeScript.The early-return at line 360 narrows
previewRef.currentfor that block, but the narrowing is lost across the awaits inside the loop, so line 403 passesHTMLDivElement | nullintorenderNodeToCanvas'sHTMLElementparameter. Capture the ref once before the loop (e.g.const previewNode = previewRef.current!;after the guard) and pass that local to keep types honest without repeated non-null assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-sdui/src/App.tsx` around lines 395 - 412, The loop uses previewRef.current across awaits so TypeScript cannot guarantee it’s non-null; before the for loop (after the existing early-return guard) capture the ref into a local constant (e.g. const previewNode = previewRef.current!) and then use that local (previewNode) in setFrameAndWaitForRender/renderNodeToCanvas calls and any logging instead of previewRef.current so the type stays narrowed across awaits and you avoid repeated non-null assertions in functions like renderNodeToCanvas, the VideoFrame creation, and encoder.encode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-sdui/src/App.tsx`:
- Around line 276-289: renderNodeToCanvas currently re-fetches and re-inlines
every <img> src on every frame causing repeated network calls; create a
per-export cache (e.g. const imageCache = new Map<string, string | null>()) in
handleGenerateVideo and pass it into renderNodeToCanvas, then change
renderNodeToCanvas to consult imageCache before calling imageToDataUrl(src) and
store the resolved dataURL (or null for failures) in the cache so subsequent
frames reuse the inlined result instead of refetching.
---
Nitpick comments:
In `@web-sdui/src/App.tsx`:
- Around line 395-412: The loop uses previewRef.current across awaits so
TypeScript cannot guarantee it’s non-null; before the for loop (after the
existing early-return guard) capture the ref into a local constant (e.g. const
previewNode = previewRef.current!) and then use that local (previewNode) in
setFrameAndWaitForRender/renderNodeToCanvas calls and any logging instead of
previewRef.current so the type stays narrowed across awaits and you avoid
repeated non-null assertions in functions like renderNodeToCanvas, the
VideoFrame creation, and encoder.encode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a4bbd36-68fb-48cb-af28-123478eeb4ad
📒 Files selected for processing (2)
web-sdui/src/App.tsxweb-sdui/src/views/MultiLyricsContainer.tsx
✅ Files skipped from review due to trivial changes (1)
- web-sdui/src/views/MultiLyricsContainer.tsx
| const renderNodeToCanvas = async (node: HTMLElement, canvas: HTMLCanvasElement, width: number, height: number) => { | ||
| const clonedNode = node.cloneNode(true) as HTMLElement; | ||
| const clonedImages = Array.from(clonedNode.querySelectorAll('img')); | ||
| for (const image of clonedImages) { | ||
| const originalSrc = image.getAttribute('src'); | ||
| if (!originalSrc) continue; | ||
| const inlined = await imageToDataUrl(originalSrc); | ||
| if (inlined) { | ||
| image.setAttribute('src', inlined); | ||
| } else { | ||
| image.style.visibility = 'hidden'; | ||
| console.warn('[VideoEncoder] Hiding non-CORS image during export', { src: originalSrc }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Cache inlined image data URLs across frames.
renderNodeToCanvas runs once per frame and re-fetches every <img> src inside the cloned DOM on each call. For the default 873-frame export with the album art image, that's ~873 network round-trips for the same URL, which is wasteful, slows export dramatically, and risks remote rate-limiting/failures partway through (causing the image to flip to visibility: hidden mid-video). Resolve and cache each unique src to a data URL once before the encoding loop (or memoize across calls), and reuse the inlined value per frame.
♻️ Sketch: hoist a per-export src→dataURL cache
- const renderNodeToCanvas = async (node: HTMLElement, canvas: HTMLCanvasElement, width: number, height: number) => {
+ const renderNodeToCanvas = async (
+ node: HTMLElement,
+ canvas: HTMLCanvasElement,
+ width: number,
+ height: number,
+ imageCache: Map<string, string | null>,
+ ) => {
const clonedNode = node.cloneNode(true) as HTMLElement;
const clonedImages = Array.from(clonedNode.querySelectorAll('img'));
for (const image of clonedImages) {
const originalSrc = image.getAttribute('src');
if (!originalSrc) continue;
- const inlined = await imageToDataUrl(originalSrc);
+ let inlined = imageCache.get(originalSrc);
+ if (inlined === undefined) {
+ inlined = await imageToDataUrl(originalSrc);
+ imageCache.set(originalSrc, inlined);
+ }
if (inlined) {
image.setAttribute('src', inlined);
} else {
image.style.visibility = 'hidden';
- console.warn('[VideoEncoder] Hiding non-CORS image during export', { src: originalSrc });
}
}Then create const imageCache = new Map<string, string | null>(); in handleGenerateVideo and pass it into each renderNodeToCanvas call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-sdui/src/App.tsx` around lines 276 - 289, renderNodeToCanvas currently
re-fetches and re-inlines every <img> src on every frame causing repeated
network calls; create a per-export cache (e.g. const imageCache = new
Map<string, string | null>()) in handleGenerateVideo and pass it into
renderNodeToCanvas, then change renderNodeToCanvas to consult imageCache before
calling imageToDataUrl(src) and store the resolved dataURL (or null for
failures) in the cache so subsequent frames reuse the inlined result instead of
refetching.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-sdui/src/App.tsx (1)
419-421: ⚡ Quick winReplace
error: anywithunknownand narrow appropriately.Flagged by
@typescript-eslint/no-explicit-any. Useunknownand narrow withinstanceof Errorfor type safety.♻️ Proposed fix
- } catch (error: any) { + } catch (error: unknown) { console.error('[VideoEncoder] Export failed', error); - setEncodeStatus(`Failed: ${error?.message || 'Unknown error'}`); + const message = error instanceof Error ? error.message : 'Unknown error'; + setEncodeStatus(`Failed: ${message}`); } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-sdui/src/App.tsx` around lines 419 - 421, The catch block in App.tsx uses an explicit any type for the caught error; change the catch parameter from error: any to error: unknown and then narrow it with an if (error instanceof Error) check before accessing error.message (falling back to a safe default otherwise). Update the console.error and setEncodeStatus calls inside the catch to use the narrowed error message (or a generic message) so we no longer rely on any, referencing the existing catch block that logs '[VideoEncoder] Export failed' and calls setEncodeStatus.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web-sdui/src/App.tsx`:
- Around line 419-421: The catch block in App.tsx uses an explicit any type for
the caught error; change the catch parameter from error: any to error: unknown
and then narrow it with an if (error instanceof Error) check before accessing
error.message (falling back to a safe default otherwise). Update the
console.error and setEncodeStatus calls inside the catch to use the narrowed
error message (or a generic message) so we no longer rely on any, referencing
the existing catch block that logs '[VideoEncoder] Export failed' and calls
setEncodeStatus.
1 participant
Motivation
VideoEncoderAPI.Description
previewRefprop toMotionCanvasand attached it to the preview container so the exporter can capture the rendered DOM (web-sdui/src/components/MotionCanvas.tsx).App.tsxincludingwaitForNextPaint, DOM-to-canvas rasterization via SVGforeignObject(renderNodeToCanvas), and per-frameVideoFramecreation and encoding withVideoEncoderconfigured forvp8(web-sdui/src/App.tsx).VP8) file via a small IVF writer (downloadIvf) and triggered an automatic download afterencoder.flush().Generate Video (VideoEncoder)button, encode status messages, and disabled-state while encoding; added local state flagsisEncodingandencodeStatus.Testing
cd web-sdui && npm run build, and the TypeScript + Vite build completed successfully.Codex Task
Summary by CodeRabbit
New Features
Improvements
Bug Fixes