diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index a7d2788ff8..641f15d986 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -92,10 +92,9 @@ void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { last_seen_packet_ms_ = now_ms; if (info.probe_cluster_id != PacketInfo::kNotAProbe) { - ProbingResult probe_result = - probe_bitrate_estimator_.PacketFeedback(info); - if (probe_result.valid()) { - remote_rate_.SetEstimate(probe_result.bps, probe_result.timestamp); + int bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); + if (bps > 0) { + remote_rate_.SetEstimate(bps, info.arrival_time_ms); update_estimate = true; } } diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc index 561fde9391..1378296628 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc @@ -12,40 +12,35 @@ #include +#include "webrtc/base/checks.h" #include "webrtc/base/logging.h" namespace { -// Max number of saved clusters. -constexpr size_t kMaxNumSavedClusters = 5; - // The minumum number of probes we need for a valid cluster. constexpr int kMinNumProbesValidCluster = 4; // The maximum (receive rate)/(send rate) ratio for a valid estimate. constexpr float kValidRatio = 1.2f; -} + +// The maximum time period over which the cluster history is retained. +// This is also the maximum time period beyond which a probing burst is not +// expected to last. +constexpr int kMaxClusterHistoryMs = 1000; + +// The maximum time interval between first and the last probe on a cluster +// on the sender side as well as the receive side. +constexpr int kMaxProbeIntervalMs = 1000; +} // namespace namespace webrtc { -ProbingResult::ProbingResult() : bps(kNoEstimate), timestamp(0) {} +ProbeBitrateEstimator::ProbeBitrateEstimator() {} -ProbingResult::ProbingResult(int bps, int64_t timestamp) - : bps(bps), timestamp(timestamp) {} - -bool ProbingResult::valid() const { - return bps != kNoEstimate; -} - -ProbeBitrateEstimator::ProbeBitrateEstimator() : last_valid_cluster_id_(0) {} - -ProbingResult ProbeBitrateEstimator::PacketFeedback( +int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( const PacketInfo& packet_info) { - // If this is not a probing packet or if this probing packet - // belongs to an old cluster, do nothing. - if (packet_info.probe_cluster_id == PacketInfo::kNotAProbe || - packet_info.probe_cluster_id < last_valid_cluster_id_) { - return ProbingResult(); - } + RTC_DCHECK_NE(packet_info.probe_cluster_id, PacketInfo::kNotAProbe); + + EraseOldClusters(packet_info.arrival_time_ms - kMaxClusterHistoryMs); AggregatedCluster* cluster = &clusters_[packet_info.probe_cluster_id]; cluster->first_send_ms = @@ -57,14 +52,10 @@ ProbingResult ProbeBitrateEstimator::PacketFeedback( cluster->last_receive_ms = std::max(cluster->last_receive_ms, packet_info.arrival_time_ms); cluster->size += packet_info.payload_size * 8; - cluster->num_probes += 1; - - // Clean up old clusters. - while (clusters_.size() > kMaxNumSavedClusters) - clusters_.erase(clusters_.begin()); + ++cluster->num_probes; if (cluster->num_probes < kMinNumProbesValidCluster) - return ProbingResult(); + return -1; float send_interval_ms = cluster->last_send_ms - cluster->first_send_ms; float receive_interval_ms = @@ -78,13 +69,13 @@ ProbingResult ProbeBitrateEstimator::PacketFeedback( send_interval_ms *= interval_correction; receive_interval_ms *= interval_correction; - if (send_interval_ms == 0 || receive_interval_ms == 0) { + if (send_interval_ms <= 0 || send_interval_ms > kMaxProbeIntervalMs || + receive_interval_ms <= 0 || receive_interval_ms > kMaxProbeIntervalMs) { LOG(LS_INFO) << "Probing unsuccessful, invalid send/receive interval" << " [cluster id: " << packet_info.probe_cluster_id << "] [send interval: " << send_interval_ms << " ms]" << " [receive interval: " << receive_interval_ms << " ms]"; - - return ProbingResult(); + return -1; } float send_bps = static_cast(cluster->size) / send_interval_ms * 1000; float receive_bps = @@ -101,12 +92,8 @@ ProbingResult ProbeBitrateEstimator::PacketFeedback( << " [ratio: " << receive_bps / 1000 << " / " << send_bps / 1000 << " = " << ratio << " > kValidRatio (" << kValidRatio << ")]"; - - return ProbingResult(); + return -1; } - // We have a valid estimate. - int result_bps = std::min(send_bps, receive_bps); - last_valid_cluster_id_ = packet_info.probe_cluster_id; LOG(LS_INFO) << "Probing successful" << " [cluster id: " << packet_info.probe_cluster_id << "] [send: " << cluster->size << " bytes / " @@ -114,7 +101,16 @@ ProbingResult ProbeBitrateEstimator::PacketFeedback( << " [receive: " << cluster->size << " bytes / " << receive_interval_ms << " ms = " << receive_bps / 1000 << " kb/s]"; + return std::min(send_bps, receive_bps); +} - return ProbingResult(result_bps, packet_info.arrival_time_ms); +void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) { + for (auto it = clusters_.begin(); it != clusters_.end();) { + if (it->second.last_receive_ms < timestamp_ms) { + it = clusters_.erase(it); + } else { + ++it; + } + } } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h index 7f0c4569c9..e6f77c0345 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h @@ -18,25 +18,13 @@ namespace webrtc { -struct ProbingResult { - static constexpr int kNoEstimate = -1; - - ProbingResult(); - ProbingResult(int bps, int64_t timestamp); - bool valid() const; - - int bps; - int64_t timestamp; -}; - class ProbeBitrateEstimator { public: ProbeBitrateEstimator(); - // Should be called for every packet we receive feedback about. If the - // packet was used for probing it will validate/calculate the resulting - // bitrate and return the result. - ProbingResult PacketFeedback(const PacketInfo& packet_info); + // Should be called for every probe packet we receive feedback about. + // Returns the estimated bitrate if the probe completes a valid cluster. + int HandleProbeAndEstimateBitrate(const PacketInfo& packet_info); private: struct AggregatedCluster { @@ -48,8 +36,10 @@ class ProbeBitrateEstimator { size_t size = 0; }; + // Erases old cluster data that was seen before |timestamp_ms|. + void EraseOldClusters(int64_t timestamp_ms); + std::map clusters_; - int last_valid_cluster_id_; }; } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc index e3bbd5a227..d895e32f58 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc @@ -19,6 +19,8 @@ namespace webrtc { +constexpr int INVALID_BPS = -1; + class TestProbeBitrateEstimator : public ::testing::Test { public: TestProbeBitrateEstimator() : probe_bitrate_estimator_() {} @@ -29,19 +31,12 @@ class TestProbeBitrateEstimator : public ::testing::Test { int64_t arrival_time_ms) { PacketInfo info(arrival_time_ms, send_time_ms, 0, size_bytes, probe_cluster_id); - ProbingResult res = probe_bitrate_estimator_.PacketFeedback(info); - if (res.valid()) - results_.emplace_back(res.bps, res.timestamp); - } - - void CheckResult(size_t index, int bps, int max_diff, int64_t timestamp) { - ASSERT_GT(results_.size(), index); - EXPECT_NEAR(results_[index].first, bps, max_diff); - EXPECT_EQ(results_[index].second, timestamp); + measured_bps_ = + probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); } protected: - std::vector> results_; + int measured_bps_ = INVALID_BPS; ProbeBitrateEstimator probe_bitrate_estimator_; }; @@ -51,7 +46,7 @@ TEST_F(TestProbeBitrateEstimator, OneCluster) { AddPacketFeedback(0, 1000, 20, 30); AddPacketFeedback(0, 1000, 30, 40); - CheckResult(0, 800000, 10, 40); + EXPECT_NEAR(measured_bps_, 800000, 10); } TEST_F(TestProbeBitrateEstimator, FastReceive) { @@ -60,7 +55,7 @@ TEST_F(TestProbeBitrateEstimator, FastReceive) { AddPacketFeedback(0, 1000, 20, 35); AddPacketFeedback(0, 1000, 30, 40); - CheckResult(0, 800000, 10, 40); + EXPECT_NEAR(measured_bps_, 800000, 10); } TEST_F(TestProbeBitrateEstimator, TooFastReceive) { @@ -69,7 +64,7 @@ TEST_F(TestProbeBitrateEstimator, TooFastReceive) { AddPacketFeedback(0, 1000, 20, 40); AddPacketFeedback(0, 1000, 40, 50); - EXPECT_TRUE(results_.empty()); + EXPECT_EQ(measured_bps_, INVALID_BPS); } TEST_F(TestProbeBitrateEstimator, SlowReceive) { @@ -78,7 +73,7 @@ TEST_F(TestProbeBitrateEstimator, SlowReceive) { AddPacketFeedback(0, 1000, 20, 70); AddPacketFeedback(0, 1000, 30, 85); - CheckResult(0, 320000, 10, 85); + EXPECT_NEAR(measured_bps_, 320000, 10); } TEST_F(TestProbeBitrateEstimator, BurstReceive) { @@ -87,7 +82,7 @@ TEST_F(TestProbeBitrateEstimator, BurstReceive) { AddPacketFeedback(0, 1000, 20, 50); AddPacketFeedback(0, 1000, 40, 50); - EXPECT_TRUE(results_.empty()); + EXPECT_EQ(measured_bps_, INVALID_BPS); } TEST_F(TestProbeBitrateEstimator, MultipleClusters) { @@ -95,20 +90,22 @@ TEST_F(TestProbeBitrateEstimator, MultipleClusters) { AddPacketFeedback(0, 1000, 10, 20); AddPacketFeedback(0, 1000, 20, 30); AddPacketFeedback(0, 1000, 40, 60); + + EXPECT_NEAR(measured_bps_, 480000, 10); + AddPacketFeedback(0, 1000, 50, 60); - CheckResult(0, 480000, 10, 60); - CheckResult(1, 640000, 10, 60); + EXPECT_NEAR(measured_bps_, 640000, 10); AddPacketFeedback(1, 1000, 60, 70); AddPacketFeedback(1, 1000, 65, 77); AddPacketFeedback(1, 1000, 70, 84); AddPacketFeedback(1, 1000, 75, 90); - CheckResult(2, 1200000, 10, 90); + EXPECT_NEAR(measured_bps_, 1200000, 10); } -TEST_F(TestProbeBitrateEstimator, OldProbe) { +TEST_F(TestProbeBitrateEstimator, IgnoreOldClusters) { AddPacketFeedback(0, 1000, 0, 10); AddPacketFeedback(0, 1000, 10, 20); AddPacketFeedback(0, 1000, 20, 30); @@ -118,11 +115,12 @@ TEST_F(TestProbeBitrateEstimator, OldProbe) { AddPacketFeedback(1, 1000, 70, 84); AddPacketFeedback(1, 1000, 75, 90); - CheckResult(0, 1200000, 10, 90); + EXPECT_NEAR(measured_bps_, 1200000, 10); - AddPacketFeedback(0, 1000, 40, 60); + // Coming in 6s later + AddPacketFeedback(0, 1000, 40 + 6000, 60 + 6000); - EXPECT_EQ(1ul, results_.size()); + EXPECT_EQ(measured_bps_, INVALID_BPS); } } // namespace webrtc