Make rendering of MediaStreams reflect changes to its set of tracks.

content/renderer/media/webmediaplayer_ms.cc

Index: content/renderer/media/webmediaplayer_ms.cc
diff --git a/content/renderer/media/webmediaplayer_ms.cc b/content/renderer/media/webmediaplayer_ms.cc
index f064d1ca786917cbb92d6ac228108fb099550fce..bf214944d24c65e00e7dff7846915318994530e4 100644
--- a/content/renderer/media/webmediaplayer_ms.cc
+++ b/content/renderer/media/webmediaplayer_ms.cc
@@ -20,6 +20,8 @@
 #include "content/public/renderer/media_stream_audio_renderer.h"
 #include "content/public/renderer/media_stream_renderer_factory.h"
 #include "content/public/renderer/media_stream_video_renderer.h"
+#include "content/renderer/media/media_stream_audio_track.h"
+#include "content/renderer/media/media_stream_video_track.h"
 #include "content/renderer/media/web_media_element_source_utils.h"
 #include "content/renderer/media/webmediaplayer_ms_compositor.h"
 #include "content/renderer/render_frame_impl.h"
@@ -190,6 +192,12 @@ WebMediaPlayerMS::~WebMediaPlayerMS() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (!web_stream_.IsNull()) {
+    MediaStream* native_stream = MediaStream::GetMediaStream(web_stream_);
+    if (native_stream)
+      native_stream->RemoveObserver(this);
+  }
+
   // Destruct compositor resources in the proper order.
   get_client()->SetWebLayer(nullptr);
   if (video_weblayer_)
@@ -223,24 +231,29 @@ void WebMediaPlayerMS::Load(LoadType load_type,
   // TODO(acolwell): Change this to DCHECK_EQ(load_type, LoadTypeMediaStream)
   // once Blink-side changes land.
   DCHECK_NE(load_type, kLoadTypeMediaSource);
-  blink::WebMediaStream web_stream =
-      GetWebMediaStreamFromWebMediaPlayerSource(source);
+  web_stream_ = GetWebMediaStreamFromWebMediaPlayerSource(source);
+  if (!web_stream_.IsNull()) {
+    MediaStream* native_stream = MediaStream::GetMediaStream(web_stream_);
+    if (native_stream)
+      native_stream->AddObserver(this);
+  }
 
   compositor_ = new WebMediaPlayerMSCompositor(
-      compositor_task_runner_, io_task_runner_, web_stream, AsWeakPtr());
+      compositor_task_runner_, io_task_runner_, web_stream_, AsWeakPtr());
 
   SetNetworkState(WebMediaPlayer::kNetworkStateLoading);
   SetReadyState(WebMediaPlayer::kReadyStateHaveNothing);
   std::string stream_id =
-      web_stream.IsNull() ? std::string() : web_stream.Id().Utf8();
+      web_stream_.IsNull() ? std::string() : web_stream_.Id().Utf8();
   media_log_->AddEvent(media_log_->CreateLoadEvent(stream_id));
 
   frame_deliverer_.reset(new WebMediaPlayerMS::FrameDeliverer(
       AsWeakPtr(),
       base::Bind(&WebMediaPlayerMSCompositor::EnqueueFrame, compositor_)));
   video_frame_provider_ = renderer_factory_->GetVideoRenderer(
-      web_stream, media::BindToCurrentLoop(base::Bind(
-                      &WebMediaPlayerMS::OnSourceError, AsWeakPtr())),
+      web_stream_,
+      media::BindToCurrentLoop(
+          base::Bind(&WebMediaPlayerMS::OnSourceError, AsWeakPtr())),
       frame_deliverer_->GetRepaintCallback(), io_task_runner_,
       media_task_runner_, worker_task_runner_, gpu_factories_);
 
@@ -253,7 +266,7 @@ void WebMediaPlayerMS::Load(LoadType load_type,
                          media_log_.get());
 
     audio_renderer_ = renderer_factory_->GetAudioRenderer(
-        web_stream, frame->GetRoutingID(), initial_audio_output_device_id_,
+        web_stream_, frame->GetRoutingID(), initial_audio_output_device_id_,
         initial_security_origin_);
   }
 
@@ -275,6 +288,63 @@ void WebMediaPlayerMS::Load(LoadType load_type,
   }
 }
 
+void WebMediaPlayerMS::TrackAdded(const blink::WebMediaStreamTrack& track) {
+  Reload();
+}
+
+void WebMediaPlayerMS::TrackRemoved(const blink::WebMediaStreamTrack& track) {
+  Reload();
+}
+
+void WebMediaPlayerMS::Reload() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  blink::WebVector<blink::WebMediaStreamTrack> video_tracks;
+  // VideoTracks() is a getter.
+  web_stream_.VideoTracks(video_tracks);
+  bool has_video_track = !video_tracks.IsEmpty() &&

[emircan, 2017/07/05 18:04:09]

const

[Guido Urdaneta, 2017/07/06 18:50:33]

Removed.

+                         MediaStreamVideoTrack::GetTrack(video_tracks[0]);
+
+  if (has_video_track && !video_frame_provider_) {

[emircan, 2017/07/05 18:04:09]

I have a question regarding multiple video track case implementation in blink. I
am not sure how many video tracks are allowed, but consider this. There are 2
tracks: A-B. We start playing and add sink to only the first track A. Suppose A
is removed. Then, we don't do anything based on this if else loop. Is that
expected?

[Guido Urdaneta, 2017/07/05 19:01:08]

Good question. I'll check what other browsers do ang get back to you.

[Guido Urdaneta, 2017/07/06 18:50:33]

Edge only plays a mediaStream if it has a single track. If it has 0, 2 or more
tracks, nothing is rendered on the video element.

Firefox, like Chrome, plays the first track. If the first track is removed, it
stops playing, but if the same stream is reassigned to the element it plays the
new first track, so it's probably a bug that it stops playing when the first
track is removed.

This new patchset makes Chrome always play the first track. If the first track
is removed, keep playing the new first track.

+    // Video track was added.
+    video_frame_provider_ = renderer_factory_->GetVideoRenderer(
+        web_stream_,
+        media::BindToCurrentLoop(
+            base::Bind(&WebMediaPlayerMS::OnSourceError, AsWeakPtr())),
+        frame_deliverer_->GetRepaintCallback(), io_task_runner_,
+        media_task_runner_, worker_task_runner_, gpu_factories_);
+    DCHECK(video_frame_provider_);
+    video_frame_provider_->Start();

[emircan, 2017/07/05 18:04:09]

You should also cover the uses of WebMediaPlayerMS::HasVideo(), since we can now
start playing with no video and add it later on. i.e.,
RendererWebMediaPlayerDelegate::DidPlay() wouldn't reflect these changes as it
is.

[Guido Urdaneta, 2017/07/05 19:01:08]

Can you elaborate a bit since I don't have the full context?

[emircan, 2017/07/05 19:45:57]

RendererWebMediaPlayerDelegate::DidPlay() [0] keeps a count of active videos.
Suppose we start playing <video> where there are no video tracks initially and
later on add a video track. AFACT, only Reload() will be called.
RendererWebMediaPlayerDelegate would not be notified of this update.

[0]
https://cs.chromium.org/chromium/src/content/renderer/media/renderer_webmedia...

[Guido Urdaneta, 2017/07/06 18:50:33]

As discussed offline, calling DidPlay twice causes a browser CHECK.
Adding dalecurtis@ for his thoughts on this.

+  } else if (video_frame_provider_ && !has_video_track) {
+    // Video track was removed.
+    video_frame_provider_->Stop();
+    video_frame_provider_ = nullptr;
+  }
+
+  RenderFrame* const frame = RenderFrame::FromWebFrame(frame_);
+  if (frame) {
+    blink::WebVector<blink::WebMediaStreamTrack> audio_tracks;
+    // AudioTracks() is a getter.
+    web_stream_.AudioTracks(audio_tracks);
+    bool has_audio_track = !audio_tracks.IsEmpty() &&
+                           MediaStreamAudioTrack::GetTrack(audio_tracks[0]);
+
+    if (has_audio_track && !audio_renderer_) {
+      // Audio track was added.
+      audio_renderer_ = renderer_factory_->GetAudioRenderer(
+          web_stream_, frame->GetRoutingID(), initial_audio_output_device_id_,
+          initial_security_origin_);
+      audio_renderer_->SetVolume(volume_);
+      audio_renderer_->Start();
+      audio_renderer_->Play();
+    } else if (!has_audio_track && audio_renderer_) {
+      // Audio track was removed.
+      audio_renderer_->Stop();
+      audio_renderer_ = nullptr;
+    }
+  }
+}
+
 void WebMediaPlayerMS::Play() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -381,6 +451,9 @@ bool WebMediaPlayerMS::HasAudio() const {
 
 blink::WebSize WebMediaPlayerMS::NaturalSize() const {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (!video_frame_provider_)
+    return blink::WebSize();
+
   if (video_rotation_ == media::VIDEO_ROTATION_90 ||
       video_rotation_ == media::VideoRotation::VIDEO_ROTATION_270) {
     const gfx::Size& current_size = compositor_->GetCurrentSize();