diff options
author | dalecurtis <dalecurtis@chromium.org> | 2015-02-05 15:19:45 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-02-05 23:21:44 +0000 |
commit | c85184da42d043995203dbba777f08af00181d65 (patch) | |
tree | 2620ac1175dfd2a8c4b7952b0d6121f419cb16c8 | |
parent | 8468daefc9eb29a3ecf895698798cabe047264f1 (diff) | |
download | chromium_src-c85184da42d043995203dbba777f08af00181d65.zip chromium_src-c85184da42d043995203dbba777f08af00181d65.tar.gz chromium_src-c85184da42d043995203dbba777f08af00181d65.tar.bz2 |
Purge AudioOutputDispatcher after close delay if no proxies exist.
This prevents transient audio creation errors from cumulatively
breaking audio output by blackholing everything to a fake audio
stream after fallback attempts are exhausted.
BUG=453560
TEST=new unittest, manual testing.
Review URL: https://codereview.chromium.org/899923003
Cr-Commit-Position: refs/heads/master@{#314910}
-rw-r--r-- | media/audio/audio_output_dispatcher_impl.cc | 4 | ||||
-rw-r--r-- | media/audio/audio_output_dispatcher_impl.h | 3 | ||||
-rw-r--r-- | media/audio/audio_output_proxy_unittest.cc | 53 | ||||
-rw-r--r-- | media/audio/audio_output_resampler.cc | 35 | ||||
-rw-r--r-- | media/audio/audio_output_resampler.h | 28 |
5 files changed, 112 insertions, 11 deletions
diff --git a/media/audio/audio_output_dispatcher_impl.cc b/media/audio/audio_output_dispatcher_impl.cc index 0cb3db8..521f91d 100644 --- a/media/audio/audio_output_dispatcher_impl.cc +++ b/media/audio/audio_output_dispatcher_impl.cc @@ -130,6 +130,10 @@ void AudioOutputDispatcherImpl::Shutdown() { DCHECK(HasOneRef()) << "Only the AudioManager should hold a reference"; } +bool AudioOutputDispatcherImpl::HasOutputProxies() const { + return idle_proxies_ || !proxy_to_physical_map_.empty(); +} + bool AudioOutputDispatcherImpl::CreateAndOpenStream() { DCHECK(task_runner_->BelongsToCurrentThread()); AudioOutputStream* stream = audio_manager_->MakeAudioOutputStream( diff --git a/media/audio/audio_output_dispatcher_impl.h b/media/audio/audio_output_dispatcher_impl.h index 1aa5a32..d271784 100644 --- a/media/audio/audio_output_dispatcher_impl.h +++ b/media/audio/audio_output_dispatcher_impl.h @@ -60,6 +60,9 @@ class MEDIA_EXPORT AudioOutputDispatcherImpl : public AudioOutputDispatcher { void Shutdown() override; + // Returns true if there are any open AudioOutputProxy objects. + bool HasOutputProxies() const; + private: friend class base::RefCountedThreadSafe<AudioOutputDispatcherImpl>; ~AudioOutputDispatcherImpl() override; diff --git a/media/audio/audio_output_proxy_unittest.cc b/media/audio/audio_output_proxy_unittest.cc index 66f8987..b0750cc 100644 --- a/media/audio/audio_output_proxy_unittest.cc +++ b/media/audio/audio_output_proxy_unittest.cc @@ -674,4 +674,57 @@ TEST_F(AudioOutputResamplerTest, LowLatencyOpenEventuallyFails) { EXPECT_TRUE(stream2.start_called()); } +// Simulate failures to open both the low latency and the fallback high latency +// stream and ensure AudioOutputResampler falls back to a fake stream. Ensure +// that after the close delay elapses, opening another stream succeeds with a +// non-fake stream. +TEST_F(AudioOutputResamplerTest, FallbackRecovery) { + MockAudioOutputStream fake_stream(&manager_, params_); + + // Trigger the fallback mechanism until a fake output stream is created. +#if defined(OS_WIN) + static const int kFallbackCount = 2; +#else + static const int kFallbackCount = 1; +#endif + EXPECT_CALL(manager(), MakeAudioOutputStream(_, _)) + .Times(kFallbackCount) + .WillRepeatedly(Return(static_cast<AudioOutputStream*>(NULL))); + EXPECT_CALL(manager(), + MakeAudioOutputStream( + AllOf(testing::Property(&AudioParameters::format, + AudioParameters::AUDIO_FAKE), + testing::Property(&AudioParameters::sample_rate, + params_.sample_rate()), + testing::Property(&AudioParameters::frames_per_buffer, + params_.frames_per_buffer())), + _)).WillOnce(Return(&fake_stream)); + EXPECT_CALL(fake_stream, Open()).WillOnce(Return(true)); + AudioOutputProxy* proxy = new AudioOutputProxy(resampler_.get()); + EXPECT_TRUE(proxy->Open()); + CloseAndWaitForCloseTimer(proxy, &fake_stream); + + // Once all proxies have been closed, AudioOutputResampler will start the + // reinitialization timer and execute it after the close delay elapses. + base::RunLoop run_loop; + message_loop_.PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromMilliseconds(2 * kTestCloseDelayMs)); + run_loop.Run(); + + // Verify a non-fake stream can be created. + MockAudioOutputStream real_stream(&manager_, params_); + EXPECT_CALL(manager(), + MakeAudioOutputStream( + testing::Property(&AudioParameters::format, + testing::Ne(AudioParameters::AUDIO_FAKE)), + _)).WillOnce(Return(&real_stream)); + + // Stream1 should be able to successfully open and start. + EXPECT_CALL(real_stream, Open()).WillOnce(Return(true)); + proxy = new AudioOutputProxy(resampler_.get()); + EXPECT_TRUE(proxy->Open()); + CloseAndWaitForCloseTimer(proxy, &real_stream); +} + } // namespace media diff --git a/media/audio/audio_output_resampler.cc b/media/audio/audio_output_resampler.cc index 7aa3284..f053444 100644 --- a/media/audio/audio_output_resampler.cc +++ b/media/audio/audio_output_resampler.cc @@ -11,6 +11,7 @@ #include "base/numerics/safe_conversions.h" #include "base/single_thread_task_runner.h" #include "base/time/time.h" +#include "base/trace_event/trace_event.h" #include "build/build_config.h" #include "media/audio/audio_io.h" #include "media/audio/audio_output_dispatcher_impl.h" @@ -153,7 +154,13 @@ AudioOutputResampler::AudioOutputResampler(AudioManager* audio_manager, : AudioOutputDispatcher(audio_manager, input_params, output_device_id), close_delay_(close_delay), output_params_(output_params), - streams_opened_(false) { + original_output_params_(output_params), + streams_opened_(false), + reinitialize_timer_(FROM_HERE, + close_delay_, + base::Bind(&AudioOutputResampler::Reinitialize, + base::Unretained(this)), + false) { DCHECK(input_params.IsValid()); DCHECK(output_params.IsValid()); DCHECK_EQ(output_params_.format(), AudioParameters::AUDIO_PCM_LOW_LATENCY); @@ -168,6 +175,24 @@ AudioOutputResampler::~AudioOutputResampler() { DCHECK(callbacks_.empty()); } +void AudioOutputResampler::Reinitialize() { + DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(streams_opened_); + + // We can only reinitialize the dispatcher if it has no active proxies. Check + // if one has been created since the reinitialization timer was started. + if (dispatcher_->HasOutputProxies()) + return; + + // Log a trace event so we can get feedback in the field when this happens. + TRACE_EVENT0("audio", "AudioOutputResampler::Reinitialize"); + + dispatcher_->Shutdown(); + output_params_ = original_output_params_; + streams_opened_ = false; + Initialize(); +} + void AudioOutputResampler::Initialize() { DCHECK(!streams_opened_); DCHECK(callbacks_.empty()); @@ -282,6 +307,14 @@ void AudioOutputResampler::CloseStream(AudioOutputProxy* stream_proxy) { delete it->second; callbacks_.erase(it); } + + // Start the reinitialization timer if there are no active proxies and we're + // not using the originally requested output parameters. This allows us to + // recover from transient output creation errors. + if (!dispatcher_->HasOutputProxies() && callbacks_.empty() && + !output_params_.Equals(original_output_params_)) { + reinitialize_timer_.Reset(); + } } void AudioOutputResampler::Shutdown() { diff --git a/media/audio/audio_output_resampler.h b/media/audio/audio_output_resampler.h index 4c7be29..18d4905 100644 --- a/media/audio/audio_output_resampler.h +++ b/media/audio/audio_output_resampler.h @@ -10,9 +10,10 @@ #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "base/time/time.h" +#include "base/timer/timer.h" #include "media/audio/audio_io.h" #include "media/audio/audio_manager.h" -#include "media/audio/audio_output_dispatcher.h" +#include "media/audio/audio_output_dispatcher_impl.h" #include "media/audio/audio_parameters.h" namespace media { @@ -29,12 +30,7 @@ class OnMoreDataConverter; // // AOR will automatically fall back from AUDIO_PCM_LOW_LATENCY to // AUDIO_PCM_LINEAR if the output device fails to open at the requested output -// parameters. -// -// TODO(dalecurtis): Ideally the low latency path will be as reliable as the -// high latency path once we have channel mixing and support querying for the -// hardware's configured bit depth. Monitor the UMA stats for fallback and -// remove fallback support once it's stable. http://crbug.com/148418 +// parameters. If opening still fails, it will fallback to AUDIO_FAKE. class MEDIA_EXPORT AudioOutputResampler : public AudioOutputDispatcher { public: AudioOutputResampler(AudioManager* audio_manager, @@ -60,11 +56,14 @@ class MEDIA_EXPORT AudioOutputResampler : public AudioOutputDispatcher { // appropriate output parameters in error situations. void SetupFallbackParams(); - // Used to initialize and reinitialize |dispatcher_|. + // Used to reinitialize |dispatcher_|. + void Reinitialize(); + + // Used to initialize |dispatcher_|. void Initialize(); // Dispatcher to proxy all AudioOutputDispatcher calls too. - scoped_refptr<AudioOutputDispatcher> dispatcher_; + scoped_refptr<AudioOutputDispatcherImpl> dispatcher_; // Map of outstanding OnMoreDataConverter objects. A new object is created // on every StartStream() call and destroyed on CloseStream(). @@ -74,13 +73,22 @@ class MEDIA_EXPORT AudioOutputResampler : public AudioOutputDispatcher { // Used by AudioOutputDispatcherImpl; kept so we can reinitialize on the fly. base::TimeDelta close_delay_; - // AudioParameters used to setup the output stream. + // AudioParameters used to setup the output stream; changed upon fallback. AudioParameters output_params_; + // The original AudioParameters we were constructed with. + const AudioParameters original_output_params_; + // Whether any streams have been opened through |dispatcher_|, if so we can't // fallback on future OpenStream() failures. bool streams_opened_; + // The reinitialization timer provides a way to recover from temporary failure + // states by clearing the dispatcher if all proxies have been closed and none + // have been created within |close_delay_|. Without this, audio may be lost + // to a fake stream indefinitely for transient errors. + base::Timer reinitialize_timer_; + DISALLOW_COPY_AND_ASSIGN(AudioOutputResampler); }; |