[rtp_rtcp] cleanup in RTCPSender class internals.

PrepareReportBlock and AddReportBlock private functions merged:
  PrepareReportBlock moved report block from statistic to temporary structure
  AddReportBlock copied that temporary structure into temporary map right after.
  Thanks to rtcp packet classes that temporary structure is now unneccesary.

BUG=webrtc:5260
R=åsapersson

Review URL: https://codereview.webrtc.org/1538833002

Cr-Commit-Position: refs/heads/master@{#11112}

webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc

@@ -259,6 +259,7 @@ RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() {
   stats.fraction_lost = local_fraction_lost;
 
   // We need a counter for cumulative loss too.
+  // TODO(danilchap): Ensure cumulative loss is below maximum value of 2^24.
   cumulative_loss_ += missing;
   stats.cumulative_lost = cumulative_loss_;
   stats.extended_max_sequence_number =

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

@@ -453,26 +453,6 @@ bool RTCPSender::SendTimeOfXrRrReport(uint32_t mid_ntp,
   return true;
 }
 
-int32_t RTCPSender::AddReportBlock(const RTCPReportBlock& report_block) {
-  if (report_blocks_.size() >= RTCP_MAX_REPORT_BLOCKS) {
-    LOG(LS_WARNING) << "Too many report blocks.";
-    return -1;
-  }
-  rtcp::ReportBlock* block = &report_blocks_[report_block.remoteSSRC];
-  block->To(report_block.remoteSSRC);
-  block->WithFractionLost(report_block.fractionLost);
-  if (!block->WithCumulativeLost(report_block.cumulativeLost)) {
-    LOG(LS_WARNING) << "Cumulative lost is oversized.";
-    return -1;
-  }
-  block->WithExtHighestSeqNum(report_block.extendedHighSeqNum);
-  block->WithJitter(report_block.jitter);
-  block->WithLastSr(report_block.lastSR);
-  block->WithDelayLastSr(report_block.delaySinceLastSR);
-
-  return 0;
-}
-
 rtc::scoped_ptr<rtcp::RtcpPacket> RTCPSender::BuildSR(const RtcpContext& ctx) {
   for (int i = (RTCP_NUMBER_OF_SR - 2); i >= 0; i--) {
     // shift old
@@ -916,33 +896,37 @@ void RTCPSender::PrepareReport(const std::set<RTCPPacketType>& packetTypes,
 
     StatisticianMap statisticians =
         receive_statistics_->GetActiveStatisticians();
-    if (!statisticians.empty()) {
+    RTC_DCHECK(report_blocks_.empty());
-      for (auto it = statisticians.begin(); it != statisticians.end(); ++it) {
+    for (auto& it : statisticians) {
-        RTCPReportBlock report_block;
+      AddReportBlock(feedback_state, it.first, it.second);
-        if (PrepareReportBlock(feedback_state, it->first, it->second,
-                               &report_block)) {
-          // TODO(danilchap) AddReportBlock may fail (for 2 different reasons).
-          // Probably it shouldn't be ignored.
-          AddReportBlock(report_block);
-        }
-      }
     }
   }
 }
 
-bool RTCPSender::PrepareReportBlock(const FeedbackState& feedback_state,
+bool RTCPSender::AddReportBlock(const FeedbackState& feedback_state,
-                                    uint32_t ssrc,
+                                uint32_t ssrc,
-                                    StreamStatistician* statistician,
+                                StreamStatistician* statistician) {
-                                    RTCPReportBlock* report_block) {
   // Do we have receive statistics to send?
   RtcpStatistics stats;
   if (!statistician->GetStatistics(&stats, true))
     return false;
-  report_block->fractionLost = stats.fraction_lost;
+
-  report_block->cumulativeLost = stats.cumulative_lost;
+  if (report_blocks_.size() >= RTCP_MAX_REPORT_BLOCKS) {
-  report_block->extendedHighSeqNum = stats.extended_max_sequence_number;
+    LOG(LS_WARNING) << "Too many report blocks.";
-  report_block->jitter = stats.jitter;
+    return false;
-  report_block->remoteSSRC = ssrc;
+  }
+  RTC_DCHECK(report_blocks_.find(ssrc) == report_blocks_.end());
+  rtcp::ReportBlock* block = &report_blocks_[ssrc];
+  block->To(ssrc);
+  block->WithFractionLost(stats.fraction_lost);
+  if (!block->WithCumulativeLost(stats.cumulative_lost)) {
+    report_blocks_.erase(ssrc);
+    LOG(LS_WARNING) << "Cumulative lost is oversized.";
+    return false;
+  }
+  block->WithExtHighestSeqNum(stats.extended_max_sequence_number);
+  block->WithJitter(stats.jitter);
+  block->WithLastSr(feedback_state.remote_sr);
 
   // TODO(sprang): Do we really need separate time stamps for each report?
   // Get our NTP as late as possible to avoid a race.
@@ -951,7 +935,6 @@ bool RTCPSender::PrepareReportBlock(const FeedbackState& feedback_state,
   clock_->CurrentNtp(ntp_secs, ntp_frac);
 
   // Delay since last received report.
-  uint32_t delaySinceLastReceivedSR = 0;
   if ((feedback_state.last_rr_ntp_secs != 0) ||
       (feedback_state.last_rr_ntp_frac != 0)) {
     // Get the 16 lowest bits of seconds and the 16 highest bits of fractions.
@@ -963,10 +946,8 @@ bool RTCPSender::PrepareReportBlock(const FeedbackState& feedback_state,
     receiveTime <<= 16;
     receiveTime += (feedback_state.last_rr_ntp_frac & 0xffff0000) >> 16;
 
-    delaySinceLastReceivedSR = now - receiveTime;
+    block->WithDelayLastSr(now - receiveTime);
   }
-  report_block->delaySinceLastSR = delaySinceLastReceivedSR;
-  report_block->lastSR = feedback_state.remote_sr;
   return true;
 }
 

webrtc/modules/rtp_rtcp/source/rtcp_sender.h

@@ -158,14 +158,11 @@ class RTCPSender {
                      const FeedbackState& feedback_state)
       EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
 
-  int32_t AddReportBlock(const RTCPReportBlock& report_block)
+  bool AddReportBlock(const FeedbackState& feedback_state,
+                      uint32_t ssrc,
+                      StreamStatistician* statistician)
       EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
 
-  bool PrepareReportBlock(const FeedbackState& feedback_state,
-                          uint32_t ssrc,
-                          StreamStatistician* statistician,
-                          RTCPReportBlock* report_block);
-
   rtc::scoped_ptr<rtcp::RtcpPacket> BuildSR(const RtcpContext& context)
       EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
   rtc::scoped_ptr<rtcp::RtcpPacket> BuildRR(const RtcpContext& context)