Change VideoTrack implementation to invoke VideoTrackSourceInterface::AddOrUpdateSink on wt

webrtc/pc/peerconnectionfactory.cc

Index: webrtc/pc/peerconnectionfactory.cc
diff --git a/webrtc/pc/peerconnectionfactory.cc b/webrtc/pc/peerconnectionfactory.cc
index 77e6f2c272582cc988416f2a801f8011ed34c765..98b7f23a632a19cc54aa11c006694e9e8288ac60 100644
--- a/webrtc/pc/peerconnectionfactory.cc
+++ b/webrtc/pc/peerconnectionfactory.cc
@@ -252,22 +252,16 @@ PeerConnectionFactory::CreateVideoSource(
     std::unique_ptr<cricket::VideoCapturer> capturer,
     const MediaConstraintsInterface* constraints) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
-  rtc::scoped_refptr<VideoTrackSourceInterface> source(

[Taylor Brandstetter, 2017/07/05 23:13:40]

Hmm... I don't know if it's safe to do this, because existing applications may
be expecting the returned object to be a proxy. Would it be too egregious to end
up with an object wrapped in two layers of proxy? I guess we could add an
"is_proxy" method or something to avoid the double wrapping... No solution seems
perfect though.

Maybe we could add a method that creates a video track directly from a video
capturer, without going through the VideoTrackSourceInterface in the middle, and
deprecate this method? All the code I'm finding calls CreateVideoTrack directly
after CreateVideoSource, and doesn't hold onto the source pointer. Then we could
avoid the double-wrapping without any hacks.

[perkj_webrtc, 2017/07/06 15:08:20]

VideoTrackSourceInterface is supposed to be an implementation of a capturer and
the cricket::VideoCapturer thing should be deprecated. This function exist to
wrap old cricket::VideoCapturer implementations such as the one in Chrome.

Our Android implementation actually inherit AdaptedVideoTrackSource and create a
proxy itself. So if we go down this road, we should remove creating the proxy
here:
https://cs.chromium.org/chromium/src/third_party/webrtc/sdk/android/src/jni/v...

An extra layer of proxies should be fine but is of course not optimal.
The proxy implementation checks the current thread and just invokes the function
if a thread hop is not needed. 
https://cs.chromium.org/chromium/src/third_party/webrtc/api/proxy.h?rcl=74848...
 

[Taylor Brandstetter, 2017/07/06 22:24:05]

So, what do you plan to do here then? If this method is deprecated anyway, maybe
biting the bullet and having two layers of proxies isn't so terrible.

I have another suggestion, though: why not have VideoTrack handle the thread
hopping itself, right here:
https://cs.chromium.org/chromium/src/third_party/webrtc/pc/videotrack.cc?type...

It's calling AddOrUpdateSink in a loop for each sink. Which is suboptimal in the
first place, since it will result in multiple thread hops if there are multiple
sinks.

-      VideoCapturerTrackSource::Create(worker_thread_, std::move(capturer),
-                                       constraints, false));
-  return VideoTrackSourceProxy::Create(signaling_thread_, worker_thread_,
-                                       source);
+  return VideoCapturerTrackSource::Create(worker_thread_, std::move(capturer),
+                                          constraints, false);
 }
 
 rtc::scoped_refptr<VideoTrackSourceInterface>
 PeerConnectionFactory::CreateVideoSource(
     std::unique_ptr<cricket::VideoCapturer> capturer) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
-  rtc::scoped_refptr<VideoTrackSourceInterface> source(
-      VideoCapturerTrackSource::Create(worker_thread_, std::move(capturer),
-                                       false));
-  return VideoTrackSourceProxy::Create(signaling_thread_, worker_thread_,
-                                       source);
+  return VideoCapturerTrackSource::Create(worker_thread_, std::move(capturer),
+                                          false);
 }
 
 bool PeerConnectionFactory::StartAecDump(rtc::PlatformFile file,
@@ -342,7 +336,8 @@ rtc::scoped_refptr<VideoTrackInterface> PeerConnectionFactory::CreateVideoTrack(
     VideoTrackSourceInterface* source) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   rtc::scoped_refptr<VideoTrackInterface> track(
-      VideoTrack::Create(id, source));
+      VideoTrack::Create(id, VideoTrackSourceProxy::Create(
+                                 signaling_thread_, worker_thread_, source)));
   return VideoTrackProxy::Create(signaling_thread_, worker_thread_, track);
 }