Change audio renderer to communicate with host using low latency codepath.

content/renderer/media/audio_renderer_impl.cc

Index: content/renderer/media/audio_renderer_impl.cc
===================================================================
--- content/renderer/media/audio_renderer_impl.cc	(revision 90349)
+++ content/renderer/media/audio_renderer_impl.cc	(working copy)
@@ -6,24 +6,20 @@
 
 #include <math.h>
 
+#include "base/command_line.h"
+#include "content/common/content_switches.h"
 #include "content/common/media/audio_messages.h"
 #include "content/renderer/render_thread.h"
 #include "content/renderer/render_view.h"
+#include "media/audio/audio_output_controller.h"
 #include "media/base/filter_host.h"
 
-namespace {
+// Static variable that says what code path we are using -- low or high
+// latency. Made separate variable so we don't have to go to command line
+// for every DCHECK().
+AudioRendererImpl::LatencyType AudioRendererImpl::latency_type_ =
+    AudioRendererImpl::kUninitializedLatency;
 
-// We will try to fill 200 ms worth of audio samples in each packet. A round
-// trip latency for IPC messages are typically 10 ms, this should give us
-// plenty of time to avoid clicks.
-const int kMillisecondsPerPacket = 200;
-
-// We have at most 3 packets in browser, i.e. 600 ms. This is a reasonable
-// amount to avoid clicks.
-const int kPacketsInBuffer = 3;
-
-}  // namespace
-
 AudioRendererImpl::AudioRendererImpl(AudioMessageFilter* filter)
     : AudioRendererBase(),
       bytes_per_second_(0),
@@ -37,6 +33,17 @@
       prerolling_(false),
       preroll_bytes_(0) {
   DCHECK(io_loop_);
+  // Figure out if we are planning to use high or low latency code path.
+  // We are initializing only one variable and double initialization is Ok,
+  // so there would not be any issues caused by CPU memory model.
+  if (latency_type_ == kUninitializedLatency) {
+    if (CommandLine::ForCurrentProcess()->HasSwitch(

[scherkus (not reviewing), 2011/06/24 21:09:26]

instead of checking the command line here, do the check in
content/renderer/render_view.cc (see RenderView::createMediaPlayer)

we can also get rid of the static var and make it member var

[enal1, 2011/06/24 22:01:09]

(1) I thought about moving check up the call tree, and decided against it. Upper
level code should not go into details exactly which protocols audio renderer
implementation is using. Other than audio renderer the only code that should be
interested is hoisting code that implements other half of the protocol, and I am
not touching it.

(2) I thought about member variable, too, but decided to use global -- I don't
see why we want to make only some instances low latency. Thank for pointing,
though -- I made method that sets mode static, and it should be called before
any instance is created. This way tests can be sure that all instances are the
"right" ones.

+        switches::kLowLatencyAudio)) {
+      latency_type_ = kLowLatency;
+    } else {
+      latency_type_ = kHighLatency;
+    }
+  }
 }
 
 AudioRendererImpl::~AudioRendererImpl() {
@@ -71,8 +78,21 @@
   // task to clean up.
   io_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &AudioRendererImpl::DestroyTask));
+
+  if (audio_thread_.get()) {
+    socket_->Close();
+    audio_thread_->Join();
+  }
 }
 
+void AudioRendererImpl::NotifyDataAvailableIfNecessary() {
+  if (latency_type_ == kHighLatency) {
+    // Post a task to render thread to notify a packet reception.
+    io_loop_->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &AudioRendererImpl::NotifyPacketReadyTask));
+  }
+}
+
 void AudioRendererImpl::ConsumeAudioSamples(
     scoped_refptr<media::Buffer> buffer_in) {
   base::AutoLock auto_lock(lock_);
@@ -84,9 +104,7 @@
   // Use the base class to queue the buffer.
   AudioRendererBase::ConsumeAudioSamples(buffer_in);
 
-  // Post a task to render thread to notify a packet reception.
-  io_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererImpl::NotifyPacketReadyTask));
+  NotifyDataAvailableIfNecessary();
 }
 
 void AudioRendererImpl::SetPlaybackRate(float rate) {
@@ -115,9 +133,7 @@
   // If we are playing, give a kick to try fulfilling the packet request as
   // the previous packet request may be stalled by a pause.
   if (rate > 0.0f) {
-    io_loop_->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(this, &AudioRendererImpl::NotifyPacketReadyTask));
+    NotifyDataAvailableIfNecessary();
   }
 }
 
@@ -170,6 +186,7 @@
 void AudioRendererImpl::OnCreated(base::SharedMemoryHandle handle,
                                   uint32 length) {
   DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK(latency_type_ == kHighLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

try to use DCHECK_EQ when possible

for pointers it's not always as interesting, but for values it can be nice for
debugging

[enal1, 2011/06/24 22:01:09]

Done.

 
   base::AutoLock auto_lock(lock_);
   if (stopped_)
@@ -180,15 +197,54 @@
   shared_memory_size_ = length;
 }
 
-void AudioRendererImpl::OnLowLatencyCreated(base::SharedMemoryHandle,
-                                            base::SyncSocket::Handle, uint32) {
-  // AudioRenderer should not have a low-latency audio channel.
-  NOTREACHED();
+void AudioRendererImpl::CreateSocket(base::SyncSocket::Handle socket_handle) {
+  DCHECK(latency_type_ == kLowLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

DCHECK_EQ

[enal1, 2011/06/24 22:01:09]

Done.

+#if defined(OS_WIN)
+  DCHECK(socket_handle);
+#else
+  DCHECK_GE(socket_handle, 0);
+#endif
+  socket_.reset(new base::SyncSocket(socket_handle));
 }
 
+void AudioRendererImpl::CreateAudioThread() {
+  DCHECK(latency_type_ == kLowLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

DCHECK_EQ

[enal1, 2011/06/24 22:01:09]

Done.

+  audio_thread_.reset(
+      new base::DelegateSimpleThread(this, "renderer_audio_thread"));
+  audio_thread_->Start();
+}
+
+void AudioRendererImpl::OnLowLatencyCreated(
+    base::SharedMemoryHandle handle,
+    base::SyncSocket::Handle socket_handle,
+    uint32 length) {
+

[scherkus (not reviewing), 2011/06/24 21:09:26]

nit: remove blank line

[enal1, 2011/06/24 22:01:09]

Done.

+  DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK(latency_type_ == kLowLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

DCHECK_EQ

[enal1, 2011/06/24 22:01:09]

Done.

+#if defined(OS_WIN)
+  DCHECK(handle);
+  DCHECK(socket_handle);
+#else
+  DCHECK_GE(handle.fd, 0);
+  DCHECK_GE(socket_handle, 0);
+#endif
+  DCHECK(length);
+
+  base::AutoLock auto_lock(lock_);
+  if (stopped_)
+    return;
+
+  shared_memory_.reset(new base::SharedMemory(handle, false));
+  shared_memory_->Map(length);
+  shared_memory_size_ = length;
+
+  CreateSocket(socket_handle);
+  CreateAudioThread();
+}
+
 void AudioRendererImpl::OnRequestPacket(AudioBuffersState buffers_state) {
   DCHECK(MessageLoop::current() == io_loop_);
-
+  DCHECK(latency_type_ == kHighLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

DCHECK_EQ

[enal1, 2011/06/24 22:01:09]

Done.

   {
     base::AutoLock auto_lock(lock_);
     DCHECK(!pending_request_);
@@ -247,8 +303,10 @@
   // Let the browser choose packet size.
   params_to_send.samples_per_packet = 0;
 
-  filter_->Send(new AudioHostMsg_CreateStream(
-      0, stream_id_, params_to_send, false));
+  filter_->Send(new AudioHostMsg_CreateStream(0,
+                                              stream_id_,
+                                              params_to_send,
+                                              latency_type_ == kLowLatency));
 }
 
 void AudioRendererImpl::PlayTask() {
@@ -293,6 +351,7 @@
 
 void AudioRendererImpl::NotifyPacketReadyTask() {
   DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK(latency_type_ == kHighLatency);

[scherkus (not reviewing), 2011/06/24 21:09:26]

DCHECK_EQ

[enal1, 2011/06/24 22:01:09]

Done.

 
   base::AutoLock auto_lock(lock_);
   if (stopped_)
@@ -346,3 +405,40 @@
   stopped_ = true;
   DestroyTask();
 }
+
+// Our audio thread runs here. We receive requests for more data and send it
+// on this thread.
+void AudioRendererImpl::Run() {
+#if !defined(OS_LINUX)  // SetThreadPriority() not implemented on Linux.
+  audio_thread_->SetThreadPriority(base::kThreadPriority_RealtimeAudio);

[scherkus (not reviewing), 2011/06/24 21:09:26]

it's ok to remove the DCHECK and have it spew a NOTIMPLEMENTED() for now (it
does a LOG(ERROR))

[enal1, 2011/06/24 22:01:09]

It is exactly what it is doing, I just did not like that extra output... Removed
#ifdef here.

+#endif
+
+  int bytes;
+  while (sizeof(bytes) == socket_->Receive(&bytes, sizeof(bytes))) {
+    if (bytes == media::AudioOutputController::kPauseMark) {
+      continue;
+    } else if (bytes < 0) {
+      break;
+    }
+    base::AutoLock auto_lock(lock_);
+    if (stopped_) {
+      break;
+    }
+    float playback_rate = GetPlaybackRate();
+    if (playback_rate > 0.0f) {

[scherkus (not reviewing), 2011/06/24 21:09:26]

nit: try to collapse nested ifs, so check for opposite condition and continue

[enal1, 2011/06/24 22:01:09]

Done, though personally I strongly prefer nested ifs: "continue" passes control
somewhere outside normal control flow. E.g. you cannot just move piece of code
with "continue" into nested loop...

+      DCHECK(shared_memory_.get());
+
+      base::TimeDelta request_delay = ConvertToDuration(bytes);
+      // We need to adjust the delay according to playback rate.
+      if (playback_rate != 1.0f) {
+        request_delay = base::TimeDelta::FromMicroseconds(
+            static_cast<int64>(ceil(request_delay.InMicroseconds() *
+                                    playback_rate)));
+      }
+      FillBuffer(static_cast<uint8*>(shared_memory_->memory()),
+                 shared_memory_size_,
+                 request_delay,
+                 true  /* buffers empty */);
+    }
+  }
+}