From 722b0dc10817c6dda6c092fc566301545371d495 Mon Sep 17 00:00:00 2001 From: henrika Date: Thu, 13 Oct 2016 01:12:33 -0700 Subject: [PATCH] Revert of Android audio playout now supports non-call media streams (patchset #3 id:10004 of https://codereview.webrtc.org/2411263003/ ) Reason for revert: There is a risk of ending up in a bad state due to race conditions with this patch. Tests in downstream clients have shown that it can happen that an output stream is opened up in MUSIC mode when it should not. Reverting since the new functionality added here is not worth the risk of breaking existing clients. Original issue's description: > Android audio playout now supports non-call media streams. > > The default (preferred) stream type for output audio is STREAM_VOICE_CALL since the WebRTC stack is mainly intended for VoIP calls. But if the user wants to run in another mode than COMM mode, we now accept it and change the stream type to STREAM_MUSIC instead. It can e.g. be suitable for applications that does not record audio or if a call shall be casted to a Chromecast device. > > The solution is somewhat experimental. > > NOTRY=TRUE > > BUG=webrtc:4767 > > Committed: https://crrev.com/872f614111f436d15e29516ce19c3b63d25b8639 > Cr-Commit-Position: refs/heads/master@{#14613} TBR=henrik.lundin@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:4767 Review-Url: https://codereview.webrtc.org/2420583002 Cr-Commit-Position: refs/heads/master@{#14626} --- .../android/audio_device_unittest.cc | 4 ---- .../audio_device/android/audio_manager.cc | 13 ------------- .../modules/audio_device/android/audio_manager.h | 5 ----- .../android/audio_manager_unittest.cc | 13 ------------- .../webrtc/voiceengine/WebRtcAudioManager.java | 10 ---------- .../org/webrtc/voiceengine/WebRtcAudioTrack.java | 16 +++------------- .../audio_device/android/opensles_player.cc | 7 ------- 7 files changed, 3 insertions(+), 65 deletions(-) diff --git a/webrtc/modules/audio_device/android/audio_device_unittest.cc b/webrtc/modules/audio_device/android/audio_device_unittest.cc index c1dba63897..eefab0f4e1 100644 --- a/webrtc/modules/audio_device/android/audio_device_unittest.cc +++ b/webrtc/modules/audio_device/android/audio_device_unittest.cc @@ -524,16 +524,12 @@ class AudioDeviceTest : public ::testing::Test { audio_device_ = CreateAudioDevice(AudioDeviceModule::kPlatformDefaultAudio); EXPECT_NE(audio_device_.get(), nullptr); EXPECT_EQ(0, audio_device_->Init()); - // Set audio mode to MODE_IN_COMMUNICATION. - audio_manager()->SetCommunicationMode(true); playout_parameters_ = audio_manager()->GetPlayoutAudioParameters(); record_parameters_ = audio_manager()->GetRecordAudioParameters(); build_info_.reset(new BuildInfo()); } virtual ~AudioDeviceTest() { EXPECT_EQ(0, audio_device_->Terminate()); - // Restore audio mode back to MODE_NORMAL. - audio_manager()->SetCommunicationMode(false); } int playout_sample_rate() const { diff --git a/webrtc/modules/audio_device/android/audio_manager.cc b/webrtc/modules/audio_device/android/audio_manager.cc index 5c88d9e5d4..d8b7640c9b 100644 --- a/webrtc/modules/audio_device/android/audio_manager.cc +++ b/webrtc/modules/audio_device/android/audio_manager.cc @@ -35,8 +35,6 @@ AudioManager::JavaAudioManager::JavaAudioManager( : audio_manager_(std::move(audio_manager)), init_(native_reg->GetMethodId("init", "()Z")), dispose_(native_reg->GetMethodId("dispose", "()V")), - set_communication_mode_( - native_reg->GetMethodId("setCommunicationMode", "(Z)V")), is_communication_mode_enabled_( native_reg->GetMethodId("isCommunicationModeEnabled", "()Z")), is_device_blacklisted_for_open_sles_usage_( @@ -57,11 +55,6 @@ void AudioManager::JavaAudioManager::Close() { audio_manager_->CallVoidMethod(dispose_); } -void AudioManager::JavaAudioManager::SetCommunicationMode(bool enable) { - audio_manager_->CallVoidMethod(set_communication_mode_, - static_cast(enable)); -} - bool AudioManager::JavaAudioManager::IsCommunicationModeEnabled() { return audio_manager_->CallBooleanMethod(is_communication_mode_enabled_); } @@ -183,12 +176,6 @@ bool AudioManager::Close() { return true; } -void AudioManager::SetCommunicationMode(bool enable) { - ALOGD("SetCommunicationMode(%d)", enable); - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - j_audio_manager_->SetCommunicationMode(enable); -} - bool AudioManager::IsCommunicationModeEnabled() const { ALOGD("IsCommunicationModeEnabled()"); RTC_DCHECK(thread_checker_.CalledOnValidThread()); diff --git a/webrtc/modules/audio_device/android/audio_manager.h b/webrtc/modules/audio_device/android/audio_manager.h index 8c32f0b082..341d426e41 100644 --- a/webrtc/modules/audio_device/android/audio_manager.h +++ b/webrtc/modules/audio_device/android/audio_manager.h @@ -47,7 +47,6 @@ class AudioManager { bool Init(); void Close(); - void SetCommunicationMode(bool enable); bool IsCommunicationModeEnabled(); bool IsDeviceBlacklistedForOpenSLESUsage(); @@ -55,7 +54,6 @@ class AudioManager { std::unique_ptr audio_manager_; jmethodID init_; jmethodID dispose_; - jmethodID set_communication_mode_; jmethodID is_communication_mode_enabled_; jmethodID is_device_blacklisted_for_open_sles_usage_; }; @@ -84,9 +82,6 @@ class AudioManager { // Revert any setting done by Init(). bool Close(); - // Set audio mode to AudioManager.MODE_IN_COMMUNICATION if |enable| is true - // and AudioManager.MODE_NORMAL otherwise. - void SetCommunicationMode(bool enable); // Returns true if current audio mode is AudioManager.MODE_IN_COMMUNICATION. bool IsCommunicationModeEnabled() const; diff --git a/webrtc/modules/audio_device/android/audio_manager_unittest.cc b/webrtc/modules/audio_device/android/audio_manager_unittest.cc index cb04beff76..2507976522 100644 --- a/webrtc/modules/audio_device/android/audio_manager_unittest.cc +++ b/webrtc/modules/audio_device/android/audio_manager_unittest.cc @@ -35,11 +35,6 @@ class AudioManagerTest : public ::testing::Test { record_parameters_ = audio_manager()->GetRecordAudioParameters(); } - virtual ~AudioManagerTest() { - // Always ensure that we restore default/normal mode after the test. - audio_manager()->SetCommunicationMode(false); - } - AudioManager* audio_manager() const { return audio_manager_.get(); } // A valid audio layer must always be set before calling Init(), hence we @@ -112,14 +107,6 @@ TEST_F(AudioManagerTest, InitClose) { EXPECT_TRUE(audio_manager()->Close()); } -// Verify communication mode functionality. -TEST_F(AudioManagerTest, CommunicationMode) { - audio_manager()->SetCommunicationMode(true); - EXPECT_TRUE(audio_manager()->IsCommunicationModeEnabled()); - audio_manager()->SetCommunicationMode(false); - EXPECT_FALSE(audio_manager()->IsCommunicationModeEnabled()); -} - TEST_F(AudioManagerTest, IsAcousticEchoCancelerSupported) { PRINT("%sAcoustic Echo Canceler support: %s\n", kTag, audio_manager()->IsAcousticEchoCancelerSupported() ? "Yes" : "No"); diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java index 2775118ca7..fb0516fa06 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java @@ -172,16 +172,6 @@ public class WebRtcAudioManager { volumeLogger.stop(); } - private void setCommunicationMode(boolean enable) { - if (enable) { - audioManager.setMode(AudioManager.MODE_IN_COMMUNICATION); - Logging.d(TAG, "audio mode is set to AudioManager.MODE_IN_COMMUNICATION"); - } else { - audioManager.setMode(AudioManager.MODE_NORMAL); - Logging.d(TAG, "audio mode is set to AudioManager.MODE_NORMAL"); - } - } - private boolean isCommunicationModeEnabled() { return (audioManager.getMode() == AudioManager.MODE_IN_COMMUNICATION); } diff --git a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java index 825f627d03..c2874316a5 100644 --- a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java +++ b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java @@ -188,17 +188,6 @@ public class WebRtcAudioTrack { Logging.e(TAG, "AudioTrack.getMinBufferSize returns an invalid value."); return false; } - // The default (preferred) stream type is STREAM_VOICE_CALL since the - // WebRTC stack is mainly intended for VoIP calls. - int streamType = AudioManager.STREAM_VOICE_CALL; - if (audioManager.getMode() != AudioManager.MODE_IN_COMMUNICATION) { - // But if the user wants to run in another mode than COMM mode, we - // accept it and change the stream type to STREAM_MUSIC instead. It can - // e.g. be suitable for applications that do not record audio or if a - // call shall be casted to a Chromecast device. - streamType = AudioManager.STREAM_MUSIC; - Logging.w(TAG, "Using AudioManager.STREAM_MUSIC as stream type."); - } // Ensure that prevision audio session was stopped correctly before trying // to create a new AudioTrack. @@ -210,8 +199,9 @@ public class WebRtcAudioTrack { // Create an AudioTrack object and initialize its associated audio buffer. // The size of this buffer determines how long an AudioTrack can play // before running out of data. - audioTrack = new AudioTrack(streamType, sampleRate, AudioFormat.CHANNEL_OUT_MONO, - AudioFormat.ENCODING_PCM_16BIT, minBufferSizeInBytes, AudioTrack.MODE_STREAM); + audioTrack = + new AudioTrack(AudioManager.STREAM_VOICE_CALL, sampleRate, AudioFormat.CHANNEL_OUT_MONO, + AudioFormat.ENCODING_PCM_16BIT, minBufferSizeInBytes, AudioTrack.MODE_STREAM); } catch (IllegalArgumentException e) { Logging.d(TAG, e.getMessage()); return false; diff --git a/webrtc/modules/audio_device/android/opensles_player.cc b/webrtc/modules/audio_device/android/opensles_player.cc index b87c5fe5ca..d675d637e3 100644 --- a/webrtc/modules/audio_device/android/opensles_player.cc +++ b/webrtc/modules/audio_device/android/opensles_player.cc @@ -308,13 +308,6 @@ bool OpenSLESPlayer::CreateAudioPlayer() { // Set audio player configuration to SL_ANDROID_STREAM_VOICE which // corresponds to android.media.AudioManager.STREAM_VOICE_CALL. SLint32 stream_type = SL_ANDROID_STREAM_VOICE; - if (!audio_manager_->IsCommunicationModeEnabled()) { - // But use STREAM_MUSIC if the user for some reason wants to run in a mode - // other than the preferred communication mode. - // SL_ANDROID_STREAM_MEDIA <=> android.media.AudioManager.STREAM_MUSIC. - stream_type = SL_ANDROID_STREAM_MEDIA; - ALOGW("Using non-default stream type SL_ANDROID_STREAM_MEDIA"); - } RETURN_ON_ERROR( (*player_config) ->SetConfiguration(player_config, SL_ANDROID_KEY_STREAM_TYPE,