Add option to disable automatic audio session deactivation #881
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a public flag Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@Docs/audio.md`:
- Around line 11-23: The phrase "By default, the SDK deactivates the
`AVAudioSession`…" should be changed to reference the deactivation flag instead
of asserting a default; update the text to state that when
`AudioManager.shared.audioSession.isAutomaticDeactivationEnabled` is true (and
`isAutomaticConfigurationEnabled` is not false), the SDK will deactivate the
`AVAudioSession` when playout and recording are disabled, and conversely explain
that setting `isAutomaticDeactivationEnabled = false` keeps the session active
after the call ends; use the symbols `AVAudioSession`,
`isAutomaticConfigurationEnabled`, and
`AudioManager.shared.audioSession.isAutomaticDeactivationEnabled` to locate and
update the wording.
In `@Sources/LiveKit/Audio/AudioSessionEngineObserver.swift`:
- Around line 36-50: The property isAutomaticDeactivationEnabled currently
defaults to true but the PR intends the default to be disabled; change the
initial state value for isAutomaticDeactivationEnabled in the State/initializer
(where _state.isAutomaticDeactivationEnabled is set) to false and update the
property documentation text and the “Default value:” comment to reflect false;
also make the same change for the other occurrence referenced (the duplicate at
lines 68-70) so both the runtime default and docs consistently indicate the
default is false.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changes/audio-session-deactivation-optionDocs/audio.mdSources/LiveKit/Audio/AudioSessionEngineObserver.swift
⏰ 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). (21)
- GitHub Check: Build & Test (macos-26, 26.1, iOS Simulator,name=iPhone 17 Pro,OS=26.1, true)
- GitHub Check: lint (macos-26, 26.1)
- GitHub Check: lint (macos-14, 15.4)
- GitHub Check: lint (macos-15, 16.4)
- GitHub Check: Build & Test (macos-15, 16.4, macOS,variant=Mac Catalyst)
- GitHub Check: Build & Test (macos-15, 16.4, iOS Simulator,name=iPhone 16 Pro,OS=18.5)
- GitHub Check: Build & Test (macos-26, 26.1, tvOS Simulator,name=Apple TV,OS=26.1)
- GitHub Check: Build & Test (macos-26, 26.1, iOS Simulator,name=iPhone 17 Pro,OS=26.1, true)
- GitHub Check: Build & Test (macos-15, 16.4, visionOS Simulator,name=Apple Vision Pro,OS=2.5)
- GitHub Check: Build & Test (macos-14, 15.4, macOS)
- GitHub Check: Build & Test (macos-26, 26.1, macOS, true)
- GitHub Check: Build & Test (macos-14, 15.4, tvOS Simulator,name=Apple TV,OS=17.5)
- GitHub Check: Build & Test (macos-15, 16.4, tvOS Simulator,name=Apple TV,OS=18.5)
- GitHub Check: Build & Test (macos-26, 26.1, macOS, true)
- GitHub Check: Build & Test (macos-15, 16.4, macOS)
- GitHub Check: Build & Test (macos-26, 26.1, macOS, true)
- GitHub Check: Build & Test (macos-26, 26.1, visionOS Simulator,name=Apple Vision Pro,OS=26.1)
- GitHub Check: Build & Test (macos-26, 26.1, macOS,variant=Mac Catalyst)
- GitHub Check: Build & Test (macos-14, 15.4, iOS Simulator,name=iPhone 15 Pro,OS=17.5)
- GitHub Check: Build & Test (macos-14, 15.4, macOS,variant=Mac Catalyst)
- GitHub Check: Analyze (swift)
🔇 Additional comments (2)
Sources/LiveKit/Audio/AudioSessionEngineObserver.swift (1)
101-135: LGTM for deactivation gating.The deactivation behavior is now properly guarded by the new flag and the log path makes the decision visible.
.changes/audio-session-deactivation-option (1)
1-1: Changelog entry looks good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ## Disabling automatic `AVAudioSession` deactivation | ||
|
|
||
| > **Note**: If you have already set `isAutomaticConfigurationEnabled = false`, you don't need to worry about this setting since the SDK won't touch the audio session at all. | ||
|
|
||
| By default, the SDK deactivates the `AVAudioSession` when both playout and recording are disabled (e.g., after disconnecting from a room). This allows other apps' audio (like Music) to resume. | ||
|
|
||
| However, if your app has its own audio features that could be disrupted by deactivating the audio session, you can disable automatic deactivation: | ||
|
|
||
| ```swift | ||
| AudioManager.shared.audioSession.isAutomaticDeactivationEnabled = false | ||
| ``` | ||
|
|
||
| When set to `false`, the audio session remains active after the LiveKit call ends, preserving your app's audio state. |
There was a problem hiding this comment.
Align “By default” wording with final default.
If the intended default is disabled, the sentence “By default, the SDK deactivates…” becomes incorrect. Consider wording it in terms of the flag instead of a default statement.
📝 Suggested wording
-By default, the SDK deactivates the `AVAudioSession` when both playout and recording are disabled (e.g., after disconnecting from a room). This allows other apps' audio (like Music) to resume.
+When `isAutomaticDeactivationEnabled` is `true`, the SDK deactivates the `AVAudioSession` when both playout and recording are disabled (e.g., after disconnecting from a room). This allows other apps' audio (like Music) to resume.🤖 Prompt for AI Agents
In `@Docs/audio.md` around lines 11 - 23, The phrase "By default, the SDK
deactivates the `AVAudioSession`…" should be changed to reference the
deactivation flag instead of asserting a default; update the text to state that
when `AudioManager.shared.audioSession.isAutomaticDeactivationEnabled` is true
(and `isAutomaticConfigurationEnabled` is not false), the SDK will deactivate
the `AVAudioSession` when playout and recording are disabled, and conversely
explain that setting `isAutomaticDeactivationEnabled = false` keeps the session
active after the call ends; use the symbols `AVAudioSession`,
`isAutomaticConfigurationEnabled`, and
`AudioManager.shared.audioSession.isAutomaticDeactivationEnabled` to locate and
update the wording.
| /// Controls whether the audio session is deactivated when the audio engine stops. | ||
| /// | ||
| /// - When `true`: The `AVAudioSession` is deactivated when both playout and recording are disabled | ||
| /// - When `false`: The `AVAudioSession` remains active when the audio engine stops | ||
| /// | ||
| /// > Note: This value is only used when `isAutomaticConfigurationEnabled` is `true`. | ||
| /// | ||
| /// > Tip: Set to `false` if your app has other audio features that could be disrupted | ||
| /// > by deactivating the audio session. | ||
| /// | ||
| /// Default value: `true` | ||
| public var isAutomaticDeactivationEnabled: Bool { | ||
| get { _state.isAutomaticDeactivationEnabled } | ||
| set { _state.mutate { $0.isAutomaticDeactivationEnabled = newValue } } | ||
| } |
There was a problem hiding this comment.
Default still enables deactivation—verify intent vs PR objective.
PR objective says deactivation should be disabled by default, but the property docs and state default are still true. If the intended default is disabled, flip the default to false and update the inline doc.
✅ Proposed fix
- /// Default value: `true`
+ /// Default value: `false`
public var isAutomaticDeactivationEnabled: Bool {
- var isAutomaticDeactivationEnabled: Bool = true
+ var isAutomaticDeactivationEnabled: Bool = falseAlso applies to: 68-70
🤖 Prompt for AI Agents
In `@Sources/LiveKit/Audio/AudioSessionEngineObserver.swift` around lines 36 - 50,
The property isAutomaticDeactivationEnabled currently defaults to true but the
PR intends the default to be disabled; change the initial state value for
isAutomaticDeactivationEnabled in the State/initializer (where
_state.isAutomaticDeactivationEnabled is set) to false and update the property
documentation text and the “Default value:” comment to reflect false; also make
the same change for the other occurrence referenced (the duplicate at lines
68-70) so both the runtime default and docs consistently indicate the default is
false.
There was a problem hiding this comment.
@coderabbitai why do you think the PR description states the deactivation should be disabled by default? It's only about adding certain possibility/config options.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the actual PR description from GitHub
gh pr view 881 --repo livekit/client-sdk-swift --json title,body
Length of output: 876
@pblazej You're right, and I apologize for the confusion. I was interpreting the PR title "Disable automatic audio session deactivation by default" literally—the phrase "by default" suggested to me that the default value should be changed to false.
However, if the intent is simply to add the configuration option while keeping the existing behavior (deactivation enabled by default), then the current implementation with isAutomaticDeactivationEnabled = true is correct. The PR title is a bit ambiguous in that case, but I understand now that you're adding the capability to disable automatic deactivation, not changing the default behavior itself.
Thank you for the clarification!
pblazej
left a comment
pblazej
left a comment
There was a problem hiding this comment.
Let's put it into the list of possible Audio API improvements
hiroshihorie
changed the title
Calling
setActive(false)could disrupt other audio in the app. I believe it's relatively harmless to not call it. But in this PR we keep the default to automatic deactivation and add doc about it.Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.