Simplify interface for media/cast: CastTransportSenderImpl

chrome/browser/media/cast_transport_host_filter.cc

 // Copyright 2014 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/browser/media/cast_transport_host_filter.h"
 
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/common/cast_messages.h"
 #include "components/net_log/chrome_net_log.h"
 #include "content/public/browser/power_save_blocker.h"
 #include "media/cast/net/cast_transport_sender.h"
 
 namespace {
 
 // How often to send raw events.
 const int kSendRawEventsIntervalSecs = 1;
 
-}
+}  // namespace
 
 namespace cast {
 
 CastTransportHostFilter::CastTransportHostFilter()
     : BrowserMessageFilter(CastMsgStart),
       weak_factory_(this) {}
 
 CastTransportHostFilter::~CastTransportHostFilter() {}
 
 bool CastTransportHostFilter::OnMessageReceived(const IPC::Message& message) {
@@ skipping 12 matching lines @@
                         OnCancelSendingFrames)
     IPC_MESSAGE_HANDLER(CastHostMsg_AddValidSsrc,
                         OnAddValidSsrc)
     IPC_MESSAGE_HANDLER(CastHostMsg_SendRtcpFromRtpReceiver,
                         OnSendRtcpFromRtpReceiver)
     IPC_MESSAGE_UNHANDLED(handled = false);
   IPC_END_MESSAGE_MAP();
   return handled;
 }
 
-void CastTransportHostFilter::ReceivedPacket(
-    int32 channel_id,
-    scoped_ptr<media::cast::Packet> packet) {
-  Send(new CastMsg_ReceivedPacket(channel_id, *packet));
+void CastTransportHostFilter::OnStatusChange(
+    media::cast::CastTransportStatus status) {
+  Send(new CastMsg_NotifyStatusChange(channel_id_, status));
 }
 
-void CastTransportHostFilter::NotifyStatusChange(
-    int32 channel_id,
-    media::cast::CastTransportStatus status) {
-  Send(new CastMsg_NotifyStatusChange(channel_id, status));
-}
-
-void CastTransportHostFilter::SendRawEvents(
-    int32 channel_id,
+void CastTransportHostFilter::OnReceivedLoggingEvents(
     scoped_ptr<std::vector<media::cast::FrameEvent>> frame_events,
     scoped_ptr<std::vector<media::cast::PacketEvent>> packet_events) {
   if (frame_events->empty() && packet_events->empty())
     return;
-  Send(new CastMsg_RawEvents(channel_id, *packet_events, *frame_events));
+  Send(new CastMsg_RawEvents(channel_id_, *packet_events, *frame_events));
+}
+
+void CastTransportHostFilter::OnReceivedPackets(
+    scoped_ptr<media::cast::Packet> packet) {
+  Send(new CastMsg_ReceivedPacket(channel_id_, *packet));
 }
 
 void CastTransportHostFilter::SendRtt(int32 channel_id,
                                       uint32 ssrc,
                                       base::TimeDelta rtt) {
   Send(new CastMsg_Rtt(channel_id, ssrc, rtt));
 }
 
 void CastTransportHostFilter::SendCastMessage(
     int32 channel_id,
@@ skipping 14 matching lines @@
         content::PowerSaveBlocker::Create(
             content::PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
             content::PowerSaveBlocker::kReasonOther,
             "Cast is streaming content to a remote receiver").Pass();
   }
 
   if (id_map_.Lookup(channel_id)) {
     id_map_.Remove(channel_id);
   }
 
+  channel_id_ = channel_id;

[Irfan, 2015/12/16 19:44:34]

as we discussed, this creates a problem since we want to be able to manage
several channel ids in CastTransportHostFilter.

Two options:

- Push channel id to CastTransportSender and let it manage it. I agree with you
that channel id is an abstraction that CastTransportSender should probably not
know about.

- Go back to your approach of abstracting channel id in a separate class of its
own and pass ownership to CastTransportSender to manage the life time

[xjz, 2015/12/16 21:58:01]

I prefer the second approach, unless there are reasons to let the sender manager
the channel id. And it is more flexible to have its own class. For example, the
client might want to add some other parameters in the future.

+  media::cast::CastTransportSender::CreateParams transport_params(
+      g_browser_process->net_log(), &clock_, local_end_point, remote_end_point,
+      this, base::TimeDelta::FromSeconds(kSendRawEventsIntervalSecs),
+      make_scoped_ptr(options.DeepCopy()), base::ThreadTaskRunnerHandle::Get());
   scoped_ptr<media::cast::CastTransportSender> sender =
-      media::cast::CastTransportSender::Create(
-          g_browser_process->net_log(),
-          &clock_,
-          local_end_point,
-          remote_end_point,
-          make_scoped_ptr(options.DeepCopy()),
-          base::Bind(&CastTransportHostFilter::NotifyStatusChange,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::Bind(&CastTransportHostFilter::SendRawEvents,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::TimeDelta::FromSeconds(kSendRawEventsIntervalSecs),
-          base::Bind(&CastTransportHostFilter::ReceivedPacket,
-                     weak_factory_.GetWeakPtr(),
-                     channel_id),
-          base::ThreadTaskRunnerHandle::Get());
+      media::cast::CastTransportSender::Create(transport_params);
   id_map_.AddWithID(sender.release(), channel_id);
 }
 
 void CastTransportHostFilter::OnDelete(int32 channel_id) {
   media::cast::CastTransportSender* sender = id_map_.Lookup(channel_id);
   if (sender) {
     id_map_.Remove(channel_id);
   } else {
     DVLOG(1) << "CastTransportHostFilter::Delete called "
              << "on non-existing channel";
@@ skipping 125 matching lines @@
                                     params.rtp_receiver_statistics.get());
   } else {
     DVLOG(1)
         << "CastTransportHostFilter::OnSendRtcpFromRtpReceiver "
         << "on non-existing channel";
   }
 }
 
 
 }  // namespace cast