From 8649e49d10e6e6efb5a98920f0b19a77abe8f070 Mon Sep 17 00:00:00 2001 From: Christoffer Rodbro Date: Tue, 15 Dec 2020 20:20:54 +0100 Subject: [PATCH] Add a field trial to skip REMB modification of BWE internal state. By enabling the field trial, REMB caps the output target bitrate, but does not change any internal BWE state variables. Bug: webrtc:12306 Change-Id: I43e9ac1d1b7dff292d7aa5800c01d874bc91aaff Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/197809 Reviewed-by: Jakob Ivarsson Reviewed-by: Magnus Flodman Commit-Queue: Christoffer Rodbro Cr-Commit-Position: refs/heads/master@{#32867} --- .../goog_cc_network_control_unittest.cc | 30 +++++++++++++++++++ .../goog_cc/send_side_bandwidth_estimation.cc | 15 ++++++++-- .../goog_cc/send_side_bandwidth_estimation.h | 1 + test/scenario/call_client.cc | 17 ++++++++++- test/scenario/call_client.h | 3 ++ 5 files changed, 62 insertions(+), 4 deletions(-) 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 33550d2a51..0510cb99b7 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 @@ -11,6 +11,7 @@ #include #include "api/transport/goog_cc_factory.h" +#include "api/units/data_rate.h" #include "logging/rtc_event_log/mock/mock_rtc_event_log.h" #include "test/field_trial.h" #include "test/gtest.h" @@ -844,5 +845,34 @@ TEST_F(GoogCcNetworkControllerTest, IsFairToTCP) { EXPECT_LT(client->send_bandwidth().kbps(), 750); } +TEST(GoogCcScenario, RampupOnRembCapLifted) { + ScopedFieldTrials trial("WebRTC-Bwe-ReceiverLimitCapsOnly/Enabled/"); + Scenario s("googcc_unit/rampup_ramb_cap_lifted"); + NetworkSimulationConfig net_conf; + net_conf.bandwidth = DataRate::KilobitsPerSec(2000); + net_conf.delay = TimeDelta::Millis(50); + auto* client = s.CreateClient("send", [&](CallClientConfig* c) { + c->transport.rates.start_rate = DataRate::KilobitsPerSec(1000); + }); + auto send_net = {s.CreateSimulationNode(net_conf)}; + auto ret_net = {s.CreateSimulationNode(net_conf)}; + auto* route = s.CreateRoutes( + client, send_net, s.CreateClient("return", CallClientConfig()), ret_net); + s.CreateVideoStream(route->forward(), VideoStreamConfig()); + + s.RunFor(TimeDelta::Seconds(10)); + EXPECT_GT(client->send_bandwidth().kbps(), 1500); + + DataRate RembLimit = DataRate::KilobitsPerSec(250); + client->SetRemoteBitrate(RembLimit); + s.RunFor(TimeDelta::Seconds(1)); + EXPECT_EQ(client->send_bandwidth(), RembLimit); + + DataRate RembLimitLifted = DataRate::KilobitsPerSec(10000); + client->SetRemoteBitrate(RembLimitLifted); + s.RunFor(TimeDelta::Seconds(10)); + EXPECT_GT(client->send_bandwidth().kbps(), 1500); +} + } // namespace test } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc index 4ca75bf263..e179ff53fe 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc @@ -20,6 +20,7 @@ #include "api/rtc_event_log/rtc_event.h" #include "api/rtc_event_log/rtc_event_log.h" #include "api/transport/webrtc_key_value_config.h" +#include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_loss_based.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" @@ -224,7 +225,8 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation( last_rtc_event_log_(Timestamp::MinusInfinity()), low_loss_threshold_(kDefaultLowLossThreshold), high_loss_threshold_(kDefaultHighLossThreshold), - bitrate_threshold_(kDefaultBitrateThreshold) { + bitrate_threshold_(kDefaultBitrateThreshold), + receiver_limit_caps_only_("Enabled") { RTC_DCHECK(event_log); if (BweLossExperimentIsEnabled()) { uint32_t bitrate_threshold_kbps; @@ -237,6 +239,8 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation( bitrate_threshold_ = DataRate::KilobitsPerSec(bitrate_threshold_kbps); } } + ParseFieldTrial({&receiver_limit_caps_only_}, + key_value_config->Lookup("WebRTC-Bwe-ReceiverLimitCapsOnly")); } SendSideBandwidthEstimation::~SendSideBandwidthEstimation() {} @@ -308,7 +312,10 @@ int SendSideBandwidthEstimation::GetMinBitrate() const { } DataRate SendSideBandwidthEstimation::target_rate() const { - return std::max(min_bitrate_configured_, current_target_); + DataRate target = current_target_; + if (receiver_limit_caps_only_) + target = std::min(target, receiver_limit_); + return std::max(min_bitrate_configured_, target); } DataRate SendSideBandwidthEstimation::GetEstimatedLinkCapacity() const { @@ -590,7 +597,9 @@ DataRate SendSideBandwidthEstimation::MaybeRampupOrBackoff(DataRate new_bitrate, } DataRate SendSideBandwidthEstimation::GetUpperLimit() const { - DataRate upper_limit = std::min(delay_based_limit_, receiver_limit_); + DataRate upper_limit = delay_based_limit_; + if (!receiver_limit_caps_only_) + upper_limit = std::min(upper_limit, receiver_limit_); upper_limit = std::min(upper_limit, max_bitrate_configured_); if (loss_based_bandwidth_estimation_.Enabled() && loss_based_bandwidth_estimation_.GetEstimate() > DataRate::Zero()) { diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h index a13800b7f6..ca42a9d67c 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h @@ -192,6 +192,7 @@ class SendSideBandwidthEstimation { float high_loss_threshold_; DataRate bitrate_threshold_; LossBasedBandwidthEstimation loss_based_bandwidth_estimation_; + FieldTrialFlag receiver_limit_caps_only_; }; } // namespace webrtc #endif // MODULES_CONGESTION_CONTROLLER_GOOG_CC_SEND_SIDE_BANDWIDTH_ESTIMATION_H_ diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc index ab529fae9a..f7cd47c36e 100644 --- a/test/scenario/call_client.cc +++ b/test/scenario/call_client.cc @@ -9,11 +9,13 @@ */ #include "test/scenario/call_client.h" +#include +#include #include -#include #include "api/rtc_event_log/rtc_event_log.h" #include "api/rtc_event_log/rtc_event_log_factory.h" +#include "api/transport/network_types.h" #include "modules/audio_mixer/audio_mixer_impl.h" namespace webrtc { @@ -197,6 +199,12 @@ TimeDelta LoggingNetworkControllerFactory::GetProcessInterval() const { return cc_factory_->GetProcessInterval(); } +void LoggingNetworkControllerFactory::SetRemoteBitrateEstimate( + RemoteBitrateReport msg) { + if (last_controller_) + last_controller_->OnRemoteBitrateReport(msg); +} + CallClient::CallClient( TimeController* time_controller, std::unique_ptr log_writer_factory, @@ -269,6 +277,13 @@ DataRate CallClient::padding_rate() const { return network_controller_factory_.GetUpdate().pacer_config->pad_rate(); } +void CallClient::SetRemoteBitrate(DataRate bitrate) { + RemoteBitrateReport msg; + msg.bandwidth = bitrate; + msg.receive_time = clock_->CurrentTime(); + network_controller_factory_.SetRemoteBitrateEstimate(msg); +} + void CallClient::UpdateBitrateConstraints( const BitrateConstraints& constraints) { SendTask([this, &constraints]() { diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h index a4f45e1e7e..27ec9fa39c 100644 --- a/test/scenario/call_client.h +++ b/test/scenario/call_client.h @@ -18,6 +18,7 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/test/time_controller.h" +#include "api/units/data_rate.h" #include "call/call.h" #include "modules/audio_device/include/test_audio_device.h" #include "modules/congestion_controller/goog_cc/test/goog_cc_printer.h" @@ -75,6 +76,7 @@ class LoggingNetworkControllerFactory TimeDelta GetProcessInterval() const override; // TODO(srte): Consider using the Columnprinter interface for this. void LogCongestionControllerStats(Timestamp at_time); + void SetRemoteBitrateEstimate(RemoteBitrateReport msg); NetworkControlUpdate GetUpdate() const; @@ -110,6 +112,7 @@ class CallClient : public EmulatedNetworkReceiverInterface { DataRate stable_target_rate() const; DataRate padding_rate() const; void UpdateBitrateConstraints(const BitrateConstraints& constraints); + void SetRemoteBitrate(DataRate bitrate); void OnPacketReceived(EmulatedIpPacket packet) override; std::unique_ptr GetLogWriter(std::string name);