Simplify interface for media/cast: CastTransportSenderImpl

media/cast/net/cast_transport_sender.h

Index: media/cast/net/cast_transport_sender.h
diff --git a/media/cast/net/cast_transport_sender.h b/media/cast/net/cast_transport_sender.h
index b20c9df0488a06dc0ae5f4529abde83ae0431961..95aa77959d0b4584ad0f0fdb84db5a88fce0abfc 100644
--- a/media/cast/net/cast_transport_sender.h
+++ b/media/cast/net/cast_transport_sender.h
@@ -56,17 +56,66 @@ typedef base::Callback<void(scoped_ptr<std::vector<FrameEvent>>,
 // The application should only trigger this class from the transport thread.
 class CastTransportSender : public base::NonThreadSafe {
  public:
-  static scoped_ptr<CastTransportSender> Create(
-      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 PacketReceiverCallback& packet_callback,
-      const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner);
+  // Interface to create a cast transport sender / receiver.

[Irfan, 2015/12/15 22:23:13]

Perhaps the name of the class should be CastTransportEndPoint

[xjz, 2015/12/16 18:11:34]

Rename it to CastTransportClient.

+  class Client {
+   public:
+    virtual void OnStatusChange(CastTransportStatus status) = 0;

[Irfan, 2015/12/15 22:23:13]

Comment suggestion: "Audio and video transport status change is reported on this
callback."

[xjz, 2015/12/16 18:11:34]

Done.

+    // Raw events call back. This will be involked every

[Irfan, 2015/12/15 22:23:13]

s/involked/invoked

Suggestion: "Raw events will be invoked on this callback every
|logging_flush_interval|. If the user is not interested in raw events,
|logging_flush_interval| should be set to |base::TimeDelta|."

[xjz, 2015/12/16 18:11:34]

Done.

+    // |logging_flush_interval|. It could be no-op function if user is not
+    // interested in raw events. |logging_flush_interval| should be set to
+    // |base::TimeDelta| in this case.
+    virtual void OnReceivedLoggingEvents(
+        scoped_ptr<std::vector<FrameEvent>>,
+        scoped_ptr<std::vector<PacketEvent>>) = 0;
+    // Return false if user is provided a no-op function.

[Irfan, 2015/12/15 22:23:13]

It appears the return status is only used for logging "stale packet received" in
the code and that log detail itself appears to be incorrect ?

Think this should be void

[xjz, 2015/12/16 18:11:34]

Done.

+    virtual bool OnReceivedPackets(scoped_ptr<Packet> packet) = 0;

[Irfan, 2015/12/15 22:23:13]

There seems to be a comment in the old code related to this somewhere later:

 // Called with incoming packets. (Unless they match the	
 // channels created by Initialize{Audio,Video}.

Not sure I follow the unless part - has that been behavior been captured ?

[xjz, 2015/12/16 18:11:34]

This would only happen on errors. 

+
+   protected:
+    virtual ~Client();
+  };
+
+  // Parameters to create a cast transport sender / receiver.
+  // |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.
+  struct CreateParams {
+    CreateParams(
+        net::NetLog* log,
+        base::TickClock* input_clock,
+        net::IPEndPoint local_addr,
+        net::IPEndPoint remote_addr,
+        Client* transport_client,
+        base::TimeDelta transport_logging_flush_interval,
+        scoped_ptr<base::DictionaryValue> options,
+        const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
+    ~CreateParams();
+
+    net::NetLog* net_log;
+    base::TickClock* clock;
+    net::IPEndPoint local_end_point;
+    net::IPEndPoint remote_end_point;
+    Client* client;
+    base::TimeDelta logging_flush_interval;
+    scoped_ptr<base::DictionaryValue> optional_config;
+    const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner;
+  };
+
+  static scoped_ptr<CastTransportSender> Create(const CreateParams& params);
 
   virtual ~CastTransportSender() {}