From d8c6ec4d2fd6804bae442c51788dfa5487ede3e2 Mon Sep 17 00:00:00 2001 From: henrika Date: Thu, 18 Jul 2019 15:17:28 +0200 Subject: [PATCH] Adds support for disabling autostart in ADM2 for Windows Landing with TBR given vacation times and the fact that none of this code is active "in production". The ADM2 implementation can be seen as experimental (non-default) code and it takes some work to enable it and replace the existing ADM. Hence, extremely low risk to break anything. TBR: henrik.lundin Bug: webrtc:9265 Change-Id: Ia5cfb2aaa8eaf9537b916b3375f55d8df6287071 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145921 Reviewed-by: Henrik Andreassson Commit-Queue: Henrik Andreassson Cr-Commit-Position: refs/heads/master@{#28600} --- modules/audio_device/audio_device_unittest.cc | 4 ++-- .../include/audio_device_factory.cc | 14 +++++++---- .../include/audio_device_factory.h | 17 +++++++++++--- .../win/audio_device_module_win.cc | 1 + .../audio_device/win/core_audio_base_win.cc | 23 +++++++++++++++++++ .../audio_device/win/core_audio_base_win.h | 3 +++ .../audio_device/win/core_audio_input_win.cc | 22 ++++++++++++------ .../audio_device/win/core_audio_input_win.h | 2 +- .../audio_device/win/core_audio_output_win.cc | 22 ++++++++++++------ .../audio_device/win/core_audio_output_win.h | 2 +- 10 files changed, 84 insertions(+), 26 deletions(-) diff --git a/modules/audio_device/audio_device_unittest.cc b/modules/audio_device/audio_device_unittest.cc index 361d2ce799..f0bf9fd8ce 100644 --- a/modules/audio_device/audio_device_unittest.cc +++ b/modules/audio_device/audio_device_unittest.cc @@ -155,7 +155,7 @@ class FifoAudioStream : public AudioStream { } // Add marker once per second to signal that audio is active. if (write_count_++ % 100 == 0) { - PRINT("."); + PRINTD("."); } written_elements_ += size; } @@ -597,7 +597,7 @@ class AudioDeviceTest EXPECT_TRUE(webrtc_win::core_audio_utility::IsSupported()); EXPECT_TRUE(webrtc_win::core_audio_utility::IsMMCSSSupported()); return CreateWindowsCoreAudioAudioDeviceModuleForTest( - task_queue_factory_.get()); + task_queue_factory_.get(), true); #else return nullptr; #endif diff --git a/modules/audio_device/include/audio_device_factory.cc b/modules/audio_device/include/audio_device_factory.cc index 1962e57ee5..df5129f3c1 100644 --- a/modules/audio_device/include/audio_device_factory.cc +++ b/modules/audio_device/include/audio_device_factory.cc @@ -24,14 +24,17 @@ namespace webrtc { rtc::scoped_refptr CreateWindowsCoreAudioAudioDeviceModule( - TaskQueueFactory* task_queue_factory) { + TaskQueueFactory* task_queue_factory, + bool automatic_restart) { RTC_DLOG(INFO) << __FUNCTION__; - return CreateWindowsCoreAudioAudioDeviceModuleForTest(task_queue_factory); + return CreateWindowsCoreAudioAudioDeviceModuleForTest(task_queue_factory, + automatic_restart); } rtc::scoped_refptr CreateWindowsCoreAudioAudioDeviceModuleForTest( - TaskQueueFactory* task_queue_factory) { + TaskQueueFactory* task_queue_factory, + bool automatic_restart) { RTC_DLOG(INFO) << __FUNCTION__; // Returns NULL if Core Audio is not supported or if COM has not been // initialized correctly using webrtc_win::ScopedCOMInitializer. @@ -41,8 +44,9 @@ CreateWindowsCoreAudioAudioDeviceModuleForTest( return nullptr; } return CreateWindowsCoreAudioAudioDeviceModuleFromInputAndOutput( - absl::make_unique(), - absl::make_unique(), task_queue_factory); + absl::make_unique(automatic_restart), + absl::make_unique(automatic_restart), + task_queue_factory); } } // namespace webrtc diff --git a/modules/audio_device/include/audio_device_factory.h b/modules/audio_device/include/audio_device_factory.h index f7ad71b7ea..4d4cb5eaf8 100644 --- a/modules/audio_device/include/audio_device_factory.h +++ b/modules/audio_device/include/audio_device_factory.h @@ -19,28 +19,39 @@ namespace webrtc { // Creates an AudioDeviceModule (ADM) for Windows based on the Core Audio API. // The creating thread must be a COM thread; otherwise nullptr will be returned. +// By default |automatic_restart| is set to true and it results in support for +// automatic restart of audio if e.g. the existing device is removed. If set to +// false, no attempt to restart audio is performed under these conditions. +// // Example (assuming webrtc namespace): // // public: // rtc::scoped_refptr CreateAudioDevice() { +// task_queue_factory_ = CreateDefaultTaskQueueFactory(); // // Tell COM that this thread shall live in the MTA. // com_initializer_ = absl::make_unique( // webrtc_win::ScopedCOMInitializer::kMTA); // if (!com_initializer_->Succeeded()) { // return nullptr; // } -// return CreateWindowsCoreAudioAudioDeviceModule(); +// // Create the ADM with support for automatic restart if devices are +// // unplugged. +// return CreateWindowsCoreAudioAudioDeviceModule( +// task_queue_factory_.get()); // } // // private: // std::unique_ptr com_initializer_; +// std::unique_ptr task_queue_factory_; // rtc::scoped_refptr CreateWindowsCoreAudioAudioDeviceModule( - TaskQueueFactory* task_queue_factory); + TaskQueueFactory* task_queue_factory, + bool automatic_restart = true); rtc::scoped_refptr CreateWindowsCoreAudioAudioDeviceModuleForTest( - TaskQueueFactory* task_queue_factory); + TaskQueueFactory* task_queue_factory, + bool automatic_restart = true); } // namespace webrtc diff --git a/modules/audio_device/win/audio_device_module_win.cc b/modules/audio_device/win/audio_device_module_win.cc index 47d1ff79f5..cc23ae6320 100644 --- a/modules/audio_device/win/audio_device_module_win.cc +++ b/modules/audio_device/win/audio_device_module_win.cc @@ -496,6 +496,7 @@ class WindowsAudioDeviceModule : public AudioDeviceModuleForTest { const std::unique_ptr output_; TaskQueueFactory* const task_queue_factory_; + // The AudioDeviceBuffer (ADB) instance is needed for sending/receiving audio // to/from the WebRTC layer. Created and owned by this object. Used by // both |input_| and |output_| but they use orthogonal parts of the ADB. diff --git a/modules/audio_device/win/core_audio_base_win.cc b/modules/audio_device/win/core_audio_base_win.cc index bd19001b36..b18d97d99e 100644 --- a/modules/audio_device/win/core_audio_base_win.cc +++ b/modules/audio_device/win/core_audio_base_win.cc @@ -126,15 +126,18 @@ bool IsLowLatencySupported(IAudioClient3* client3, } // namespace CoreAudioBase::CoreAudioBase(Direction direction, + bool automatic_restart, OnDataCallback data_callback, OnErrorCallback error_callback) : format_(), direction_(direction), + automatic_restart_(automatic_restart), on_data_callback_(data_callback), on_error_callback_(error_callback), device_index_(kUndefined), is_restarting_(false) { RTC_DLOG(INFO) << __FUNCTION__ << "[" << DirectionToString(direction) << "]"; + RTC_DLOG(INFO) << "Automatic restart: " << automatic_restart; RTC_DLOG(INFO) << "Windows version: " << rtc::rtc_win::GetVersion(); // Create the event which the audio engine will signal each time a buffer @@ -640,6 +643,9 @@ bool CoreAudioBase::IsVolumeControlAvailable(bool* available) const { bool CoreAudioBase::Restart() { RTC_DLOG(INFO) << __FUNCTION__ << "[" << DirectionToString(direction()) << "]"; + if (!automatic_restart()) { + return false; + } is_restarting_ = true; SetEvent(restart_event_.Get()); return true; @@ -765,11 +771,28 @@ HRESULT CoreAudioBase::OnStateChanged(AudioSessionState new_state) { // When a session is disconnected because of a device removal or format change // event, we want to inform the audio thread about the lost audio session and // trigger an attempt to restart audio using a new (default) device. +// This method is called on separate threads owned by the session manager and +// it can happen that the same type of callback is called more than once for the +// same event. HRESULT CoreAudioBase::OnSessionDisconnected( AudioSessionDisconnectReason disconnect_reason) { RTC_DLOG(INFO) << "___" << __FUNCTION__ << "[" << DirectionToString(direction()) << "] reason: " << SessionDisconnectReasonToString(disconnect_reason); + // Ignore changes in the audio session (don't try to restart) if the user + // has explicitly asked for this type of ADM during construction. + if (!automatic_restart()) { + RTC_DLOG(LS_WARNING) << "___Automatic restart is disabled"; + return S_OK; + } + + if (IsRestarting()) { + RTC_DLOG(LS_WARNING) << "___Ignoring since restart is already active"; + return S_OK; + } + + // By default, automatic restart is enabled and the restart event will be set + // below if the device was removed or the format was changed. if (disconnect_reason == DisconnectReasonDeviceRemoval || disconnect_reason == DisconnectReasonFormatChanged) { is_restarting_ = true; diff --git a/modules/audio_device/win/core_audio_base_win.h b/modules/audio_device/win/core_audio_base_win.h index 56efc56b27..3e33d689aa 100644 --- a/modules/audio_device/win/core_audio_base_win.h +++ b/modules/audio_device/win/core_audio_base_win.h @@ -77,6 +77,7 @@ class CoreAudioBase : public IAudioSessionEvents { protected: explicit CoreAudioBase(Direction direction, + bool automatic_restart, OnDataCallback data_callback, OnErrorCallback error_callback); ~CoreAudioBase(); @@ -97,6 +98,7 @@ class CoreAudioBase : public IAudioSessionEvents { bool Restart(); Direction direction() const { return direction_; } + bool automatic_restart() const { return automatic_restart_; } // Releases all allocated COM resources in the base class. void ReleaseCOMObjects(); @@ -141,6 +143,7 @@ class CoreAudioBase : public IAudioSessionEvents { private: const Direction direction_; + const bool automatic_restart_; const OnDataCallback on_data_callback_; const OnErrorCallback on_error_callback_; ScopedHandle audio_samples_event_; diff --git a/modules/audio_device/win/core_audio_input_win.cc b/modules/audio_device/win/core_audio_input_win.cc index 3b523f6932..8405e6d61e 100644 --- a/modules/audio_device/win/core_audio_input_win.cc +++ b/modules/audio_device/win/core_audio_input_win.cc @@ -26,10 +26,12 @@ enum AudioDeviceMessageType : uint32_t { kMessageInputStreamDisconnected, }; -CoreAudioInput::CoreAudioInput() - : CoreAudioBase(CoreAudioBase::Direction::kInput, - [this](uint64_t freq) { return OnDataCallback(freq); }, - [this](ErrorType err) { return OnErrorCallback(err); }) { +CoreAudioInput::CoreAudioInput(bool automatic_restart) + : CoreAudioBase( + CoreAudioBase::Direction::kInput, + automatic_restart, + [this](uint64_t freq) { return OnDataCallback(freq); }, + [this](ErrorType err) { return OnErrorCallback(err); }) { RTC_DLOG(INFO) << __FUNCTION__; RTC_DCHECK_RUN_ON(&thread_checker_); thread_checker_audio_.Detach(); @@ -149,14 +151,16 @@ int CoreAudioInput::InitRecording() { int CoreAudioInput::StartRecording() { RTC_DLOG(INFO) << __FUNCTION__; RTC_DCHECK(!Recording()); + RTC_DCHECK(fine_audio_buffer_); + RTC_DCHECK(audio_device_buffer_); if (!initialized_) { RTC_DLOG(LS_WARNING) << "Recording can not start since InitRecording must succeed first"; return 0; } - if (fine_audio_buffer_) { - fine_audio_buffer_->ResetRecord(); - } + + fine_audio_buffer_->ResetRecord(); + audio_device_buffer_->StartRecording(); if (!Start()) { return -1; @@ -186,6 +190,9 @@ int CoreAudioInput::StopRecording() { return -1; } + RTC_DCHECK(audio_device_buffer_); + audio_device_buffer_->StopRecording(); + // Release all allocated resources to allow for a restart without // intermediate destruction. ReleaseCOMObjects(); @@ -405,6 +412,7 @@ absl::optional CoreAudioInput::EstimateLatencyMillis( bool CoreAudioInput::HandleStreamDisconnected() { RTC_DLOG(INFO) << "<<<--- " << __FUNCTION__; RTC_DCHECK_RUN_ON(&thread_checker_audio_); + RTC_DCHECK(automatic_restart()); if (StopRecording() != 0) { return false; diff --git a/modules/audio_device/win/core_audio_input_win.h b/modules/audio_device/win/core_audio_input_win.h index 709dced9f3..be290f9f4e 100644 --- a/modules/audio_device/win/core_audio_input_win.h +++ b/modules/audio_device/win/core_audio_input_win.h @@ -30,7 +30,7 @@ namespace webrtc_win { // and streaming of captured audio to a WebRTC client. class CoreAudioInput final : public CoreAudioBase, public AudioInput { public: - CoreAudioInput(); + CoreAudioInput(bool automatic_restart); ~CoreAudioInput() override; // AudioInput implementation. diff --git a/modules/audio_device/win/core_audio_output_win.cc b/modules/audio_device/win/core_audio_output_win.cc index 16cf10d907..6921805395 100644 --- a/modules/audio_device/win/core_audio_output_win.cc +++ b/modules/audio_device/win/core_audio_output_win.cc @@ -23,10 +23,12 @@ using Microsoft::WRL::ComPtr; namespace webrtc { namespace webrtc_win { -CoreAudioOutput::CoreAudioOutput() - : CoreAudioBase(CoreAudioBase::Direction::kOutput, - [this](uint64_t freq) { return OnDataCallback(freq); }, - [this](ErrorType err) { return OnErrorCallback(err); }) { +CoreAudioOutput::CoreAudioOutput(bool automatic_restart) + : CoreAudioBase( + CoreAudioBase::Direction::kOutput, + automatic_restart, + [this](uint64_t freq) { return OnDataCallback(freq); }, + [this](ErrorType err) { return OnErrorCallback(err); }) { RTC_DLOG(INFO) << __FUNCTION__; RTC_DCHECK_RUN_ON(&thread_checker_); thread_checker_audio_.Detach(); @@ -146,13 +148,15 @@ int CoreAudioOutput::InitPlayout() { int CoreAudioOutput::StartPlayout() { RTC_DLOG(INFO) << __FUNCTION__ << ": " << IsRestarting(); RTC_DCHECK(!Playing()); + RTC_DCHECK(fine_audio_buffer_); + RTC_DCHECK(audio_device_buffer_); if (!initialized_) { RTC_DLOG(LS_WARNING) << "Playout can not start since InitPlayout must succeed first"; } - if (fine_audio_buffer_) { - fine_audio_buffer_->ResetPlayout(); - } + + fine_audio_buffer_->ResetPlayout(); + audio_device_buffer_->StartPlayout(); if (!core_audio_utility::FillRenderEndpointBufferWithSilence( audio_client_.Get(), audio_render_client_.Get())) { @@ -189,6 +193,9 @@ int CoreAudioOutput::StopPlayout() { return -1; } + RTC_DCHECK(audio_device_buffer_); + audio_device_buffer_->StopPlayout(); + // Release all allocated resources to allow for a restart without // intermediate destruction. ReleaseCOMObjects(); @@ -381,6 +388,7 @@ int CoreAudioOutput::EstimateOutputLatencyMillis(uint64_t device_frequency) { bool CoreAudioOutput::HandleStreamDisconnected() { RTC_DLOG(INFO) << "<<<--- " << __FUNCTION__; RTC_DCHECK_RUN_ON(&thread_checker_audio_); + RTC_DCHECK(automatic_restart()); if (StopPlayout() != 0) { return false; diff --git a/modules/audio_device/win/core_audio_output_win.h b/modules/audio_device/win/core_audio_output_win.h index f0f619756b..5a547498a3 100644 --- a/modules/audio_device/win/core_audio_output_win.h +++ b/modules/audio_device/win/core_audio_output_win.h @@ -30,7 +30,7 @@ namespace webrtc_win { // layer. class CoreAudioOutput final : public CoreAudioBase, public AudioOutput { public: - CoreAudioOutput(); + CoreAudioOutput(bool automatic_restart); ~CoreAudioOutput() override; // AudioOutput implementation.