Cleanups in the video encoding decoding code. Reenable VP8.

1. Moved video-related protobuf messages from event.proto to video.proto. Removed those that we don't need anymore
2. Fixed naming for enums and some types.
3. Reenabled VP8.
4. Proper RGB-YUV converter for VP8 encoder.
5. Changed the capturer_fake to show more meaningful picture.

BUG=57374
TEST=unittests

Review URL: http://codereview.chromium.org/4136010

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

15 files changed, 112 insertions, 65 deletions

diff --git a/remoting/host/capturer.cc b/remoting/host/capturer.cc
index 61148e6..9e63041 100644
--- a/remoting/host/capturer.cc
+++ b/remoting/host/capturer.cc
@@ -13,7 +13,7 @@ namespace remoting {
 Capturer::Capturer()
     : width_(0),
       height_(0),
-      pixel_format_(PixelFormatInvalid),
+      pixel_format_(PIXEL_FORMAT_INVALID),
       bytes_per_row_(0),
       current_buffer_(0) {
 }
diff --git a/remoting/host/capturer_fake.cc b/remoting/host/capturer_fake.cc
index 56b43cf..072f19a 100644
--- a/remoting/host/capturer_fake.cc
+++ b/remoting/host/capturer_fake.cc
@@ -8,13 +8,27 @@
 
 namespace remoting {
 
-static const int kWidth = 320;
-static const int kHeight = 240;
+// CapturerFake generates a white picture of size kWidth x kHeight with a
+// rectangle of size kBoxWidth x kBoxHeight. The rectangle moves kSpeed pixels
+// per frame along both axes, and bounces off the sides of the screen.
+static const int kWidth = 800;
+static const int kHeight = 600;
+static const int kBoxWidth = 140;
+static const int kBoxHeight = 140;
+static const int kSpeed = 20;
+
+COMPILE_ASSERT(kBoxWidth < kWidth && kBoxHeight < kHeight, bad_box_size);
+COMPILE_ASSERT((kBoxWidth % kSpeed == 0) && (kWidth % kSpeed == 0) &&
+               (kBoxHeight % kSpeed == 0) && (kHeight % kSpeed == 0),
+               sizes_must_be_multiple_of_kSpeed);
+
 static const int kBytesPerPixel = 4;  // 32 bit RGB is 4 bytes per pixel.
-static const int kMaxColorChannelValue = 255;
 
 CapturerFake::CapturerFake()
-    : seed_(0) {
+    : box_pos_x_(0),
+      box_pos_y_(0),
+      box_speed_x_(kSpeed),
+      box_speed_y_(kSpeed) {
   ScreenConfigurationChanged();
 }
 
@@ -24,7 +38,7 @@ CapturerFake::~CapturerFake() {
 void CapturerFake::ScreenConfigurationChanged() {
   width_ = kWidth;
   height_ = kHeight;
-  pixel_format_ = PixelFormatRgb32;
+  pixel_format_ = PIXEL_FORMAT_RGB32;
   bytes_per_row_ = width_ * kBytesPerPixel;
 
   // Create memory for the buffers.
@@ -54,16 +68,36 @@ void CapturerFake::CaptureRects(const InvalidRects& rects,
 }
 
 void CapturerFake::GenerateImage() {
-  uint8* row = buffers_[current_buffer_].get();
-  for (int y = 0; y < height_; ++y) {
-    int offset = y % 3;
-    for (int x = 0; x < width_; ++x) {
-      row[x * kBytesPerPixel + offset] = seed_++;
-      seed_ &= kMaxColorChannelValue;
+  memset(buffers_[current_buffer_].get(), 0xff,
+         width_ * height_ * kBytesPerPixel);
+
+  uint8* row = buffers_[current_buffer_].get() +
+      (box_pos_y_ * width_ + box_pos_x_) * kBytesPerPixel;
+
+  box_pos_x_ += box_speed_x_;
+  if (box_pos_x_ + kBoxWidth >= width_ || box_pos_x_ == 0)
+    box_speed_x_ = -box_speed_x_;
+
+  box_pos_y_ += box_speed_y_;
+  if (box_pos_y_ + kBoxHeight >= height_ || box_pos_y_ == 0)
+    box_speed_y_ = -box_speed_y_;
+
+  // Draw rectangle with the following colors in it's corners:
+  //     cyan....yellow
+  //     ..............
+  //     blue.......red
+  for (int y = 0; y < kBoxHeight; ++y) {
+    for (int x = 0; x < kBoxWidth; ++x) {
+      int r = x * 255 / kBoxWidth;
+      int g = y * 255 / kBoxHeight;
+      int b = 255 - (x * 255 / kBoxWidth);
+      row[x * kBytesPerPixel] = r;
+      row[x * kBytesPerPixel+1] = g;
+      row[x * kBytesPerPixel+2] = b;
+      row[x * kBytesPerPixel+3] = 0xff;
     }
     row += bytes_per_row_;
   }
-  ++seed_;
 }
 
 }  // namespace remoting
diff --git a/remoting/host/capturer_fake.h b/remoting/host/capturer_fake.h
index 84cc7ba..46e0266 100644
--- a/remoting/host/capturer_fake.h
+++ b/remoting/host/capturer_fake.h
@@ -10,8 +10,7 @@
 
 namespace remoting {
 
-// A CapturerFake always output an image of 640x480 in 24bit RGB. The image
-// is artificially generated for testing purpose.
+// A CapturerFake generates artificial image for testing purpose.
 //
 // CapturerFake is doubled buffered as required by Capturer. See
 // remoting/host/capturer.h.
@@ -30,8 +29,10 @@ class CapturerFake : public Capturer {
   // Generates an image in the front buffer.
   void GenerateImage();
 
-  // The seed for generating the image.
-  int seed_;
+  int box_pos_x_;
+  int box_pos_y_;
+  int box_speed_x_;
+  int box_speed_y_;
 
   // We have two buffers for the screen images as required by Capturer.
   scoped_array<uint8> buffers_[kNumBuffers];
diff --git a/remoting/host/capturer_fake_ascii.cc b/remoting/host/capturer_fake_ascii.cc
index 1bb9d44..4b259a9 100644
--- a/remoting/host/capturer_fake_ascii.cc
+++ b/remoting/host/capturer_fake_ascii.cc
@@ -21,7 +21,7 @@ CapturerFakeAscii::~CapturerFakeAscii() {
 void CapturerFakeAscii::ScreenConfigurationChanged() {
   width_ = kWidth;
   height_ = kHeight;
-  pixel_format_ = PixelFormatAscii;
+  pixel_format_ = PIXEL_FORMAT_ASCII;
   bytes_per_row_ = width_ * kBytesPerPixel;
 
   // Create memory for the buffers.
diff --git a/remoting/host/capturer_gdi.cc b/remoting/host/capturer_gdi.cc
index 209eea4..7742ff9 100644
--- a/remoting/host/capturer_gdi.cc
+++ b/remoting/host/capturer_gdi.cc
@@ -57,7 +57,7 @@ void CapturerGdi::ScreenConfigurationChanged() {
   int rounded_width = (width_ + 3) & (~3);
 
   // Dimensions of screen.
-  pixel_format_ = PixelFormatRgb32;
+  pixel_format_ = PIXEL_FORMAT_RGB32;
   bytes_per_row_ = rounded_width * kBytesPerPixel;
 
   // Create a differ for this screen size.
diff --git a/remoting/host/capturer_linux.cc b/remoting/host/capturer_linux.cc
index fcf7a01..99013d0 100644
--- a/remoting/host/capturer_linux.cc
+++ b/remoting/host/capturer_linux.cc
@@ -235,7 +235,7 @@ void CapturerLinuxPimpl::CaptureRects(
 
   scoped_refptr<CaptureData> capture_data(
       new CaptureData(planes, capturer_->width(), capturer_->height(),
-                      PixelFormatRgb32));
+                      PIXEL_FORMAT_RGB32));
 
   for (InvalidRects::const_iterator it = rects.begin();
        it != rects.end();
diff --git a/remoting/host/capturer_mac.cc b/remoting/host/capturer_mac.cc
index 6d6b6cc..3eafa34 100644
--- a/remoting/host/capturer_mac.cc
+++ b/remoting/host/capturer_mac.cc
@@ -47,7 +47,7 @@ void CapturerMac::ScreenConfigurationChanged() {
   width_ = CGDisplayPixelsWide(mainDevice);
   height_ = CGDisplayPixelsHigh(mainDevice);
   bytes_per_row_ = width_ * sizeof(uint32_t);
-  pixel_format_ = PixelFormatRgb32;
+  pixel_format_ = PIXEL_FORMAT_RGB32;
   size_t buffer_size = height() * bytes_per_row_;
   for (int i = 0; i < kNumBuffers; ++i) {
     buffers_[i].reset(new uint8[buffer_size]);
diff --git a/remoting/host/client_connection.cc b/remoting/host/client_connection.cc
index 8063d04..e325bf7 100644
--- a/remoting/host/client_connection.cc
+++ b/remoting/host/client_connection.cc
@@ -55,15 +55,20 @@ void ClientConnection::SendInitClientMessage(int width, int height) {
   video_writer_.SendMessage(msg);
 }
 
-void ClientConnection::SendUpdateStreamPacketMessage(
-    const ChromotingHostMessage& message) {
+void ClientConnection::SendVideoPacket(const VideoPacket& packet) {
   DCHECK_EQ(loop_, MessageLoop::current());
 
   // If we are disconnected then return.
   if (!connection_)
     return;
 
-  video_writer_.SendMessage(message);
+  ChromotingHostMessage* message = new ChromotingHostMessage();
+  // TODO(sergeyu): avoid memcopy here.
+  *message->mutable_video_packet() = packet;
+
+  video_writer_.SendMessage(*message);
+
+  delete message;
 }
 
 int ClientConnection::GetPendingUpdateStreamMessages() {
diff --git a/remoting/host/client_connection.h b/remoting/host/client_connection.h
index 69283e2..38e383c 100644
--- a/remoting/host/client_connection.h
+++ b/remoting/host/client_connection.h
@@ -64,8 +64,7 @@ class ClientConnection : public base::RefCountedThreadSafe<ClientConnection> {
   virtual void SendInitClientMessage(int width, int height);
 
   // Send encoded update stream data to the viewer.
-  virtual void SendUpdateStreamPacketMessage(
-      const ChromotingHostMessage& message);
+  virtual void SendVideoPacket(const VideoPacket& packet);
 
   // Gets the number of update stream messages not yet transmitted.
   // Note that the value returned is an estimate using average size of the
diff --git a/remoting/host/client_connection_unittest.cc b/remoting/host/client_connection_unittest.cc
index 240d8f4..b5802cf 100644
--- a/remoting/host/client_connection_unittest.cc
+++ b/remoting/host/client_connection_unittest.cc
@@ -46,8 +46,8 @@ class ClientConnectionTest : public testing::Test {
 
 TEST_F(ClientConnectionTest, SendUpdateStream) {
   // Then send the actual data.
-  ChromotingHostMessage message;
-  viewer_->SendUpdateStreamPacketMessage(message);
+  VideoPacket packet;
+  viewer_->SendVideoPacket(packet);
 
   // And then close the connection to ClientConnection.
   viewer_->Disconnect();
@@ -76,8 +76,8 @@ TEST_F(ClientConnectionTest, Close) {
   message_loop_.RunAllPending();
   EXPECT_TRUE(connection_->is_closed());
 
-  ChromotingHostMessage message;
-  viewer_->SendUpdateStreamPacketMessage(message);
+  VideoPacket packet;
+  viewer_->SendVideoPacket(packet);
   viewer_->Disconnect();
   message_loop_.RunAllPending();
 
diff --git a/remoting/host/mock_objects.h b/remoting/host/mock_objects.h
index 50c94ef..428bd74 100644
--- a/remoting/host/mock_objects.h
+++ b/remoting/host/mock_objects.h
@@ -47,10 +47,7 @@ class MockClientConnection : public ClientConnection {
 
   MOCK_METHOD1(Init, void(ChromotingConnection* connection));
   MOCK_METHOD2(SendInitClientMessage, void(int width, int height));
-  MOCK_METHOD0(SendBeginUpdateStreamMessage, void());
-  MOCK_METHOD1(SendUpdateStreamPacketMessage,
-               void(const ChromotingHostMessage& message));
-  MOCK_METHOD0(SendEndUpdateStreamMessage, void());
+  MOCK_METHOD1(SendVideoPacket, void(const VideoPacket& packet));
   MOCK_METHOD0(GetPendingUpdateStreamMessages, int());
   MOCK_METHOD0(Disconnect, void());
 
diff --git a/remoting/host/session_manager.cc b/remoting/host/session_manager.cc
index a61a6bdd..1867f07 100644
--- a/remoting/host/session_manager.cc
+++ b/remoting/host/session_manager.cc
@@ -328,18 +328,16 @@ void SessionManager::DoRateControl() {
   ScheduleNextRateControl();
 }
 
-void SessionManager::DoSendUpdate(ChromotingHostMessage* message,
-                                  Encoder::EncodingState state) {
+void SessionManager::DoSendVideoPacket(VideoPacket* packet) {
   DCHECK_EQ(network_loop_, MessageLoop::current());
 
   TraceContext::tracer()->PrintString("DoSendUpdate");
 
   for (ClientConnectionList::const_iterator i = clients_.begin();
        i < clients_.end(); ++i) {
-    (*i)->SendUpdateStreamPacketMessage(*message);
+    (*i)->SendVideoPacket(*packet);
   }
-
-  delete message;
+  delete packet;
 
   TraceContext::tracer()->PrintString("DoSendUpdate done");
 }
@@ -399,19 +397,20 @@ void SessionManager::DoEncode(
   TraceContext::tracer()->PrintString("Encode Done");
 }
 
-void SessionManager::EncodeDataAvailableTask(
-    ChromotingHostMessage* message, Encoder::EncodingState state) {
+void SessionManager::EncodeDataAvailableTask(VideoPacket* packet) {
   DCHECK_EQ(encode_loop_, MessageLoop::current());
 
+  bool last = (packet->flags() & VideoPacket::LAST_PACKET) != 0;
+
   // Before a new encode task starts, notify clients a new update
   // stream is coming.
   // Notify this will keep a reference to the DataBuffer in the
   // task. The ownership will eventually pass to the ClientConnections.
   network_loop_->PostTask(
       FROM_HERE,
-      NewTracedMethod(this, &SessionManager::DoSendUpdate, message, state));
+      NewTracedMethod(this, &SessionManager::DoSendVideoPacket, packet));
 
-  if (state & Encoder::EncodingEnded) {
+  if (last) {
     capture_loop_->PostTask(
         FROM_HERE, NewTracedMethod(this, &SessionManager::DoFinishEncode));
   }
diff --git a/remoting/host/session_manager.h b/remoting/host/session_manager.h
index 46e02df..fedfc4f 100644
--- a/remoting/host/session_manager.h
+++ b/remoting/host/session_manager.h
@@ -14,8 +14,7 @@
 #include "base/time.h"
 #include "remoting/base/encoder.h"
 #include "remoting/host/capturer.h"
-// TODO(hclam): This class should not know the internal protobuf types.
-#include "remoting/proto/internal.pb.h"
+#include "remoting/proto/video.pb.h"
 
 namespace remoting {
 
@@ -126,8 +125,7 @@ class SessionManager : public base::RefCountedThreadSafe<SessionManager> {
   void DoRateControl();
 
   // DoSendUpdate takes ownership of header and is responsible for deleting it.
-  void DoSendUpdate(ChromotingHostMessage* message,
-                    Encoder::EncodingState state);
+  void DoSendVideoPacket(VideoPacket* packet);
   void DoSendInit(scoped_refptr<ClientConnection> client,
                   int width, int height);
 
@@ -141,8 +139,7 @@ class SessionManager : public base::RefCountedThreadSafe<SessionManager> {
 
   // EncodeDataAvailableTask takes ownership of header and is responsible for
   // deleting it.
-  void EncodeDataAvailableTask(ChromotingHostMessage* message,
-                               Encoder::EncodingState state);
+  void EncodeDataAvailableTask(VideoPacket* packet);
 
   // Message loops used by this class.
   MessageLoop* capture_loop_;
diff --git a/remoting/host/session_manager_unittest.cc b/remoting/host/session_manager_unittest.cc
index 9aa34d8..f3870e2 100644
--- a/remoting/host/session_manager_unittest.cc
+++ b/remoting/host/session_manager_unittest.cc
@@ -19,8 +19,9 @@ namespace remoting {
 
 static const int kWidth = 640;
 static const int kHeight = 480;
-static const PixelFormat kFormat = PixelFormatRgb32;
-static const UpdateStreamEncoding kEncoding = EncodingNone;
+static const PixelFormat kFormat = PIXEL_FORMAT_RGB32;
+static const VideoPacketFormat::Encoding kEncoding =
+    VideoPacketFormat::ENCODING_VERBATIM;
 
 class SessionManagerTest : public testing::Test {
  public:
@@ -64,10 +65,7 @@ ACTION_P2(RunCallback, rects, data) {
 }
 
 ACTION_P(FinishEncode, msg) {
-  Encoder::EncodingState state = (Encoder::EncodingStarting |
-                                  Encoder::EncodingInProgress |
-                                  Encoder::EncodingEnded);
-  arg2->Run(msg, state);
+  arg2->Run(msg);
   delete arg2;
 }
 
@@ -98,14 +96,12 @@ TEST_F(SessionManagerTest, DISABLED_OneRecordCycle) {
       .WillOnce(RunCallback(update_rects, data));
 
   // Expect the encoder be called.
-  ChromotingHostMessage* msg = new ChromotingHostMessage();
+  VideoPacket* packet = new VideoPacket();
   EXPECT_CALL(*encoder_, Encode(data, false, NotNull()))
-      .WillOnce(FinishEncode(msg));
+      .WillOnce(FinishEncode(packet));
 
   // Expect the client be notified.
-  EXPECT_CALL(*client_, SendBeginUpdateStreamMessage());
-  EXPECT_CALL(*client_, SendUpdateStreamPacketMessage(_));
-  EXPECT_CALL(*client_, SendEndUpdateStreamMessage());
+  EXPECT_CALL(*client_, SendVideoPacket(_));
   EXPECT_CALL(*client_, GetPendingUpdateStreamMessages())
       .Times(AtLeast(0))
       .WillRepeatedly(Return(0));
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index d998ef9..2733850 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -25,14 +25,18 @@
 #include "base/logging.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/nss_util.h"
+#include "base/path_service.h"
 #include "base/thread.h"
+#include "media/base/media.h"
 #include "remoting/base/encoder_verbatim.h"
+#include "remoting/base/encoder_vp8.h"
 #include "remoting/base/encoder_zlib.h"
+#include "remoting/base/tracer.h"
 #include "remoting/host/capturer_fake.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/json_host_config.h"
-#include "remoting/base/tracer.h"
+#include "remoting/proto/video.pb.h"
 
 #if defined(OS_WIN)
 #include "remoting/host/capturer_gdi.h"
@@ -62,6 +66,7 @@ void ShutdownTask(MessageLoop* message_loop) {
 const std::string kFakeSwitchName = "fake";
 const std::string kConfigSwitchName = "config";
 const std::string kVerbatimSwitchName = "verbatim";
+const std::string kVp8SwitchName = "vp8";
 
 int main(int argc, char** argv) {
   // Needed for the Mac, so we don't leak objects when threads are created.
@@ -92,14 +97,15 @@ int main(int argc, char** argv) {
   // Check the argument to see if we should use a fake capturer and encoder.
   bool fake = cmd_line->HasSwitch(kFakeSwitchName);
   bool verbatim = cmd_line->HasSwitch(kVerbatimSwitchName);
+  bool vp8 = cmd_line->HasSwitch(kVp8SwitchName);
 
 #if defined(OS_WIN)
-  std::wstring path = GetEnvironmentVar(kHomeDrive);
-  path += GetEnvironmentVar(kHomePath);
+  std::wstring home_path = GetEnvironmentVar(kHomeDrive);
+  home_path += GetEnvironmentVar(kHomePath);
 #else
-  std::string path = GetEnvironmentVar(base::env_vars::kHome);
+  std::string home_path = GetEnvironmentVar(base::env_vars::kHome);
 #endif
-  FilePath config_path(path);
+  FilePath config_path(home_path);
   config_path = config_path.Append(kDefaultConfigPath);
   if (cmd_line->HasSwitch(kConfigSwitchName)) {
     config_path = cmd_line->GetSwitchValuePath(kConfigSwitchName);
@@ -116,6 +122,14 @@ int main(int argc, char** argv) {
     encoder.reset(new remoting::EncoderVerbatim());
   }
 
+  // TODO(sergeyu): Enable VP8 on ARM builds.
+#if !defined(ARCH_CPU_ARM_FAMILY)
+  if (vp8) {
+    LOG(INFO) << "Using the verbatim encoder.";
+    encoder.reset(new remoting::EncoderVp8());
+  }
+#endif
+
   base::Thread file_io_thread("FileIO");
   file_io_thread.Start();
 
@@ -132,6 +146,11 @@ int main(int argc, char** argv) {
   remoting::ChromotingHostContext context;
   context.Start();
 
+  FilePath module_path;
+  PathService::Get(base::DIR_MODULE, &module_path);
+  CHECK(media::InitializeMediaLibrary(module_path))
+      << "Cannot load media library";
+
   // Construct a chromoting host.
   scoped_refptr<remoting::ChromotingHost> host(
       new remoting::ChromotingHost(&context,