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

webrtc/modules/congestion_controller/send_side_congestion_controller.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 14 matching lines @@
 #include "webrtc/rtc_base/ptr_util.h"
 #include "webrtc/rtc_base/rate_limiter.h"
 #include "webrtc/rtc_base/socket.h"
 #include "webrtc/rtc_base/timeutils.h"
 #include "webrtc/system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 namespace {
 
 const char kCwndExperiment[] = "WebRTC-CwndExperiment";
+const char kPacerPushbackExperiment[] = "WebRTC-PacerPushbackExperiment";
 const int64_t kDefaultAcceptedQueueMs = 250;
 
 bool CwndExperimentEnabled() {
   std::string experiment_string =
       webrtc::field_trial::FindFullName(kCwndExperiment);
   // The experiment is enabled iff the field trial string begins with "Enabled".
   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 =
       webrtc::field_trial::FindFullName(kCwndExperiment);
   int parsed_values =
       sscanf(experiment_string.c_str(), "Enabled-%" PRId64, accepted_queue_ms);
   if (parsed_values == 1) {
     RTC_CHECK_GE(*accepted_queue_ms, 0)
         << "Accepted must be greater than or equal to 0.";
     return true;
@@ skipping 61 matching lines @@
       last_reported_bitrate_bps_(0),
       last_reported_fraction_loss_(0),
       last_reported_rtt_(0),
       network_state_(kNetworkUp),
       pause_pacer_(false),
       pacer_paused_(false),
       min_bitrate_bps_(congestion_controller::GetMinBitrateBps()),
       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_)) {
     LOG(LS_WARNING) << "Failed to parse parameters for CwndExperiment "
                        "from field trial string. Experiment disabled.";
     in_cwnd_experiment_ = false;
   }
 }
 
 SendSideCongestionController::SendSideCongestionController(
@@ skipping 16 matching lines @@
       last_reported_bitrate_bps_(0),
       last_reported_fraction_loss_(0),
       last_reported_rtt_(0),
       network_state_(kNetworkUp),
       pause_pacer_(false),
       pacer_paused_(false),
       min_bitrate_bps_(congestion_controller::GetMinBitrateBps()),
       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_)) {
     LOG(LS_WARNING) << "Failed to parse parameters for CwndExperiment "
                        "from field trial string. Experiment disabled.";
     in_cwnd_experiment_ = false;
   }
 }
 
 SendSideCongestionController::~SendSideCongestionController() {}
@@ skipping 236 matching lines @@
   uint8_t fraction_loss;
   int64_t rtt;
   bool estimate_changed = bitrate_controller_->GetNetworkParameters(
       &bitrate_bps, &fraction_loss, &rtt);
   if (estimate_changed) {
     pacer_->SetEstimatedBitrate(bitrate_bps);
     probe_controller_->SetEstimatedBitrate(bitrate_bps);
     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;
     {
       rtc::CritScope cs(&bwe_lock_);
       probing_interval_ms = delay_based_bwe_->GetExpectedBwePeriodMs();
     }
     {
       rtc::CritScope cs(&observer_lock_);
       if (observer_) {
@@ skipping 26 matching lines @@
 bool SendSideCongestionController::IsSendQueueFull() const {
   return pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
 }
 
 bool SendSideCongestionController::IsNetworkDown() const {
   rtc::CritScope cs(&network_state_lock_);
   return network_state_ == kNetworkDown;
 }
 
 }  // namespace webrtc