Drop frames until specified bitrate is achieved.

webrtc/video/vie_encoder.cc

Index: webrtc/video/vie_encoder.cc
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index f78633d532f438f809dce7453fb57accfeff0dd7..56033f4c93a4666ce9c04b46adab289dfb3b4135 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -55,6 +55,17 @@ CpuOveruseOptions GetCpuOveruseOptions(bool full_overuse_time) {
   return options;
 }
 
+int MaximumFrameSizeForBitrate(int kbps) {

[perkj_webrtc, 2017/01/17 08:53:10]

uint32_t encoder_start_bitrate_bps_

 MaximumFrameSizeForBitrate can never get a negative bitrate and thus does not
have to check for that. Param should also be a uint.

[kthelgason, 2017/01/18 09:16:16]

Done.

+  if (kbps > 0) {
+    if (kbps < 300 /* qvga */) {
+      return 320 * 240;
+    } else if (kbps < 500 /* vga */) {
+      return 640 * 480;
+    }
+  }
+  return std::numeric_limits<int>::max();
+}
+
 }  //  namespace
 
 class ViEEncoder::ConfigureEncoderTask : public rtc::QueuedTask {
@@ -244,6 +255,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores,
                        EncodedFrameObserver* encoder_timing)
     : shutdown_event_(true /* manual_reset */, false),
       number_of_cores_(number_of_cores),
+      initial_rampup_(false),
       source_proxy_(new VideoSourceProxy(this)),
       sink_(nullptr),
       settings_(settings),
@@ -441,6 +453,8 @@ void ViEEncoder::ReconfigureEncoder() {
 
   const auto scaling_settings = settings_.encoder->GetScalingSettings();
   if (scaling_enabled_ && scaling_settings.enabled) {
+    // Drop frames and scale down until desired quality is achieved.
+    initial_rampup_ = true;
     if (scaling_settings.thresholds) {
       quality_scaler_.reset(
           new QualityScaler(this, *(scaling_settings.thresholds)));
@@ -560,6 +574,15 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
   last_frame_height_ = video_frame.height();
   last_frame_width_ = video_frame.width();
 
+  int expected_pixel_count =

[perkj_webrtc, 2017/01/17 08:53:10]

nit: please remove this unnecessary temp.

[perkj_webrtc, 2017/01/17 10:30:02]

oh, we should probably do this before Reconfiguring the encoder too since will
drop the frame and wait for a new resolution anyway.

[kthelgason, 2017/01/18 09:16:16]

Done.

+      MaximumFrameSizeForBitrate(encoder_start_bitrate_bps_ / 1000);

[perkj_webrtc, 2017/01/17 08:53:10]

nit: Why not use unit bps here too since that is what we already have.

[kthelgason, 2017/01/18 09:16:16]

I started out implementing it that way, but changed it to kbps for readability.
It's really hard (at least for me) to quickly tell the difference between 100000
and 1000000.

+  if (initial_rampup_ && video_frame.size() > expected_pixel_count) {

[perkj_webrtc, 2017/01/17 08:53:10]

instead of initial_rampup being a bool- can we let it be a counter and drop the
first say 3 frames after a reconfiguration if the configured bitrate is too low
?
Otherwise, what happens if the source can not scale down further? Then we will
never encode these frames and drop all. 

something like 
if( video_frame.size() > MaximumFrameSizeForBitrate(encoder_start_bitrate_bps_)
&& frames_dropped_due_to_bitrate_ > ....) { 

[kthelgason, 2017/01/18 09:16:16]

That's a very good point, thanks. Done!

+    LOG(LS_INFO) << "Dropping frame. Too large for target bitrate.";
+    ScaleDown(kQuality);
+    return;
+  }
+  initial_rampup_ = false;
+
   TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(),
                           "Encode");
 
@@ -725,7 +748,7 @@ void ViEEncoder::ScaleDown(ScaleReason reason) {
   LOG(LS_INFO) << "Scaling down resolution.";
   for (size_t i = 0; i < kScaleReasonSize; ++i) {
     LOG(LS_INFO) << "Scaled " << scale_counter_[i]
-                 << " times for reason: " << (i ? "quality" : "cpu");
+                 << " times for reason: " << (i ? "cpu" : "quality");
   }
 }