Always wait for DataReady() on Windows WaveOut.

content/browser/renderer_host/media/audio_sync_reader.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/audio_sync_reader.h"
 
 #include <algorithm>
 
 #include "base/process_util.h"
 #include "base/shared_memory.h"
 #include "base/threading/platform_thread.h"
 #include "media/audio/audio_buffers_state.h"
 #include "media/audio/audio_parameters.h"
 #include "media/audio/shared_memory_util.h"
 
-#if defined(OS_WIN)
-const int kMinIntervalBetweenReadCallsInMs = 10;
-#endif
-
 using media::AudioBus;
 
 namespace content {
 
 AudioSyncReader::AudioSyncReader(base::SharedMemory* shared_memory,
                                  const media::AudioParameters& params,
                                  int input_channels)
     : shared_memory_(shared_memory),
       input_channels_(input_channels) {
   packet_size_ = media::PacketSizeInBytes(shared_memory_->created_size());
@@ skipping 26 matching lines @@
     media::SetUnknownDataSize(shared_memory_, packet_size_);
   }
 
   if (socket_.get()) {
     socket_->Send(&bytes, sizeof(bytes));
   }
 }
 
 int AudioSyncReader::Read(AudioBus* source, AudioBus* dest) {
 #if defined(OS_WIN)
-  // HACK: yield if reader is called too often.
-  // Problem is lack of synchronization between host and renderer. We cannot be
-  // sure if renderer already filled the buffer, and due to all the plugins we
-  // cannot change the API, so we yield if previous call was too recent.
-  // Optimization: if renderer is "new" one that writes length of data we can
-  // stop yielding the moment length is written -- not ideal solution,
-  // but better than nothing.
-  while (!DataReady() &&
-         ((base::Time::Now() - previous_call_time_).InMilliseconds() <
-          kMinIntervalBetweenReadCallsInMs)) {
+  // HACK: Yield if Read() is called too often.  On older platforms which are
+  // still using the WaveOut back end, 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

[scherkus (not reviewing), 2012/11/20 21:08:46]

does it make sense to add a run-time check for XP?

[DaleCurtis, 2012/11/20 21:33:47]

I thought about that, but there are cases where we fall back to WaveOut even on
Vista+.  Ideally this check would be something like !low_latency_path ||
!DataReady() ...

On WASAPI based platforms this should be called in regular intervals large
enough that DataReady() is always true, so it becomes a semi-nop.

+  while (!DataReady()) {
     base::PlatformThread::YieldCurrentThread();
   }
-  previous_call_time_ = base::Time::Now();
 #endif
 
   // Copy optional synchronized live audio input for consumption by renderer
   // process.
   if (source && input_bus_.get()) {
     DCHECK_EQ(source->channels(), input_bus_->channels());
     DCHECK_LE(source->frames(), input_bus_->frames());
     source->CopyTo(input_bus_.get());
   }
 
@@ skipping 64 matching lines @@
     base::FileDescriptor* foreign_handle) {
   foreign_handle->fd = foreign_socket_->handle();
   foreign_handle->auto_close = false;
   if (foreign_handle->fd != -1)
     return true;
   return false;
 }
 #endif
 
 }  // namespace content