diff --git a/content/renderer/media/audio/audio_device_factory.cc b/content/renderer/media/audio/audio_device_factory.cc index 624021608199f0..3f7f5275fd1d9d 100644 --- a/content/renderer/media/audio/audio_device_factory.cc +++ b/content/renderer/media/audio/audio_device_factory.cc @@ -78,10 +78,9 @@ scoped_refptr NewMixableSink( DCHECK(render_thread) << "RenderThreadImpl is not instantiated, or " << "GetOutputDeviceInfo() is called on a wrong thread "; DCHECK(!params.processing_id.has_value()); - return scoped_refptr( - render_thread->GetAudioRendererMixerManager()->CreateInput( - render_frame_id, params.session_id, params.device_id, - AudioDeviceFactory::GetSourceLatencyType(source_type))); + return render_thread->GetAudioRendererMixerManager()->CreateInput( + render_frame_id, params.session_id, params.device_id, + AudioDeviceFactory::GetSourceLatencyType(source_type)); } } // namespace @@ -110,8 +109,9 @@ scoped_refptr AudioDeviceFactory::NewAudioRendererMixerSink( int render_frame_id, const media::AudioSinkParameters& params) { - return NewFinalAudioRendererSink(render_frame_id, params, - GetDefaultAuthTimeout()); + // AudioRendererMixer sinks are always used asynchronously and thus can + // operate without a timeout value. + return NewFinalAudioRendererSink(render_frame_id, params, base::TimeDelta()); } // static diff --git a/content/renderer/media/audio/audio_device_factory.h b/content/renderer/media/audio/audio_device_factory.h index 38795beeecc557..79ec56d88e1f20 100644 --- a/content/renderer/media/audio/audio_device_factory.h +++ b/content/renderer/media/audio/audio_device_factory.h @@ -51,9 +51,11 @@ class CONTENT_EXPORT AudioDeviceFactory { static media::AudioLatency::LatencyType GetSourceLatencyType( SourceType source); - // Creates a sink for AudioRendererMixer. - // |render_frame_id| refers to the RenderFrame containing the entity - // producing the audio. + // Creates a sink for AudioRendererMixer. |render_frame_id| refers to the + // RenderFrame containing the entity producing the audio. Note: These sinks do + // not support the blocking GetOutputDeviceInfo() API and instead clients are + // required to use the GetOutputDeviceInfoAsync() API. As such they are + // configured with no authorization timeout value. static scoped_refptr NewAudioRendererMixerSink( int render_frame_id, const media::AudioSinkParameters& params); diff --git a/content/renderer/media/audio/audio_renderer_mixer_manager.cc b/content/renderer/media/audio/audio_renderer_mixer_manager.cc index fd9c6022806d1d..ee7e296ca42623 100644 --- a/content/renderer/media/audio/audio_renderer_mixer_manager.cc +++ b/content/renderer/media/audio/audio_renderer_mixer_manager.cc @@ -147,7 +147,8 @@ std::unique_ptr AudioRendererMixerManager::Create() { base::BindRepeating(&AudioDeviceFactory::NewAudioRendererMixerSink))); } -media::AudioRendererMixerInput* AudioRendererMixerManager::CreateInput( +scoped_refptr +AudioRendererMixerManager::CreateInput( int source_render_frame_id, int session_id, const std::string& device_id, @@ -155,23 +156,26 @@ media::AudioRendererMixerInput* AudioRendererMixerManager::CreateInput( // AudioRendererMixerManager lives on the renderer thread and is destroyed on // renderer thread destruction, so it's safe to pass its pointer to a mixer // input. - return new media::AudioRendererMixerInput( - this, source_render_frame_id, - media::AudioDeviceDescription::UseSessionIdToSelectDevice(session_id, - device_id) - ? GetOutputDeviceInfo(source_render_frame_id, session_id, device_id) - .device_id() - : device_id, - latency); + // + // TODO(olka, grunell): |session_id| is always zero, delete since + // NewAudioRenderingMixingStrategy didn't ship, https://crbug.com/870836. + DCHECK_EQ(session_id, 0); + return base::MakeRefCounted( + this, source_render_frame_id, device_id, latency); } media::AudioRendererMixer* AudioRendererMixerManager::GetMixer( int source_render_frame_id, const media::AudioParameters& input_params, media::AudioLatency::LatencyType latency, - const std::string& device_id, - media::OutputDeviceStatus* device_status) { - const MixerKey key(source_render_frame_id, input_params, latency, device_id); + const media::OutputDeviceInfo& sink_info, + scoped_refptr sink) { + // Ownership of the sink must be given to GetMixer(). + DCHECK(sink->HasOneRef()); + DCHECK_EQ(sink_info.device_status(), media::OUTPUT_DEVICE_STATUS_OK); + + const MixerKey key(source_render_frame_id, input_params, latency, + sink_info.device_id()); base::AutoLock auto_lock(mixers_lock_); // Update latency map when the mixer is requested, i.e. there is an attempt to @@ -189,27 +193,22 @@ media::AudioRendererMixer* AudioRendererMixerManager::GetMixer( auto it = mixers_.find(key); if (it != mixers_.end()) { - if (device_status) - *device_status = media::OUTPUT_DEVICE_STATUS_OK; - it->second.ref_count++; DVLOG(1) << "Reusing mixer: " << it->second.mixer; - return it->second.mixer; - } - scoped_refptr sink = create_sink_cb_.Run( - source_render_frame_id, media::AudioSinkParameters(0, device_id)); - - const media::OutputDeviceInfo& device_info = sink->GetOutputDeviceInfo(); - if (device_status) - *device_status = device_info.device_status(); - if (device_info.device_status() != media::OUTPUT_DEVICE_STATUS_OK) { + // Sink will now be released unused, but still must be stopped. + // + // TODO(dalecurtis): Is it worth caching this sink instead for a future + // GetSink() call? We should experiment with a few top sites. We can't just + // drop in AudioRendererSinkCache here since it doesn't reuse sinks once + // they've been vended externally to the class. sink->Stop(); - return nullptr; + + return it->second.mixer; } const media::AudioParameters& mixer_output_params = - GetMixerOutputParams(input_params, device_info.output_params(), latency); + GetMixerOutputParams(input_params, sink_info.output_params(), latency); media::AudioRendererMixer* mixer = new media::AudioRendererMixer( mixer_output_params, std::move(sink), base::BindRepeating(LogMixerUmaHistogram, latency)); @@ -237,16 +236,11 @@ void AudioRendererMixerManager::ReturnMixer(media::AudioRendererMixer* mixer) { } } -media::OutputDeviceInfo AudioRendererMixerManager::GetOutputDeviceInfo( +scoped_refptr AudioRendererMixerManager::GetSink( int source_render_frame_id, - int session_id, const std::string& device_id) { - auto sink = - create_sink_cb_.Run(source_render_frame_id, - media::AudioSinkParameters(session_id, device_id)); - auto info = sink->GetOutputDeviceInfo(); - sink->Stop(); - return info; + return create_sink_cb_.Run(source_render_frame_id, + media::AudioSinkParameters(0, device_id)); } AudioRendererMixerManager::MixerKey::MixerKey( diff --git a/content/renderer/media/audio/audio_renderer_mixer_manager.h b/content/renderer/media/audio/audio_renderer_mixer_manager.h index 8b82abec774850..11901b0d21d48b 100644 --- a/content/renderer/media/audio/audio_renderer_mixer_manager.h +++ b/content/renderer/media/audio/audio_renderer_mixer_manager.h @@ -53,7 +53,7 @@ class CONTENT_EXPORT AudioRendererMixerManager // device to use. If |device_id| is empty and |session_id| is nonzero, // output device associated with the opened input device designated by // |session_id| is used. Otherwise, |session_id| is ignored. - media::AudioRendererMixerInput* CreateInput( + scoped_refptr CreateInput( int source_render_frame_id, int session_id, const std::string& device_id, @@ -65,14 +65,13 @@ class CONTENT_EXPORT AudioRendererMixerManager int source_render_frame_id, const media::AudioParameters& input_params, media::AudioLatency::LatencyType latency, - const std::string& device_id, - media::OutputDeviceStatus* device_status) final; + const media::OutputDeviceInfo& sink_info, + scoped_refptr sink) final; void ReturnMixer(media::AudioRendererMixer* mixer) final; - media::OutputDeviceInfo GetOutputDeviceInfo( + scoped_refptr GetSink( int source_render_frame_id, - int session_id, const std::string& device_id) final; protected: diff --git a/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc b/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc index 8b118c28e28da5..ba5a48b99911dc 100644 --- a/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc +++ b/content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc @@ -7,9 +7,11 @@ #include #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/logging.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/test/scoped_task_environment.h" #include "build/build_config.h" #include "media/audio/audio_device_description.h" #include "media/base/audio_parameters.h" @@ -39,86 +41,118 @@ const int kRenderFrameId = 124; const int kAnotherRenderFrameId = 678; } // namespace -using media::AudioParameters; using media::AudioLatency; +using media::AudioParameters; class AudioRendererMixerManagerTest : public testing::Test { public: AudioRendererMixerManagerTest() : manager_(new AudioRendererMixerManager( - base::BindRepeating(&AudioRendererMixerManagerTest::GetSink, - base::Unretained(this)))), - mock_sink_(new media::MockAudioRendererSink( - kDefaultDeviceId, - media::OUTPUT_DEVICE_STATUS_OK, - AudioParameters(AudioParameters::AUDIO_PCM_LINEAR, - kChannelLayout, - kHardwareSampleRate, - kHardwareBufferSize))), - mock_sink_no_device_(new media::MockAudioRendererSink( - kNonexistentDeviceId, - media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND)), - mock_sink_matched_device_( - new media::MockAudioRendererSink(kMatchedDeviceId, - media::OUTPUT_DEVICE_STATUS_OK)) {} - - media::AudioRendererMixer* GetMixer( - int source_render_frame_id, - const media::AudioParameters& params, - AudioLatency::LatencyType latency, - const std::string& device_id, - media::OutputDeviceStatus* device_status) { + base::BindRepeating(&AudioRendererMixerManagerTest::GetPlainSink, + base::Unretained(this)))) {} + + scoped_refptr CreateNormalSink( + const std::string& device_id = std::string(kDefaultDeviceId)) { + auto sink = base::MakeRefCounted( + device_id, media::OUTPUT_DEVICE_STATUS_OK, + AudioParameters(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, + kHardwareSampleRate, kHardwareBufferSize)); + EXPECT_CALL(*sink, Stop()).Times(1); + return sink; + } + + scoped_refptr CreateNoDeviceSink() { + return base::MakeRefCounted( + kNonexistentDeviceId, media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND); + } + + scoped_refptr CreateMatchedDeviceSink() { + auto sink = base::MakeRefCounted( + kMatchedDeviceId, media::OUTPUT_DEVICE_STATUS_OK); + EXPECT_CALL(*sink, Stop()).Times(1); + return sink; + } + + enum class SinkUseState { kExistingSink, kNewSink }; + media::AudioRendererMixer* GetMixer(int source_render_frame_id, + const media::AudioParameters& params, + AudioLatency::LatencyType latency, + const std::string& device_id, + SinkUseState sink_state) { + auto sink = GetSink(source_render_frame_id, + media::AudioSinkParameters(0, device_id)); + auto device_info = sink->GetOutputDeviceInfo(); + if (sink_state == SinkUseState::kNewSink) + EXPECT_CALL(*sink, Start()).Times(1); return manager_->GetMixer(source_render_frame_id, params, latency, - device_id, device_status); + device_info, std::move(sink)); } void ReturnMixer(media::AudioRendererMixer* mixer) { return manager_->ReturnMixer(mixer); } + scoped_refptr CreateInputHelper( + int source_render_frame_id, + int session_id, + const std::string& device_id, + media::AudioLatency::LatencyType latency, + const media::AudioParameters params, + media::AudioRendererSink::RenderCallback* callback) { + auto input = manager_->CreateInput(source_render_frame_id, session_id, + device_id, latency); + input->GetOutputDeviceInfoAsync( + base::DoNothing()); // Primes input, needed for tests. + task_env_.RunUntilIdle(); + input->Initialize(params, callback); + return input; + } + // Number of instantiated mixers. int mixer_count() { return manager_->mixers_.size(); } protected: - scoped_refptr GetSink( + scoped_refptr GetSink( int source_render_frame_id, const media::AudioSinkParameters& params) { if ((params.device_id == kDefaultDeviceId) || (params.device_id == kAnotherDeviceId)) { - // We don't care about separate sinks for these devices. - return mock_sink_; + return mock_sink_ ? std::move(mock_sink_) + : CreateNormalSink(params.device_id); } if (params.device_id == kNonexistentDeviceId) - return mock_sink_no_device_; + return CreateNoDeviceSink(); if (params.device_id.empty()) { // The sink used to get device ID from session ID if it's not empty - return params.session_id ? mock_sink_matched_device_ : mock_sink_; + return params.session_id + ? CreateMatchedDeviceSink() + : (mock_sink_ ? std::move(mock_sink_) + : CreateNormalSink(params.device_id)); } if (params.device_id == kMatchedDeviceId) - return mock_sink_matched_device_; + return CreateMatchedDeviceSink(); NOTREACHED(); return nullptr; } + base::test::ScopedTaskEnvironment task_env_; std::unique_ptr manager_; - scoped_refptr mock_sink_; - scoped_refptr mock_sink_no_device_; - scoped_refptr mock_sink_matched_device_; private: + scoped_refptr GetPlainSink( + int source_render_frame_id, + const media::AudioSinkParameters& params) { + return GetSink(source_render_frame_id, params); + } + DISALLOW_COPY_AND_ASSIGN(AudioRendererMixerManagerTest); }; // Verify GetMixer() and ReturnMixer() both work as expected; particularly with // respect to the explicit ref counting done. TEST_F(AudioRendererMixerManagerTest, GetReturnMixer) { - // Since we're testing two different sets of parameters, we expect - // AudioRendererMixerManager to call Start and Stop on our mock twice. - EXPECT_CALL(*mock_sink_.get(), Start()).Times(2); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); - // There should be no mixers outstanding to start with. EXPECT_EQ(0, mixer_count()); @@ -127,14 +161,14 @@ TEST_F(AudioRendererMixerManagerTest, GetReturnMixer) { media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params1, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); // The same parameters should return the same mixer1. EXPECT_EQ(mixer1, GetMixer(kRenderFrameId, params1, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr)); + kDefaultDeviceId, SinkUseState::kExistingSink)); EXPECT_EQ(1, mixer_count()); // Return the extra mixer we just acquired. @@ -146,7 +180,7 @@ TEST_F(AudioRendererMixerManagerTest, GetReturnMixer) { kBufferSize * 2); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params2, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer2); EXPECT_EQ(2, mixer_count()); @@ -163,8 +197,6 @@ TEST_F(AudioRendererMixerManagerTest, GetReturnMixer) { // Verify GetMixer() correctly deduplicates mixer with irrelevant AudioParameter // differences. TEST_F(AudioRendererMixerManagerTest, MixerReuse) { - EXPECT_CALL(*mock_sink_.get(), Start()).Times(2); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); EXPECT_EQ(mixer_count(), 0); media::AudioParameters params1(AudioParameters::AUDIO_PCM_LINEAR, @@ -173,7 +205,7 @@ TEST_F(AudioRendererMixerManagerTest, MixerReuse) { kBufferSize); media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params1, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); @@ -185,7 +217,7 @@ TEST_F(AudioRendererMixerManagerTest, MixerReuse) { kBufferSize * 2); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params2, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); EXPECT_EQ(mixer1, mixer2); EXPECT_EQ(1, mixer_count()); ReturnMixer(mixer2); @@ -199,7 +231,7 @@ TEST_F(AudioRendererMixerManagerTest, MixerReuse) { ASSERT_NE(params3.channel_layout(), params1.channel_layout()); media::AudioRendererMixer* mixer3 = GetMixer(kRenderFrameId, params3, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); EXPECT_NE(mixer1, mixer3); EXPECT_EQ(2, mixer_count()); ReturnMixer(mixer3); @@ -215,26 +247,26 @@ TEST_F(AudioRendererMixerManagerTest, MixerReuse) { // mixers are created for separate RenderFrames, even though the // AudioParameters are the same. TEST_F(AudioRendererMixerManagerTest, CreateInput) { - // Expect AudioRendererMixerManager to call Start and Stop on our mock twice - // each. Note: Under normal conditions, each mixer would get its own sink! - EXPECT_CALL(*mock_sink_.get(), Start()).Times(2); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); - media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); // Create two mixer inputs and ensure this doesn't instantiate any mixers yet. EXPECT_EQ(0, mixer_count()); media::FakeAudioRenderCallback callback(0, kSampleRate); - scoped_refptr input(manager_->CreateInput( - kRenderFrameId, 0, kDefaultDeviceId, AudioLatency::LATENCY_PLAYBACK)); - input->Initialize(params, &callback); + mock_sink_ = CreateNormalSink(); + EXPECT_CALL(*mock_sink_, Start()).Times(1); + auto input = + CreateInputHelper(kRenderFrameId, 0, kDefaultDeviceId, + AudioLatency::LATENCY_PLAYBACK, params, &callback); EXPECT_EQ(0, mixer_count()); media::FakeAudioRenderCallback another_callback(1, kSampleRate); - scoped_refptr another_input( - manager_->CreateInput(kAnotherRenderFrameId, 0, kDefaultDeviceId, - AudioLatency::LATENCY_PLAYBACK)); - another_input->Initialize(params, &another_callback); + + EXPECT_FALSE(!!mock_sink_); + mock_sink_ = CreateNormalSink(); + EXPECT_CALL(*mock_sink_, Start()).Times(1); + auto another_input = CreateInputHelper( + kAnotherRenderFrameId, 0, kDefaultDeviceId, + AudioLatency::LATENCY_PLAYBACK, params, &another_callback); EXPECT_EQ(0, mixer_count()); // Implicitly test that AudioRendererMixerInput was provided with the expected @@ -260,49 +292,33 @@ TEST_F(AudioRendererMixerManagerTest, CreateInput) { // synchronous GetOutputDeviceInfo call, this is not allowed. So this test is // disabled. This should be deleted in the future, https://crbug.com/870836. TEST_F(AudioRendererMixerManagerTest, DISABLED_CreateInputWithSessionId) { - // Expect AudioRendererMixerManager to call Start and Stop on our mock twice - // each: for kDefaultDeviceId and for kAnotherDeviceId. Note: Under normal - // conditions, each mixer would get its own sink! - EXPECT_CALL(*mock_sink_.get(), Start()).Times(2); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); - - // Expect AudioRendererMixerManager to call Start and Stop on the matched sink - // once. - EXPECT_CALL(*mock_sink_matched_device_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_matched_device_.get(), Stop()).Times(1); - media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); media::FakeAudioRenderCallback callback(0, kSampleRate); EXPECT_EQ(0, mixer_count()); // Empty device id, zero session id; - scoped_refptr input_to_default_device( - manager_->CreateInput(kRenderFrameId, 0, // session_id - std::string(), AudioLatency::LATENCY_PLAYBACK)); - input_to_default_device->Initialize(params, &callback); + auto input_to_default_device = CreateInputHelper( + kRenderFrameId, 0, // session_id + std::string(), AudioLatency::LATENCY_PLAYBACK, params, &callback); EXPECT_EQ(0, mixer_count()); // Specific device id, zero session id; - scoped_refptr input_to_matched_device( - manager_->CreateInput(kRenderFrameId, 0, // session_id - kMatchedDeviceId, AudioLatency::LATENCY_PLAYBACK)); - input_to_matched_device->Initialize(params, &callback); + auto input_to_another_device = CreateInputHelper( + kRenderFrameId, 0, // session_id + kMatchedDeviceId, AudioLatency::LATENCY_PLAYBACK, params, &callback); EXPECT_EQ(0, mixer_count()); // Specific device id, non-zero session id (to be ignored); - scoped_refptr input_to_another_device( - manager_->CreateInput(kRenderFrameId, 1, // session id - kAnotherDeviceId, AudioLatency::LATENCY_PLAYBACK)); - input_to_another_device->Initialize(params, &callback); + auto input_to_matched_device = CreateInputHelper( + kRenderFrameId, 1, // session id + kAnotherDeviceId, AudioLatency::LATENCY_PLAYBACK, params, &callback); EXPECT_EQ(0, mixer_count()); // Empty device id, non-zero session id; - scoped_refptr - input_to_matched_device_with_session_id( - manager_->CreateInput(kRenderFrameId, 2, // session id - std::string(), AudioLatency::LATENCY_PLAYBACK)); - input_to_matched_device_with_session_id->Initialize(params, &callback); + auto input_to_matched_device_with_session_id = CreateInputHelper( + kRenderFrameId, 2, // session id + std::string(), AudioLatency::LATENCY_PLAYBACK, params, &callback); EXPECT_EQ(0, mixer_count()); // Implicitly test that AudioRendererMixerInput was provided with the expected @@ -338,21 +354,19 @@ TEST_F(AudioRendererMixerManagerTest, DISABLED_CreateInputWithSessionId) { // Verify GetMixer() correctly creates different mixers with the same // parameters, but different device ID. TEST_F(AudioRendererMixerManagerTest, MixerDevices) { - EXPECT_CALL(*mock_sink_.get(), Start()).Times(2); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); EXPECT_EQ(0, mixer_count()); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kAnotherDeviceId, nullptr); + kAnotherDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer2); EXPECT_EQ(2, mixer_count()); EXPECT_NE(mixer1, mixer2); @@ -366,21 +380,19 @@ TEST_F(AudioRendererMixerManagerTest, MixerDevices) { // Verify GetMixer() correctly deduplicate mixers with the same // parameters and default device ID, even if one is "" and one is "default". TEST_F(AudioRendererMixerManagerTest, OneMixerDifferentDefaultDeviceIDs) { - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); EXPECT_EQ(0, mixer_count()); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - std::string(), nullptr); + std::string(), SinkUseState::kExistingSink); ASSERT_TRUE(mixer2); EXPECT_EQ(1, mixer_count()); EXPECT_EQ(mixer1, mixer2); @@ -398,62 +410,58 @@ TEST_F(AudioRendererMixerManagerTest, NonexistentDevice) { media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); - media::OutputDeviceStatus device_status = media::OUTPUT_DEVICE_STATUS_OK; - media::AudioRendererMixer* mixer = - GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kNonexistentDeviceId, &device_status); + auto sink = GetSink(kRenderFrameId, + media::AudioSinkParameters(0, kNonexistentDeviceId)); + auto device_info = sink->GetOutputDeviceInfo(); - EXPECT_FALSE(mixer); - EXPECT_EQ(media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND, device_status); + EXPECT_EQ(media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND, + device_info.device_status()); EXPECT_EQ(0, mixer_count()); } // Verify GetMixer() correctly deduplicate mixers basing on latency // requirements. TEST_F(AudioRendererMixerManagerTest, LatencyMixing) { - EXPECT_CALL(*mock_sink_.get(), Start()).Times(3); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(3); - EXPECT_EQ(0, mixer_count()); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, kSampleRate, kBufferSize); media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); ASSERT_TRUE(mixer2); EXPECT_EQ(mixer1, mixer2); // Same latency => same mixer. EXPECT_EQ(1, mixer_count()); media::AudioRendererMixer* mixer3 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_RTC, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer3); EXPECT_NE(mixer1, mixer3); EXPECT_EQ(2, mixer_count()); // Another latency => another mixer. media::AudioRendererMixer* mixer4 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_RTC, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); EXPECT_EQ(mixer3, mixer4); EXPECT_EQ(2, mixer_count()); // Same latency => same mixer. media::AudioRendererMixer* mixer5 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_INTERACTIVE, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer5); EXPECT_EQ(3, mixer_count()); // Another latency => another mixer. media::AudioRendererMixer* mixer6 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_INTERACTIVE, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); EXPECT_EQ(mixer5, mixer6); EXPECT_EQ(3, mixer_count()); // Same latency => same mixer. @@ -474,9 +482,6 @@ TEST_F(AudioRendererMixerManagerTest, LatencyMixing) { // Verify GetMixer() correctly deduplicate mixers basing on the effects // requirements. TEST_F(AudioRendererMixerManagerTest, EffectsMixing) { - EXPECT_CALL(*mock_sink_.get(), Start()).Times(3); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(3); - EXPECT_EQ(0, mixer_count()); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, @@ -484,13 +489,13 @@ TEST_F(AudioRendererMixerManagerTest, EffectsMixing) { params.set_effects(1); media::AudioRendererMixer* mixer1 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer1); EXPECT_EQ(1, mixer_count()); media::AudioRendererMixer* mixer2 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); ASSERT_TRUE(mixer2); EXPECT_EQ(mixer1, mixer2); // Same effects => same mixer. EXPECT_EQ(1, mixer_count()); @@ -498,27 +503,27 @@ TEST_F(AudioRendererMixerManagerTest, EffectsMixing) { params.set_effects(2); media::AudioRendererMixer* mixer3 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer3); EXPECT_NE(mixer1, mixer3); EXPECT_EQ(2, mixer_count()); // Another effects => another mixer. media::AudioRendererMixer* mixer4 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); EXPECT_EQ(mixer3, mixer4); EXPECT_EQ(2, mixer_count()); // Same effects => same mixer. params.set_effects(3); media::AudioRendererMixer* mixer5 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); ASSERT_TRUE(mixer5); EXPECT_EQ(3, mixer_count()); // Another effects => another mixer. media::AudioRendererMixer* mixer6 = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kExistingSink); EXPECT_EQ(mixer5, mixer6); EXPECT_EQ(3, mixer_count()); // Same effects => same mixer. @@ -539,6 +544,8 @@ TEST_F(AudioRendererMixerManagerTest, EffectsMixing) { // Verify output bufer size of the mixer is correctly adjusted for Playback // latency. TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlayback) { + mock_sink_ = CreateNormalSink(); + // Expecting hardware buffer size of 128 frames EXPECT_EQ(44100, mock_sink_->GetOutputDeviceInfo().output_params().sample_rate()); @@ -547,15 +554,13 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlayback) { 128, mock_sink_->GetOutputDeviceInfo().output_params().frames_per_buffer()); - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); - media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); params.set_latency_tag(AudioLatency::LATENCY_PLAYBACK); - media::AudioRendererMixer* mixer = GetMixer( - kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, nullptr); + media::AudioRendererMixer* mixer = + GetMixer(kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, + SinkUseState::kNewSink); if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) { // Expecting input sample rate @@ -588,16 +593,15 @@ TEST_F(AudioRendererMixerManagerTest, std::string(), media::OUTPUT_DEVICE_STATUS_OK, AudioParameters(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 44100, 2048)); - - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); + EXPECT_CALL(*mock_sink_, Stop()).Times(1); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); params.set_latency_tag(AudioLatency::LATENCY_PLAYBACK); - media::AudioRendererMixer* mixer = GetMixer( - kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, nullptr); + media::AudioRendererMixer* mixer = + GetMixer(kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, + SinkUseState::kNewSink); // 20 ms at 44100 is 882 frames per buffer. if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) { @@ -621,16 +625,14 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlaybackFakeAudio) { std::string(), media::OUTPUT_DEVICE_STATUS_OK, AudioParameters(AudioParameters::AUDIO_FAKE, kChannelLayout, 44100, 2048)); - - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); + EXPECT_CALL(*mock_sink_, Stop()).Times(1); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); media::AudioRendererMixer* mixer = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_PLAYBACK, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); // Expecting input sample rate EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate()); @@ -649,6 +651,8 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlaybackFakeAudio) { // Verify output bufer size of the mixer is correctly adjusted for RTC latency. TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtc) { + mock_sink_ = CreateNormalSink(); + // Expecting hardware buffer size of 128 frames EXPECT_EQ(44100, mock_sink_->GetOutputDeviceInfo().output_params().sample_rate()); @@ -657,15 +661,13 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtc) { 128, mock_sink_->GetOutputDeviceInfo().output_params().frames_per_buffer()); - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); - media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); params.set_latency_tag(AudioLatency::LATENCY_RTC); - media::AudioRendererMixer* mixer = GetMixer( - kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, nullptr); + media::AudioRendererMixer* mixer = + GetMixer(kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, + SinkUseState::kNewSink); int output_sample_rate = AudioLatency::IsResamplingPassthroughSupported(params.latency_tag()) @@ -697,16 +699,14 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtcFakeAudio) { mock_sink_ = new media::MockAudioRendererSink( std::string(), media::OUTPUT_DEVICE_STATUS_OK, AudioParameters(AudioParameters::AUDIO_FAKE, kChannelLayout, 44100, 128)); - - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); + EXPECT_CALL(*mock_sink_, Stop()).Times(1); media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); media::AudioRendererMixer* mixer = GetMixer(kRenderFrameId, params, AudioLatency::LATENCY_RTC, - kDefaultDeviceId, nullptr); + kDefaultDeviceId, SinkUseState::kNewSink); // Expecting input sample rate. EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate()); @@ -721,6 +721,8 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtcFakeAudio) { // Verify output bufer size of the mixer is correctly adjusted for Interactive // latency. TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyInteractive) { + mock_sink_ = CreateNormalSink(); + // Expecting hardware buffer size of 128 frames EXPECT_EQ(44100, mock_sink_->GetOutputDeviceInfo().output_params().sample_rate()); @@ -729,15 +731,13 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyInteractive) { 128, mock_sink_->GetOutputDeviceInfo().output_params().frames_per_buffer()); - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); - media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 32000, 512); params.set_latency_tag(AudioLatency::LATENCY_INTERACTIVE); - media::AudioRendererMixer* mixer = GetMixer( - kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, nullptr); + media::AudioRendererMixer* mixer = + GetMixer(kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, + SinkUseState::kNewSink); if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) { // Expecting input sample rate. @@ -760,16 +760,15 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsBitstreamFormat) { std::string(), media::OUTPUT_DEVICE_STATUS_OK, AudioParameters(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout, 44100, 2048)); - - EXPECT_CALL(*mock_sink_.get(), Start()).Times(1); - EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1); + EXPECT_CALL(*mock_sink_, Stop()).Times(1); media::AudioParameters params(AudioParameters::AUDIO_BITSTREAM_EAC3, kAnotherChannelLayout, 32000, 512); params.set_latency_tag(AudioLatency::LATENCY_PLAYBACK); - media::AudioRendererMixer* mixer = GetMixer( - kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, nullptr); + media::AudioRendererMixer* mixer = + GetMixer(kRenderFrameId, params, params.latency_tag(), kDefaultDeviceId, + SinkUseState::kNewSink); // Output parameters should be the same as input properties for bitstream // formats. diff --git a/media/audio/BUILD.gn b/media/audio/BUILD.gn index 38e7bb1d7f4ac8..863e8d358ad45a 100644 --- a/media/audio/BUILD.gn +++ b/media/audio/BUILD.gn @@ -56,6 +56,8 @@ source_set("audio") { visibility = [ "//media", + "//media/renderers", + # TODO(dalecurtis): CoreAudioUtil::IsCoreAudioSupported() should probably # move into media/base/win. "//media/device_monitors", diff --git a/media/base/audio_renderer_mixer.cc b/media/base/audio_renderer_mixer.cc index bb365e32606463..61f9d5b52ce566 100644 --- a/media/base/audio_renderer_mixer.cc +++ b/media/base/audio_renderer_mixer.cc @@ -155,9 +155,10 @@ void AudioRendererMixer::SetPauseDelayForTesting(base::TimeDelta delay) { pause_delay_ = delay; } -OutputDeviceInfo AudioRendererMixer::GetOutputDeviceInfo() { +void AudioRendererMixer::GetOutputDeviceInfoAsync( + AudioRendererSink::OutputDeviceInfoCB info_cb) { DVLOG(1) << __func__; - return audio_sink_->GetOutputDeviceInfo(); + return audio_sink_->GetOutputDeviceInfoAsync(std::move(info_cb)); } bool AudioRendererMixer::CurrentThreadIsRenderingThread() { diff --git a/media/base/audio_renderer_mixer.h b/media/base/audio_renderer_mixer.h index ef41c7385f6630..5cdeb5e84e998c 100644 --- a/media/base/audio_renderer_mixer.h +++ b/media/base/audio_renderer_mixer.h @@ -48,7 +48,7 @@ class MEDIA_EXPORT AudioRendererMixer void SetPauseDelayForTesting(base::TimeDelta delay); - OutputDeviceInfo GetOutputDeviceInfo(); + void GetOutputDeviceInfoAsync(AudioRendererSink::OutputDeviceInfoCB info_cb); // Returns true if called on rendering thread, otherwise false. bool CurrentThreadIsRenderingThread(); diff --git a/media/base/audio_renderer_mixer_input.cc b/media/base/audio_renderer_mixer_input.cc index 9754bb688a4020..723466bad44178 100644 --- a/media/base/audio_renderer_mixer_input.cc +++ b/media/base/audio_renderer_mixer_input.cc @@ -24,14 +24,22 @@ AudioRendererMixerInput::AudioRendererMixerInput( owner_id_(owner_id), device_id_(device_id), latency_(latency), - error_cb_(base::Bind(&AudioRendererMixerInput::OnRenderError, - base::Unretained(this))) { + error_cb_(base::BindRepeating(&AudioRendererMixerInput::OnRenderError, + // Unretained is safe here because Stop() + // must always be called before destruction. + base::Unretained(this))) { DCHECK(mixer_pool_); } AudioRendererMixerInput::~AudioRendererMixerInput() { + // Note: This may not happen on the thread the sink was used. E.g., this may + // end up destroyed on the render thread despite being used on the media + // thread. + DCHECK(!started_); DCHECK(!mixer_); + if (sink_) + sink_->Stop(); } void AudioRendererMixerInput::Initialize( @@ -41,6 +49,12 @@ void AudioRendererMixerInput::Initialize( DCHECK(!mixer_); DCHECK(callback); + // Current usage ensures we always call GetOutputDeviceInfoAsync() and wait + // for the result before calling this method. We could add support for doing + // otherwise here, but it's not needed for now, so for simplicity just DCHECK. + DCHECK(sink_); + DCHECK(device_info_); + params_ = params; callback_ = callback; } @@ -50,13 +64,13 @@ void AudioRendererMixerInput::Start() { DCHECK(!mixer_); DCHECK(callback_); // Initialized. + DCHECK(sink_); + DCHECK(device_info_); + DCHECK_EQ(device_info_->device_status(), OUTPUT_DEVICE_STATUS_OK); + started_ = true; - mixer_ = - mixer_pool_->GetMixer(owner_id_, params_, latency_, device_id_, nullptr); - if (!mixer_) { - callback_->OnRenderError(); - return; - } + mixer_ = mixer_pool_->GetMixer(owner_id_, params_, latency_, *device_info_, + std::move(sink_)); // Note: OnRenderError() may be called immediately after this call returns. mixer_->AddErrorCallback(error_cb_); @@ -102,15 +116,30 @@ bool AudioRendererMixerInput::SetVolume(double volume) { } OutputDeviceInfo AudioRendererMixerInput::GetOutputDeviceInfo() { - return mixer_ ? mixer_->GetOutputDeviceInfo() - : mixer_pool_->GetOutputDeviceInfo( - owner_id_, 0 /* session_id */, device_id_); + NOTREACHED(); // The blocking API is intentionally not supported. + return OutputDeviceInfo(); } void AudioRendererMixerInput::GetOutputDeviceInfoAsync( OutputDeviceInfoCB info_cb) { - // TODO(dalecurtis): Implement this for https://crbug.com/905506. - NOTREACHED(); + // If we device information and for a current sink or mixer, just return it + // immediately. + if (device_info_.has_value() && (sink_ || mixer_)) { + std::move(info_cb).Run(*device_info_); + return; + } + + // We may have |device_info_|, but a Stop() has been called since if we don't + // have a |sink_| or a |mixer_|, so request the information again in case it + // has changed (which may occur due to browser side device changes). + device_info_.reset(); + + // If we don't have a sink yet start the process of getting one. Retain a ref + // to this sink to ensure it is not destructed while this occurs. + sink_ = mixer_pool_->GetSink(owner_id_, device_id_); + sink_->GetOutputDeviceInfoAsync( + base::BindOnce(&AudioRendererMixerInput::OnDeviceInfoReceived, + base::RetainedRef(this), std::move(info_cb))); } bool AudioRendererMixerInput::IsOptimizedForHardwareParameters() { @@ -129,38 +158,14 @@ void AudioRendererMixerInput::SwitchOutputDevice( return; } - if (mixer_) { - OutputDeviceStatus new_mixer_status = OUTPUT_DEVICE_STATUS_ERROR_INTERNAL; - AudioRendererMixer* new_mixer = mixer_pool_->GetMixer( - owner_id_, params_, latency_, device_id, &new_mixer_status); - if (new_mixer_status != OUTPUT_DEVICE_STATUS_OK) { - std::move(callback).Run(new_mixer_status); - return; - } - - bool was_playing = playing_; - Stop(); - device_id_ = device_id; - mixer_ = new_mixer; - mixer_->AddErrorCallback(error_cb_); - started_ = true; - - if (was_playing) - Play(); - - } else { - OutputDeviceStatus new_mixer_status = - mixer_pool_ - ->GetOutputDeviceInfo(owner_id_, 0 /* session_id */, device_id) - .device_status(); - if (new_mixer_status != OUTPUT_DEVICE_STATUS_OK) { - std::move(callback).Run(new_mixer_status); - return; - } - device_id_ = device_id; - } - - std::move(callback).Run(OUTPUT_DEVICE_STATUS_OK); + // Request a new sink using the new device id. This process may fail, so to + // avoid interrupting working audio, don't set any class variables until we + // know it's a success. Retain a ref to this sink to ensure it is not + // destructed while this occurs. + auto new_sink = mixer_pool_->GetSink(owner_id_, device_id); + new_sink->GetOutputDeviceInfoAsync( + base::BindOnce(&AudioRendererMixerInput::OnDeviceSwitchReady, + base::RetainedRef(this), std::move(callback), new_sink)); } double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus, @@ -191,4 +196,42 @@ void AudioRendererMixerInput::OnRenderError() { callback_->OnRenderError(); } +void AudioRendererMixerInput::OnDeviceInfoReceived( + OutputDeviceInfoCB info_cb, + OutputDeviceInfo device_info) { + device_info_ = device_info; + std::move(info_cb).Run(*device_info_); +} + +void AudioRendererMixerInput::OnDeviceSwitchReady( + OutputDeviceStatusCB switch_cb, + scoped_refptr sink, + OutputDeviceInfo device_info) { + if (device_info.device_status() != OUTPUT_DEVICE_STATUS_OK) { + sink->Stop(); + std::move(switch_cb).Run(device_info.device_status()); + return; + } + + const bool has_mixer = !!mixer_; + const bool is_playing = playing_; + + // This may occur if Start() hasn't yet been called. + if (sink_) + sink_->Stop(); + + sink_ = std::move(sink); + device_info_ = device_info; + device_id_ = device_info.device_id(); + + Stop(); + if (has_mixer) { + Start(); + if (is_playing) + Play(); + } + + std::move(switch_cb).Run(device_info.device_status()); +} + } // namespace media diff --git a/media/base/audio_renderer_mixer_input.h b/media/base/audio_renderer_mixer_input.h index aeb578147c1778..1a9bab7d136a98 100644 --- a/media/base/audio_renderer_mixer_input.h +++ b/media/base/audio_renderer_mixer_input.h @@ -80,9 +80,27 @@ class MEDIA_EXPORT AudioRendererMixerInput bool playing_ = false; double volume_ GUARDED_BY(volume_lock_) = 1.0; + scoped_refptr sink_; + base::Optional device_info_; + // AudioConverter::InputCallback implementation. double ProvideInput(AudioBus* audio_bus, uint32_t frames_delayed) override; + void OnDeviceInfoReceived(OutputDeviceInfoCB info_cb, + OutputDeviceInfo device_info); + + // Method to help handle device changes. Must be static to ensure we can still + // execute the |switch_cb| even if the pipeline is destructed. Restarts (if + // necessary) Start() and Play() state with a new |sink| and |device_info|. + // + // |switch_cb| is the callback given to the SwitchOutputDevice() call. + // |sink| is a fresh sink which should be used if device info is good. + // |device_info| is the OutputDeviceInfo for |sink| after + // GetOutputDeviceInfoAsync() completes. + void OnDeviceSwitchReady(OutputDeviceStatusCB switch_cb, + scoped_refptr sink, + OutputDeviceInfo device_info); + // AudioParameters received during Initialize(). AudioParameters params_; diff --git a/media/base/audio_renderer_mixer_input_unittest.cc b/media/base/audio_renderer_mixer_input_unittest.cc index 7b6013c84cf43d..f42decfd5626c8 100644 --- a/media/base/audio_renderer_mixer_input_unittest.cc +++ b/media/base/audio_renderer_mixer_input_unittest.cc @@ -35,7 +35,7 @@ static const char kUnauthorizedDeviceId[] = "unauthorized"; static const char kNonexistentDeviceId[] = "nonexistent"; class AudioRendererMixerInputTest : public testing::Test, - AudioRendererMixerPool { + public AudioRendererMixerPool { public: AudioRendererMixerInputTest() { audio_parameters_ = @@ -49,42 +49,26 @@ class AudioRendererMixerInputTest : public testing::Test, void CreateMixerInput(const std::string& device_id) { mixer_input_ = new AudioRendererMixerInput(this, kRenderFrameId, device_id, - AudioLatency::LATENCY_PLAYBACK); + mixer_input_->GetOutputDeviceInfoAsync(base::DoNothing()); + scoped_task_environment_.RunUntilIdle(); } AudioRendererMixer* GetMixer(int owner_id, const AudioParameters& params, AudioLatency::LatencyType latency, - const std::string& device_id, - OutputDeviceStatus* device_status) override { + const OutputDeviceInfo& sink_info, + scoped_refptr sink) override { EXPECT_TRUE(params.IsValid()); - if (device_id == kNonexistentDeviceId) { - if (device_status) - *device_status = OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND; - return nullptr; - } - - if (device_id == kUnauthorizedDeviceId) { - if (device_status) - *device_status = OUTPUT_DEVICE_STATUS_ERROR_NOT_AUTHORIZED; - return nullptr; - } - - size_t idx = (device_id == kDefaultDeviceId) ? 0 : 1; + size_t idx = (sink_info.device_id() == kDefaultDeviceId) ? 0 : 1; if (!mixers_[idx]) { - sinks_[idx] = - new MockAudioRendererSink(device_id, OUTPUT_DEVICE_STATUS_OK); - EXPECT_CALL(*(sinks_[idx].get()), Start()); - EXPECT_CALL(*(sinks_[idx].get()), Stop()); + EXPECT_CALL(*reinterpret_cast(sink.get()), + Start()); mixers_[idx].reset(new AudioRendererMixer( - audio_parameters_, sinks_[idx].get(), base::Bind(&LogUma))); + audio_parameters_, std::move(sink), base::BindRepeating(&LogUma))); } EXPECT_CALL(*this, ReturnMixer(mixers_[idx].get())); - - if (device_status) - *device_status = OUTPUT_DEVICE_STATUS_OK; return mixers_[idx].get(); } @@ -92,22 +76,19 @@ class AudioRendererMixerInputTest : public testing::Test, return mixer_input_->ProvideInput(audio_bus_.get(), 0); } - OutputDeviceInfo GetOutputDeviceInfo(int source_render_frame_id, - int session_id, - const std::string& device_id) override { + scoped_refptr GetSink( + int owner_id, + const std::string& device_id) override { OutputDeviceStatus status = OUTPUT_DEVICE_STATUS_OK; if (device_id == kNonexistentDeviceId) status = OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND; else if (device_id == kUnauthorizedDeviceId) status = OUTPUT_DEVICE_STATUS_ERROR_NOT_AUTHORIZED; - - GetOutputDeviceInfoCalled(device_id); - return OutputDeviceInfo(device_id, status, - AudioParameters::UnavailableDeviceParams()); + auto sink = base::MakeRefCounted(device_id, status); + EXPECT_CALL(*sink, Stop()); + return sink; } - MOCK_METHOD1(GetOutputDeviceInfoCalled, void(const std::string&)); - MOCK_METHOD1(ReturnMixer, void(AudioRendererMixer*)); MOCK_METHOD1(SwitchCallbackCalled, void(OutputDeviceStatus)); @@ -118,13 +99,15 @@ class AudioRendererMixerInputTest : public testing::Test, } AudioRendererMixer* GetInputMixer() { return mixer_input_->mixer_; } + MockAudioRendererSink* GetMockSink() const { + return reinterpret_cast(mixer_input_->sink_.get()); + } protected: ~AudioRendererMixerInputTest() override = default; base::test::ScopedTaskEnvironment scoped_task_environment_; AudioParameters audio_parameters_; - scoped_refptr sinks_[2]; std::unique_ptr mixers_[2]; scoped_refptr mixer_input_; std::unique_ptr fake_callback_; @@ -134,25 +117,6 @@ class AudioRendererMixerInputTest : public testing::Test, DISALLOW_COPY_AND_ASSIGN(AudioRendererMixerInputTest); }; -TEST_F(AudioRendererMixerInputTest, GetDeviceInfo) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kDefaultDeviceId)) - .Times(testing::Exactly(1)); - - mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); - - // Calling GetOutputDeviceInfo() should result in the mock call, since there - // is no mixer created yet for mixer input. - mixer_input_->GetOutputDeviceInfo(); - mixer_input_->Start(); - - // This call should be directed to the mixer and should not result in the mock - // call. - EXPECT_STREQ(kDefaultDeviceId, - mixer_input_->GetOutputDeviceInfo().device_id().c_str()); - - mixer_input_->Stop(); -} - // Test that getting and setting the volume work as expected. The volume is // returned from ProvideInput() only when playing. TEST_F(AudioRendererMixerInputTest, GetSetVolume) { @@ -197,6 +161,9 @@ TEST_F(AudioRendererMixerInputTest, StopBeforeInitializeOrStart) { TEST_F(AudioRendererMixerInputTest, StartAfterStop) { mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Stop(); + + mixer_input_->GetOutputDeviceInfoAsync(base::DoNothing()); + scoped_task_environment_.RunUntilIdle(); mixer_input_->Start(); mixer_input_->Stop(); } @@ -206,14 +173,15 @@ TEST_F(AudioRendererMixerInputTest, InitializeAfterStop) { mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); mixer_input_->Stop(); + + mixer_input_->GetOutputDeviceInfoAsync(base::DoNothing()); + scoped_task_environment_.RunUntilIdle(); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Stop(); } // Test SwitchOutputDevice(). TEST_F(AudioRendererMixerInputTest, SwitchOutputDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(testing::_)) - .Times(testing::Exactly(0)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); const std::string kDeviceId("mock-device-id"); @@ -233,8 +201,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDevice) { // Test SwitchOutputDevice() to the same device as the current (default) device TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToSameDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(testing::_)) - .Times(testing::Exactly(0)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); EXPECT_CALL(*this, SwitchCallbackCalled(OUTPUT_DEVICE_STATUS_OK)); @@ -251,8 +217,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToSameDevice) { // Test SwitchOutputDevice() to the new device TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToAnotherDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(testing::_)) - .Times(testing::Exactly(0)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); EXPECT_CALL(*this, SwitchCallbackCalled(OUTPUT_DEVICE_STATUS_OK)); @@ -269,8 +233,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToAnotherDevice) { // Test that SwitchOutputDevice() to a nonexistent device fails. TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToNonexistentDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(testing::_)) - .Times(testing::Exactly(0)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); EXPECT_CALL(*this, @@ -286,8 +248,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToNonexistentDevice) { // Test that SwitchOutputDevice() to an unauthorized device fails. TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToUnauthorizedDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(testing::_)) - .Times(testing::Exactly(0)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); EXPECT_CALL(*this, @@ -303,8 +263,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceToUnauthorizedDevice) { // Test that calling SwitchOutputDevice() before Start() succeeds. TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceBeforeStart) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kAnotherDeviceId)) - .Times(testing::Exactly(1)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); base::RunLoop run_loop; EXPECT_CALL(*this, SwitchCallbackCalled(OUTPUT_DEVICE_STATUS_OK)); @@ -319,47 +277,19 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceBeforeStart) { // Test that calling SwitchOutputDevice() succeeds even if Start() is never // called. TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceWithoutStart) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kAnotherDeviceId)) - .Times(testing::Exactly(1)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); base::RunLoop run_loop; EXPECT_CALL(*this, SwitchCallbackCalled(OUTPUT_DEVICE_STATUS_OK)); mixer_input_->SwitchOutputDevice( kAnotherDeviceId, base::Bind(&AudioRendererMixerInputTest::SwitchCallback, base::Unretained(this), &run_loop)); - mixer_input_->Stop(); run_loop.Run(); -} - -// Test creation with an invalid device. OnRenderError() should be called. -// Play(), Pause() and SwitchOutputDevice() should not cause crashes, even if -// they have no effect. -TEST_F(AudioRendererMixerInputTest, CreateWithInvalidDevice) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kDefaultDeviceId)) - .Times(testing::Exactly(1)); - // |mixer_input_| was initialized during construction. mixer_input_->Stop(); - - CreateMixerInput(kNonexistentDeviceId); - EXPECT_CALL(*fake_callback_, OnRenderError()); - mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); - mixer_input_->Start(); - mixer_input_->Play(); - mixer_input_->Pause(); - base::RunLoop run_loop; - EXPECT_CALL(*this, SwitchCallbackCalled(testing::_)); - mixer_input_->SwitchOutputDevice( - kDefaultDeviceId, base::Bind(&AudioRendererMixerInputTest::SwitchCallback, - base::Unretained(this), &run_loop)); - mixer_input_->Stop(); - run_loop.Run(); } // Test that calling SwitchOutputDevice() works after calling Stop(), and that // restarting works after the call to SwitchOutputDevice(). TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceAfterStopBeforeRestart) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kAnotherDeviceId)) - .Times(testing::Exactly(1)); mixer_input_->Initialize(audio_parameters_, fake_callback_.get()); mixer_input_->Start(); mixer_input_->Stop(); @@ -379,8 +309,6 @@ TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceAfterStopBeforeRestart) { // and that initialization and restart work after the call to // SwitchOutputDevice(). TEST_F(AudioRendererMixerInputTest, SwitchOutputDeviceBeforeInitialize) { - EXPECT_CALL(*this, GetOutputDeviceInfoCalled(kAnotherDeviceId)) - .Times(testing::Exactly(1)); base::RunLoop run_loop; EXPECT_CALL(*this, SwitchCallbackCalled(OUTPUT_DEVICE_STATUS_OK)); mixer_input_->SwitchOutputDevice( diff --git a/media/base/audio_renderer_mixer_pool.h b/media/base/audio_renderer_mixer_pool.h index 760f3eed032567..461960d7ee46cc 100644 --- a/media/base/audio_renderer_mixer_pool.h +++ b/media/base/audio_renderer_mixer_pool.h @@ -13,31 +13,38 @@ namespace media { class AudioParameters; class AudioRendererMixer; +class AudioRendererSink; // Provides AudioRendererMixer instances for shared usage. // Thread safe. class MEDIA_EXPORT AudioRendererMixerPool { public: - AudioRendererMixerPool() {} - virtual ~AudioRendererMixerPool() {} + AudioRendererMixerPool() = default; + virtual ~AudioRendererMixerPool() = default; // Obtains a pointer to mixer instance based on AudioParameters. The pointer // is guaranteed to be valid (at least) until it's rereleased by a call to // ReturnMixer(). - virtual AudioRendererMixer* GetMixer(int owner_id, - const AudioParameters& params, - AudioLatency::LatencyType latency, - const std::string& device_id, - OutputDeviceStatus* device_status) = 0; + // + // Ownership of |sink| must be passed to GetMixer(), it will be stopped and + // discard if an existing mixer can be reused. Clients must have called + // GetOutputDeviceInfoAsync() on |sink| to get |sink_info|, and it must have + // a device_status() == OUTPUT_DEVICE_STATUS_OK. + virtual AudioRendererMixer* GetMixer( + int owner_id, + const AudioParameters& input_params, + AudioLatency::LatencyType latency, + const OutputDeviceInfo& sink_info, + scoped_refptr sink) = 0; // Returns mixer back to the pool, must be called when the mixer is not needed // any more to avoid memory leakage. virtual void ReturnMixer(AudioRendererMixer* mixer) = 0; - // Returns output device information - virtual OutputDeviceInfo GetOutputDeviceInfo( + // Returns an AudioRendererSink for use with GetMixer(). Inputs must call this + // to get a sink to use with a subsequent GetMixer() + virtual scoped_refptr GetSink( int owner_id, - int session_id, const std::string& device_id) = 0; private: diff --git a/media/base/audio_renderer_mixer_unittest.cc b/media/base/audio_renderer_mixer_unittest.cc index ed519411270116..1c00ec274b7765 100644 --- a/media/base/audio_renderer_mixer_unittest.cc +++ b/media/base/audio_renderer_mixer_unittest.cc @@ -16,6 +16,7 @@ #include "base/bind_helpers.h" #include "base/macros.h" #include "base/synchronization/waitable_event.h" +#include "base/test/scoped_task_environment.h" #include "base/threading/platform_thread.h" #include "media/base/audio_renderer_mixer_input.h" #include "media/base/audio_renderer_mixer_pool.h" @@ -55,7 +56,7 @@ using AudioRendererMixerTestData = class AudioRendererMixerTest : public testing::TestWithParam, - AudioRendererMixerPool { + public AudioRendererMixerPool { public: AudioRendererMixerTest() : epsilon_(std::get<3>(GetParam())), half_fill_(false) { @@ -93,8 +94,8 @@ class AudioRendererMixerTest AudioRendererMixer* GetMixer(int owner_id, const AudioParameters& params, AudioLatency::LatencyType latency, - const std::string& device_id, - OutputDeviceStatus* device_status) final { + const OutputDeviceInfo& sink_info, + scoped_refptr sink) final { return mixer_.get(); }; @@ -102,8 +103,11 @@ class AudioRendererMixerTest EXPECT_EQ(mixer_.get(), mixer); } - MOCK_METHOD3(GetOutputDeviceInfo, - OutputDeviceInfo(int, int, const std::string&)); + scoped_refptr GetSink( + int owner_id, + const std::string& device_id) override { + return sink_; + } void InitializeInputs(int inputs_per_sample_rate) { mixer_inputs_.reserve(inputs_per_sample_rate * input_parameters_.size()); @@ -334,15 +338,20 @@ class AudioRendererMixerTest } scoped_refptr CreateMixerInput() { - return new AudioRendererMixerInput(this, - // Zero frame id, default device ID. - 0, std::string(), - AudioLatency::LATENCY_PLAYBACK); + auto input = base::MakeRefCounted( + this, + // Zero frame id, default device ID. + 0, std::string(), AudioLatency::LATENCY_PLAYBACK); + input->GetOutputDeviceInfoAsync( + base::DoNothing()); // Primes input, needed for tests. + task_env_.RunUntilIdle(); + return input; } protected: virtual ~AudioRendererMixerTest() = default; + base::test::ScopedTaskEnvironment task_env_; scoped_refptr sink_; std::unique_ptr mixer_; AudioRendererSink::RenderCallback* mixer_callback_; @@ -350,7 +359,7 @@ class AudioRendererMixerTest AudioParameters output_parameters_; std::unique_ptr audio_bus_; std::unique_ptr expected_audio_bus_; - std::vector< scoped_refptr > mixer_inputs_; + std::vector> mixer_inputs_; std::vector> fake_callbacks_; std::unique_ptr expected_callback_; double epsilon_; diff --git a/media/blink/webaudiosourceprovider_impl.cc b/media/blink/webaudiosourceprovider_impl.cc index e24103f0552195..50f23d03021ac3 100644 --- a/media/blink/webaudiosourceprovider_impl.cc +++ b/media/blink/webaudiosourceprovider_impl.cc @@ -18,7 +18,6 @@ #include "base/thread_annotations.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h" -#include "media/audio/null_audio_sink.h" #include "media/base/audio_timestamp_helper.h" #include "media/base/bind_to_current_loop.h" #include "media/base/media_log.h" @@ -197,22 +196,6 @@ void WebAudioSourceProviderImpl::Initialize(const AudioParameters& params, base::AutoLock auto_lock(sink_lock_); DCHECK_EQ(state_, kStopped); - OutputDeviceStatus device_status = - sink_ ? sink_->GetOutputDeviceInfo().device_status() - : OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND; - - UMA_HISTOGRAM_ENUMERATION("Media.WebAudioSourceProvider.SinkStatus", - device_status, OUTPUT_DEVICE_STATUS_MAX + 1); - - if (device_status != OUTPUT_DEVICE_STATUS_OK) { - // Since null sink is always OK, we will fall back to it once and forever. - if (sink_) - sink_->Stop(); - sink_ = CreateFallbackSink(); - MEDIA_LOG(ERROR, media_log_) - << "Output device error, falling back to null sink"; - } - tee_filter_->Initialize(renderer, params.channels(), params.sample_rate()); sink_->Initialize(params, tee_filter_.get()); @@ -262,9 +245,8 @@ bool WebAudioSourceProviderImpl::SetVolume(double volume) { } OutputDeviceInfo WebAudioSourceProviderImpl::GetOutputDeviceInfo() { - base::AutoLock auto_lock(sink_lock_); - return sink_ ? sink_->GetOutputDeviceInfo() - : OutputDeviceInfo(OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND); + NOTREACHED(); // The blocking API is intentionally not supported. + return OutputDeviceInfo(); } void WebAudioSourceProviderImpl::GetOutputDeviceInfoAsync( @@ -324,12 +306,6 @@ void WebAudioSourceProviderImpl::OnSetFormat() { client_->SetFormat(tee_filter_->channels(), tee_filter_->sample_rate()); } -scoped_refptr -WebAudioSourceProviderImpl::CreateFallbackSink() { - // Assuming it is called on media thread. - return new NullAudioSink(base::ThreadTaskRunnerHandle::Get()); -} - int WebAudioSourceProviderImpl::TeeFilter::Render( base::TimeDelta delay, base::TimeTicks delay_timestamp, diff --git a/media/blink/webaudiosourceprovider_impl.h b/media/blink/webaudiosourceprovider_impl.h index 4c68db2eee9e13..15038b8d52219d 100644 --- a/media/blink/webaudiosourceprovider_impl.h +++ b/media/blink/webaudiosourceprovider_impl.h @@ -80,7 +80,6 @@ class MEDIA_BLINK_EXPORT WebAudioSourceProviderImpl int RenderForTesting(AudioBus* audio_bus); protected: - virtual scoped_refptr CreateFallbackSink(); ~WebAudioSourceProviderImpl() override; private: diff --git a/media/blink/webaudiosourceprovider_impl_unittest.cc b/media/blink/webaudiosourceprovider_impl_unittest.cc index d253537fdeb5de..101575d7d64298 100644 --- a/media/blink/webaudiosourceprovider_impl_unittest.cc +++ b/media/blink/webaudiosourceprovider_impl_unittest.cc @@ -23,43 +23,10 @@ namespace media { namespace { const float kTestVolume = 0.25; const int kSampleRate = 48000; - -class WebAudioSourceProviderImplUnderTest : public WebAudioSourceProviderImpl { - public: - WebAudioSourceProviderImplUnderTest( - scoped_refptr sink) - : WebAudioSourceProviderImpl(std::move(sink), &media_log_), - fallback_sink_(new MockAudioRendererSink()) {} - - MockAudioRendererSink* fallback_sink() { return fallback_sink_.get(); } - - protected: - scoped_refptr CreateFallbackSink() override { - return fallback_sink_; - } - - private: - ~WebAudioSourceProviderImplUnderTest() override = default; - MediaLog media_log_; - scoped_refptr fallback_sink_; - - DISALLOW_COPY_AND_ASSIGN(WebAudioSourceProviderImplUnderTest); -}; - -enum class WaspSinkStatus { WASP_SINK_OK, WASP_SINK_ERROR, WASP_SINK_NULL }; - -scoped_refptr CreateWaspMockSink(WaspSinkStatus status) { - if (status == WaspSinkStatus::WASP_SINK_NULL) - return nullptr; - return new MockAudioRendererSink(status == WaspSinkStatus::WASP_SINK_OK - ? OUTPUT_DEVICE_STATUS_OK - : OUTPUT_DEVICE_STATUS_ERROR_INTERNAL); -} - } // namespace class WebAudioSourceProviderImplTest - : public testing::TestWithParam, + : public testing::Test, public blink::WebAudioSourceProviderClient { public: WebAudioSourceProviderImplTest() @@ -68,30 +35,27 @@ class WebAudioSourceProviderImplTest kSampleRate, 64), fake_callback_(0.1, kSampleRate), - mock_sink_(CreateWaspMockSink(GetParam())), - wasp_impl_(new WebAudioSourceProviderImplUnderTest(mock_sink_)), - expected_sink_(GetParam() == WaspSinkStatus::WASP_SINK_OK - ? mock_sink_.get() - : wasp_impl_->fallback_sink()) {} + mock_sink_(new MockAudioRendererSink()), + wasp_impl_(new WebAudioSourceProviderImpl(mock_sink_, &media_log_)) {} virtual ~WebAudioSourceProviderImplTest() = default; void CallAllSinkMethodsAndVerify(bool verify) { testing::InSequence s; - EXPECT_CALL(*expected_sink_, Start()).Times(verify); + EXPECT_CALL(*mock_sink_, Start()).Times(verify); wasp_impl_->Start(); - EXPECT_CALL(*expected_sink_, Play()).Times(verify); + EXPECT_CALL(*mock_sink_, Play()).Times(verify); wasp_impl_->Play(); - EXPECT_CALL(*expected_sink_, Pause()).Times(verify); + EXPECT_CALL(*mock_sink_, Pause()).Times(verify); wasp_impl_->Pause(); - EXPECT_CALL(*expected_sink_, SetVolume(kTestVolume)).Times(verify); + EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume)).Times(verify); wasp_impl_->SetVolume(kTestVolume); - EXPECT_CALL(*expected_sink_, Stop()).Times(verify); + EXPECT_CALL(*mock_sink_, Stop()).Times(verify); wasp_impl_->Stop(); testing::Mock::VerifyAndClear(mock_sink_.get()); @@ -101,14 +65,13 @@ class WebAudioSourceProviderImplTest testing::InSequence s; if (client) { - EXPECT_CALL(*expected_sink_, Stop()); + EXPECT_CALL(*mock_sink_, Stop()); EXPECT_CALL(*this, SetFormat(params_.channels(), params_.sample_rate())); } wasp_impl_->SetClient(client); base::RunLoop().RunUntilIdle(); testing::Mock::VerifyAndClear(mock_sink_.get()); - testing::Mock::VerifyAndClear(wasp_impl_->fallback_sink()); testing::Mock::VerifyAndClear(this); } @@ -140,31 +103,23 @@ class WebAudioSourceProviderImplTest return wasp_impl_->RenderForTesting(audio_bus); } - void ExpectUnhealthySinkToStop() { - if (GetParam() == WaspSinkStatus::WASP_SINK_ERROR) - EXPECT_CALL(*mock_sink_.get(), Stop()); - } - protected: AudioParameters params_; FakeAudioRenderCallback fake_callback_; + MediaLog media_log_; scoped_refptr mock_sink_; - scoped_refptr wasp_impl_; - MockAudioRendererSink* expected_sink_; + scoped_refptr wasp_impl_; DISALLOW_COPY_AND_ASSIGN(WebAudioSourceProviderImplTest); }; -TEST_P(WebAudioSourceProviderImplTest, SetClientBeforeInitialize) { +TEST_F(WebAudioSourceProviderImplTest, SetClientBeforeInitialize) { // setClient() with a NULL client should do nothing if no client is set. wasp_impl_->SetClient(NULL); // If |mock_sink_| is not null, it should be stopped during setClient(this). - // If it is unhealthy, it should also be stopped during fallback in - // Initialize(). if (mock_sink_) - EXPECT_CALL(*mock_sink_.get(), Stop()) - .Times(2 + static_cast(GetParam())); + EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2); wasp_impl_->SetClient(this); base::RunLoop().RunUntilIdle(); @@ -189,8 +144,7 @@ TEST_P(WebAudioSourceProviderImplTest, SetClientBeforeInitialize) { } // Verify AudioRendererSink functionality w/ and w/o a client. -TEST_P(WebAudioSourceProviderImplTest, SinkMethods) { - ExpectUnhealthySinkToStop(); +TEST_F(WebAudioSourceProviderImplTest, SinkMethods) { wasp_impl_->Initialize(params_, &fake_callback_); // Without a client all WASP calls should fall through to the underlying sink. @@ -202,23 +156,22 @@ TEST_P(WebAudioSourceProviderImplTest, SinkMethods) { CallAllSinkMethodsAndVerify(false); // Removing the client should cause WASP to revert to the underlying sink. - EXPECT_CALL(*expected_sink_, SetVolume(kTestVolume)); + EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume)); SetClient(NULL); CallAllSinkMethodsAndVerify(true); } // Verify underlying sink state is restored after client removal. -TEST_P(WebAudioSourceProviderImplTest, SinkStateRestored) { - ExpectUnhealthySinkToStop(); +TEST_F(WebAudioSourceProviderImplTest, SinkStateRestored) { wasp_impl_->Initialize(params_, &fake_callback_); // Verify state set before the client is set propagates back afterward. - EXPECT_CALL(*expected_sink_, Start()); + EXPECT_CALL(*mock_sink_, Start()); wasp_impl_->Start(); SetClient(this); - EXPECT_CALL(*expected_sink_, SetVolume(1.0)); - EXPECT_CALL(*expected_sink_, Start()); + EXPECT_CALL(*mock_sink_, SetVolume(1.0)); + EXPECT_CALL(*mock_sink_, Start()); SetClient(NULL); // Verify state set while the client was attached propagates back afterward. @@ -226,15 +179,14 @@ TEST_P(WebAudioSourceProviderImplTest, SinkStateRestored) { wasp_impl_->Play(); wasp_impl_->SetVolume(kTestVolume); - EXPECT_CALL(*expected_sink_, SetVolume(kTestVolume)); - EXPECT_CALL(*expected_sink_, Start()); - EXPECT_CALL(*expected_sink_, Play()); + EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume)); + EXPECT_CALL(*mock_sink_, Start()); + EXPECT_CALL(*mock_sink_, Play()); SetClient(NULL); } // Test the AudioRendererSink state machine and its effects on provideInput(). -TEST_P(WebAudioSourceProviderImplTest, ProvideInput) { - ExpectUnhealthySinkToStop(); +TEST_F(WebAudioSourceProviderImplTest, ProvideInput) { std::unique_ptr bus1 = AudioBus::Create(params_); std::unique_ptr bus2 = AudioBus::Create(params_); @@ -320,9 +272,7 @@ TEST_P(WebAudioSourceProviderImplTest, ProvideInput) { } // Verify CopyAudioCB is called if registered. -TEST_P(WebAudioSourceProviderImplTest, CopyAudioCB) { - ExpectUnhealthySinkToStop(); - +TEST_F(WebAudioSourceProviderImplTest, CopyAudioCB) { testing::InSequence s; wasp_impl_->Initialize(params_, &fake_callback_); wasp_impl_->SetCopyAudioCallback(base::Bind( @@ -339,11 +289,4 @@ TEST_P(WebAudioSourceProviderImplTest, CopyAudioCB) { testing::Mock::VerifyAndClear(mock_sink_.get()); } -INSTANTIATE_TEST_CASE_P( - /* prefix intentionally left blank due to only one parameterization */, - WebAudioSourceProviderImplTest, - testing::Values(WaspSinkStatus::WASP_SINK_OK, - WaspSinkStatus::WASP_SINK_ERROR, - WaspSinkStatus::WASP_SINK_NULL)); - } // namespace media diff --git a/media/renderers/BUILD.gn b/media/renderers/BUILD.gn index 9a0852784d1162..56af800cd2d82b 100644 --- a/media/renderers/BUILD.gn +++ b/media/renderers/BUILD.gn @@ -39,6 +39,7 @@ source_set("renderers") { "//components/viz/client", "//gpu/command_buffer/client:gles2_interface", "//gpu/command_buffer/common", + "//media/audio", "//media/base", "//media/filters", "//media/video", diff --git a/media/renderers/audio_renderer_impl.cc b/media/renderers/audio_renderer_impl.cc index 79c34da2ab0ff3..287361281bd60c 100644 --- a/media/renderers/audio_renderer_impl.cc +++ b/media/renderers/audio_renderer_impl.cc @@ -15,11 +15,13 @@ #include "base/callback_helpers.h" #include "base/command_line.h" #include "base/logging.h" +#include "base/metrics/histogram_macros.h" #include "base/power_monitor/power_monitor.h" #include "base/single_thread_task_runner.h" #include "base/time/default_tick_clock.h" #include "base/trace_event/trace_event.h" #include "build/build_config.h" +#include "media/audio/null_audio_sink.h" #include "media/base/audio_buffer.h" #include "media/base/audio_buffer_converter.h" #include "media/base/audio_latency.h" @@ -367,18 +369,53 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream, // If we are re-initializing playback (e.g. switching media tracks), stop the // sink first. - if (state_ == kFlushed) { + if (state_ == kFlushed) sink_->Stop(); - audio_clock_.reset(); - } state_ = kInitializing; client_ = client; + // Always post |init_cb_| because |this| could be destroyed if initialization + // failed. + init_cb_ = BindToCurrentLoop(init_cb); + + // Retrieve hardware device parameters asynchronously so we don't block the + // media thread on synchronous IPC. + sink_->GetOutputDeviceInfoAsync( + base::BindOnce(&AudioRendererImpl::OnDeviceInfoReceived, + weak_factory_.GetWeakPtr(), stream, cdm_context)); +} + +void AudioRendererImpl::OnDeviceInfoReceived( + DemuxerStream* stream, + CdmContext* cdm_context, + OutputDeviceInfo output_device_info) { + DVLOG(1) << __func__; + DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(client_); + DCHECK(stream); + DCHECK_EQ(stream->type(), DemuxerStream::AUDIO); + DCHECK(init_cb_); + DCHECK_EQ(state_, kInitializing); + + // Fall-back to a fake audio sink if the audio device can't be setup; this + // allows video playback in cases where there is no audio hardware. + // + // TODO(dalecurtis): We could disable the audio track here too. + UMA_HISTOGRAM_ENUMERATION("Media.AudioRendererImpl.SinkStatus", + output_device_info.device_status(), + OUTPUT_DEVICE_STATUS_MAX + 1); + if (output_device_info.device_status() != OUTPUT_DEVICE_STATUS_OK) { + sink_ = new NullAudioSink(task_runner_); + output_device_info = sink_->GetOutputDeviceInfo(); + MEDIA_LOG(ERROR, media_log_) + << "Output device error, falling back to null sink. device_status=" + << output_device_info.device_status(); + } + current_decoder_config_ = stream->audio_decoder_config(); DCHECK(current_decoder_config_.IsValidConfig()); - auto output_device_info = sink_->GetOutputDeviceInfo(); const AudioParameters& hw_params = output_device_info.output_params(); ChannelLayout hw_channel_layout = hw_params.IsValid() ? hw_params.channel_layout() : CHANNEL_LAYOUT_NONE; @@ -391,10 +428,6 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream, audio_decoder_stream_->set_config_change_observer(base::BindRepeating( &AudioRendererImpl::OnConfigChange, weak_factory_.GetWeakPtr())); - // Always post |init_cb_| because |this| could be destroyed if initialization - // failed. - init_cb_ = BindToCurrentLoop(init_cb); - AudioCodec codec = stream->audio_decoder_config().codec(); if (auto* mc = GetMediaClient()) is_passthrough_ = mc->IsSupportedBitstreamAudioCodec(codec); @@ -532,8 +565,13 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream, last_decoded_channels_ = stream->audio_decoder_config().channels(); - audio_clock_.reset( - new AudioClock(base::TimeDelta(), audio_parameters_.sample_rate())); + { + // Set the |audio_clock_| under lock in case this is a reinitialize and some + // external caller to GetWallClockTimes() exists. + base::AutoLock lock(lock_); + audio_clock_.reset( + new AudioClock(base::TimeDelta(), audio_parameters_.sample_rate())); + } audio_decoder_stream_->Initialize( stream, diff --git a/media/renderers/audio_renderer_impl.h b/media/renderers/audio_renderer_impl.h index 883c5e5579c1d5..ddd7db18c394a8 100644 --- a/media/renderers/audio_renderer_impl.h +++ b/media/renderers/audio_renderer_impl.h @@ -125,6 +125,11 @@ class MEDIA_EXPORT AudioRendererImpl kPlaying }; + // Called after hardware device information is available. + void OnDeviceInfoReceived(DemuxerStream* stream, + CdmContext* cdm_context, + OutputDeviceInfo output_device_info); + // Callback from the audio decoder delivering decoded audio samples. void DecodedAudioReady(AudioDecoderStream::Status status, const scoped_refptr& buffer); diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 42fc61210e31a0..e52afc7e3f98f0 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -45433,6 +45433,18 @@ uploading your change for review. Captures statistics for various AudioRendererImpl events. + + olka@chromium.org + dalecurtis@chromium.org + + Status of audio sink used by AudioRendererImpl. If not OK, a NullAudioSink + will be used for audio output instead. This is logged for every call to + AudioRendererImpl::Initialize, which generally occurs once per active audio + session (i.e., between a play and pause). If audio track changes are ever + enabled, it may additionally be called for every audio track change. + + + Deprecated 02/2017. No longer needed. @@ -48692,6 +48704,9 @@ uploading your change for review. + + Replaced by Media.AudioRendererImpl.SinkStatus in Nov 2018. + olka@chromium.org dalecurtis@chromium.org