summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordalecurtis <dalecurtis@chromium.org>2015-02-05 15:19:45 -0800
committerCommit bot <commit-bot@chromium.org>2015-02-05 23:21:44 +0000
commitc85184da42d043995203dbba777f08af00181d65 (patch)
tree2620ac1175dfd2a8c4b7952b0d6121f419cb16c8
parent8468daefc9eb29a3ecf895698798cabe047264f1 (diff)
downloadchromium_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.cc4
-rw-r--r--media/audio/audio_output_dispatcher_impl.h3
-rw-r--r--media/audio/audio_output_proxy_unittest.cc53
-rw-r--r--media/audio/audio_output_resampler.cc35
-rw-r--r--media/audio/audio_output_resampler.h28
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);
};