[asterisk-commits] russell: branch 1.4 r65842 - /branches/1.4/main/rtp.c

[asterisk-commits at lists.digium.com]

Author: russell
Date: Thu May 24 09:49:05 2007
New Revision: 65842

URL: http://svn.digium.com/view/asterisk?view=rev&rev=65842
Log:
Fix the calculation of the RTT for RTCP.  The previous code would result in
oscillating and incorrect data.  Additionally, the RTT would sometimes report
negative values due to incorrect calculations.
(issue #9601, patch from davetroy)

Modified:
    branches/1.4/main/rtp.c

Modified: branches/1.4/main/rtp.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/rtp.c?view=diff&rev=65842&r1=65841&r2=65842
==============================================================================
--- branches/1.4/main/rtp.c (original)
+++ branches/1.4/main/rtp.c Thu May 24 09:49:05 2007
@@ -831,10 +831,10 @@
 	struct timeval now;
 	unsigned int length;
 	int rc;
-	double rtt = 0;
-	double a;
-	double dlsr;
-	double lsr;
+	double rttsec;
+	uint64_t rtt;
+	unsigned int dlsr;
+	unsigned int lsr;
 	unsigned int msw;
 	unsigned int lsw;
 	unsigned int comp;
@@ -914,26 +914,44 @@
 				break;
 			/* Intentional fall through */
 		case RTCP_PT_RR:
-			/* This is the place to calculate RTT */
 			/* Don't handle multiple reception reports (rc > 1) yet */
+			/* Calculate RTT per RFC */
 			gettimeofday(&now, NULL);
 			timeval2ntp(now, &msw, &lsw);
-			/* Use the one we sent them in our SR instead, rtcp->txlsr could have been rewritten if the dlsr is large */
-			if (ntohl(rtcpheader[i + 4])) { /* We must have the LSR */
+			if (ntohl(rtcpheader[i + 4]) && ntohl(rtcpheader[i + 5])) { /* We must have the LSR && DLSR */
 				comp = ((msw & 0xffff) << 16) | ((lsw & 0xffff0000) >> 16);
-				a = (double)((comp & 0xffff0000) >> 16) + (double)((double)(comp & 0xffff)/1000000.);
-				lsr = (double)((ntohl(rtcpheader[i + 4]) & 0xffff0000) >> 16) + (double)((double)(ntohl(rtcpheader[i + 4]) & 0xffff) / 1000000.);
-				dlsr = (double)(ntohl(rtcpheader[i + 5])/65536.);
-				rtt = a - dlsr - lsr;
-				if (rtt >= 0) {
-					rtp->rtcp->accumulated_transit += rtt;
-					rtp->rtcp->rtt = rtt;
-					if (rtp->rtcp->maxrtt < rtt)
-						rtp->rtcp->maxrtt = rtt;
-					if (rtp->rtcp->minrtt > rtt)
-						rtp->rtcp->minrtt = rtt;
+				lsr = ntohl(rtcpheader[i + 4]);
+				dlsr = ntohl(rtcpheader[i + 5]);
+				rtt = comp - lsr - dlsr;
+
+				/* Convert end to end delay to usec (keeping the calculation in 64bit space)
+				   sess->ee_delay = (eedelay * 1000) / 65536; */
+				if (rtt < 4294) {
+				    rtt = (rtt * 1000000) >> 16;
+				} else {
+				    rtt = (rtt * 1000) >> 16;
+				    rtt *= 1000;
+				}
+				rtt = rtt / 1000.;
+				rttsec = rtt / 1000.;
+
+				if (comp - dlsr >= lsr) {
+					rtp->rtcp->accumulated_transit += rttsec;
+					rtp->rtcp->rtt = rttsec;
+					if (rtp->rtcp->maxrtt<rttsec)
+						rtp->rtcp->maxrtt = rttsec;
+					if (rtp->rtcp->minrtt>rttsec)
+						rtp->rtcp->minrtt = rttsec;
+				} else {
+					ast_verbose("Internal RTCP NTP clock skew detected: "
+							   "lsr=%u, now=%u, dlsr=%u (%d:%03dms), "
+							   "diff=%d",
+							   lsr, comp, dlsr, dlsr / 65536,
+							   (dlsr % 65536) * 1000 / 65536,
+							   dlsr - (comp - lsr));
 				}
 			}
+
 			rtp->rtcp->reported_jitter = ntohl(rtcpheader[i + 3]);
 			rtp->rtcp->reported_lost = ntohl(rtcpheader[i + 1]) & 0xffffff;
 			if (rtcp_debug_test_addr(&sin)) {
@@ -945,7 +963,7 @@
 				ast_verbose("  Last SR(our NTP): %lu.%010lu\n",(unsigned long) ntohl(rtcpheader[i + 4]) >> 16,((unsigned long) ntohl(rtcpheader[i + 4]) << 16) * 4096);
 				ast_verbose("  DLSR: %4.4f (sec)\n",ntohl(rtcpheader[i + 5])/65536.0);
 				if (rtt)
-					ast_verbose("  RTT: %f(sec)\n", rtt);
+					ast_verbose("  RTT: %lu(sec)\n", rtt);
 			}
 			break;
 		case RTCP_PT_FUR: