Fix issue where the number of packets reported in tests/simulations sometimes are negative.
BUG=webrtc:6159
Review-Url: https://codereview.webrtc.org/2223033002
Cr-Commit-Position: refs/heads/master@{#13850}
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
index 8a2464d..d283796 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -15,6 +15,8 @@
#include <map>
#include <utility>
+#include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
namespace webrtc {
@@ -43,22 +45,28 @@
// Compute the a weighted average of the fraction loss from all report
// blocks.
- for (ReportBlockList::const_iterator it = report_blocks.begin();
- it != report_blocks.end(); ++it) {
+ for (const RTCPReportBlock& report_block : report_blocks) {
std::map<uint32_t, uint32_t>::iterator seq_num_it =
- ssrc_to_last_received_extended_high_seq_num_.find(it->sourceSSRC);
+ ssrc_to_last_received_extended_high_seq_num_.find(
+ report_block.sourceSSRC);
int number_of_packets = 0;
- if (seq_num_it != ssrc_to_last_received_extended_high_seq_num_.end())
- number_of_packets = it->extendedHighSeqNum -
- seq_num_it->second;
+ if (seq_num_it != ssrc_to_last_received_extended_high_seq_num_.end()) {
+ number_of_packets =
+ report_block.extendedHighSeqNum - seq_num_it->second;
+ }
- fraction_lost_aggregate += number_of_packets * it->fractionLost;
+ fraction_lost_aggregate += number_of_packets * report_block.fractionLost;
total_number_of_packets += number_of_packets;
// Update last received for this SSRC.
- ssrc_to_last_received_extended_high_seq_num_[it->sourceSSRC] =
- it->extendedHighSeqNum;
+ ssrc_to_last_received_extended_high_seq_num_[report_block.sourceSSRC] =
+ report_block.extendedHighSeqNum;
+ }
+ if (total_number_of_packets < 0) {
+ LOG(LS_WARNING) << "Received report block where extended high sequence "
+ "number goes backwards, ignoring.";
+ return;
}
if (total_number_of_packets == 0)
fraction_lost_aggregate = 0;
@@ -68,6 +76,8 @@
if (fraction_lost_aggregate > 255)
return;
+ RTC_DCHECK_GE(total_number_of_packets, 0);
+
owner_->OnReceivedRtcpReceiverReport(fraction_lost_aggregate, rtt,
total_number_of_packets, now_ms);
}
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
index b40332e..a70eecf 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -152,14 +152,14 @@
time_ms += 1000;
report_blocks.clear();
- report_blocks.push_back(CreateReportBlock(1, 2, 0, 801));
+ report_blocks.push_back(CreateReportBlock(1, 2, 0, 101));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms);
EXPECT_EQ(299732, bitrate_observer_.last_bitrate_);
time_ms += 1000;
// Reach max cap.
report_blocks.clear();
- report_blocks.push_back(CreateReportBlock(1, 2, 0, 101));
+ report_blocks.push_back(CreateReportBlock(1, 2, 0, 121));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms);
EXPECT_EQ(300000, bitrate_observer_.last_bitrate_);
time_ms += 1000;
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
index b1f364b..c12b269 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc
@@ -113,11 +113,11 @@
StatisticianMap statisticians = recv_stats_->GetActiveStatisticians();
RTCPReportBlock report_block;
if (!statisticians.empty()) {
- report_block = BuildReportBlock(statisticians.begin()->second);
+ latest_report_block_ = BuildReportBlock(statisticians.begin()->second);
}
feedback = new RembFeedback(flow_id_, now_ms * 1000, last_feedback_ms_,
- estimated_bps, report_block);
+ estimated_bps, latest_report_block_);
last_feedback_ms_ = now_ms;
double estimated_kbps = static_cast<double>(estimated_bps) / 1000.0;
@@ -137,8 +137,7 @@
StreamStatistician* statistician) {
RTCPReportBlock report_block;
RtcpStatistics stats;
- if (!statistician->GetStatistics(&stats, true))
- return report_block;
+ RTC_DCHECK(statistician->GetStatistics(&stats, true));
report_block.fractionLost = stats.fraction_lost;
report_block.cumulativeLost = stats.cumulative_lost;
report_block.extendedHighSeqNum = stats.extended_max_sequence_number;
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h
index d9e4f57..6bf5721 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h
@@ -76,6 +76,7 @@
int64_t latest_estimate_bps_;
int64_t last_feedback_ms_;
std::unique_ptr<RemoteBitrateEstimator> estimator_;
+ RTCPReportBlock latest_report_block_;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RembReceiver);
};
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
index 777761a..0b93128 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
@@ -10,6 +10,8 @@
#include "webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h"
+#include <algorithm>
+
#include "webrtc/base/logging.h"
#include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h"
#include "webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h"
@@ -78,8 +80,10 @@
static_cast<int>(fb.packet_feedback_vector().size());
report_block_.fractionLost = (lost_packets << 8) / expected_packets;
report_block_.cumulativeLost += lost_packets;
+ uint32_t unwrapped = seq_num_unwrapper_.Unwrap(
+ packet_feedback_vector.back().sequence_number);
report_block_.extendedHighSeqNum =
- packet_feedback_vector.back().sequence_number;
+ std::max(unwrapped, report_block_.extendedHighSeqNum);
ReportBlockList report_blocks;
report_blocks.push_back(report_block_);
feedback_observer_->OnReceivedRtcpReceiverReport(
diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h
index bbe3eac..aa7914e 100644
--- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h
+++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h
@@ -47,6 +47,7 @@
bool has_received_ack_;
uint16_t last_acked_seq_num_;
int64_t last_log_time_ms_;
+ SequenceNumberUnwrapper seq_num_unwrapper_;
::testing::NiceMock<MockRtcEventLog> event_log_;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(FullBweSender);
