fix(recording): preserve mic audio when recovering mac capture#142
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
macOS Capture Recovery Logic electron/ipc/handlers.ts |
Modified recoverNativeMacCaptureOutput() to extract audio paths from macDiagnostics with fallback to nativeCapture* variables, conditionally invoke muxNativeMacRecordingWithAudio(), update catch-block diagnostics logging to use local path variables, and remove trailing whitespace. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
🐰 A path once global, now local and neat,
Audio tracks from diagnostics we'll greet,
With muxing logic that conditionally flows,
The rabbit's refactored—watch how it goes! 🎵
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and concisely describes the primary change: preserving microphone audio in the macOS capture recovery path, which is the main focus of the code modifications. |
| Description check | ✅ Passed | The description is comprehensive and well-structured, addressing all key sections of the template including purpose, motivation, change type, testing guide, and checklist completion. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 2668-2672: Wrap the call to
muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath)
in a try-catch so mux failures do not abort recovery; if an error occurs, log it
(using the same logger used elsewhere) and continue to call
finalizeStoredVideo(candidatePath) so a video-only file is returned; ensure you
reference the same symbols muxNativeMacRecordingWithAudio, candidatePath,
systemAudioPath, microphonePath, and finalizeStoredVideo when making the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 281a8fa4-6986-428d-81c2-5ec4eca899b3
📒 Files selected for processing (1)
electron/ipc/handlers.ts
| if (systemAudioPath || microphonePath) { | ||
| await muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath) | ||
| } | ||
|
|
||
| return await finalizeStoredVideo(candidatePath) |
There was a problem hiding this comment.
Wrap muxing call in try-catch for consistency with the normal stop path.
The normal stop path (lines 3660-3666) wraps muxNativeMacRecordingWithAudio in a try-catch and continues to finalize even if muxing fails. Here, a muxing error will cause the entire recovery to fail and return null, preventing users from getting even a video-only file.
🐛 Proposed fix to handle muxing errors gracefully
if (systemAudioPath || microphonePath) {
- await muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath)
+ try {
+ await muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath)
+ } catch (muxError) {
+ console.warn('Failed to mux recovered native macOS audio into capture:', muxError)
+ }
}
return await finalizeStoredVideo(candidatePath)📝 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.
| if (systemAudioPath || microphonePath) { | |
| await muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath) | |
| } | |
| return await finalizeStoredVideo(candidatePath) | |
| if (systemAudioPath || microphonePath) { | |
| try { | |
| await muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath) | |
| } catch (muxError) { | |
| console.warn('Failed to mux recovered native macOS audio into capture:', muxError) | |
| } | |
| } | |
| return await finalizeStoredVideo(candidatePath) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/ipc/handlers.ts` around lines 2668 - 2672, Wrap the call to
muxNativeMacRecordingWithAudio(candidatePath, systemAudioPath, microphonePath)
in a try-catch so mux failures do not abort recovery; if an error occurs, log it
(using the same logger used elsewhere) and continue to call
finalizeStoredVideo(candidatePath) so a video-only file is returned; ensure you
reference the same symbols muxNativeMacRecordingWithAudio, candidatePath,
systemAudioPath, microphonePath, and finalizeStoredVideo when making the change.
2 participants
Description
Fixes a macOS native recording issue where microphone audio could be missing after a screen recording opens in the editor, even though mic capture was enabled and the microphone sidecar audio file was created.
Motivation
On macOS, native ScreenCaptureKit recordings can go through a recovery path before the editor opens. In that path, the app finalized the recovered video file directly and skipped muxing any existing microphone/system-audio sidecar files back into the final
.mp4. As a result, users could complete a recording with mic input enabled, but hear no voice audio in the editor.This change makes the recovery path reuse the recorded audio sidecars and mux them before finalizing the recovered video.
Type of Change
Related Issue(s)
N/A
Screenshots / Video
N/A - audio-only bugfix, no UI changes.
Testing Guide
Optional regression check:
Checklist
Thank you for contributing!
Summary by CodeRabbit