fix(#264): read raw pixels from canvas for VideoFrame to avoid silent failures on Linux#281
fix(#264): read raw pixels from canvas for VideoFrame to avoid silent failures on Linux#281GuilhermeFaga wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe video exporter's frame construction was refactored to extract pixel data from the canvas using the 2D context's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 914a3c7f7b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const imageData = canvasCtx.getImageData(0, 0, canvas.width, canvas.height); | ||
| const exportFrame = new VideoFrame(imageData.data.buffer, { |
There was a problem hiding this comment.
Scope raw-pixel fallback to affected Linux cases
This change forces a synchronous getImageData readback and CPU-side copy for every frame in every environment, even though the comment and issue describe a Linux-specific shared-image failure. On unaffected platforms this removes the prior zero-copy VideoFrame(canvas, ...) path and can significantly reduce export throughput (especially at high resolutions/long videos), so the workaround should be gated (e.g., Linux-only or runtime fallback after a detected failure) rather than always-on.
Useful? React with 👍 / 👎.
Description
Fix green static output for MP4 export.
Motivation
On some linux OSs the MP4 output is corrupted.
Type of Change
Related Issue(s)
#264
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit