Use high quantizer value for "big" frames after a sequence of "small" frames.

remoting/protocol/webrtc_frame_scheduler_simple.cc

Index: remoting/protocol/webrtc_frame_scheduler_simple.cc
diff --git a/remoting/protocol/webrtc_frame_scheduler_simple.cc b/remoting/protocol/webrtc_frame_scheduler_simple.cc
index 6f6d94895cdd46ca8c16d4c97150423a4e3fc5e4..315a3078348173129ed4473b48a275006c092625 100644
--- a/remoting/protocol/webrtc_frame_scheduler_simple.cc
+++ b/remoting/protocol/webrtc_frame_scheduler_simple.cc
@@ -23,16 +23,31 @@ constexpr base::TimeDelta kTargetFrameInterval =
 // Target quantizer at which stop the encoding top-off.
 const int kTargetQuantizerForVp8TopOff = 30;
 
+const int64_t kPixelsPerMegapixel = 1000000;
+
 // Minimum target bitrate per megapixel. The value is chosen experimentally such
 // that when screen is not changing the codec converges to the target quantizer
 // above in less than 10 frames.
-const int kVp8MinimumTargetBitrateKbpsPerMegapixel = 2500;
+const int kVp8MinimumTargetBitrateBytesPerMegapixel = 300000;

[Irfan, 2016/10/03 20:21:49]

"BytesPerMegapixel" sounds like size. I do not understand how this represents
target bitrate.

May be explain more or reconsider units here.

[Sergey Ulanov, 2016/10/06 21:54:34]

The correct units would be bytes/(megapixel*second).
Reverted this line back to what it was.

+
+// Estimated size in bytes per megapixel of encoded frame at target quality
+// level.
+const int kEstimatedBytesPerMegapixel = 150000;

[Irfan, 2016/10/03 20:21:49]

Clarify what is "target quality"  here ?

[Sergey Ulanov, 2016/10/06 21:54:34]

Done.

+
+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::WebrtcFrameSchedulerSimple()
     : pacing_bucket_(LeakyBucket::kUnlimitedDepth, 0),
-      frame_processing_delay_us_(kStatsWindow) {}
+      frame_processing_delay_us_(kStatsWindow),
+      encoded_frame_size_(kStatsWindow) {}

[Irfan, 2016/10/03 20:21:49]

may be encoded_frame_size_bytes_ ?

[Sergey Ulanov, 2016/10/06 21:54:34]

Removed it now and replaced with updated_region_area_.

 WebrtcFrameSchedulerSimple::~WebrtcFrameSchedulerSimple() {}
 
 void WebrtcFrameSchedulerSimple::Start(const base::Closure& capture_callback) {
@@ -71,20 +86,34 @@ bool WebrtcFrameSchedulerSimple::GetEncoderFrameParams(
   }
 
   // TODO(sergeyu): This logic is applicable only to VP8. Reconsider it for VP9.
-  int minimum_bitrate =
-      static_cast<uint64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) *
-      frame.size().width() * frame.size().height() / 1000000LL;
-  params_out->bitrate_kbps =
-      std::max(minimum_bitrate, pacing_bucket_.rate() * 8 / 1000);
-
-  // TODO(sergeyu): Currently duration is always set to 1/15 of a second.
-  // Experiment with different values, and try changing it dynamically.
-  params_out->duration = base::TimeDelta::FromSeconds(1) / 15;
-
+  int minimum_bitrate_bytes_per_second =
+      kVp8MinimumTargetBitrateBytesPerMegapixel * frame.size().width() *
+      frame.size().height() / kPixelsPerMegapixel;
+  int bitrate_bytes_per_second =
+      std::max(minimum_bitrate_bytes_per_second, pacing_bucket_.rate());
+  params_out->bitrate_kbps = bitrate_bytes_per_second * 8 / 1000;
+
+  params_out->duration = kTargetFrameInterval;
   params_out->key_frame = key_frame_request_;
   key_frame_request_ = false;
 
   params_out->vpx_min_quantizer = 10;
+
+  // If bandwidth is being underutilized then libvpx is likely to choose the
+  // minimum allowed quantizer value, which means that encoded frame size may be
+  // significantly bigger than the bandwidth allows. Detect this case and set
+  // vpx_min_quantizer to 60. The quality will be topped off later.
+  if (encoded_frame_size_.Average() * kTargetFrameRate <

[Irfan, 2016/10/03 20:21:49]

Should we measure the frame rate to get real usage ?

[Sergey Ulanov, 2016/10/06 21:54:34]

No. What matters here is the bitrate that libvpx observes, not the actual BW
usages. Duration is set to kTargetFrameInterval, so from libvpx point of view
the stream is always at 30 FPS.

But I updated this logic now to use updated_region area instead of encoded frame
size.

+      bitrate_bytes_per_second / 2) {
+    int expected_frame_size = GetRegionArea(frame.updated_region()) *
+                              kEstimatedBytesPerMegapixel / kPixelsPerMegapixel;
+    base::TimeDelta expected_send_delay = base::TimeDelta::FromMicroseconds(
+        base::Time::kMicrosecondsPerSecond * expected_frame_size /
+        pacing_bucket_.rate());
+    if (expected_send_delay > kTargetFrameInterval)
+      params_out->vpx_min_quantizer = 60;
+  }
+
   params_out->vpx_max_quantizer = 63;
 
   params_out->clear_active_map = !top_off_is_active_;
@@ -97,6 +126,7 @@ void WebrtcFrameSchedulerSimple::OnFrameEncoded(
     const webrtc::EncodedImageCallback::Result& send_result) {
   base::TimeTicks now = base::TimeTicks::Now();
   pacing_bucket_.RefillOrSpill(encoded_frame.data.size(), now);
+  encoded_frame_size_.Record(encoded_frame.data.size());
 
   if (encoded_frame.data.empty()) {
     top_off_is_active_ = false;