Wire the stable target bitrate from GoogCC to the BitrateAllocator
Deprecated the field BitrateAllocationUpdate::link_capacity since it is only
used by the Opus codec in order to smooth the target bitrate, which is
equivalent to the stable_target_bitrate field.
The unused field trial WebRTC-Bwe-StableBandwidthEstimate is also removed.
Bug: webrtc:10126
Change-Id: Ic4a8a9ca4202136d011b91dc23c3a27cfd00d975
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149839
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28941}
diff --git a/api/call/bitrate_allocation.h b/api/call/bitrate_allocation.h
index 2d7f21b..c52969b 100644
--- a/api/call/bitrate_allocation.h
+++ b/api/call/bitrate_allocation.h
@@ -27,12 +27,14 @@
// the target as it is based on the underlying link capacity estimate. This
// should be used to change encoder configuration when the cost of change is
// high.
- DataRate link_capacity = DataRate::Zero();
+ DataRate stable_target_bitrate = DataRate::Zero();
// Predicted packet loss ratio.
double packet_loss_ratio = 0;
// Predicted round trip time.
TimeDelta round_trip_time = TimeDelta::PlusInfinity();
- // |bwe_period| is deprecated, use the link capacity allocation instead.
+ // |link_capacity| is deprecated, use |stable_target_bitrate| instead.
+ DataRate link_capacity = DataRate::Zero();
+ // |bwe_period| is deprecated, use |stable_target_bitrate| allocation instead.
TimeDelta bwe_period = TimeDelta::PlusInfinity();
};
diff --git a/api/transport/network_types.h b/api/transport/network_types.h
index 9e79bce..22c7d23 100644
--- a/api/transport/network_types.h
+++ b/api/transport/network_types.h
@@ -202,6 +202,7 @@
// The estimate on which the target rate is based on.
NetworkEstimate network_estimate;
DataRate target_rate = DataRate::Zero();
+ DataRate stable_target_rate = DataRate::Zero();
};
// Contains updates of network controller comand state. Using optionals to
diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc
index 487dd47..9fe4c54 100644
--- a/call/bitrate_allocator.cc
+++ b/call/bitrate_allocator.cc
@@ -53,7 +53,8 @@
BitrateAllocator::BitrateAllocator(Clock* clock, LimitObserver* limit_observer)
: limit_observer_(limit_observer),
last_target_bps_(0),
- last_link_capacity_bps_(0),
+ last_stable_target_bps_(0),
+ last_bandwidth_bps_(0),
last_non_zero_bitrate_bps_(kDefaultBitrateBps),
last_fraction_loss_(0),
last_rtt_(0),
@@ -94,13 +95,15 @@
}
void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps,
- uint32_t link_capacity_bps,
+ uint32_t stable_target_bitrate_bps,
+ uint32_t bandwidth_bps,
uint8_t fraction_loss,
int64_t rtt,
int64_t bwe_period_ms) {
RTC_DCHECK_RUN_ON(&sequenced_checker_);
last_target_bps_ = target_bitrate_bps;
- last_link_capacity_bps_ = link_capacity_bps;
+ last_bandwidth_bps_ = bandwidth_bps;
+ last_stable_target_bps_ = stable_target_bitrate_bps;
last_non_zero_bitrate_bps_ =
target_bitrate_bps > 0 ? target_bitrate_bps : last_non_zero_bitrate_bps_;
last_fraction_loss_ = fraction_loss;
@@ -115,13 +118,18 @@
}
ObserverAllocation allocation = AllocateBitrates(target_bitrate_bps);
- ObserverAllocation bandwidth_allocation = AllocateBitrates(link_capacity_bps);
+ ObserverAllocation bandwidth_allocation = AllocateBitrates(bandwidth_bps);
+ ObserverAllocation stable_bitrate_allocation =
+ AllocateBitrates(stable_target_bitrate_bps);
for (auto& config : bitrate_observer_configs_) {
uint32_t allocated_bitrate = allocation[config.observer];
uint32_t allocated_bandwidth = bandwidth_allocation[config.observer];
+ uint32_t allocated_stable_target_rate =
+ stable_bitrate_allocation[config.observer];
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::bps(allocated_bitrate);
+ update.stable_target_bitrate = DataRate::bps(allocated_stable_target_rate);
update.link_capacity = DataRate::bps(allocated_bandwidth);
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
@@ -183,12 +191,17 @@
ObserverAllocation allocation = AllocateBitrates(last_target_bps_);
ObserverAllocation bandwidth_allocation =
- AllocateBitrates(last_link_capacity_bps_);
+ AllocateBitrates(last_bandwidth_bps_);
+ ObserverAllocation stable_bitrate_allocation =
+ AllocateBitrates(last_stable_target_bps_);
for (auto& config : bitrate_observer_configs_) {
uint32_t allocated_bitrate = allocation[config.observer];
+ uint32_t allocated_stable_bitrate =
+ stable_bitrate_allocation[config.observer];
uint32_t bandwidth = bandwidth_allocation[config.observer];
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::bps(allocated_bitrate);
+ update.stable_target_bitrate = DataRate::bps(allocated_stable_bitrate);
update.link_capacity = DataRate::bps(bandwidth);
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
@@ -205,6 +218,7 @@
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::Zero();
+ update.stable_target_bitrate = DataRate::Zero();
update.link_capacity = DataRate::Zero();
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h
index ecff422..bfa9a0a 100644
--- a/call/bitrate_allocator.h
+++ b/call/bitrate_allocator.h
@@ -95,7 +95,8 @@
// Allocate target_bitrate across the registered BitrateAllocatorObservers.
void OnNetworkChanged(uint32_t target_bitrate_bps,
- uint32_t link_capacity_bps,
+ uint32_t stable_target_bitrate_bps,
+ uint32_t bandwidth_bps,
uint8_t fraction_loss,
int64_t rtt,
int64_t bwe_period_ms);
@@ -228,7 +229,8 @@
// Stored in a list to keep track of the insertion order.
ObserverConfigs bitrate_observer_configs_ RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_target_bps_ RTC_GUARDED_BY(&sequenced_checker_);
- uint32_t last_link_capacity_bps_ RTC_GUARDED_BY(&sequenced_checker_);
+ uint32_t last_stable_target_bps_ RTC_GUARDED_BY(&sequenced_checker_);
+ uint32_t last_bandwidth_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_non_zero_bitrate_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint8_t last_fraction_loss_ RTC_GUARDED_BY(&sequenced_checker_);
int64_t last_rtt_ RTC_GUARDED_BY(&sequenced_checker_);
diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc
index 69dfa1a..6857d22 100644
--- a/call/bitrate_allocator_unittest.cc
+++ b/call/bitrate_allocator_unittest.cc
@@ -75,7 +75,8 @@
int64_t rtt,
int64_t bwe_period_ms) {
BitrateAllocator::OnNetworkChanged(target_bitrate_bps, target_bitrate_bps,
- fraction_loss, rtt, bwe_period_ms);
+ target_bitrate_bps, fraction_loss, rtt,
+ bwe_period_ms);
}
};
diff --git a/call/call.cc b/call/call.cc
index 5d37e46..8771380 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -1082,15 +1082,16 @@
int64_t rtt_ms = msg.network_estimate.round_trip_time.ms();
int64_t probing_interval_ms = msg.network_estimate.bwe_period.ms();
uint32_t bandwidth_bps = msg.network_estimate.bandwidth.bps();
+ uint32_t stable_target_rate_bps = msg.stable_target_rate.bps();
{
rtc::CritScope cs(&last_bandwidth_bps_crit_);
last_bandwidth_bps_ = bandwidth_bps;
}
// For controlling the rate of feedback messages.
receive_side_cc_.OnBitrateChanged(target_bitrate_bps);
- bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, bandwidth_bps,
- fraction_loss, rtt_ms,
- probing_interval_ms);
+ bitrate_allocator_->OnNetworkChanged(
+ target_bitrate_bps, stable_target_rate_bps, bandwidth_bps, fraction_loss,
+ rtt_ms, probing_interval_ms);
// Ignore updates if bitrate is zero (the aggregate network state is down).
if (target_bitrate_bps == 0) {
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 7dfff3d..2d65168 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -67,8 +67,6 @@
packet_feedback_only_(goog_cc_config.feedback_only),
safe_reset_on_route_change_("Enabled"),
safe_reset_acknowledged_rate_("ack"),
- use_stable_bandwidth_estimate_(
- IsEnabled(key_value_config_, "WebRTC-Bwe-StableBandwidthEstimate")),
use_downlink_delay_for_congestion_window_(
IsEnabled(key_value_config_,
"WebRTC-Bwe-CongestionWindowDownlinkDelay")),
@@ -571,14 +569,11 @@
NetworkControlUpdate GoogCcNetworkController::GetNetworkState(
Timestamp at_time) const {
- DataRate bandwidth = use_stable_bandwidth_estimate_
- ? bandwidth_estimation_->GetEstimatedLinkCapacity()
- : last_raw_target_rate_;
TimeDelta rtt = TimeDelta::ms(last_estimated_rtt_ms_);
NetworkControlUpdate update;
update.target_rate = TargetTransferRate();
update.target_rate->network_estimate.at_time = at_time;
- update.target_rate->network_estimate.bandwidth = bandwidth;
+ update.target_rate->network_estimate.bandwidth = last_raw_target_rate_;
update.target_rate->network_estimate.loss_rate_ratio =
last_estimated_fraction_loss_ / 255.0;
update.target_rate->network_estimate.round_trip_time = rtt;
@@ -586,7 +581,9 @@
delay_based_bwe_->GetExpectedBwePeriod();
update.target_rate->at_time = at_time;
- update.target_rate->target_rate = bandwidth;
+ update.target_rate->target_rate = last_raw_target_rate_;
+ update.target_rate->stable_target_rate =
+ bandwidth_estimation_->GetEstimatedLinkCapacity();
update.pacer_config = GetPacingRates(at_time);
update.congestion_window = current_data_window_;
return update;
@@ -629,18 +626,17 @@
alr_detector_->SetEstimatedBitrate(estimated_bitrate_bps);
last_raw_target_rate_ = DataRate::bps(estimated_bitrate_bps);
- DataRate bandwidth = use_stable_bandwidth_estimate_
- ? bandwidth_estimation_->GetEstimatedLinkCapacity()
- : last_raw_target_rate_;
TimeDelta bwe_period = delay_based_bwe_->GetExpectedBwePeriod();
TargetTransferRate target_rate_msg;
target_rate_msg.at_time = at_time;
target_rate_msg.target_rate = target_rate;
+ target_rate_msg.stable_target_rate =
+ bandwidth_estimation_->GetEstimatedLinkCapacity();
target_rate_msg.network_estimate.at_time = at_time;
target_rate_msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms);
- target_rate_msg.network_estimate.bandwidth = bandwidth;
+ target_rate_msg.network_estimate.bandwidth = last_raw_target_rate_;
target_rate_msg.network_estimate.loss_rate_ratio = fraction_loss / 255.0f;
target_rate_msg.network_estimate.bwe_period = bwe_period;
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 3de04d1..e7e60fc 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
@@ -85,7 +85,6 @@
const bool packet_feedback_only_;
FieldTrialFlag safe_reset_on_route_change_;
FieldTrialFlag safe_reset_acknowledged_rate_;
- const bool use_stable_bandwidth_estimate_;
const bool use_downlink_delay_for_congestion_window_;
const bool fall_back_to_probe_rate_;
const bool use_min_allocatable_as_lower_bound_;
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
index ee8bf5d..dd50896 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
@@ -487,37 +487,9 @@
UpdatesTargetRateBasedOnLinkCapacity();
}
-TEST_F(GoogCcNetworkControllerTest, DefaultEstimateVariesInSteadyState) {
- auto factory = CreateFeedbackOnlyFactory();
- ScopedFieldTrials trial("WebRTC-Bwe-StableBandwidthEstimate/Disabled/");
- Scenario s("googcc_unit/no_stable_varies", false);
- CallClientConfig config;
- config.transport.cc_factory = &factory;
- NetworkSimulationConfig net_conf;
- net_conf.bandwidth = DataRate::kbps(500);
- net_conf.delay = TimeDelta::ms(100);
- auto send_net = s.CreateSimulationNode(net_conf);
- auto ret_net = s.CreateSimulationNode(net_conf);
-
- auto* client = CreateVideoSendingClient(&s, config, {send_net}, {ret_net});
- // Run for a while to allow the estimate to stabilize.
- s.RunFor(TimeDelta::seconds(20));
- DataRate min_estimate = DataRate::PlusInfinity();
- DataRate max_estimate = DataRate::MinusInfinity();
- // Measure variation in steady state.
- for (int i = 0; i < 20; ++i) {
- min_estimate = std::min(min_estimate, client->link_capacity());
- max_estimate = std::max(max_estimate, client->link_capacity());
- s.RunFor(TimeDelta::seconds(1));
- }
- // We should expect drops by at least 15% (default backoff.)
- EXPECT_LT(min_estimate / max_estimate, 0.85);
-}
-
TEST_F(GoogCcNetworkControllerTest, StableEstimateDoesNotVaryInSteadyState) {
auto factory = CreateFeedbackOnlyFactory();
- ScopedFieldTrials trial("WebRTC-Bwe-StableBandwidthEstimate/Enabled/");
- Scenario s("googcc_unit/stable_is_stable", false);
+ Scenario s("googcc_unit/stable_target", false);
CallClientConfig config;
config.transport.cc_factory = &factory;
NetworkSimulationConfig net_conf;
@@ -529,16 +501,25 @@
auto* client = CreateVideoSendingClient(&s, config, {send_net}, {ret_net});
// Run for a while to allow the estimate to stabilize.
s.RunFor(TimeDelta::seconds(30));
- DataRate min_estimate = DataRate::PlusInfinity();
- DataRate max_estimate = DataRate::MinusInfinity();
+ DataRate min_stable_target = DataRate::PlusInfinity();
+ DataRate max_stable_target = DataRate::MinusInfinity();
+ DataRate min_target = DataRate::PlusInfinity();
+ DataRate max_target = DataRate::MinusInfinity();
+
// Measure variation in steady state.
for (int i = 0; i < 20; ++i) {
- min_estimate = std::min(min_estimate, client->link_capacity());
- max_estimate = std::max(max_estimate, client->link_capacity());
+ min_stable_target =
+ std::min(min_stable_target, client->stable_target_rate());
+ max_stable_target =
+ std::max(max_stable_target, client->stable_target_rate());
+ min_target = std::min(min_target, client->link_capacity());
+ max_target = std::max(max_target, client->link_capacity());
s.RunFor(TimeDelta::seconds(1));
}
// We expect no variation under the trial in steady state.
- EXPECT_GT(min_estimate / max_estimate, 0.95);
+ EXPECT_GT(min_stable_target / max_stable_target, 0.95);
+ // We should expect drops by at least 15% (default backoff.)
+ EXPECT_LT(min_target / max_target, 0.85);
}
TEST_F(GoogCcNetworkControllerTest,
diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc
index e5e1726..31435bb 100644
--- a/test/scenario/call_client.cc
+++ b/test/scenario/call_client.cc
@@ -256,6 +256,11 @@
.target_rate->network_estimate.bandwidth;
}
+DataRate CallClient::stable_target_rate() const {
+ return network_controller_factory_.GetUpdate()
+ .target_rate->stable_target_rate;
+}
+
DataRate CallClient::padding_rate() const {
return network_controller_factory_.GetUpdate().pacer_config->pad_rate();
}
diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h
index 49939ed..d2603a8 100644
--- a/test/scenario/call_client.h
+++ b/test/scenario/call_client.h
@@ -107,6 +107,7 @@
return DataRate::bps(GetStats().send_bandwidth_bps);
}
DataRate target_rate() const;
+ DataRate stable_target_rate() const;
DataRate link_capacity() const;
DataRate padding_rate() const;
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index ed1f6e3..1d33129 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -659,7 +659,6 @@
DataRate::bps(qvga_stream.target_bitrate_bps);
BitrateAllocationUpdate update;
update.target_bitrate = network_constrained_rate;
- update.link_capacity = network_constrained_rate;
update.round_trip_time = TimeDelta::ms(1);
EXPECT_CALL(rtp_video_sender_,
OnBitrateUpdated(network_constrained_rate.bps(), _,
@@ -686,7 +685,6 @@
EXPECT_CALL(video_stream_encoder_,
OnBitrateUpdated(qvga_max_bitrate, rate_with_headroom, 0, _));
update.target_bitrate = rate_with_headroom;
- update.link_capacity = rate_with_headroom;
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
->OnBitrateUpdated(update);
