Adds trial to fall back to probe rate if ack rate is missing.
Bug: webrtc:9718
Change-Id: I7b6e1d3c051e67b97f6de1ec95e84631af9c5b0d
Reviewed-on: https://webrtc-review.googlesource.com/c/113600
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25953}diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
index c47922c..c99a6c9 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -136,6 +136,8 @@
safe_reset_acknowledged_rate_("ack"),
use_stable_bandwidth_estimate_(
field_trial::IsEnabled("WebRTC-Bwe-StableBandwidthEstimate")),
+ fall_back_to_probe_rate_(
+ field_trial::IsEnabled("WebRTC-Bwe-ProbeRateFallback")),
probe_controller_(new ProbeController()),
congestion_window_pushback_controller_(
MaybeInitalizeCongestionWindowPushbackController()),
@@ -486,9 +488,6 @@
acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(
received_feedback_vector);
auto acknowledged_bitrate = acknowledged_bitrate_estimator_->bitrate();
- bandwidth_estimation_->SetAcknowledgedRate(acknowledged_bitrate,
- report.feedback_time);
- bandwidth_estimation_->IncomingPacketFeedbackVector(report);
for (const auto& feedback : received_feedback_vector) {
if (feedback.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) {
probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(feedback);
@@ -496,7 +495,11 @@
}
absl::optional<DataRate> probe_bitrate =
probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate();
-
+ if (fall_back_to_probe_rate_ && !acknowledged_bitrate)
+ acknowledged_bitrate = probe_bitrate_estimator_->last_estimate();
+ bandwidth_estimation_->SetAcknowledgedRate(acknowledged_bitrate,
+ report.feedback_time);
+ bandwidth_estimation_->IncomingPacketFeedbackVector(report);
DelayBasedBwe::Result result;
result = delay_based_bwe_->IncomingPacketFeedbackVector(
received_feedback_vector, acknowledged_bitrate, probe_bitrate,
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
index 94aad4c..057ed12 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
@@ -72,6 +72,7 @@
FieldTrialFlag safe_reset_on_route_change_;
FieldTrialFlag safe_reset_acknowledged_rate_;
const bool use_stable_bandwidth_estimate_;
+ const bool fall_back_to_probe_rate_;
const std::unique_ptr<ProbeController> probe_controller_;
const std::unique_ptr<CongestionWindowPushbackController>
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
index 70a5a3c..e892664 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
@@ -74,12 +74,10 @@
EXPECT_NEAR(client->send_bandwidth().kbps(), kStartRate.kbps(), 30);
}
-// This test is flaky because probing on route change can trigger overuse
-// without having any acknowledged rate, causing a 50% backoff from the probe
-// rate.
-// TODO(srte): Add a fix for the above problem and enable this test.
-TEST(GoogCcNetworkControllerTest, DISABLED_DetectsHighRateInSafeResetTrial) {
- ScopedFieldTrials trial("WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/");
+TEST(GoogCcNetworkControllerTest, DetectsHighRateInSafeResetTrial) {
+ ScopedFieldTrials trial(
+ "WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/"
+ "WebRTC-Bwe-ProbeRateFallback/Enabled/");
const DataRate kInitialLinkCapacity = DataRate::kbps(200);
const DataRate kNewLinkCapacity = DataRate::kbps(800);
const DataRate kStartRate = DataRate::kbps(300);
@@ -113,7 +111,7 @@
EXPECT_NEAR(client->send_bandwidth().kbps(), kInitialLinkCapacity.kbps(), 50);
// However, probing should have made us detect the higher rate.
s.RunFor(TimeDelta::ms(2000));
- EXPECT_NEAR(client->send_bandwidth().kbps(), kNewLinkCapacity.kbps(), 200);
+ EXPECT_GT(client->send_bandwidth().kbps(), kNewLinkCapacity.kbps() - 300);
}
} // namespace test
diff --git a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc
index 7ba2fb5..380e7d3 100644
--- a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc
+++ b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc
@@ -166,6 +166,7 @@
event_log_->Log(
absl::make_unique<RtcEventProbeResultSuccess>(cluster_id, res));
}
+ last_estimate_ = DataRate::bps(res);
estimated_bitrate_bps_ = res;
return *estimated_bitrate_bps_;
}
@@ -179,6 +180,10 @@
return absl::nullopt;
}
+absl::optional<DataRate> ProbeBitrateEstimator::last_estimate() const {
+ return last_estimate_;
+}
+
void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) {
for (auto it = clusters_.begin(); it != clusters_.end();) {
if (it->second.last_receive_ms < timestamp_ms) {
diff --git a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h
index ae83ed3..ce4eb99 100644
--- a/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h
+++ b/modules/congestion_controller/goog_cc/probe_bitrate_estimator.h
@@ -14,6 +14,8 @@
#include <limits>
#include <map>
+#include "absl/types/optional.h"
+#include "api/units/data_rate.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
namespace webrtc {
@@ -30,6 +32,8 @@
absl::optional<DataRate> FetchAndResetLastEstimatedBitrate();
+ absl::optional<DataRate> last_estimate() const;
+
private:
struct AggregatedCluster {
int num_probes = 0;
@@ -48,6 +52,7 @@
std::map<int, AggregatedCluster> clusters_;
RtcEventLog* const event_log_;
absl::optional<int> estimated_bitrate_bps_;
+ absl::optional<DataRate> last_estimate_;
};
} // namespace webrtc
