Reland "Ignore allocated bitrate during initial exponential BWE."
This reverts commit 501c4f37bfee47b26999ee291c5355ad64554df7.
Patch set 1 contains pure reland.
The reason why we want to do this is because audio can allocate a needed bitrate before video when starting a call, which may lead to a race between the first probe result and updating the allocated bitrate.
That is the, initial probe will try to probe up to the max configured bitrate.
Bug: webrtc:14928
Change-Id: I6a8660da20ac54237f04a29461e03b31bd988bb0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347643
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Erik Språng <sprang@google.com>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42086}
diff --git a/api/transport/network_types.h b/api/transport/network_types.h
index 258a3a4..46470d6 100644
--- a/api/transport/network_types.h
+++ b/api/transport/network_types.h
@@ -46,6 +46,10 @@
~StreamsConfig();
Timestamp at_time = Timestamp::PlusInfinity();
absl::optional<bool> requests_alr_probing;
+ // If `initial_probe_to_max_bitrate` is set to true, the first probe
+ // may probe up to the max configured bitrate and can ignore
+ // max_total_allocated_bitrate.
+ absl::optional<bool> initial_probe_to_max_bitrate;
absl::optional<double> pacing_factor;
// TODO(srte): Use BitrateAllocationLimits here.
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 0ee6751..d739ed1 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -263,6 +263,11 @@
RTC_DCHECK_RUN_ON(&sequence_checker_);
bwe_settings_ = settings;
+ bool allow_probe_without_media = bwe_settings_.allow_probe_without_media &&
+ packet_router_.SupportsRtxPayloadPadding();
+ streams_config_.initial_probe_to_max_bitrate = allow_probe_without_media;
+ pacer_.SetAllowProbeWithoutMediaPacket(allow_probe_without_media);
+
if (controller_) {
// Recreate the controller and handler.
control_handler_ = nullptr;
@@ -276,9 +281,6 @@
UpdateNetworkAvailability();
}
}
- pacer_.SetAllowProbeWithoutMediaPacket(
- bwe_settings_.allow_probe_without_media &&
- packet_router_.SupportsRtxPayloadPadding());
}
void RtpTransportControllerSend::RegisterTargetTransferRateObserver(
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 d8a0ce9..815520a 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -218,6 +218,10 @@
probe_controller_->EnablePeriodicAlrProbing(
*initial_config_->stream_based_config.requests_alr_probing);
}
+ if (initial_config_->stream_based_config.initial_probe_to_max_bitrate) {
+ probe_controller_->SetFirstProbeToMaxBitrate(
+ *initial_config_->stream_based_config.initial_probe_to_max_bitrate);
+ }
absl::optional<DataRate> total_bitrate =
initial_config_->stream_based_config.max_total_allocated_bitrate;
if (total_bitrate) {
diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc
index 3fc8677..36d61bf 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller.cc
@@ -272,6 +272,22 @@
return std::vector<ProbeClusterConfig>();
}
+void ProbeController::UpdateState(State new_state) {
+ switch (new_state) {
+ case State::kInit:
+ state_ = State::kInit;
+ break;
+ case State::kWaitingForProbingResult:
+ state_ = State::kWaitingForProbingResult;
+ break;
+ case State::kProbingComplete:
+ state_ = State::kProbingComplete;
+ waiting_for_initial_probe_result_ = false;
+ min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
+ break;
+ }
+}
+
std::vector<ProbeClusterConfig> ProbeController::InitiateExponentialProbing(
Timestamp at_time) {
RTC_DCHECK(network_available_);
@@ -287,6 +303,8 @@
probes.push_back(config_.second_exponential_probe_scale.Value() *
start_bitrate_);
}
+ waiting_for_initial_probe_result_ = true;
+
return InitiateProbing(at_time, probes, true);
}
@@ -307,6 +325,7 @@
if (config_.abort_further_probe_if_max_lower_than_current &&
(bitrate > max_bitrate_ ||
(!max_total_allocated_bitrate_.IsZero() &&
+ !(waiting_for_initial_probe_result_ && first_probe_to_max_bitrate_) &&
bitrate > 2 * max_total_allocated_bitrate_))) {
// No need to continue probing.
min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
@@ -335,6 +354,11 @@
enable_periodic_alr_probing_ = enable;
}
+void ProbeController::SetFirstProbeToMaxBitrate(
+ bool first_probe_to_max_bitrate) {
+ first_probe_to_max_bitrate_ = first_probe_to_max_bitrate;
+}
+
void ProbeController::SetAlrStartTimeMs(
absl::optional<int64_t> alr_start_time_ms) {
if (alr_start_time_ms) {
@@ -391,6 +415,7 @@
void ProbeController::Reset(Timestamp at_time) {
bandwidth_limited_cause_ = BandwidthLimitedCause::kDelayBasedLimited;
state_ = State::kInit;
+ waiting_for_initial_probe_result_ = false;
min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
time_last_probing_initiated_ = Timestamp::Zero();
estimated_bitrate_ = DataRate::Zero();
@@ -452,8 +477,7 @@
kMaxWaitingTimeForProbingResult) {
if (state_ == State::kWaitingForProbingResult) {
RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout";
- state_ = State::kProbingComplete;
- min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
+ UpdateState(State::kProbingComplete);
}
}
if (estimated_bitrate_.IsZero() || state_ != State::kProbingComplete) {
@@ -480,14 +504,14 @@
: std::min(max_total_allocated_bitrate_, max_bitrate_);
if (std::min(network_estimate, estimated_bitrate_) >
config_.skip_if_estimate_larger_than_fraction_of_max * max_probe_rate) {
- state_ = State::kProbingComplete;
- min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
+ UpdateState(State::kProbingComplete);
return {};
}
}
DataRate max_probe_bitrate = max_bitrate_;
- if (max_total_allocated_bitrate_ > DataRate::Zero()) {
+ if (max_total_allocated_bitrate_ > DataRate::Zero() &&
+ !(first_probe_to_max_bitrate_ && waiting_for_initial_probe_result_)) {
// If a max allocated bitrate has been configured, allow probing up to 2x
// that rate. This allows some overhead to account for bursty streams,
// which otherwise would have to ramp up when the overshoot is already in
@@ -555,15 +579,14 @@
}
time_last_probing_initiated_ = now;
if (probe_further) {
- state_ = State::kWaitingForProbingResult;
+ UpdateState(State::kWaitingForProbingResult);
// Dont expect probe results to be larger than a fraction of the actual
// probe rate.
min_bitrate_to_probe_further_ =
std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) *
config_.further_probe_threshold;
} else {
- state_ = State::kProbingComplete;
- min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
+ UpdateState(State::kProbingComplete);
}
return pending_probes;
}
diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h
index cec6157..ec078ad 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.h
+++ b/modules/congestion_controller/goog_cc/probe_controller.h
@@ -121,6 +121,9 @@
Timestamp at_time);
void EnablePeriodicAlrProbing(bool enable);
+ // The first initial probe ignores allocated bitrate constraints and probe up
+ // to max configured bitrate configured via SetBitrates.
+ void SetFirstProbeToMaxBitrate(bool first_probe_to_max_bitrate);
void SetAlrStartTimeMs(absl::optional<int64_t> alr_start_time);
void SetAlrEndedTimeMs(int64_t alr_end_time);
@@ -148,6 +151,7 @@
kProbingComplete,
};
+ void UpdateState(State new_state);
ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig>
InitiateExponentialProbing(Timestamp at_time);
ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> InitiateProbing(
@@ -158,6 +162,8 @@
bool TimeForNetworkStateProbe(Timestamp at_time) const;
bool network_available_;
+ bool waiting_for_initial_probe_result_ = false;
+ bool first_probe_to_max_bitrate_ = false;
BandwidthLimitedCause bandwidth_limited_cause_ =
BandwidthLimitedCause::kDelayBasedLimited;
State state_;
diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
index aa62c47..8024fec 100644
--- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
@@ -327,6 +327,32 @@
EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800);
}
+TEST(ProbeControllerTest, ExponentialProbingStopIfMaxBitrateLow) {
+ ProbeControllerFixture fixture(
+ "WebRTC-Bwe-ProbingConfiguration/abort_further:true/");
+ std::unique_ptr<ProbeController> probe_controller =
+ fixture.CreateController();
+ ASSERT_THAT(
+ probe_controller->OnNetworkAvailability({.network_available = true}),
+ IsEmpty());
+ auto probes = probe_controller->SetBitrates(
+ kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+ ASSERT_THAT(probes, SizeIs(Gt(0)));
+
+ // Repeated probe normally is sent when estimated bitrate climbs above
+ // 0.7 * 6 * kStartBitrate = 1260. But since max bitrate is low, expect
+ // exponential probing to stop.
+ probes = probe_controller->SetBitrates(kMinBitrate, kStartBitrate,
+ /*max_bitrate=*/kStartBitrate,
+ fixture.CurrentTime());
+ EXPECT_THAT(probes, IsEmpty());
+
+ probes = probe_controller->SetEstimatedBitrate(
+ DataRate::BitsPerSec(1800), BandwidthLimitedCause::kDelayBasedLimited,
+ fixture.CurrentTime());
+ EXPECT_THAT(probes, IsEmpty());
+}
+
TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) {
ProbeControllerFixture fixture(
"WebRTC-Bwe-ProbingConfiguration/abort_further:true/");
@@ -352,6 +378,66 @@
EXPECT_THAT(probes, IsEmpty());
}
+TEST(ProbeControllerTest,
+ InitialProbingIgnoreLowMaxAllocatedbitrateIfSetFirstProbeToMaxBitrate) {
+ ProbeControllerFixture fixture(
+ "WebRTC-Bwe-ProbingConfiguration/abort_further:true/");
+ std::unique_ptr<ProbeController> probe_controller =
+ fixture.CreateController();
+ ASSERT_THAT(
+ probe_controller->OnNetworkAvailability({.network_available = true}),
+ IsEmpty());
+ auto probes = probe_controller->SetBitrates(
+ kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+ ASSERT_THAT(probes, SizeIs(Gt(0)));
+ probe_controller->SetFirstProbeToMaxBitrate(true);
+
+ // Repeated probe is sent when estimated bitrate climbs above
+ // 0.7 * 6 * kStartBitrate = 1260. During the initial probe, we ignore the
+ // allocation limit and probe up to the max.
+ probes = probe_controller->OnMaxTotalAllocatedBitrate(kStartBitrate,
+ fixture.CurrentTime());
+ EXPECT_THAT(probes, IsEmpty());
+
+ probes = probe_controller->SetEstimatedBitrate(
+ DataRate::BitsPerSec(1800), BandwidthLimitedCause::kDelayBasedLimited,
+ fixture.CurrentTime());
+ EXPECT_EQ(probes.size(), 1u);
+ EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800);
+
+ probes = probe_controller->SetEstimatedBitrate(
+ probes[0].target_data_rate, BandwidthLimitedCause::kDelayBasedLimited,
+ fixture.CurrentTime());
+ EXPECT_EQ(probes.size(), 1u);
+}
+
+TEST(ProbeControllerTest,
+ InitialProbingToLowMaxAllocatedbitrateIfNotSetFirstProbeToMaxBitrate) {
+ ProbeControllerFixture fixture;
+ std::unique_ptr<ProbeController> probe_controller =
+ fixture.CreateController();
+ ASSERT_THAT(
+ probe_controller->OnNetworkAvailability({.network_available = true}),
+ IsEmpty());
+ auto probes = probe_controller->SetBitrates(
+ kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+ ASSERT_THAT(probes, SizeIs(Gt(0)));
+
+ // Repeated probe is sent when estimated bitrate climbs above
+ // 0.7 * 6 * kStartBitrate = 1260.
+ probes = probe_controller->OnMaxTotalAllocatedBitrate(kStartBitrate,
+ fixture.CurrentTime());
+ EXPECT_THAT(probes, IsEmpty());
+
+ // If the inital probe result is received, a new probe is sent at 2x the
+ // needed max bitrate.
+ probes = probe_controller->SetEstimatedBitrate(
+ DataRate::BitsPerSec(1800), BandwidthLimitedCause::kDelayBasedLimited,
+ fixture.CurrentTime());
+ ASSERT_EQ(probes.size(), 1u);
+ EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * kStartBitrate.bps());
+}
+
TEST(ProbeControllerTest, TestExponentialProbingTimeout) {
ProbeControllerFixture fixture;
std::unique_ptr<ProbeController> probe_controller =
