Bug 1343691 - fix missing rtcp stats;r?jib draft
authorNico Grunbaum
Wed, 08 Mar 2017 23:26:24 -0800
changeset 498489 03f635e6299c8d57a429b89ae8ead4f1140b00fa
parent 498474 c7d1e31935ce5185b13f0e26dd1afc3d8aaaa875
child 549169 ed53d50958c92d91f8638a568c74292093709b38
push id49209
push userna-g@nostrum.com
push dateTue, 14 Mar 2017 21:04:11 +0000
reviewersjib
bugs1343691, 1344970
milestone55.0a1
Bug 1343691 - fix missing rtcp stats;r?jib Omitting the RTT when it is not available breaks a lot of tests (as jesup warned). I am going to fix the RTT behavior and the tests in bug 1344970, for now RTT will be zero when unavailable. MozReview-Commit-ID: 9x3eQfbM3ZT
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/trunk/webrtc/video/video_receive_stream.cc
media/webrtc/trunk/webrtc/video/video_receive_stream.h
media/webrtc/trunk/webrtc/video/video_send_stream.h
media/webrtc/trunk/webrtc/video/vie_channel.cc
media/webrtc/trunk/webrtc/video_receive_stream.h
media/webrtc/trunk/webrtc/video_send_stream.h
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -200,20 +200,23 @@ bool WebrtcAudioConduit::GetRTCPReceiver
   uint16_t fractionLost;
   bool result = !mPtrRTP->GetRemoteRTCPReceiverInfo(mChannel, ntpHigh, ntpLow,
                                                     *packetsReceived,
                                                     *bytesReceived,
                                                     *jitterMs,
                                                     fractionLost,
                                                     *cumulativeLost,
                                                     *rttMs);
-  if (result) {
-    *timestamp = NTPtoDOMHighResTimeStamp(ntpHigh, ntpLow);
+  if (!result) {
+    return false;
   }
-  return result;
+  // 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 WebrtcAudioConduit::GetRTCPSenderReport(DOMHighResTimeStamp* timestamp,
                                              unsigned int* packetsSent,
                                              uint64_t* bytesSent) {
   webrtc::RTCPSenderInfo senderInfo;
   webrtc::RtpRtcp * rtpRtcpModule;
   webrtc::RtpReceiver * rtp_receiver;
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -816,60 +816,72 @@ bool WebrtcVideoConduit::GetRTCPReceiver
                                                uint32_t* packetsReceived,
                                                uint64_t* bytesReceived,
                                                uint32_t* cumulativeLost,
                                                int32_t* rttMs)
 {
   {
     CSFLogVerbose(logTag, "%s for VideoConduit:%p", __FUNCTION__, this);
     MutexAutoLock lock(mCodecMutex);
-    if (!mRecvStream) {
+    if (!mSendStream) {
+      return false;
+    }
+    const webrtc::VideoSendStream::Stats& sendStats = mSendStream->GetStats();
+    if (sendStats.substreams.size() == 0
+        || mSendStreamConfig.rtp.ssrcs.size() == 0) {
       return false;
     }
-
-    const webrtc::VideoReceiveStream::Stats &stats = mRecvStream->GetStats();
-    *jitterMs = stats.rtcp_stats.jitter;
-    *cumulativeLost = stats.rtcp_stats.cumulative_lost;
-    *bytesReceived = stats.rtp_stats.MediaPayloadBytes();
-    *packetsReceived = stats.rtp_stats.transmitted.packets;
-    // Note: timestamp is not correct per the spec... should be time the rtcp
-    // was received (remote) or sent (local)
-    *timestamp = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds();
-    int64_t rtt = mRecvStream->GetRtt();
+    uint32_t ssrc = mSendStreamConfig.rtp.ssrcs.front();
+    auto ind = sendStats.substreams.find(ssrc);
+    if (ind == sendStats.substreams.end()) {
+      CSFLogError(logTag,
+        "%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;
     }
+#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
+    // 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
 WebrtcVideoConduit::GetRTCPSenderReport(DOMHighResTimeStamp* timestamp,
                                         unsigned int* packetsSent,
                                         uint64_t* bytesSent)
 {
   CSFLogVerbose(logTag, "%s for VideoConduit:%p", __FUNCTION__, this);
-  MutexAutoLock lock(mCodecMutex);
-  if (!mSendStream) {
-    return false;
+  webrtc::RTCPSenderInfo senderInfo;
+  {
+    MutexAutoLock lock(mCodecMutex);
+    if (!mRecvStream || !mRecvStream->GetRemoteRTCPSenderInfo(&senderInfo)) {
+      return false;
+    }
   }
-
-  const webrtc::VideoSendStream::Stats& stats = mSendStream->GetStats();
-  *packetsSent = 0;
-  for (auto entry: stats.substreams){
-    *packetsSent += entry.second.rtp_stats.transmitted.packets;
-    // NG -- per https://www.w3.org/TR/webrtc-stats/ this is only payload bytes
-    *bytesSent += entry.second.rtp_stats.MediaPayloadBytes();
-  }
-
-  // Note: timestamp is not correct per the spec... should be time the rtcp
-  // was received (remote) or sent (local)
   *timestamp = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds();
+  *packetsSent = senderInfo.sendPacketCount;
+  *bytesSent = senderInfo.sendOctetCount;
   return true;
 }
 
 MediaConduitErrorCode
 WebrtcVideoConduit::InitMain()
 {
   // already know we must be on MainThread barring unit test weirdness
   MOZ_ASSERT(NS_IsMainThread());
--- a/media/webrtc/trunk/webrtc/video/video_receive_stream.cc
+++ b/media/webrtc/trunk/webrtc/video/video_receive_stream.cc
@@ -402,10 +402,15 @@ int64_t VideoReceiveStream::GetRtt() con
         timestampNTPHigh, timestampNTPLow, receivedPacketCount,
         receivedOctetCount, &jitterSamples, &fractionLost, &cumulativeLost,
         &rttMs)) {
     return rttMs;
   }
   return -1;
 }
 
+bool
+VideoReceiveStream::GetRemoteRTCPSenderInfo(RTCPSenderInfo* sender_info) const {
+  return -1 != vie_channel_->GetRemoteRTCPSenderInfo(sender_info);
+}
+
 }  // namespace internal
 }  // namespace webrtc
--- a/media/webrtc/trunk/webrtc/video/video_receive_stream.h
+++ b/media/webrtc/trunk/webrtc/video/video_receive_stream.h
@@ -71,16 +71,17 @@ class VideoReceiveStream : public webrtc
   int32_t Encoded(const EncodedImage& encoded_image,
                   const CodecSpecificInfo* codec_specific_info,
                   const RTPFragmentationHeader* fragmentation) override;
 
   const Config& config() const { return config_; }
 
   void SetSyncChannel(VoiceEngine* voice_engine, int audio_channel_id) override;
   int64_t GetRtt() const override;
+  bool GetRemoteRTCPSenderInfo(RTCPSenderInfo* sender_info) const override;
  private:
   TransportAdapter transport_adapter_;
   EncodedFrameCallbackAdapter encoded_frame_proxy_;
   const VideoReceiveStream::Config config_;
   Clock* const clock_;
 
   CongestionController* const congestion_controller_;
   CallStats* const call_stats_;
--- a/media/webrtc/trunk/webrtc/video/video_send_stream.h
+++ b/media/webrtc/trunk/webrtc/video/video_send_stream.h
@@ -65,17 +65,17 @@ class VideoSendStream : public webrtc::V
 
   // webrtc::CpuOveruseObserver implementation.
   void OveruseDetected() override;
   void NormalUsage() override;
 
   typedef std::map<uint32_t, RtpState> RtpStateMap;
   RtpStateMap GetRtpStates() const;
 
-  int64_t GetRtt() const;
+  int64_t GetRtt() const override;
   int GetPaddingNeededBps() const;
 
  private:
   bool SetSendCodec(VideoCodec video_codec);
   void ConfigureSsrcs();
 
   SendStatisticsProxy stats_proxy_;
   TransportAdapter transport_adapter_;
--- a/media/webrtc/trunk/webrtc/video/vie_channel.cc
+++ b/media/webrtc/trunk/webrtc/video/vie_channel.cc
@@ -886,31 +886,25 @@ int32_t ViEChannel::GetRemoteRTCPReceive
   if (rtp_rtcp_modules_[0]->RTT(remote_ssrc, &rtt, &dummy, &dummy, &dummy) != 0) {
     LOG_F(LS_ERROR) << "failed to get RTT";
     return -1;
   }
   *rttMs = rtt;
   return 0;
 }
 
-//->@@NG // int32_t ViEChannel::GetRemoteRTCPSenderInfo(RTCPSenderInfo* sender_info) const {
-//->@@NG //   // Get the sender info from the latest received RTCP Sender Report.
-//->@@NG //   RTCPSenderInfo rtcp_sender_info;
-//->@@NG //   if (rtp_rtcp_->RemoteRTCPStat(&rtcp_sender_info) != 0) {
-//->@@NG //     LOG_F(LS_ERROR) << "failed to read RTCP SR sender info";
-//->@@NG //     return -1;
-//->@@NG //   }
-//->@@NG //
-//->@@NG //   sender_info->NTP_timestamp_high = rtcp_sender_info.NTPseconds;
-//->@@NG //   sender_info->NTP_timestamp_low = rtcp_sender_info.NTPfraction;
-//->@@NG //   sender_info->RTP_timestamp = rtcp_sender_info.RTPtimeStamp;
-//->@@NG //   sender_info->sender_packet_count = rtcp_sender_info.sendPacketCount;
-//->@@NG //   sender_info->sender_octet_count = rtcp_sender_info.sendOctetCount;
-//->@@NG //   return 0;
-//->@@NG // }
+int32_t ViEChannel::GetRemoteRTCPSenderInfo(RTCPSenderInfo* sender_info) const {
+  // Get the sender info from the latest received RTCP Sender Report.
+  if (rtp_rtcp_modules_[0] &&
+    rtp_rtcp_modules_[0]->RemoteRTCPStat(sender_info) != 0) {
+    LOG_F(LS_ERROR) << "failed to read RTCP SR sender info";
+    return -1;
+  }
+  return 0;
+}
 
 void ViEChannel::RegisterSendChannelRtcpStatisticsCallback(
     RtcpStatisticsCallback* callback) {
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
     rtp_rtcp->RegisterRtcpStatisticsCallback(callback);
 }
 
 void ViEChannel::RegisterReceiveChannelRtcpStatisticsCallback(
--- a/media/webrtc/trunk/webrtc/video_receive_stream.h
+++ b/media/webrtc/trunk/webrtc/video_receive_stream.h
@@ -174,14 +174,18 @@ class VideoReceiveStream : public Receiv
     // Target delay in milliseconds. A positive value indicates this stream is
     // used for streaming instead of a real-time call.
     int target_delay_ms = 0;
   };
 
   // TODO(pbos): Add info on currently-received codec to Stats.
   virtual Stats GetStats() const = 0;
   virtual int64_t GetRtt() const = 0;
+
+  virtual bool
+  GetRemoteRTCPSenderInfo(RTCPSenderInfo* sender_info) const = 0;
+
   virtual void SetSyncChannel(VoiceEngine* voice_engine, int audio_channel_id) = 0;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_VIDEO_RECEIVE_STREAM_H_
--- a/media/webrtc/trunk/webrtc/video_send_stream.h
+++ b/media/webrtc/trunk/webrtc/video_send_stream.h
@@ -182,13 +182,15 @@ class VideoSendStream : public SendStrea
   virtual CPULoadStateObserver* LoadStateObserver() = 0;
 
   // Set which streams to send. Must have at least as many SSRCs as configured
   // in the config. Encoder settings are passed on to the encoder instance along
   // with the VideoStream settings.
   virtual bool ReconfigureVideoEncoder(const VideoEncoderConfig& config) = 0;
 
   virtual Stats GetStats() = 0;
+
+  virtual int64_t GetRtt() const = 0;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_VIDEO_SEND_STREAM_H_