Remove GetFeedbackInterval in sender side BWE.
And changed the minimum increase rate in |aimd_rate_control| to prevent the system from overusing on short twcc report send interval.
BUG=webrtc:6514
Review-Url: https://codereview.webrtc.org/2407143002
Cr-Original-Commit-Position: refs/heads/master@{#17794}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 8490f8af2112f7dfd45ce4009b05e167b2cae565
diff --git a/modules/congestion_controller/delay_based_bwe.cc b/modules/congestion_controller/delay_based_bwe.cc
index c091cb9..7312038 100644
--- a/modules/congestion_controller/delay_based_bwe.cc
+++ b/modules/congestion_controller/delay_based_bwe.cc
@@ -153,7 +153,6 @@
trendline_estimator_(),
detector_(),
receiver_incoming_bitrate_(),
- last_update_ms_(-1),
last_seen_packet_ms_(-1),
uma_recorded_(false),
probe_bitrate_estimator_(event_log),
@@ -189,27 +188,27 @@
BweNames::kBweNamesMax);
uma_recorded_ = true;
}
- Result aggregated_result;
+ bool overusing = false;
bool delayed_feedback = true;
for (const auto& packet_feedback : sorted_packet_feedback_vector) {
if (packet_feedback.send_time_ms < 0)
continue;
delayed_feedback = false;
- Result result = IncomingPacketFeedback(packet_feedback);
- if (result.updated)
- aggregated_result = result;
+ IncomingPacketFeedback(packet_feedback);
+ overusing |= detector_.State() == BandwidthUsage::kBwOverusing;
}
if (delayed_feedback) {
++consecutive_delayed_feedbacks_;
+ if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
+ consecutive_delayed_feedbacks_ = 0;
+ return OnLongFeedbackDelay(
+ sorted_packet_feedback_vector.back().arrival_time_ms);
+ }
} else {
consecutive_delayed_feedbacks_ = 0;
+ return MaybeUpdateEstimate(overusing);
}
- if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
- aggregated_result = OnLongFeedbackDelay(
- sorted_packet_feedback_vector.back().arrival_time_ms);
- consecutive_delayed_feedbacks_ = 0;
- }
- return aggregated_result;
+ return Result();
}
DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay(
@@ -229,7 +228,7 @@
return result;
}
-DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedback(
+void DelayBasedBwe::IncomingPacketFeedback(
const PacketFeedback& packet_feedback) {
int64_t now_ms = clock_->TimeInMilliseconds();
@@ -272,40 +271,36 @@
trendline_estimator_->num_of_deltas(),
packet_feedback.arrival_time_ms);
}
-
- int probing_bps = 0;
if (packet_feedback.pacing_info.probe_cluster_id !=
PacedPacketInfo::kNotAProbe) {
- probing_bps =
- probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback);
+ probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(packet_feedback);
}
+}
+
+DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate(bool overusing) {
+ Result result;
+ int64_t now_ms = clock_->TimeInMilliseconds();
+
rtc::Optional<uint32_t> acked_bitrate_bps =
receiver_incoming_bitrate_.bitrate_bps();
+ rtc::Optional<int> probe_bitrate_bps =
+ probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps();
// Currently overusing the bandwidth.
- if (detector_.State() == BandwidthUsage::kBwOverusing) {
+ if (overusing) {
if (acked_bitrate_bps &&
rate_control_.TimeToReduceFurther(now_ms, *acked_bitrate_bps)) {
- result.updated =
- UpdateEstimate(packet_feedback.arrival_time_ms, now_ms,
- acked_bitrate_bps, &result.target_bitrate_bps);
+ result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing,
+ &result.target_bitrate_bps);
}
- } else if (probing_bps > 0) {
- // No overuse, but probing measured a bitrate.
- rate_control_.SetEstimate(probing_bps, packet_feedback.arrival_time_ms);
- result.probe = true;
- result.updated =
- UpdateEstimate(packet_feedback.arrival_time_ms, now_ms,
- acked_bitrate_bps, &result.target_bitrate_bps);
- }
- if (!result.updated &&
- (last_update_ms_ == -1 ||
- now_ms - last_update_ms_ > rate_control_.GetFeedbackInterval())) {
- result.updated =
- UpdateEstimate(packet_feedback.arrival_time_ms, now_ms,
- acked_bitrate_bps, &result.target_bitrate_bps);
+ } else {
+ if (probe_bitrate_bps) {
+ rate_control_.SetEstimate(*probe_bitrate_bps, now_ms);
+ result.probe = true;
+ }
+ result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing,
+ &result.target_bitrate_bps);
}
if (result.updated) {
- last_update_ms_ = now_ms;
BWE_TEST_LOGGING_PLOT(1, "target_bitrate_bps", now_ms,
result.target_bitrate_bps);
if (event_log_ && (result.target_bitrate_bps != last_logged_bitrate_ ||
@@ -316,17 +311,18 @@
last_logged_state_ = detector_.State();
}
}
-
return result;
}
-bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms,
- int64_t now_ms,
+bool DelayBasedBwe::UpdateEstimate(int64_t now_ms,
rtc::Optional<uint32_t> acked_bitrate_bps,
+ bool overusing,
uint32_t* target_bitrate_bps) {
// TODO(terelius): RateControlInput::noise_var is deprecated and will be
// removed. In the meantime, we set it to zero.
- const RateControlInput input(detector_.State(), acked_bitrate_bps, 0);
+ const RateControlInput input(
+ overusing ? BandwidthUsage::kBwOverusing : detector_.State(),
+ acked_bitrate_bps, 0);
*target_bitrate_bps = rate_control_.Update(&input, now_ms);
return rate_control_.ValidEstimate();
}
diff --git a/modules/congestion_controller/delay_based_bwe.h b/modules/congestion_controller/delay_based_bwe.h
index c1c68c2..49a767f 100644
--- a/modules/congestion_controller/delay_based_bwe.h
+++ b/modules/congestion_controller/delay_based_bwe.h
@@ -77,13 +77,15 @@
float bitrate_estimate_var_;
};
- Result IncomingPacketFeedback(const PacketFeedback& packet_feedback);
+ void IncomingPacketFeedback(const PacketFeedback& packet_feedback);
Result OnLongFeedbackDelay(int64_t arrival_time_ms);
+
+ Result MaybeUpdateEstimate(bool overusing);
// Updates the current remote rate estimate and returns true if a valid
// estimate exists.
- bool UpdateEstimate(int64_t packet_arrival_time_ms,
- int64_t now_ms,
+ bool UpdateEstimate(int64_t now_ms,
rtc::Optional<uint32_t> acked_bitrate_bps,
+ bool overusing,
uint32_t* target_bitrate_bps);
rtc::ThreadChecker network_thread_;
@@ -93,7 +95,6 @@
std::unique_ptr<TrendlineEstimator> trendline_estimator_;
OveruseDetector detector_;
BitrateEstimator receiver_incoming_bitrate_;
- int64_t last_update_ms_;
int64_t last_seen_packet_ms_;
bool uma_recorded_;
AimdRateControl rate_control_;
diff --git a/modules/congestion_controller/delay_based_bwe_unittest.cc b/modules/congestion_controller/delay_based_bwe_unittest.cc
index fae4656..dcec0f8 100644
--- a/modules/congestion_controller/delay_based_bwe_unittest.cc
+++ b/modules/congestion_controller/delay_based_bwe_unittest.cc
@@ -140,18 +140,18 @@
}
TEST_F(DelayBasedBweTest, InitialBehavior) {
- InitialBehaviorTestHelper(674840);
+ InitialBehaviorTestHelper(730000);
}
TEST_F(DelayBasedBweTest, RateIncreaseReordering) {
- RateIncreaseReorderingTestHelper(674840);
+ RateIncreaseReorderingTestHelper(730000);
}
TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) {
- RateIncreaseRtpTimestampsTestHelper(1288);
+ RateIncreaseRtpTimestampsTestHelper(627);
}
TEST_F(DelayBasedBweTest, CapacityDropOneStream) {
- CapacityDropTestHelper(1, false, 333, 0);
+ CapacityDropTestHelper(1, false, 300, 0);
}
TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) {
@@ -159,12 +159,13 @@
}
TEST_F(DelayBasedBweTest, CapacityDropNegOffsetChange) {
- CapacityDropTestHelper(1, false, 867, -30000);
+ CapacityDropTestHelper(1, false, 933, -30000);
}
TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) {
CapacityDropTestHelper(1, true, 333, 0);
}
+
TEST_F(DelayBasedBweTest, TestTimestampGrouping) {
TestTimestampGroupingTestHelper();
}
diff --git a/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/modules/congestion_controller/delay_based_bwe_unittest_helper.cc
index e7a98c3..479ecb4 100644
--- a/modules/congestion_controller/delay_based_bwe_unittest_helper.cc
+++ b/modules/congestion_controller/delay_based_bwe_unittest_helper.cc
@@ -492,7 +492,7 @@
clock_.AdvanceTimeMilliseconds(silence_time_s * 1000);
send_time_ms += silence_time_s * 1000;
- for (size_t i = 0; i < 23; ++i) {
+ for (size_t i = 0; i < 24; ++i) {
IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms,
sequence_number++, 1000);
clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs);
diff --git a/modules/congestion_controller/probe_bitrate_estimator.cc b/modules/congestion_controller/probe_bitrate_estimator.cc
index 8406387..2d66198 100644
--- a/modules/congestion_controller/probe_bitrate_estimator.cc
+++ b/modules/congestion_controller/probe_bitrate_estimator.cc
@@ -133,10 +133,19 @@
<< " [receive: " << receive_size << " bytes / "
<< receive_interval_ms << " ms = " << receive_bps / 1000
<< " kb/s]";
+
float res = std::min(send_bps, receive_bps);
if (event_log_)
event_log_->LogProbeResultSuccess(cluster_id, res);
- return res;
+ estimated_bitrate_bps_ = rtc::Optional<int>(std::min(send_bps, res));
+ return *estimated_bitrate_bps_;
+}
+
+rtc::Optional<int>
+ProbeBitrateEstimator::FetchAndResetLastEstimatedBitrateBps() {
+ rtc::Optional<int> estimated_bitrate_bps = estimated_bitrate_bps_;
+ estimated_bitrate_bps_.reset();
+ return estimated_bitrate_bps;
}
void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) {
diff --git a/modules/congestion_controller/probe_bitrate_estimator.h b/modules/congestion_controller/probe_bitrate_estimator.h
index 247e905..7286f72 100644
--- a/modules/congestion_controller/probe_bitrate_estimator.h
+++ b/modules/congestion_controller/probe_bitrate_estimator.h
@@ -27,6 +27,8 @@
// Returns the estimated bitrate if the probe completes a valid cluster.
int HandleProbeAndEstimateBitrate(const PacketFeedback& packet_feedback);
+ rtc::Optional<int> FetchAndResetLastEstimatedBitrateBps();
+
private:
struct AggregatedCluster {
int num_probes = 0;
@@ -44,6 +46,7 @@
std::map<int, AggregatedCluster> clusters_;
RtcEventLog* const event_log_;
+ rtc::Optional<int> estimated_bitrate_bps_;
};
} // namespace webrtc
diff --git a/modules/congestion_controller/probe_bitrate_estimator_unittest.cc b/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
index f86c93f..5686f57 100644
--- a/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
+++ b/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
@@ -193,4 +193,21 @@
EXPECT_NEAR(measured_bps_, 800000, 10);
}
+TEST_F(TestProbeBitrateEstimator, NoLastEstimatedBitrateBps) {
+ EXPECT_FALSE(probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps());
+}
+
+TEST_F(TestProbeBitrateEstimator, FetchLastEstimatedBitrateBps) {
+ AddPacketFeedback(0, 1000, 0, 10);
+ AddPacketFeedback(0, 1000, 10, 20);
+ AddPacketFeedback(0, 1000, 20, 30);
+ AddPacketFeedback(0, 1000, 30, 40);
+
+ auto estimated_bitrate_bps =
+ probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps();
+ EXPECT_TRUE(estimated_bitrate_bps);
+ EXPECT_NEAR(*estimated_bitrate_bps, 800000, 10);
+ EXPECT_FALSE(probe_bitrate_estimator_.FetchAndResetLastEstimatedBitrateBps());
+}
+
} // namespace webrtc
