[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}
diff --git a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc
index f089d36..24f1e2c 100644
--- a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc
@@ -259,6 +259,7 @@
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 =
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
index c29ab85..a4f6f39 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -453,26 +453,6 @@
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 @@
StatisticianMap statisticians =
receive_statistics_->GetActiveStatisticians();
- if (!statisticians.empty()) {
- for (auto it = statisticians.begin(); it != statisticians.end(); ++it) {
- RTCPReportBlock report_block;
- 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);
- }
- }
+ RTC_DCHECK(report_blocks_.empty());
+ for (auto& it : statisticians) {
+ AddReportBlock(feedback_state, it.first, it.second);
}
}
}
-bool RTCPSender::PrepareReportBlock(const FeedbackState& feedback_state,
- uint32_t ssrc,
- StreamStatistician* statistician,
- RTCPReportBlock* report_block) {
+bool RTCPSender::AddReportBlock(const FeedbackState& feedback_state,
+ uint32_t ssrc,
+ StreamStatistician* statistician) {
// 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;
- report_block->extendedHighSeqNum = stats.extended_max_sequence_number;
- report_block->jitter = stats.jitter;
- report_block->remoteSSRC = ssrc;
+
+ if (report_blocks_.size() >= RTCP_MAX_REPORT_BLOCKS) {
+ LOG(LS_WARNING) << "Too many report blocks.";
+ return false;
+ }
+ 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 @@
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 @@
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;
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
index 11a389e..dd3aec4 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
@@ -158,14 +158,11 @@
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)