Return an aggregated report from ViERtpRtcp::GetSentRTCPStatistics().

Fixes issues where statistics only was reported for the first stream if
configured with simulcast, and in case of RTX the reported statistics was
depending on the order of the report blocks.

Also fixes issues with multiple report blocks in the SendStatisticsProxy and the
RtcpStatisticsCallback. SendStatisticsProxy is now aware of RTX ssrcs, and the
RTCPReceiver is calling the RtcpStatisticsCallback with the correct SSRCs, and
not only the primary stream SSRC.

R=mflodman@webrtc.org, sprang@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/20149004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6903 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -1470,7 +1470,7 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket(
         stats.fraction_lost = it->fractionLost;
         stats.jitter = it->jitter;
 
-        stats_callback_->StatisticsUpdated(stats, local_ssrc);
+        stats_callback_->StatisticsUpdated(stats, it->sourceSSRC);
       }
     }
   }

webrtc/video/send_statistics_proxy.cc

@@ -53,8 +53,12 @@ StreamStats* SendStatisticsProxy::GetStatsEntry(uint32_t ssrc) {
     return &it->second;
 
   if (std::find(config_.rtp.ssrcs.begin(), config_.rtp.ssrcs.end(), ssrc) ==
-      config_.rtp.ssrcs.end())
+          config_.rtp.ssrcs.end() &&
+      std::find(config_.rtp.rtx.ssrcs.begin(),
+                config_.rtp.rtx.ssrcs.end(),
+                ssrc) == config_.rtp.rtx.ssrcs.end()) {
     return NULL;
+  }
 
   return &stats_.substreams[ssrc];  // Insert new entry and return ptr.
 }

webrtc/video/send_statistics_proxy_unittest.cc

@@ -36,6 +36,8 @@ class SendStatisticsProxyTest : public ::testing::Test {
     VideoSendStream::Config config;
     config.rtp.ssrcs.push_back(17);
     config.rtp.ssrcs.push_back(42);
+    config.rtp.rtx.ssrcs.push_back(18);
+    config.rtp.rtx.ssrcs.push_back(43);
     return config;
   }
 
@@ -101,7 +103,20 @@ TEST_F(SendStatisticsProxyTest, RtcpStatistics) {
     ssrc_stats.rtcp_stats.jitter = offset + 3;
     callback->StatisticsUpdated(ssrc_stats.rtcp_stats, ssrc);
   }
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.rtx.ssrcs.begin();
+       it != config_.rtp.rtx.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    StreamStats& ssrc_stats = expected_.substreams[ssrc];
 
+    // Add statistics with some arbitrary, but unique, numbers.
+    uint32_t offset = ssrc * sizeof(RtcpStatistics);
+    ssrc_stats.rtcp_stats.cumulative_lost = offset;
+    ssrc_stats.rtcp_stats.extended_max_sequence_number = offset + 1;
+    ssrc_stats.rtcp_stats.fraction_lost = offset + 2;
+    ssrc_stats.rtcp_stats.jitter = offset + 3;
+    callback->StatisticsUpdated(ssrc_stats.rtcp_stats, ssrc);
+  }
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
   ExpectEqual(expected_, stats);
 }
@@ -148,6 +163,18 @@ TEST_F(SendStatisticsProxyTest, FrameCounts) {
     observer->FrameCountUpdated(kVideoFrameKey, stats.key_frames, ssrc);
     observer->FrameCountUpdated(kVideoFrameDelta, stats.delta_frames, ssrc);
   }
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.rtx.ssrcs.begin();
+       it != config_.rtp.rtx.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    // Add statistics with some arbitrary, but unique, numbers.
+    StreamStats& stats = expected_.substreams[ssrc];
+    uint32_t offset = ssrc * sizeof(StreamStats);
+    stats.key_frames = offset;
+    stats.delta_frames = offset + 1;
+    observer->FrameCountUpdated(kVideoFrameKey, stats.key_frames, ssrc);
+    observer->FrameCountUpdated(kVideoFrameDelta, stats.delta_frames, ssrc);
+  }
 
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
   ExpectEqual(expected_, stats);
@@ -170,6 +197,21 @@ TEST_F(SendStatisticsProxyTest, DataCounters) {
     counters.packets = offset + 5;
     callback->DataCountersUpdated(counters, ssrc);
   }
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.rtx.ssrcs.begin();
+       it != config_.rtp.rtx.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    StreamDataCounters& counters = expected_.substreams[ssrc].rtp_stats;
+    // Add statistics with some arbitrary, but unique, numbers.
+    uint32_t offset = ssrc * sizeof(StreamDataCounters);
+    counters.bytes = offset;
+    counters.header_bytes = offset + 1;
+    counters.fec_packets = offset + 2;
+    counters.padding_bytes = offset + 3;
+    counters.retransmitted_packets = offset + 4;
+    counters.packets = offset + 5;
+    callback->DataCountersUpdated(counters, ssrc);
+  }
 
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
   ExpectEqual(expected_, stats);
@@ -187,6 +229,16 @@ TEST_F(SendStatisticsProxyTest, Bitrate) {
     observer->Notify(bitrate, ssrc);
     expected_.substreams[ssrc].bitrate_bps = ssrc;
   }
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.rtx.ssrcs.begin();
+       it != config_.rtp.rtx.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    BitrateStatistics bitrate;
+    // Use ssrc as bitrate_bps to get a unique value for each stream.
+    bitrate.bitrate_bps = ssrc;
+    observer->Notify(bitrate, ssrc);
+    expected_.substreams[ssrc].bitrate_bps = ssrc;
+  }
 
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
   ExpectEqual(expected_, stats);
@@ -206,14 +258,29 @@ TEST_F(SendStatisticsProxyTest, SendSideDelay) {
     expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms;
     expected_.substreams[ssrc].max_delay_ms = max_delay_ms;
   }
-
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.rtx.ssrcs.begin();
+       it != config_.rtp.rtx.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    // Use ssrc as avg_delay_ms and max_delay_ms to get a unique value for each
+    // stream.
+    int avg_delay_ms = ssrc;
+    int max_delay_ms = ssrc + 1;
+    observer->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc);
+    expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms;
+    expected_.substreams[ssrc].max_delay_ms = max_delay_ms;
+  }
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
   ExpectEqual(expected_, stats);
 }
 
 TEST_F(SendStatisticsProxyTest, NoSubstreams) {
   uint32_t exluded_ssrc =
-      *std::max_element(config_.rtp.ssrcs.begin(), config_.rtp.ssrcs.end()) + 1;
+      std::max(
+          *std::max_element(config_.rtp.ssrcs.begin(), config_.rtp.ssrcs.end()),
+          *std::max_element(config_.rtp.rtx.ssrcs.begin(),
+                            config_.rtp.rtx.ssrcs.end())) +
+      1;
   // From RtcpStatisticsCallback.
   RtcpStatistics rtcp_stats;
   RtcpStatisticsCallback* rtcp_callback = statistics_proxy_.get();

webrtc/video_engine/include/vie_rtp_rtcp.h

@@ -305,8 +305,11 @@ class WEBRTC_DLLEXPORT ViERTP_RTCP {
                                               RtcpStatistics& basic_stats,
                                               int& rtt_ms) const = 0;
 
-  // This function returns statistics reported by the remote client in a RTCP
-  // packet.
+  // This function returns statistics reported by the remote client in RTCP
+  // report blocks. If several streams are reported, the statistics will be
+  // aggregated.
+  // If statistics are aggregated, extended_max_sequence_number is not reported,
+  // and will always be set to 0.
   virtual int GetSendChannelRtcpStatistics(const int video_channel,
                                            RtcpStatistics& basic_stats,
                                            int& rtt_ms) const = 0;

webrtc/video_engine/vie_channel.cc

@@ -41,6 +41,50 @@ const int kInvalidRtpExtensionId = 0;
 static const int kMaxTargetDelayMs = 10000;
 static const float kMaxIncompleteTimeMultiplier = 3.5f;
 
+namespace {
+
+RTCPReportBlock AggregateReportBlocks(
+    const std::vector<RTCPReportBlock>& report_blocks,
+    std::map<uint32_t, RTCPReportBlock>* prev_report_blocks) {
+  int fraction_lost_sum = 0;
+  int fl_seq_num_sum = 0;
+  int jitter_sum = 0;
+  int number_of_report_blocks = 0;
+  RTCPReportBlock aggregate;
+  std::vector<RTCPReportBlock>::const_iterator report_block =
+      report_blocks.begin();
+  for (; report_block != report_blocks.end(); ++report_block) {
+    aggregate.cumulativeLost += report_block->cumulativeLost;
+    std::map<uint32_t, RTCPReportBlock>::iterator prev_report_block =
+        prev_report_blocks->find(report_block->sourceSSRC);
+    if (prev_report_block != prev_report_blocks->end()) {
+      // Skip the first report block since we won't be able to get a correct
+      // weight for it.
+      int seq_num_diff = report_block->extendedHighSeqNum -
+                         prev_report_block->second.extendedHighSeqNum;
+      if (seq_num_diff > 0) {
+        fraction_lost_sum += report_block->fractionLost * seq_num_diff;
+        fl_seq_num_sum += seq_num_diff;
+      }
+    }
+    jitter_sum += report_block->jitter;
+    ++number_of_report_blocks;
+    (*prev_report_blocks)[report_block->sourceSSRC] = *report_block;
+  }
+  if (fl_seq_num_sum > 0) {
+    aggregate.fractionLost =
+        (fraction_lost_sum + fl_seq_num_sum / 2) / fl_seq_num_sum;
+  }
+  if (number_of_report_blocks > 0) {
+    aggregate.jitter =
+        (jitter_sum + number_of_report_blocks / 2) / number_of_report_blocks;
+  }
+  // Not well defined for aggregated report blocks.
+  aggregate.extendedHighSeqNum = 0;
+  return aggregate;
+}
+}  // namespace
+
 // Helper class receiving statistics callbacks.
 class ChannelStatsObserver : public CallStatsObserver {
  public:
@@ -965,48 +1009,33 @@ int32_t ViEChannel::GetSendRtcpStatistics(uint16_t* fraction_lost,
                                           uint32_t* extended_max,
                                           uint32_t* jitter_samples,
                                           int32_t* rtt_ms) {
-  // TODO(pwestin) how do we do this for simulcast ? average for all
-  // except cumulative_lost that is the sum ?
-  // CriticalSectionScoped cs(rtp_rtcp_cs_.get());
+  // Aggregate the report blocks associated with streams sent on this channel.
+  std::vector<RTCPReportBlock> report_blocks;
+  rtp_rtcp_->RemoteRTCPStat(&report_blocks);
+  for (std::list<RtpRtcp*>::iterator it = simulcast_rtp_rtcp_.begin();
+       it != simulcast_rtp_rtcp_.end();
+       ++it) {
+    (*it)->RemoteRTCPStat(&report_blocks);
+  }
 
-  // for (std::list<RtpRtcp*>::const_iterator it = simulcast_rtp_rtcp_.begin();
-  //      it != simulcast_rtp_rtcp_.end();
-  //      it++) {
-  //   RtpRtcp* rtp_rtcp = *it;
-  // }
-  uint32_t remote_ssrc = vie_receiver_.GetRemoteSsrc();
-
-  // Get all RTCP receiver report blocks that have been received on this
-  // channel. If we receive RTP packets from a remote source we know the
-  // remote SSRC and use the report block from him.
-  // Otherwise use the first report block.
-  std::vector<RTCPReportBlock> remote_stats;
-  if (rtp_rtcp_->RemoteRTCPStat(&remote_stats) != 0 || remote_stats.empty()) {
+  if (report_blocks.empty())
     return -1;
-  }
-  std::vector<RTCPReportBlock>::const_iterator statistics =
-      remote_stats.begin();
-  for (; statistics != remote_stats.end(); ++statistics) {
-    if (statistics->remoteSSRC == remote_ssrc)
-      break;
-  }
 
-  if (statistics == remote_stats.end()) {
-    // If we have not received any RTCP packets from this SSRC it probably means
-    // we have not received any RTP packets.
-    // Use the first received report block instead.
-    statistics = remote_stats.begin();
-    remote_ssrc = statistics->remoteSSRC;
-  }
+  RTCPReportBlock report;
+  if (report_blocks.size() > 1)
+    report = AggregateReportBlocks(report_blocks, &prev_report_blocks_);
+  else
+    report = report_blocks[0];
 
-  *fraction_lost = statistics->fractionLost;
-  *cumulative_lost = statistics->cumulativeLost;
-  *extended_max = statistics->extendedHighSeqNum;
-  *jitter_samples = statistics->jitter;
+  *fraction_lost = report.fractionLost;
+  *cumulative_lost = report.cumulativeLost;
+  *extended_max = report.extendedHighSeqNum;
+  *jitter_samples = report.jitter;
 
   uint16_t dummy;
   uint16_t rtt = 0;
-  if (rtp_rtcp_->RTT(remote_ssrc, &rtt, &dummy, &dummy, &dummy) != 0) {
+  if (rtp_rtcp_->RTT(
+          vie_receiver_.GetRemoteSsrc(), &rtt, &dummy, &dummy, &dummy) != 0) {
     return -1;
   }
   *rtt_ms = rtt;

webrtc/video_engine/vie_channel.h

@@ -499,6 +499,8 @@ class ViEChannel
   int nack_history_size_sender_;
   int max_nack_reordering_threshold_;
   I420FrameCallback* pre_render_callback_;
+
+  std::map<uint32_t, RTCPReportBlock> prev_report_blocks_;
 };
 
 }  // namespace webrtc