Cast: Refactor RTCP handling

media/cast/net/rtcp/rtcp.h

Index: media/cast/net/rtcp/rtcp.h
diff --git a/media/cast/net/rtcp/rtcp.h b/media/cast/net/rtcp/rtcp.h
index 813518ecb617e9e012ce21f26d3efba8445239ec..b68af28ea9c8aca712c878d9c3467528ab23e6cd 100644
--- a/media/cast/net/rtcp/rtcp.h
+++ b/media/cast/net/rtcp/rtcp.h
@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// This class maintains a bi-directional RTCP connection with a remote
+// peer.
+
 #ifndef MEDIA_CAST_RTCP_RTCP_H_
 #define MEDIA_CAST_RTCP_RTCP_H_
 
@@ -11,15 +14,14 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "media/cast/cast_config.h"
 #include "media/cast/cast_defines.h"
-#include "media/cast/cast_environment.h"
 #include "media/cast/common/clock_drift_smoother.h"
 #include "media/cast/net/cast_transport_defines.h"
 #include "media/cast/net/cast_transport_sender.h"
-#include "media/cast/net/pacing/paced_sender.h"
 #include "media/cast/net/rtcp/receiver_rtcp_event_subscriber.h"
 #include "media/cast/net/rtcp/rtcp_defines.h"
 
@@ -27,7 +29,6 @@ namespace media {
 namespace cast {
 
 class LocalRtcpReceiverFeedback;
-class LocalRtcpRttFeedback;
 class PacedPacketSender;
 class RtcpReceiver;
 class RtcpSender;
@@ -36,13 +37,6 @@ typedef std::pair<uint32, base::TimeTicks> RtcpSendTimePair;
 typedef std::map<uint32, base::TimeTicks> RtcpSendTimeMap;
 typedef std::queue<RtcpSendTimePair> RtcpSendTimeQueue;
 
-class RtcpSenderFeedback {
- public:
-  virtual void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) = 0;
-
-  virtual ~RtcpSenderFeedback() {}
-};
-
 class RtpReceiverStatistics {
  public:
   virtual void GetStatistics(uint8* fraction_lost,
@@ -53,46 +47,49 @@ class RtpReceiverStatistics {
   virtual ~RtpReceiverStatistics() {}
 };
 
+class RtcpMessageHandlerImpl;

[miu, 2014/07/16 00:09:31]

Any reason why this isn't just an private internal class of Rtcp?  Seems this
would be simpler than forward declaring it here and also "friending" it later.

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

+
+// TODO(hclam): This should be renamed to RtcpSession.
 class Rtcp {
  public:
-  // Rtcp accepts two transports, one to be used by Cast senders
-  // (CastTransportSender) only, and the other (PacedPacketSender) should only
-  // be used by the Cast receivers and test applications.
-  Rtcp(scoped_refptr<CastEnvironment> cast_environment,
-       RtcpSenderFeedback* sender_feedback,
-       CastTransportSender* const transport_sender,  // Send-side.
-       PacedPacketSender* paced_packet_sender,       // Receive side.
-       RtpReceiverStatistics* rtp_receiver_statistics,
-       RtcpMode rtcp_mode,
-       const base::TimeDelta& rtcp_interval,
+  Rtcp(const RtcpCastMessageCallback& cast_callback,
+       const RtcpRttCallback& rtt_callback,
+       const RtcpLogMessageCallback& log_callback,
+       base::TickClock* clock,  // Not owned.
+       PacedPacketSender* packet_sender,  // Not owned.
        uint32 local_ssrc,
        uint32 remote_ssrc,
-       const std::string& c_name,
-       EventMediaType event_media_type);
+       const std::string& c_name);
 
   virtual ~Rtcp();
 
-  static bool IsRtcpPacket(const uint8* rtcp_buffer, size_t length);
-
-  static uint32 GetSsrcOfSender(const uint8* rtcp_buffer, size_t length);
-
-  base::TimeTicks TimeToSendNextRtcpReport();
-
   // Send a RTCP sender report.
   // |current_time| is the current time reported by a tick clock.
   // |current_time_as_rtp_timestamp| is the corresponding RTP timestamp.
-  void SendRtcpFromRtpSender(base::TimeTicks current_time,
-                             uint32 current_time_as_rtp_timestamp);
+  // |send_packet_count| is the number of packets sent.
+  // |send_octet_count| is the number of octets sent.
+  void SendRtcpFromRtpSender(

[miu, 2014/07/16 00:09:31]

ditto re: method rename: SendSenderReport

[Alpha Left Google, 2014/07/17 01:01:45]

I'd like to do this in a later change. This is so that SenderRtcpFromRtp* looks
consistent in this class.

+      base::TimeTicks current_time,
+      uint32 current_time_as_rtp_timestamp,
+      uint32 send_packet_count,
+      size_t send_octet_count);
 
   // |cast_message| and |rtcp_events| is optional; if |cast_message| is
   // provided the RTCP receiver report will append a Cast message containing
-  // Acks and Nacks; if |rtcp_events| is provided the RTCP receiver report
-  // will append the log messages.
+  // Acks and Nacks; |target_delay| is sent together with |cast_message|.
+  // If |rtcp_events| is provided the RTCP receiver report will append the
+  // log messages.
   void SendRtcpFromRtpReceiver(
       const RtcpCastMessage* cast_message,
-      const ReceiverRtcpEventSubscriber::RtcpEventMultiMap* rtcp_events);
+      base::TimeDelta target_delay,
+      const ReceiverRtcpEventSubscriber::RtcpEventMultiMap* rtcp_events,
+      RtpReceiverStatistics* rtp_receiver_statistics);
 
-  void IncomingRtcpPacket(const uint8* rtcp_buffer, size_t length);
+  // Submit a received packet to this object. The packet will be parsed
+  // and uesd to maintain a RTCP session.

[miu, 2014/07/16 00:09:31]

s/uesd/used/

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

+  // Returns false if this is not a RTCP packet or it is not directed to
+  // this session, e.g. SSRC doesn't match.
+  bool IncomingRtcpPacket(const uint8* data, size_t length);
 
   // TODO(miu): Clean up this method and downstream code: Only VideoSender uses
   // this (for congestion control), and only the |rtt| and |avg_rtt| values, and
@@ -103,8 +100,6 @@ class Rtcp {
            base::TimeDelta* min_rtt,
            base::TimeDelta* max_rtt) const;
 
-  bool is_rtt_available() const { return number_of_rtt_in_avg_ > 0; }
-
   // If available, returns true and sets the output arguments to the latest
   // lip-sync timestamps gleaned from the sender reports.  While the sender
   // provides reference NTP times relative to its own wall clock, the
@@ -113,15 +108,6 @@ class Rtcp {
   bool GetLatestLipSyncTimes(uint32* rtp_timestamp,
                              base::TimeTicks* reference_time) const;
 
-  // Set the history size to record Cast receiver events. The event history is
-  // used to remove duplicates. The history will store at most |size| events.
-  void SetCastReceiverEventHistorySize(size_t size);
-
-  // Update the target delay. Will be added to every report sent back to the
-  // sender.
-  // TODO(miu): Remove this deprecated functionality. The sender ignores this.
-  void SetTargetDelay(base::TimeDelta target_delay);
-
   void OnReceivedReceiverLog(const RtcpReceiverLogMessage& receiver_log);
 
  protected:
@@ -131,42 +117,32 @@ class Rtcp {
                              uint32 ntp_fraction);
 
  private:
-  friend class LocalRtcpRttFeedback;
-  friend class LocalRtcpReceiverFeedback;
+  friend class RtcpMessageHandlerImpl;

[miu, 2014/07/16 00:09:31]

s/friend //  (see comment above)

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

 
-  void OnReceivedDelaySinceLastReport(uint32 receivers_ssrc,
-                                      uint32 last_report,
+  void OnReceivedDelaySinceLastReport(uint32 last_report,
                                       uint32 delay_since_last_report);
 
-  void OnReceivedSendReportRequest();
+  void OnReceivedCastFeedback(const RtcpCastMessage& cast_message);
 
   void UpdateRtt(const base::TimeDelta& sender_delay,
                  const base::TimeDelta& receiver_delay);
 
-  void UpdateNextTimeToSendRtcp();
-
   void SaveLastSentNtpTime(const base::TimeTicks& now,
                            uint32 last_ntp_seconds,
                            uint32 last_ntp_fraction);
 
-  scoped_refptr<CastEnvironment> cast_environment_;
-  CastTransportSender* const transport_sender_;
-  const base::TimeDelta rtcp_interval_;
-  const RtcpMode rtcp_mode_;
+  base::TickClock* clock_;  // Not owned by this class.

[miu, 2014/07/16 00:09:31]

nit: base::TickClock* const clock_;

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

+  const RtcpCastMessageCallback cast_callback_;
+  const RtcpRttCallback rtt_callback_;
+  const RtcpLogMessageCallback log_callback_;
   const uint32 local_ssrc_;
   const uint32 remote_ssrc_;
   const std::string c_name_;
-  const EventMediaType event_media_type_;
-
-  // Not owned by this class.
-  RtpReceiverStatistics* const rtp_receiver_statistics_;
 
-  scoped_ptr<LocalRtcpRttFeedback> rtt_feedback_;
-  scoped_ptr<LocalRtcpReceiverFeedback> receiver_feedback_;
+  scoped_ptr<RtcpMessageHandlerImpl> handler_;

[miu, 2014/07/16 00:09:31]

nit: const scoped_ptr<RtcpMessageHandlerImpl> handler_;

   scoped_ptr<RtcpSender> rtcp_sender_;

[miu, 2014/07/16 00:09:31]

nit: const

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

   scoped_ptr<RtcpReceiver> rtcp_receiver_;
 
-  base::TimeTicks next_time_to_send_rtcp_;
   RtcpSendTimeMap last_reports_sent_map_;
   RtcpSendTimeQueue last_reports_sent_queue_;
 
@@ -195,7 +171,6 @@ class Rtcp {
   base::TimeDelta max_rtt_;
   int number_of_rtt_in_avg_;
   base::TimeDelta avg_rtt_;
-  base::TimeDelta target_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(Rtcp);
 };