MediaStream audio: Refactor 3 separate "glue" implementations into one.

content/renderer/media/media_stream_audio_source.h

Index: content/renderer/media/media_stream_audio_source.h
diff --git a/content/renderer/media/media_stream_audio_source.h b/content/renderer/media/media_stream_audio_source.h
index 6d7da5ccf003b6b1c54accf24d9c2d857f8a32de..d259b9ce113b7a82acc60a2849ff6a04a1b7c833 100644
--- a/content/renderer/media/media_stream_audio_source.h
+++ b/content/renderer/media/media_stream_audio_source.h
@@ -5,94 +5,162 @@
 #ifndef CONTENT_RENDERER_MEDIA_MEDIA_STREAM_AUDIO_SOURCE_H_
 #define CONTENT_RENDERER_MEDIA_MEDIA_STREAM_AUDIO_SOURCE_H_
 
+#include <string>
+#include <vector>
+
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/synchronization/lock.h"
 #include "content/common/content_export.h"
 #include "content/renderer/media/media_stream_source.h"
-#include "content/renderer/media/webaudio_capturer_source.h"
-#include "content/renderer/media/webrtc/peer_connection_dependency_factory.h"
-#include "content/renderer/media/webrtc_audio_capturer.h"
-#include "third_party/webrtc/api/mediastreaminterface.h"
+#include "media/audio/audio_parameters.h"
+#include "third_party/WebKit/public/platform/WebMediaStreamSource.h"
+#include "third_party/WebKit/public/platform/WebMediaStreamTrack.h"
 
 namespace content {
 
 class MediaStreamAudioTrack;
 
-// TODO(miu): In a soon-upcoming set of refactoring changes, this class will
-// become a base class for managing tracks (part of what WebRtcAudioCapturer
-// does today).  Then, the rest of WebRtcAudioCapturer will be rolled into a
-// subclass.   http://crbug.com/577874
+// Represents a source of audio, and manages the delivery of audio data between
+// the source implementation and one or more MediaStreamAudioTracks.  This is
+// a base class providing all the necessary functionality to connect tracks and
+// have audio data delivered to them.  Subclasses provide the actual audio
+// source implementation (e.g., media::AudioCapturerSource), and should
+// implement the DoStopSource() and EnsureSourceIsStarted() methods, and call

[perkj_chrome, 2016/04/08 14:05:41]

How does DoStopSource and EnsureSourceStoped relate? Looks like this should be
EnsureSourceIsStopped()

[miu, 2016/04/19 00:40:22]

Whoops.  I needed to update this comment.  The intention is for subclasses to
only worry about implementing EnsureSourceIsStarted() and
EnsureSourceIsStopped().  I also declared DoStopSource() as "final" to make this
clearer.

+// DeliverDataToTracks().
+//
+// This base class can be instantiated, to be used as a place-holder or a "null"
+// source of audio.  This can be useful for unit testing, wherever a mock is
+// needed, and/or calls to DeliverDataToTracks() must be made at very specific
+// times.
+//
+// An instance of this class is owned by blink::WebMediaStreamSource.
+//
+// Usage example:
+//
+//   class MyAudioSource : public MediaStreamSource { ... };
+//
+//   blink::WebMediaStreamSource blink_source = ...;
+//   blink::WebMediaStreamTrack blink_track = ...;
+//   blink_source.setExtraData(new MyAudioSource());  // Takes ownership.
+//   if (MediaStreamAudioSource::From(blink_source)
+//           ->ConnectToTrack(blink_track)) {

[perkj_chrome, 2016/04/08 14:05:41]

For video- the tracks register and deregister with the source instead using a
ConnectToTrack. Would it make sence for audio to work the same way for
consistency?

[miu, 2016/04/19 00:40:22]

I looked into this a bit.  IMO, it would be better for MediaStreamVideoSource to
instead change to the scheme used here since it makes the client code much
simpler.  Some fine points:

1. MSAudioSources sometimes need to provide extended MSAudioTrack
implementations.  It's more appropriate for the source to create the
MSAudioTrack rather than the client code (e.g., media_stream_center.cc) since
the client code does not (and probably should not) know the implementation
details of the source.

2. Client code must be sure to call MSVideoSource::RemoveTrack() at the right
time and get it right for all possible tear-down sequences and code paths.  With
this scheme, MSAudioSources and MSAudioTracks are guaranteed to be safely
shutdown and destroyed whenever their blink owners are destroyed, with no
additional client code written to ensure this.

3. Safety/Robustness: There are comments in MSVideoSource about it's blink owner
having to outlive all tracks.  To me, that feels a bit brittle, as future
changes to blink code could break this assumption.  If the blink code happens to
be changed in some way that causes a MSAudioSource to tear-down before its
tracks, the weak references will make sure nothing bad happens.

[perkj_chrome, 2016/04/20 13:34:53]

Ok- we should try to get video to behave the same once this is landed. Feel free
to file a bug and assign to me.

[miu, 2016/04/20 22:04:52]

Done.  https://bugs.chromium.org/p/chromium/issues/detail?id=605250

+//     LOG(INFO) << "Success!";
+//   } else {
+//     LOG(ERROR) << "Failed!";
+//   }
 class CONTENT_EXPORT MediaStreamAudioSource
     : NON_EXPORTED_BASE(public MediaStreamSource) {
  public:
-  MediaStreamAudioSource(int render_frame_id,
-                         const StreamDeviceInfo& device_info,
-                         const SourceStoppedCallback& stop_callback,
-                         PeerConnectionDependencyFactory* factory);
-  MediaStreamAudioSource();
+  explicit MediaStreamAudioSource(bool is_local_source);
   ~MediaStreamAudioSource() override;
 
   // Returns the MediaStreamAudioSource instance owned by the given blink
   // |source| or null.
-  static MediaStreamAudioSource* From(const blink::WebMediaStreamSource& track);
-
-  void AddTrack(const blink::WebMediaStreamTrack& track,
-                const blink::WebMediaConstraints& constraints,
-                const ConstraintsCallback& callback);
+  static MediaStreamAudioSource* From(
+      const blink::WebMediaStreamSource& source);
 
+  // Provides a weak reference to this MediaStreamAudioSource.  The weak
+  // pointer may only be dereferenced on the main thread.
   base::WeakPtr<MediaStreamAudioSource> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
 
-  // Removes |track| from the list of instances that get a copy of the source
-  // audio data.
-  void StopAudioDeliveryTo(MediaStreamAudioTrack* track);
+  // Returns true if the source of audio is local to the application (e.g.,
+  // microphone input or loopback audio capture) as opposed to audio being
+  // streamed-in from outside the application.
+  bool is_local_source() const { return is_local_source_; }
 
-  WebRtcAudioCapturer* audio_capturer() const { return audio_capturer_.get(); }
+  // Connects this source to the given |track|, creating the appropriate
+  // implementation of the content::MediaStreamAudioTrack interface, which
+  // becomes associated with and owned by |track|.
+  //
+  // Returns true if the source was successfully started and the
+  // MediaStreamAudioTrack assigned to |track.extraData()|.
+  bool ConnectToTrack(const blink::WebMediaStreamTrack& track);
 
-  void SetAudioCapturer(scoped_ptr<WebRtcAudioCapturer> capturer) {
-    DCHECK(!audio_capturer_.get());
-    audio_capturer_ = std::move(capturer);
-  }
+  // Returns the current format of the audio passing through this source to the
+  // sinks.  This can return invalid parameters if the source has not yet been
+  // started.  This method is thread-safe.
+  media::AudioParameters GetAudioParameters() const;
 
-  webrtc::AudioSourceInterface* local_audio_source() {
-    return local_audio_source_.get();
-  }
+  // Returns a unique class identifier.  Some subclasses override and use this
+  // method to provide safe down-casting to their type.
+  virtual void* GetClassIdentifier() const;
 
-  void SetLocalAudioSource(scoped_refptr<webrtc::AudioSourceInterface> source) {
-    local_audio_source_ = std::move(source);
-  }
+ protected:
+  // Returns a new MediaStreamAudioTrack.  |id| is the blink track's ID in
+  // UTF-8.  Subclasses may override this to provide an extended implementation.
+  virtual scoped_ptr<MediaStreamAudioTrack> CreateMediaStreamAudioTrack(
+      const std::string& id);
+
+  // Returns true if the source has already been started and has not yet been
+  // stopped.  Otherwise, attempts to start the source and returns true if
+  // successful.  While a the source is running, it may provide audio on any

[perkj_chrome, 2016/04/08 14:05:41]

nit: While a the /s  While the... 

[miu, 2016/04/19 00:40:22]

Good catch.  Done.

+  // thread by calling DeliverDataToTracks().
+  //
+  // A default no-op implementation is provided in this base class.  Subclasses
+  // should override this method.
+  virtual bool EnsureSourceIsStarted();
 
-  WebAudioCapturerSource* webaudio_capturer() const {
-    return webaudio_capturer_.get();
-  }
+  // Stops the source and guarantees the the flow of audio data has stopped
+  // (i.e., by the time this method returns, there will be no further calls to
+  // DeliverDataToTracks() on any thread).
+  //
+  // A default no-op implementation is provided in this base class.  Subclasses
+  // should override this method.
+  virtual void EnsureSourceIsStopped();
 
-  void SetWebAudioCapturer(scoped_ptr<WebAudioCapturerSource> capturer) {
-    DCHECK(!webaudio_capturer_.get());
-    webaudio_capturer_ = std::move(capturer);
-  }
+  // Called by subclasses to update the format of the audio passing through this
+  // source to the sinks.  This may be called at any time, before or after
+  // tracks have been connected; but must be called at least once before
+  // DeliverDataToTracks().  This method is thread-safe.
+  void SetFormat(const media::AudioParameters& params);
 
- protected:
-  void DoStopSource() override;
+  // Called by subclasses to deliver audio data to the currently-connected
+  // tracks.  This method is thread-safe.
+  void DeliverDataToTracks(const media::AudioBus& audio_bus,
+                           base::TimeTicks reference_time);
 
  private:
-  const int render_frame_id_;
-  PeerConnectionDependencyFactory* const factory_;
+  // MediaStreamSource override.
+  void DoStopSource() override;
 
-  // MediaStreamAudioSource is the owner of either a WebRtcAudioCapturer or a
-  // WebAudioCapturerSource.
-  //
-  // TODO(miu): In a series of soon-upcoming changes, WebRtcAudioCapturer and
-  // WebAudioCapturerSource will become subclasses of MediaStreamAudioSource
-  // instead.
-  scoped_ptr<WebRtcAudioCapturer> audio_capturer_;
-  scoped_ptr<WebAudioCapturerSource> webaudio_capturer_;
-
-  // This member holds an instance of webrtc::LocalAudioSource. This is used
-  // as a container for audio options.
-  scoped_refptr<webrtc::AudioSourceInterface> local_audio_source_;
+  // Instantiates the appropriate MediaStreamAudioTrack implementation and sets
+  // it as |track|'s extraData.  This is called by ConnectToTrack() after all
+  // preconditions have been met: 1) A prior call to EnsureSourceIsStarted()
+  // returned true; 2) |track| does not already hold a MediaStreamAudioTrack
+  // instance.
+  void ConnectStartedSourceToTrack(const blink::WebMediaStreamTrack& track);
+
+  // Removes |track| from the list of instances that get a copy of the source
+  // audio data.  The "stop callback" that was provided to the track calls
+  // this.
+  void StopAudioDeliveryTo(MediaStreamAudioTrack* track);
+
+  // True if the source of audio is a local device.  False if the source is
+  // remote (e.g., streamed-in from a server).
+  const bool is_local_source_;
+
+  // In debug builds, check that all methods that could cause object graph
+  // or data flow changes are being called on the main thread.
+  base::ThreadChecker thread_checker_;
+
+  // Set to true once this source has been permanently stopped.
+  bool is_stopped_;
+
+  // Protects concurrent access to |params_| and |tracks_|.
+  mutable base::Lock lock_;
+
+  // Specifies the current format of the audio passing through the pipeline.
+  media::AudioParameters params_;
+
+  // List of currently-connected MediaStreamAudioTracks.  While
+  // MediaStreamAudioSource creates these instances, blink::WebMediaStreamTrack
+  // instances own the objects.
+  std::vector<MediaStreamAudioTrack*> tracks_;
 
   // Provides weak pointers so that MediaStreamAudioTracks won't call

[perkj_chrome, 2016/04/08 14:05:41]

Good but I think its safe regardless- a blink track keeps the blink source
alive.

[miu, 2016/04/19 00:40:22]

Ack.  Discussed in reply to your comment above.

[perkj_chrome, 2016/04/20 13:34:53]

Acknowledged.

   // StopAudioDeliveryTo() if this instance dies first.