Bug 1541553 - allow RTCRtpReceiverStats roundTripTime of 0 after non-zero values have been reported r=bwc
authorNico Grunbaum <na-g@nostrum.com>
Fri, 05 Apr 2019 19:40:04 +0000
changeset 530122 8e81194c97ea42470bbf14a846b215101b4b5946
parent 530121 675ac787a5d3982388309ab83585858f0aacfb9b
child 530123 eea220d8fd4c656997171fd24f9d29b4988aeb6a
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1541553
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1541553 - allow RTCRtpReceiverStats roundTripTime of 0 after non-zero values have been reported r=bwc roundTripTime can be zero once it has been non-zero this should help with intermitents in WPT and mochitests Differential Revision: https://phabricator.services.mozilla.com/D25981
dom/media/tests/mochitest/pc.js
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.h
media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.h
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -1648,32 +1648,31 @@ PeerConnectionWrapper.prototype = {
           info(`${v.id} is missing roundTripTime: ${JSON.stringify(v)}`);
           return null;
         }
       }
       return report;
     }
     let attempts = 0;
     // Time-units are MS
-    const waitPeriod = 500;
+    const waitPeriod = 100;
     const maxTime = 20000;
     for (let totalTime = maxTime; totalTime > 0; totalTime -= waitPeriod) {
       try {
         let syncedStats = await ensureSyncedRtcp();
         if (syncedStats) {
           return syncedStats;
         }
       } catch (e) {
           info(e);
           info(e.stack);
           throw e;
       }
       attempts += 1;
-      info("waitForSyncedRtcp: no synced RTCP on attempt" + attempts
-           + ", retrying.\n");
+      info(`waitForSyncedRtcp: no sync on attempt ${attempts}, retrying.`);
       await wait(waitPeriod);
     }
     throw Error("Waiting for synced RTCP timed out after at least "
                 + maxTime + "ms");
   },
 
   /**
    * Check that correct audio (typically a flat tone) is flowing to this
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -187,43 +187,51 @@ bool WebrtcAudioConduit::GetRTPReceiverS
   *cumulativeLost = stats.packets_lost;
   return true;
 }
 
 bool WebrtcAudioConduit::GetRTCPReceiverReport(uint32_t* jitterMs,
                                                uint32_t* packetsReceived,
                                                uint64_t* bytesReceived,
                                                uint32_t* cumulativeLost,
-                                               int32_t* rttMs) {
+                                               Maybe<double>* aOutRttSec) {
   ASSERT_ON_THREAD(mStsThread);
   double fractionLost = 0.0;
   int64_t timestampTmp = 0;
   int64_t rttMsTmp = 0;
   bool res = false;
   if (mSendChannelProxy) {
     res = mSendChannelProxy->GetRTCPReceiverStatistics(
         &timestampTmp, jitterMs, cumulativeLost, packetsReceived, bytesReceived,
         &fractionLost, &rttMsTmp);
   }
-  auto stats = mCall->Call()->GetStats();
-  int64_t rtt = stats.rtt_ms;
+
+  const auto stats = mCall->Call()->GetStats();
+  const auto rtt = stats.rtt_ms;
+  if (rtt > static_cast<decltype(stats.rtt_ms)>(INT32_MAX)) {
+    // If we get a bogus RTT we will keep using the previous RTT
 #ifdef DEBUG
-  if (rtt > INT32_MAX) {
     CSFLogError(LOGTAG,
-                "%s for VideoConduit:%p RTT is larger than the"
+                "%s for AudioConduit:%p RTT is larger than the"
                 " maximum size of an RTCP RTT.",
                 __FUNCTION__, this);
+#endif
+  } else {
+    if (mRttSec && rtt < 0) {
+      CSFLogError(LOGTAG,
+                  "%s for AudioConduit:%p RTT returned an error after "
+                  " previously succeeding.",
+                  __FUNCTION__, this);
+      mRttSec = Nothing();
+    }
+    if (rtt >= 0) {
+      mRttSec = Some(static_cast<DOMHighResTimeStamp>(rtt) / 1000.0);
+    }
   }
-#endif
-  if (rtt > 0) {
-    *rttMs = rtt;
-  } else {
-    *rttMs = 0;
-  }
-
+  *aOutRttSec = mRttSec;
   return res;
 }
 
 bool WebrtcAudioConduit::GetRTCPSenderReport(unsigned int* packetsSent,
                                              uint64_t* bytesSent) {
   ASSERT_ON_THREAD(mStsThread);
   if (!mRecvChannelProxy) {
     return false;
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -221,17 +221,17 @@ class WebrtcAudioConduit : public AudioS
 
   bool GetRecvPacketTypeStats(
       webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
 
   bool GetRTPReceiverStats(unsigned int* jitterMs,
                            unsigned int* cumulativeLost) override;
   bool GetRTCPReceiverReport(uint32_t* jitterMs, uint32_t* packetsReceived,
                              uint64_t* bytesReceived, uint32_t* cumulativeLost,
-                             int32_t* rttMs) override;
+                             Maybe<double>* aOutRttSec) override;
   bool GetRTCPSenderReport(unsigned int* packetsSent,
                            uint64_t* bytesSent) override;
 
   bool SetDtmfPayloadType(unsigned char type, int freq) override;
 
   bool InsertDTMFTone(int channel, int eventCode, bool outOfBand, int lengthMs,
                       int attenuationDb) override;
 
@@ -344,13 +344,16 @@ class WebrtcAudioConduit : public AudioS
   // Accessed from audio thread.
   webrtc::AudioFrame mAudioFrame;  // for output pulls
 
   // Accessed from both main and mStsThread. Uses locks internally.
   RtpSourceObserver mRtpSourceObserver;
 
   // Socket transport service thread. Any thread.
   const nsCOMPtr<nsIEventTarget> mStsThread;
+
+  // Accessed from mStsThread. Last successfully polled RTT
+  Maybe<DOMHighResTimeStamp> mRttSec;
 };
 
 }  // namespace mozilla
 
 #endif
--- a/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
+++ b/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
@@ -231,17 +231,17 @@ class MediaSessionConduit {
       webrtc::RtcpPacketTypeCounter* aPacketCounts) = 0;
 
   virtual bool GetRTPReceiverStats(unsigned int* jitterMs,
                                    unsigned int* cumulativeLost) = 0;
   virtual bool GetRTCPReceiverReport(uint32_t* jitterMs,
                                      uint32_t* packetsReceived,
                                      uint64_t* bytesReceived,
                                      uint32_t* cumulativeLost,
-                                     int32_t* rttMs) = 0;
+                                     Maybe<double>* aOutRttMs) = 0;
   virtual bool GetRTCPSenderReport(unsigned int* packetsSent,
                                    uint64_t* bytesSent) = 0;
 
   virtual uint64_t CodecPluginID() = 0;
 
   virtual void SetPCHandle(const std::string& aPCHandle) = 0;
 
   virtual MediaConduitErrorCode DeliverPacket(const void* data, int len) = 0;
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -180,36 +180,44 @@ static MediaConduitErrorCode ValidateCod
 
   return kMediaConduitNoError;
 }
 
 void WebrtcVideoConduit::CallStatistics::Update(
     const webrtc::Call::Stats& aStats) {
   ASSERT_ON_THREAD(mStatsThread);
 
-  int64_t rtt = aStats.rtt_ms;
+  const auto rtt = aStats.rtt_ms;
+  if (rtt > static_cast<decltype(aStats.rtt_ms)>(INT32_MAX)) {
+    // If we get a bogus RTT we will keep using the previous RTT
 #ifdef DEBUG
-  if (rtt > INT32_MAX) {
     CSFLogError(LOGTAG,
                 "%s for VideoConduit:%p RTT is larger than the"
                 " maximum size of an RTCP RTT.",
                 __FUNCTION__, this);
-  }
 #endif
-  if (rtt > 0) {
-    mRttMs = rtt;
+    mRttSec = Nothing();
   } else {
-    mRttMs = 0;
+    if (mRttSec && rtt < 0) {
+      CSFLogError(LOGTAG,
+                  "%s for VideoConduit:%p RTT returned an error after "
+                  " previously succeeding.",
+                  __FUNCTION__, this);
+      mRttSec = Nothing();
+    }
+    if (rtt >= 0) {
+      mRttSec = Some(static_cast<DOMHighResTimeStamp>(rtt) / 1000.0);
+    }
   }
 }
 
-int32_t WebrtcVideoConduit::CallStatistics::RttMs() const {
+Maybe<DOMHighResTimeStamp> WebrtcVideoConduit::CallStatistics::RttSec() const {
   ASSERT_ON_THREAD(mStatsThread);
 
-  return mRttMs;
+  return mRttSec;
 }
 
 void WebrtcVideoConduit::StreamStatistics::Update(
     const double aFrameRate, const double aBitrate,
     const webrtc::RtcpPacketTypeCounter& aPacketCounts) {
   ASSERT_ON_THREAD(mStatsThread);
 
   mFrameRate.Push(aFrameRate);
@@ -1180,31 +1188,32 @@ bool WebrtcVideoConduit::GetRTPReceiverS
   *packetsLost = mRecvStreamStats.PacketsLost();
   return true;
 }
 
 bool WebrtcVideoConduit::GetRTCPReceiverReport(uint32_t* jitterMs,
                                                uint32_t* packetsReceived,
                                                uint64_t* bytesReceived,
                                                uint32_t* cumulativeLost,
-                                               int32_t* rttMs) {
+                                               Maybe<double>* aOutRttSec) {
   ASSERT_ON_THREAD(mStsThread);
 
   CSFLogVerbose(LOGTAG, "%s for VideoConduit:%p", __FUNCTION__, this);
+  aOutRttSec->reset();
   if (!mSendStreamStats.Active()) {
     return false;
   }
   if (!mSendStreamStats.SsrcFound()) {
     return false;
   }
   *jitterMs = mSendStreamStats.JitterMs();
   *packetsReceived = mSendStreamStats.PacketsReceived();
   *bytesReceived = mSendStreamStats.BytesReceived();
   *cumulativeLost = mSendStreamStats.PacketsLost();
-  *rttMs = mCallStats.RttMs();
+  *aOutRttSec = mCallStats.RttSec();
   return true;
 }
 
 bool WebrtcVideoConduit::GetRTCPSenderReport(unsigned int* packetsSent,
                                              uint64_t* bytesSent) {
   ASSERT_ON_THREAD(mStsThread);
 
   CSFLogVerbose(LOGTAG, "%s for VideoConduit:%p", __FUNCTION__, this);
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ -261,17 +261,17 @@ class WebrtcVideoConduit
   bool GetVideoDecoderStats(double* framerateMean, double* framerateStdDev,
                             double* bitrateMean, double* bitrateStdDev,
                             uint32_t* discardedPackets,
                             uint32_t* framesDecoded) override;
   bool GetRTPReceiverStats(unsigned int* jitterMs,
                            unsigned int* cumulativeLost) override;
   bool GetRTCPReceiverReport(uint32_t* jitterMs, uint32_t* packetsReceived,
                              uint64_t* bytesReceived, uint32_t* cumulativeLost,
-                             int32_t* rttMs) override;
+                             Maybe<double>* aOutRttSec) override;
   bool GetRTCPSenderReport(unsigned int* packetsSent,
                            uint64_t* bytesSent) override;
   uint64_t MozVideoLatencyAvg();
 
   void DisableSsrcChanges() override {
     ASSERT_ON_THREAD(mStsThread);
     mAllowSsrcChange = false;
   }
@@ -289,23 +289,23 @@ class WebrtcVideoConduit
    * Statistics for the Call associated with this VideoConduit.
    * Single threaded.
    */
   class CallStatistics {
    public:
     explicit CallStatistics(nsCOMPtr<nsIEventTarget> aStatsThread)
         : mStatsThread(aStatsThread) {}
     void Update(const webrtc::Call::Stats& aStats);
-    int32_t RttMs() const;
+    Maybe<DOMHighResTimeStamp> RttSec() const;
 
    protected:
     const nsCOMPtr<nsIEventTarget> mStatsThread;
 
    private:
-    int32_t mRttMs = 0;
+    Maybe<DOMHighResTimeStamp> mRttSec = Nothing();
   };
 
   /**
    * Shared statistics for receive and transmit video streams.
    * Single threaded.
    */
   class StreamStatistics {
    public:
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -2766,17 +2766,17 @@ RefPtr<RTCStatsQueryPromise> PeerConnect
         {
           // First, fill in remote stat with rtcp receiver data, if present.
           // ReceiverReports have less information than SenderReports,
           // so fill in what we can.
           uint32_t jitterMs;
           uint32_t packetsReceived;
           uint64_t bytesReceived;
           uint32_t packetsLost;
-          int32_t rtt;
+          Maybe<double> rtt;
           if (mp.Conduit()->GetRTCPReceiverReport(&jitterMs, &packetsReceived,
                                                   &bytesReceived, &packetsLost,
                                                   &rtt)) {
             remoteId = NS_LITERAL_STRING("outbound_rtcp_") + idstr;
             RTCRemoteInboundRtpStreamStats s;
             // TODO Bug 1496533 - use reception time not query time
             s.mTimestamp.Construct(query->now);
             s.mId.Construct(remoteId);
@@ -2785,19 +2785,17 @@ RefPtr<RTCStatsQueryPromise> PeerConnect
             s.mMediaType.Construct(
                 kind);  // mediaType is the old name for kind.
             s.mKind.Construct(kind);
             s.mJitter.Construct(double(jitterMs) / 1000);
             s.mLocalId.Construct(localId);
             s.mPacketsReceived.Construct(packetsReceived);
             s.mBytesReceived.Construct(bytesReceived);
             s.mPacketsLost.Construct(packetsLost);
-            if (rtt > 0) {  // RTT is not reported when it is zero
-              s.mRoundTripTime.Construct(static_cast<double>(rtt) / 1000);
-            }
+            rtt.apply([&s](auto r) { s.mRoundTripTime.Construct(r); });
             query->report->mRemoteInboundRtpStreamStats.Value().AppendElement(
                 s, fallible);
           }
         }
         // Then, fill in local side (with cross-link to remote only if present)
         {
           RTCOutboundRtpStreamStats s;
           // TODO Bug 1496533 - use reception time not query time