Add WebThread in AudioDestination to support AudioWorkletThread

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

 // Copyright 2017 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.
 
 #ifndef PushPullFIFO_h
 #define PushPullFIFO_h
 
 #include "platform/audio/AudioBus.h"
 #include "public/platform/WebCommon.h"
 #include "wtf/Allocator.h"
+#include "wtf/ThreadingPrimitives.h"
 
 namespace blink {
 
 // A configuration data container for PushPullFIFO unit test.
 struct PushPullFIFOStateForTest {
   const size_t fifoLength;
   const unsigned numberOfChannels;
   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

[o1ka, 2017/03/29 09:05:11]

I understand that this is POC, so it's a side note (which actually deserves a
TODO in unit tests): we'll need multy-threading unit tests.

[hongchan, 2017/03/29 19:39:59]

Of course. This CL will eventually have a unit test.

 // Blink-WebAudio and the renderer. The renderer's hardware callback buffer size
 // varies on the platform, but the WebAudio always renders 128 frames (render
 // quantum, RQ) thus FIFO is needed to handle the general case.

[nhiroki, 2017/03/29 01:54:05]

Can you add comments about thread safety of this class? Which thread can access
getters like framesAvailable()?

[Raymond Toy, 2017/03/29 16:13:13]

Yes, please.  And add appropriate CHECKs in each of the routines to verify that
only the right threads are allowed.

[hongchan, 2017/03/29 19:39:58]

nhiroki@

Can you suggest a way to check which thread I am on here? Both are not the main
thread so we can't use isMainThread() to differentiate. The webaudio rendering
thread is WebThread and also we have the audio device thread. Not sure how to
check within the method.

[nhiroki, 2017/03/30 09:04:41]

Hmm... I cannot think of a good idea. If this class has pointers to threads, we
could use WebThread::isCurrentThread(). Or, we could keep a returned value of
wtf::currentThread() on the first call and then compare it on following calls.

(It'd be nice if we could use base/threading/thread_checker.h in blink...)

 class BLINK_PLATFORM_EXPORT PushPullFIFO {
   USING_FAST_MALLOC(PushPullFIFO);
   WTF_MAKE_NONCOPYABLE(PushPullFIFO);
 
  public:
   // Maximum FIFO length. (512 render quanta)
   static const size_t kMaxFIFOLength;
 
   // |fifoLength| cannot exceed |kMaxFIFOLength|. Otherwise it crashes.
   explicit PushPullFIFO(unsigned numberOfChannels, size_t fifoLength);
   ~PushPullFIFO();
 
   // Pushes the rendered frames by WebAudio engine.
   //  - The |inputBus| length is 128 frames (1 render quantum), fixed.
   //  - In case of overflow (FIFO full while push), the existing frames in FIFO
   //    will be overwritten and |indexRead| will be forcibly moved to
   //    |indexWrite| to avoid reading overwritten frames.
   void push(const AudioBus* inputBus);
 
   // Pulling |framesRequested| by the audio device thread.
   //  - If |framesRequested| is bigger than the length of |outputBus|, it
   //    violates SECURITY_CHECK().
   //  - If |framesRequested| is bigger than FIFO length, it violates
   //    SECURITY_CHECK().
   //  - In case of underflow (FIFO empty while pull), the remaining space in the
   //    requested output bus will be filled with silence. Thus it will fulfill
   //    the request from the consumer without causing error, but with a glitch.
   void pull(AudioBus* outputBus, size_t framesRequested);
 
   size_t framesAvailable() const { return m_framesAvailable; }

[o1ka, 2017/03/29 09:05:11]

This is a non thread-safe method.

[hongchan, 2017/03/29 19:39:58]

Added a lock. Done.

   size_t length() const { return m_fifoLength; }
   unsigned numberOfChannels() const { return m_fifoBus->numberOfChannels(); }
   AudioBus* bus() const { return m_fifoBus.get(); }

[o1ka, 2017/03/29 09:05:11]

Providing direct access to a memory buffer which is accessed from multiple
threads is dangerous even for testing.
If we can't avoid having it, we need to name it ForTesting, and it should be
guarded with the same mutex as other access to the bus.

[hongchan, 2017/03/29 19:39:59]

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:
+  Mutex m_lock;

[nhiroki, 2017/03/29 01:54:05]

What does this lock protect?

[haraken, 2017/03/29 07:56:24]

Same question from me.

[hongchan, 2017/03/29 19:39:59]

See push()/pull() methods. This lock protects:
m_fifoBus
m_indexWrite
m_indexRead
m_framesAvailable

+
   // The size of the FIFO.
   const size_t m_fifoLength = 0;
 
   RefPtr<AudioBus> m_fifoBus;
 
   // 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;
   unsigned m_underflowCount;
 };
 
 }  // namespace blink
 
 #endif  // PushPullFIFO_h