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

content/renderer/media/media_stream_audio_source.cc

Index: content/renderer/media/media_stream_audio_source.cc
diff --git a/content/renderer/media/media_stream_audio_source.cc b/content/renderer/media/media_stream_audio_source.cc
index d2027fd939e797db1171625fcf469f8466c2fd36..efe1720ffc84b751d40b258cd4e6db1bef18d63b 100644
--- a/content/renderer/media/media_stream_audio_source.cc
+++ b/content/renderer/media/media_stream_audio_source.cc
@@ -4,29 +4,26 @@
 
 #include "content/renderer/media/media_stream_audio_source.h"
 
-#include "content/renderer/media/webrtc_local_audio_track.h"
-#include "content/renderer/render_frame_impl.h"
+#include "base/bind.h"
+#include "content/renderer/media/media_stream_audio_track.h"
 #include "third_party/WebKit/public/platform/WebMediaStreamSource.h"
+#include "third_party/WebKit/public/platform/WebString.h"
 
 namespace content {
 
-MediaStreamAudioSource::MediaStreamAudioSource(
-    int render_frame_id,
-    const StreamDeviceInfo& device_info,
-    const SourceStoppedCallback& stop_callback,
-    PeerConnectionDependencyFactory* factory)
-    : render_frame_id_(render_frame_id), factory_(factory),
+MediaStreamAudioSource::MediaStreamAudioSource(bool is_local_source)
+    : is_local_source_(is_local_source),
+      is_stopped_(false),
       weak_factory_(this) {
-  SetDeviceInfo(device_info);
-  SetStopCallback(stop_callback);
+  DVLOG(1) << "MediaStreamAudioSource@" << this << "::MediaStreamAudioSource("
+           << (is_local_source_ ? "local" : "remote") << " source)";
 }
 
-MediaStreamAudioSource::MediaStreamAudioSource()
-    : render_frame_id_(-1), factory_(NULL), weak_factory_(this) {
+MediaStreamAudioSource::~MediaStreamAudioSource() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "MediaStreamAudioSource@" << this << " is being destroyed.";
 }
 
-MediaStreamAudioSource::~MediaStreamAudioSource() {}
-
 // static
 MediaStreamAudioSource* MediaStreamAudioSource::From(
     const blink::WebMediaStreamSource& source) {
@@ -37,50 +34,104 @@ MediaStreamAudioSource* MediaStreamAudioSource::From(
   return static_cast<MediaStreamAudioSource*>(source.getExtraData());
 }
 
-void MediaStreamAudioSource::DoStopSource() {
-  if (audio_capturer_)
-    audio_capturer_->Stop();
-  if (webaudio_capturer_)
-    webaudio_capturer_->Stop();
-}
+bool MediaStreamAudioSource::ConnectToTrack(
+    const blink::WebMediaStreamTrack& blink_track) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!blink_track.isNull());
 
-void MediaStreamAudioSource::AddTrack(
-    const blink::WebMediaStreamTrack& track,
-    const blink::WebMediaConstraints& constraints,
-    const ConstraintsCallback& callback) {
-  // TODO(xians): Properly implement for audio sources.
-  if (!local_audio_source_.get()) {
-    if (!factory_->InitializeMediaStreamAudioSource(render_frame_id_,
-                                                    constraints, this)) {
-      // The source failed to start.
-      // UserMediaClientImpl rely on the |stop_callback| to be triggered when
-      // the last track is removed from the source. But in this case, the
-      // source is is not even started. So we need to fail both adding the
-      // track and trigger |stop_callback|.
-      callback.Run(this, MEDIA_DEVICE_TRACK_START_FAILURE, "");
-      StopSource();
-      return;
-    }
+  // Sanity-check that there is not already a MediaStreamAudioTrack instance
+  // associated with |blink_track|.
+  if (MediaStreamAudioTrack::From(blink_track)) {
+    LOG(DFATAL)
+        << "Attempting to connect another source to a WebMediaStreamTrack.";
+    return false;
   }
 
-  factory_->CreateLocalAudioTrack(track);
-  callback.Run(this, MEDIA_DEVICE_OK, "");
+  // Unless the source has already been permanently stopped, ensure it is
+  // started. If the source is not started, the new MediaStreamAudioTrack will
+  // be initialized to the stopped/ended state.
+  const bool initializing_ended_track = is_stopped_ || !EnsureSourceIsStarted();
+
+  // Create and initialize a new MediaStreamAudioTrack and pass ownership of it
+  // to the WebMediaStreamTrack.
+  blink::WebMediaStreamTrack mutable_blink_track = blink_track;
+  mutable_blink_track.setExtraData(
+      CreateMediaStreamAudioTrack(blink_track.id().utf8()).release());
+
+  // Start the track and add it as a consumer of audio from this source.
+  MediaStreamAudioTrack* const track = MediaStreamAudioTrack::From(blink_track);
+  DCHECK(track);
+  track->Start(base::Bind(&MediaStreamAudioSource::StopAudioDeliveryTo,
+                          weak_factory_.GetWeakPtr(), track));
+  track->SetEnabled(blink_track.isEnabled());
+  DVLOG(1) << "Adding MediaStreamAudioTrack@" << track
+           << " as a consumer of MediaStreamAudioSource@" << this << '.';
+  deliverer_.AddConsumer(track);
+
+  // If the source failed to start, or has already been permanently stopped,
+  // stop the track to set its ready state to "ended."

[o1ka, 2016/04/21 18:51:22]

"ended."->"ended".
It's not quite clear what "ready state" is in relation to MediaStreamAudioTrack.

[miu, 2016/04/21 20:42:30]

Done.
I had assumed "ready state" is a property of the blink::WebMediaStreamTrack,
which is changed by MediaStreamAudioTrack.  However, looking at this more
in-depth, I see that this is only used as a way to notify sinks when a source
has ended.  See reply to comment below...

[o1ka, 2016/04/22 11:29:25]

Yes, I realized it, the problem about the comment was that it had not enough of
context since it referred to a state of a blink class but mentioned media class
only (but looks like my review comment did not have enough of the context either
:) )

+  if (initializing_ended_track)
+    track->Stop();

[o1ka, 2016/04/21 18:51:22]

Consumer is added at l.69 and it is removed here when track->Stop() calls
StopAudioDeliveryTo() as a callback. Is it intentional? Can it be done in a more
obvious way? What is the "state" of the track if we have not called Start()?
Shouldn't it be "ended"?

[miu, 2016/04/21 20:42:30]

It was.  Per suggested (previous round of comments) that we always create an
"ended" track when the source is stopped.  
Yep.  See my reply to your comment above (L72).  Since it's impossible for any
sinks to have been connected to this track, there's no need to start/stop it. 
So, I've changed this code to just return early without the extra overhead.  The
MediaStreamAudioTrack will be in the correct state.

Also, I've added some code to MediaStreamAudioTrack::AddSink(): If client code
still tries to add sinks to a stopped track, we'll just notify the sink that the
track has ended and NOT add the sink to the track.

[o1ka, 2016/04/22 11:29:25]

Yes, I understand the reason of a changed, just was confused with the execution
flow. Great that it can be done in a more straightforward way.

+
+  return !initializing_ended_track;
+}
+
+media::AudioParameters MediaStreamAudioSource::GetAudioParameters() const {
+  return deliverer_.GetAudioParameters();
+}
+
+void* MediaStreamAudioSource::GetClassIdentifier() const {
+  return nullptr;
+}
+
+std::unique_ptr<MediaStreamAudioTrack>
+MediaStreamAudioSource::CreateMediaStreamAudioTrack(const std::string& id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return std::unique_ptr<MediaStreamAudioTrack>(
+      new MediaStreamAudioTrack(is_local_source()));
+}
+
+bool MediaStreamAudioSource::EnsureSourceIsStarted() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "MediaStreamAudioSource@" << this << "::EnsureSourceIsStarted()";
+  return true;
+}
+
+void MediaStreamAudioSource::EnsureSourceIsStopped() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "MediaStreamAudioSource@" << this << "::EnsureSourceIsStopped()";
+}
+
+void MediaStreamAudioSource::SetFormat(const media::AudioParameters& params) {
+  DVLOG(1) << "MediaStreamAudioSource@" << this << "::SetFormat("
+           << params.AsHumanReadableString() << "), was previously set to {"
+           << deliverer_.GetAudioParameters().AsHumanReadableString() << "}.";
+  deliverer_.OnSetFormat(params);
+}
+
+void MediaStreamAudioSource::DeliverDataToTracks(
+    const media::AudioBus& audio_bus,
+    base::TimeTicks reference_time) {
+  deliverer_.OnData(audio_bus, reference_time);
+}
+
+void MediaStreamAudioSource::DoStopSource() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  EnsureSourceIsStopped();
+  is_stopped_ = true;
 }
 
 void MediaStreamAudioSource::StopAudioDeliveryTo(MediaStreamAudioTrack* track) {
-  DCHECK(track);
-  if (audio_capturer_) {
-    // The cast here is safe because only WebRtcLocalAudioTracks are ever used
-    // with WebRtcAudioCapturer sources.
-    //
-    // TODO(miu): That said, this ugly cast won't be necessary after my
-    // soon-upcoming refactoring change.
-    audio_capturer_->RemoveTrack(static_cast<WebRtcLocalAudioTrack*>(track));
-  }
-  if (webaudio_capturer_) {
-    // A separate source is created for each track, so just stop the source.
-    webaudio_capturer_->Stop();
-  }
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const bool did_remove_last_track = deliverer_.RemoveConsumer(track);
+  DVLOG(1) << "Removed MediaStreamAudioTrack@" << track
+           << " as a consumer of MediaStreamAudioSource@" << this << '.';
+
+  // The W3C spec requires a source automatically stop when the last track is
+  // stopped.
+  if (!is_stopped_ && did_remove_last_track)
+    MediaStreamSource::StopSource();
 }
 
 }  // namespace content