[Cast] RTT clean-up to the max!

Executing some very badly needed clean-up of code that handles passing
around round trip time measurements and managing stats.  This change
removes a lot of dead code, addresses TODOs for minor things that were
dependent on this clean-up, and enhances RTCP unit testing.

BUG=404813

Review URL: https://codereview.chromium.org/532373003

Cr-Commit-Position: refs/heads/master@{#293887}

chrome/browser/media/cast_transport_host_filter.cc

@@ -61,16 +61,8 @@ void CastTransportHostFilter::SendRawEvents(
 
 void CastTransportHostFilter::SendRtt(int32 channel_id,
                                       uint32 ssrc,
-                                      base::TimeDelta rtt,
-                                      base::TimeDelta avg_rtt,
-                                      base::TimeDelta min_rtt,
-                                      base::TimeDelta max_rtt) {
-  media::cast::RtcpRttReport report;
-  report.rtt = rtt;
-  report.avg_rtt = avg_rtt;
-  report.min_rtt = min_rtt;
-  report.max_rtt = max_rtt;
-  Send(new CastMsg_Rtt(channel_id, ssrc, report));
+                                      base::TimeDelta rtt) {
+  Send(new CastMsg_Rtt(channel_id, ssrc, rtt));
 }
 
 void CastTransportHostFilter::SendCastMessage(

chrome/browser/media/cast_transport_host_filter.h

@@ -34,12 +34,7 @@ class CastTransportHostFilter : public content::BrowserMessageFilter {
       int32 channel_id,
       const std::vector<media::cast::PacketEvent>& packet_events,
       const std::vector<media::cast::FrameEvent>& frame_events);
-  void SendRtt(int32 channel_id,
-               uint32 ssrc,
-               base::TimeDelta rtt,
-               base::TimeDelta avg_rtt,
-               base::TimeDelta min_rtt,
-               base::TimeDelta max_rtt);
+  void SendRtt(int32 channel_id, uint32 ssrc, base::TimeDelta rtt);
   void SendCastMessage(int32 channel_id,
                        uint32 ssrc,
                        const media::cast::RtcpCastMessage& cast_message);

chrome/common/cast_messages.h

@@ -82,19 +82,12 @@ IPC_STRUCT_TRAITS_BEGIN(media::cast::RtcpCastMessage)
   IPC_STRUCT_TRAITS_MEMBER(missing_frames_and_packets)
 IPC_STRUCT_TRAITS_END()
 
-IPC_STRUCT_TRAITS_BEGIN(media::cast::RtcpRttReport)
-  IPC_STRUCT_TRAITS_MEMBER(rtt)
-  IPC_STRUCT_TRAITS_MEMBER(avg_rtt)
-  IPC_STRUCT_TRAITS_MEMBER(min_rtt)
-  IPC_STRUCT_TRAITS_MEMBER(max_rtt)
-IPC_STRUCT_TRAITS_END()
-
 // Cast messages sent from the browser to the renderer.
 
 IPC_MESSAGE_CONTROL3(CastMsg_Rtt,
                      int32 /* channel_id */,
                      uint32 /* ssrc */,
-                     media::cast::RtcpRttReport /* rtt_report */)
+                     base::TimeDelta /* rtt */)
 
 IPC_MESSAGE_CONTROL3(CastMsg_RtcpCastMessage,
                      int32 /* channel_id */,

chrome/renderer/media/cast_ipc_dispatcher.cc

@@ -102,10 +102,10 @@ void CastIPCDispatcher::OnRawEvents(
 
 void CastIPCDispatcher::OnRtt(int32 channel_id,
                               uint32 ssrc,
-                              const media::cast::RtcpRttReport& rtt_report) {
+                              base::TimeDelta rtt) {
   CastTransportSenderIPC* sender = id_map_.Lookup(channel_id);
   if (sender) {
-    sender->OnRtt(ssrc, rtt_report);
+    sender->OnRtt(ssrc, rtt);
   } else {
     DVLOG(1) << "CastIPCDispatcher::OnRtt on non-existing channel.";
   }

chrome/renderer/media/cast_ipc_dispatcher.h

@@ -49,9 +49,7 @@ class CastIPCDispatcher : public IPC::MessageFilter {
   void OnRawEvents(int32 channel_id,
                    const std::vector<media::cast::PacketEvent>& packet_events,
                    const std::vector<media::cast::FrameEvent>& frame_events);
-  void OnRtt(int32 channel_id,
-             uint32 ssrc,
-             const media::cast::RtcpRttReport& rtt_report);
+  void OnRtt(int32 channel_id, uint32 ssrc, base::TimeDelta rtt);
   void OnRtcpCastMessage(int32 channel_id,
                          uint32 ssrc,
                          const media::cast::RtcpCastMessage& cast_message);

chrome/renderer/media/cast_transport_sender_ipc.cc

@@ -90,21 +90,14 @@ void CastTransportSenderIPC::OnRawEvents(
   raw_events_callback_.Run(packet_events, frame_events);
 }
 
-void CastTransportSenderIPC::OnRtt(
-    uint32 ssrc,
-    const media::cast::RtcpRttReport& rtt_report) {
+void CastTransportSenderIPC::OnRtt(uint32 ssrc, base::TimeDelta rtt) {
   ClientMap::iterator it = clients_.find(ssrc);
   if (it == clients_.end()) {
     LOG(ERROR) << "Received RTT report from for unknown SSRC: " << ssrc;
     return;
   }
-  if (it->second.rtt_cb.is_null())
-    return;
-  it->second.rtt_cb.Run(
-      rtt_report.rtt,
-      rtt_report.avg_rtt,
-      rtt_report.min_rtt,
-      rtt_report.max_rtt);
+  if (!it->second.rtt_cb.is_null())
+    it->second.rtt_cb.Run(rtt);
 }
 
 void CastTransportSenderIPC::OnRtcpCastMessage(

chrome/renderer/media/cast_transport_sender_ipc.h

@@ -51,7 +51,7 @@ class CastTransportSenderIPC
       media::cast::CastTransportStatus status);
   void OnRawEvents(const std::vector<media::cast::PacketEvent>& packet_events,
                    const std::vector<media::cast::FrameEvent>& frame_events);
-  void OnRtt(uint32 ssrc, const media::cast::RtcpRttReport& rtt_report);
+  void OnRtt(uint32 ssrc, base::TimeDelta rtt);
   void OnRtcpCastMessage(uint32 ssrc,
                          const media::cast::RtcpCastMessage& cast_message);
 

media/cast/cast_defines.h

@@ -32,7 +32,6 @@ const uint32 kStartFrameId = UINT32_C(0xffffffff);
 // can handle wrap around and compare two frame IDs.
 const int kMaxUnackedFrames = 120;
 
-const int kStartRttMs = 20;
 const int64 kCastMessageUpdateIntervalMs = 33;
 const int64 kNackRepeatIntervalMs = 30;
 

media/cast/net/cast_transport_sender_impl.cc

@@ -222,12 +222,14 @@ void CastTransportSenderImpl::ResendFrameForKickstart(uint32 ssrc,
                                                       uint32 frame_id) {
   if (audio_sender_ && ssrc == audio_sender_->ssrc()) {
     DCHECK(audio_rtcp_session_);
-    audio_sender_->ResendFrameForKickstart(frame_id,
-                                           audio_rtcp_session_->rtt());
+    audio_sender_->ResendFrameForKickstart(
+        frame_id,
+        audio_rtcp_session_->current_round_trip_time());
   } else if (video_sender_ && ssrc == video_sender_->ssrc()) {
     DCHECK(video_rtcp_session_);
-    video_sender_->ResendFrameForKickstart(frame_id,
-                                           video_rtcp_session_->rtt());
+    video_sender_->ResendFrameForKickstart(
+        frame_id,
+        video_rtcp_session_->current_round_trip_time());
   } else {
     NOTREACHED() << "Invalid request for kickstart.";
   }
@@ -338,7 +340,7 @@ void CastTransportSenderImpl::OnReceivedCastMessage(
     last_byte_acked_for_audio_ =
         std::max(acked_bytes, last_byte_acked_for_audio_);
   } else if (video_sender_ && video_sender_->ssrc() == ssrc) {
-    dedup_info.resend_interval = video_rtcp_session_->rtt();
+    dedup_info.resend_interval = video_rtcp_session_->current_round_trip_time();
 
     // Only use audio stream to dedup if there is one.
     if (audio_sender_) {

media/cast/net/rtcp/rtcp.cc

@@ -18,7 +18,7 @@ using base::TimeDelta;
 namespace media {
 namespace cast {
 
-static const int32 kMaxRttMs = 10000;  // 10 seconds.
+static const int32 kStatsHistoryWindowMs = 10000;  // 10 seconds.
 // Reject packets that are older than 0.5 seconds older than
 // the newest packet we've seen so far. This protect internal
 // states from crazy routers. (Based on RRTR)
@@ -72,9 +72,7 @@ Rtcp::Rtcp(const RtcpCastMessageCallback& cast_callback,
       last_report_truncated_ntp_(0),
       local_clock_ahead_by_(ClockDriftSmoother::GetDefaultTimeConstant()),
       lip_sync_rtp_timestamp_(0),
-      lip_sync_ntp_timestamp_(0),
-      min_rtt_(TimeDelta::FromMilliseconds(kMaxRttMs)),
-      number_of_rtt_in_avg_(0) {
+      lip_sync_ntp_timestamp_(0) {
 }
 
 Rtcp::~Rtcp() {}
@@ -211,11 +209,9 @@ void Rtcp::SendRtcpFromRtpReceiver(
   if (rtp_receiver_statistics) {
     report_block.remote_ssrc = 0;            // Not needed to set send side.
     report_block.media_ssrc = remote_ssrc_;  // SSRC of the RTP packet sender.
-    if (rtp_receiver_statistics) {
     rtp_receiver_statistics->GetStatistics(
         &report_block.fraction_lost, &report_block.cumulative_lost,
         &report_block.extended_high_sequence_number, &report_block.jitter);
-    }
 
     report_block.last_sr = last_report_truncated_ntp_;
     if (!time_last_report_received_.is_null()) {
@@ -271,8 +267,9 @@ void Rtcp::OnReceivedNtp(uint32 ntp_seconds, uint32 ntp_fraction) {
 
   // TODO(miu): This clock offset calculation does not account for packet
   // transit time over the network.  End2EndTest.EvilNetwork confirms that this
-  // contributes a very significant source of error here.  Fix this along with
-  // the RTT clean-up.
+  // contributes a very significant source of error here.  Determine whether
+  // RTT should be factored-in, and how that changes the rest of the
+  // calculation.
   const base::TimeDelta measured_offset =
       now - ConvertNtpToTimeTicks(ntp_seconds, ntp_fraction);
   local_clock_ahead_by_.Update(now, measured_offset);
@@ -326,8 +323,20 @@ void Rtcp::OnReceivedDelaySinceLastReport(uint32 last_report,
     return;  // Feedback on another report.
   }
 
-  base::TimeDelta sender_delay = clock_->NowTicks() - it->second;
-  UpdateRtt(sender_delay, ConvertFromNtpDiff(delay_since_last_report));
+  const base::TimeDelta sender_delay = clock_->NowTicks() - it->second;
+  const base::TimeDelta receiver_delay =
+      ConvertFromNtpDiff(delay_since_last_report);
+  current_round_trip_time_ = sender_delay - receiver_delay;
+  // If the round trip time was computed as less than 1 ms, assume clock
+  // imprecision by one or both peers caused a bad value to be calculated.
+  // While plenty of networks do easily achieve less than 1 ms round trip time,
+  // such a level of precision cannot be measured with our approach; and 1 ms is
+  // good enough to represent "under 1 ms" for our use cases.
+  current_round_trip_time_ =
+      std::max(current_round_trip_time_, base::TimeDelta::FromMilliseconds(1));
+
+  if (!rtt_callback_.is_null())
+    rtt_callback_.Run(current_round_trip_time_);
 }
 
 void Rtcp::OnReceivedCastFeedback(const RtcpCastMessage& cast_message) {
@@ -348,7 +357,8 @@ void Rtcp::SaveLastSentNtpTime(const base::TimeTicks& now,
   last_reports_sent_map_[last_report] = now;
   last_reports_sent_queue_.push(std::make_pair(last_report, now));
 
-  base::TimeTicks timeout = now - TimeDelta::FromMilliseconds(kMaxRttMs);
+  const base::TimeTicks timeout =
+      now - TimeDelta::FromMilliseconds(kStatsHistoryWindowMs);
 
   // Cleanup old statistics older than |timeout|.
   while (!last_reports_sent_queue_.empty()) {
@@ -362,48 +372,6 @@ void Rtcp::SaveLastSentNtpTime(const base::TimeTicks& now,
   }
 }
 
-void Rtcp::UpdateRtt(const base::TimeDelta& sender_delay,
-                     const base::TimeDelta& receiver_delay) {
-  base::TimeDelta rtt = sender_delay - receiver_delay;
-  // TODO(miu): Find out why this must be >= 1 ms, and remove the fudge if it's
-  // bogus.
-  rtt = std::max(rtt, base::TimeDelta::FromMilliseconds(1));
-  rtt_ = rtt;
-  min_rtt_ = std::min(min_rtt_, rtt);
-  max_rtt_ = std::max(max_rtt_, rtt);
-
-  // TODO(miu): Replace "average for all time" with an EWMA, or suitable
-  // "average over recent past" mechanism.
-  if (number_of_rtt_in_avg_ != 0) {
-    // Integer math equivalent of (ac/(ac+1.0))*avg_rtt_ + (1.0/(ac+1.0))*rtt).
-    // (TimeDelta only supports math with other TimeDeltas and int64s.)
-    avg_rtt_ = (avg_rtt_ * number_of_rtt_in_avg_ + rtt) /
-        (number_of_rtt_in_avg_ + 1);
-  } else {
-    avg_rtt_ = rtt;
-  }
-  number_of_rtt_in_avg_++;
-
-  if (!rtt_callback_.is_null())
-    rtt_callback_.Run(rtt, avg_rtt_, min_rtt_, max_rtt_);
-}
-
-bool Rtcp::Rtt(base::TimeDelta* rtt, base::TimeDelta* avg_rtt,
-               base::TimeDelta* min_rtt, base::TimeDelta* max_rtt) const {
-  DCHECK(rtt) << "Invalid argument";
-  DCHECK(avg_rtt) << "Invalid argument";
-  DCHECK(min_rtt) << "Invalid argument";
-  DCHECK(max_rtt) << "Invalid argument";
-
-  if (number_of_rtt_in_avg_ == 0) return false;
-
-  *rtt = rtt_;
-  *avg_rtt = avg_rtt_;
-  *min_rtt = min_rtt_;
-  *max_rtt = max_rtt_;
-  return true;
-}
-
 void Rtcp::OnReceivedReceiverLog(const RtcpReceiverLogMessage& receiver_log) {
   if (log_callback_.is_null())
     return;

media/cast/net/rtcp/rtcp.h

@@ -89,15 +89,6 @@ class Rtcp {
   // 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
-  // it's not clear that any of the downstream code is doing the right thing
-  // with this data.
-  bool Rtt(base::TimeDelta* rtt,
-           base::TimeDelta* avg_rtt,
-           base::TimeDelta* min_rtt,
-           base::TimeDelta* max_rtt) const;
-
   // 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
@@ -108,9 +99,13 @@ class Rtcp {
 
   void OnReceivedReceiverLog(const RtcpReceiverLogMessage& receiver_log);
 
+  // If greater than zero, this is the last measured network round trip time.
+  base::TimeDelta current_round_trip_time() const {
+    return current_round_trip_time_;
+  }
+
   static bool IsRtcpPacket(const uint8* packet, size_t length);
   static uint32 GetSsrcOfSender(const uint8* rtcp_buffer, size_t length);
-  const base::TimeDelta& rtt() const { return rtt_; }
 
  protected:
   void OnReceivedNtp(uint32 ntp_seconds, uint32 ntp_fraction);
@@ -124,9 +119,6 @@ class Rtcp {
 
   void OnReceivedCastFeedback(const RtcpCastMessage& cast_message);
 
-  void UpdateRtt(const base::TimeDelta& sender_delay,
-                 const base::TimeDelta& receiver_delay);
-
   void SaveLastSentNtpTime(const base::TimeTicks& now,
                            uint32 last_ntp_seconds,
                            uint32 last_ntp_fraction);
@@ -167,11 +159,10 @@ class Rtcp {
   uint32 lip_sync_rtp_timestamp_;
   uint64 lip_sync_ntp_timestamp_;
 
-  base::TimeDelta rtt_;
-  base::TimeDelta min_rtt_;
-  base::TimeDelta max_rtt_;
-  int number_of_rtt_in_avg_;
-  base::TimeDelta avg_rtt_;
+  // The last measured network round trip time.  This is updated with each
+  // sender report --> receiver report round trip.  If this is zero, then the
+  // round trip time has not been measured yet.
+  base::TimeDelta current_round_trip_time_;
 
   base::TimeTicks largest_seen_timestamp_;
 

media/cast/net/rtcp/rtcp_defines.cc

@@ -33,8 +33,5 @@ RtcpReceiverReferenceTimeReport::~RtcpReceiverReferenceTimeReport() {}
 RtcpEvent::RtcpEvent() : type(UNKNOWN), packet_id(0u) {}
 RtcpEvent::~RtcpEvent() {}
 
-RtcpRttReport::RtcpRttReport() {}
-RtcpRttReport::~RtcpRttReport() {}
-
 }  // namespace cast
 }  // namespace media

media/cast/net/rtcp/rtcp_defines.h

@@ -104,21 +104,8 @@ struct RtcpEvent {
   uint16 packet_id;
 };
 
-struct RtcpRttReport {
-  RtcpRttReport();
-  ~RtcpRttReport();
-
-  base::TimeDelta rtt;
-  base::TimeDelta avg_rtt;
-  base::TimeDelta min_rtt;
-  base::TimeDelta max_rtt;
-};
-
 typedef base::Callback<void(const RtcpCastMessage&)> RtcpCastMessageCallback;
-typedef base::Callback<void(base::TimeDelta,
-                            base::TimeDelta,
-                            base::TimeDelta,
-                            base::TimeDelta)> RtcpRttCallback;
+typedef base::Callback<void(base::TimeDelta)> RtcpRttCallback;
 typedef
 base::Callback<void(const RtcpReceiverLogMessage&)> RtcpLogMessageCallback;
 

media/cast/sender/audio_sender.cc

@@ -71,7 +71,8 @@ AudioSender::AudioSender(scoped_refptr<CastEnvironment> cast_environment,
       transport_config,
       base::Bind(&AudioSender::OnReceivedCastFeedback,
                  weak_factory_.GetWeakPtr()),
-      base::Bind(&AudioSender::OnReceivedRtt, weak_factory_.GetWeakPtr()));
+      base::Bind(&AudioSender::OnMeasuredRoundTripTime,
+                 weak_factory_.GetWeakPtr()));
 }
 
 AudioSender::~AudioSender() {}

media/cast/sender/frame_sender.cc

@@ -27,7 +27,6 @@ FrameSender::FrameSender(scoped_refptr<CastEnvironment> cast_environment,
     : cast_environment_(cast_environment),
       transport_sender_(transport_sender),
       ssrc_(ssrc),
-      rtt_available_(false),
       rtcp_interval_(rtcp_interval),
       max_frame_rate_(max_frame_rate),
       frames_in_encoder_(0),
@@ -39,7 +38,9 @@ FrameSender::FrameSender(scoped_refptr<CastEnvironment> cast_environment,
       congestion_control_(congestion_control),
       is_audio_(is_audio),
       weak_factory_(this) {
+  DCHECK(transport_sender_);
   DCHECK_GT(rtp_timebase_, 0);
+  DCHECK(congestion_control_);
   SetTargetPlayoutDelay(playout_delay);
   send_target_playout_delay_ = false;
   memset(frame_rtp_timestamps_, 0, sizeof(frame_rtp_timestamps_));
@@ -88,15 +89,9 @@ void FrameSender::SendRtcpReport(bool schedule_future_reports) {
     ScheduleNextRtcpReport();
 }
 
-void FrameSender::OnReceivedRtt(base::TimeDelta rtt,
-                                base::TimeDelta avg_rtt,
-                                base::TimeDelta min_rtt,
-                                base::TimeDelta max_rtt) {
-  rtt_available_ = true;
-  rtt_ = rtt;
-  avg_rtt_ = avg_rtt;
-  min_rtt_ = min_rtt;
-  max_rtt_ = max_rtt;
+void FrameSender::OnMeasuredRoundTripTime(base::TimeDelta rtt) {
+  DCHECK(rtt > base::TimeDelta());
+  current_round_trip_time_ = rtt;
 }
 
 void FrameSender::SetTargetPlayoutDelay(
@@ -241,22 +236,11 @@ void FrameSender::SendEncodedFrame(
 void FrameSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
 
-  base::TimeDelta rtt;
-  base::TimeDelta avg_rtt;
-  base::TimeDelta min_rtt;
-  base::TimeDelta max_rtt;
-  if (is_rtt_available()) {
-    rtt = rtt_;
-    avg_rtt = avg_rtt_;
-    min_rtt = min_rtt_;
-    max_rtt = max_rtt_;
+  const bool have_valid_rtt = current_round_trip_time_ > base::TimeDelta();
+  if (have_valid_rtt) {
+    congestion_control_->UpdateRtt(current_round_trip_time_);
 
-    congestion_control_->UpdateRtt(rtt);
-
-    // Don't use a RTT lower than our average.
-    rtt = std::max(rtt, avg_rtt);
-
-    // Having the RTT values implies the receiver sent back a receiver report
+    // Having the RTT value implies the receiver sent back a receiver report
     // based on it having received a report from here.  Therefore, ensure this
     // sender stops aggressively sending reports.
     if (num_aggressive_rtcp_reports_sent_ < kNumAggressiveReportsSentAtStart) {
@@ -265,9 +249,6 @@ void FrameSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
       num_aggressive_rtcp_reports_sent_ = kNumAggressiveReportsSentAtStart;
       ScheduleNextRtcpReport();
     }
-  } else {
-    // We have no measured value use default.
-    rtt = base::TimeDelta::FromMilliseconds(kStartRttMs);
   }
 
   if (last_send_time_.is_null())

media/cast/sender/frame_sender.h

@@ -50,12 +50,7 @@ class FrameSender {
   void ScheduleNextRtcpReport();
   void SendRtcpReport(bool schedule_future_reports);
 
-  void OnReceivedRtt(base::TimeDelta rtt,
-                     base::TimeDelta avg_rtt,
-                     base::TimeDelta min_rtt,
-                     base::TimeDelta max_rtt);
-
-  bool is_rtt_available() const { return rtt_available_; }
+  void OnMeasuredRoundTripTime(base::TimeDelta rtt);
 
   const scoped_refptr<CastEnvironment> cast_environment_;
 
@@ -68,13 +63,6 @@ class FrameSender {
 
   const uint32 ssrc_;
 
-  // RTT information from RTCP.
-  bool rtt_available_;
-  base::TimeDelta rtt_;
-  base::TimeDelta avg_rtt_;
-  base::TimeDelta min_rtt_;
-  base::TimeDelta max_rtt_;
-
  protected:
   // Schedule and execute periodic checks for re-sending packets.  If no
   // acknowledgements have been received for "too long," AudioSender will
@@ -175,6 +163,9 @@ class FrameSender {
   base::TimeTicks frame_reference_times_[256];
   RtpTimestamp frame_rtp_timestamps_[256];
 
+  // The most recently measured round trip time.
+  base::TimeDelta current_round_trip_time_;
+
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<FrameSender> weak_factory_;
 

media/cast/sender/video_sender.cc

@@ -77,7 +77,8 @@ VideoSender::VideoSender(
       transport_config,
       base::Bind(&VideoSender::OnReceivedCastFeedback,
                  weak_factory_.GetWeakPtr()),
-      base::Bind(&VideoSender::OnReceivedRtt, weak_factory_.GetWeakPtr()));
+      base::Bind(&VideoSender::OnMeasuredRoundTripTime,
+                 weak_factory_.GetWeakPtr()));
 }
 
 VideoSender::~VideoSender() {