From 62430fc9c6d311f1be08a85b48dacfa024d7287d Mon Sep 17 00:00:00 2001 From: Ilias Pavlidakis <3liaspav@gmail.com> Date: Tue, 28 Jul 2026 14:13:05 +0300 Subject: [PATCH 1/2] [Fix] Prevent AVAudioEngine crash when enabling recording --- modules/audio_device/audio_engine_device.h | 25 +- modules/audio_device/audio_engine_device.mm | 308 ++++++++++++++---- ...PeerConnectionFactoryAudioEngine_xctest.mm | 43 +++ 3 files changed, 308 insertions(+), 68 deletions(-) diff --git a/modules/audio_device/audio_engine_device.h b/modules/audio_device/audio_engine_device.h index f5ceab9c6c..47ba4aaa13 100644 --- a/modules/audio_device/audio_engine_device.h +++ b/modules/audio_device/audio_engine_device.h @@ -418,16 +418,25 @@ class AudioEngineDevice : public AudioDeviceModule, public AudioSessionObserver bool default_device = (DidUpdateDefaultOutputDevice() && next.IsOutputDefaultDevice()) || (DidUpdateDefaultInputDevice() && next.IsInputDefaultDevice()); - // Special case to re-create engine when switching from Speaker & Mic -> - // Speaker only. - bool special_case = (prev.IsOutputEnabled() && next.IsOutputEnabled()) && - (prev.IsInputEnabled() && !next.IsInputEnabled()); + // Adding or removing input while output remains enabled changes the I/O + // topology. Recreate the engine so voice processing is configured on a + // fresh graph rather than one whose I/O unit is still stopping. + bool input_topology = prev.IsOutputEnabled() && next.IsOutputEnabled() && + prev.IsInputEnabled() != next.IsInputEnabled(); // When stereo output channels preference changes or stereo playout becomes // unavailable/available. bool output_channels = DidUpdateDesiredOutputChannels(); - return device || default_device || special_case || output_channels; + // A live graph cannot safely toggle voice processing while its I/O unit + // may still be stopping, so require a fresh engine for that transition. + bool voice_processing = + prev.IsAnyEnabled() && DidUpdateVoiceProcessingEnabled(); + + // Centralize every graph-invalidating transition so callers always take + // the same deterministic release-and-recreate path. + return device || default_device || input_topology || voice_processing || + output_channels; } bool DidEnableManualRenderingMode() const { @@ -484,8 +493,10 @@ class AudioEngineDevice : public AudioDeviceModule, public AudioSessionObserver void StartRenderLoop(); AVAudioEngineManualRenderingBlock render_block_; - void ConfigureVoiceProcessingNode(AVAudioInputNode* input_node, - EngineStateUpdate state); + // Return the native failure so callers can avoid running input without the + // requested voice-processing configuration. + int32_t ConfigureVoiceProcessingNode(AVAudioInputNode* input_node, + EngineStateUpdate state); void ConfigureMutedSpeechActivityEventListener(AVAudioInputNode* input_node, EngineStateUpdate state); diff --git a/modules/audio_device/audio_engine_device.mm b/modules/audio_device/audio_engine_device.mm index 270e030d9d..3952aac62d 100644 --- a/modules/audio_device/audio_engine_device.mm +++ b/modules/audio_device/audio_engine_device.mm @@ -56,6 +56,10 @@ const UInt16 kFixedRecordDelayEstimate = 0; const UInt16 kStartEngineMaxRetries = 10; // Maximum blocking 1sec. const useconds_t kStartEngineRetryDelayMs = 100; +// Bound VP recovery so a persistent AVAudioEngine failure cannot hang setup. +const UInt16 kVoiceProcessingMaxRetries = 3; +// Give the released I/O unit time to stop before building the next graph. +const useconds_t kVoiceProcessingRetryDelayMs = 100; const size_t kMaximumFramesPerBuffer = 3072; const size_t kAudioSampleSize = 2; // Signed 16-bit integer @@ -1788,7 +1792,14 @@ fine_audio_buffer_.reset(new FineAudioBuffer(audio_device_buffer_.get())); if (state.next.IsInputEnabled()) { - ConfigureVoiceProcessingNode(engine_manual_input_.inputNode, state); + // Capture the native result so manual mode honors the same VP guarantee. + int32_t result = + ConfigureVoiceProcessingNode(engine_manual_input_.inputNode, state); + // Stop setup if the requested processing could not be applied. + if (result != 0) { + // Surface the error instead of recording with degraded call audio. + return result; + } } } else if (state.prev.IsOutputEnabled() && !state.next.IsOutputEnabled()) { @@ -1941,6 +1952,75 @@ return engine_device_.outputNode; }; + // Keep every release and observer notification identical across retries, + // normal recreation, and final shutdown. + auto releaseEngine = [this]() { + // AVAudioEngine ownership is confined to the device-control thread. + RTC_DCHECK_RUN_ON(thread_); + // Give the owner a chance to detach from the engine before destruction. + if (observer_ != nullptr) { + // Preserve the observer's error so lifecycle recovery remains atomic. + int32_t result = observer_->OnEngineWillRelease(engine_device_); + // Do not discard an engine the observer failed to release safely. + if (result != 0) { + // Record which lifecycle callback prevented the transition. + LOGE() << "Call to OnEngineWillRelease returned error: " << result; + // Propagate the exact callback failure to the state transition. + return result; + } + } + // Drop the old graph only after all owners have released it. + engine_device_ = nil; + // Report that the engine is fully released and safe to replace. + return 0; + }; + + // Keep engine allocation and observer attachment identical for retries and + // ordinary state transitions. + auto createEngine = [this]() { + // AVAudioEngine ownership is confined to the device-control thread. + RTC_DCHECK_RUN_ON(thread_); + // A fresh graph avoids reconnecting nodes on the stopped I/O unit. + engine_device_ = [[AVAudioEngine alloc] init]; + // Let the owner attach its configuration to this exact engine instance. + if (observer_ != nullptr) { + // Preserve the observer's result so a partial create cannot continue. + int32_t result = observer_->OnEngineDidCreate(engine_device_); + // Stop before node configuration when owner setup failed. + if (result != 0) { + // Record which lifecycle callback prevented engine creation. + LOGE() << "Call to OnEngineDidCreate returned error: " << result; + // Propagate the callback failure to the transaction rollback. + return result; + } + } + // Report that a fresh engine and its observer state are ready. + return 0; + }; + + // Reapply session-dependent owner configuration to every fresh retry engine. + auto prepareEngine = [this, &state]() { + // AVAudioEngine ownership is confined to the device-control thread. + RTC_DCHECK_RUN_ON(thread_); + // Notify only for transitions that enable media and have an observer. + if (state.DidAnyEnable() && observer_ != nullptr) { + // Configure the audio session before voice-processing touches the graph. + int32_t result = + observer_->OnEngineWillEnable(engine_device_, + state.next.IsOutputEnabled(), + state.next.IsInputEnabled()); + // Stop before node configuration when session preparation failed. + if (result != 0) { + // Record which lifecycle callback prevented engine preparation. + LOGE() << "Call to OnEngineWillEnable returned error: " << result; + // Propagate the callback failure to the transaction rollback. + return result; + } + } + // Report that node configuration may safely proceed. + return 0; + }; + // -------------------------------------------------------------------------------------------- // Step: Stop AVAudioEngine // @@ -1975,14 +2055,13 @@ // if (state.IsEngineRecreateRequired()) { LOGI() << "Recreate required, releasing AVAudioEngine..."; - if (observer_ != nullptr) { - int32_t result = observer_->OnEngineWillRelease(engine_device_); - if (result != 0) { - LOGE() << "Call to OnEngineWillRelease returned error: " << result; - return rollback(result); - } + // Use the shared release path so observer ordering matches retry recovery. + int32_t result = releaseEngine(); + // Preserve the existing rollback semantics when release fails. + if (result != 0) { + // Undo earlier transition work and return the release error. + return rollback(result); } - engine_device_ = nil; } // -------------------------------------------------------------------------------------------- @@ -1993,20 +2072,18 @@ LOGI() << "Creating AVAudioEngine (device)..."; RTC_DCHECK(engine_device_ == nil); - engine_device_ = [[AVAudioEngine alloc] init]; - rollback_actions.push_back([=, this]() { RTC_DCHECK_RUN_ON(thread_); LOGI() << "Rolling back create AVAudioEngine (device)..."; engine_device_ = nil; }); - if (observer_ != nullptr) { - int32_t result = observer_->OnEngineDidCreate(engine_device_); - if (result != 0) { - LOGE() << "Call to OnEngineDidCreate returned error: " << result; - return rollback(result); - } + // Use the shared creation path so initial and retry engines are equivalent. + int32_t result = createEngine(); + // Preserve the existing rollback semantics when creation fails. + if (result != 0) { + // Undo earlier transition work and return the creation error. + return rollback(result); } } @@ -2037,15 +2114,14 @@ // -------------------------------------------------------------------------------------------- // Step: Trigger "engine will enable" event // - if (state.DidAnyEnable() && observer_ != nullptr) { - // Invoke here before configuring nodes. In iOS, session configuration is required before - // enabling AGC, muted talker etc. - int32_t result = observer_->OnEngineWillEnable(engine_device_, state.next.IsOutputEnabled(), - state.next.IsInputEnabled()); - if (result != 0) { - LOGE() << "Call to OnEngineWillEnable returned error: " << result; - return rollback(result); - } + // Invoke here before configuring nodes. In iOS, session configuration is + // required before enabling AGC, muted talker etc. + // Use the shared preparation path so retries preserve lifecycle ordering. + int32_t prepare_result = prepareEngine(); + // Do not touch nodes when the owner could not prepare the audio session. + if (prepare_result != 0) { + // Roll back the transition and return the preparation failure. + return rollback(prepare_result); } // -------------------------------------------------------------------------------------------- @@ -2054,7 +2130,75 @@ // - Note: We configure input VP before output to avoid artifacts playout during configuration. // if (state.next.IsInputEnabled()) { - ConfigureVoiceProcessingNode(inputNode(), state); + // Attempt VP before output setup so unprocessed input is never started. + int32_t voice_processing_result = + ConfigureVoiceProcessingNode(inputNode(), state); + // Count the first attempt so the retry bound includes all engine graphs. + UInt16 attempt = 1; + + // Retry only native VP failures and stop at the fixed recovery bound. + while (voice_processing_result != 0 && + attempt < kVoiceProcessingMaxRetries) { + // Make retry progress visible without exposing raw audio data. + LOGW() << "Retrying voice processing on a fresh engine (attempt " + << (attempt + 1) << "/" << kVoiceProcessingMaxRetries << ")"; + + // Fully release the failed graph before allocating its replacement. + int32_t release_result = releaseEngine(); + // Stop recovery if the failed graph cannot be released safely. + if (release_result != 0) { + // Roll back earlier work and return the release failure. + return rollback(release_result); + } + + // Avoid racing AVAudioEngine's asynchronous I/O-unit shutdown. + usleep(kVoiceProcessingRetryDelayMs * 1000); + + // Allocate a new graph rather than reconnecting the failed one. + int32_t create_result = createEngine(); + // Stop recovery if a replacement graph cannot be created. + if (create_result != 0) { + // Roll back earlier work and return the creation failure. + return rollback(create_result); + } + + // Reapply session preparation to the replacement engine. + prepare_result = prepareEngine(); + // Stop recovery if the replacement cannot be prepared. + if (prepare_result != 0) { + // Roll back earlier work and return the preparation failure. + return rollback(prepare_result); + } + + // Retry VP only after the replacement graph is fully prepared. + voice_processing_result = + ConfigureVoiceProcessingNode(inputNode(), state); + // Advance the bounded attempt count after each complete retry. + attempt++; + } + + // Recover the previous stable media state when every VP attempt fails. + if (voice_processing_result != 0) { + // Record the bounded failure for production crash diagnostics. + LOGE() << "Failed to configure voice processing after " << attempt + << " attempts"; + // Release the last failed graph before rebuilding the prior state. + int32_t release_result = releaseEngine(); + // The explicit prior-state recovery supersedes generic rollback actions. + rollback_actions.clear(); + // Avoid recovery on top of an engine that failed to release. + if (release_result != 0) { + // Surface the release failure because no stable state was restored. + return release_result; + } + + // Rebuild the last known stable state from a guaranteed blank graph. + EngineStateUpdate recovery_state = {{}, state.prev}; + // Use the normal transition machinery to restore observer and node state. + int32_t recovery_result = ApplyDeviceEngineState(recovery_state); + // Prefer a recovery error; otherwise preserve the original VP failure. + return recovery_result != 0 ? recovery_result : voice_processing_result; + } } // -------------------------------------------------------------------------------------------- @@ -2621,16 +2765,15 @@ if (state.prev.IsAnyEnabled() && !state.next.IsAnyEnabled()) { RTC_DCHECK(engine_device_ != nullptr); - if (observer_ != nullptr) { - int32_t result = observer_->OnEngineWillRelease(engine_device_); - if (result != 0) { - LOGE() << "Call to OnEngineWillRelease returned error: " << result; - return rollback(result); - } + // Use the shared release path so final shutdown matches recreation. + int32_t result = releaseEngine(); + // Preserve the existing rollback semantics when release fails. + if (result != 0) { + // Undo earlier transition work and return the release error. + return rollback(result); } LOGI() << "Releasing AVAudioEngine..."; - engine_device_ = nil; } return 0; @@ -2639,36 +2782,79 @@ // ---------------------------------------------------------------------------------------------------- // Private - EngineState -void AudioEngineDevice::ConfigureVoiceProcessingNode(AVAudioInputNode* input_node, - EngineStateUpdate state) { - if (state.next.IsInputEnabled() && input_node.voiceProcessingEnabled != state.next.voice_processing_enabled) { - #if TARGET_OS_SIMULATOR - LOGI() << "setVoiceProcessingEnabled (input): " - << (state.next.voice_processing_enabled ? "YES" : "NO") << " (Ignored on Simulator)"; - #else - LOGI() << "setVoiceProcessingEnabled (input): " << state.next.voice_processing_enabled ? "YES" - : "NO"; - NSError* error = nil; - BOOL set_vp_result = [input_node setVoiceProcessingEnabled:state.next.voice_processing_enabled - error:&error]; - if (!set_vp_result) { - NSLog(@"AudioEngineDevice setVoiceProcessingEnabled error: %@", error.localizedDescription); - RTC_DCHECK(set_vp_result); - } - LOGI() << "setVoiceProcessingEnabled (input) result: " << set_vp_result ? "YES" : "NO"; - #endif - - if (input_node.voiceProcessingEnabled) { - // Always unmute vp if restart mute mode. - if (state.next.mute_mode == MuteMode::RestartEngine && - input_node.voiceProcessingInputMuted) { - LOGI() << "Update mute (voice processing) unmuting vp for restart engine mode"; - input_node.voiceProcessingInputMuted = false; - } +// Return an error instead of continuing with input whose requested processing +// could not be applied. +int32_t AudioEngineDevice::ConfigureVoiceProcessingNode( + AVAudioInputNode* input_node, EngineStateUpdate state) { + // Skip native graph mutation when input is off or VP already matches. + if (!state.next.IsInputEnabled() || + input_node.voiceProcessingEnabled == + state.next.voice_processing_enabled) { + // Report success because no voice-processing transition is required. + return 0; + } - ConfigureMutedSpeechActivityEventListener(input_node, state); +#if TARGET_OS_SIMULATOR + // The simulator lacks the device I/O path, so retain its existing no-op. + LOGI() << "setVoiceProcessingEnabled (input): " + << (state.next.voice_processing_enabled ? "YES" : "NO") + << " (Ignored on Simulator)"; +#else + // Record the requested transition before entering the throwing Apple API. + LOGI() << "setVoiceProcessingEnabled (input): " + << (state.next.voice_processing_enabled ? "YES" : "NO"); + + // Capture NSError failures from the documented API contract. + NSError* error = nil; + // Default to failure so only an explicit native success may continue. + BOOL set_vp_result = NO; + // Convert Objective-C graph exceptions into the existing ADM error path. + @try { + // Apply VP on the prepared fresh graph and capture NSError details. + set_vp_result = [input_node + setVoiceProcessingEnabled:state.next.voice_processing_enabled + error:&error]; + } @catch (NSException* exception) { + // Preserve exception context without allowing it to terminate the process. + LOGE() << "setVoiceProcessingEnabled exception: " + << (exception.reason ? exception.reason.UTF8String : + "Unknown exception"); + // Trigger bounded fresh-engine recovery through the caller. + return kAudioEngineVoiceProcessingError; + } + + // Treat a reported native failure as equivalent to the caught exception. + if (!set_vp_result) { + // Preserve the NSError context for crash and device diagnostics. + LOGE() << "setVoiceProcessingEnabled error: " + << (error.localizedDescription ? + error.localizedDescription.UTF8String : + "Unknown error"); + // Trigger bounded fresh-engine recovery through the caller. + return kAudioEngineVoiceProcessingError; + } + + // Record that the requested processing state was applied successfully. + LOGI() << "setVoiceProcessingEnabled (input) result: YES"; +#endif + + // Configure VP-dependent features only when the input node confirms VP. + if (input_node.voiceProcessingEnabled) { + // Always unmute vp if restart mute mode. + if (state.next.mute_mode == MuteMode::RestartEngine && + input_node.voiceProcessingInputMuted) { + LOGI() << "Update mute (voice processing) unmuting vp for restart engine " + "mode"; + // Clear the VP mute left by the previous restart-based mute strategy. + input_node.voiceProcessingInputMuted = false; } - } + + // Restore muted-speech callbacks after the VP audio unit is recreated. + ConfigureMutedSpeechActivityEventListener(input_node, state); + } + + // Report success only after all VP-dependent configuration is complete. + return 0; } void AudioEngineDevice::ConfigureMutedSpeechActivityEventListener(AVAudioInputNode* input_node, diff --git a/sdk/objc/unittests/RTCPeerConnectionFactoryAudioEngine_xctest.mm b/sdk/objc/unittests/RTCPeerConnectionFactoryAudioEngine_xctest.mm index 135a16c1b5..c37f20711c 100644 --- a/sdk/objc/unittests/RTCPeerConnectionFactoryAudioEngine_xctest.mm +++ b/sdk/objc/unittests/RTCPeerConnectionFactoryAudioEngine_xctest.mm @@ -172,6 +172,49 @@ - (void)testAudioEngineDoesNotDropAudioTransportWhenPreparedBeforeOffer { [logger stop]; } +- (void)testEnablingRecordingWhilePlayoutIsRunningKeepsVoiceProcessingEnabled { + RETURN_IF_SIMULATOR_AUDIO_TEST_DISABLED(); + + AVAudioSession *session = [AVAudioSession sharedInstance]; + NSError *sessionError = nil; + XCTAssertTrue([session setCategory:AVAudioSessionCategoryPlayAndRecord + mode:AVAudioSessionModeVoiceChat + options:0 + error:&sessionError]); + XCTAssertNil(sessionError); + XCTAssertTrue([session setActive:YES error:&sessionError]); + XCTAssertNil(sessionError); + + RTC_OBJC_TYPE(RTCPeerConnectionFactory) *factory = + [[RTC_OBJC_TYPE(RTCPeerConnectionFactory) alloc] + initWithAudioDeviceModuleType:RTC_OBJC_TYPE( + RTCAudioDeviceModuleTypeAudioEngine) + bypassVoiceProcessing:NO + encoderFactory:nil + decoderFactory:nil + audioProcessingModule:nil]; + RTC_OBJC_TYPE(RTCAudioDeviceModule) *audioDeviceModule = + factory.audioDeviceModule; + + XCTAssertEqual(0, [audioDeviceModule initPlayout]); + XCTAssertEqual(0, [audioDeviceModule startPlayout]); + + for (NSUInteger iteration = 0; iteration < 5; iteration++) { + XCTAssertEqual(0, [audioDeviceModule initAndStartRecording]); + XCTAssertTrue(audioDeviceModule.isRecording); + XCTAssertTrue(audioDeviceModule.isVoiceProcessingEnabled); + XCTAssertEqual(0, [audioDeviceModule stopRecording]); + XCTAssertTrue(audioDeviceModule.isPlaying); + } + + XCTAssertEqual(0, [audioDeviceModule stopPlayout]); + XCTAssertTrue([session + setActive:NO + withOptions:AVAudioSessionSetActiveOptionNotifyOthersOnDeactivation + error:&sessionError]); + XCTAssertNil(sessionError); +} + @end #undef RETURN_IF_SIMULATOR_AUDIO_TEST_DISABLED From 2b3641c5b6843ac40f904807104ee1fa98d7f402 Mon Sep 17 00:00:00 2001 From: Ilias Pavlidakis <3liaspav@gmail.com> Date: Tue, 28 Jul 2026 14:38:43 +0300 Subject: [PATCH 2/2] Address feedback --- modules/audio_device/audio_engine_device.mm | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/modules/audio_device/audio_engine_device.mm b/modules/audio_device/audio_engine_device.mm index 3952aac62d..8a861b8a67 100644 --- a/modules/audio_device/audio_engine_device.mm +++ b/modules/audio_device/audio_engine_device.mm @@ -2192,8 +2192,20 @@ return release_result; } - // Rebuild the last known stable state from a guaranteed blank graph. - EngineStateUpdate recovery_state = {{}, state.prev}; + // Preserve the effective output state even when it was linked to input. + EngineState recovery_next = state.prev; + // Materialize linked output so speaker-only recovery keeps playout alive. + recovery_next.output_enabled = state.prev.IsOutputEnabled(); + // Preserve active playout while discarding the failed input transition. + recovery_next.output_running = state.prev.IsOutputRunning(); + // Disable regular input so recovery cannot configure voice processing. + recovery_next.input_enabled = false; + // Stop input with the graph so recording cannot outlive its failed VP. + recovery_next.input_running = false; + // Disable persistent input because it also contributes to IsInputEnabled. + recovery_next.input_enabled_persistent_mode = false; + // Rebuild the speaker-only state from a guaranteed blank graph. + EngineStateUpdate recovery_state = {{}, recovery_next}; // Use the normal transition machinery to restore observer and node state. int32_t recovery_result = ApplyDeviceEngineState(recovery_state); // Prefer a recovery error; otherwise preserve the original VP failure.