Implement missing features in WebrtcVideoStream.

remoting/protocol/webrtc_video_stream.cc

Index: remoting/protocol/webrtc_video_stream.cc
diff --git a/remoting/protocol/webrtc_video_stream.cc b/remoting/protocol/webrtc_video_stream.cc
index 50c6ddd2e4d2521ee05b540951739604949c27e6..b073798b8dc71298e873df746158b5622be6908f 100644
--- a/remoting/protocol/webrtc_video_stream.cc
+++ b/remoting/protocol/webrtc_video_stream.cc
@@ -5,28 +5,68 @@
 #include "remoting/protocol/webrtc_video_stream.h"
 
 #include "base/logging.h"
+#include "remoting/protocol/webrtc_video_capturer_adapter.h"
 #include "third_party/libjingle/source/talk/app/webrtc/mediastreaminterface.h"
 #include "third_party/libjingle/source/talk/app/webrtc/peerconnectioninterface.h"
+#include "third_party/libjingle/source/talk/app/webrtc/test/fakeconstraints.h"
 #include "third_party/libjingle/source/talk/app/webrtc/videosourceinterface.h"
 
 namespace remoting {
 namespace protocol {
 
-WebrtcVideoStream::WebrtcVideoStream(
-  rtc::scoped_refptr<webrtc::PeerConnectionInterface> connection,
-    rtc::scoped_refptr<webrtc::MediaStreamInterface> stream)
-    : connection_(connection), stream_(stream) {}
+const char kStreamLabel[] = "screen_stream";
+const char kVideoLabel[] = "screen_video";
+
+WebrtcVideoStream::WebrtcVideoStream() {}
 
 WebrtcVideoStream::~WebrtcVideoStream() {
-  for (const auto& track : stream_->GetVideoTracks()) {
-    track->GetSource()->Stop();
-    stream_->RemoveTrack(track.get());
+  if (stream_) {
+    for (const auto& track : stream_->GetVideoTracks()) {
+      track->GetSource()->Stop();
+      stream_->RemoveTrack(track.get());
+    }
+    connection_->RemoveStream(stream_.get());
+  }
+}
+
+bool WebrtcVideoStream::Start(
+    scoped_ptr<webrtc::DesktopCapturer> desktop_capturer,
+    scoped_refptr<webrtc::PeerConnectionInterface> connection,
+    scoped_refptr<webrtc::PeerConnectionFactoryInterface>
+        peer_connection_factory) {
+  scoped_ptr<WebrtcVideoCapturerAdapter> capturer_adapter(
+      new WebrtcVideoCapturerAdapter(std::move(desktop_capturer)));
+
+  connection_ = connection;
+
+  // Set video stream constraints.
+  webrtc::FakeConstraints video_constraints;
+  video_constraints.AddMandatory(
+      webrtc::MediaConstraintsInterface::kMinFrameRate, 5);
+
+  video_track_ =
+      peer_connection_factory->CreateVideoTrack(
+                                 kVideoLabel,
+                                 peer_connection_factory->CreateVideoSource(
+                                     capturer_adapter.release(),
+                                     &video_constraints))
+          .get();
+
+  stream_ = peer_connection_factory->CreateLocalMediaStream(kStreamLabel);
+
+  if (!stream_->AddTrack(video_track_.get()) ||
+      !connection_->AddStream(stream_.get())) {
+    video_track_ = nullptr;
+    stream_ = nullptr;
+    connection_ = nullptr;
+    return false;
   }
-  connection_->RemoveStream(stream_.get());
+
+  return true;
 }
 
 void WebrtcVideoStream::Pause(bool pause) {
-  NOTIMPLEMENTED();
+  capturer_adapter()->Pause(pause);
 }
 
 void WebrtcVideoStream::OnInputEventReceived(int64_t event_timestamp) {
@@ -42,7 +82,15 @@ void WebrtcVideoStream::SetLosslessColor(bool want_lossless) {
 }
 
 void WebrtcVideoStream::SetSizeCallback(const SizeCallback& size_callback) {
-  NOTIMPLEMENTED();
+  capturer_adapter()->SetSizeCallback(size_callback);
+}
+
+WebrtcVideoCapturerAdapter* WebrtcVideoStream::capturer_adapter() {
+  WebrtcVideoCapturerAdapter* capturer_adapter =
+      reinterpret_cast<WebrtcVideoCapturerAdapter*>(
+          video_track_->GetSource()->GetVideoCapturer());

[Jamie, 2016/01/08 22:59:14]

Is this allowed? I've never seen reinterpret_cast used to cast from a base class
to a derived one. Even if it's safe, you're relying on an implementation detail
of webrtc::PeerConnectionFactoryInterface, which doesn't seem very nice.

[Sergey Ulanov, 2016/01/08 23:39:56]

The VideoCapturer passed to the VideoTrack is created in
WebrtcVideoStream::Start(), so it's always WebrtcVideoCapturerAdapter and the
upcast is always safe. This is not implementation detail of
VideoCapturerAdapter.

Anyway, I agree it's ugly. Replaced this with a WeakPtr<> for
WebrtcVideoCapturerAdapter.

[Jamie, 2016/01/08 23:44:45]

It's conceivable that video_track_ might wrap the capturer though, so while I
don't think it's very likely, it's still a potential source of tricky future
bugs.

WeakPtr is much nicer though, thanks for changing it.

+  DCHECK(capturer_adapter);
+  return capturer_adapter;
 }
 
 }  // namespace protocol