-
Couldn't load subscription status.
- Fork 3
[Enhancement]Stereo playout #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: patch/m137.5
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds stereo playout support to the audio engine and exposes availability/enabled properties in the Objective-C API.
- Introduces API properties to query and toggle stereo playout.
- Implements engine-side stereo channel negotiation, route capability checks, and voice processing overrides.
- Extends engine state management and render paths to handle variable (mono/stereo) output channels.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/objc/api/peerconnection/RTCAudioDeviceModule.h | Adds stereo playout availability and enablement properties to public API. |
| sdk/objc/api/peerconnection/RTCAudioDeviceModule.mm | Implements property accessors and setter invoking native stereo playout logic. |
| modules/audio_device/audio_engine_device.h | Extends EngineState, state update logic, and public/internal methods for stereo handling. |
| modules/audio_device/audio_engine_device.mm | Adds stereo capability checks, playout channel negotiation, voice processing override/fallback, and channel-aware render paths. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| stereo_voice_processing_override_active_) { | ||
| *restore_voice_processing = true; | ||
| state.next.voice_processing_enabled = stereo_saved_voice_processing_enabled_; | ||
| state.next.voice_processing_bypassed = stereo_saved_voice_processing_bypassed_; | ||
| #if !TARGET_OS_SIMULATOR | ||
| if (inputNode().voiceProcessingEnabled != state.next.voice_processing_enabled) { | ||
| NSError* vp_error = nil; | ||
| BOOL set_vp = [inputNode() setVoiceProcessingEnabled:state.next.voice_processing_enabled | ||
| error:&vp_error]; | ||
| if (!set_vp) { | ||
| LOGE() << "Failed to restore voice processing: " | ||
| << (vp_error ? vp_error.localizedDescription.UTF8String : "unknown"); | ||
| state.next.voice_processing_enabled = inputNode().voiceProcessingEnabled; | ||
| state.next.voice_processing_bypassed = inputNode().voiceProcessingBypassed; | ||
| if (restore_voice_processing) { | ||
| *restore_voice_processing = false; | ||
| } | ||
| stereo_voice_processing_override_active_ = false; | ||
| } else if (state.next.voice_processing_enabled && | ||
| inputNode().voiceProcessingBypassed != state.next.voice_processing_bypassed) { | ||
| inputNode().voiceProcessingBypassed = state.next.voice_processing_bypassed; | ||
| } | ||
| } else if (state.next.voice_processing_enabled && | ||
| inputNode().voiceProcessingBypassed != state.next.voice_processing_bypassed) { | ||
| inputNode().voiceProcessingBypassed = state.next.voice_processing_bypassed; | ||
| } | ||
| #endif | ||
| } | ||
| } else if (stereo_playout_reset && requested_channels < 2 && state.next.stereo_playout_enabled) { | ||
| // Handle cases where desired stereo was set but route currently only reports mono channels. | ||
| *stereo_playout_reset = true; | ||
| if (restore_voice_processing && stereo_voice_processing_override_active_) { | ||
| *restore_voice_processing = true; | ||
| state.next.voice_processing_enabled = stereo_saved_voice_processing_enabled_; | ||
| state.next.voice_processing_bypassed = stereo_saved_voice_processing_bypassed_; | ||
| #if !TARGET_OS_SIMULATOR | ||
| if (inputNode().voiceProcessingEnabled != state.next.voice_processing_enabled) { | ||
| NSError* vp_error = nil; | ||
| BOOL set_vp = [inputNode() setVoiceProcessingEnabled:state.next.voice_processing_enabled | ||
| error:&vp_error]; | ||
| if (!set_vp) { | ||
| LOGE() << "Failed to restore voice processing: " | ||
| << (vp_error ? vp_error.localizedDescription.UTF8String : "unknown"); | ||
| state.next.voice_processing_enabled = inputNode().voiceProcessingEnabled; | ||
| state.next.voice_processing_bypassed = inputNode().voiceProcessingBypassed; | ||
| if (restore_voice_processing) { | ||
| *restore_voice_processing = false; | ||
| } | ||
| stereo_voice_processing_override_active_ = false; | ||
| } else if (state.next.voice_processing_enabled && | ||
| inputNode().voiceProcessingBypassed != state.next.voice_processing_bypassed) { | ||
| inputNode().voiceProcessingBypassed = state.next.voice_processing_bypassed; | ||
| } | ||
| } else if (state.next.voice_processing_enabled && | ||
| inputNode().voiceProcessingBypassed != state.next.voice_processing_bypassed) { | ||
| inputNode().voiceProcessingBypassed = state.next.voice_processing_bypassed; | ||
| } | ||
| #endif | ||
| } |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The voice processing restore logic is duplicated in two branches (requested_channels fallback and mono route case). Extract this into a helper (e.g. RestoreVoiceProcessingIfNeeded(state, restore_voice_processing)) to reduce duplication and future maintenance risk.
| [center removeObserver:(__bridge_transfer id)configuration_observer_ | ||
| name:AVAudioEngineConfigurationChangeNotification | ||
| object:nil]; |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using __bridge_transfer when removing the existing observer releases the retained object during removeObserver, which is unconventional; removeObserver does not assume ownership. Prefer (__bridge id) for removal followed by CFRelease/ARC release to avoid unintended lifetime side effects, or wrap observer management in a RAII/objc helper.
| [center removeObserver:(__bridge_transfer id)configuration_observer_ | |
| name:AVAudioEngineConfigurationChangeNotification | |
| object:nil]; | |
| [center removeObserver:(__bridge id)configuration_observer_ | |
| name:AVAudioEngineConfigurationChangeNotification | |
| object:nil]; | |
| CFRelease(configuration_observer_); |
Summary
This revision adds support on ADM for stereo playout control. The changes were made in the
audio_engin_deviceand specifically target the AVAudioEngine deviceModule.Usage
The ADM now provides API that can be observed around StereoPlayout availability and status. In order to toggle Stereo Playout based on the current route or user input you can do the following