Pass VideoRenderer interface to ConnectionToHost.

remoting/protocol/ice_connection_to_host.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/ice_connection_to_host.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "remoting/base/constants.h"
 #include "remoting/protocol/audio_reader.h"
 #include "remoting/protocol/audio_stub.h"
 #include "remoting/protocol/auth_util.h"
 #include "remoting/protocol/client_control_dispatcher.h"
 #include "remoting/protocol/client_event_dispatcher.h"
 #include "remoting/protocol/client_stub.h"
 #include "remoting/protocol/client_video_dispatcher.h"
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/errors.h"
 #include "remoting/protocol/ice_transport.h"
 #include "remoting/protocol/transport_context.h"
-#include "remoting/protocol/video_stub.h"
+#include "remoting/protocol/video_renderer.h"
 
 namespace remoting {
 namespace protocol {
 
 IceConnectionToHost::IceConnectionToHost() {}
 IceConnectionToHost::~IceConnectionToHost() {}
 
 void IceConnectionToHost::Connect(
     scoped_ptr<Session> session,
     scoped_refptr<TransportContext> transport_context,
@@ skipping 31 matching lines @@
 }
 
 void IceConnectionToHost::set_client_stub(ClientStub* client_stub) {
   client_stub_ = client_stub;
 }
 
 void IceConnectionToHost::set_clipboard_stub(ClipboardStub* clipboard_stub) {
   clipboard_stub_ = clipboard_stub;
 }
 
-void IceConnectionToHost::set_video_stub(VideoStub* video_stub) {
-  DCHECK(video_stub);
-  monitored_video_stub_.reset(new MonitoredVideoStub(
-      video_stub, base::TimeDelta::FromSeconds(
-                      MonitoredVideoStub::kConnectivityCheckDelaySeconds),
-      base::Bind(&IceConnectionToHost::OnVideoChannelStatus,
-                 base::Unretained(this))));

[Jamie, 2016/01/04 22:41:15]

I think it would be clearer to keep initialization of monitored_video_stub_
here, since it's tied to the video_renderer_. What's the benefit of moving it to
OnSessionStateChange(AUTHENTICATED)?

[Sergey Ulanov, 2016/01/04 23:35:22]

There are no very significant benefits, but I think it's cleaner this way:
 - it keeps all video renderer initialization logic in one place.
 - MonitoredVideoStub starts "monitoring" the video stream when it's created,
but the stream cannot be active before connection is authenticated.
 - this is a lower-case function, so it's not supposed to do anything complex
such as memory allocation.
 - This doesn't matter right now, but potentially an implementation of
VideoRenderer interface may require that GetVideoStub() is called only after
OnSessionConfig(), e.g. to create different stabs depending on session
configuration.

[Jamie, 2016/01/04 23:40:49]

Maybe a DCHECK that MonitoredVideoStub is null in that case (I think it will do
the wrong thing otherwise).

[Sergey Ulanov, 2016/01/05 18:03:03]

Done.

+void IceConnectionToHost::set_video_renderer(VideoRenderer* video_renderer) {
+  DCHECK(video_renderer);
+  video_renderer_ = video_renderer;
 }
 
 void IceConnectionToHost::set_audio_stub(AudioStub* audio_stub) {
   audio_stub_ = audio_stub;
 }
 
 void IceConnectionToHost::OnSessionStateChange(Session::State state) {
   DCHECK(CalledOnValidThread());
   DCHECK(event_callback_);
 
   switch (state) {
     case Session::INITIALIZING:
     case Session::CONNECTING:
     case Session::ACCEPTING:
     case Session::ACCEPTED:
     case Session::AUTHENTICATING:
       // Don't care about these events.
       break;
 
     case Session::AUTHENTICATED:
       SetState(AUTHENTICATED, OK);
+
+      // Setup control channel.
       control_dispatcher_.reset(new ClientControlDispatcher());
       control_dispatcher_->Init(transport_->GetMultiplexedChannelFactory(),
                                 this);
       control_dispatcher_->set_client_stub(client_stub_);
       control_dispatcher_->set_clipboard_stub(clipboard_stub_);
 
+      // Setup event channel.
       event_dispatcher_.reset(new ClientEventDispatcher());
       event_dispatcher_->Init(transport_->GetMultiplexedChannelFactory(), this);
 
+      // Configure video pipeline.
+      video_renderer_->OnSessionConfig(session_->config());
+      monitored_video_stub_.reset(new MonitoredVideoStub(
+          video_renderer_->GetVideoStub(),
+          base::TimeDelta::FromSeconds(
+              MonitoredVideoStub::kConnectivityCheckDelaySeconds),
+          base::Bind(&IceConnectionToHost::OnVideoChannelStatus,
+                     base::Unretained(this))));
       video_dispatcher_.reset(
           new ClientVideoDispatcher(monitored_video_stub_.get()));
       video_dispatcher_->Init(transport_->GetStreamChannelFactory(), this);
 
+      // Configure audio pipeline if necessary.
       if (session_->config().is_audio_enabled()) {
         audio_reader_.reset(new AudioReader(audio_stub_));
         audio_reader_->Init(transport_->GetMultiplexedChannelFactory(), this);
       }
       break;
 
     case Session::CLOSED:
       CloseChannels();
       SetState(CLOSED, OK);
       break;
@@ skipping 89 matching lines @@
 
   if (state != state_) {
     state_ = state;
     error_ = error;
     event_callback_->OnConnectionState(state_, error_);
   }
 }
 
 }  // namespace protocol
 }  // namespace remoting