diff --git a/api/call/bitrate_allocation.h b/api/call/bitrate_allocation.h index 2d7f21bc1e..c52969b691 100644 --- a/api/call/bitrate_allocation.h +++ b/api/call/bitrate_allocation.h @@ -27,12 +27,14 @@ struct BitrateAllocationUpdate { // 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 9e79bce844..22c7d23cf2 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -202,6 +202,7 @@ struct TargetTransferRate { // 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 487dd47ad8..9fe4c54254 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -53,7 +53,8 @@ double MediaRatio(uint32_t allocated_bitrate, uint32_t protection_bitrate) { 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 @@ uint8_t BitrateAllocator::GetTransmissionMaxBitrateMultiplier() { } 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 @@ void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, } 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 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, 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 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, 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 ecff422b92..bfa9a0afba 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -95,7 +95,8 @@ class BitrateAllocator : public BitrateAllocatorInterface { // 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 @@ class BitrateAllocator : public BitrateAllocatorInterface { // 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 69dfa1a035..6857d22d82 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -75,7 +75,8 @@ class BitrateAllocatorForTest : public BitrateAllocator { 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 5d37e46be7..8771380194 100644 --- a/call/call.cc +++ b/call/call.cc @@ -1082,15 +1082,16 @@ void Call::OnTargetTransferRate(TargetTransferRate msg) { 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 7dfff3d5f5..2d6516813e 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 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config, 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::OnNetworkStateEstimate( 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 @@ NetworkControlUpdate GoogCcNetworkController::GetNetworkState( 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 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( 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 3de04d1139..e7e60fc107 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 @@ class GoogCcNetworkController : public NetworkControllerInterface { 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 ee8bf5d59a..dd50896ed4 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 @@ TEST_F(GoogCcNetworkControllerTest, UpdatesTargetRateBasedOnLinkCapacity) { 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 @@ TEST_F(GoogCcNetworkControllerTest, StableEstimateDoesNotVaryInSteadyState) { 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 e5e1726813..31435bb21a 100644 --- a/test/scenario/call_client.cc +++ b/test/scenario/call_client.cc @@ -256,6 +256,11 @@ DataRate CallClient::link_capacity() const { .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 49939ed8e6..d2603a87c7 100644 --- a/test/scenario/call_client.h +++ b/test/scenario/call_client.h @@ -107,6 +107,7 @@ class CallClient : public EmulatedNetworkReceiverInterface { 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 ed1f6e3873..1d3312921d 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -659,7 +659,6 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { 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 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { 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(vss_impl.get()) ->OnBitrateUpdated(update);