Revert 64672 - 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

TBR=sergeyu@chromium.org
Review URL: http://codereview.chromium.org/4255001

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

15 files changed, 65 insertions, 112 deletions

diff --git a/remoting/host/capturer.cc b/remoting/host/capturer.cc
index 9e63041..61148e6 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_(PIXEL_FORMAT_INVALID),
+      pixel_format_(PixelFormatInvalid),
       bytes_per_row_(0),
       current_buffer_(0) {
 }
diff --git a/remoting/host/capturer_fake.cc b/remoting/host/capturer_fake.cc
index 072f19a..56b43cf 100644
--- a/remoting/host/capturer_fake.cc
+++ b/remoting/host/capturer_fake.cc
@@ -8,27 +8,13 @@
 
 namespace remoting {
 
-// 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 kWidth = 320;
+static const int kHeight = 240;
 static const int kBytesPerPixel = 4;  // 32 bit RGB is 4 bytes per pixel.
+static const int kMaxColorChannelValue = 255;
 
 CapturerFake::CapturerFake()
-    : box_pos_x_(0),
-      box_pos_y_(0),
-      box_speed_x_(kSpeed),
-      box_speed_y_(kSpeed) {
+    : seed_(0) {
   ScreenConfigurationChanged();
 }
 
@@ -38,7 +24,7 @@ CapturerFake::~CapturerFake() {
 void CapturerFake::ScreenConfigurationChanged() {
   width_ = kWidth;
   height_ = kHeight;
-  pixel_format_ = PIXEL_FORMAT_RGB32;
+  pixel_format_ = PixelFormatRgb32;
   bytes_per_row_ = width_ * kBytesPerPixel;
 
   // Create memory for the buffers.
@@ -68,36 +54,16 @@ void CapturerFake::CaptureRects(const InvalidRects& rects,
 }
 
 void CapturerFake::GenerateImage() {
-  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;
+  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;
     }
     row += bytes_per_row_;
   }
+  ++seed_;
 }
 
 }  // namespace remoting
diff --git a/remoting/host/capturer_fake.h b/remoting/host/capturer_fake.h
index 46e0266..84cc7ba 100644
--- a/remoting/host/capturer_fake.h
+++ b/remoting/host/capturer_fake.h
@@ -10,7 +10,8 @@
 
 namespace remoting {
 
-// A CapturerFake generates artificial image for testing purpose.
+// A CapturerFake always output an image of 640x480 in 24bit RGB. The image
+// is artificially generated for testing purpose.
 //
 // CapturerFake is doubled buffered as required by Capturer. See
 // remoting/host/capturer.h.
@@ -29,10 +30,8 @@ class CapturerFake : public Capturer {
   // Generates an image in the front buffer.
   void GenerateImage();
 
-  int box_pos_x_;
-  int box_pos_y_;
-  int box_speed_x_;
-  int box_speed_y_;
+  // The seed for generating the image.
+  int seed_;
 
   // 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 4b259a9..1bb9d44 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_ = PIXEL_FORMAT_ASCII;
+  pixel_format_ = PixelFormatAscii;
   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 7742ff9..209eea4 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_ = PIXEL_FORMAT_RGB32;
+  pixel_format_ = PixelFormatRgb32;
   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 99013d0..fcf7a01 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(),
-                      PIXEL_FORMAT_RGB32));
+                      PixelFormatRgb32));
 
   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 3eafa34..6d6b6cc 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_ = PIXEL_FORMAT_RGB32;
+  pixel_format_ = PixelFormatRgb32;
   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 e325bf7..8063d04 100644
--- a/remoting/host/client_connection.cc
+++ b/remoting/host/client_connection.cc
@@ -55,20 +55,15 @@ void ClientConnection::SendInitClientMessage(int width, int height) {
   video_writer_.SendMessage(msg);
 }
 
-void ClientConnection::SendVideoPacket(const VideoPacket& packet) {
+void ClientConnection::SendUpdateStreamPacketMessage(
+    const ChromotingHostMessage& message) {
   DCHECK_EQ(loop_, MessageLoop::current());
 
   // If we are disconnected then return.
   if (!connection_)
     return;
 
-  ChromotingHostMessage* message = new ChromotingHostMessage();
-  // TODO(sergeyu): avoid memcopy here.
-  *message->mutable_video_packet() = packet;
-
-  video_writer_.SendMessage(*message);
-
-  delete message;
+  video_writer_.SendMessage(message);
 }
 
 int ClientConnection::GetPendingUpdateStreamMessages() {
diff --git a/remoting/host/client_connection.h b/remoting/host/client_connection.h
index 38e383c..69283e2 100644
--- a/remoting/host/client_connection.h
+++ b/remoting/host/client_connection.h
@@ -64,7 +64,8 @@ class ClientConnection : public base::RefCountedThreadSafe<ClientConnection> {
   virtual void SendInitClientMessage(int width, int height);
 
   // Send encoded update stream data to the viewer.
-  virtual void SendVideoPacket(const VideoPacket& packet);
+  virtual void SendUpdateStreamPacketMessage(
+      const ChromotingHostMessage& message);
 
   // 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 b5802cf..240d8f4 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.
-  VideoPacket packet;
-  viewer_->SendVideoPacket(packet);
+  ChromotingHostMessage message;
+  viewer_->SendUpdateStreamPacketMessage(message);
 
   // 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());
 
-  VideoPacket packet;
-  viewer_->SendVideoPacket(packet);
+  ChromotingHostMessage message;
+  viewer_->SendUpdateStreamPacketMessage(message);
   viewer_->Disconnect();
   message_loop_.RunAllPending();
 
diff --git a/remoting/host/mock_objects.h b/remoting/host/mock_objects.h
index 428bd74..50c94ef 100644
--- a/remoting/host/mock_objects.h
+++ b/remoting/host/mock_objects.h
@@ -47,7 +47,10 @@ class MockClientConnection : public ClientConnection {
 
   MOCK_METHOD1(Init, void(ChromotingConnection* connection));
   MOCK_METHOD2(SendInitClientMessage, void(int width, int height));
-  MOCK_METHOD1(SendVideoPacket, void(const VideoPacket& packet));
+  MOCK_METHOD0(SendBeginUpdateStreamMessage, void());
+  MOCK_METHOD1(SendUpdateStreamPacketMessage,
+               void(const ChromotingHostMessage& message));
+  MOCK_METHOD0(SendEndUpdateStreamMessage, void());
   MOCK_METHOD0(GetPendingUpdateStreamMessages, int());
   MOCK_METHOD0(Disconnect, void());
 
diff --git a/remoting/host/session_manager.cc b/remoting/host/session_manager.cc
index 1867f07..a61a6bdd 100644
--- a/remoting/host/session_manager.cc
+++ b/remoting/host/session_manager.cc
@@ -328,16 +328,18 @@ void SessionManager::DoRateControl() {
   ScheduleNextRateControl();
 }
 
-void SessionManager::DoSendVideoPacket(VideoPacket* packet) {
+void SessionManager::DoSendUpdate(ChromotingHostMessage* message,
+                                  Encoder::EncodingState state) {
   DCHECK_EQ(network_loop_, MessageLoop::current());
 
   TraceContext::tracer()->PrintString("DoSendUpdate");
 
   for (ClientConnectionList::const_iterator i = clients_.begin();
        i < clients_.end(); ++i) {
-    (*i)->SendVideoPacket(*packet);
+    (*i)->SendUpdateStreamPacketMessage(*message);
   }
-  delete packet;
+
+  delete message;
 
   TraceContext::tracer()->PrintString("DoSendUpdate done");
 }
@@ -397,20 +399,19 @@ void SessionManager::DoEncode(
   TraceContext::tracer()->PrintString("Encode Done");
 }
 
-void SessionManager::EncodeDataAvailableTask(VideoPacket* packet) {
+void SessionManager::EncodeDataAvailableTask(
+    ChromotingHostMessage* message, Encoder::EncodingState state) {
   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::DoSendVideoPacket, packet));
+      NewTracedMethod(this, &SessionManager::DoSendUpdate, message, state));
 
-  if (last) {
+  if (state & Encoder::EncodingEnded) {
     capture_loop_->PostTask(
         FROM_HERE, NewTracedMethod(this, &SessionManager::DoFinishEncode));
   }
diff --git a/remoting/host/session_manager.h b/remoting/host/session_manager.h
index fedfc4f..46e02df 100644
--- a/remoting/host/session_manager.h
+++ b/remoting/host/session_manager.h
@@ -14,7 +14,8 @@
 #include "base/time.h"
 #include "remoting/base/encoder.h"
 #include "remoting/host/capturer.h"
-#include "remoting/proto/video.pb.h"
+// TODO(hclam): This class should not know the internal protobuf types.
+#include "remoting/proto/internal.pb.h"
 
 namespace remoting {
 
@@ -125,7 +126,8 @@ class SessionManager : public base::RefCountedThreadSafe<SessionManager> {
   void DoRateControl();
 
   // DoSendUpdate takes ownership of header and is responsible for deleting it.
-  void DoSendVideoPacket(VideoPacket* packet);
+  void DoSendUpdate(ChromotingHostMessage* message,
+                    Encoder::EncodingState state);
   void DoSendInit(scoped_refptr<ClientConnection> client,
                   int width, int height);
 
@@ -139,7 +141,8 @@ class SessionManager : public base::RefCountedThreadSafe<SessionManager> {
 
   // EncodeDataAvailableTask takes ownership of header and is responsible for
   // deleting it.
-  void EncodeDataAvailableTask(VideoPacket* packet);
+  void EncodeDataAvailableTask(ChromotingHostMessage* message,
+                               Encoder::EncodingState state);
 
   // 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 f3870e2..9aa34d8 100644
--- a/remoting/host/session_manager_unittest.cc
+++ b/remoting/host/session_manager_unittest.cc
@@ -19,9 +19,8 @@ namespace remoting {
 
 static const int kWidth = 640;
 static const int kHeight = 480;
-static const PixelFormat kFormat = PIXEL_FORMAT_RGB32;
-static const VideoPacketFormat::Encoding kEncoding =
-    VideoPacketFormat::ENCODING_VERBATIM;
+static const PixelFormat kFormat = PixelFormatRgb32;
+static const UpdateStreamEncoding kEncoding = EncodingNone;
 
 class SessionManagerTest : public testing::Test {
  public:
@@ -65,7 +64,10 @@ ACTION_P2(RunCallback, rects, data) {
 }
 
 ACTION_P(FinishEncode, msg) {
-  arg2->Run(msg);
+  Encoder::EncodingState state = (Encoder::EncodingStarting |
+                                  Encoder::EncodingInProgress |
+                                  Encoder::EncodingEnded);
+  arg2->Run(msg, state);
   delete arg2;
 }
 
@@ -96,12 +98,14 @@ TEST_F(SessionManagerTest, DISABLED_OneRecordCycle) {
       .WillOnce(RunCallback(update_rects, data));
 
   // Expect the encoder be called.
-  VideoPacket* packet = new VideoPacket();
+  ChromotingHostMessage* msg = new ChromotingHostMessage();
   EXPECT_CALL(*encoder_, Encode(data, false, NotNull()))
-      .WillOnce(FinishEncode(packet));
+      .WillOnce(FinishEncode(msg));
 
   // Expect the client be notified.
-  EXPECT_CALL(*client_, SendVideoPacket(_));
+  EXPECT_CALL(*client_, SendBeginUpdateStreamMessage());
+  EXPECT_CALL(*client_, SendUpdateStreamPacketMessage(_));
+  EXPECT_CALL(*client_, SendEndUpdateStreamMessage());
   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 2733850..d998ef9 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -25,18 +25,14 @@
 #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/proto/video.pb.h"
+#include "remoting/base/tracer.h"
 
 #if defined(OS_WIN)
 #include "remoting/host/capturer_gdi.h"
@@ -66,7 +62,6 @@ 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.
@@ -97,15 +92,14 @@ 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 home_path = GetEnvironmentVar(kHomeDrive);
-  home_path += GetEnvironmentVar(kHomePath);
+  std::wstring path = GetEnvironmentVar(kHomeDrive);
+  path += GetEnvironmentVar(kHomePath);
 #else
-  std::string home_path = GetEnvironmentVar(base::env_vars::kHome);
+  std::string path = GetEnvironmentVar(base::env_vars::kHome);
 #endif
-  FilePath config_path(home_path);
+  FilePath config_path(path);
   config_path = config_path.Append(kDefaultConfigPath);
   if (cmd_line->HasSwitch(kConfigSwitchName)) {
     config_path = cmd_line->GetSwitchValuePath(kConfigSwitchName);
@@ -122,14 +116,6 @@ 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();
 
@@ -146,11 +132,6 @@ 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,