Moves pushback controller to GoogCC
Since the pushback controller doesn't strictly adhere to the congestion
window, it better belongs together with the congestion controller logic.
Also ensuring that it does not override the configured min bitrate.
Bug: webrtc:9586
Change-Id: I57dcfc946d470247e66c67adabddaafa3d9d83ad
Reviewed-on: https://webrtc-review.googlesource.com/c/105102
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25115}diff --git a/modules/congestion_controller/BUILD.gn b/modules/congestion_controller/BUILD.gn
index 2fcb712..5c1a610 100644
--- a/modules/congestion_controller/BUILD.gn
+++ b/modules/congestion_controller/BUILD.gn
@@ -20,8 +20,6 @@
visibility = [ "*" ]
configs += [ ":bwe_test_logging" ]
sources = [
- "congestion_window_pushback_controller.cc",
- "congestion_window_pushback_controller.h",
"include/network_changed_observer.h",
"include/receive_side_congestion_controller.h",
"include/send_side_congestion_controller.h",
@@ -52,6 +50,7 @@
"goog_cc:delay_based_bwe",
"goog_cc:estimators",
"goog_cc:probe_controller",
+ "goog_cc:pushback_controller",
"//third_party/abseil-cpp/absl/memory",
]
@@ -98,7 +97,6 @@
testonly = true
sources = [
- "congestion_window_pushback_controller_unittest.cc",
"receive_side_congestion_controller_unittest.cc",
"send_side_congestion_controller_unittest.cc",
"transport_feedback_adapter_unittest.cc",
diff --git a/modules/congestion_controller/goog_cc/BUILD.gn b/modules/congestion_controller/goog_cc/BUILD.gn
index 3fbc09e..6e4af2f 100644
--- a/modules/congestion_controller/goog_cc/BUILD.gn
+++ b/modules/congestion_controller/goog_cc/BUILD.gn
@@ -28,6 +28,7 @@
":delay_based_bwe",
":estimators",
":probe_controller",
+ ":pushback_controller",
"../..:module_api",
"../../..:webrtc_common",
"../../../api/transport:network_control",
@@ -47,6 +48,20 @@
]
}
+rtc_source_set("pushback_controller") {
+ sources = [
+ "congestion_window_pushback_controller.cc",
+ "congestion_window_pushback_controller.h",
+ ]
+ deps = [
+ "../../../api/transport:network_control",
+ "../../../rtc_base:checks",
+ "../../../rtc_base:rtc_base_approved",
+ "../../../system_wrappers:field_trial",
+ "//third_party/abseil-cpp/absl/types:optional",
+ ]
+}
+
rtc_source_set("alr_detector") {
sources = [
"alr_detector.cc",
@@ -156,6 +171,7 @@
sources = [
"acknowledged_bitrate_estimator_unittest.cc",
"alr_detector_unittest.cc",
+ "congestion_window_pushback_controller_unittest.cc",
"delay_based_bwe_unittest.cc",
"delay_based_bwe_unittest_helper.cc",
"delay_based_bwe_unittest_helper.h",
@@ -174,6 +190,7 @@
":estimators",
":goog_cc",
":probe_controller",
+ ":pushback_controller",
"../../../api/transport:goog_cc",
"../../../api/transport:network_control",
"../../../api/transport:network_control_test",
diff --git a/modules/congestion_controller/congestion_window_pushback_controller.cc b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller.cc
similarity index 95%
rename from modules/congestion_controller/congestion_window_pushback_controller.cc
rename to modules/congestion_controller/goog_cc/congestion_window_pushback_controller.cc
index daa51bd..2011bef 100644
--- a/modules/congestion_controller/congestion_window_pushback_controller.cc
+++ b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller.cc
@@ -8,10 +8,12 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include <string>
#include <algorithm>
+#include <string>
-#include "modules/congestion_controller/congestion_window_pushback_controller.h"
+#include "modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/format_macros.h"
#include "system_wrappers/include/field_trial.h"
namespace webrtc {
diff --git a/modules/congestion_controller/congestion_window_pushback_controller.h b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h
similarity index 78%
rename from modules/congestion_controller/congestion_window_pushback_controller.h
rename to modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h
index f608590..d8a3343 100644
--- a/modules/congestion_controller/congestion_window_pushback_controller.h
+++ b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h
@@ -8,13 +8,10 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#ifndef MODULES_CONGESTION_CONTROLLER_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
-#define MODULES_CONGESTION_CONTROLLER_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
+#ifndef MODULES_CONGESTION_CONTROLLER_GOOG_CC_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
+#define MODULES_CONGESTION_CONTROLLER_GOOG_CC_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
#include "api/transport/network_types.h"
-#include "common_types.h" // NOLINT(build/include)
-#include "rtc_base/criticalsection.h"
-#include "rtc_base/format_macros.h"
namespace webrtc {
@@ -40,4 +37,4 @@
} // namespace webrtc
-#endif // MODULES_CONGESTION_CONTROLLER_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
+#endif // MODULES_CONGESTION_CONTROLLER_GOOG_CC_CONGESTION_WINDOW_PUSHBACK_CONTROLLER_H_
diff --git a/modules/congestion_controller/congestion_window_pushback_controller_unittest.cc b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller_unittest.cc
similarity index 95%
rename from modules/congestion_controller/congestion_window_pushback_controller_unittest.cc
rename to modules/congestion_controller/goog_cc/congestion_window_pushback_controller_unittest.cc
index d9d6a79d..30617ee 100644
--- a/modules/congestion_controller/congestion_window_pushback_controller_unittest.cc
+++ b/modules/congestion_controller/goog_cc/congestion_window_pushback_controller_unittest.cc
@@ -8,7 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "modules/congestion_controller/congestion_window_pushback_controller.h"
+#include "modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h"
#include "test/gmock.h"
#include "test/gtest.h"
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 79c5375..ffc582e 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -35,6 +35,11 @@
namespace {
const char kCwndExperiment[] = "WebRTC-CwndExperiment";
+// When CongestionWindowPushback is enabled, the pacer is oblivious to
+// the congestion window. The relation between outstanding data and
+// the congestion window affects encoder allocations directly.
+const char kCongestionPushbackExperiment[] = "WebRTC-CongestionWindowPushback";
+
const int64_t kDefaultAcceptedQueueMs = 250;
// From RTCPSender video report interval.
@@ -47,6 +52,18 @@
// overshoots from the encoder.
const float kDefaultPaceMultiplier = 2.5f;
+bool IsCongestionWindowPushbackExperimentEnabled() {
+ return webrtc::field_trial::IsEnabled(kCongestionPushbackExperiment) &&
+ webrtc::field_trial::IsEnabled(kCwndExperiment);
+}
+
+std::unique_ptr<CongestionWindowPushbackController>
+MaybeInitalizeCongestionWindowPushbackController() {
+ return IsCongestionWindowPushbackExperimentEnabled()
+ ? absl::make_unique<CongestionWindowPushbackController>()
+ : nullptr;
+}
+
bool CwndExperimentEnabled() {
std::string experiment_string =
webrtc::field_trial::FindFullName(kCwndExperiment);
@@ -113,13 +130,14 @@
} // namespace
-
GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log,
NetworkControllerConfig config,
bool feedback_only)
: event_log_(event_log),
packet_feedback_only_(feedback_only),
probe_controller_(new ProbeController()),
+ congestion_window_pushback_controller_(
+ MaybeInitalizeCongestionWindowPushbackController()),
bandwidth_estimation_(
absl::make_unique<SendSideBandwidthEstimation>(event_log_)),
alr_detector_(absl::make_unique<AlrDetector>()),
@@ -256,7 +274,15 @@
bandwidth_estimation_->UpdatePropagationRtt(sent_packet.send_time,
TimeDelta::Zero());
}
- return NetworkControlUpdate();
+ if (congestion_window_pushback_controller_) {
+ congestion_window_pushback_controller_->UpdateOutstandingData(
+ sent_packet.data_in_flight.bytes());
+ NetworkControlUpdate update;
+ MaybeTriggerOnNetworkChanged(&update, sent_packet.send_time);
+ return update;
+ } else {
+ return NetworkControlUpdate();
+ }
}
NetworkControlUpdate GoogCcNetworkController::OnStreamsConfig(
@@ -337,6 +363,10 @@
NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
TransportPacketsFeedback report) {
+ if (congestion_window_pushback_controller_) {
+ congestion_window_pushback_controller_->UpdateOutstandingData(
+ report.data_in_flight.bytes());
+ }
TimeDelta max_feedback_rtt = TimeDelta::MinusInfinity();
TimeDelta min_propagation_rtt = TimeDelta::PlusInfinity();
Timestamp max_recv_time = Timestamp::MinusInfinity();
@@ -458,7 +488,12 @@
RTC_LOG(LS_INFO) << "Feedback rtt: " << min_feedback_max_rtt_ms
<< " Bitrate: " << last_bandwidth_.bps();
}
- update.congestion_window = current_data_window_;
+ if (congestion_window_pushback_controller_ && current_data_window_) {
+ congestion_window_pushback_controller_->SetDataWindow(
+ *current_data_window_);
+ } else {
+ update.congestion_window = current_data_window_;
+ }
return update;
}
@@ -517,18 +552,28 @@
TimeDelta bwe_period =
TimeDelta::ms(delay_based_bwe_->GetExpectedBwePeriodMs());
- TargetTransferRate target_rate;
- target_rate.at_time = at_time;
// Set the target rate to the full estimated bandwidth since the estimation
// for legacy reasons includes target rate constraints.
- target_rate.target_rate = bandwidth;
+ DataRate target_rate = bandwidth;
+ if (congestion_window_pushback_controller_) {
+ int64_t pushback_rate =
+ congestion_window_pushback_controller_->UpdateTargetBitrate(
+ target_rate.bps());
+ pushback_rate = std::max<int64_t>(bandwidth_estimation_->GetMinBitrate(),
+ pushback_rate);
+ target_rate = DataRate::bps(pushback_rate);
+ }
- target_rate.network_estimate.at_time = at_time;
- target_rate.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms);
- target_rate.network_estimate.bandwidth = bandwidth;
- target_rate.network_estimate.loss_rate_ratio = fraction_loss / 255.0f;
- target_rate.network_estimate.bwe_period = bwe_period;
- update->target_rate = target_rate;
+ TargetTransferRate target_rate_msg;
+ target_rate_msg.at_time = at_time;
+ target_rate_msg.target_rate = target_rate;
+ 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.loss_rate_ratio = fraction_loss / 255.0f;
+ target_rate_msg.network_estimate.bwe_period = bwe_period;
+
+ update->target_rate = target_rate_msg;
auto probes =
probe_controller_->SetEstimatedBitrate(bandwidth.bps(), at_time.ms());
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 64401a7..323f313 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
@@ -22,6 +22,7 @@
#include "modules/bitrate_controller/send_side_bandwidth_estimation.h"
#include "modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h"
#include "modules/congestion_controller/goog_cc/alr_detector.h"
+#include "modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h"
#include "modules/congestion_controller/goog_cc/delay_based_bwe.h"
#include "modules/congestion_controller/goog_cc/probe_controller.h"
#include "rtc_base/constructormagic.h"
@@ -65,6 +66,8 @@
const bool packet_feedback_only_;
const std::unique_ptr<ProbeController> probe_controller_;
+ const std::unique_ptr<CongestionWindowPushbackController>
+ congestion_window_pushback_controller_;
std::unique_ptr<SendSideBandwidthEstimation> bandwidth_estimation_;
std::unique_ptr<AlrDetector> alr_detector_;
diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
index cb65b25..953214e 100644
--- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
@@ -17,7 +17,6 @@
#include "absl/memory/memory.h"
#include "api/transport/goog_cc_factory.h"
#include "api/transport/network_types.h"
-#include "modules/congestion_controller/congestion_window_pushback_controller.h"
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
#include "rtc_base/bind.h"
#include "rtc_base/checks.h"
@@ -39,13 +38,6 @@
namespace {
using send_side_cc_internal::PeriodicTask;
-const char kCwndExperiment[] = "WebRTC-CwndExperiment";
-
-// When CongestionWindowPushback is enabled, the pacer is oblivious to
-// the congestion window. The relation between outstanding data and
-// the congestion window affects encoder allocations directly.
-const char kCongestionPushbackExperiment[] = "WebRTC-CongestionWindowPushback";
-
// When PacerPushbackExperiment is enabled, build-up in the pacer due to
// the congestion window and/or data spikes reduces encoder allocations.
const char kPacerPushbackExperiment[] = "WebRTC-PacerPushbackExperiment";
@@ -55,17 +47,6 @@
return webrtc::field_trial::IsEnabled(kPacerPushbackExperiment);
}
-bool IsCongestionWindowPushbackExperimentEnabled() {
- return webrtc::field_trial::IsEnabled(kCongestionPushbackExperiment) &&
- webrtc::field_trial::IsEnabled(kCwndExperiment);
-}
-
-std::unique_ptr<CongestionWindowPushbackController>
-MaybeInitalizeCongestionWindowPushbackController() {
- return IsCongestionWindowPushbackExperimentEnabled()
- ? absl::make_unique<CongestionWindowPushbackController>()
- : nullptr;
-}
void SortPacketFeedbackVector(std::vector<webrtc::PacketFeedback>* input) {
std::sort(input->begin(), input->end(), PacketFeedbackComparator());
@@ -203,8 +184,6 @@
uint32_t min_pushback_target_bitrate_bps_;
int64_t pacer_expected_queue_ms_ = 0;
double encoding_rate_ratio_ = 1.0;
- const std::unique_ptr<CongestionWindowPushbackController>
- congestion_window_pushback_controller_;
rtc::SequencedTaskChecker sequenced_checker_;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(ControlHandler);
@@ -215,21 +194,14 @@
const Clock* clock)
: observer_(observer),
pacer_controller_(pacer_controller),
- pacer_pushback_experiment_(IsPacerPushbackExperimentEnabled()),
- congestion_window_pushback_controller_(
- MaybeInitalizeCongestionWindowPushbackController()) {
+ pacer_pushback_experiment_(IsPacerPushbackExperimentEnabled()) {
sequenced_checker_.Detach();
}
void ControlHandler::PostUpdates(NetworkControlUpdate update) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_);
if (update.congestion_window) {
- if (congestion_window_pushback_controller_) {
- congestion_window_pushback_controller_->SetDataWindow(
- update.congestion_window.value());
- } else {
- pacer_controller_->OnCongestionWindow(*update.congestion_window);
- }
+ pacer_controller_->OnCongestionWindow(*update.congestion_window);
}
if (update.pacer_config) {
pacer_controller_->OnPacerConfig(*update.pacer_config);
@@ -251,10 +223,6 @@
void ControlHandler::OnOutstandingData(DataSize in_flight_data) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_);
- if (congestion_window_pushback_controller_) {
- congestion_window_pushback_controller_->UpdateOutstandingData(
- in_flight_data.bytes());
- }
OnNetworkInvalidation();
}
@@ -283,10 +251,6 @@
if (!network_available_) {
target_bitrate_bps = 0;
- } else if (congestion_window_pushback_controller_) {
- target_bitrate_bps =
- congestion_window_pushback_controller_->UpdateTargetBitrate(
- target_bitrate_bps);
} else if (!pacer_pushback_experiment_) {
target_bitrate_bps = IsSendQueueFull() ? 0 : target_bitrate_bps;
} else {
diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc
index f8b3155..f26e162 100644
--- a/modules/congestion_controller/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/send_side_congestion_controller.cc
@@ -18,8 +18,8 @@
#include "absl/memory/memory.h"
#include "modules/bitrate_controller/include/bitrate_controller.h"
-#include "modules/congestion_controller/congestion_window_pushback_controller.h"
#include "modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h"
+#include "modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h"
#include "modules/congestion_controller/goog_cc/probe_controller.h"
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
#include "rtc_base/checks.h"
