Bug 1348657 - Part 2 - split send/recv packet counts draft
authorNico Grunbaum
Thu, 30 Mar 2017 10:59:46 -0700
changeset 555754 cfc3a5709f1c6861bd8545e2914f656a9933c003
parent 553341 9b198ac3a28ceaa35712df3d53355ac55e17093c
child 622689 ae2c9d40edd128a219a44d50205082e9862574e0
push id52327
push userna-g@nostrum.com
push dateTue, 04 Apr 2017 19:59:54 +0000
bugs1348657
milestone55.0a1
Bug 1348657 - Part 2 - split send/recv packet counts MozReview-Commit-ID: 2SENT4rGsLX
dom/media/tests/mochitest/test_peerConnection_stats.html
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/test_peerConnection_stats.html
+++ b/dom/media/tests/mochitest/test_peerConnection_stats.html
@@ -10,29 +10,29 @@
     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",],
-    videoOnly: ["discardedPackets", "framerateStdDev", "framerateMean",
+    localVideoOnly: ["discardedPackets", "framerateStdDev", "framerateMean",
       "bitrateMean", "bitrateStdDev", "firCount", "pliCount",],
     unimplemented: ["mediaTrackId", "transportId", "codecId", "framesDecoded",
       "packetsDiscarded", "associateStatsId",
       "sliCount", "qpSum", "packetsRepaired", "fractionLost",
       "burstPacketsLost", "burstLossCount", "burstDiscardCount",
       "gapDiscardRate", "gapLossRate",],
   },
   "outbound-rtp": {
     expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType",
       "packetsSent", "bytesSent", "remoteId",],
     optional: ["remoteId", "nackCount",],
-    videoOnly: ["droppedFrames", "bitrateMean", "bitrateStdDev",
+    localVideoOnly: ["droppedFrames", "bitrateMean", "bitrateStdDev",
       "framerateMean", "framerateStdDev", "framesEncoded", "firCount",
       "pliCount",],
     unimplemented: ["mediaTrackId", "transportId", "codecId",
       "sliCount", "qpSum", "roundTripTime", "targetBitrate",],
   },
   "codec": { skip: true },
   "peer-connection": { skip: true },
   "data-channel": { skip: true },
@@ -40,17 +40,17 @@ var statsExpectedByType = {
   "transport": { skip: true },
   "candidate-pair": { skip : true },
   "local-candidate": { skip: true },
   "remote-candidate": { skip: true },
   "certificate": { skip: true },
 };
 ["in", "out"].forEach(pre => {
   let s = statsExpectedByType[pre + "bound-rtp"];
-  s.optional = [...s.optional, ...s.videoOnly];
+  s.optional = [...s.optional, ...s.localVideoOnly];
 });
 
 //
 //  Checks that the fields in a report conform to the expectations in
 // statExpectedByType
 //
 var checkExpectedFields = report => report.forEach(stat => {
   let expectations = statsExpectedByType[stat.type];
@@ -134,16 +134,19 @@ var pedanticChecks = report => {
           "remote ssrc and local ssrc match.");
         is(report.get(stat.remoteId).remoteId, stat.id,
           "remote object has local object as it's own remote object.");
       }
 
       // nackCount
       if (!stat.inner.isRemote) {
         ok(stat.nackCount >= 0, stat.type + ".nackCount is sane.");
+      } else {
+        is(stat.nackCount, undefined, stat.type
+          + ".nackCount is only set when isRemote is false");
       }
 
       if (!stat.inner.isRemote && stat.inner.mediaType == "video") {
         // firCount
         ok(stat.firCount >= 0 && stat.firCount < 100,
           stat.type + ".firCount is a sane number for a short test. value="
           + stat.firCount);
 
@@ -339,16 +342,17 @@ var pedanticChecks = report => {
         ok(stat.droppedFrames >= 0,
           stat.type + ".droppedFrames is not negative. value="
           + stat.droppedFrames);
 
         // framesEncoded
         ok(stat.framesEncoded >= 0 && stat.framesEncoded < 100000, stat.type
           + ".framesEncoded is a sane number for a short test. value="
           + stat.framesEncoded);
+      }
     }
 
     //
     // Ensure everything was tested
     //
     [...expectations.expected, ...expectations.optional].forEach(field => {
       ok(Object.keys(tested).includes(field), stat.type + "." + field
         + " was tested.");
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -160,22 +160,34 @@ bool WebrtcAudioConduit::GetRemoteSSRC(u
 bool WebrtcAudioConduit::SetLocalCNAME(const char* cname)
 {
   char temp[256];
   strncpy(temp, cname, sizeof(temp) - 1);
   temp[sizeof(temp) - 1] = 0;
   return !mPtrRTP->SetRTCP_CNAME(mChannel, temp);
 }
 
-bool WebrtcAudioConduit::GetPacketTypeStats(
+bool WebrtcAudioConduit::GetSendPacketTypeStats(
   webrtc::RtcpPacketTypeCounter* aPacketCounts)
 {
+  if (!mEngineTransmitting) {
+    return false;
+  }
   return !mPtrVoERTP_RTCP->GetRTCPPacketTypeCounters(mChannel, *aPacketCounts);
 }
 
+bool WebrtcAudioConduit::GetRecvPacketTypeStats(
+  webrtc::RtcpPacketTypeCounter* aPacketCounts)
+{
+  if (!mEngineReceiving) {
+    return false;
+  }
+  return !mPtrRTP->GetRTCPPacketTypeCounters(mChannel, *aPacketCounts);
+}
+
 bool WebrtcAudioConduit::GetAVStats(int32_t* jitterBufferDelayMs,
                                     int32_t* playoutBufferDelayMs,
                                     int32_t* avSyncOffsetMs) {
   return !mPtrVoEVideoSync->GetDelayEstimate(mChannel,
                                              jitterBufferDelayMs,
                                              playoutBufferDelayMs,
                                              avSyncOffsetMs);
 }
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -197,18 +197,21 @@ public:
   std::vector<unsigned int> GetLocalSSRCs() const override;
   bool SetRemoteSSRC(unsigned int ssrc) override
   {
     return false;
   }
   bool GetRemoteSSRC(unsigned int* ssrc) override;
   bool SetLocalCNAME(const char* cname) override;
 
-  bool
-  GetPacketTypeStats(webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
+  bool GetSendPacketTypeStats(
+      webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
+
+  bool GetRecvPacketTypeStats(
+      webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
 
   bool GetVideoEncoderStats(double* framerateMean,
                             double* framerateStdDev,
                             double* bitrateMean,
                             double* bitrateStdDev,
                             uint32_t* droppedFrames,
                             uint32_t* framesEncoded) override
   {
--- a/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
+++ b/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
@@ -248,17 +248,20 @@ public:
   virtual bool SetRemoteSSRC(unsigned int ssrc) = 0;
   virtual bool SetLocalCNAME(const char* cname) = 0;
 
   /**
    * Functions returning stats needed by w3c stats model.
    */
 
   virtual bool
-  GetPacketTypeStats(webrtc::RtcpPacketTypeCounter* aPacketCounts) = 0;
+  GetSendPacketTypeStats(webrtc::RtcpPacketTypeCounter* aPacketCounts) = 0;
+
+  virtual bool
+  GetRecvPacketTypeStats(webrtc::RtcpPacketTypeCounter* aPacketCounts) = 0;
 
   virtual bool GetVideoEncoderStats(double* framerateMean,
                                     double* framerateStdDev,
                                     double* bitrateMean,
                                     double* bitrateStdDev,
                                     uint32_t* droppedFrames,
                                     uint32_t* framesEncoded) = 0;
   virtual bool GetVideoDecoderStats(double* framerateMean,
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -209,31 +209,28 @@ WebrtcVideoConduit::WebrtcVideoConduit(R
 {
   mRecvStreamConfig.renderer = this;
 
   // Video Stats Callback
   nsTimerCallbackFunc callback = [](nsITimer* aTimer, void* aClosure) {
     CSFLogDebug(logTag, "StreamStats polling scheduled for VideoConduit: %p", aClosure);
     auto self = static_cast<WebrtcVideoConduit*>(aClosure);
     MutexAutoLock lock(self->mCodecMutex);
-    MOZ_ASSERT(!self->mEngineTransmitting || !self->mEngineReceiving,
-      "Video conduit is not both receiving and transmitting");
     if (self->mEngineTransmitting && self->mSendStream) {
       const auto& stats = self->mSendStream->GetStats();
       self->mSendStreamStats.Update(stats);
-      if (stats.substreams.empty()) {
-        return;
+      if (!stats.substreams.empty()) {
+          self->mSendPacketCounts =
+            stats.substreams.begin()->second.rtcp_packet_type_counts;
       }
-      self->mPacketCounts =
-        stats.substreams.begin()->second.rtcp_packet_type_counts;
     }
     if (self->mEngineReceiving && self->mRecvStream) {
       const auto& stats = self->mRecvStream->GetStats();
       self->mRecvStreamStats.Update(stats);
-      self->mPacketCounts = stats.rtcp_packet_type_counts;
+      self->mRecvPacketCounts = stats.rtcp_packet_type_counts;
     }
   };
   mVideoStatsTimer->InitWithFuncCallback(
     callback, this, 1000, nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP);
 }
 
 WebrtcVideoConduit::~WebrtcVideoConduit()
 {
@@ -756,26 +753,36 @@ WebrtcVideoConduit::GetRemoteSSRC(unsign
     const webrtc::VideoReceiveStream::Stats& stats = mRecvStream->GetStats();
     *ssrc = stats.ssrc;
   }
 
   return true;
 }
 
 bool
-WebrtcVideoConduit::GetPacketTypeStats(
+WebrtcVideoConduit::GetSendPacketTypeStats(
     webrtc::RtcpPacketTypeCounter* aPacketCounts)
 {
   MutexAutoLock lock(mCodecMutex);
-  if ((!mEngineTransmitting || !mSendStream) // Not transmitting
-      && (!mEngineReceiving || !mRecvStream)) // And not receiving
-  {
+  if (!mEngineTransmitting || !mSendStream) { // Not transmitting
     return false;
   }
-  *aPacketCounts = mPacketCounts;
+  *aPacketCounts = mSendPacketCounts;
+  return true;
+}
+
+bool
+WebrtcVideoConduit::GetRecvPacketTypeStats(
+    webrtc::RtcpPacketTypeCounter* aPacketCounts)
+{
+  MutexAutoLock lock(mCodecMutex);
+  if (!mEngineReceiving || !mRecvStream) { // Not receiving
+    return false;
+  }
+  *aPacketCounts = mRecvPacketCounts;
   return true;
 }
 
 bool
 WebrtcVideoConduit::GetVideoEncoderStats(double* framerateMean,
                                          double* framerateStdDev,
                                          double* bitrateMean,
                                          double* bitrateStdDev,
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ -296,18 +296,21 @@ public:
   virtual void Destroy();
 
   std::vector<unsigned int> GetLocalSSRCs() const override;
   bool SetLocalSSRCs(const std::vector<unsigned int> & ssrcs) override;
   bool GetRemoteSSRC(unsigned int* ssrc) override;
   bool SetRemoteSSRC(unsigned int ssrc) override;
   bool SetLocalCNAME(const char* cname) override;
 
-  bool
-  GetPacketTypeStats(webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
+  bool GetSendPacketTypeStats(
+      webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
+
+  bool GetRecvPacketTypeStats(
+      webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
 
   bool GetVideoEncoderStats(double* framerateMean,
                             double* framerateStdDev,
                             double* bitrateMean,
                             double* bitrateStdDev,
                             uint32_t* droppedFrames,
                             uint32_t* framesEncoded) override;
   bool GetVideoDecoderStats(double* framerateMean,
@@ -461,23 +464,24 @@ private:
   // Engine state we are concerned with.
   mozilla::Atomic<bool> mEngineTransmitting; // If true ==> Transmit Subsystem is up and running
   mozilla::Atomic<bool> mEngineReceiving;    // if true ==> Receive Subsystem up and running
 
   int mCapId;   // Capturer for this conduit
   //Local database of currently applied receive codecs
   nsTArray<UniquePtr<VideoCodecConfig>> mRecvCodecList;
 
-  // protects mCurSendCodecConfig, mInReconfig,mVideoSend/RecvStreamStats, mSend/RecvStreams, mPacketCounts
+  // protects mCurSendCodecConfig, mInReconfig,mVideoSend/RecvStreamStats, mSend/RecvStreams, mSendPacketCounts, mRecvPacketCounts
   Mutex mCodecMutex;
   nsAutoPtr<VideoCodecConfig> mCurSendCodecConfig;
   bool mInReconfig;
   SendStreamStatistics mSendStreamStats;
   ReceiveStreamStatistics mRecvStreamStats;
-  webrtc::RtcpPacketTypeCounter mPacketCounts;
+  webrtc::RtcpPacketTypeCounter mSendPacketCounts;
+  webrtc::RtcpPacketTypeCounter mRecvPacketCounts;
 
   // Must call webrtc::Call::DestroyVideoReceive/SendStream to delete these:
   webrtc::VideoReceiveStream* mRecvStream;
   webrtc::VideoSendStream* mSendStream;
 
   unsigned short mLastWidth;
   unsigned short mLastHeight;
   unsigned short mSendingWidth;
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -3656,17 +3656,17 @@ PeerConnectionImpl::ExecuteStatsQuery_s(
           s.mMediaType.Construct(mediaType);
           s.mRemoteId.Construct(remoteId);
           s.mIsRemote = false;
           s.mPacketsSent.Construct(mp.rtp_packets_sent());
           s.mBytesSent.Construct(mp.rtp_bytes_sent());
 
           // Fill in packet type statistics
           webrtc::RtcpPacketTypeCounter counters;
-          if (mp.Conduit()->GetPacketTypeStats(&counters)) {
+          if (mp.Conduit()->GetSendPacketTypeStats(&counters)) {
             s.mNackCount.Construct(counters.nack_packets);
             // Fill in video only packet type stats
             if (!isAudio) {
               s.mFirCount.Construct(counters.fir_packets);
               s.mPliCount.Construct(counters.pli_packets);
             }
           }
 
@@ -3758,17 +3758,17 @@ PeerConnectionImpl::ExecuteStatsQuery_s(
                                        &playoutBufferDelay,
                                        &avSyncDelta)) {
             s.mMozJitterBufferDelay.Construct(jitterBufferDelay);
             s.mMozAvSyncDelay.Construct(avSyncDelta);
           }
         }
         // Fill in packet type statistics
         webrtc::RtcpPacketTypeCounter counters;
-        if (mp.Conduit()->GetPacketTypeStats(&counters)) {
+        if (mp.Conduit()->GetRecvPacketTypeStats(&counters)) {
           s.mNackCount.Construct(counters.nack_packets);
           // Fill in video only packet type stats
           if (!isAudio) {
             s.mFirCount.Construct(counters.fir_packets);
             s.mPliCount.Construct(counters.pli_packets);
           }
         }
         // Lastly, fill in video decoder stats if this is video