Cast:Transport: Dividing A/V Initialization pipeline

chrome/browser/media/cast_transport_host_filter.cc

Index: chrome/browser/media/cast_transport_host_filter.cc
diff --git a/chrome/browser/media/cast_transport_host_filter.cc b/chrome/browser/media/cast_transport_host_filter.cc
index dda0c50a654f4b21cf187fa818c77f9a85afa9fb..be101fbc988b6ebcc6ab1fd6f75d404db2fc9b67 100644
--- a/chrome/browser/media/cast_transport_host_filter.cc
+++ b/chrome/browser/media/cast_transport_host_filter.cc
@@ -8,7 +8,8 @@
 namespace cast {
 
 CastTransportHostFilter::CastTransportHostFilter()
-    : BrowserMessageFilter(CastMsgStart) {
+    : BrowserMessageFilter(CastMsgStart),
+      weak_factory_(this) {
 }
 
 CastTransportHostFilter::~CastTransportHostFilter() {
@@ -51,46 +52,32 @@ void CastTransportHostFilter::ReceivedRtpStatistics(
     const media::cast::transport::RtcpSenderInfo& sender_info,
     base::TimeTicks time_sent,
     uint32 rtp_timestamp) {
-  Send(new CastMsg_RtpStatistics(channel_id,
-                                 audio,
-                                 sender_info,
-                                 time_sent,
-                                 rtp_timestamp));
+  Send(new CastMsg_RtpStatistics(
+      channel_id, audio, sender_info, time_sent, rtp_timestamp));
 }
 
-void CastTransportHostFilter::OnNew(
-    int32 channel_id,
-    const media::cast::transport::CastTransportConfig& config) {
-  media::cast::transport::CastTransportSender* sender =
-      id_map_.Lookup(channel_id);
-  if (sender) {
+void CastTransportHostFilter::OnNew(int32 channel_id,
+                                    const net::IPEndPoint& local_end_point,
+                                    const net::IPEndPoint& remote_end_point) {
+  if (id_map_.Lookup(channel_id)) {
     id_map_.Remove(channel_id);
   }
 
-  sender = media::cast::transport::CastTransportSender::
-      CreateCastTransportSender(
+  scoped_ptr<media::cast::transport::CastTransportSender> sender =
+      media::cast::transport::CastTransportSender::Create(
           &clock_,
-          config,
+          local_end_point,
+          remote_end_point,
           base::Bind(&CastTransportHostFilter::NotifyStatusChange,
-                     base::Unretained(this),
+                     weak_factory_.GetWeakPtr(),

[hubbe, 2014/02/21 23:41:23]

There should be no need for weak PTRs here as this object destroys all
transports when it is destroyed, and there should be no more callbacks after
that.

If there is something subtle her that I'm missing, we really need to document it
somewhere in some comments.

[mikhal1, 2014/02/24 17:02:50]

Done.

                      channel_id),
           base::MessageLoopProxy::current());
-  sender->SetPacketReceiver(
-          base::Bind(&CastTransportHostFilter::ReceivedPacket,
-                     base::Unretained(this),
-                     channel_id));
-  sender->SubscribeAudioRtpStatsCallback(
-          base::Bind(&CastTransportHostFilter::ReceivedRtpStatistics,
-                     base::Unretained(this),
-                     channel_id,
-                     true  /* audio */));
-  sender->SubscribeVideoRtpStatsCallback(
-          base::Bind(&CastTransportHostFilter::ReceivedRtpStatistics,
-                     base::Unretained(this),
-                     channel_id,
-                     false /* not audio */));
-
-  id_map_.AddWithID(sender, channel_id);
+
+  sender->SetPacketReceiver(base::Bind(&CastTransportHostFilter::ReceivedPacket,
+                                       weak_factory_.GetWeakPtr(),
+                                       channel_id));
+
+  id_map_.AddWithID(sender.release(), channel_id);
 }
 
 void CastTransportHostFilter::OnDelete(int32 channel_id) {
@@ -104,6 +91,42 @@ void CastTransportHostFilter::OnDelete(int32 channel_id) {
   }
 }
 
+void CastTransportHostFilter::OnInitializeAudio(
+    int32 channel_id,
+    const media::cast::transport::CastTransportAudioConfig& config) {
+  media::cast::transport::CastTransportSender* sender =
+      id_map_.Lookup(channel_id);
+  if (sender) {
+    sender->InitializeAudio(config);
+    sender->SubscribeAudioRtpStatsCallback(
+        base::Bind(&CastTransportHostFilter::ReceivedRtpStatistics,
+                   weak_factory_.GetWeakPtr(),
+                   channel_id,
+                   true /* audio */));
+  } else {
+    DVLOG(1)
+        << "CastTransportHostFilter::OnInitializeAudio on non-existing channel";
+  }
+}
+
+void CastTransportHostFilter::OnInitializeVideo(
+    int32 channel_id,
+    const media::cast::transport::CastTransportVideoConfig& config) {
+  media::cast::transport::CastTransportSender* sender =
+      id_map_.Lookup(channel_id);
+  if (sender) {
+    sender->InitializeVideo(config);
+    sender->SubscribeVideoRtpStatsCallback(
+        base::Bind(&CastTransportHostFilter::ReceivedRtpStatistics,
+                   weak_factory_.GetWeakPtr(),
+                   channel_id,
+                   false /* not audio */));
+  } else {
+    DVLOG(1)
+        << "CastTransportHostFilter::OnInitializeVideo on non-existing channel";
+  }
+}
+
 void CastTransportHostFilter::OnInsertCodedAudioFrame(
     int32 channel_id,
     const media::cast::transport::EncodedAudioFrame& audio_frame,