Introduce PushPullFIFO class and remove other FIFOs

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

+// Copyright 2016 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 "platform/audio/PushPullFIFO.h"
+
+#include "wtf/PtrUtil.h"
+#include <memory>
+
+namespace blink {
+
+// Suppress the warning log if over/underflow happens more than 100 times.
+const int kLogCapOverUnderflow = 100;
+
+PushPullFIFO::PushPullFIFO(unsigned numberOfChannels, size_t fifoLength)
+    : m_fifoBus(AudioBus::create(numberOfChannels, fifoLength)),
+      m_fifoLength(fifoLength),
+      m_framesAvailable(0),
+      m_indexRead(0),
+      m_indexWrite(0),
+      m_overflowCount(0),
+      m_underflowCount(0) {}
+
+PushPullFIFO::~PushPullFIFO() {}
+
+// Push the data from |inputBus| to FIFO. The size of push is determined by
+// the length of |inputBus|.
+void PushPullFIFO::push(const AudioBus* inputBus) {
+  DCHECK(inputBus);
+  if (!inputBus)
+    return;
+
+  const size_t inputBusLength = inputBus->length();
+  const size_t remainder = m_fifoLength - m_indexWrite;
+  SECURITY_CHECK(inputBusLength <= m_fifoLength);
+  SECURITY_CHECK(m_indexWrite < m_fifoLength);
+
+  for (unsigned i = 0; i < m_fifoBus->numberOfChannels(); ++i) {
+    float* fifoBusChannel = m_fifoBus->channel(i)->mutableData();
+    const float* inputBusChannel = inputBus->channel(i)->data();
+    if (remainder >= inputBusLength) {
+      // The remainder is big enough for the input data.
+      memcpy(fifoBusChannel + m_indexWrite, inputBusChannel,
+             inputBusLength * sizeof(*fifoBusChannel));
+    } else {
+      // The input data overflows the remainder size. Wrap around the index.
+      memcpy(fifoBusChannel + m_indexWrite, inputBusChannel,
+             remainder * sizeof(*fifoBusChannel));
+      memcpy(fifoBusChannel, inputBusChannel + remainder,
+             (inputBusLength - remainder) * sizeof(*fifoBusChannel));
+    }
+  }
+
+  // Update the write index; wrap it around if necessary.
+  m_indexWrite = (m_indexWrite + inputBusLength) % m_fifoLength;
+
+  // In case of overflow, move the read index to the updated write index.
+  if (inputBusLength > m_fifoLength - m_framesAvailable) {
+    m_indexRead = m_indexWrite;
+    if (++m_overflowCount < kLogCapOverUnderflow) {
+      LOG(WARNING) << "PushPullFIFO::overflow while pushing ("

[Raymond Toy, 2017/01/27 17:39:44]

Grammar:  "PushPullFIFO: overflow..."  The message currently looks like its
you're talking about the function overflow.

[hongchan, 2017/01/27 22:39:14]

Done.

+                   << "overflowCount=" << m_overflowCount
+                   << ", availalbeFrames=" << m_framesAvailable

[Raymond Toy, 2017/01/27 17:39:43]

Typo.

[hongchan, 2017/01/27 22:39:14]

Done.

+                   << ", inputFrames=" << inputBusLength
+                   << ", fifoLength=" << m_fifoLength << ")";
+    }
+  }
+
+  // Update the number of frames available in FIFO.
+  m_framesAvailable += inputBusLength;

[o1ka, 2017/01/27 13:47:54]

m_framesAvailable cannot be more than m_fifoLength.
But if you got to l.59, it will be.

[hongchan, 2017/01/27 22:39:13]

You're right. I am reverting it with std::min().

+}
+
+// Pull the data out of FIFO to |outputBus|. If the remaining frames in the FIFO
+// is less than the frames to pull, provides the remaining frames plus the
+// silence.
+void PushPullFIFO::pull(AudioBus* outputBus, size_t framesRequested) {
+  DCHECK(outputBus);
+  if (!outputBus)
+    return;

[o1ka, 2017/01/27 13:47:54]

Can we avoid silent failures by using CHECK()?

[Raymond Toy, 2017/01/27 17:39:44]

What is the intent here?  Because of the checks below, outpuBus really needs to
exist and have a size greater than framesRequested.  I agree with olka.

[hongchan, 2017/01/27 22:39:13]

Yes. I was simply following WebAudio's convention and will be happy to remove
this pattern. I will use CHECK() and crash when it fails.

+
+  // TODO(hongchan): add a tryLock here for this operation when two threads are
+  // introduced. Pulling operation has a higher priority to ensure the audio
+  // device callback to be uninterrupted.
+
+  const size_t remainder = m_fifoLength - m_indexRead;
+  const size_t framesToFill = std::min(m_framesAvailable, framesRequested);
+  SECURITY_CHECK(framesRequested <= outputBus->length());
+  SECURITY_CHECK(framesRequested <= m_fifoLength);
+  SECURITY_CHECK(m_indexRead < m_fifoLength);

[Raymond Toy, 2017/01/27 17:39:44]

Move these to the beginning of the function.

[hongchan, 2017/01/27 22:39:14]

Done.

+
+  for (unsigned i = 0; i < m_fifoBus->numberOfChannels(); ++i) {
+    const float* fifoBusChannel = m_fifoBus->channel(i)->data();
+    float* outputBusChannel = outputBus->channel(i)->mutableData();
+
+    // Fill up the output bus with the available frames first.
+    if (remainder >= framesToFill) {
+      // The remainder is big enough for the frames to pull.
+      memcpy(outputBusChannel, fifoBusChannel + m_indexRead,
+             framesToFill * sizeof(*fifoBusChannel));

[o1ka, 2017/01/27 13:47:54]

What if framesToFill * sizeof(*fifoBusChannel) overflows?
Switch to safe math?
(same in other places)

[hongchan, 2017/01/27 22:39:14]

With the newly introduced max FIFO length, the overflow cannot happen.

+    } else {
+      // The frames to pull is bigger than the remainder size.
+      // Wrap around the index.
+      memcpy(outputBusChannel, fifoBusChannel + m_indexRead,
+             remainder * sizeof(*fifoBusChannel));
+      memcpy(outputBusChannel + remainder, fifoBusChannel,
+             (framesToFill - remainder) * sizeof(*fifoBusChannel));
+    }
+
+    // The frames available was not enough to fulfill the requested frames. Fill
+    // the rest of the channel with silence.
+    if (framesRequested > framesToFill) {
+      memset(outputBusChannel + framesToFill, 0,
+             (framesRequested - framesToFill) * sizeof(*outputBusChannel));
+    }
+  }
+
+  // Update the read index; wrap it around if necessary.
+  m_indexRead = (m_indexRead + framesToFill) % m_fifoLength;

[o1ka, 2017/01/27 13:47:54]

What is addition overflows?

[hongchan, 2017/01/27 22:39:13]

With the newly introduced max FIFO length, the overflow cannot happen.

+
+  // In case of underflow, move the write index to the updated read index.
+  if (framesRequested > framesToFill) {
+    m_indexWrite = m_indexRead;
+    if (m_underflowCount++ < kLogCapOverUnderflow) {
+      LOG(WARNING) << "PushPullFIFO::underflow while pulling ("

[Raymond Toy, 2017/01/27 17:39:44]

Grammar: "PushPullFIFO: overflow while...". Looks like you're talking about the
function underflow.

[hongchan, 2017/01/27 22:39:13]

Done.

+                   << "underflowCount=" << m_underflowCount
+                   << ", availableFrames=" << m_framesAvailable
+                   << ", requestedFrames=" << framesRequested
+                   << ", fifoLength=" << m_fifoLength << ")";
+    }
+  }
+
+  // Update the number of frames in FIFO.
+  m_framesAvailable -= framesToFill;
+}
+
+const PushPullFIFOStateForTest PushPullFIFO::getStateForTest() const {
+  return {length(),     numberOfChannels(), framesAvailable(), m_indexRead,

[o1ka, 2017/01/27 13:47:54]

Does this spacing comply with style guide?

[hongchan, 2017/01/27 22:39:14]

I checked twice; this is the result of |git cl format|. I really hate what it
did, so I can indent this myself and ignore the warning.

+          m_indexWrite, m_overflowCount,    m_underflowCount};
+}
+
+}  // namespace blink