Update VideoFramePump to pass un-changed frames to encoders.

remoting/codec/video_encoder_vpx.cc

 // Copyright 2013 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/codec/video_encoder_vpx.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/sys_info.h"
 #include "remoting/base/util.h"
 #include "remoting/proto/video.pb.h"
 #include "third_party/libyuv/include/libyuv/convert_from_argb.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_region.h"
 
 extern "C" {
 #define VPX_CODEC_DISABLE_COMPAT 1
 #include "third_party/libvpx/source/libvpx/vpx/vpx_encoder.h"
 #include "third_party/libvpx/source/libvpx/vpx/vp8cx.h"
 }
 
 namespace remoting {
 
 namespace {
 
-// Name of command-line flag to enable VP9 to use I444 by default.
-const char kEnableI444SwitchName[] = "enable-i444";
-
 // Number of bytes in an RGBx pixel.
 const int kBytesPerRgbPixel = 4;
 
 // Defines the dimension of a macro block. This is used to compute the active
 // map for the encoder.
 const int kMacroBlockSize = 16;
 
 // Magic encoder profile numbers for I420 and I444 input formats.
 const int kVp9I420ProfileNumber = 0;
 const int kVp9I444ProfileNumber = 1;
@@ skipping 55 matching lines @@
   // Configure VP9 for I420 or I444 source frames.
   config->g_profile =
       lossless_color ? kVp9I444ProfileNumber : kVp9I420ProfileNumber;
 
   if (lossless_encode) {
     // Disable quantization entirely, putting the encoder in "lossless" mode.
     config->rc_min_quantizer = 0;
     config->rc_max_quantizer = 0;
     config->rc_end_usage = VPX_VBR;
   } else {
-    config->rc_min_quantizer = 4;
+    // TODO(wez): Set quantization range to 4-40, once the libvpx encoder is
+    // updated not to output any bits if nothing needs topping-off.
+    config->rc_min_quantizer = 20;
     config->rc_max_quantizer = 30;
     config->rc_end_usage = VPX_CBR;
     // In the absence of a good bandwidth estimator set the target bitrate to a
     // conservative default.
     config->rc_target_bitrate = 500;
   }
 }
 
 void SetVp8CodecOptions(vpx_codec_ctx_t* codec) {
   // CPUUSED of 16 will have the smallest CPU load. This turns off sub-pixel
@@ skipping 144 matching lines @@
     //                                codec_->config.enc->g_h));
     codec_.reset();
   }
 }
 
 scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
     const webrtc::DesktopFrame& frame) {
   DCHECK_LE(32, frame.size().width());
   DCHECK_LE(32, frame.size().height());
 
+  // If nothing has changed, then only encode data to top-off from lossy to
+  // lossless quantization.
+  if (frame.updated_region().is_empty()) {
+    // VP8 doesn't support top-off, and VP9's lossless mode doesn't need it.
+    if (!use_vp9_ || lossless_encode_)
+      return nullptr;
+    if (!--topoff_frame_count_) {

[Sergey Ulanov, 2015/05/26 21:57:22]

nit: --topoff_frame_count > 0 (or != 0) would be more readable here

[Wez, 2015/06/09 01:46:20]

Plus, in cleaning it up, I fixed a bug in my implementation... :-/

+      return nullptr;
+    }
+  } else {
+    // Just allow a couple of fully empty frames' worth of top-off.
+    topoff_frame_count_ = 2;

[Sergey Ulanov, 2015/05/26 21:57:22]

topoff_frame_count_ is set even when using VP8, but it's ignored later. Maybe
wrap this while block in if(use_vp9_) {}, to make it clear that topoff applies
only for VP9.

[Wez, 2015/06/09 01:46:20]

Done.

+  }
+
   base::TimeTicks encode_start_time = base::TimeTicks::Now();
 
   // Create or reconfigure the codec to match the size of |frame|.
   if (!codec_ ||
       (image_ &&
        !frame.size().equals(webrtc::DesktopSize(image_->w, image_->h)))) {
     Configure(frame.size());
   }
 
   // Convert the updated capture data ready for encode.
   webrtc::DesktopRegion updated_region;
   PrepareImage(frame, &updated_region);
 
   // Update active map based on updated region.
   SetActiveMapFromRegion(updated_region);

[Sergey Ulanov, 2015/05/26 21:57:22]

Do we need to supply active map for top-off, or does VPX keep track of the
changes in the previous frames?

[Wez, 2015/06/09 01:46:20]

Cyclic Refresh will top-off even during "active" frames, so we still need to set
the active map to cover the active areas.

 
   // Apply active map to the encoder.
   vpx_active_map_t act_map;
   act_map.rows = active_map_height_;
   act_map.cols = active_map_width_;
   act_map.active_map = active_map_.get();
   if (vpx_codec_control(codec_.get(), VP8E_SET_ACTIVEMAP, &act_map)) {
     LOG(ERROR) << "Unable to apply active map";
   }
 
@@ skipping 45 matching lines @@
       (base::TimeTicks::Now() - encode_start_time).InMillisecondsRoundedUp());
 
   return packet.Pass();
 }
 
 VideoEncoderVpx::VideoEncoderVpx(bool use_vp9)
     : use_vp9_(use_vp9),
       lossless_encode_(false),
       lossless_color_(false),
       active_map_width_(0),
-      active_map_height_(0) {
-  if (use_vp9_) {
-    // Use I444 colour space, by default, if specified on the command-line.
-    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-            kEnableI444SwitchName)) {
-      SetLosslessColor(true);
-    }
-  }
+      active_map_height_(0),
+      topoff_frame_count_(0) {
 }
 
 void VideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {
   DCHECK(use_vp9_ || !lossless_color_);
   DCHECK(use_vp9_ || !lossless_encode_);
 
   // Tear down |image_| if it no longer matches the size and color settings.
   // PrepareImage() will then create a new buffer of the required dimensions if
   // |image_| is not allocated.
   FreeImageIfMismatched(lossless_color_, size, &image_, &image_buffer_);
@@ skipping 174 matching lines @@
             kMacroBlockSize * (y + 1)));
       }
       x0 = x1 + 1;
     }
   }
   updated_region->IntersectWith(
       webrtc::DesktopRect::MakeWH(image_->w, image_->h));
 }
 
 }  // namespace remoting