Cast:Transport: Dividing A/V Initialization pipeline

chrome/renderer/media/cast_session_delegate.cc

Index: chrome/renderer/media/cast_session_delegate.cc
diff --git a/chrome/renderer/media/cast_session_delegate.cc b/chrome/renderer/media/cast_session_delegate.cc
index d5133b61cc0259899b38a5d422f96ca694b321a3..463c0bdfa1c9710f3503669730c83e1b01b0d946 100644
--- a/chrome/renderer/media/cast_session_delegate.cc
+++ b/chrome/renderer/media/cast_session_delegate.cc
@@ -26,7 +26,8 @@ CastSessionDelegate::CastSessionDelegate()
       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_);
 }
 
@@ -105,36 +106,37 @@ void CastSessionDelegate::StartSendingInternal() {
 
   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());
 }