Use audio shifter instead of a fifo for local mediastream playback.

content/renderer/media/webrtc_local_audio_renderer.cc

Index: content/renderer/media/webrtc_local_audio_renderer.cc
diff --git a/content/renderer/media/webrtc_local_audio_renderer.cc b/content/renderer/media/webrtc_local_audio_renderer.cc
index 5fcd58997dabc488bac306ca35867a29dec046fc..d1347e12b32a0f8722458cf4f429e7f05dd7c0ef 100644
--- a/content/renderer/media/webrtc_local_audio_renderer.cc
+++ b/content/renderer/media/webrtc_local_audio_renderer.cc
@@ -16,7 +16,7 @@
 #include "content/renderer/render_frame_impl.h"
 #include "media/audio/audio_output_device.h"
 #include "media/base/audio_bus.h"
-#include "media/base/audio_fifo.h"
+#include "media/base/audio_shifter.h"
 
 namespace content {
 
@@ -36,21 +36,15 @@ int WebRtcLocalAudioRenderer::Render(
   TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::Render");
   base::AutoLock auto_lock(thread_lock_);
 
-  if (!playing_ || !volume_ || !loopback_fifo_) {
+  if (!playing_ || !volume_ || !audio_shifter_) {
     audio_bus->Zero();
     return 0;
   }
 
-  // Provide data by reading from the FIFO if the FIFO contains enough
-  // to fulfill the request.
-  if (loopback_fifo_->frames() >= audio_bus->frames()) {
-    loopback_fifo_->Consume(audio_bus, 0, audio_bus->frames());
-  } else {
-    audio_bus->Zero();
-    // This warning is perfectly safe if it happens for the first audio
-    // frames. It should not happen in a steady-state mode.
-    DVLOG(2) << "loopback FIFO is not full enough yet";
-  }
+  audio_shifter_->Pull(

[henrika (OOO until Aug 14), 2015/01/21 08:54:38]

What effect does the provided time value have here?

[hubbe, 2015/01/23 00:54:56]

The audio shifter tries to provide audio for the given timestamp.
There is a fair amount of comments in audio_shifter.h, but the gist of
it is that Push() can specify exactly when a piece of audio should be played,
and Pull() can specify when it intends to play the pulled audio and audio
shifter will take care of doing the right amount of buffering/stretching.

If Push() uses timestamps that are int he past, audio shifter tries to use the
smallest amount of buffering it can that will still result in smooth playout.

+      audio_bus,
+      base::TimeTicks::Now() -
+      base::TimeDelta::FromMilliseconds(audio_delay_milliseconds));
 
   return audio_bus->frames();
 }
@@ -68,24 +62,16 @@ void WebRtcLocalAudioRenderer::OnData(const media::AudioBus& audio_bus,
   TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::CaptureData");
 
   base::AutoLock auto_lock(thread_lock_);
-  if (!playing_ || !volume_ || !loopback_fifo_)
+  if (!playing_ || !volume_ || !audio_shifter_)
     return;
 
-  // Push captured audio to FIFO so it can be read by a local sink.
-  if ((loopback_fifo_->frames() + audio_bus.frames()) <=
-          loopback_fifo_->max_frames()) {
-    // TODO(miu): Make sure the Render() method accounts for time shifting
-    // appropriately, per the comments for the usage of the
-    // |estimated_capture_time| field found in media_stream_audio_sink.h.
-    // http://crbug.com/437064
-    loopback_fifo_->Push(&audio_bus);
-
-    const base::TimeTicks now = base::TimeTicks::Now();
-    total_render_time_ += now - last_render_time_;
-    last_render_time_ = now;
-  } else {
-    DVLOG(1) << "FIFO is full";
-  }
+  scoped_ptr<media::AudioBus> audio_data(
+      media::AudioBus::Create(audio_bus.channels(), audio_bus.frames()));
+  audio_bus.CopyTo(audio_data.get());
+  audio_shifter_->Push(audio_data.Pass(), estimated_capture_time);
+  const base::TimeTicks now = base::TimeTicks::Now();
+  total_render_time_ += now - last_render_time_;
+  last_render_time_ = now;
 }
 
 void WebRtcLocalAudioRenderer::OnSetFormat(
@@ -152,7 +138,7 @@ void WebRtcLocalAudioRenderer::Stop() {
   {
     base::AutoLock auto_lock(thread_lock_);
     playing_ = false;
-    loopback_fifo_.reset();
+    audio_shifter_.reset();
   }
 
   // Stop the output audio stream, i.e, stop asking for data to render.
@@ -246,7 +232,7 @@ void WebRtcLocalAudioRenderer::MaybeStartSink() {
   {
     // Clear up the old data in the FIFO.
     base::AutoLock auto_lock(thread_lock_);
-    loopback_fifo_->Clear();
+    audio_shifter_->Flush();
   }
 
   if (!sink_params_.IsValid() || !playing_ || !volume_ || sink_started_)
@@ -296,19 +282,15 @@ void WebRtcLocalAudioRenderer::ReconfigureSink(
       source_params_.effects() | implicit_ducking_effect);
 
   {
-    // TODO(henrika): we could add a more dynamic solution here but I prefer
-    // a fixed size combined with bad audio at overflow. The alternative is
-    // that we start to build up latency and that can be more difficult to
-    // detect. Tests have shown that the FIFO never contains more than 2 or 3
-    // audio frames but I have selected a max size of ten buffers just
-    // in case since these tests were performed on a 16 core, 64GB Win 7
-    // machine. We could also add some sort of error notifier in this area if
-    // the FIFO overflows.
-    media::AudioFifo* const new_fifo = new media::AudioFifo(
-        params.channels(), 10 * params.frames_per_buffer());
+    media::AudioShifter* const new_shifter = new media::AudioShifter(

[henrika (OOO until Aug 14), 2015/01/21 08:54:38]

It is not clear to me why you use 2 and 20 here. Care to comment about who they
were selected?

[hubbe, 2015/01/23 00:54:56]

2 was selected because we need at least 1 second for cast.
Note that if push timestamps are in the past, a minimal buffer
will be used regardless of this argument

The 20 millseconds comes from the fact that windows clocks tend to have a 15
millsecond accuracy.

(Comment added)

+        base::TimeDelta::FromSeconds(2),
+        base::TimeDelta::FromMilliseconds(20),
+        base::TimeDelta::FromSeconds(20),
+        source_params_.sample_rate(),
+        params.channels());
 
     base::AutoLock auto_lock(thread_lock_);
-    loopback_fifo_.reset(new_fifo);
+    audio_shifter_.reset(new_shifter);
   }
 
   if (!sink_.get())