Change VideoTrack implementation to invoke VideoTrackSourceInterface::AddOrUpdateSink on wt

webrtc/pc/peerconnectionfactory.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 234 matching lines @@
   rtc::scoped_refptr<LocalAudioSource> source(
       LocalAudioSource::Create(&options));
   return source;
 }
 
 rtc::scoped_refptr<VideoTrackSourceInterface>
 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,
                                          int64_t max_size_bytes) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   return channel_manager_->StartAecDump(file, max_size_bytes);
 }
 
 void PeerConnectionFactory::StopAecDump() {
   RTC_DCHECK(signaling_thread_->IsCurrent());
@@ skipping 54 matching lines @@
   RTC_DCHECK(signaling_thread_->IsCurrent());
   return MediaStreamProxy::Create(signaling_thread_,
                                   MediaStream::Create(label));
 }
 
 rtc::scoped_refptr<VideoTrackInterface> PeerConnectionFactory::CreateVideoTrack(
     const std::string& id,
     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);
 }
 
 rtc::scoped_refptr<AudioTrackInterface>
 PeerConnectionFactory::CreateAudioTrack(const std::string& id,
                                         AudioSourceInterface* source) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   rtc::scoped_refptr<AudioTrackInterface> track(AudioTrack::Create(id, source));
   return AudioTrackProxy::Create(signaling_thread_, track);
 }
@@ skipping 30 matching lines @@
   RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
   return std::unique_ptr<cricket::MediaEngineInterface>(
       cricket::WebRtcMediaEngineFactory::Create(
           default_adm_.get(), audio_encoder_factory_,
           audio_decoder_factory_,
           video_encoder_factory_.get(), video_decoder_factory_.get(),
           external_audio_mixer_));
 }
 
 }  // namespace webrtc