Preparations for Cast Host Extension

remoting/host/client_session.cc

Index: remoting/host/client_session.cc
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 55e4decc1ca3bb70e29df4836cd787e0f1fbed1c..03a2289968941d7dc82c2af002ec7ccad3610d83 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -67,7 +67,8 @@ ClientSession::ClientSession(
       video_encode_task_runner_(video_encode_task_runner),
       network_task_runner_(network_task_runner),
       ui_task_runner_(ui_task_runner),
-      pairing_registry_(pairing_registry) {
+      pairing_registry_(pairing_registry),
+      enable_cast_(false) {
   connection_->SetEventHandler(this);
 
   // TODO(sergeyu): Currently ConnectionToClient expects stubs to be
@@ -148,6 +149,11 @@ void ClientSession::NotifyClientResolution(
 void ClientSession::ControlVideo(const protocol::VideoControl& video_control) {
   DCHECK(CalledOnValidThread());
 
+  // If cast is enabled, do nothing.

[Sergey Ulanov, 2014/07/17 19:00:43]

This doesn't look right. if enable_cast_ is true it doesn't mean that the host
is always casting. I think it should indicate that the host supports casting
when the client supports it and has it enabled.

[Wez, 2014/07/17 22:56:36]

My understanding is that |enable_cast_| means "Output video to Cast, don't
output video to the Chromoting client at all" - if |enable_cast_| is true then
we don't even create the VideoScheduler, below, it seems.

[aiguha, 2014/07/18 21:30:37]

Thanks. Both of your comments helped me catch a bad assumption I was making,
namely that if the flag was passed to the host then we're definitely casting. It
should really mean, as Sergey said, that the host supports it and can switch to
it if the client asks.

I've rewired some things and now, the ClientSession uses default video unless
the client requests cast mode. The switching is now at a per-session basis. Wez
and I discussed that this would be good enough for now. Sergey?

+  if (enable_cast_) {
+    return;
+  }
+
   if (video_control.has_enable()) {
     VLOG(1) << "Received VideoControl (enable="
             << video_control.enable() << ")";
@@ -310,6 +316,11 @@ void ClientSession::OnConnectionAuthenticated(
   // Create the event executor.
   input_injector_ = desktop_environment_->CreateInputInjector();
 
+  // Create the screen capturer.
+  // TODO(aiguha): Implement dynamic sharing of the screen capturer between
+  // VideoScheduler and the CastExtension/CastVideoCapturer.
+  screen_capturer_ = desktop_environment_->CreateVideoCapturer();
+
   // Connect the host clipboard and input stubs.
   host_input_filter_.set_input_stub(input_injector_.get());
   clipboard_echo_filter_.set_host_stub(input_injector_.get());
@@ -319,15 +330,18 @@ void ClientSession::OnConnectionAuthenticated(
       CreateVideoEncoder(connection_->session()->config());
 
   // Create a VideoScheduler to pump frames from the capturer to the client.
-  video_scheduler_ = new VideoScheduler(
+  // If cast mode is enabled, do not create it since screen capturer sharing
+  // has not yet been implemented.
+  if (!enable_cast_) {

[Sergey Ulanov, 2014/07/17 19:00:43]

Same as above - you still want VideoScheduler to work if the client doesn't
support casting or has it disabled.

[aiguha, 2014/07/18 21:30:37]

Done.

+    video_scheduler_ = new VideoScheduler(
       video_capture_task_runner_,
       video_encode_task_runner_,
       network_task_runner_,
-      desktop_environment_->CreateVideoCapturer(),
+      screen_capturer_.Pass(),
       video_encoder.Pass(),
       connection_->client_stub(),
       &mouse_clamping_filter_);
-
+  }
   // Create an AudioScheduler if audio is enabled, to pump audio samples.
   if (connection_->session()->config().is_audio_enabled()) {
     scoped_ptr<AudioEncoder> audio_encoder =
@@ -363,8 +377,10 @@ void ClientSession::OnConnectionChannelsConnected(
   input_injector_->Start(CreateClipboardProxy());
   SetDisableInputs(false);
 
-  // Start capturing the screen.
-  video_scheduler_->Start();
+  // Start capturing the screen, if cast is not enabled.
+  if (!enable_cast_) {
+    video_scheduler_->Start();

[Wez, 2014/07/17 22:56:36]

nit: Suggest "if (video_scheduler_)" instead

[aiguha, 2014/07/18 21:30:37]

As discussed above, this is now (as originally) always started.

+  }
 
   // Start recording audio.
   if (connection_->session()->config().is_audio_enabled())