Always wait for DataReady() on Windows WaveOut.

Only WaveOut still needs this hack and since all renderer clients
now use the shared memory marker we can always wait for it.

I suspect on low end machines this timeout is leading to pops and
clicks for some users on XP.

BUG=161307
TEST=audio still works.

Review URL: https://codereview.chromium.org/11348166

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@169553 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 52 insertions, 25 deletions

diff --git a/media/audio/audio_io.h b/media/audio/audio_io.h
index cfc71bd..e7b9a36 100644
--- a/media/audio/audio_io.h
+++ b/media/audio/audio_io.h
@@ -74,14 +74,10 @@ class MEDIA_EXPORT AudioOutputStream {
     // specific.
     virtual void OnError(AudioOutputStream* stream, int code) = 0;
 
-    // Waits till data becomes available. Used when buffering data starting
-    // new audio stream.
-    // Polling is not the best approach, but incorporating messaging loop
-    // with delayed tasks into guts of complex code is even worse, as it is
-    // very error-prone. We cannot easily add synchronization, interface is
-    // already cut in stone because of need of backward compatibility with
-    // plugins. In any case, data is usually immediately available,
-    // so there would be no delay.
+    // Deprecated.  DO NOT USE.  Waits until data becomes available.  Used only
+    // by Windows' WaveOut clients which may be extremely laggy.  Will yield the
+    // current thread until the renderer client has written its audio data or
+    // 1.5 seconds have elapsed.
     virtual void WaitTillDataReady() {}
 
    protected:
diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index f21da8f..50850c9 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -11,6 +11,7 @@
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
+#include "build/build_config.h"
 #include "media/audio/shared_memory_util.h"
 
 using base::Time;
@@ -319,17 +320,21 @@ int AudioOutputController::OnMoreIOData(AudioBus* source,
 }
 
 void AudioOutputController::WaitTillDataReady() {
-  if (!sync_reader_->DataReady()) {
-    // In the different place we use different mechanism to poll, get max
-    // polling delay from constants used there.
-    const base::TimeDelta kMaxPollingDelay = TimeDelta::FromMilliseconds(
-        kPollNumAttempts * kPollPauseInMilliseconds);
-    Time start_time = Time::Now();
-    do {
-      base::PlatformThread::Sleep(TimeDelta::FromMilliseconds(1));
-    } while (!sync_reader_->DataReady() &&
-             Time::Now() - start_time < kMaxPollingDelay);
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  base::Time start = base::Time::Now();
+  // Wait for up to 1.5 seconds for DataReady().  1.5 seconds was chosen because
+  // it's larger than the playback time of the WaveOut buffer size using the
+  // minimum supported sample rate: 4096 / 3000 = ~1.4 seconds.  Even a client
+  // expecting real time playout should be able to fill in this time.
+  const base::TimeDelta max_wait = base::TimeDelta::FromMilliseconds(1500);
+  while (!sync_reader_->DataReady() &&
+         ((base::Time::Now() - start) < max_wait)) {
+    base::PlatformThread::YieldCurrentThread();
   }
+#else
+  // WaitTillDataReady() is deprecated and should not be used.
+  CHECK(false);
+#endif
 }
 
 void AudioOutputController::OnError(AudioOutputStream* stream, int code) {
diff --git a/media/audio/audio_output_controller.h b/media/audio/audio_output_controller.h
index 3dae719..762a948 100644
--- a/media/audio/audio_output_controller.h
+++ b/media/audio/audio_output_controller.h
@@ -102,11 +102,7 @@ class MEDIA_EXPORT AudioOutputController
     // Close this synchronous reader.
     virtual void Close() = 0;
 
-    // Poll if data is ready.
-    // Not reliable, as there is no guarantee that renderer is "new-style"
-    // renderer that writes metadata into buffer. After several unsuccessful
-    // attempts caller should assume the data is ready even if that function
-    // returns false.
+    // Check if data is ready.
     virtual bool DataReady() = 0;
   };
 
diff --git a/media/audio/audio_output_resampler.cc b/media/audio/audio_output_resampler.cc
index baad88a..4734e40 100644
--- a/media/audio/audio_output_resampler.cc
+++ b/media/audio/audio_output_resampler.cc
@@ -11,6 +11,7 @@
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/time.h"
+#include "build/build_config.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_output_dispatcher_impl.h"
 #include "media/audio/audio_output_proxy.h"
@@ -20,6 +21,10 @@
 #include "media/base/limits.h"
 #include "media/base/media_switches.h"
 
+#if defined(OS_WIN)
+#include "media/audio/win/core_audio_util_win.h"
+#endif
+
 namespace media {
 
 class OnMoreDataConverter
@@ -72,6 +77,10 @@ class OnMoreDataConverter
   // parameters.
   AudioConverter audio_converter_;
 
+  // If we're using WaveOut on Windows' we always have to wait for DataReady()
+  // before calling |source_callback_|.
+  bool waveout_wait_hack_;
+
   DISALLOW_COPY_AND_ASSIGN(OnMoreDataConverter);
 };
 
@@ -278,10 +287,22 @@ OnMoreDataConverter::OnMoreDataConverter(const AudioParameters& input_params,
     : source_callback_(NULL),
       source_bus_(NULL),
       input_bytes_per_second_(input_params.GetBytesPerSecond()),
-      audio_converter_(input_params, output_params, false) {
+      audio_converter_(input_params, output_params, false),
+      waveout_wait_hack_(false) {
   io_ratio_ =
       static_cast<double>(input_params.GetBytesPerSecond()) /
       output_params.GetBytesPerSecond();
+
+  // TODO(dalecurtis): We should require all render side clients to use a
+  // buffer size that's a multiple of the hardware buffer size scaled by the
+  // request_sample_rate / hw_sample_rate.  Doing so ensures each hardware
+  // request for audio data results in only a single render side callback and
+  // would allow us to remove this hack.  See http://crbug.com/162207.
+#if defined(OS_WIN)
+  waveout_wait_hack_ =
+      output_params.format() == AudioParameters::AUDIO_PCM_LINEAR ||
+      !CoreAudioUtil::IsSupported();
+#endif
 }
 
 OnMoreDataConverter::~OnMoreDataConverter() {}
@@ -340,6 +361,9 @@ double OnMoreDataConverter::ProvideInput(AudioBus* dest,
       io_ratio_ * (current_buffers_state_.total_bytes() +
                    buffer_delay.InSecondsF() * input_bytes_per_second_);
 
+  if (waveout_wait_hack_)
+    source_callback_->WaitTillDataReady();
+
   // Retrieve data from the original callback.
   int frames = source_callback_->OnMoreIOData(
       source_bus_, dest, new_buffers_state);
diff --git a/media/audio/win/waveout_output_win.cc b/media/audio/win/waveout_output_win.cc
index 5e56169..f18b2fa 100644
--- a/media/audio/win/waveout_output_win.cc
+++ b/media/audio/win/waveout_output_win.cc
@@ -76,7 +76,6 @@ inline WAVEHDR* PCMWaveOutAudioOutputStream::GetBuffer(int n) const {
   return reinterpret_cast<WAVEHDR*>(&buffers_[n * BufferSize()]);
 }
 
-
 PCMWaveOutAudioOutputStream::PCMWaveOutAudioOutputStream(
     AudioManagerWin* manager, const AudioParameters& params, int num_buffers,
     UINT device_id)
@@ -342,6 +341,13 @@ void PCMWaveOutAudioOutputStream::QueueNextPacket(WAVEHDR *buffer) {
   // return to us how many bytes were used.
   // TODO(fbarchard): Handle used 0 by queueing more.
 
+  // HACK: Yield if Read() is called too often.  On older platforms which are
+  // still using the WaveOut backend, we run into synchronization issues where
+  // the renderer has not finished filling the shared memory when Read() is
+  // called.  Reading too early will lead to clicks and pops.  See issues:
+  // http://crbug.com/161307 and http://crbug.com/61022
+  callback_->WaitTillDataReady();
+
   // TODO(sergeyu): Specify correct hardware delay for AudioBuffersState.
   int frames_filled = callback_->OnMoreData(
       audio_bus_.get(), AudioBuffersState(pending_bytes_, 0));