fix(macos): resolve ffmpeg export memory crashes and macOS desktopCapturer permission blocks#474
Conversation
📝 WalkthroughWalkthroughAdded a conditional empty-state message to the screens tab in SourceSelector. When no screen sources are available and not loading, users now see a centered notification explaining that macOS blocked screen access and need to adjust Screen Recording permissions. the grid renders only when sources are present. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/launch/SourceSelector.tsx (1)
130-130: nit:!loadingis always true herethe component early-returns the spinner at line 61-73 when
loadingis truthy, so by the time this JSX rendersloadingis guaranteedfalse. the&& !loadingguard is dead weight — safe to drop for a cleaner condition. not a bug, just tidy-up fuel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/SourceSelector.tsx` at line 130, The JSX condition in SourceSelector.tsx currently checks {screenSources.length === 0 && !loading && (...)} but loading is already gated by the early return spinner in the component (spinner returned in the loading block around lines 61-73), so remove the redundant && !loading check and simplify the condition to just screenSources.length === 0 to make the code cleaner (update the conditional expression where the empty-state render is performed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/launch/SourceSelector.tsx`:
- Around line 130-135: The fallback message for an empty screenSources (in the
JSX branch that checks screenSources.length === 0 && !loading) is hardcoded and
macOS-specific; update it to use the component's i18n hooks (useScopedT / t)
with new keys like sourceSelector.screenBlocked.title and
sourceSelector.screenBlocked.hint, and make the wording platform-neutral by
default, only appending a macOS-specific hint when a platform check indicates
macOS (use window.electronAPI.platform or navigator.platform). Also replace HTML
entities with plain characters for the settings path. Locate the conditional
that renders when screenSources.length === 0 && !loading and swap the hardcoded
strings for t(...) lookups and conditional platform logic.
---
Nitpick comments:
In `@src/components/launch/SourceSelector.tsx`:
- Line 130: The JSX condition in SourceSelector.tsx currently checks
{screenSources.length === 0 && !loading && (...)} but loading is already gated
by the early return spinner in the component (spinner returned in the loading
block around lines 61-73), so remove the redundant && !loading check and
simplify the condition to just screenSources.length === 0 to make the code
cleaner (update the conditional expression where the empty-state render is
performed).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7958e4c0-fe48-45c7-aaad-17c97a69324d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/components/launch/SourceSelector.tsx
| {screenSources.length === 0 && !loading && ( | ||
| <div className="flex flex-col items-center justify-center h-[280px] text-center px-4"> | ||
| <p className="text-sm text-red-400 mb-2 font-medium">Screen access blocked by macOS</p> | ||
| <p className="text-xs text-zinc-400">Please check Screen Recording in System Settings > Privacy & Security. If checked, remove it with (-) and add it again.</p> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
hardcoded strings bypass i18n + message is platform-assuming
two things worth tightening here:
-
the rest of this component routes every user-facing string through
useScopedT(t("sourceSelector.*"),tc("actions.*")), but this new fallback hardcodes english copy. users on non-english locales will get a jarring mix. worth adding keys likesourceSelector.screenBlocked.title/sourceSelector.screenBlocked.hintand pulling them throught(). -
the copy asserts "Screen access blocked by macOS", but this branch fires on any empty
screenSourcesresult — linux/windows builds, transient electron hiccups, or even a genuinely screenless session would all show this mac-specific guidance. kinda cursed if someone hits this on windows. either gate the message behind a platform check (e.g.window.electronAPIexposing platform, ornavigator.platform), or soften the copy to be platform-neutral with the mac-specific hint conditional.
also minor: raw > / & entities render fine but in JSX you can just write > and & directly (or use › for the settings-path breadcrumb, which reads nicer).
♻️ sketch of the shape
- {screenSources.length === 0 && !loading && (
+ {screenSources.length === 0 && (
<div className="flex flex-col items-center justify-center h-[280px] text-center px-4">
- <p className="text-sm text-red-400 mb-2 font-medium">Screen access blocked by macOS</p>
- <p className="text-xs text-zinc-400">Please check Screen Recording in System Settings > Privacy & Security. If checked, remove it with (-) and add it again.</p>
+ <p className="text-sm text-red-400 mb-2 font-medium">
+ {t("sourceSelector.screenBlocked.title")}
+ </p>
+ <p className="text-xs text-zinc-400">
+ {t("sourceSelector.screenBlocked.hint")}
+ </p>
</div>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/launch/SourceSelector.tsx` around lines 130 - 135, The
fallback message for an empty screenSources (in the JSX branch that checks
screenSources.length === 0 && !loading) is hardcoded and macOS-specific; update
it to use the component's i18n hooks (useScopedT / t) with new keys like
sourceSelector.screenBlocked.title and sourceSelector.screenBlocked.hint, and
make the wording platform-neutral by default, only appending a macOS-specific
hint when a platform check indicates macOS (use window.electronAPI.platform or
navigator.platform). Also replace HTML entities with plain characters for the
settings path. Locate the conditional that renders when screenSources.length ===
0 && !loading and swap the hardcoded strings for t(...) lookups and conditional
platform logic.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Lines 5009 to 5012 in 943c290
Restore Vitest nested vite entries in lockfile
This lockfile now keeps @vitest/mocker entries that peer on vite but drops the corresponding node_modules/@vitest/browser*/node_modules/vite package records, which makes clean installs fail. On a fresh checkout, npm ci --dry-run errors with Missing: vite@ from lock file, so CI/developer environments that use npm ci cannot install dependencies from this commit.
ℹ️ 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".
| {screenSources.length === 0 && !loading && ( | ||
| <div className="flex flex-col items-center justify-center h-[280px] text-center px-4"> | ||
| <p className="text-sm text-red-400 mb-2 font-medium">Screen access blocked by macOS</p> | ||
| <p className="text-xs text-zinc-400">Please check Screen Recording in System Settings > Privacy & Security. If checked, remove it with (-) and add it again.</p> |
There was a problem hiding this comment.
The new warning is rendered only inside the screens tab content, but this component defaults to the windows tab whenever screenSources.length === 0 (existing logic at defaultValue), so in the exact no-screen scenario the user first sees an empty grid and not the remediation message. That defeats the new fallback for the primary failure case (blocked screen capture permissions).
Useful? React with 👍 / 👎.
1 participant
Pull Request Template
Description
This PR comprehensively resolves a chain of critical bugs impacting the core video rendering engine and macOS screen capture capabilities.
ffmpeg-staticchild processes. By clearing out the stale dependency tree and updating the lockfile, video frames are now strictly rendered and packaged into MP4s natively within the browser sandbox using HTML5WebCodecsandmediabunny. This completely eliminates theCannot allocate memory(ENOMEM) crashes caused byffmpegspawning massive memory buffers during export.SourceSelector.tsxto beautifully accommodate and explain macOS's hidden permission lockouts. If macOS silently blocks thedesktopCapturerpermissions due to code-signing changes, the UI correctly instructs the user exactly how to restore the permission instead of trapping them in an ambiguous 'blank box' screen.Motivation
Previously, video exports would crash on long buffers due to internal
ffmpegmemory saturation. When trying to fix this via a fresh install targeting the newWebCodecspipeline, macOS would aggressively and silently block the newly built app's Screen Recording permissions due to underlying invalid binary hashes and revoked code-signing identities. This created a frustrating dual-failure state where the user couldn't export videos reliably, and local debug builds were met with a blank screen-capture window. This PR restores stable, low-memory exports and introduces a highly visible user experience when macOS's OS-level security silently restricts the capture pipeline.Type of Change
Related Issue(s)
Screenshots / Video
Screenshot (if applicable):
N/A
Video (if applicable):
N/A
Testing
WebCodecssafely renders the.mp4utilizing native Chromium browser memory without throwingCannot allocate memoryNode.js pipeline failures.macOSbuild to force macOS to naturally invalidate the previousPrivacy & Security > Screen RecordingOS configuration.SourceSelector.tsxproperly catches the empty screen array and renders the new "Screen access blocked by macOS" red fallback warning prompt.Checklist
Summary by CodeRabbit