Refactor QualityScaler and MovingAverage

webrtc/modules/video_coding/utility/quality_scaler.cc

Index: webrtc/modules/video_coding/utility/quality_scaler.cc
diff --git a/webrtc/modules/video_coding/utility/quality_scaler.cc b/webrtc/modules/video_coding/utility/quality_scaler.cc
index c509e843642f2237e94fe1727749479a9523faa7..05a1ef7b339ec9f38504fad7316b263071393052 100644
--- a/webrtc/modules/video_coding/utility/quality_scaler.cc
+++ b/webrtc/modules/video_coding/utility/quality_scaler.cc
@@ -10,10 +10,11 @@
 
 #include "webrtc/modules/video_coding/utility/quality_scaler.h"
 
+#include <algorithm>
+
 namespace webrtc {
 
 namespace {
-static const int kMinFps = 5;
 // Threshold constant used until first downscale (to permit fast rampup).
 static const int kMeasureSecondsFastUpscale = 2;
 static const int kMeasureSecondsUpscale = 5;
@@ -54,14 +55,15 @@ void QualityScaler::Init(int low_qp_threshold,
                          int width,
                          int height,
                          int fps) {
-  ClearSamples();
   low_qp_threshold_ = low_qp_threshold;
   high_qp_threshold_ = high_qp_threshold;
   downscale_shift_ = 0;
-  // Use a faster window for upscaling initially (but be more graceful later).
-  // This enables faster initial rampups without risking strong up-down
-  // behavior later.
-  measure_seconds_upscale_ = kMeasureSecondsFastUpscale;
+
+  fast_rampup_ = true;
+
+  ClearSamples();
+  ReportFramerate(fps);
+
   const int init_width = width;
   const int init_height = height;
   if (initial_bitrate_kbps > 0) {
@@ -76,24 +78,25 @@ void QualityScaler::Init(int low_qp_threshold,
       height /= 2;
     }
   }
-
-  // Zero out width/height so they can be checked against inside
-  // UpdateTargetResolution.
-  res_.width = res_.height = 0;
   UpdateTargetResolution(init_width, init_height);
   ReportFramerate(fps);
 }
 
 // Report framerate(fps) to estimate # of samples.
 void QualityScaler::ReportFramerate(int framerate) {
-  framerate_ = framerate;
-  UpdateSampleCounts();
+  // Use a faster window for upscaling initially.
+  // This enables faster initial rampups without risking strong up-down
+  // behavior later.
+  if (fast_rampup_)

[magjed_webrtc, 2016/09/05 15:01:52]

I would prefer to write this as:
num_samples_upscale_ = framerate * (fast_rampup_ ? kMeasureSecondsFastUpscale :
kMeasureSecondsUpscale);

+    num_samples_upscale_ = framerate * kMeasureSecondsFastUpscale;
+  else
+    num_samples_upscale_ = framerate * kMeasureSecondsUpscale;
+  num_samples_downscale_ = framerate * kMeasureSecondsDownscale;
 }
 
 void QualityScaler::ReportQP(int qp) {
   framedrop_percent_.AddSample(0);
-  average_qp_downscale_.AddSample(qp);
-  average_qp_upscale_.AddSample(qp);
+  average_qp_.AddSample(qp);
 }
 
 void QualityScaler::ReportDroppedFrame() {
@@ -103,27 +106,36 @@ void QualityScaler::ReportDroppedFrame() {
 void QualityScaler::OnEncodeFrame(int width, int height) {
   // Should be set through InitEncode -> Should be set by now.
   RTC_DCHECK_GE(low_qp_threshold_, 0);
-  RTC_DCHECK_GT(num_samples_upscale_, 0u);
-  RTC_DCHECK_GT(num_samples_downscale_, 0u);
-
-  // Update scale factor.
-  int avg_drop = 0;
-  int avg_qp = 0;
-
-  if ((framedrop_percent_.GetAverage(num_samples_downscale_, &avg_drop) &&
-       avg_drop >= kFramedropPercentThreshold) ||
-      (average_qp_downscale_.GetAverage(num_samples_downscale_, &avg_qp) &&
-       avg_qp > high_qp_threshold_)) {
-    AdjustScale(false);
-  } else if (average_qp_upscale_.GetAverage(num_samples_upscale_, &avg_qp) &&
-             avg_qp <= low_qp_threshold_) {
-    AdjustScale(true);
+  if (res_.width != width || res_.height != height) {
+    UpdateTargetResolution(width, height);
+  }
+
+  if ((framedrop_percent_.size() >= num_samples_downscale_ &&
+      (framedrop_percent_.GetAverage(num_samples_downscale_) >=
+       kFramedropPercentThreshold)) ||
+      (average_qp_.size() >= num_samples_downscale_ &&
+      (average_qp_.GetAverage(num_samples_downscale_) > high_qp_threshold_))) {
+    // too much framedrop or too high QP. We want to scale down.

[magjed_webrtc, 2016/09/05 15:01:52]

nit: Start with capital letter in comments.

+    downscale_shift_ = std::min(maximum_shift_, downscale_shift_ + 1);
+    if (fast_rampup_) {
+      fast_rampup_ = false;
+      num_samples_upscale_ =
+          (num_samples_upscale_ / kMeasureSecondsFastUpscale)
+          * kMeasureSecondsUpscale;
+    }
+    ClearSamples();
+  } else if (average_qp_.size() >= num_samples_upscale_ &&
+      average_qp_.GetAverage(num_samples_upscale_) <= low_qp_threshold_) {
+    // QP has been high. We want to try a higher resolution.
+    downscale_shift_ = std::max(0, downscale_shift_ - 1);
+    ClearSamples();
   }
-  UpdateTargetResolution(width, height);
 }
 
 QualityScaler::Resolution QualityScaler::GetScaledResolution() const {
-  return res_;
+  int frame_width = res_.width >> downscale_shift_;

[magjed_webrtc, 2016/09/05 15:01:52]

Use const (or inline these).

+  int frame_height = res_.height >> downscale_shift_;
+  return Resolution{frame_width, frame_height};
 }
 
 rtc::scoped_refptr<VideoFrameBuffer> QualityScaler::GetScaledBuffer(
@@ -142,50 +154,20 @@ rtc::scoped_refptr<VideoFrameBuffer> QualityScaler::GetScaledBuffer(
   return scaled_buffer;
 }
 
-void QualityScaler::UpdateTargetResolution(int frame_width, int frame_height) {
-  RTC_DCHECK_GE(downscale_shift_, 0);
-  int shifts_performed = 0;
-  for (int shift = downscale_shift_;
-       shift > 0 && (frame_width / 2 >= kMinDownscaleDimension) &&
-       (frame_height / 2 >= kMinDownscaleDimension);
-       --shift, ++shifts_performed) {
-    frame_width /= 2;
-    frame_height /= 2;
-  }
-  // Clamp to number of shifts actually performed to not be stuck trying to
-  // scale way beyond QVGA.
-  downscale_shift_ = shifts_performed;
-  if (res_.width == frame_width && res_.height == frame_height) {
-    // No reset done/needed, using same resolution.
-    return;
+void QualityScaler::UpdateTargetResolution(int width, int height) {
+  if (width < kMinDownscaleDimension || height < kMinDownscaleDimension) {
+    maximum_shift_ = 0;
+  } else {
+    maximum_shift_ = static_cast<int>(
+        std::log2(std::min(width, height)/kMinDownscaleDimension));

[magjed_webrtc, 2016/09/05 15:01:52]

nit: formatting. Use git cl format.

   }
-  res_.width = frame_width;
-  res_.height = frame_height;
-  ClearSamples();
+  res_ = Resolution{width, height};

[magjed_webrtc, 2016/09/05 15:01:52]

Maybe this variable should be called target_resolution_ instead.

 }
 
 void QualityScaler::ClearSamples() {
   framedrop_percent_.Reset();
-  average_qp_downscale_.Reset();
-  average_qp_upscale_.Reset();
-}
-
-void QualityScaler::UpdateSampleCounts() {
-  num_samples_downscale_ = static_cast<size_t>(
-      kMeasureSecondsDownscale * (framerate_ < kMinFps ? kMinFps : framerate_));
-  num_samples_upscale_ = static_cast<size_t>(
-      measure_seconds_upscale_ * (framerate_ < kMinFps ? kMinFps : framerate_));
+  average_qp_.Reset();
 }
 
-void QualityScaler::AdjustScale(bool up) {
-  downscale_shift_ += up ? -1 : 1;
-  if (downscale_shift_ < 0)
-    downscale_shift_ = 0;
-  if (!up) {
-    // First downscale hit, start using a slower threshold for going up.
-    measure_seconds_upscale_ = kMeasureSecondsUpscale;
-    UpdateSampleCounts();
-  }
-}
 
 }  // namespace webrtc