Clean up and refactor platform/AudioDestination

third_party/WebKit/Source/platform/audio/AudioDestination.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 11 matching lines @@
  * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "platform/audio/AudioDestination.h"
 
 #include "platform/Histogram.h"
-#include "platform/audio/AudioFIFO.h"
 #include "platform/audio/AudioPullFIFO.h"
 #include "platform/audio/AudioUtilities.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebSecurityOrigin.h"
 #include "wtf/PtrUtil.h"
 #include <memory>
 
 namespace blink {
 
-// Size of the FIFO
-const size_t fifoSize = 8192;
+// FIFO Size; determined arbitrarily.

[Raymond Toy, 2016/12/22 16:59:59]

That's not really true (arbitrarily).  It was an estimate of the largest
callback buffer size that we would ever need. And the FIFO size must be greater
than this.

The current UMA stats indicates that this is, in fact, probably too small. 
There are Android devices out there with a size of 8000 or so.  We might need to
make this larger.  In another CL.

[hongchan, 2016/12/22 17:19:49]

Acknowledged.

[hongchan, 2016/12/22 18:59:45]

Added comments as well. Done.

+const size_t kFIFOSize = 8192;
 
-// Factory method: Chromium-implementation
 std::unique_ptr<AudioDestination> AudioDestination::create(
     AudioIOCallback& callback,
-    const String& inputDeviceId,
-    unsigned numberOfInputChannels,
     unsigned numberOfOutputChannels,
     float sampleRate,
     PassRefPtr<SecurityOrigin> securityOrigin) {
   return WTF::wrapUnique(new AudioDestination(
-      callback, inputDeviceId, numberOfInputChannels, numberOfOutputChannels,
-      sampleRate, std::move(securityOrigin)));
+      callback, numberOfOutputChannels, sampleRate, std::move(securityOrigin)));
 }
 
 AudioDestination::AudioDestination(AudioIOCallback& callback,
-                                   const String& inputDeviceId,
-                                   unsigned numberOfInputChannels,
                                    unsigned numberOfOutputChannels,
                                    float sampleRate,
                                    PassRefPtr<SecurityOrigin> securityOrigin)
-    : m_callback(callback),
-      m_numberOfOutputChannels(numberOfOutputChannels),
-      m_renderBus(AudioBus::create(numberOfOutputChannels,
+    : m_numberOfOutputChannels(numberOfOutputChannels),
+      m_sampleRate(sampleRate),
+      m_isPlaying(false),
+      m_callback(callback),
+      m_outputBus(AudioBus::create(numberOfOutputChannels,
                                    AudioUtilities::kRenderQuantumFrames,
                                    false)),
-      m_sampleRate(sampleRate),
-      m_isPlaying(false),
-      m_framesElapsed(0),
-      m_outputPosition() {

[Raymond Toy, 2016/12/22 18:38:39]

What happened to this initializer?

[hongchan, 2016/12/22 18:59:45]

This is a non-pointer member variable. So we don't need to use the initializer -
it will be implicitly initialized at the construction.

-  // Histogram for audioHardwareBufferSize
-  DEFINE_STATIC_LOCAL(SparseHistogram, hardwareBufferSizeHistogram,
-                      ("WebAudio.AudioDestination.HardwareBufferSize"));
-  // Histogram for the actual callback size used.  Typically, this is the same
-  // as audioHardwareBufferSize, but can be adjusted depending on some
-  // heuristics below.
-  DEFINE_STATIC_LOCAL(SparseHistogram, callbackBufferSizeHistogram,
-                      ("WebAudio.AudioDestination.CallbackBufferSize"));
+      m_framesElapsed(0) {
+  // Calculate the optimum buffer size first.
+  bool isBufferSizeValid = calculateBufferSize();
+  DCHECK(isBufferSizeValid);
 
-  // Use the optimal buffer size recommended by the audio backend.
-  size_t recommendedHardwareBufferSize =
-      Platform::current()->audioHardwareBufferSize();
-  m_callbackBufferSize = recommendedHardwareBufferSize;
+  if (isBufferSizeValid) {
+    // Create WebAudioDevice. blink::WebAudioDevice is designed to support the
+    // local input (e.g. loopback from OS audio system), but Chromium's media
+    // renderer does not support it currently. Thus, we use zero for the number
+    // of input channels.
+    const unsigned numberOfInputChannels = 0;
+    const String inputDeviceId;
+    m_webAudioDevice = WTF::wrapUnique(Platform::current()->createAudioDevice(
+        m_callbackBufferSize, numberOfInputChannels, numberOfOutputChannels,
+        sampleRate, this, inputDeviceId, std::move(securityOrigin)));

[Raymond Toy, 2016/12/22 18:38:39]

Why define these variables that are only used once?  Why not just pass them in
directly to the function?

[hongchan, 2016/12/22 18:59:45]

Sure. I'll do that.

+    DCHECK(m_webAudioDevice);
 
-#if OS(ANDROID)
-  // The optimum low-latency hardware buffer size is usually too small on
-  // Android for WebAudio to render without glitching. So, if it is small, use
-  // a larger size. If it was already large, use the requested size.
-  //
-  // Since WebAudio renders in 128-frame blocks, the small buffer sizes (144
-  // for a Galaxy Nexus), cause significant processing jitter. Sometimes
-  // multiple blocks will processed, but other times will not be since the FIFO
-  // can satisfy the request. By using a larger callbackBufferSize, we smooth
-  // out the jitter.
-  const size_t kSmallBufferSize = 1024;
-  const size_t kDefaultCallbackBufferSize = 2048;
-
-  if (m_callbackBufferSize <= kSmallBufferSize)
-    m_callbackBufferSize = kDefaultCallbackBufferSize;
-
-  LOG(INFO) << "audioHardwareBufferSize = " << recommendedHardwareBufferSize;
-  LOG(INFO) << "callbackBufferSize      = " << m_callbackBufferSize;
-#endif
-
-  // Quick exit if the requested size is too large.
-  DCHECK_LE(m_callbackBufferSize + AudioUtilities::kRenderQuantumFrames,
-            fifoSize);
-  if (m_callbackBufferSize + AudioUtilities::kRenderQuantumFrames > fifoSize)
-    return;
-
-  m_audioDevice = WTF::wrapUnique(Platform::current()->createAudioDevice(
-      m_callbackBufferSize, numberOfInputChannels, numberOfOutputChannels,
-      sampleRate, this, inputDeviceId, std::move(securityOrigin)));
-  ASSERT(m_audioDevice);
-
-  // Record the sizes if we successfully created an output device.
-  hardwareBufferSizeHistogram.sample(recommendedHardwareBufferSize);
-  callbackBufferSizeHistogram.sample(m_callbackBufferSize);
-
-  // Create a FIFO to handle the possibility of the callback size
-  // not being a multiple of the render size. If the FIFO already
-  // contains enough data, the data will be provided directly.
-  // Otherwise, the FIFO will call the provider enough times to
-  // satisfy the request for data.
-  m_fifo =
-      WTF::wrapUnique(new AudioPullFIFO(*this, numberOfOutputChannels, fifoSize,
-                                        AudioUtilities::kRenderQuantumFrames));
+    // Create a FIFO.
+    m_fifo = WTF::wrapUnique(
+        new AudioPullFIFO(*this, numberOfOutputChannels, kFIFOSize,
+                          AudioUtilities::kRenderQuantumFrames));
+  } else {
+    NOTREACHED();

[Raymond Toy, 2016/12/22 18:38:39]

Is this really necessary? You already have a DCHECK that the size is valid.

[hongchan, 2016/12/22 18:59:45]

I can remove it, however I am not sure what we should do if the buffer size
calculation fails.

+  }
 }
 
 AudioDestination::~AudioDestination() {
   stop();
 }
 
-void AudioDestination::start() {
-  if (!m_isPlaying && m_audioDevice) {
-    m_audioDevice->start();
-    m_isPlaying = true;
-  }
-}
-
-void AudioDestination::stop() {
-  if (m_isPlaying && m_audioDevice) {
-    m_audioDevice->stop();
-    m_isPlaying = false;
-  }
-}
-
-float AudioDestination::hardwareSampleRate() {
-  return static_cast<float>(Platform::current()->audioHardwareSampleRate());
-}
-
-unsigned long AudioDestination::maxChannelCount() {
-  return static_cast<float>(Platform::current()->audioHardwareOutputChannels());
-}
-
-void AudioDestination::render(const WebVector<float*>& audioData,
+void AudioDestination::render(const WebVector<float*>& destinationData,
                               size_t numberOfFrames,
                               double delay,
                               double delayTimestamp,
                               size_t priorFramesSkipped) {
-  bool isNumberOfChannelsGood = audioData.size() == m_numberOfOutputChannels;
-  if (!isNumberOfChannelsGood) {
-    ASSERT_NOT_REACHED();
+  DCHECK_EQ(destinationData.size(), m_numberOfOutputChannels);
+  if (destinationData.size() != m_numberOfOutputChannels)
     return;
-  }
 
-  bool isBufferSizeGood = numberOfFrames == m_callbackBufferSize;
-  if (!isBufferSizeGood) {
-    ASSERT_NOT_REACHED();
+  DCHECK_EQ(numberOfFrames, m_callbackBufferSize);
+  if (numberOfFrames != m_callbackBufferSize)
     return;
-  }
 
   m_framesElapsed -= std::min(m_framesElapsed, priorFramesSkipped);
   double outputPosition =
       m_framesElapsed / static_cast<double>(m_sampleRate) - delay;
   m_outputPosition.position = outputPosition;
   m_outputPosition.timestamp = delayTimestamp;
   m_outputPositionReceivedTimestamp = base::TimeTicks::Now();
 
+  // Associate the destination data array with the output bus then fill the
+  // FIFO.
   for (unsigned i = 0; i < m_numberOfOutputChannels; ++i)
-    m_renderBus->setChannelMemory(i, audioData[i], numberOfFrames);
-
-  m_fifo->consume(m_renderBus.get(), numberOfFrames);
+    m_outputBus->setChannelMemory(i, destinationData[i], numberOfFrames);
+  m_fifo->consume(m_outputBus.get(), numberOfFrames);
 
   m_framesElapsed += numberOfFrames;
 }
 
-void AudioDestination::provideInput(AudioBus* bus, size_t framesToProcess) {
+void AudioDestination::provideInput(AudioBus* outputBus,
+                                    size_t framesToProcess) {
   AudioIOPosition outputPosition = m_outputPosition;
 
-  // If platfrom buffer is more than two times longer than |framesToProcess|
+  // If platform buffer is more than two times longer than |framesToProcess|
   // we do not want output position to get stuck so we promote it
   // using the elapsed time from the moment it was initially obtained.
   if (m_callbackBufferSize > framesToProcess * 2) {
     double delta = (base::TimeTicks::Now() - m_outputPositionReceivedTimestamp)
                        .InSecondsF();
     outputPosition.position += delta;
     outputPosition.timestamp += delta;
   }
 
   // Some implementations give only rough estimation of |delay| so
   // we might have negative estimation |outputPosition| value.
   if (outputPosition.position < 0.0)
     outputPosition.position = 0.0;
 
-  m_callback.render(nullptr, bus, framesToProcess, outputPosition);
+  // To fill the FIFO, start the render call chain of the destination node.
+  m_callback.render(nullptr, outputBus, framesToProcess, outputPosition);
+}
+
+void AudioDestination::start() {
+  if (m_webAudioDevice && !m_isPlaying) {
+    m_webAudioDevice->start();
+    m_isPlaying = true;
+  }
+}
+
+void AudioDestination::stop() {
+  if (m_webAudioDevice && m_isPlaying) {
+    m_webAudioDevice->stop();
+    m_isPlaying = false;
+  }
+}
+
+size_t AudioDestination::hardwareBufferSize() {
+  return Platform::current()->audioHardwareBufferSize();
+}
+
+float AudioDestination::hardwareSampleRate() {
+  return static_cast<float>(Platform::current()->audioHardwareSampleRate());
+}
+
+unsigned long AudioDestination::maxChannelCount() {
+  return static_cast<unsigned long>(
+      Platform::current()->audioHardwareOutputChannels());
+}
+
+bool AudioDestination::calculateBufferSize() {
+  // Use the optimal buffer size recommended by the audio backend.
+  size_t recommendedHardwareBufferSize = hardwareBufferSize();
+  m_callbackBufferSize = recommendedHardwareBufferSize;
+
+#if OS(ANDROID)
+  // The optimum low-latency hardware buffer size is usually too small on
+  // Android for WebAudio to render without glitching. So, if it is small, use a
+  // larger size. If it was already large, use the requested size.
+  //
+  // Since WebAudio renders in 128-frame blocks, the small buffer sizes (144 for
+  // a Galaxy Nexus), cause significant processing jitter. Sometimes multiple
+  // blocks will processed, but other times will not be since the FIFO can
+  // satisfy the request. By using a larger callbackBufferSize, we smooth out
+  // the jitter.
+  const size_t kSmallBufferSize = 1024;
+  const size_t kDefaultCallbackBufferSize = 2048;
+
+  if (m_callbackBufferSize <= kSmallBufferSize)
+    m_callbackBufferSize = kDefaultCallbackBufferSize;
+
+  LOG(INFO) << "audioHardwareBufferSize = " << recommendedHardwareBufferSize;
+  LOG(INFO) << "callbackBufferSize      = " << m_callbackBufferSize;
+#endif
+
+  // Histogram for audioHardwareBufferSize
+  DEFINE_STATIC_LOCAL(SparseHistogram, hardwareBufferSizeHistogram,
+                      ("WebAudio.AudioDestination.HardwareBufferSize"));
+
+  // Histogram for the actual callback size used.  Typically, this is the same
+  // as audioHardwareBufferSize, but can be adjusted depending on some
+  // heuristics below.
+  DEFINE_STATIC_LOCAL(SparseHistogram, callbackBufferSizeHistogram,
+                      ("WebAudio.AudioDestination.CallbackBufferSize"));
+
+  // Record the sizes if we successfully created an output device.
+  hardwareBufferSizeHistogram.sample(recommendedHardwareBufferSize);
+  callbackBufferSizeHistogram.sample(m_callbackBufferSize);
+
+  // Quick exit if the requested size is too large.
+  DCHECK_LE(m_callbackBufferSize + AudioUtilities::kRenderQuantumFrames,
+            kFIFOSize);
+  if (m_callbackBufferSize + AudioUtilities::kRenderQuantumFrames > kFIFOSize)
+    return false;
+
+  return true;
 }
 
 }  // namespace blink