Push back on the video encoder to avoid building queues in the pacer.

webrtc/modules/congestion_controller/send_side_congestion_controller.cc

Index: webrtc/modules/congestion_controller/send_side_congestion_controller.cc
diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
index fee49902c52ed1f7e6aeb6060e54acf94e6a01ca..9294c77c3f807278136d92825e12301c08079e33 100644
--- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
@@ -32,6 +32,7 @@ namespace webrtc {
 namespace {
 
 const char kCwndExperiment[] = "WebRTC-CwndExperiment";
+const char kPacerPushbackExperiment[] = "WebRTC-PacerPushbackExperiment";
 const int64_t kDefaultAcceptedQueueMs = 250;
 
 bool CwndExperimentEnabled() {
@@ -41,6 +42,13 @@ bool CwndExperimentEnabled() {
   return experiment_string.find("Enabled") == 0;
 }
 
+bool PacerPushbackExperimentEnabled() {
+  std::string experiment_string =
+      webrtc::field_trial::FindFullName(kPacerPushbackExperiment);
+  // The experiment is enabled iff the field trial string begins with "Enabled".
+  return experiment_string.find("Enabled") == 0;
+}

[stefan-webrtc, 2017/08/29 12:17:20]

Replace with:
webrtc::field_trial::IsEnabled(kPacerPushbackExperiment)

[philipel, 2017/08/29 15:14:18]

Done.

+
 bool ReadCwndExperimentParameter(int64_t* accepted_queue_ms) {
   RTC_DCHECK(accepted_queue_ms);
   std::string experiment_string =
@@ -122,7 +130,8 @@ SendSideCongestionController::SendSideCongestionController(
       delay_based_bwe_(new DelayBasedBwe(event_log_, clock_)),
       in_cwnd_experiment_(CwndExperimentEnabled()),
       accepted_queue_ms_(kDefaultAcceptedQueueMs),
-      was_in_alr_(0) {
+      was_in_alr_(false),
+      pacer_pushback_experiment_(PacerPushbackExperimentEnabled()) {
   delay_based_bwe_->SetMinBitrate(min_bitrate_bps_);
   if (in_cwnd_experiment_ &&
       !ReadCwndExperimentParameter(&accepted_queue_ms_)) {
@@ -159,7 +168,8 @@ SendSideCongestionController::SendSideCongestionController(
       delay_based_bwe_(new DelayBasedBwe(event_log_, clock_)),
       in_cwnd_experiment_(CwndExperimentEnabled()),
       accepted_queue_ms_(kDefaultAcceptedQueueMs),
-      was_in_alr_(0) {
+      was_in_alr_(false),
+      pacer_pushback_experiment_(PacerPushbackExperimentEnabled()) {
   delay_based_bwe_->SetMinBitrate(min_bitrate_bps_);
   if (in_cwnd_experiment_ &&
       !ReadCwndExperimentParameter(&accepted_queue_ms_)) {
@@ -416,7 +426,25 @@ void SendSideCongestionController::MaybeTriggerOnNetworkChanged() {
     retransmission_rate_limiter_->SetMaxRate(bitrate_bps);
   }
 
-  bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps;
+  if (!pacer_pushback_experiment_) {
+    bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps;
+  } else {
+    if (IsNetworkDown()) {
+      bitrate_bps = 0;
+    } else {
+      int64_t queue_length_ms = pacer_->ExpectedQueueTimeMs();
+
+      if (queue_length_ms == 0) {
+        encoding_rate_ = 1.0;
+      } else if (queue_length_ms > 50) {

[stefan-webrtc, 2017/08/29 12:17:20]

50 ms dead zone sounds pretty small. WDYT of increasing to at least 100 ms?

[philipel, 2017/08/29 15:14:18]

I got better results using 50 ms rather than 100, but I think it might not
matter, see my comment below.

+        float encoding_rate = 1.0 - queue_length_ms / 1000.0;

[stefan-webrtc, 2017/08/29 12:17:20]

Should this be something like this, so that we have a dead zone followed by a
linear slope, instead of now where we'll have a jump and then a slope?
1.0 - (queue_length_ms - 50) / 1000.0

[philipel, 2017/08/29 15:14:18]

The idea is to drain the queue completely, so I think having a jump is
preferable.

I also realized we might need to change this P-like controller to a PI-like
controller. The reason we need pushback in the first place is to not build
sender side queues since the pacing rate will be 1.0 or something close to it.
In the current state we could still cause a standing queue in the pacer if the
encoder overproduce >= 5%.

+        encoding_rate_ = std::min(encoding_rate_, encoding_rate);
+        encoding_rate_ = std::max(encoding_rate_, 0.0f);
+      }
+
+      bitrate_bps *= encoding_rate_;
+    }
+  }
 
   if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) {
     int64_t probing_interval_ms;