[Fix] Recover microphone after an interruption ends while backgrounded#1174
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthrough
ChangesForeground Microphone Recovery
Sequence DiagramsequenceDiagram
participant AVFoundation
participant InterruptionsEffect
participant processingQueue
participant applicationStateAdapter
participant dispatcher
AVFoundation->>InterruptionsEffect: interruption ended while backgrounded
InterruptionsEffect->>processingQueue: deliver handle(_:)
processingQueue->>InterruptionsEffect: set pendingForegroundRecovery = true
applicationStateAdapter->>InterruptionsEffect: statePublisher emits .foreground
InterruptionsEffect->>processingQueue: recoverPendingForegroundInterruption()
processingQueue->>InterruptionsEffect: validate stateProvider
InterruptionsEffect->>dispatcher: dispatch setRecording(false)
InterruptionsEffect->>dispatcher: dispatch setRecording(true)
InterruptionsEffect->>dispatcher: dispatch setMicrophoneMuted(true)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore+InterruptionsEffect.swift (1)
80-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly arm foreground recovery when a recording restart was actually deferred.
pendingForegroundRecoveryis currently set for any backgrounded interruption end, includingshouldResumeSession == falseand cases where no restart was attempted. On the next.foreground,recoverPendingForegroundInterruption()can still dispatch.setRecording(false/true)because it only checksisActive/isRecording, so a non-resumable interruption can unexpectedly restart capture.Track a
needsForegroundRecoveryflag inside the same branch that appends the restart actions, and only setpendingForegroundRecoverywhen that restart was both needed and blocked by background state.Also applies to: 118-133
🤖 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 `@Sources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore`+InterruptionsEffect.swift around lines 80 - 103, `pendingForegroundRecovery` is being armed too broadly in `RTCAudioStore+InterruptionsEffect`, which can cause `recoverPendingForegroundInterruption()` to restart recording after interruptions that were not meant to resume. Update the interruption-end handling in `handleInterruptionEnded` so a local `needsForegroundRecovery` flag is set only when the `.setRecording(false)`/`.setRecording(true)` restart actions are actually appended, then assign `pendingForegroundRecovery` only if that restart was needed and the app is still backgrounded. Keep the microphone mute restore behavior unchanged, and make sure the same gating is applied in the related recovery path referenced by `recoverPendingForegroundInterruption`.
🧹 Nitpick comments (1)
StreamVideoTests/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore_InterruptionsEffectTests.swift (1)
215-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a backgrounded no-resume/no-restart negative test.
These new tests cover foreground transitions well, but they still miss the case where interruption end happens in the background with
shouldResumeSession == false(or without a restart-capable state). That is the edge wherependingForegroundRecoverycan be armed incorrectly, so a dedicated negative test would pin the intended contract.🤖 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 `@StreamVideoTests/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore_InterruptionsEffectTests.swift` around lines 215 - 245, Add a negative test for the interruption-end background path where no resume/restart should occur, since the current tests only cover foreground transitions. Extend RTCAudioStore_InterruptionsEffectTests with a case around audioSessionDidEndInterruption that sets appState to background and uses shouldResumeSession false (or a non-restart-capable state from makeState), then verify dispatch stays empty and pendingForegroundRecovery is not armed. Use the existing publisher, subject.dispatcher, and stateProvider setup patterns to locate and mirror the surrounding tests.
🤖 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.
Inline comments:
In `@CHANGELOG.md`:
- Around line 7-8: The changelog has a heading level jump: after the top-level
“Upcoming” heading, the “Fixed” section uses a level 3 heading instead of the
next level down. Update the heading in CHANGELOG.md so the section under
“Upcoming” uses the correct next heading level (or add an intermediate level 2
parent) to keep the hierarchy consistent and avoid MD001; use the “Upcoming” and
“Fixed” headings to locate the spot.
In
`@Sources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore`+InterruptionsEffect.swift:
- Around line 49-55: The interruption handling in
RTCAudioStore+InterruptionsEffect is reading applicationStateAdapter.state from
processingQueue, which can race with StreamAppStateAdapter’s `@Published` state
updates. Update the statePublisher pipeline to cache the latest ApplicationState
on processingQueue, and have handle(_:) use that cached value instead of
querying the adapter directly; keep the recovery path in
recoverPendingForegroundInterruption() driven only by the queue-local cached
state.
---
Outside diff comments:
In
`@Sources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore`+InterruptionsEffect.swift:
- Around line 80-103: `pendingForegroundRecovery` is being armed too broadly in
`RTCAudioStore+InterruptionsEffect`, which can cause
`recoverPendingForegroundInterruption()` to restart recording after
interruptions that were not meant to resume. Update the interruption-end
handling in `handleInterruptionEnded` so a local `needsForegroundRecovery` flag
is set only when the `.setRecording(false)`/`.setRecording(true)` restart
actions are actually appended, then assign `pendingForegroundRecovery` only if
that restart was needed and the app is still backgrounded. Keep the microphone
mute restore behavior unchanged, and make sure the same gating is applied in the
related recovery path referenced by `recoverPendingForegroundInterruption`.
---
Nitpick comments:
In
`@StreamVideoTests/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore_InterruptionsEffectTests.swift`:
- Around line 215-245: Add a negative test for the interruption-end background
path where no resume/restart should occur, since the current tests only cover
foreground transitions. Extend RTCAudioStore_InterruptionsEffectTests with a
case around audioSessionDidEndInterruption that sets appState to background and
uses shouldResumeSession false (or a non-restart-capable state from makeState),
then verify dispatch stays empty and pendingForegroundRecovery is not armed. Use
the existing publisher, subject.dispatcher, and stateProvider setup patterns to
locate and mirror the surrounding tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1430c6cc-fe30-4b90-b937-3d11a33a5cd7
📒 Files selected for processing (4)
CHANGELOG.mdSources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore+InterruptionsEffect.swiftStreamVideo.xcodeproj/xcshareddata/xcschemes/DemoApp.xcschemeStreamVideoTests/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore_InterruptionsEffectTests.swift
| applicationStateAdapter | ||
| .statePublisher | ||
| .removeDuplicates() | ||
| .filter { $0 == .foreground } | ||
| .receive(on: processingQueue) | ||
| .sink { [weak self] _ in self?.recoverPendingForegroundInterruption() } | ||
| .store(in: disposableBag) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid reading applicationStateAdapter.state off the processing queue.
The new background check reads applicationStateAdapter.state on processingQueue, but StreamAppStateAdapter stores state in a plain @Published property. That introduces a cross-thread read/write race between lifecycle notifications and this queue, which can mis-arm or skip recovery.
Cache the latest ApplicationState from statePublisher on processingQueue and use that cached value inside handle(_:) instead of reading the adapter directly.
Also applies to: 101-103
🤖 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
`@Sources/StreamVideo/Utils/AudioSession/RTCAudioStore/Namespace/Effects/RTCAudioStore`+InterruptionsEffect.swift
around lines 49 - 55, The interruption handling in
RTCAudioStore+InterruptionsEffect is reading applicationStateAdapter.state from
processingQueue, which can race with StreamAppStateAdapter’s `@Published` state
updates. Update the statePublisher pipeline to cache the latest ApplicationState
on processingQueue, and have handle(_:) use that cached value instead of
querying the adapter directly; keep the recovery path in
recoverPendingForegroundInterruption() driven only by the queue-local cached
state.
|
SDK Size
|
Public Interface🚀 No changes affecting the public interface. |
StreamVideo XCSize
|
3 participants
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-1773/gh-issueaudio-does-not-recover-after-avaudiosession-interruption-in
🎯 Goal
In Audio Rooms (and any call), the microphone could go permanently silent
after the app spent time in the background. The participant appears unmuted and
the SDK reports
isRecording: true, but no audio is captured — and onlyleaving and rejoining restores it.
Confirmed from a customer log: an
AVAudioSessioninterruption ends while theapp is still backgrounded (~2s before it returns to the foreground).
InterruptionsEffectruns its recording restart at that moment, but WebRTC canonly (re)start the input audio unit while the app is active, so the restart is a
no-op (
IsInputEnabled: 0 / IsInputRunning: 0). When the app later becomesactive, WebRTC's own
handleApplicationDidBecomeActivefinds nothing to resume,and nothing replays the restart — leaving the mic recording silence.
📝 Summary
interruption ended while the app was not active, so the microphone recovers
without leaving the call.
InterruptionsEffect(no new type).stress test.
🛠 Implementation
InterruptionsEffectnow also observesapplicationStateAdapter.statePublisher.When
didEndInterruptionis handled whileapplicationStateAdapter.stateisnot
.foreground, it sets apendingForegroundRecoveryflag..foregroundtransition, if the store is stillisActiveandisRecording, it dispatches the recording restart(
setRecording(false)→setRecording(true)→ re-apply mute) — now againstthe reactivated session.
RTCAudioSessionPublisherevents and the app-lifecyclepublisher) are funneled through a single serial
OperationQueue(
processingQueue), sopendingForegroundRecoveryis always read/writtenfrom one serial context — no extra locking needed.
🎨 Showcase
N/A — no UI changes.
🧪 Manual Testing Notes
then stop it — or simply leave the app backgrounded for several minutes.
IsInputRunning: 1with noactiveSessionIdentifierchange (i.e. no implicit leave/rejoin).
confirm the mic keeps working and is not stopped by the transition.
Automated:
RTCAudioStore_InterruptionsEffectTestscovers the recover /no-recover paths and a concurrent interruption + app-state storm
(
test_concurrentInterruptionAndForegroundStorm_doesNotCrashAndStaysResponsive).☑️ Contributor Checklist
Summary by CodeRabbit