fix(recording): start webcam recorder after native macOS capture begins#85
fix(recording): start webcam recorder after native macOS capture begins#85josiahcoad wants to merge 1 commit into
Conversation
On the native macOS path the webcam MediaRecorder was started while the ScreenCaptureKit helper was still spinning up (permission checks, stream construction, first-frame latency — ~1s). The webcam sidecar therefore contained ~1s of pre-roll footage from before the screen/mic recording began, and since the editor and exporter align both files at t=0, the webcam PiP lagged the voice/screen by that startup latency in both preview and export. Start the webcam recorder only after startNativeMacRecording resolves, which happens when the helper emits recording-started (first video frame written), so both recordings share the same time origin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe native macOS recording startup flow now handles missing webcam streams, discards recordings when countdowns become inactive, and creates the webcam recorder after the native recording-started event. ChangesNative recording synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useScreenRecorder.ts (1)
1014-1051: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNative macOS recording leaks if webcam recorder creation throws.
startNativeMacRecordingresolves at line 1014, butcreateRecorderHandleat line 1029 runs afterward. IfcreateRecorderHandlethrows (e.g.,MediaRecorderconstructor orrecorder.start()fails), the catch block at line 1049 logs and rethrows without stopping the now-running native recording. SincenativeMacRecording.currentis never assigned (line 1036 is unreachable), no cleanup path — the unmount effect (line 716),cancelRecording, orstartRecording's catch — will find or stop the orphaned native recording.🔒 Proposed fix: stop native recording in the catch block if it was started
const result = await window.electronAPI.startNativeMacRecording(request); if (!result.success || !result.recordingId) { throw new Error(result.error ?? "Native macOS capture failed."); } + const nativeRecordingStarted = true; if (!isCountdownRunActive(countdownRunToken)) { await window.electronAPI.stopNativeMacRecording(true); return true; } // Start the webcam recorder only after the helper has emitted // recording-started (startNativeMacRecording resolves on that event). // Starting it earlier bakes the helper's capture startup latency // (~1s of pre-roll) into the webcam file, desyncing it from the // screen/mic recording that the editor and exporter align at t=0. if (webcamEnabled && webcamStream.current) { nativeWebcamRecorder = createRecorderHandle(webcamStream.current, { mimeType: selectMimeType(), videoBitsPerSecond: BITRATE_BASE, }); } recordingId.current = result.recordingId; nativeMacRecording.current = { recordingId: result.recordingId, finalizing: false, paused: false, }; webcamRecorder.current = nativeWebcamRecorder; accumulatedDurationMs.current = 0; segmentStartedAt.current = Date.now(); allowAutoFinalize.current = true; setRecording(true); setPaused(false); setElapsedSeconds(0); return true; } catch (error) { console.error("Native macOS capture failed:", error); + if (nativeRecordingStarted) { + await window.electronAPI.stopNativeMacRecording(true).catch(() => undefined); + } throw error; }Note:
nativeRecordingStartedshould be declared aslet nativeRecordingStarted = false;before thetryblock and set totrueafter the success check.🤖 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 `@src/hooks/useScreenRecorder.ts` around lines 1014 - 1051, Track whether native recording started in the recording startup flow by declaring nativeRecordingStarted before the try block and setting it true after the startNativeMacRecording success check. In the catch block, stop the native recording with stopNativeMacRecording(true) when that flag is set, then log and rethrow the original error.
🧹 Nitpick comments (1)
src/hooks/useScreenRecorder.ts (1)
946-960: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
waitForWebcamReadyinstead of duplicating its logic.The inline webcam-ready wait at lines 948-959 is identical to the
waitForWebcamReadyhelper at lines 779-796. The Windows path already calls the helper (line 830). Extracting this reduces duplication across three call sites (macOS, Windows, browser capture at lines 1261-1272).♻️ Proposed refactor
if (webcamEnabled) { - if (!webcamReady.current) { - await new Promise<void>((resolve) => { - const interval = setInterval(() => { - if (webcamReady.current) { - clearInterval(interval); - resolve(); - } - }, 50); - setTimeout(() => { - clearInterval(interval); - resolve(); - }, 5000); - }); - } + await waitForWebcamReady(); if (!isCountdownRunActive(countdownRunToken)) { return true; }🤖 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 `@src/hooks/useScreenRecorder.ts` around lines 946 - 960, Replace the inline webcam-ready polling block in the webcamEnabled recording flow with the existing waitForWebcamReady helper, preserving the current conditional check and timeout behavior provided by that helper. Do not duplicate the interval or timeout logic at this call site.
🤖 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.
Outside diff comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1014-1051: Track whether native recording started in the recording
startup flow by declaring nativeRecordingStarted before the try block and
setting it true after the startNativeMacRecording success check. In the catch
block, stop the native recording with stopNativeMacRecording(true) when that
flag is set, then log and rethrow the original error.
---
Nitpick comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 946-960: Replace the inline webcam-ready polling block in the
webcamEnabled recording flow with the existing waitForWebcamReady helper,
preserving the current conditional check and timeout behavior provided by that
helper. Do not duplicate the interval or timeout logic at this call site.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d1000923-2481-41dc-9cc3-7c06b3b9fff0
📒 Files selected for processing (1)
src/hooks/useScreenRecorder.ts
1 participant
Problem
On the native macOS capture path, recordings made with the webcam enabled are audibly out of sync: the voice/screen lead the webcam PiP by about a second, in both the editor preview and exports.
Root cause
startNativeMacRecordingIfAvailablecreated the webcamRecorderHandle(whose constructor callsMediaRecorder.start()) beforewindow.electronAPI.startNativeMacRecording(request). That IPC call only resolves once the ScreenCaptureKit helper emitsrecording-started— i.e. after permission checks, stream construction, and first-frame latency. Measured on an M1 Pro, that startup takes ~1.03s (diagnostic: sessioncreatedAtvs helperrecording-startedtimestamps).So the webcam webm contains ~1s of pre-roll footage from before the screen/mic recording began. The editor and exporter align both files at t=0, which shifts the webcam behind the voice/screen by exactly the helper's startup latency. The screen recording itself (video + system audio + mic) is internally in sync — only the webcam sidecar is offset.
Fix
Start the webcam recorder only after
startNativeMacRecordingresolves, so both recordings share the same time origin. Residual offset is now just IPC + MediaRecorder start latency (tens of ms) instead of the helper's full startup time.Note: the native Windows path (
startNativeWindowsRecordingIfAvailable) has the same pattern and likely the same skew; I left it untouched since I can only verify on macOS.Testing
tsc --noEmit,biome check, and the full vitest suite (352 tests) pass.🤖 Generated with Claude Code
Summary by CodeRabbit