rtt calculation handles time go backwards

webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc

Index: webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
index 0259b73ff4e12fd666fda20d4b986df85d9d22bd..639d25441b285fbc1bfc08519520be6d3d769e84 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -13,8 +13,6 @@
 #include <assert.h>
 #include <string.h>
 
-#include <algorithm>
-
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/trace_event.h"
@@ -66,6 +64,7 @@ RTCPReceiver::RTCPReceiver(
       _lastReceivedSRNTPfrac(0),
       _lastReceivedXRNTPsecs(0),
       _lastReceivedXRNTPfrac(0),
+      xr_rrtr_status_(false),
       xr_rr_rtt_ms_(0),
       _receivedInfoMap(),
       _lastReceivedRrMs(0),
@@ -191,6 +190,10 @@ int32_t RTCPReceiver::RTT(uint32_t remoteSSRC,
   return 0;
 }
 
+void RTCPReceiver::SetRtcpXrRrtrStatus(bool enable) {

[åsapersson, 2016/03/09 09:00:13]

should xr_rrtr_status_ be GUARDED_BY(_criticalSectionRTCPReceiver)?

[danilchap, 2016/03/09 09:52:53]

Done. Do not think it should, but it doesn't hurt either. Better be safe.

+  xr_rrtr_status_ = enable;
+}
+
 bool RTCPReceiver::GetAndResetXrRrRtt(int64_t* rtt_ms) {
   assert(rtt_ms);
   CriticalSectionScoped lock(_criticalSectionRTCPReceiver);
@@ -520,10 +523,13 @@ void RTCPReceiver::HandleReportBlock(
     reportBlock->remoteMaxJitter = rtcpPacket.ReportBlockItem.Jitter;
   }
 
+  int64_t rtt = 0;
   uint32_t send_time = rtcpPacket.ReportBlockItem.LastSR;
-  uint32_t rtt = 0;
-
-  if (send_time > 0) {
+  // RFC3550, section 6.4.1, LSR field discription states:
+  // If no SR has been received yet, the field is set to zero.
+  // Receiver rtp_rtcp module is not expected to calculate rtt using
+  // Sender Reports even if it accidentely can.

[åsapersson, 2016/03/09 09:00:13]

nit: accidentally

[danilchap, 2016/03/09 09:52:53]

Done.

+  if (!receiver_only_ && send_time != 0) {
     uint32_t delay = rtcpPacket.ReportBlockItem.DelayLastSR;
     // Local NTP time.
     uint32_t receive_time = CompactNtp(NtpTime(*_clock));
@@ -531,8 +537,7 @@ void RTCPReceiver::HandleReportBlock(
     // RTT in 1/(2^16) seconds.
     uint32_t rtt_ntp = receive_time - delay - send_time;
     // Convert to 1/1000 seconds (milliseconds).
-    uint32_t rtt_ms = CompactNtpIntervalToMs(rtt_ntp);
-    rtt = std::max<uint32_t>(rtt_ms, 1);
+    rtt = CompactNtpRttToMs(rtt_ntp);
     if (rtt > reportBlock->maxRTT) {
       // Store max RTT.
       reportBlock->maxRTT = rtt;
@@ -916,9 +921,13 @@ void RTCPReceiver::HandleXrDlrrReportBlockItem(
 
   rtcpPacketInformation.xr_dlrr_item = true;
 
+  // Caller should explicetely enable rtt calculation using extended reports.

[åsapersson, 2016/03/09 09:00:13]

nit: explicitly

[danilchap, 2016/03/09 09:52:53]

Done.

+  if (!xr_rrtr_status_)

[åsapersson, 2016/03/09 09:00:13]

move check to line 921

[danilchap, 2016/03/09 09:52:53]

Would rather not in this CL:
xr_dlrr_item flag is used in tests only, and the only impact of moving check
before setting the flag is breaking some tests. (Tests explicitly expect this
flag to be set even when rtt can't be calculated)

+    return;
+
   // The send_time and delay_rr fields are in units of 1/2^16 sec.
   uint32_t send_time = packet.XRDLRRReportBlockItem.LastRR;
-  // RFC3411, section 4.5, LRR field discription states:
+  // RFC3611, section 4.5, LRR field discription states:
   // If no such block has been received, the field is set to zero.
   if (send_time == 0)
     return;
@@ -927,8 +936,7 @@ void RTCPReceiver::HandleXrDlrrReportBlockItem(
   uint32_t now = CompactNtp(NtpTime(*_clock));
 
   uint32_t rtt_ntp = now - delay_rr - send_time;
-  uint32_t rtt_ms = CompactNtpIntervalToMs(rtt_ntp);
-  xr_rr_rtt_ms_ = std::max<uint32_t>(rtt_ms, 1);
+  xr_rr_rtt_ms_ = CompactNtpRttToMs(rtt_ntp);
 
   rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpXrDlrrReportBlock;
 }