fix(macos): keep mic audio when system audio is enabled - #913
Conversation
getCompanionAudioFallbackInfo returned only the video path when a macOS system sidecar existed, assuming the inline mp4 track was a complete mix. The capture helper writes system audio alone to that track, so the microphone was dropped from preview and export whenever both sources were enabled — the recorded narration was silent even though it had been captured correctly to recording-<ts>.mic.m4a. Return both macOS sidecars instead, so the renderer routes them as separate system and mic tracks and mutes the silent embedded track. Existing recordings recover their audio on reopen. Fixes webadderallorg#912
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe macOS recording diagnostics fallback now documents sidecar precedence. A regression test verifies that embedded audio with system and microphone sidecars returns both sidecar paths. ChangesmacOS audio routing
Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change restores routing of macOS system and microphone sidecars for preview and export, with no unresolved merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
AbhijeetDash
changed the title
|
The executable change is in commit The full diff against } else if (hasUsableMacSystemCompanion) {
- paths = [videoPath];
+ paths = Array.from(
+ new Set(
+ companionCandidates.flatMap((candidate) =>
+ candidate.platform === "mac" ? candidate.usablePaths : [],
+ ),
+ ),
+ );
}
The regression test asserts exactly the behaviour called for — that both sidecars are returned: await expect(getCompanionAudioFallbackPaths(videoPath)).resolves.toEqual([
systemPath,
micPath,
]);It fails against |
b9bdda4 to
9b72ff0
Compare
|
@mvanhorn I've made this fix for audio track not reaching sidecar. Is there a process for getting this PR reviewed by the team? |
Use the macOS-only sidecar selection and documentation from upstream PR webadderallorg#913. Preserve both sets of regression tests and the preview/export routing, timing, and committed callback fixes on this branch.
2 participants
Description
getCompanionAudioFallbackInforeturned only the video path when a macOS system sidecar existed, assuming the inline mp4 track was a complete mix. The capture helper writes system audio alone to that track, so the microphone was dropped from preview and export whenever both sources were enabled — the recorded narration was silent even though it had been captured correctly torecording-<ts>.mic.m4a.Return both macOS sidecars instead, so the renderer routes them as separate system and mic tracks and mutes the silent embedded track. Existing recordings recover their audio on reopen.
Motivation
Recording with both System audio and Microphone enabled produces a video with no audible narration on macOS. That's the ordinary configuration for a narrated screen recording, so the failure hits a common path — and it's silent in both senses: nothing warns the user, and the loss is only discovered on playback.
Measured on affected recordings:
Capture works; routing of the captured audio does not.
Root cause.
audioRoutingEngineis built around two sidecars — it derives a track id from the filename, plays.system.and.mic.with independent gains, and mutes the embedded track when dedicated sidecars exist.getCompanionAudioFallbackInfonever handed it those sidecars:Downstream this collapses to
externalAudioPaths: []→pathsByTrack: {}→includeEmbeddedInExport: true, so preview and export use only the embedded track and.mic.m4ais never opened.The branch assumed that embedded track was a complete system+mic mix. It isn't:
ScreenCaptureKitRecorder.swiftappends the mic to the inline track only when system audio is off (if !capturesSystemAudio), so with system audio enabled it carries system audio alone — silence when nothing is playing."Usable" is decided purely by
stat.size > 0, so a 4 KB silent.system.m4ais enough to trigger the bad branch.Type of Change
Related Issue(s)
Fixes #912
Screenshots / Video
Not applicable in the usual sense — this is an audio-only defect with no visual change, and a screenshot of a silent waveform proves little. The measurements above are the evidence; they're reproducible with the commands in the testing guide below.
Testing Guide
Reproduce (on
main):npm run dev.system.m4ais what triggers the bad branch, and with audio playing you'd hear the system track and might not notice the mic is gone.Verify the captured files:
The newest
.mic.m4asits near -20 dB while the.mp4sits at -91 dB — the mic captured, the video silent.Confirm the fix: check out this branch and reopen that same recording. The narration is audible without re-recording, since the change affects which files the editor loads rather than how anything is captured.
Automated:
The added regression test was verified non-vacuous: without the fix it fails, returning
[videoPath]instead of the two sidecars.Scope
This fixes preview and export. The raw
.mp4in the recordings folder remains system-audio-only, because the helper still skips the mic on the inline track. Making that file self-contained requires real PCM mixing in the capture path — tracked as the secondary defect in #912 and deliberately kept out of this change.The fix is guarded to
platform === "mac", so the Windows.wavlayout is untouched.Checklist
volumedetectmeasurements included in place of a screenshot)Summary by CodeRabbit
Tests
Documentation