Bug 1344970 - rename mozRtt to roundTripTime draft
authorNico Grunbaum
Mon, 06 Mar 2017 15:50:10 -0800
changeset 556412 cd1073f4c2e63d28320895667fca16741de0a45d
parent 556407 c2bb2f9f7dc3413071a91c2bc7b9eaf6be1d09f5
child 622878 9a3196c9ca946077de9898afbc19aba1d33c51de
push id52537
push userna-g@nostrum.com
push dateWed, 05 Apr 2017 20:16:46 +0000
bugs1344970
milestone55.0a1
Bug 1344970 - rename mozRtt to roundTripTime MozReview-Commit-ID: 3kES8JUPd3n
dom/media/tests/mochitest/pc.js
dom/media/tests/mochitest/test_peerConnection_stats.html
dom/media/webrtc/WebrtcGlobal.h
dom/webidl/RTCStatsReport.webidl
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
toolkit/content/aboutwebrtc/aboutWebrtc.js
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -1643,19 +1643,22 @@ PeerConnectionWrapper.prototype = {
 		 } else {
                   info("REVERSED timestamps: rec:" +
 		     rem.packetsReceived + " time:" + rem.timestamp + " sent:" + res.packetsSent + " time:" + res.timestamp);
 		 }
 		// Else we may have received more than outdated Rtcp packetsSent
                 ok(rem.bytesReceived <= res.bytesSent, "No more than sent bytes");
               }
               ok(rem.jitter !== undefined, "Rtcp jitter");
-              ok(rem.mozRtt !== undefined, "Rtcp rtt");
-              ok(rem.mozRtt >= 0, "Rtcp rtt " + rem.mozRtt + " >= 0");
-              ok(rem.mozRtt < 60000, "Rtcp rtt " + rem.mozRtt + " < 1 min");
+              if (rem.roundTripTime) {
+                ok(rem.roundTripTime > 0,
+                   "Rtcp rtt " + rem.roundTripTime + " >= 0");
+                ok(rem.roundTripTime < 60000,
+                   "Rtcp rtt " + rem.roundTripTime + " < 1 min");
+              }
             } else {
               ok(rem.type == "outbound-rtp", "Rtcp is outbound");
               ok(rem.packetsSent !== undefined, "Rtcp packetsSent");
               // We may have received more than outdated Rtcp packetsSent
               ok(rem.bytesSent >= rem.packetsSent, "Rtcp bytesSent");
             }
             ok(rem.ssrc == res.ssrc, "Remote ssrc match");
           } else {
--- a/dom/media/tests/mochitest/test_peerConnection_stats.html
+++ b/dom/media/tests/mochitest/test_peerConnection_stats.html
@@ -9,34 +9,36 @@
   createHTML({
     bug: "1337525",
     title: "webRtc Stats composition and sanity"
   });
 var statsExpectedByType = {
   "inbound-rtp": {
     expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType",
       "packetsReceived", "packetsLost", "bytesReceived", "jitter",],
-    optional: ["mozRtt", "remoteId", "nackCount",],
+    optional: ["roundTripTime", "remoteId", "nackCount",],
     localVideoOnly: ["discardedPackets", "framerateStdDev", "framerateMean",
       "bitrateMean", "bitrateStdDev", "firCount", "pliCount",],
     unimplemented: ["mediaTrackId", "transportId", "codecId", "framesDecoded",
       "packetsDiscarded", "associateStatsId",
       "sliCount", "qpSum", "packetsRepaired", "fractionLost",
       "burstPacketsLost", "burstLossCount", "burstDiscardCount",
       "gapDiscardRate", "gapLossRate",],
+    deprecated: ["mozRtt"],
   },
   "outbound-rtp": {
     expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType",
       "packetsSent", "bytesSent", "remoteId",],
     optional: ["remoteId", "nackCount",],
     localVideoOnly: ["droppedFrames", "bitrateMean", "bitrateStdDev",
       "framerateMean", "framerateStdDev", "framesEncoded", "firCount",
       "pliCount",],
     unimplemented: ["mediaTrackId", "transportId", "codecId",
-      "sliCount", "qpSum", "roundTripTime", "targetBitrate",],
+      "sliCount", "qpSum", "targetBitrate",],
+    deprecated: [],
   },
   "codec": { skip: true },
   "peer-connection": { skip: true },
   "data-channel": { skip: true },
   "track": { skip: true },
   "transport": { skip: true },
   "candidate-pair": { skip : true },
   "local-candidate": { skip: true },
@@ -76,16 +78,24 @@ var checkExpectedFields = report => repo
   // Ensure that unimplemented fields are not implemented
   //   note: if a field is implemented it should be moved to expected or
   //   optional.
   //
   expectations.unimplemented.forEach(field => {
     ok(!Object.keys(stat).includes(field), "Unimplemented field " + stat.type
       + "." + field + " does not exist.");
   });
+
+  //
+  // Ensure that all deprecated fields are not present
+  //
+  expectations.deprecated.forEach(field => {
+    ok(!Object.keys(stat).includes(field), "Deprecated field " + stat.type
+      + "." + field + " does not exist.");
+  });
 });
 
 var pedanticChecks = report => {
   report.forEach((statObj, mapKey) => {
     let tested = {};
     // Record what fields get tested.
     // To access a field foo without marking it as tested use stat.inner.foo
     let stat = new Proxy(statObj, {
@@ -196,22 +206,23 @@ var pedanticChecks = report => {
       //     stat.type + ".packetsDiscarded is only set when isRemote is "
       //     + "false");
       // }
 
       //
       // Optional fields
       //
 
-      // mozRtt
+      // roundTripTime
       if (stat.inner.isRemote) {
-        ok(stat.mozRtt >= 0, stat.type + ".mozRtt is sane.");
+        ok(stat.roundTripTime >= 0, stat.type + ".roundTripTime is sane with" +
+          "value of:" + stat.roundTripTime);
       } else {
-        is(stat.mozRtt, undefined, stat.type
-          + ".mozRtt is only set when isRemote is true");
+        is(stat.roundTripTime, undefined, stat.type
+          + ".roundTripTime is only set when isRemote is true");
       }
 
       //
       // Local video only stats
       //
       if (stat.inner.isRemote || stat.inner.mediaType != "video") {
         expectations.localVideoOnly.forEach(field => {
           if (stat.inner.isRemote) {
@@ -357,50 +368,56 @@ var pedanticChecks = report => {
       ok(Object.keys(tested).includes(field), stat.type + "." + field
         + " was tested.");
     });
   });
 }
 
 // This MUST be run after PC_*_WAIT_FOR_MEDIA_FLOW to ensure that we have RTP
 // before checking for RTCP.
-var waitForRtcp = async pc => {
+var waitForSyncedRtcp = async pc => {
   // Ensures that RTCP is present
-  let ensureRtcp = async () => pc.getStats().then(stats => {
+  let ensureSyncedRtcp = async () => {
+    let stats = await pc.getStats();
     for (let [k, v] of stats) {
       if (v.type.endsWith("bound-rtp") && !v.remoteId) {
         throw new Error(v.id + " is missing remoteId: "
           + JSON.stringify(v));
       }
+      if (v.type == "inbound-rtp" && v.isRemote == true
+          && v.roundTripTime === undefined) {
+        throw new Error(v.id + " is missing roundTripTime: "
+          + JSON.stringify(v));
+      }
     }
     return stats;
-  });
-
+  }
   const waitPeriod = 500;
-  for (let totalTime = 10000; totalTime > 0; totalTime -= waitPeriod) {
+  const maxTime = 15000;
+  for (let totalTime = maxTime; totalTime > 0; totalTime -= waitPeriod) {
     try {
-      return await ensureRtcp();
+      return await ensureSyncedRtcp();
     } catch (e) {
       info(e);
       await wait(waitPeriod);
     }
   }
-  throw new Error("Waiting for RTCP timed out after at least " + totalTime
+  throw new Error("Waiting for synced RTCP timed out after at least " + maxTime
     + "ms");
 }
 
 var PC_LOCAL_TEST_LOCAL_STATS = test => {
-  return waitForRtcp(test.pcLocal).then(stats => {
+  return waitForSyncedRtcp(test.pcLocal).then(stats => {
     checkExpectedFields(stats);
     pedanticChecks(stats);
   });
 }
 
 var PC_REMOTE_TEST_REMOTE_STATS = test => {
-  return waitForRtcp(test.pcRemote).then(stats => {
+  return waitForSyncedRtcp(test.pcRemote).then(stats => {
     checkExpectedFields(stats);
     pedanticChecks(stats);
   });
 }
 
 var test;
 runNetworkTest(function (options) {
   test = new PeerConnectionTest(options);
--- a/dom/media/webrtc/WebrtcGlobal.h
+++ b/dom/media/webrtc/WebrtcGlobal.h
@@ -339,31 +339,31 @@ struct ParamTraits<mozilla::dom::RTCInbo
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     WriteParam(aMsg, aParam.mBytesReceived);
     WriteParam(aMsg, aParam.mDiscardedPackets);
     WriteParam(aMsg, aParam.mJitter);
     WriteParam(aMsg, aParam.mMozAvSyncDelay);
     WriteParam(aMsg, aParam.mMozJitterBufferDelay);
-    WriteParam(aMsg, aParam.mMozRtt);
+    WriteParam(aMsg, aParam.mRoundTripTime);
     WriteParam(aMsg, aParam.mPacketsLost);
     WriteParam(aMsg, aParam.mPacketsReceived);
     WriteRTCRTPStreamStats(aMsg, aParam);
     WriteRTCStats(aMsg, aParam);
   }
 
   static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
   {
     if (!ReadParam(aMsg, aIter, &(aResult->mBytesReceived)) ||
         !ReadParam(aMsg, aIter, &(aResult->mDiscardedPackets)) ||
         !ReadParam(aMsg, aIter, &(aResult->mJitter)) ||
         !ReadParam(aMsg, aIter, &(aResult->mMozAvSyncDelay)) ||
         !ReadParam(aMsg, aIter, &(aResult->mMozJitterBufferDelay)) ||
-        !ReadParam(aMsg, aIter, &(aResult->mMozRtt)) ||
+        !ReadParam(aMsg, aIter, &(aResult->mRoundTripTime)) ||
         !ReadParam(aMsg, aIter, &(aResult->mPacketsLost)) ||
         !ReadParam(aMsg, aIter, &(aResult->mPacketsReceived)) ||
         !ReadRTCRTPStreamStats(aMsg, aIter, aResult) ||
         !ReadRTCStats(aMsg, aIter, aResult)) {
       return false;
     }
 
     return true;
--- a/dom/webidl/RTCStatsReport.webidl
+++ b/dom/webidl/RTCStatsReport.webidl
@@ -37,29 +37,29 @@ dictionary RTCRTPStreamStats : RTCStats 
   // Video encoder/decoder measurements, not present in RTCP case
   double bitrateMean;
   double bitrateStdDev;
   double framerateMean;
   double framerateStdDev;
 
   // Local only measurements, RTCP related but not communicated via RTCP. Not
   // present in RTCP case.
-  unsigned long firCount;  
+  unsigned long firCount;
   unsigned long pliCount;
   unsigned long nackCount;
 };
 
 dictionary RTCInboundRTPStreamStats : RTCRTPStreamStats {
   unsigned long packetsReceived;
   unsigned long long bytesReceived;
   double jitter;
   unsigned long packetsLost;
   long mozAvSyncDelay;
   long mozJitterBufferDelay;
-  long mozRtt;
+  long roundTripTime;
 
   // Video decoder measurement, not present in RTCP case
   unsigned long discardedPackets;
 };
 
 dictionary RTCOutboundRTPStreamStats : RTCRTPStreamStats {
   unsigned long packetsSent;
   unsigned long long bytesSent;
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -218,16 +218,19 @@ bool WebrtcAudioConduit::GetRTCPReceiver
   uint16_t fractionLost;
   bool result = !mPtrRTP->GetRemoteRTCPReceiverInfo(mChannel, ntpHigh, ntpLow,
                                                     *packetsReceived,
                                                     *bytesReceived,
                                                     *jitterMs,
                                                     fractionLost,
                                                     *cumulativeLost,
                                                     *rttMs);
+  // Note: rrtMs is 0 when unavailable before the VoE rework. It is likely
+  // that after the audio moves to the new Call API that rttMs will be -1
+  // when unavailable.
   if (!result) {
     return false;
   }
   // Note: timestamp is not correct per the spec... should be time the rtcp
   // was received (remote) or sent (local)
   *timestamp = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds();
   return true;
 }
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -871,29 +871,29 @@ bool WebrtcVideoConduit::GetRTCPReceiver
         "%s for VideoConduit:%p ssrc not found in SendStream stats.",
         __FUNCTION__, this);
       return false;
     }
     *jitterMs = ind->second.rtcp_stats.jitter;
     *cumulativeLost = ind->second.rtcp_stats.cumulative_lost;
     *bytesReceived = ind->second.rtp_stats.MediaPayloadBytes();
     *packetsReceived = ind->second.rtp_stats.transmitted.packets;
-    int64_t rtt = mSendStream->GetRtt(); // TODO: BUG 1241066, mozRtt is 0 or 1
-    if (rtt >= 0) {
-      *rttMs = rtt;
-    } else {
-      *rttMs = 0;
-    }
+    int64_t rtt = mSendStream->GetRtt();
 #ifdef DEBUG
     if (rtt > INT32_MAX) {
       CSFLogError(logTag,
         "%s for VideoConduit:%p mRecvStream->GetRtt() is larger than the"
         " maximum size of an RTCP RTT.", __FUNCTION__, this);
     }
 #endif
+    if (rtt > 0) {
+      *rttMs = rtt;
+    } else {
+      *rttMs = 0;
+    }
     // Note: timestamp is not correct per the spec... should be time the rtcp
     // was received (remote) or sent (local)
     *timestamp = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds();
   }
   return true;
 }
 
 bool
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
@@ -262,21 +262,21 @@ EverySecondTelemetryCallback_s(nsAutoPtr
             id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_JITTER :
                            WEBRTC_VIDEO_QUALITY_OUTBOUND_JITTER;
           } else {
             id = isAudio ? WEBRTC_AUDIO_QUALITY_INBOUND_JITTER :
                            WEBRTC_VIDEO_QUALITY_INBOUND_JITTER;
           }
           Accumulate(id, s.mJitter.Value());
         }
-        if (s.mMozRtt.WasPassed()) {
+        if (s.mRoundTripTime.WasPassed()) {
           MOZ_ASSERT(s.mIsRemote);
           HistogramID id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_RTT :
                                      WEBRTC_VIDEO_QUALITY_OUTBOUND_RTT;
-          Accumulate(id, s.mMozRtt.Value());
+          Accumulate(id, s.mRoundTripTime.Value());
         }
         if (lastInboundStats && s.mBytesReceived.WasPassed()) {
           auto& laststats = *lastInboundStats;
           auto i = FindId(laststats, s.mId.Value());
           if (i != laststats.NoIndex) {
             auto& lasts = laststats[i];
             if (lasts.mBytesReceived.WasPassed()) {
               auto delta_ms = int32_t(s.mTimestamp.Value() -
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -3635,17 +3635,19 @@ PeerConnectionImpl::ExecuteStatsQuery_s(
             }
             s.mMediaType.Construct(mediaType);
             s.mJitter.Construct(double(jitterMs)/1000);
             s.mRemoteId.Construct(localId);
             s.mIsRemote = true;
             s.mPacketsReceived.Construct(packetsReceived);
             s.mBytesReceived.Construct(bytesReceived);
             s.mPacketsLost.Construct(packetsLost);
-            s.mMozRtt.Construct(rtt);
+            if (rtt > 0) {
+              s.mRoundTripTime.Construct(rtt);
+            }
             query->report->mInboundRTPStreamStats.Value().AppendElement(s,
                                                                         fallible);
           }
         }
         // Then, fill in local side (with cross-link to remote only if present)
         {
           RTCOutboundRTPStreamStats s;
           s.mTimestamp.Construct(query->now);
--- a/toolkit/content/aboutwebrtc/aboutWebrtc.js
+++ b/toolkit/content/aboutwebrtc/aboutWebrtc.js
@@ -607,18 +607,18 @@ RTPStats.prototype = {
       statsString += ` ${getString("received_label")}: ${stats.packetsReceived} ${getString("packets")}`;
 
       if (stats.bytesReceived) {
         statsString += ` (${(stats.bytesReceived / 1024).toFixed(2)} Kb)`;
       }
 
       statsString += ` ${getString("lost_label")}: ${stats.packetsLost} ${getString("jitter_label")}: ${stats.jitter}`;
 
-      if (stats.mozRtt) {
-        statsString += ` RTT: ${stats.mozRtt} ms`;
+      if (stats.roundTripTime) {
+        statsString += ` RTT: ${stats.roundTripTime} ms`;
       }
     } else if (stats.packetsSent) {
       statsString += ` ${getString("sent_label")}: ${stats.packetsSent} ${getString("packets")}`;
       if (stats.bytesSent) {
         statsString += ` (${(stats.bytesSent / 1024).toFixed(2)} Kb)`;
       }
     }