Change VideoRtpReceiver to create remote VideoTrack and VideoTrackSource.

webrtc/api/peerconnection.cc

Index: webrtc/api/peerconnection.cc
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index c2039aeea414321e612e461d635df72e486904a3..e1ef6e2d67d42d2572a4933bf765f53d6f3ba570 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -381,10 +381,8 @@ namespace webrtc {
 // Factory class for creating remote MediaStreams and MediaStreamTracks.
 class RemoteMediaStreamFactory {
  public:
-  explicit RemoteMediaStreamFactory(rtc::Thread* signaling_thread,
-                                    rtc::Thread* worker_thread)
-      : signaling_thread_(signaling_thread),
-        worker_thread_(worker_thread) {}
+  explicit RemoteMediaStreamFactory(rtc::Thread* signaling_thread)
+      : signaling_thread_(signaling_thread) {}
 
   rtc::scoped_refptr<MediaStreamInterface> CreateMediaStream(
       const std::string& stream_label) {
@@ -400,15 +398,6 @@ class RemoteMediaStreamFactory {
         stream, track_id, RemoteAudioSource::Create(ssrc, provider));
   }
 
-  VideoTrackInterface* AddVideoTrack(webrtc::MediaStreamInterface* stream,
-                                     const std::string& track_id) {
-    return AddTrack<VideoTrackInterface, VideoTrack, VideoTrackProxy>(
-        stream, track_id,
-        VideoCapturerTrackSource::Create(
-            worker_thread_, new RemoteVideoCapturer(), nullptr, true)
-            .get());
-  }
-
  private:
   template <typename TI, typename T, typename TP, typename S>
   TI* AddTrack(MediaStreamInterface* stream,
@@ -424,7 +413,6 @@ class RemoteMediaStreamFactory {
   }
 
   rtc::Thread* signaling_thread_;
-  rtc::Thread* worker_thread_;
 };
 
 bool ExtractMediaSessionOptions(
@@ -611,8 +599,8 @@ bool PeerConnection::Initialize(
 
   media_controller_.reset(factory_->CreateMediaController(media_config));
 
-  remote_stream_factory_.reset(new RemoteMediaStreamFactory(
-      factory_->signaling_thread(), factory_->worker_thread()));
+  remote_stream_factory_.reset(
+      new RemoteMediaStreamFactory(factory_->signaling_thread()));
 
   session_.reset(
       new WebRtcSession(media_controller_.get(), factory_->signaling_thread(),
@@ -1324,12 +1312,15 @@ void PeerConnection::CreateAudioReceiver(MediaStreamInterface* stream,
       new AudioRtpReceiver(audio_track, ssrc, session_.get())));
 }
 
-void PeerConnection::CreateVideoReceiver(MediaStreamInterface* stream,
-                                         VideoTrackInterface* video_track,
-                                         uint32_t ssrc) {
-  receivers_.push_back(RtpReceiverProxy::Create(
-      signaling_thread(),
-      new VideoRtpReceiver(video_track, ssrc, session_.get())));
+VideoRtpReceiver* PeerConnection::CreateVideoReceiver(
+    const std::string& track_id,
+    uint32_t ssrc) {
+  VideoRtpReceiver* video_receiver = new VideoRtpReceiver(
+      track_id, factory_->worker_thread(), ssrc, session_.get());
+  // RtpReceiverProxy takes ownership of |video_receiver|;
+  receivers_.push_back(
+      RtpReceiverProxy::Create(signaling_thread(), video_receiver));
+  return video_receiver;

[pthatcher1, 2016/03/09 18:16:49]

Why do we keep ownership of the proxy but then return the non-proxy?  That seems
weird.

[perkj_webrtc, 2016/03/09 21:22:59]

That is my attempt to avoid a static cast and to be able to get a
videotrackinterface and not a mediastreamtrack from the VideoRtpReceiver. I can
have this method return a VideoRtpReceiverInteface and return the proxy if I add
a new method to the interface. Let me try that and see if  you like it better.

[Taylor Brandstetter, 2016/03/09 22:10:41]

I assume you mean "return an RtpReceiverInterface"? I'm actually planning on
cleaning up that interface so it doesn't have any methods besides those in the
spec. So I'd prefer the current implementation. Or better yet, pass a stream
into CreateVideoReceiver and add the track to the stream within the method, to
avoid the need for returning anything.

[perkj_webrtc, 2016/03/10 00:26:38]

that is a very good idea.

 }
 
 // TODO(deadbeef): Keep RtpReceivers around even if track goes away in remote
@@ -1677,9 +1668,8 @@ void PeerConnection::OnRemoteTrackSeen(const std::string& stream_label,
         ssrc, session_.get(), stream, track_id);
     CreateAudioReceiver(stream, audio_track, ssrc);
   } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
-    VideoTrackInterface* video_track =
-        remote_stream_factory_->AddVideoTrack(stream, track_id);
-    CreateVideoReceiver(stream, video_track, ssrc);
+    VideoRtpReceiver* receiver = CreateVideoReceiver(track_id, ssrc);
+    stream->AddTrack(receiver->video_track());
   } else {
     RTC_DCHECK(false && "Invalid media type");
   }
@@ -1702,7 +1692,6 @@ void PeerConnection::OnRemoteTrackRemoved(const std::string& stream_label,
     rtc::scoped_refptr<VideoTrackInterface> video_track =
         stream->FindVideoTrack(track_id);
     if (video_track) {
-      video_track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
       stream->RemoveTrack(video_track);
       DestroyVideoReceiver(stream, video_track);

[pthatcher1, 2016/03/09 18:16:49]

Can you put a comment here saying "Stopping or destroying a VideoRtpReceiver
will end VideoRtpReceiver::track()"?

[perkj_webrtc, 2016/03/09 21:22:59]

Done.

     }