Introduce PushPullFIFO class and remove other FIFOs

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

+// Copyright 2016 The Chromium Authors. All rights reserved.

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

2016 or 2017?

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

A good question. I'll take 2017.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PushPullFIFO_h
+#define PushPullFIFO_h
+
+#include "platform/audio/AudioBus.h"
+#include "public/platform/WebCommon.h"
+#include "wtf/Allocator.h"
+
+namespace blink {
+
+// A configuration data container for PushPullFIFO unit test.
+struct PushPullFIFOStateForTest {
+  const size_t fifoLength;
+  const unsigned numberOfChannels;

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

is "unsigned" a part of WebKit style guide? Chromium usually uses int for these
purposes.

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

Yes, I have seen it in many files in WebKit.

https://cs.chromium.org/search/?q=unsigned+file:%5Esrc/third_party/WebKit/Sou...

WebAudio's convention is using |unsigned| for channel count.

+  const size_t framesAvailable;
+  const size_t indexRead;
+  const size_t indexWrite;
+  const unsigned overflowCount;
+  const unsigned underflowCount;
+};
+
+// PushPullFIFO class is an intermediate audio sample storage between
+// Blink-WebAudio and the renderer. The renderer's hardware callback buffer size
+// varies on the platform, but the WebAudio always operates by 128 frames of
+// render quantum; thus FIFO is needed.

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

I think there have been quite a few comments on what the constraints are on this
fifo.  Can we had some comments here are the assumed contract?  You mean
WebAudio operates on 128 frames, so is that a contract on the push side of the
fifo?  Do you want this FIFO to be usable for "anything"? (I think not.)

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

I agree. I will add few constraints in the comment.

+class BLINK_PLATFORM_EXPORT PushPullFIFO {
+  USING_FAST_MALLOC(PushPullFIFO);
+  WTF_MAKE_NONCOPYABLE(PushPullFIFO);
+
+ public:
+  explicit PushPullFIFO(unsigned numberOfChannels, size_t fifoLength);
+  ~PushPullFIFO();
+
+  // Pushing audio data is done by the WebAudio render-backing thread. The
+  // priority of this thread is lower than the actual audio device thread,
+  // thus the thread can be blocked when the audio device thread is pulling
+  // data from the FIFO.
+  void push(const AudioBus* inputBus);
+
+  // Pulling audio data is done by the audio device thread. In the case of
+  // underflow, the rest will be filled up with zeros (thus glitches). Pulling
+  // an empty FIFO is a valid operation, and the consumer will get a block of
+  // silence.
+  void pull(AudioBus* outputBus, size_t framesRequested);
+
+  size_t framesAvailable() const { return m_framesAvailable; }
+  size_t length() const { return m_fifoLength; }
+  unsigned numberOfChannels() const { return m_fifoBus->numberOfChannels(); }
+  AudioBus* bus() { return m_fifoBus.get(); }

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

const

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

Done.

+
+  // For unit test. Get the current configuration that consists of FIFO length,
+  // number of channels, read/write index position and under/overflow count.
+  const PushPullFIFOStateForTest getStateForTest() const;
+
+ private:
+  RefPtr<AudioBus> m_fifoBus;
+
+  // The size of the FIFO.
+  const size_t m_fifoLength = 0;
+
+  // The number of frames in the FIFO actually available for pulling.
+  size_t m_framesAvailable;
+
+  size_t m_indexRead;
+  size_t m_indexWrite;
+
+  unsigned m_overflowCount;

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

just int? you count up to ~100 only

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

I am using unsigned for the same reason explained above.

+  unsigned m_underflowCount;
+};
+
+}  // namespace blink
+
+#endif  // PushPullFIFO_h