fix: auto-recover desktop mic feed and ScreenCaptureKit stream failures #1374
Conversation
|
Warning Rate limit exceeded@richiemcilroy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds microphone metering and persisted selected-mic state to App; wires mic error logging and restart logic; refactors recording actor spawning with panic/error handling and retry semantics; introduces MicrophoneSourceError and makes macOS screen-capture/audio paths restart-aware on system-stop. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Recorder as Recording Actor Task
participant MicFeed as Microphone Feed
participant ErrorLog as Mic Error Logger
Note over App,Recorder: actor spawn + mic attach + error handling
App->>Recorder: spawn actor_task (async)
activate Recorder
Recorder->>MicFeed: request microphone feed (AddSender / SetInput)
alt feed attached successfully
MicFeed-->>Recorder: feed returned
Recorder->>App: emit RecordingStarted
else AddSender/SetInput fails with ActorNotRunning
Recorder->>App: request restart_mic_feed()
App->>MicFeed: recreate/attach feed (async)
MicFeed-->>App: feed restored
Recorder->>MicFeed: retry attach
end
alt panic during actor creation
Recorder->>Recorder: catch_unwind -> Err
Recorder->>App: handle_spawn_failure(error)
App->>App: emit Failed event + show error dialog
end
deactivate Recorder
opt runtime mic errors
MicFeed->>ErrorLog: send StreamError via channel
ErrorLog->>ErrorLog: async logging / decide restart
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/lib.rs(8 hunks)apps/desktop/src-tauri/src/recording.rs(7 hunks)crates/recording/src/sources/microphone.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/sources/microphone.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/lib.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/sources/microphone.rs
🧠 Learnings (1)
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.
Applied to files:
crates/recording/src/sources/microphone.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/recording.rs (5)
crates/recording/src/studio_recording.rs (8)
new(376-387)new(612-631)done_fut(338-340)builder(356-361)handle(110-142)handle(150-173)handle(181-205)handle(213-221)crates/recording/src/sources/screen_capture/mod.rs (2)
clone(235-249)window(74-79)apps/desktop/src-tauri/src/platform/macos/sc_shareable_content.rs (2)
state(22-24)get_shareable_content(57-73)crates/recording/src/instant_recording.rs (7)
new(259-269)done_fut(52-54)builder(243-245)handle(110-121)handle(129-145)handle(153-169)handle(177-181)crates/recording/src/feeds/microphone.rs (7)
handle(320-492)handle(498-510)handle(516-518)handle(524-544)handle(560-596)handle(602-614)handle(620-636)
🪛 GitHub Actions: CI
apps/desktop/src-tauri/src/recording.rs
[error] 739-739: Suggestion to change recording_dir: &PathBuf to recording_dir: &Path for better practice.
[error] 762-762: Suggestion to replace recording_dir.clone() with recording_dir.to_path_buf() for efficiency.
[error] 739-739: Error: writing &PathBuf instead of &Path involves creating a new object where a slice will suffice. Consider changing parameter to &Path.
🪛 GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
apps/desktop/src-tauri/src/recording.rs
[failure] 739-739: writing &PathBuf instead of &Path involves a new object where a slice will do
error: writing &PathBuf instead of &Path involves a new object where a slice will do
--> apps/desktop/src-tauri/src/recording.rs:739:20
|
739 | recording_dir: &PathBuf,
| ^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#ptr_arg
= note: requested on the command line with -D clippy::ptr-arg
help: change this to
|
739 ~ recording_dir: &Path,
740 | message: String,
...
761 | let _ =
762 ~ handle_recording_end(app.clone(), Err(message), &mut state, recording_dir.to_path_buf()).await;
|
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
richiemcilroy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/recording/src/sources/screen_capture/macos.rs (4)
292-310: Consider stronger memory ordering for atomics.The
compare_exchangeusesRelaxedordering on both success and failure paths. While this is likely acceptable given that the actual async operations have their own synchronization, usingAcquire/ReleaseorSeqCstordering would provide stronger guarantees and be more defensive against future refactoring.if self .started .compare_exchange( false, true, - atomic::Ordering::Relaxed, - atomic::Ordering::Relaxed, + atomic::Ordering::AcqRel, + atomic::Ordering::Acquire, ) .is_ok() {
312-327: Consider stronger memory ordering for atomics.Same concern as in
start(): consider usingAcqRel/Acquireordering for thecompare_exchangeto provide stronger memory ordering guarantees.if self .started .compare_exchange( true, false, - atomic::Ordering::Relaxed, - atomic::Ordering::Relaxed, + atomic::Ordering::AcqRel, + atomic::Ordering::Acquire, ) .is_ok() {
329-331: Consider adding documentation and stronger memory ordering.The
mark_stoppedmethod resets the internal state without verifying the capturer is actually stopped. While this is safe when called afteris_system_stop_error, consider:
- Adding a doc comment explaining when this should be called
- Using
Releaseordering to ensure any prior writes are visible+ /// Resets the started flag after the system has stopped the capturer. + /// This should only be called when `is_system_stop_error` returns true + /// or when externally confirmed that the capturer has been stopped. fn mark_stopped(&self) { - self.started.store(false, atomic::Ordering::Relaxed); + self.started.store(false, atomic::Ordering::Release); }
488-511: Consider clarifying the warning message.The audio path correctly relies on the video path to restart the shared capturer. However, the warning message "awaiting restart" might be misleading since the audio task isn't actively waiting—it's just continuing its error loop.
- warn!("Screen capture audio stream stopped by the system; awaiting restart"); + warn!("Screen capture audio stream stopped by the system; will resume when video path restarts capturer");This makes the coordination between video and audio restart paths more explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/recording/src/sources/screen_capture/macos.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/sources/screen_capture/macos.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/sources/screen_capture/macos.rs
🧠 Learnings (2)
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.
Applied to files:
crates/recording/src/sources/screen_capture/macos.rs
📚 Learning: 2025-10-17T05:58:22.586Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1219
File: crates/enc-avfoundation/src/mp4.rs:350-373
Timestamp: 2025-10-17T05:58:22.586Z
Learning: In crates/enc-avfoundation/src/mp4.rs, the `finish()` method intentionally skips video extension when `is_paused` is true. This is correct behavior because if recording is paused, the video should not be extended beyond the pause point—the pause is user-initiated, unlike the case where ScreenCaptureKit stops providing frames during static content.
Applied to files:
crates/recording/src/sources/screen_capture/macos.rs
🧬 Code graph analysis (1)
crates/recording/src/sources/screen_capture/macos.rs (2)
crates/recording/src/output_pipeline/core.rs (18)
new(307-325)new(675-677)new(718-720)new(751-753)new(795-812)start(778-780)start(828-830)stop(632-642)stop(782-784)stop(832-834)stop(855-855)cancel_token(660-662)setup(686-701)setup(726-738)setup(768-774)setup(818-824)setup(844-853)source(622-624)crates/recording/src/sources/screen_capture/windows.rs (6)
start(287-296)start(373-376)stop(298-307)stop(378-381)setup(143-281)setup(329-367)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
crates/recording/src/sources/screen_capture/macos.rs (3)
20-20: LGTM: Import additions support new restart logic.The added
selectandwarnimports are properly used in the new monitor tasks for handling system-stop errors and stream restarts.Also applies to: 25-25
285-290: LGTM: Simplified Capturer constructor.Moving error handling outside the Capturer struct improves separation of concerns and makes the lifecycle management cleaner.
354-413: LGTM: Well-structured restart logic for video stream.The monitor task correctly:
- Detects system-stop errors via
is_system_stop_error- Resets state with
mark_stopped()before restarting- Propagates genuine errors while recovering from system stops
- Handles channel lag gracefully
- Respects cancellation
The restart will fail fast if the capturer can't be restarted, preventing infinite retry loops.
| fn is_system_stop_error(err: &ns::Error) -> bool { | ||
| const SCK_ERROR_DOMAIN: &str = "com.apple.ScreenCaptureKit.error"; | ||
|
|
||
| if err.domain().to_string() != SCK_ERROR_DOMAIN { | ||
| return false; | ||
| } | ||
|
|
||
| err.localized_description().to_string() == "Stream was stopped by the system" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Potential localization issue with error detection.
The function uses localized_description() which may return different strings in non-English locales, causing the restart logic to fail for international users. If ScreenCaptureKit provides an error code instead of relying on the description string, that would be more robust.
Consider verifying whether:
- The error domain check alone is sufficient, or
- ScreenCaptureKit provides an error code (check
err.code()) that can be used instead of string matching
🏁 Script executed:
#!/bin/bash
# Search for any documentation or other error handling patterns in the codebase
# that might reveal error codes or alternative detection methods
rg -n "ScreenCaptureKit.*error" --type rust -C 3
rg -n "localized_description" --type rust -C 3Length of output: 1149
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find where is_system_stop_error is called
rg -n "is_system_stop_error" --type rust -C 5
# Look for error code usage patterns with ns::Error or similar
rg -n "\.code\(\)" --type rust -C 3
# Look for other error domain checks to see if codes are used elsewhere
rg -n "\.domain\(\)" --type rust -C 3Length of output: 9135
🏁 Script executed:
#!/bin/bash
# Check the ns module imports and usage to understand the API
rg -n "use.*ns::" --type rust -C 2
rg -n "^mod\s" --type rust crates/recording/src/sources/screen_capture/macos.rs -C 1Length of output: 5387
🏁 Script executed:
#!/bin/bash
# Find the ns module definition in this file
head -50 crates/recording/src/sources/screen_capture/macos.rs | grep -E "^(use|mod)" -A 2 -B 2
# Look for ns imports and usage
rg -n "ns::" crates/recording/src/sources/screen_capture/macos.rs | head -20
# Search for Error type definition or FFI bindings
rg -n "struct Error|impl.*Error" --type rust crates/recording/src/sources/screen_capture/ -C 2Length of output: 764
🌐 Web query:
Apple ScreenCaptureKit error codes API documentation macOS
💡 Result:
Apple’s ScreenCaptureKit error-code reference (SCStreamError / SCStreamErrorDomain) is in the Apple Developer docs:
- SCStreamErrorDomain (error domain and overview). [1]
- Individual SCStreamError.Code entries (list of codes such as .userDeclined, .systemStoppedStream, .noWindowList, .internalError, etc.). [2]
Notes/tips:
- The raw domain string is com.apple.ScreenCaptureKit.SCStreamErrorDomain (you can match NSError.domain against it). [1]
- Avoid directly referencing SCStreamError types on older macOS targets (can cause dyld crashes); use the raw domain and numeric codes if you must support pre‑12.3 Macs. [3]
Sources:
[1] Apple Developer — SCStreamErrorDomain.
[2] Apple Developer — SCStreamError.Code (individual code pages).
[3] Nonstrict blog — SCStreamError availability / workaround (raw domain & codes).
🏁 Script executed:
#!/bin/bash
# Search for cidre ScreenCaptureKit or SCStream bindings
rg -n "sc_stream|SCStream|SCError" --type rust -i | head -30
# Look for error code usage patterns in cidre
rg -n "code\(\)" --type rust crates/recording/src/sources/screen_capture/macos.rs
# Check if there are any constants or enums for SCStreamError
rg -n "systemStoppedStream|system_stopped|STOPPED" --type rust crates/recording/src/sources/screen_capture/macos.rsLength of output: 0
🌐 Web query:
cidre Rust bindings ScreenCaptureKit SCStreamError error codes
💡 Result:
Short answer — where they are and the codes:
-
The Rust bindings expose SCStreamError/SCStreamErrorCode in crates such as screen-capture-kit and objc2-screen-capture-kit; see the Rust docs (SCStreamErrorCode, SCStreamErrorDomain). [1]
-
The numeric error values come from Apple’s ScreenCaptureKit headers (SCError.h). Common SCStreamError codes (raw NSError.code values) include:
- UserDeclined = -3816. [2]
- UserStopped = -3817. [2]
- AttemptToStopStreamState = -3808 (example used in workarounds). [3]
- FailedToStartAudioCapture = -3818; FailedToStopAudioCapture = -3819. [4]
- FailedToStartMicrophoneCapture = -3820; SystemStoppedStream = -3821 (macOS 15 additions). [4]
Notes:
- The Rust constants wrap these NSInteger values; you can match either the Rust enum/consts (SCStreamErrorCode) or compare NSError.domain == "com.apple.ScreenCaptureKit.SCStreamErrorDomain" and the numeric code. [1][3]
- Beware: referencing SCStreamError types directly can load the framework on older macOS and crash — many sources recommend comparing domain+raw code to avoid that. [3]
Sources:
[1] screen-capture-kit (docs.rs) — SCStreamErrorCode / SCStreamErrorDomain.
[2] Apple Developer — SCStreamError.Code entries.
[3] Nonstrict blog: workaround and numeric example (attemptToStopStreamState = -3808).
[4] dotnet/xamarin diff of SCError.h showing -3818..-3821 additions.
Use error code instead of localized string to fix fragile locale-dependent detection.
The function relies on localized_description() which returns locale-specific strings, causing it to fail on non-English systems. Apple's ScreenCaptureKit provides error codes for this purpose. Update the implementation to check the error code instead:
- Error domain should be
"com.apple.ScreenCaptureKit.SCStreamErrorDomain" - The system-stopped-stream error code is
-3821(or use theSCStreamErrorCode::SystemStoppedStreamenum if available in cidre bindings) - Replace the localized string comparison with a code comparison to make the check reliable across all locales
🤖 Prompt for AI Agents
In crates/recording/src/sources/screen_capture/macos.rs around lines 459 to 467,
the function currently checks err.localized_description() which is
locale-dependent; change it to verify the error domain and numeric code instead:
use the domain "com.apple.ScreenCaptureKit.SCStreamErrorDomain" and compare
err.code() to -3821 (or to the SCStreamErrorCode::SystemStoppedStream enum value
from the cidre bindings if available), removing the localized_description()
string comparison so detection works reliably across locales.
better errors and retry once when the mic actor isn’t running (apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/recording.rs, crates/
recording/src/sources/microphone.rs).
stream with “Stream was stopped by the system,” automatically restart instead of failing the recording (crates/recording/src/sources/screen_capture/
macos.rs).
Summary by CodeRabbit
New Features
Bug Fixes & Improvements