refactor AudioInputDevice to remove race condition.

content/renderer/media/audio_input_device.h

 // Copyright (c) 2011 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.
 
 // Low-latency audio capturing unit utilizing audio input stream provided
 // by browser process through IPC.
 //
 // Relationship of classes.
 //
 //  AudioInputController                 AudioInputDevice
@@ skipping 35 matching lines @@
 #ifndef CONTENT_RENDERER_MEDIA_AUDIO_INPUT_DEVICE_H_
 #define CONTENT_RENDERER_MEDIA_AUDIO_INPUT_DEVICE_H_
 #pragma once
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "base/threading/simple_thread.h"
+#include "base/threading/thread.h"
 #include "content/renderer/media/audio_input_message_filter.h"
 
-struct AudioParameters;
-
 // TODO(henrika): This class is based on the AudioDevice class and it has
 // many components in common. Investigate potential for re-factoring.
 class AudioInputDevice
     : public AudioInputMessageFilter::Delegate,
       public base::DelegateSimpleThread::Delegate,
       public base::RefCountedThreadSafe<AudioInputDevice> {
  public:
   class CaptureCallback {
    public:
     virtual void Capture(const std::vector<float*>& audio_data,
@@ skipping 28 matching lines @@
   size_t buffer_size() const { return buffer_size_; }
 
   // Methods called on IO thread ----------------------------------------------
   // AudioInputMessageFilter::Delegate impl., called by AudioInputMessageFilter
   virtual void OnLowLatencyCreated(base::SharedMemoryHandle handle,
                                    base::SyncSocket::Handle socket_handle,
                                    uint32 length);
   virtual void OnVolume(double volume);
 
  private:
-  // Methods called on IO thread ----------------------------------------------
-  // The following methods are tasks posted on the IO thread that needs to
-  // be executed on that thread. They interact with AudioInputMessageFilter and
-  // sends IPC messages on that thread.
-  void InitializeOnIOThread(const AudioParameters& params);
-  void StartOnIOThread();
-  void ShutDownOnIOThread();
-  void SetVolumeOnIOThread(double volume);
+  // Methods called on capture thread-------------------------------------------
+  // The following methods are tasks posted on the capture thread that needs to
+  // be executed on that thread.

[henrika_dont_use, 2011/07/30 15:50:59]

Perhaps mention that this thread is the main working thread.

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  void StartOnCaptureThread();
+  void StartRecordingOnCaptureThread(base::SharedMemoryHandle handle,
+                                     base::SyncSocket::Handle socket_handle,
+                                     uint32 length);
+  void ShutDownOnCaptureThread();
+  void SetVolumeOnCaptureThread(double volume);
 
   void Send(IPC::Message* message);
 
   // Method called on the audio thread ----------------------------------------
   // Calls the client's callback for capturing audio.
   void FireCaptureCallback();
 
   // DelegateSimpleThread::Delegate implementation.
   virtual void Run();
 
+  // Add/Remove delegate on IO thread.

[henrika_dont_use, 2011/07/30 15:50:59]

Use same comment style as above, i.e., --------------------?

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  void AddDelegateOnIOThread();
+  void RemoveDelegateOnIOThread(int32 stream_id);
+
   // Format
   size_t buffer_size_;  // in sample-frames
   int channels_;
   int bits_per_sample_;
   double sample_rate_;
 
   CaptureCallback* callback_;
 
   // The client callback receives captured audio here.
   std::vector<float*> audio_data_;
 
   // The client stores the last reported audio delay in this member.
   // The delay shall reflect the amount of audio which still resides in
   // the input buffer, i.e., the expected audio input delay.
   int audio_delay_milliseconds_;
 
   // The current volume scaling [0.0, 1.0] of the audio stream.
   double volume_;
 
+  // The working thread for AudioInputDevice.
+  base::Thread capture_thread_;
+  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;

[henrika_dont_use, 2011/07/30 15:50:59]

Sorry, but I actually don't understand why a proxy is needed. What is the
advantage? Why not access the thread message loop directly?

[wjia(left Chromium), 2011/08/01 22:40:13]

MessageLoop and MessageLoopProxy are very similar, except that MessageLoopProxy
can outlive the target thread and is a safe way to hold on to message loop
pointers and leads to far less brittle code.

+
+  // These 2 WaitableEvents are added to translate async operation (between

[henrika_dont_use, 2011/07/30 15:50:59]

two

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // AudioInputDevice and AudioInputMessageFilter) to sync operation (between
+  // AudioInputDevice and ADM). The main reason is ADM can delete

[henrika_dont_use, 2011/07/30 15:50:59]

that the

[xians, 2011/08/01 11:41:23]

AudioInputDevice and client). The main reason is client can ..

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // AudioInputDevice anytime after Stop() is called. It's highly recommended

[henrika_dont_use, 2011/07/30 15:50:59]

Hmm, we state that events are added to emulate sync operation and then we say
that async is highly recommended. Can this comment/section be more clear? Also,
today Start is async and Stop is sync. Does this section deal with Stop
only?Feels like the complete comment section can be even more clear.

[wjia(left Chromium), 2011/08/01 22:40:13]

The recommendation is to get rid of emulating sync operation. Actually, Start
always returns "true" now, which is not quite right. This is just an
intermediate stage before changing API between AudioInputDevice and ADM.
Eventually, ADM will be an EventHanlder of AudioInputDevice which receives
status update events from AudioInputDevice.

+  // to use async API between ADM and AudioInputDevice.

[xians, 2011/08/01 11:41:23]

client?

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // WaitableEvent used to synchronize delegate operation since delegate
+  // needs to be added/removed on IO thread.
+  base::WaitableEvent delegate_event_;
+  // WaitableEvent used to synchronize Stop() function call to ensure it's safe
+  // to destruct AudioInputDevice.
+  base::WaitableEvent stop_event_;
+
   // Callbacks for capturing audio occur on this thread.
   scoped_ptr<base::DelegateSimpleThread> audio_thread_;
 
   // IPC message stuff.
   base::SharedMemory* shared_memory() { return shared_memory_.get(); }
   base::SyncSocket* socket() { return socket_.get(); }
   void* shared_memory_data() { return shared_memory()->memory(); }
 
   // Cached audio input message filter (lives on the main render thread).
   scoped_refptr<AudioInputMessageFilter> filter_;
 
   // Our stream ID on the message filter. Only modified on the IO thread.
   int32 stream_id_;
 
   scoped_ptr<base::SharedMemory> shared_memory_;
   scoped_ptr<base::SyncSocket> socket_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(AudioInputDevice);
 };
 
 #endif  // CONTENT_RENDERER_MEDIA_AUDIO_INPUT_DEVICE_H_