Limit target encoder bitrate change frequency in the frame scheduler.

remoting/protocol/webrtc_frame_scheduler_simple.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/webrtc_frame_scheduler_simple.h"
 
 #include <algorithm>
 
 #include "remoting/protocol/frame_stats.h"
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
@@ skipping 27 matching lines @@
 // ensure that they get delivered to the client as soon as possible, in exchange
 // for lower-quality image.
 const int kBigFrameThresholdPixels = 300000;
 
 // Estimated size (in bytes per megapixel) of encoded frame at target quantizer
 // value (see kTargetQuantizerForVp8TopOff). Compression ratio varies depending
 // on the image, so this is just a rough estimate. It's used to predict when
 // encoded "big" frame may be too large to be delivered to the client quickly.
 const int kEstimatedBytesPerMegapixel = 100000;
 
+// Interval over which the bandwidth estimates is averaged to set target encoder
+// bitrate.
+constexpr base::TimeDelta kBandwidthAveragingInterval =
+    base::TimeDelta::FromSeconds(1);
+
 int64_t GetRegionArea(const webrtc::DesktopRegion& region) {
   int64_t result = 0;
   for (webrtc::DesktopRegion::Iterator r(region); !r.IsAtEnd(); r.Advance()) {
     result += r.rect().width() * r.rect().height();
   }
   return result;
 }
 
 }  // namespace
 
+WebrtcFrameSchedulerSimple::EncoderBitrateFilter::EncoderBitrateFilter() {}
+WebrtcFrameSchedulerSimple::EncoderBitrateFilter::~EncoderBitrateFilter() {}
+
+void WebrtcFrameSchedulerSimple::EncoderBitrateFilter::SetBandwidthEstimate(
+    int bandwidth_kbps,
+    base::TimeTicks now) {
+  bandwidth_samples_.push(std::make_pair(now, bandwidth_kbps));
+  bandwidth_samples_sum_ += bandwidth_kbps;
+
+  while (bandwidth_samples_.size() > 1 &&

[Jamie, 2016/12/09 23:39:18]

It took me a while to work out why you needed at least two elements for this
loop before I realized that you don't want to remove the one you just added. I
think it might be clearer to clean up the old entries (using !empty() as a
conditional, which is a lot more self-documenting) and then adding the new one
afterwards.

[Sergey Ulanov, 2016/12/10 01:22:57]

Done.

+         now - bandwidth_samples_.front().first > kBandwidthAveragingInterval) {
+    bandwidth_samples_sum_ -= bandwidth_samples_.front().second;
+    bandwidth_samples_.pop();
+  }
+}
+
+int WebrtcFrameSchedulerSimple::EncoderBitrateFilter::GetTargetBitrateKbps(
+    webrtc::DesktopSize size,
+    base::TimeTicks now) {
+  DCHECK(!bandwidth_samples_.empty());
+
+  // TODO(sergeyu): This logic is applicable only to VP8. Reconsider it for
+  // VP9.
+  int bandwidth_estimate = bandwidth_samples_sum_ / bandwidth_samples_.size();
+  int minimum_bitrate =
+      static_cast<int64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) *
+      size.width() * size.height() / 1000000LL;
+  int target_bitrate = std::max(minimum_bitrate, bandwidth_estimate);
+
+  // Update encoder bitrate only when it changes by more than 30%. This is
+  // necessary because the encoder resets internal state it's reconfigured and

[Jamie, 2016/12/09 23:39:18]

s/it's/when it's/

[Sergey Ulanov, 2016/12/10 01:22:57]

Done.

+  // this causes visible drop in quality.
+  if (current_target_bitrate_ == 0 ||
+      std::abs(target_bitrate - current_target_bitrate_) >
+          current_target_bitrate_ / 3) {

[Jamie, 2016/12/09 23:39:18]

Add a named constant with rationale to explain why 33% was chosen.

[Sergey Ulanov, 2016/12/10 01:22:56]

Done.

+    current_target_bitrate_ = target_bitrate;
+  }
+  return current_target_bitrate_;
+}
+
 WebrtcFrameSchedulerSimple::WebrtcFrameSchedulerSimple()
     : pacing_bucket_(LeakyBucket::kUnlimitedDepth, 0),
       frame_processing_delay_us_(kStatsWindow),
       updated_region_area_(kStatsWindow),
       weak_factory_(this) {}
+
 WebrtcFrameSchedulerSimple::~WebrtcFrameSchedulerSimple() {}
 
 void WebrtcFrameSchedulerSimple::OnKeyFrameRequested() {
   DCHECK(thread_checker_.CalledOnValidThread());
   key_frame_request_ = true;
   ScheduleNextFrame(base::TimeTicks::Now());
 }
 
 void WebrtcFrameSchedulerSimple::OnChannelParameters(int packet_loss,
                                                      base::TimeDelta rtt) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   rtt_estimate_ = rtt;
 }
 
-void WebrtcFrameSchedulerSimple::OnTargetBitrateChanged(int bitrate_kbps) {
+void WebrtcFrameSchedulerSimple::OnTargetBitrateChanged(int bandwidth_kbps) {
   DCHECK(thread_checker_.CalledOnValidThread());
   base::TimeTicks now = base::TimeTicks::Now();
-  pacing_bucket_.UpdateRate(bitrate_kbps * 1000 / 8, now);
+  pacing_bucket_.UpdateRate(bandwidth_kbps * 1000 / 8, now);
+  encoder_bitrate_.SetBandwidthEstimate(bandwidth_kbps, now);
   ScheduleNextFrame(now);
 }
 
 void WebrtcFrameSchedulerSimple::Start(
     WebrtcDummyVideoEncoderFactory* video_encoder_factory,
     const base::Closure& capture_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   capture_callback_ = capture_callback;
   video_encoder_factory->SetVideoChannelStateObserver(
       weak_factory_.GetWeakPtr());
@@ skipping 17 matching lines @@
 
   base::TimeTicks now = base::TimeTicks::Now();
 
   if (frame.updated_region().is_empty() && !top_off_is_active_ &&
       !key_frame_request_) {
     frame_pending_ = false;
     ScheduleNextFrame(now);
     return false;
   }
 
-  // TODO(sergeyu): This logic is applicable only to VP8. Reconsider it for VP9.
-  int minimum_bitrate =
-      static_cast<int64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) *
-      frame.size().width() * frame.size().height() / 1000000LL;
   params_out->bitrate_kbps =
-      std::max(minimum_bitrate, pacing_bucket_.rate() * 8 / 1000);
+      encoder_bitrate_.GetTargetBitrateKbps(frame.size(), now);
 
   params_out->duration = kTargetFrameInterval;
   params_out->key_frame = key_frame_request_;
   key_frame_request_ = false;
 
   params_out->vpx_min_quantizer = 10;
 
   int64_t updated_area = params_out->key_frame
                              ? frame.size().width() * frame.size().height()
                              : GetRegionArea(frame.updated_region());
@@ skipping 89 matching lines @@
 void WebrtcFrameSchedulerSimple::CaptureNextFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!frame_pending_);
   last_capture_started_time_ = base::TimeTicks::Now();
   frame_pending_ = true;
   capture_callback_.Run();
 }
 
 }  // namespace protocol
 }  // namespace remoting