feat(ios): configure the default audio session natively in the observer#96
Draft
hiroshihorie wants to merge 7 commits into
Draft
feat(ios): configure the default audio session natively in the observer#96hiroshihorie wants to merge 7 commits into
hiroshihorie wants to merge 7 commits into
Conversation
When no JS handler is registered for willEnableEngine/didDisableEngine, AudioDeviceModuleObserver now applies an audio-session policy natively on the audio worker thread instead of round-tripping to JS. The policy (an Apple audio configuration per engine state, plus deactivate-on-stop) is pushed once from JS via AudioDeviceModule.setAutomaticAudioSessionConfiguration(), and passing null clears it. Registered JS handlers keep taking precedence. This removes the JS round trip from the default configuration path entirely: the worker thread no longer waits on a JS thread that may itself be blocked in a synchronous bridge call, which previously could stall engine operations until the bounded wait timed out (#89). Session activation is tracked as a hold against RTCAudioSession, which reference-counts activations: the observer activates only when the session is not already active and deactivates only while holding the activation, so repeated policy pushes, switches between the native and JS paths, and interruptions keep the refcount balanced.
2 tasks
…rnal deactivation An interruption (or an external deactivation such as CallKit) sets RTCAudioSession.isActive to NO without touching its activation count. The next enable transition then saw an inactive session and activated again, stacking a second count onto the hold the observer already had. The single release at stop left count 1, so the OS session stayed active forever. When the observer still holds an activation but the session reads inactive, reactivate first and then drop the now-extra count. The order matters: releasing first would zero the count whenever the reactivation fails, and RTCAudioSession's interruption-end recovery (and its other system-event handlers) deactivates outright at count zero, killing the session the engine was still using. Activating first leaves the prior hold untouched on failure so that recovery can restore it. If the drop itself deactivates the session, the held count had already been consumed by an external unmatched release (for example a stray stopAudioSession call) and the drop returned the count the reactivation took. Reactivate once more and keep that single fresh count, so even a stale hold converges to a balanced state instead of leaking or oscillating.
Clearing the automatic configuration (teardown of a setup, or switching paths) while the observer still held an activation stranded the hold: the didDisable native branch was gated on a non-nil policy, so nothing ever released the count and the OS session stayed active. Enter the native branch also when a hold is outstanding with a nil policy, and release the orphaned hold at the next full stop. No configuration is applied in that case since the native path is disarmed. Deactivate-on-stop semantics do not apply to an abandoned policy, releasing is the only balanced outcome.
didDisable fires after the engine's destructive disable work: buffers are stopped, converters disposed, and RTCAudioSession has already decremented its activation count even when setActive:NO fails. Its rollback replays only constructive actions, so nothing it does can restore what the disable removed, and a non-zero return only desyncs libwebrtc's logical engine state from hardware that already changed. This applies to the final deactivation edge and equally to partial transitions such as duplex to playout, where the policy branch reconfigures the session. Swallow and log every non-zero result at the didDisable entry point. willEnable keeps propagating configuration and activation errors, where the enable has not happened yet and a rollback is still meaningful.
- Gate setAutomaticAudioSessionConfiguration to iOS: the macOS build excludes the audio device module natives, so reaching the bridge there threw on an undefined method. tvOS builds include them and keep working. - Type the configuration with literal unions matching the native maps instead of bare strings, so unknown categories, modes, and options are rejected at compile time rather than silently falling back. - Add voicePrompt to the native mode map, matching AudioUtils in the LiveKit React Native SDK and keeping the SDK's AppleAudioMode assignable to the new union. - Export the configuration types from the package root.
The native path's decision logs were os_log_debug, which iOS neither streams to a console attach nor persists to collected log archives. Field verification of this subsystem had to be inferred from audiomxd system records. Log the per-transition decisions (skip, configure, activate, deactivate, hold rebalance) at default level so they survive into log archives. Frequency is a handful of lines per engine transition.
- Handler precedence is evaluated per hook, so custom willEnable and didDisable handlers must be registered or cleared as a pair while a policy is set. A JS handler owning one hook while the native policy owns the other can activate a session that the owning regime never releases. registerGlobals() must have reconciled the native handler flags before the policy takes effect. - Clearing a deactivateOnStop:NO policy while the engine is already stopped keeps the session activation held until the next engine cycle, because no callback fires in between. Callers who need an immediate release should stop under a deactivateOnStop:YES policy before clearing. - Update the reassignment note now that a hold orphaned by clearing is released at the next full stop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When no JS handler is registered for the audio engine lifecycle hooks,
AudioDeviceModuleObserverstill round-tripped to JS forwillEnableEngine/didDisableEngineso the default LiveKit path could configure the AVAudioSession. That round trip parks the audio worker thread on the JS thread, which is the deadlock mechanism from #89: if the JS thread is itself blocked in a synchronous bridge call (e.g.addTransceiver), the two wait on each other. #90 bounded the wait and #91 skipped the round trip when no handler is registered, but the default path still needed the JS round trip to configure the session at all.Change
AudioDeviceModule.setAutomaticAudioSessionConfiguration(config | null): JS pushes a session policy once (an Apple audio configuration for recording and for playout-only, plusdeactivateOnStop). iOS only (including tvOS), a no-op elsewhere.AVAudioSessionnatively on the worker thread inwillEnable/didDisable. The default path involves no JS per engine transition. Registered JS handlers keep taking precedence, still under the bounded wait from fix(ios): bound AudioDeviceModuleObserver JS waits to break the deadlock #90, and must be registered or cleared as a pair while a policy is set.RTCAudioSession, which reference-counts activations: the observer activates only when the session is not already active and deactivates only while holding. Interruptions and external deactivations (e.g. CallKit) are rebalanced by reactivating first and then dropping the extra count, so a failed reactivation leaves the prior hold intact for the count-gated system recovery. A hold orphaned by clearing the policy is released at the next full stop.didDisableare logged and reported as completed: the callback fires after the engine's destructive disable work, so a non-zero return cannot roll anything back and would only desync libwebrtc's logical engine state. OnlywillEnablepropagates errors.Review
Two adversarial review rounds (external reviewer, plus independent source verification of every finding against
RTCAudioSession.mmandaudio_engine_device.mm) drove the hardening commits on this branch: the activation-hold rebalance ordering, the orphaned-hold release, thedidDisableerror contract, API hardening (literal unions, root exports, platform gate), and default-level decision logging.Testing
Before undrafting
willStartEnginewhileaddTransceiverruns)Companion PR: livekit/client-sdk-react-native#434.