Bug 1343691 - "RTCP stats missing in FF53 (fallout from webrtc.org 49 update)" r=jib a=gchang
authorNico Grunbaum [:ng] <na-g@nostrum.com>
Wed, 22 Mar 2017 15:28:00 +0100
changeset 379209 410550719c977a11fc6561c8f791e8aece2f797a
parent 379208 2e62913dde4d50f483b154f9f7499a446db96ee6
child 379210 129cd2f7ede3c7b3bc36aa802be201d07bc154c7
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib, gchang
bugs1343691
milestone53.0
Bug 1343691 - "RTCP stats missing in FF53 (fallout from webrtc.org 49 update)" r=jib a=gchang
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
@@ -202,20 +202,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
@@ -821,60 +821,75 @@ 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;
-    }
-
-    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();
+    webrtc::RTCPSenderInfo senderInfo;
+    {
+      MutexAutoLock lock(mCodecMutex);
+      if (!mRecvStream || !mRecvStream->GetRemoteRTCPSenderInfo(&senderInfo)) {
+        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();
+    *packetsSent = senderInfo.sendPacketCount;
+    *bytesSent = senderInfo.sendOctetCount;
     return true;
   }
   return false;
 }
 
 MediaConduitErrorCode
 WebrtcVideoConduit::InitMain()
 {
--- 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
@@ -890,31 +890,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
@@ -180,13 +180,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_