Revert "Ignore allocated bitrate during initial exponential BWE."

This reverts commit 33cc83595a.

Reason for revert: Perf bots showed that this cl cause a change in metrics. It looks like it is for the better, but we want this to be behind a field trial. 

Original change's description:
> Ignore allocated bitrate during initial exponential BWE.
>
> 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.
>
> ProbeController::SetFirstProbeToMaxBitrate will allow the first probe to
> continue up to the max configured bitrate, regardless of of the max
> allocated bitrate.
>
> Bug: webrtc:14928
> Change-Id: I6e0ae90e21a78466527f3464951e6033dc846470
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346760
> Reviewed-by: Diep Bui <diepbp@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#42049}

Bug: webrtc:14928
Change-Id: I56ba58560b6857b6069552c02df822691f7af64d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347622
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42081}
This commit is contained in:
Per Kjellander 2024-04-16 09:08:29 +00:00 committed by WebRTC LUCI CQ
parent a57229bf36
commit 501c4f37bf
6 changed files with 11 additions and 109 deletions

View file

@ -46,10 +46,6 @@ struct StreamsConfig {
~StreamsConfig(); ~StreamsConfig();
Timestamp at_time = Timestamp::PlusInfinity(); Timestamp at_time = Timestamp::PlusInfinity();
absl::optional<bool> requests_alr_probing; 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; absl::optional<double> pacing_factor;
// TODO(srte): Use BitrateAllocationLimits here. // TODO(srte): Use BitrateAllocationLimits here.

View file

@ -263,11 +263,6 @@ void RtpTransportControllerSend::ReconfigureBandwidthEstimation(
RTC_DCHECK_RUN_ON(&sequence_checker_); RTC_DCHECK_RUN_ON(&sequence_checker_);
bwe_settings_ = settings; 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_) { if (controller_) {
// Recreate the controller and handler. // Recreate the controller and handler.
control_handler_ = nullptr; control_handler_ = nullptr;
@ -281,6 +276,9 @@ void RtpTransportControllerSend::ReconfigureBandwidthEstimation(
UpdateNetworkAvailability(); UpdateNetworkAvailability();
} }
} }
pacer_.SetAllowProbeWithoutMediaPacket(
bwe_settings_.allow_probe_without_media &&
packet_router_.SupportsRtxPayloadPadding());
} }
void RtpTransportControllerSend::RegisterTargetTransferRateObserver( void RtpTransportControllerSend::RegisterTargetTransferRateObserver(

View file

@ -218,10 +218,6 @@ NetworkControlUpdate GoogCcNetworkController::OnProcessInterval(
probe_controller_->EnablePeriodicAlrProbing( probe_controller_->EnablePeriodicAlrProbing(
*initial_config_->stream_based_config.requests_alr_probing); *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 = absl::optional<DataRate> total_bitrate =
initial_config_->stream_based_config.max_total_allocated_bitrate; initial_config_->stream_based_config.max_total_allocated_bitrate;
if (total_bitrate) { if (total_bitrate) {

View file

@ -272,22 +272,6 @@ std::vector<ProbeClusterConfig> ProbeController::OnNetworkAvailability(
return std::vector<ProbeClusterConfig>(); 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( std::vector<ProbeClusterConfig> ProbeController::InitiateExponentialProbing(
Timestamp at_time) { Timestamp at_time) {
RTC_DCHECK(network_available_); RTC_DCHECK(network_available_);
@ -303,8 +287,6 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateExponentialProbing(
probes.push_back(config_.second_exponential_probe_scale.Value() * probes.push_back(config_.second_exponential_probe_scale.Value() *
start_bitrate_); start_bitrate_);
} }
waiting_for_initial_probe_result_ = true;
return InitiateProbing(at_time, probes, true); return InitiateProbing(at_time, probes, true);
} }
@ -325,7 +307,6 @@ std::vector<ProbeClusterConfig> ProbeController::SetEstimatedBitrate(
if (config_.abort_further_probe_if_max_lower_than_current && if (config_.abort_further_probe_if_max_lower_than_current &&
(bitrate > max_bitrate_ || (bitrate > max_bitrate_ ||
(!max_total_allocated_bitrate_.IsZero() && (!max_total_allocated_bitrate_.IsZero() &&
!(waiting_for_initial_probe_result_ && first_probe_to_max_bitrate_) &&
bitrate > 2 * max_total_allocated_bitrate_))) { bitrate > 2 * max_total_allocated_bitrate_))) {
// No need to continue probing. // No need to continue probing.
min_bitrate_to_probe_further_ = DataRate::PlusInfinity(); min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
@ -354,11 +335,6 @@ void ProbeController::EnablePeriodicAlrProbing(bool enable) {
enable_periodic_alr_probing_ = enable; 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( void ProbeController::SetAlrStartTimeMs(
absl::optional<int64_t> alr_start_time_ms) { absl::optional<int64_t> alr_start_time_ms) {
if (alr_start_time_ms) { if (alr_start_time_ms) {
@ -415,7 +391,6 @@ void ProbeController::SetNetworkStateEstimate(
void ProbeController::Reset(Timestamp at_time) { void ProbeController::Reset(Timestamp at_time) {
bandwidth_limited_cause_ = BandwidthLimitedCause::kDelayBasedLimited; bandwidth_limited_cause_ = BandwidthLimitedCause::kDelayBasedLimited;
state_ = State::kInit; state_ = State::kInit;
waiting_for_initial_probe_result_ = false;
min_bitrate_to_probe_further_ = DataRate::PlusInfinity(); min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
time_last_probing_initiated_ = Timestamp::Zero(); time_last_probing_initiated_ = Timestamp::Zero();
estimated_bitrate_ = DataRate::Zero(); estimated_bitrate_ = DataRate::Zero();
@ -477,7 +452,8 @@ std::vector<ProbeClusterConfig> ProbeController::Process(Timestamp at_time) {
kMaxWaitingTimeForProbingResult) { kMaxWaitingTimeForProbingResult) {
if (state_ == State::kWaitingForProbingResult) { if (state_ == State::kWaitingForProbingResult) {
RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout"; RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout";
UpdateState(State::kProbingComplete); state_ = State::kProbingComplete;
min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
} }
} }
if (estimated_bitrate_.IsZero() || state_ != State::kProbingComplete) { if (estimated_bitrate_.IsZero() || state_ != State::kProbingComplete) {
@ -504,14 +480,14 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
: std::min(max_total_allocated_bitrate_, max_bitrate_); : std::min(max_total_allocated_bitrate_, max_bitrate_);
if (std::min(network_estimate, estimated_bitrate_) > if (std::min(network_estimate, estimated_bitrate_) >
config_.skip_if_estimate_larger_than_fraction_of_max * max_probe_rate) { config_.skip_if_estimate_larger_than_fraction_of_max * max_probe_rate) {
UpdateState(State::kProbingComplete); state_ = State::kProbingComplete;
min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
return {}; return {};
} }
} }
DataRate max_probe_bitrate = max_bitrate_; DataRate max_probe_bitrate = max_bitrate_;
if (max_total_allocated_bitrate_ > DataRate::Zero() && if (max_total_allocated_bitrate_ > DataRate::Zero()) {
!waiting_for_initial_probe_result_) {
// If a max allocated bitrate has been configured, allow probing up to 2x // If a max allocated bitrate has been configured, allow probing up to 2x
// that rate. This allows some overhead to account for bursty streams, // that rate. This allows some overhead to account for bursty streams,
// which otherwise would have to ramp up when the overshoot is already in // which otherwise would have to ramp up when the overshoot is already in
@ -579,14 +555,15 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
} }
time_last_probing_initiated_ = now; time_last_probing_initiated_ = now;
if (probe_further) { if (probe_further) {
UpdateState(State::kWaitingForProbingResult); state_ = State::kWaitingForProbingResult;
// Dont expect probe results to be larger than a fraction of the actual // Dont expect probe results to be larger than a fraction of the actual
// probe rate. // probe rate.
min_bitrate_to_probe_further_ = min_bitrate_to_probe_further_ =
std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) * std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) *
config_.further_probe_threshold; config_.further_probe_threshold;
} else { } else {
UpdateState(State::kProbingComplete); state_ = State::kProbingComplete;
min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
} }
return pending_probes; return pending_probes;
} }

View file

@ -121,9 +121,6 @@ class ProbeController {
Timestamp at_time); Timestamp at_time);
void EnablePeriodicAlrProbing(bool enable); 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 SetAlrStartTimeMs(absl::optional<int64_t> alr_start_time);
void SetAlrEndedTimeMs(int64_t alr_end_time); void SetAlrEndedTimeMs(int64_t alr_end_time);
@ -151,7 +148,6 @@ class ProbeController {
kProbingComplete, kProbingComplete,
}; };
void UpdateState(State new_state);
ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig>
InitiateExponentialProbing(Timestamp at_time); InitiateExponentialProbing(Timestamp at_time);
ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> InitiateProbing( ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> InitiateProbing(
@ -162,8 +158,6 @@ class ProbeController {
bool TimeForNetworkStateProbe(Timestamp at_time) const; bool TimeForNetworkStateProbe(Timestamp at_time) const;
bool network_available_; bool network_available_;
bool waiting_for_initial_probe_result_ = false;
bool first_probe_to_max_bitrate_ = false;
BandwidthLimitedCause bandwidth_limited_cause_ = BandwidthLimitedCause bandwidth_limited_cause_ =
BandwidthLimitedCause::kDelayBasedLimited; BandwidthLimitedCause::kDelayBasedLimited;
State state_; State state_;

View file

@ -327,32 +327,6 @@ TEST(ProbeControllerTest, TestExponentialProbing) {
EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800); 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) { TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) {
ProbeControllerFixture fixture( ProbeControllerFixture fixture(
"WebRTC-Bwe-ProbingConfiguration/abort_further:true/"); "WebRTC-Bwe-ProbingConfiguration/abort_further:true/");
@ -378,39 +352,6 @@ TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) {
EXPECT_THAT(probes, IsEmpty()); 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, TestExponentialProbingTimeout) { TEST(ProbeControllerTest, TestExponentialProbingTimeout) {
ProbeControllerFixture fixture; ProbeControllerFixture fixture;
std::unique_ptr<ProbeController> probe_controller = std::unique_ptr<ProbeController> probe_controller =