Cast:Transport: Dividing A/V Initialization pipeline

chrome/renderer/media/cast_session_delegate.cc

 // Copyright 2013 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 "chrome/renderer/media/cast_session_delegate.h"
 
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "chrome/renderer/media/cast_transport_sender_ipc.h"
 #include "content/public/renderer/p2p_socket_client.h"
 #include "content/public/renderer/render_thread.h"
 #include "media/cast/cast_config.h"
 #include "media/cast/cast_environment.h"
 #include "media/cast/cast_sender.h"
 #include "media/cast/logging/logging_defines.h"
 #include "media/cast/transport/cast_transport_config.h"
 #include "media/cast/transport/cast_transport_sender.h"
 
 using media::cast::AudioSenderConfig;
 using media::cast::CastEnvironment;
 using media::cast::CastSender;
 using media::cast::VideoSenderConfig;
 
 CastSessionDelegate::CastSessionDelegate()
     : audio_encode_thread_("CastAudioEncodeThread"),
       video_encode_thread_("CastVideoEncodeThread"),
       transport_configured_(false),
       io_message_loop_proxy_(
-          content::RenderThread::Get()->GetIOMessageLoopProxy()) {
+          content::RenderThread::Get()->GetIOMessageLoopProxy()),
+      weak_factory_(this) {
   DCHECK(io_message_loop_proxy_);
 }
 
 CastSessionDelegate::~CastSessionDelegate() {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 }
 
 void CastSessionDelegate::Initialize() {
   if (cast_environment_)
     return;  // Already initialized.
@@ skipping 58 matching lines @@
   // No transport, wait.
   if (!transport_configured_)
     return;
 
   // No audio or video, wait.
   if (!audio_config_ && !video_config_)
     return;
 
   Initialize();
 
-  media::cast::transport::CastTransportConfig config;
+  media::cast::transport::CastTransportAudioConfig audio_config;
+  media::cast::transport::CastTransportVideoConfig video_config;

[Ami GONE FROM CHROMIUM, 2014/02/25 00:22:17]

move decl's into respective code blocks where they are used below

[mikhal1, 2014/02/25 18:07:02]

irrelevant
On 2014/02/25 00:22:17, Ami Fischman wrote:

[mikhal1, 2014/02/25 18:07:02]

Done.

 
   // TODO(hubbe): set config.aes_key and config.aes_iv_mask.
-  config.local_endpoint = local_endpoint_;
-  config.receiver_endpoint = remote_endpoint_;
+  cast_transport_.reset(new CastTransportSenderIPC(
+      local_endpoint_,
+      remote_endpoint_,
+      base::Bind(&CastSessionDelegate::StatusNotificationCB,
+                 weak_factory_.GetWeakPtr())));
   if (audio_config_) {
-    config.audio_ssrc = audio_config_->sender_ssrc;
-    config.audio_codec = audio_config_->codec;
-    config.audio_rtp_config = audio_config_->rtp_config;
-    config.audio_frequency = audio_config_->frequency;
-    config.audio_channels = audio_config_->channels;
+    audio_config.base.ssrc = audio_config_->sender_ssrc;

[Ami GONE FROM CHROMIUM, 2014/02/25 00:22:17]

would it make more sense to have CastTransport{Audio,Video}Config ctors that
accept an {Audio,Video}SenderConfig?
Or simply have Initialize{Audio,Video} accept {Audio,Video}SenderConfig ?

[mikhal1, 2014/02/25 18:07:02]

Initially I thought it would, but in second thought, I'm not sure about it.
There are values like the encryption that aren't in the cast_sender config, and
the regular config has a lot of settings that aren't included here. Added a todo
for now.

On 2014/02/25 00:22:17, Ami Fischman wrote:

+    audio_config.codec = audio_config_->codec;
+    audio_config.base.rtp_config = audio_config_->rtp_config;
+    audio_config.frequency = audio_config_->frequency;
+    audio_config.channels = audio_config_->channels;
+    cast_transport_->InitializeAudio(audio_config);
   }
   if (video_config_) {
-    config.video_ssrc = video_config_->sender_ssrc;
-    config.video_codec = video_config_->codec;
-    config.video_rtp_config = video_config_->rtp_config;
+    video_config.base.ssrc = video_config_->sender_ssrc;
+    video_config.codec = video_config_->codec;
+    video_config.base.rtp_config = video_config_->rtp_config;
+    cast_transport_->InitializeVideo(video_config);
   }
 
-  cast_transport_.reset(new CastTransportSenderIPC(
-      config,
-      base::Bind(&CastSessionDelegate::StatusNotificationCB,
-                 base::Unretained(this))));
-
   cast_sender_.reset(CastSender::CreateCastSender(
       cast_environment_,
       audio_config_.get(),
       video_config_.get(),
       NULL,  // GPU.
       base::Bind(&CastSessionDelegate::InitializationResult,
-                 base::Unretained(this)),
+                 weak_factory_.GetWeakPtr()),
       cast_transport_.get()));
   cast_transport_->SetPacketReceiver(cast_sender_->packet_receiver());
 }
 
 void CastSessionDelegate::InitializationResult(
     media::cast::CastInitializationStatus result) const {
   DCHECK(cast_sender_);
 
   // TODO(pwestin): handle the error codes.
   if (result == media::cast::STATUS_INITIALIZED) {
     if (!audio_frame_input_available_callback_.is_null()) {
       audio_frame_input_available_callback_.Run(cast_sender_->frame_input());
     }
     if (!video_frame_input_available_callback_.is_null()) {
       video_frame_input_available_callback_.Run(cast_sender_->frame_input());
     }
   }
 }