Extend VideoControl to allow clients to request lossless modes.

- Lossless encode/color requests may be ignored by codecs.
- VideoEncoderVpx supports lossless color when using VP9.
- The --enable-i444 flag now controls the default color mode for VP9.

BUG=260879,134202

Review URL: https://codereview.chromium.org/304653002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273752 0039d316-1c4b-4281-b951-d872f2087c98

remoting/codec/video_decoder_vpx_unittest.cc

@@ -45,7 +45,7 @@ class VideoDecoderVp8Test : public VideoDecoderVpxTest {
 class VideoDecoderVp9Test : public VideoDecoderVpxTest {
  protected:
   VideoDecoderVp9Test() {
-    encoder_ = VideoEncoderVpx::CreateForVP9I420();
+    encoder_ = VideoEncoderVpx::CreateForVP9();
     decoder_ = VideoDecoderVpx::CreateForVP9();
   }
 };

remoting/codec/video_encoder.h

@@ -21,6 +21,10 @@ class VideoEncoder {
  public:
   virtual ~VideoEncoder() {}
 
+  // Request that the encoder provide lossless encoding, or color, if possible.
+  virtual void SetLosslessEncode(bool want_lossless) {}
+  virtual void SetLosslessColor(bool want_lossless) {}
+
   // Encode an image stored in |frame|.
   virtual scoped_ptr<VideoPacket> Encode(const webrtc::DesktopFrame& frame) = 0;
 };

remoting/codec/video_encoder_vpx.cc

@@ -5,6 +5,7 @@
 #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"
@@ -24,6 +25,9 @@ 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;
 
@@ -97,7 +101,9 @@ ScopedVpxCodec CreateVP8Codec(const webrtc::DesktopSize& size) {
   return codec.Pass();
 }
 
-ScopedVpxCodec CreateVP9Codec(bool use_i444, const webrtc::DesktopSize& size) {
+ScopedVpxCodec CreateVP9Codec(const webrtc::DesktopSize& size,
+                              bool lossless_color,
+                              bool lossless_encode) {
   ScopedVpxCodec codec(new vpx_codec_ctx_t);
 
   // Configure the encoder.
@@ -111,11 +117,18 @@ ScopedVpxCodec CreateVP9Codec(bool use_i444, const webrtc::DesktopSize& size) {
   SetCommonCodecParameters(size, &config);
 
   // Configure VP9 for I420 or I444 source frames.
-  config.g_profile = use_i444 ? kVp9I444ProfileNumber : kVp9I420ProfileNumber;
-
-  // Disable quantization entirely, putting the encoder in "lossless" mode.
-  config.rc_min_quantizer = 0;
-  config.rc_max_quantizer = 0;
+  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;
+  } else {
+    // Lossy encode using the same settings as for VP8.
+    config.rc_min_quantizer = 20;
+    config.rc_max_quantizer = 30;
+  }
 
   if (vpx_codec_enc_init(codec.get(), algo, &config, 0))
     return ScopedVpxCodec();
@@ -202,26 +215,29 @@ void CreateImage(bool use_i444,
 
 // static
 scoped_ptr<VideoEncoderVpx> VideoEncoderVpx::CreateForVP8() {
-  return scoped_ptr<VideoEncoderVpx>(
-      new VideoEncoderVpx(base::Bind(&CreateVP8Codec),
-                          base::Bind(&CreateImage, false)));
+  return scoped_ptr<VideoEncoderVpx>(new VideoEncoderVpx(false));
 }
 
 // static
-scoped_ptr<VideoEncoderVpx> VideoEncoderVpx::CreateForVP9I420() {
-  return scoped_ptr<VideoEncoderVpx>(
-      new VideoEncoderVpx(base::Bind(&CreateVP9Codec, false),
-                          base::Bind(&CreateImage, false)));
+scoped_ptr<VideoEncoderVpx> VideoEncoderVpx::CreateForVP9() {
+  return scoped_ptr<VideoEncoderVpx>(new VideoEncoderVpx(true));
 }
 
-// static
-scoped_ptr<VideoEncoderVpx> VideoEncoderVpx::CreateForVP9I444() {
-  return scoped_ptr<VideoEncoderVpx>(
-      new VideoEncoderVpx(base::Bind(&CreateVP9Codec, true),
-                          base::Bind(&CreateImage, true)));
+VideoEncoderVpx::~VideoEncoderVpx() {}
+
+void VideoEncoderVpx::SetLosslessEncode(bool want_lossless) {
+  if (use_vp9_ && (want_lossless != lossless_encode_)) {
+    lossless_encode_ = want_lossless;
+    codec_.reset(); // Force encoder re-initialization.
+  }
 }
 
-VideoEncoderVpx::~VideoEncoderVpx() {}
+void VideoEncoderVpx::SetLosslessColor(bool want_lossless) {
+  if (use_vp9_ && (want_lossless != lossless_color_)) {
+    lossless_color_ = want_lossless;
+    codec_.reset(); // Force encoder re-initialization.
+  }
+}
 
 scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
     const webrtc::DesktopFrame& frame) {
@@ -312,19 +328,31 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
   return packet.Pass();
 }
 
-VideoEncoderVpx::VideoEncoderVpx(const CreateCodecCallback& create_codec,
-                                 const CreateImageCallback& create_image)
-    : create_codec_(create_codec),
-      create_image_(create_image),
+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 lossless encoding mode by default.
+    SetLosslessEncode(true);
+
+    // Use I444 colour space, by default, if specified on the command-line.
+    if (CommandLine::ForCurrentProcess()->HasSwitch(kEnableI444SwitchName)) {
+      SetLosslessColor(true);
+    }
+  }
 }
 
 bool VideoEncoderVpx::Initialize(const webrtc::DesktopSize& size) {
+  DCHECK(use_vp9_ || !lossless_color_);
+  DCHECK(use_vp9_ || !lossless_encode_);
+
   codec_.reset();
 
   // (Re)Create the VPX image structure and pixel buffer.
-  create_image_.Run(size, &image_, &image_buffer_);
+  CreateImage(lossless_color_, size, &image_, &image_buffer_);
 
   // Initialize active map.
   active_map_width_ = (image_->w + kMacroBlockSize - 1) / kMacroBlockSize;
@@ -332,7 +360,11 @@ bool VideoEncoderVpx::Initialize(const webrtc::DesktopSize& size) {
   active_map_.reset(new uint8[active_map_width_ * active_map_height_]);
 
   // (Re)Initialize the codec.
-  codec_ = create_codec_.Run(size);
+  if (use_vp9_) {
+    codec_ = CreateVP9Codec(size, lossless_color_, lossless_encode_);
+  } else {
+    codec_ = CreateVP8Codec(size);
+  }
 
   return codec_;
 }

remoting/codec/video_encoder_vpx.h

@@ -23,25 +23,18 @@ class VideoEncoderVpx : public VideoEncoder {
  public:
   // Create encoder for the specified protocol.
   static scoped_ptr<VideoEncoderVpx> CreateForVP8();
-  static scoped_ptr<VideoEncoderVpx> CreateForVP9I420();
-  static scoped_ptr<VideoEncoderVpx> CreateForVP9I444();
+  static scoped_ptr<VideoEncoderVpx> CreateForVP9();
 
   virtual ~VideoEncoderVpx();
 
   // VideoEncoder interface.
+  virtual void SetLosslessEncode(bool want_lossless) OVERRIDE;
+  virtual void SetLosslessColor(bool want_lossless) OVERRIDE;
   virtual scoped_ptr<VideoPacket> Encode(
       const webrtc::DesktopFrame& frame) OVERRIDE;
 
  private:
-  typedef base::Callback<ScopedVpxCodec(const webrtc::DesktopSize&)>
-      CreateCodecCallback;
-  typedef base::Callback<void(const webrtc::DesktopSize&,
-                              scoped_ptr<vpx_image_t>* out_image,
-                              scoped_ptr<uint8[]>* out_image_buffer)>
-      CreateImageCallback;
-
-  VideoEncoderVpx(const CreateCodecCallback& create_codec,
-                  const CreateImageCallback& create_image);
+  explicit VideoEncoderVpx(bool use_vp9);
 
   // Initializes the codec for frames of |size|. Returns true if successful.
   bool Initialize(const webrtc::DesktopSize& size);
@@ -55,8 +48,13 @@ class VideoEncoderVpx : public VideoEncoder {
   // given to the encoder to speed up encoding.
   void PrepareActiveMap(const webrtc::DesktopRegion& updated_region);
 
-  CreateCodecCallback create_codec_;
-  CreateImageCallback create_image_;
+  // True if the encoder should generate VP9, false for VP8.
+  bool use_vp9_;
+
+  // Options controlling VP9 encode quantization and color space.
+  // These are always off (false) for VP8.
+  bool lossless_encode_;
+  bool lossless_color_;
 
   ScopedVpxCodec codec_;
   base::TimeTicks timestamp_base_;

remoting/codec/video_encoder_vpx_unittest.cc

@@ -13,19 +13,104 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 
-namespace {
-
-const int kIntMax = std::numeric_limits<int>::max();
+namespace remoting {
 
-}  // namespace
+// xRGB pixel colors for use by tests.
+const uint32 kBlueColor = 0x0000ff;
+const uint32 kGreenColor = 0x00ff00;
 
-namespace remoting {
+// Creates a frame stippled between blue and red pixels, which is useful for
+// lossy/lossless encode and color tests.
+static scoped_ptr<webrtc::DesktopFrame> CreateTestFrame(
+    const webrtc::DesktopSize& frame_size) {
+  scoped_ptr<webrtc::DesktopFrame> frame(
+      new webrtc::BasicDesktopFrame(frame_size));
+  for (int x = 0; x < frame_size.width(); ++x) {
+    for (int y = 0; y < frame_size.height(); ++y) {
+      uint8* pixel_u8 = frame->data() + (y * frame->stride()) +
+          (x * webrtc::DesktopFrame::kBytesPerPixel);
+      *(reinterpret_cast<uint32*>(pixel_u8)) =
+          ((x + y) & 1) ? kGreenColor : kBlueColor;
+    }
+  }
+  return frame.Pass();
+}
 
-TEST(VideoEncoderVp8Test, TestVideoEncoder) {
+TEST(VideoEncoderVpxTest, TestVp8VideoEncoder) {
   scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP8());
   TestVideoEncoder(encoder.get(), false);
 }
 
+TEST(VideoEncoderVpxTest, TestVp9VideoEncoder) {
+  scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP9());
+  // VP9 encoder defaults to lossless encode and lossy (I420) color.
+  TestVideoEncoder(encoder.get(), false);
+}
+
+// Test that the VP9 encoder can switch between lossy & lossless encode.
+TEST(VideoEncoderVpxTest, TestVp9VideoEncoderLossyEncode) {
+  scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP9());
+
+  webrtc::DesktopSize frame_size(1024, 768);
+  scoped_ptr<webrtc::DesktopFrame> frame(CreateTestFrame(frame_size));
+  frame->mutable_updated_region()->SetRect(
+      webrtc::DesktopRect::MakeSize(frame_size));
+
+  // Lossy encode the first frame.
+  encoder->SetLosslessEncode(false);
+  scoped_ptr<VideoPacket> lossy_packet = encoder->Encode(*frame);
+
+  // Lossless encode the second frame.
+  encoder->SetLosslessEncode(true);
+  scoped_ptr<VideoPacket> lossless_packet = encoder->Encode(*frame);
+  EXPECT_GT(lossless_packet->data().size(), lossy_packet->data().size());
+
+  // Lossy encode one more frame.
+  encoder->SetLosslessEncode(false);
+  lossy_packet = encoder->Encode(*frame);
+  EXPECT_LT(lossy_packet->data().size(), lossless_packet->data().size());
+}
+
+// Test that the VP9 encoder can switch between lossy & lossless color.
+TEST(VideoEncoderVpxTest, TestVp9VideoEncoderLossyColor) {
+  scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP9());
+
+  webrtc::DesktopSize frame_size(1024, 768);
+  scoped_ptr<webrtc::DesktopFrame> frame(CreateTestFrame(frame_size));
+  frame->mutable_updated_region()->SetRect(
+      webrtc::DesktopRect::MakeSize(frame_size));
+
+  // Lossy encode the first frame.
+  encoder->SetLosslessColor(false);
+  scoped_ptr<VideoPacket> lossy_packet = encoder->Encode(*frame);
+
+  // Lossless encode the second frame.
+  encoder->SetLosslessColor(true);
+  scoped_ptr<VideoPacket> lossless_packet = encoder->Encode(*frame);
+  EXPECT_GT(lossless_packet->data().size(), lossy_packet->data().size());
+
+  // Lossy encode one more frame.
+  encoder->SetLosslessColor(false);
+  lossy_packet = encoder->Encode(*frame);
+  EXPECT_LT(lossy_packet->data().size(), lossless_packet->data().size());
+}
+
+// Test that the VP8 encoder ignores lossless modes without crashing.
+TEST(VideoEncoderVpxTest, TestVp8VideoEncoderIgnoreLossy) {
+  scoped_ptr<VideoEncoderVpx> encoder(VideoEncoderVpx::CreateForVP8());
+
+  webrtc::DesktopSize frame_size(1024, 768);
+  scoped_ptr<webrtc::DesktopFrame> frame(CreateTestFrame(frame_size));
+  frame->mutable_updated_region()->SetRect(
+      webrtc::DesktopRect::MakeSize(frame_size));
+
+  // Encode a frame, to give the encoder a chance to crash if misconfigured.
+  encoder->SetLosslessEncode(true);
+  encoder->SetLosslessColor(true);
+  scoped_ptr<VideoPacket> packet = encoder->Encode(*frame);
+  EXPECT_TRUE(packet);
+}
+
 // Test that calling Encode with a differently-sized media::ScreenCaptureData
 // does not leak memory.
 TEST(VideoEncoderVp8Test, TestSizeChangeNoLeak) {

remoting/host/client_session.cc

@@ -133,6 +133,16 @@ void ClientSession::ControlVideo(const protocol::VideoControl& video_control) {
             << video_control.enable() << ")";
     video_scheduler_->Pause(!video_control.enable());
   }
+  if (video_control.has_lossless_encode()) {
+    VLOG(1) << "Received VideoControl (lossless_encode="
+            << video_control.lossless_encode() << ")";
+    video_scheduler_->SetLosslessEncode(video_control.lossless_encode());
+  }
+  if (video_control.has_lossless_color()) {
+    VLOG(1) << "Received VideoControl (lossless_color="
+            << video_control.lossless_color() << ")";
+    video_scheduler_->SetLosslessColor(video_control.lossless_color());
+  }
 }
 
 void ClientSession::ControlAudio(const protocol::AudioControl& audio_control) {
@@ -450,7 +460,7 @@ scoped_ptr<VideoEncoder> ClientSession::CreateVideoEncoder(
   if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) {
     return remoting::VideoEncoderVpx::CreateForVP8().PassAs<VideoEncoder>();
   } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP9) {
-    return remoting::VideoEncoderVpx::CreateForVP9I420().PassAs<VideoEncoder>();
+    return remoting::VideoEncoderVpx::CreateForVP9().PassAs<VideoEncoder>();
   }
 
   NOTREACHED();

remoting/host/video_scheduler.cc

@@ -165,6 +165,30 @@ void VideoScheduler::UpdateSequenceNumber(int64 sequence_number) {
   sequence_number_ = sequence_number;
 }
 
+void VideoScheduler::SetLosslessEncode(bool want_lossless) {
+  if (!encode_task_runner_->BelongsToCurrentThread()) {
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
+    encode_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VideoScheduler::SetLosslessEncode,
+                              this, want_lossless));
+    return;
+  }
+
+  encoder_->SetLosslessEncode(want_lossless);
+}
+
+void VideoScheduler::SetLosslessColor(bool want_lossless) {
+  if (!encode_task_runner_->BelongsToCurrentThread()) {
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
+    encode_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VideoScheduler::SetLosslessColor,
+                              this, want_lossless));
+    return;
+  }
+
+  encoder_->SetLosslessColor(want_lossless);
+}
+
 // Private methods -----------------------------------------------------------
 
 VideoScheduler::~VideoScheduler() {

remoting/host/video_scheduler.h

@@ -112,6 +112,11 @@ class VideoScheduler : public base::RefCountedThreadSafe<VideoScheduler>,
   // Sequence numbers are used for performance measurements.
   void UpdateSequenceNumber(int64 sequence_number);
 
+  // Sets whether the video encoder should be requested to encode losslessly,
+  // or to use a lossless color space (typically requiring higher bandwidth).
+  void SetLosslessEncode(bool want_lossless);
+  void SetLosslessColor(bool want_lossless);
+
  private:
   friend class base::RefCountedThreadSafe<VideoScheduler>;
   virtual ~VideoScheduler();
@@ -197,7 +202,7 @@ class VideoScheduler : public base::RefCountedThreadSafe<VideoScheduler>,
   // True if capture of video frames is paused.
   bool is_paused_;
 
-  // This is a number updated by client to trace performance.
+  // Number updated by the caller to trace performance.
   int64 sequence_number_;
 
   // An object to schedule capturing.

remoting/proto/control.proto

@@ -28,6 +28,10 @@ message ClientResolution {
 message VideoControl {
   // Enables the video channel if true, pauses if false.
   optional bool enable = 1;
+
+  // Controls whether lossless encode and color translation are requested.
+  optional bool lossless_encode = 2;
+  optional bool lossless_color = 3;
 }
 
 message AudioControl {