Simplify interface for media/cast: CastTransportSenderImpl

media/cast/net/cast_transport_sender_impl.h

Index: media/cast/net/cast_transport_sender_impl.h
diff --git a/media/cast/net/cast_transport_sender_impl.h b/media/cast/net/cast_transport_sender_impl.h
index 09b42999175cbaedad7b09515d9ba2485ae75397..0cf991b2e1397408c86a0ddb13a592f0cae6920d 100644
--- a/media/cast/net/cast_transport_sender_impl.h
+++ b/media/cast/net/cast_transport_sender_impl.h
@@ -52,43 +52,8 @@ class UdpTransport;
 class CastTransportSenderImpl : public CastTransportSender {
  public:
   // |external_transport| is only used for testing.
-  // |raw_events_callback|: Raw events will be returned on this callback
-  // which will be invoked every |raw_events_callback_interval|.
-  // This can be a null callback, i.e. if user is not interested in raw events.
-  // |raw_events_callback_interval|: This can be |base::TimeDelta()| if
-  // |raw_events_callback| is a null callback.
-  // |options| contains optional settings for the transport, possible
-  // keys are:
-  //   "DSCP" (value ignored)
-  //       - Turns DSCP on (higher IP Precedence and Type of Service).
-  //   "disable_non_blocking_io" (value ignored)
-  //       - Windows only.  Turns off non-blocking IO for the socket.
-  //         Note: Non-blocking IO is, by default, enabled on all platforms.
-  //   "pacer_target_burst_size": int
-  //        - Specifies how many packets to send per 10 ms ideally.
-  //   "pacer_max_burst_size": int
-  //        - Specifies how many pakcets to send per 10 ms, maximum.
-  //   "send_buffer_min_size": int
-  //        - Specifies the minimum socket send buffer size.
-  //   "disable_wifi_scan" (value ignored)
-  //        - Disable wifi scans while streaming.
-  //   "media_streaming_mode" (value ignored)
-  //        - Turn media streaming mode on.
-  // Note, these options may be ignored on some platforms.
-  // TODO(hubbe): Too many callbacks, replace with an interface.
-  // http://crbug.com/557477
-  CastTransportSenderImpl(
-      net::NetLog* net_log,
-      base::TickClock* clock,
-      const net::IPEndPoint& local_end_point,
-      const net::IPEndPoint& remote_end_point,
-      scoped_ptr<base::DictionaryValue> options,
-      const CastTransportStatusCallback& status_callback,
-      const BulkRawEventsCallback& raw_events_callback,
-      base::TimeDelta raw_events_callback_interval,
-      const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner,
-      const PacketReceiverCallback& packet_callback,
-      PacketSender* external_transport);
+  CastTransportSenderImpl(const CreateParams& params,
+                          PacketSender* external_transport);
 
   ~CastTransportSenderImpl() final;
 
@@ -140,8 +105,9 @@ class CastTransportSenderImpl : public CastTransportSender {
                      bool cancel_rtx_if_not_in_list,
                      const DedupInfo& dedup_info);
 
-  // If |raw_events_callback_| is non-null, calls it with events collected
-  // in |recent_frame_events_| and |recent_packet_events_| since last call.
+  // If |logging_flush_interval| is not |base::TimeDelta|, calls it with events

[imcheng, 2015/12/17 00:18:11]

s/base::TimeDelta|base::TimeDelta()

[xjz, 2015/12/17 22:54:58]

Done.

+  // collected in |logging_flush_interval| and |recent_packet_events_| since
+  // last call.
   void SendRawEvents();
 
   // Called when a packet is received.
@@ -156,12 +122,17 @@ class CastTransportSenderImpl : public CastTransportSender {
                              const RtcpCastMessageCallback& cast_message_cb,
                              const RtcpCastMessage& cast_message);
 
+  // Used as status callback to create udp transport.
+  void OnStatusChange(CastTransportStatus status);
+
+  const CreateParams& transport_params_;

[Irfan, 2015/12/16 22:37:04]

If we only pass raw pointer on client, we can get rid of these and track a
scoped_ptr on the Client

[imcheng, 2015/12/17 00:18:11]

Also, this shouldn't be a reference.

[xjz, 2015/12/17 22:54:58]

Removed.

[xjz, 2015/12/17 22:54:58]

I think we could do this, but might not be a good way. If for some reson this
pointer is not successfully parsed to the scoped_ptr, this pointer will not be
deleted. I prefer take it out of the CreateParams structure and pass it
separately as Derek suggested.

   base::TickClock* clock_;  // Not owned by this class.
-  CastTransportStatusCallback status_callback_;
-  scoped_refptr<base::SingleThreadTaskRunner> transport_task_runner_;
+  const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner_;

[imcheng, 2015/12/17 00:18:11]

This shouldn't be a reference.

[xjz, 2015/12/17 22:54:58]

Done.

+  base::TimeDelta logging_flush_interval_;

[imcheng, 2015/12/17 00:18:11]

const base::TimeDelta

[xjz, 2015/12/17 22:54:58]

Done.

 
-  // FrameEvents and PacketEvents pending delivery via |raw_events_callback_|.
-  // Do not add elements to these when |raw_events_callback_.is_null()|.
+  // FrameEvents and PacketEvents pending delivery via raw events callback.
+  // Do not add elements to these when |logging_flush_interval| is
+  // |base::TimeDelta()|.
   std::vector<FrameEvent> recent_frame_events_;
   std::vector<PacketEvent> recent_packet_events_;
 
@@ -169,7 +140,7 @@ class CastTransportSenderImpl : public CastTransportSender {
   scoped_ptr<UdpTransport> transport_;
 
   // Packet sender that performs pacing.
-  PacedSender pacer_;
+  scoped_ptr<PacedSender> pacer_;

[imcheng, 2015/12/17 00:18:11]

Why this change?

[xjz, 2015/12/17 22:54:58]

It cannot be initialized in the initialization list after the interface change.
So I changed it to scoped_ptr to initialize it inside the constructor instead.

 
   // Packetizer for audio and video frames.
   scoped_ptr<RtpSender> audio_sender_;
@@ -186,9 +157,6 @@ class CastTransportSenderImpl : public CastTransportSender {
   TransportEncryptionHandler audio_encryptor_;
   TransportEncryptionHandler video_encryptor_;
 
-  BulkRawEventsCallback raw_events_callback_;
-  base::TimeDelta raw_events_callback_interval_;
-
   // Right after a frame is sent we record the number of bytes sent to the
   // socket. We record the corresponding bytes sent for the most recent ACKed
   // audio packet.
@@ -197,10 +165,6 @@ class CastTransportSenderImpl : public CastTransportSender {
   // Packets that don't match these ssrcs are ignored.
   std::set<uint32> valid_ssrcs_;
 
-  // Called with incoming packets. (Unless they match the
-  // channels created by Initialize{Audio,Video}.
-  PacketReceiverCallback packet_callback_;
-
   scoped_ptr<net::ScopedWifiOptions> wifi_options_autoreset_;
 
   base::WeakPtrFactory<CastTransportSenderImpl> weak_factory_;