Add VideoPacket struct for video packets. Refactor Decode interface to use it.

Various cleanups.
BUG=None
TEST=Unittests.

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

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

14 files changed, 62 insertions, 53 deletions

diff --git a/remoting/protocol/connection_to_host.h b/remoting/protocol/connection_to_host.h
index 39aeecd..e2b0ce2 100644
--- a/remoting/protocol/connection_to_host.h
+++ b/remoting/protocol/connection_to_host.h
@@ -15,6 +15,7 @@
 namespace remoting {
 namespace protocol {
 
+class SessionConfig;
 class VideoStub;
 
 class ConnectionToHost {
@@ -23,11 +24,6 @@ class ConnectionToHost {
    public:
     virtual ~HostEventCallback() {}
 
-    // Handles an event received by the protocol::ConnectionToHost. Ownership
-    // of the message is passed to the callee.
-    virtual void HandleMessage(ConnectionToHost* conn,
-                               ChromotingHostMessage* message) = 0;
-
     // Called when the network connection is opened.
     virtual void OnConnectionOpened(ConnectionToHost* conn) = 0;
 
@@ -48,6 +44,8 @@ class ConnectionToHost {
                        VideoStub* video_stub) = 0;
   virtual void Disconnect() = 0;
 
+  virtual const SessionConfig* config() = 0;
+
   // Send an input event to the host.
   virtual void SendEvent(const ChromotingClientMessage& msg) = 0;
 
diff --git a/remoting/protocol/jingle_connection_to_host.cc b/remoting/protocol/jingle_connection_to_host.cc
index e31d351..065ae55 100644
--- a/remoting/protocol/jingle_connection_to_host.cc
+++ b/remoting/protocol/jingle_connection_to_host.cc
@@ -64,8 +64,10 @@ void JingleConnectionToHost::Disconnect() {
   }
 }
 
-void JingleConnectionToHost::OnControlMessage(ChromotingHostMessage* msg) {
-  event_callback_->HandleMessage(this, msg);
+void JingleConnectionToHost::OnControlMessage(ControlMessage* msg) {
+  // TODO(sergeyu): Remove this method and pass ClientStub to the control
+  // stream dispatcher.
+  delete msg;
 }
 
 void JingleConnectionToHost::InitSession() {
@@ -112,6 +114,10 @@ void JingleConnectionToHost::OnServerClosed() {
   }
 }
 
+const SessionConfig* JingleConnectionToHost::config() {
+  return session_->config();
+}
+
 void JingleConnectionToHost::SendEvent(const ChromotingClientMessage& msg) {
   // This drops the message if we are not connected yet.
   event_writer_.SendMessage(msg);
@@ -156,7 +162,7 @@ void JingleConnectionToHost::OnSessionStateChange(
 
     case protocol::Session::CONNECTED:
       // Initialize reader and writer.
-      control_reader_.Init<ChromotingHostMessage>(
+      control_reader_.Init<ControlMessage>(
           session_->control_channel(),
           NewCallback(this, &JingleConnectionToHost::OnControlMessage));
       event_writer_.Init(session_->event_channel());
diff --git a/remoting/protocol/jingle_connection_to_host.h b/remoting/protocol/jingle_connection_to_host.h
index fe56e95..c25d872 100644
--- a/remoting/protocol/jingle_connection_to_host.h
+++ b/remoting/protocol/jingle_connection_to_host.h
@@ -34,6 +34,7 @@ class MessageLoop;
 namespace remoting {
 
 class JingleThread;
+class VideoPacket;
 
 namespace protocol {
 
@@ -54,6 +55,8 @@ class JingleConnectionToHost : public ConnectionToHost,
                        VideoStub* video_stub);
   virtual void Disconnect();
 
+  virtual const SessionConfig* config();
+
   virtual void SendEvent(const ChromotingClientMessage& msg);
 
   // JingleClient::Callback interface.
@@ -76,7 +79,7 @@ class JingleConnectionToHost : public ConnectionToHost,
   void InitSession();
 
   // Callback for |control_reader_|.
-  void OnControlMessage(ChromotingHostMessage* msg);
+  void OnControlMessage(ControlMessage* msg);
 
   // Callback for |video_reader_|.
   void OnVideoPacket(VideoPacket* packet);
diff --git a/remoting/protocol/message_decoder_unittest.cc b/remoting/protocol/message_decoder_unittest.cc
index 90c6680..c893d37 100644
--- a/remoting/protocol/message_decoder_unittest.cc
+++ b/remoting/protocol/message_decoder_unittest.cc
@@ -15,11 +15,9 @@
 
 namespace remoting {
 
-static const int kWidth = 640;
-static const int kHeight = 480;
-static const std::string kTestData = "Chromoting rockz";
+static const int kTestKey = 142;
 
-static void AppendMessage(const ChromotingHostMessage& msg,
+static void AppendMessage(const ChromotingClientMessage& msg,
                           std::string* buffer) {
   // Contains one encoded message.
   scoped_refptr<net::IOBufferWithSize> encoded_msg;
@@ -32,17 +30,12 @@ static void PrepareData(uint8** buffer, int* size) {
   // Contains all encoded messages.
   std::string encoded_data;
 
-  // The first message is InitClient.
-  ChromotingHostMessage msg;
-  msg.mutable_init_client()->set_width(kWidth);
-  msg.mutable_init_client()->set_height(kHeight);
-  AppendMessage(msg, &encoded_data);
-  msg.Clear();
+  ChromotingClientMessage msg;
 
   // Then append 10 update sequences to the data.
   for (int i = 0; i < 10; ++i) {
-    msg.mutable_video_packet()->set_sequence_number(0);
-    msg.mutable_video_packet()->set_data(kTestData);
+    msg.mutable_key_event()->set_key(kTestKey + i);
+    msg.mutable_key_event()->set_pressed((i % 2) != 0);
     AppendMessage(msg, &encoded_data);
     msg.Clear();
   }
@@ -67,7 +60,7 @@ void SimulateReadSequence(const int read_sequence[], int sequence_size) {
 
   // Then feed the protocol decoder using the above generated data and the
   // read pattern.
-  std::list<ChromotingHostMessage*> message_list;
+  std::list<ChromotingClientMessage*> message_list;
   for (int i = 0; i < size;) {
     // First generate the amount to feed the decoder.
     int read = std::min(size - i, read_sequence[i % sequence_size]);
@@ -80,19 +73,21 @@ void SimulateReadSequence(const int read_sequence[], int sequence_size) {
   }
 
   // Then verify the decoded messages.
-  EXPECT_EQ(11u, message_list.size());
-  EXPECT_TRUE(message_list.front()->has_init_client());
-  delete message_list.front();
-  message_list.pop_front();
+  EXPECT_EQ(10u, message_list.size());
 
-  for (std::list<ChromotingHostMessage*>::iterator it =
+  int index = 0;
+  for (std::list<ChromotingClientMessage*>::iterator it =
            message_list.begin();
        it != message_list.end(); ++it) {
-    ChromotingHostMessage* message = *it;
+    ChromotingClientMessage* message = *it;
     // Partial update stream.
-    EXPECT_TRUE(message->has_video_packet());
-    EXPECT_EQ(kTestData,
-              message->video_packet().data().data());
+    EXPECT_TRUE(message->has_key_event());
+
+    // TODO(sergeyu): Don't use index here. Instead store the expected values
+    // in an array.
+    EXPECT_EQ(kTestKey + index, message->key_event().key());
+    EXPECT_EQ((index % 2) != 0, message->key_event().pressed());
+    ++index;
   }
   STLDeleteElements(&message_list);
 }
diff --git a/remoting/protocol/protobuf_video_reader.cc b/remoting/protocol/protobuf_video_reader.cc
index f777363..31f4aae 100644
--- a/remoting/protocol/protobuf_video_reader.cc
+++ b/remoting/protocol/protobuf_video_reader.cc
@@ -5,18 +5,23 @@
 #include "remoting/protocol/protobuf_video_reader.h"
 
 #include "base/task.h"
+#include "remoting/proto/video.pb.h"
 #include "remoting/protocol/session.h"
 
 namespace remoting {
 namespace protocol {
 
-ProtobufVideoReader::ProtobufVideoReader() { }
+ProtobufVideoReader::ProtobufVideoReader(VideoPacketFormat::Encoding encoding)
+    : encoding_(encoding) {
+}
+
 ProtobufVideoReader::~ProtobufVideoReader() { }
 
 void ProtobufVideoReader::Init(protocol::Session* session,
                                VideoStub* video_stub) {
-  reader_.Init<VideoPacket>(session->video_channel(),
-                            NewCallback(this, &ProtobufVideoReader::OnNewData));
+  reader_.Init<VideoPacket>(
+      session->video_channel(),
+      NewCallback(this, &ProtobufVideoReader::OnNewData));
   video_stub_ = video_stub;
 }
 
diff --git a/remoting/protocol/protobuf_video_reader.h b/remoting/protocol/protobuf_video_reader.h
index 669cd0b..42e1c97 100644
--- a/remoting/protocol/protobuf_video_reader.h
+++ b/remoting/protocol/protobuf_video_reader.h
@@ -5,6 +5,7 @@
 #ifndef REMOTING_PROTOCOL_PROTOBUF_VIDEO_READER_H_
 #define REMOTING_PROTOCOL_PROTOBUF_VIDEO_READER_H_
 
+#include "remoting/proto/video.pb.h"
 #include "remoting/protocol/message_reader.h"
 #include "remoting/protocol/video_reader.h"
 
@@ -15,7 +16,7 @@ class Session;
 
 class ProtobufVideoReader : public VideoReader {
  public:
-  ProtobufVideoReader();
+  ProtobufVideoReader(VideoPacketFormat::Encoding encoding);
   virtual ~ProtobufVideoReader();
 
   // VideoReader interface.
@@ -25,6 +26,8 @@ class ProtobufVideoReader : public VideoReader {
  private:
   void OnNewData(VideoPacket* packet);
 
+  VideoPacketFormat::Encoding encoding_;
+
   MessageReader reader_;
 
   // The stub that processes all received packets.
diff --git a/remoting/protocol/protobuf_video_writer.cc b/remoting/protocol/protobuf_video_writer.cc
index 9bbd88b..fdd3b9e 100644
--- a/remoting/protocol/protobuf_video_writer.cc
+++ b/remoting/protocol/protobuf_video_writer.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/protocol/protobuf_video_writer.h"
 
+#include "remoting/proto/video.pb.h"
 #include "remoting/protocol/rtp_writer.h"
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/util.h"
diff --git a/remoting/protocol/rtp_video_reader.cc b/remoting/protocol/rtp_video_reader.cc
index 8660371..dbc2555 100644
--- a/remoting/protocol/rtp_video_reader.cc
+++ b/remoting/protocol/rtp_video_reader.cc
@@ -5,6 +5,7 @@
 #include "remoting/protocol/rtp_video_reader.h"
 
 #include "base/task.h"
+#include "remoting/proto/video.pb.h"
 #include "remoting/protocol/session.h"
 
 namespace remoting {
@@ -13,8 +14,7 @@ namespace protocol {
 RtpVideoReader::RtpVideoReader() { }
 RtpVideoReader::~RtpVideoReader() { }
 
-void RtpVideoReader::Init(protocol::Session* session,
-                          VideoStub* video_stub) {
+void RtpVideoReader::Init(protocol::Session* session, VideoStub* video_stub) {
   rtp_reader_.Init(session->video_rtp_channel(),
                    NewCallback(this, &RtpVideoReader::OnRtpPacket));
   video_stub_ = video_stub;
@@ -26,15 +26,10 @@ void RtpVideoReader::Close() {
 
 void RtpVideoReader::OnRtpPacket(const RtpPacket& rtp_packet) {
   VideoPacket* packet = new VideoPacket();
-  packet->set_data(rtp_packet.payload, rtp_packet.payload_size);
 
+  packet->set_data(rtp_packet.payload, rtp_packet.payload_size);
   packet->mutable_format()->set_encoding(VideoPacketFormat::ENCODING_VP8);
   packet->set_flags(rtp_packet.header.marker ? VideoPacket::LAST_PACKET : 0);
-  packet->mutable_format()->set_pixel_format(PIXEL_FORMAT_RGB32);
-  packet->mutable_format()->set_x(0);
-  packet->mutable_format()->set_y(0);
-  packet->mutable_format()->set_width(800);
-  packet->mutable_format()->set_height(600);
 
   video_stub_->ProcessVideoPacket(packet, new DeleteTask<VideoPacket>(packet));
 }
diff --git a/remoting/protocol/rtp_video_writer.cc b/remoting/protocol/rtp_video_writer.cc
index 5bd1c80..1a7ead4 100644
--- a/remoting/protocol/rtp_video_writer.cc
+++ b/remoting/protocol/rtp_video_writer.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/protocol/rtp_video_writer.h"
 
+#include "remoting/proto/video.pb.h"
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/rtp_writer.h"
 
@@ -15,12 +16,11 @@ RtpVideoWriter::RtpVideoWriter() { }
 RtpVideoWriter::~RtpVideoWriter() { }
 
 void RtpVideoWriter::Init(protocol::Session* session) {
-  rtp_writer_.Init(session->video_rtp_channel(),
-                   session->video_rtcp_channel());
+  rtp_writer_.Init(session->video_rtp_channel(), session->video_rtcp_channel());
 }
 
 void RtpVideoWriter::SendPacket(const VideoPacket& packet) {
-  rtp_writer_.SendPacket(packet.data().data(), packet.data().length(),
+  rtp_writer_.SendPacket(packet.data().data(), packet.data().size(),
                          packet.timestamp());
 }
 
diff --git a/remoting/protocol/stream_writer.cc b/remoting/protocol/stream_writer.cc
index 33a4798..9e1ddd4 100644
--- a/remoting/protocol/stream_writer.cc
+++ b/remoting/protocol/stream_writer.cc
@@ -39,7 +39,7 @@ bool EventStreamWriter::SendMessage(const ChromotingClientMessage& message) {
   return buffered_writer_->Write(SerializeAndFrameMessage(message));
 }
 
-bool ControlStreamWriter::SendMessage(const ChromotingHostMessage& message) {
+bool ControlStreamWriter::SendMessage(const ControlMessage& message) {
   return buffered_writer_->Write(SerializeAndFrameMessage(message));
 }
 
diff --git a/remoting/protocol/stream_writer.h b/remoting/protocol/stream_writer.h
index 7c5a3bd..15b9bdd 100644
--- a/remoting/protocol/stream_writer.h
+++ b/remoting/protocol/stream_writer.h
@@ -50,7 +50,7 @@ class ControlStreamWriter : public StreamWriterBase {
  public:
   // Sends the |message| or returns false if called before Init().
   // Can be called on any thread.
-  bool SendMessage(const ChromotingHostMessage& message);
+  bool SendMessage(const ControlMessage& message);
 };
 
 }  // namespace protocol
diff --git a/remoting/protocol/video_reader.cc b/remoting/protocol/video_reader.cc
index 832eb04..2c1585a 100644
--- a/remoting/protocol/video_reader.cc
+++ b/remoting/protocol/video_reader.cc
@@ -19,7 +19,10 @@ VideoReader* VideoReader::Create(const SessionConfig* config) {
   if (video_config.transport == ChannelConfig::TRANSPORT_SRTP) {
     return new RtpVideoReader();
   } else if (video_config.transport == ChannelConfig::TRANSPORT_STREAM) {
-    return new ProtobufVideoReader();
+    if (video_config.codec == ChannelConfig::CODEC_ZIP)
+      return new ProtobufVideoReader(VideoPacketFormat::ENCODING_ZLIB);
+    else if (video_config.codec == ChannelConfig::CODEC_VERBATIM)
+      return new ProtobufVideoReader(VideoPacketFormat::ENCODING_VERBATIM);
   }
   return NULL;
 }
diff --git a/remoting/protocol/video_stub.h b/remoting/protocol/video_stub.h
index 5c59ca0..c1b6b11 100644
--- a/remoting/protocol/video_stub.h
+++ b/remoting/protocol/video_stub.h
@@ -5,11 +5,12 @@
 #ifndef REMOTING_PROTOCOL_VIDEO_STUB_H_
 #define REMOTING_PROTOCOL_VIDEO_STUB_H_
 
-#include "remoting/proto/video.pb.h"
-
 class Task;
 
 namespace remoting {
+
+class VideoPacket;
+
 namespace protocol {
 
 class VideoStub {
diff --git a/remoting/protocol/video_writer.h b/remoting/protocol/video_writer.h
index eae6b6e..eafa2b4 100644
--- a/remoting/protocol/video_writer.h
+++ b/remoting/protocol/video_writer.h
@@ -11,11 +11,10 @@
 #define REMOTING_PROTOCOL_VIDEO_WRITER_H_
 
 #include "base/basictypes.h"
-#include "remoting/proto/video.pb.h"
 
 namespace remoting {
 
-class ChromotocolConnection;
+class VideoPacket;
 
 namespace protocol {