Fix audio "clicking" for strongbad demo

The strongbad demo plays an audio file of 11kHz. However we hardcoded
in the AudioRendererHost to always open an audio device of 8k samples
which happens to be too big for streams of 11khz. This makes the IPC
transport packet size much smaller than the hardware buffer size and
hardware packet is always partially filled.

This change allow the AudioRendererHost to designate the transport
packet size based on the hardware packet size. The hardware packet
size is now adjusted based on the sample rate.

BUG=46007
TEST=http://smokescreen.us/demos/sb45demo.html
The above audio plays fine.

Review URL: http://codereview.chromium.org/2651001

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

7 files changed, 59 insertions, 68 deletions

diff --git a/chrome/browser/renderer_host/audio_renderer_host.cc b/chrome/browser/renderer_host/audio_renderer_host.cc
index 9019c50..0fbc5e6 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host.cc
@@ -31,20 +31,22 @@
 #include "chrome/common/render_messages.h"
 #include "ipc/ipc_logging.h"
 
-namespace {
+// The minimum number of samples in a hardware packet.
+// This value is selected so that we can handle down to 5khz sample rate.
+static const int kMinSamplesPerHardwarePacket = 1024;
 
-// This constant governs the hardware audio buffer size, this value should be
-// choosen carefully and is platform specific.
-static const int kSamplesPerHardwarePacket = 8192;
-
-// If the size of the buffer is less than this number, then the low latency
-// mode is to be used.
-static const uint32 kLowLatencyPacketThreshold = 1025;
+// The maximum number of samples in a hardware packet.
+// This value is selected so that we can handle up to 192khz sample rate.
+static const int kMaxSamplesPerHardwarePacket = 64 * 1024;
 
-static const uint32 kMegabytes = 1024 * 1024;
+// This constant governs the hardware audio buffer size, this value should be
+// chosen carefully.
+// This value is selected so that we have 8192 samples for 48khz streams.
+static const int kMillisecondsPerHardwarePacket = 170;
 
 // The following parameters limit the request buffer and packet size from the
 // renderer to avoid renderer from requesting too much memory.
+static const uint32 kMegabytes = 1024 * 1024;
 static const uint32 kMaxDecodedPacketSize = 2 * kMegabytes;
 static const uint32 kMaxBufferCapacity = 5 * kMegabytes;
 static const int kMaxChannels = 32;
@@ -61,7 +63,7 @@ static const size_t kMaxStreamsLeopard = 15;
 
 // Returns the number of audio streams allowed. This is a practical limit to
 // prevent failure caused by too many audio streams opened.
-size_t GetMaxAudioStreamsAllowed() {
+static size_t GetMaxAudioStreamsAllowed() {
 #if defined(OS_MACOSX)
   // We are hitting a bug in Leopard where too many audio streams will cause
   // a deadlock in the AudioQueue API when starting the stream. Unfortunately
@@ -79,7 +81,18 @@ size_t GetMaxAudioStreamsAllowed() {
   return kMaxStreams;
 }
 
-}  // namespace
+static uint32 SelectHardwarePacketSize(int channels, int sample_rate,
+                                       int bits_per_sample) {
+  // Select the number of samples that can provide at least
+  // |kMillisecondsPerHardwarePacket| worth of audio data.
+  int samples = kMinSamplesPerHardwarePacket;
+  while (samples <= kMaxSamplesPerHardwarePacket &&
+         samples * base::Time::kMillisecondsPerSecond <
+         sample_rate * kMillisecondsPerHardwarePacket) {
+    samples *= 2;
+  }
+  return channels * samples * bits_per_sample / 8;
+}
 
 //-----------------------------------------------------------------------------
 // AudioRendererHost::IPCAudioSource implementations.
@@ -91,7 +104,6 @@ AudioRendererHost::IPCAudioSource::IPCAudioSource(
     int stream_id,
     AudioOutputStream* stream,
     uint32 hardware_packet_size,
-    uint32 decoded_packet_size,
     uint32 buffer_capacity)
     : host_(host),
       process_id_(process_id),
@@ -99,7 +111,6 @@ AudioRendererHost::IPCAudioSource::IPCAudioSource(
       stream_id_(stream_id),
       stream_(stream),
       hardware_packet_size_(hardware_packet_size),
-      decoded_packet_size_(decoded_packet_size),
       buffer_capacity_(buffer_capacity),
       state_(kCreated),
       outstanding_request_(false),
@@ -122,19 +133,9 @@ AudioRendererHost::IPCAudioSource::CreateIPCAudioSource(
     int channels,
     int sample_rate,
     char bits_per_sample,
-    uint32 decoded_packet_size,
-    uint32 buffer_capacity,
+    uint32 packet_size,
     bool low_latency) {
   // Perform come preliminary checks on the parameters.
-  // Make sure the renderer didn't ask for too much memory.
-  if (buffer_capacity > kMaxBufferCapacity ||
-      decoded_packet_size > kMaxDecodedPacketSize)
-    return NULL;
-
-  // Make sure the packet size and buffer capacity parameters are valid.
-  if (buffer_capacity < decoded_packet_size)
-    return NULL;
-
   if (channels <= 0 || channels > kMaxChannels)
     return NULL;
 
@@ -149,8 +150,21 @@ AudioRendererHost::IPCAudioSource::CreateIPCAudioSource(
       AudioManager::GetAudioManager()->MakeAudioStream(
           format, channels, sample_rate, bits_per_sample);
 
-  uint32 hardware_packet_size = kSamplesPerHardwarePacket * channels *
-                                bits_per_sample / 8;
+  uint32 hardware_packet_size = packet_size;
+
+  // If the packet size is not specified in the message we generate the best
+  // value here.
+  if (!hardware_packet_size) {
+    hardware_packet_size =
+        SelectHardwarePacketSize(channels, sample_rate, bits_per_sample);
+  }
+  uint32 transport_packet_size = hardware_packet_size;
+
+  // We set the buffer capacity to be more than the total buffer amount in the
+  // audio hardware. This gives about 500ms of buffer which is a safe amount
+  // to avoid "audio clicks".
+  uint32 buffer_capacity = 3 * hardware_packet_size;
+
   if (stream && !stream->Open(hardware_packet_size)) {
     stream->Close();
     stream = NULL;
@@ -164,7 +178,6 @@ AudioRendererHost::IPCAudioSource::CreateIPCAudioSource(
         stream_id,
         stream,
         hardware_packet_size,
-        decoded_packet_size,
         buffer_capacity);
     // If we can open the stream, proceed with sharing the shared memory.
     base::SharedMemoryHandle foreign_memory_handle;
@@ -175,11 +188,9 @@ AudioRendererHost::IPCAudioSource::CreateIPCAudioSource(
     // Note that the low latency mode is not yet ready and the if part of this
     // method is never executed. TODO(cpu): Enable this mode.
 
-    if (source->shared_memory_.Create(L"",
-                                      false,
-                                      false,
-                                      decoded_packet_size) &&
-        source->shared_memory_.Map(decoded_packet_size) &&
+    if (source->shared_memory_.Create(L"", false, false,
+                                      transport_packet_size) &&
+        source->shared_memory_.Map(transport_packet_size) &&
         source->shared_memory_.ShareToProcess(process_handle,
                                               &foreign_memory_handle)) {
       if (low_latency) {
@@ -202,14 +213,15 @@ AudioRendererHost::IPCAudioSource::CreateIPCAudioSource(
           if (valid) {
             host->Send(new ViewMsg_NotifyLowLatencyAudioStreamCreated(
                 route_id, stream_id, foreign_memory_handle,
-                foreign_socket_handle, decoded_packet_size));
+                foreign_socket_handle, transport_packet_size));
             return source;
           }
         }
       } else {
         // Regular latency mode.
         host->Send(new ViewMsg_NotifyAudioStreamCreated(
-            route_id, stream_id, foreign_memory_handle, decoded_packet_size));
+            route_id, stream_id, foreign_memory_handle,
+            transport_packet_size));
 
         // Also request the first packet to kick start the pre-rolling.
         source->StartBuffering();
@@ -374,7 +386,7 @@ void AudioRendererHost::IPCAudioSource::NotifyPacketReady(
   // renderer process.
   // If reported size is greater than capacity of the shared memory, we have
   // an error.
-  if (decoded_packet_size && decoded_packet_size <= decoded_packet_size_) {
+  if (decoded_packet_size && decoded_packet_size <= shared_memory_.max_size()) {
     bool ok = push_source_.Write(
         static_cast<char*>(shared_memory_.memory()), decoded_packet_size);
 
@@ -390,8 +402,7 @@ void AudioRendererHost::IPCAudioSource::SubmitPacketRequest_Locked() {
   // 1. No outstanding request
   // 2. There's space for data of the new request.
   if (!outstanding_request_ &&
-      (push_source_.UnProcessedBytes() + decoded_packet_size_ <=
-       buffer_capacity_)) {
+      (push_source_.UnProcessedBytes() <= buffer_capacity_)) {
     outstanding_request_ = true;
 
     // This variable keeps track of the total amount of bytes buffered for
@@ -528,7 +539,6 @@ void AudioRendererHost::OnCreateStream(
       params.sample_rate,
       params.bits_per_sample,
       params.packet_size,
-      params.buffer_capacity,
       low_latency);
 
   // If we have created the source successfully, adds it to the map.
diff --git a/chrome/browser/renderer_host/audio_renderer_host.h b/chrome/browser/renderer_host/audio_renderer_host.h
index 289d258..4cee126 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.h
+++ b/chrome/browser/renderer_host/audio_renderer_host.h
@@ -181,8 +181,7 @@ class AudioRendererHost
         int channels,                        // Number of channels.
         int sample_rate,                     // Sampling frequency/rate.
         char bits_per_sample,                // Number of bits per sample.
-        uint32 decoded_packet_size,          // Number of bytes per packet.
-        uint32 buffer_capacity,              // Number of bytes in the buffer.
+        uint32 packet_size,                  // Size of hardware packet.
         bool low_latency                     // Use low-latency (socket) code
     );
     ~IPCAudioSource();
@@ -237,8 +236,6 @@ class AudioRendererHost
                    int stream_id,               // ID of this source.
                    AudioOutputStream* stream,   // Stream associated.
                    uint32 hardware_packet_size,
-                   uint32 decoded_packet_size,  // Size of shared memory
-                                                // buffer for writing.
                    uint32 buffer_capacity);     // Capacity of transportation
                                                 // buffer.
 
@@ -258,7 +255,6 @@ class AudioRendererHost
     int stream_id_;
     AudioOutputStream* stream_;
     uint32 hardware_packet_size_;
-    uint32 decoded_packet_size_;
     uint32 buffer_capacity_;
 
     State state_;
diff --git a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
index b778dff..537ba9c 100644
--- a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc
@@ -161,7 +161,6 @@ class AudioRendererHostTest : public testing::Test {
             AudioManager::kAudioCDSampleRate,
             16,
             kPacketSize,
-            kBufferCapacity,
             false);
     EXPECT_TRUE(source);
     EXPECT_EQ(kProcessId, source->process_id());
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index 0f6419c..f5e4d72 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -466,11 +466,9 @@ struct ViewHostMsg_Audio_CreateStream_Params {
 
   // Number of bytes per packet. Determines the maximum number of bytes
   // transported for each audio packet request.
+  // A value of 0 means that the audio packet size is selected automatically
+  // by the browser process.
   uint32 packet_size;
-
-  // Maximum number of bytes of audio packets that should be kept in the browser
-  // process.
-  uint32 buffer_capacity;
 };
 
 // This message is used for supporting popup menus on Mac OS X using native
@@ -1791,7 +1789,6 @@ struct ParamTraits<ViewHostMsg_Audio_CreateStream_Params> {
     WriteParam(m, p.sample_rate);
     WriteParam(m, p.bits_per_sample);
     WriteParam(m, p.packet_size);
-    WriteParam(m, p.buffer_capacity);
   }
   static bool Read(const Message* m, void** iter, param_type* p) {
     return
@@ -1799,8 +1796,7 @@ struct ParamTraits<ViewHostMsg_Audio_CreateStream_Params> {
       ReadParam(m, iter, &p->channels) &&
       ReadParam(m, iter, &p->sample_rate) &&
       ReadParam(m, iter, &p->bits_per_sample) &&
-      ReadParam(m, iter, &p->packet_size) &&
-      ReadParam(m, iter, &p->buffer_capacity);
+      ReadParam(m, iter, &p->packet_size);
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(L"<ViewHostMsg_Audio_CreateStream_Params>(");
@@ -1814,8 +1810,6 @@ struct ParamTraits<ViewHostMsg_Audio_CreateStream_Params> {
     l->append(L", ");
     LogParam(p.packet_size, l);
     l->append(L")");
-    LogParam(p.buffer_capacity, l);
-    l->append(L")");
   }
 };
 
diff --git a/chrome/renderer/media/audio_renderer_impl.cc b/chrome/renderer/media/audio_renderer_impl.cc
index 494a702..a1cae17 100644
--- a/chrome/renderer/media/audio_renderer_impl.cc
+++ b/chrome/renderer/media/audio_renderer_impl.cc
@@ -69,15 +69,12 @@ bool AudioRendererImpl::OnInitialize(const media::MediaFormat& media_format) {
     return false;
   }
 
-  // Create the audio output stream in browser process.
+  // Calculate the number of bytes per second using information of the stream.
   bytes_per_second_ = sample_rate_ * channels_ * sample_bits_ / 8;
-  uint32 packet_size = bytes_per_second_ * kMillisecondsPerPacket / 1000;
-  uint32 buffer_capacity = packet_size * kPacketsInBuffer;
-
   io_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &AudioRendererImpl::CreateStreamTask,
-          AudioManager::AUDIO_PCM_LINEAR, channels_, sample_rate_, sample_bits_,
-          packet_size, buffer_capacity));
+          AudioManager::AUDIO_PCM_LINEAR, channels_,
+          sample_rate_, sample_bits_));
   return true;
 }
 
@@ -232,8 +229,8 @@ void AudioRendererImpl::OnVolume(double volume) {
 }
 
 void AudioRendererImpl::CreateStreamTask(
-    AudioManager::Format format, int channels, int sample_rate,
-    int bits_per_sample, uint32 packet_size, uint32 buffer_capacity) {
+    AudioManager::Format format, int channels,
+    int sample_rate, int bits_per_sample) {
   DCHECK(MessageLoop::current() == io_loop_);
 
   AutoLock auto_lock(lock_);
@@ -250,8 +247,7 @@ void AudioRendererImpl::CreateStreamTask(
   params.channels = channels;
   params.sample_rate = sample_rate;
   params.bits_per_sample = bits_per_sample;
-  params.packet_size = packet_size;
-  params.buffer_capacity = buffer_capacity;
+  params.packet_size = 0;
 
   filter_->Send(new ViewHostMsg_CreateAudioStream(0, stream_id_, params,
                                                   false));
diff --git a/chrome/renderer/media/audio_renderer_impl.h b/chrome/renderer/media/audio_renderer_impl.h
index d156e63..8bc6d3c 100644
--- a/chrome/renderer/media/audio_renderer_impl.h
+++ b/chrome/renderer/media/audio_renderer_impl.h
@@ -166,8 +166,7 @@ class AudioRendererImpl : public media::AudioRendererBase,
   // be executed on that thread. They interact with AudioMessageFilter and
   // sends IPC messages on that thread.
   void CreateStreamTask(AudioManager::Format format, int channels,
-                        int sample_rate, int bits_per_sample,
-                        uint32 packet_size, uint32 buffer_capacity);
+                        int sample_rate, int bits_per_sample);
   void PlayTask();
   void PauseTask();
   void SeekTask();
diff --git a/chrome/renderer/pepper_devices.cc b/chrome/renderer/pepper_devices.cc
index 12e34d1..99f1275 100644
--- a/chrome/renderer/pepper_devices.cc
+++ b/chrome/renderer/pepper_devices.cc
@@ -188,9 +188,6 @@ NPError AudioDeviceContext::Initialize(AudioMessageFilter* filter,
   params.packet_size = config->sampleFrameCount * config->outputChannelMap
       * (params.bits_per_sample >> 3);
 
-  // TODO(neb): figure out if this number is grounded in reality
-  params.buffer_capacity = params.packet_size * 3;
-
   stream_id_ = filter_->AddDelegate(this);
   filter->Send(new ViewHostMsg_CreateAudioStream(0, stream_id_, params, true));
   return NPERR_NO_ERROR;