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

remoting/base/decoder_row_based.cc

Index: remoting/base/decoder_row_based.cc
diff --git a/remoting/base/decoder_row_based.cc b/remoting/base/decoder_row_based.cc
index d378e96d6fdf2fa0f1da7d186be348a8d1eceacd..ed68cc5e7bff5f56eb2dd719fe3c83306df8ed0d 100644
--- a/remoting/base/decoder_row_based.cc
+++ b/remoting/base/decoder_row_based.cc
@@ -11,6 +11,11 @@
 
 namespace remoting {
 
+namespace {
+// Both input and output data are assumed to be RGBA32.
+const int kBytesPerPixel = 4;
+}
+
 DecoderRowBased* DecoderRowBased::CreateZlibDecoder() {
   return new DecoderRowBased(new DecompressorZlib(),
                              VideoPacketFormat::ENCODING_ZLIB);
@@ -26,7 +31,6 @@ DecoderRowBased::DecoderRowBased(Decompressor* decompressor,
     : state_(kUninitialized),
       decompressor_(decompressor),
       encoding_(encoding),
-      bytes_per_src_pixel_(0),
       row_pos_(0),
       row_y_(0),
       // TODO(hclam): We should use the information from the update stream
@@ -45,38 +49,63 @@ void DecoderRowBased::Reset() {
 }
 
 bool DecoderRowBased::IsReadyForData() {
-  return state_ == kReady;
+  return state_ != kUninitialized && state_ != kError;

[awong, 2010/11/09 01:54:38]

Positive filters are usually a little more robust against adding states to the
the enum in the future.  Switch to that?

[Sergey Ulanov, 2010/11/09 21:34:42]

Done.

 }
 
-void DecoderRowBased::Initialize(scoped_refptr<media::VideoFrame> frame,
-                                 const gfx::Rect& clip,
-                                 int bytes_per_src_pixel) {
+void DecoderRowBased::Initialize(scoped_refptr<media::VideoFrame> frame) {
   // Make sure we are not currently initialized.
   CHECK_EQ(kUninitialized, state_);
 
-  if (static_cast<PixelFormat>(frame->format()) != PIXEL_FORMAT_RGB32) {
+  if (frame->format() != media::VideoFrame::RGB32) {
     LOG(WARNING) << "DecoderRowBased only supports RGB32.";
     state_ = kError;
     return;
   }
 
   frame_ = frame;
-
-  // Reset the buffer location status variables.
-  clip_ = clip;
-  row_pos_ = 0;
-  row_y_ = 0;
-  bytes_per_src_pixel_ = bytes_per_src_pixel;
-
   state_ = kReady;
 }
 
-void DecoderRowBased::DecodeBytes(const std::string& encoded_bytes) {
-  DCHECK_EQ(kReady, state_);
+Decoder::DecodeResult DecoderRowBased::DecodePacket(
+    const VideoPacket* packet) {
+  if (state_ == kError) {
+    return DECODE_ERROR;
+  }
+
+  if (packet->flags() & VideoPacket::FIRST_PACKET) {

[awong, 2010/11/09 01:54:38]

Can we pull the state transition logic out into a DetermineDecoderState()
function or something?

[Sergey Ulanov, 2010/11/09 21:34:42]

Done.

+    if (state_ != kReady && state_ != kDone) {
+      state_ = kError;
+      LOG(WARNING) << "Received unexpected FIRST_PACKET.";
+      return DECODE_ERROR;
+    }
+    state_ = kProcessing;
+
+    // Reset the buffer location status variables on the first packet.
+    clip_.SetRect(packet->format().x(), packet->format().y(),
+                  packet->format().width(), packet->format().height());
+    row_pos_ = 0;
+    row_y_ = 0;
+  }
+
+  if (state_ != kProcessing) {
+    state_ = kError;
+    LOG(WARNING) << "Received unexpected packet.";
+    return DECODE_ERROR;
+  }
+
+  if (packet->flags() & VideoPacket::LAST_PACKET) {
+    if (state_ != kProcessing) {
+      state_ = kError;
+      LOG(WARNING) << "Received unexpected LAST_PACKET.";
+      return DECODE_ERROR;
+    }
+    state_ = kDone;
+  }
 
-  const uint8* in = reinterpret_cast<const uint8*>(encoded_bytes.data());
-  const int in_size = encoded_bytes.size();
-  const int row_size = clip_.width() * bytes_per_src_pixel_;
+  const uint8* in = reinterpret_cast<const uint8*>(packet->data().data());
+  const int in_size = packet->data().size();
+
+  const int row_size = clip_.width() * kBytesPerPixel;
   int stride = frame_->stride(media::VideoFrame::kRGBPlane);
   uint8* rect_begin = frame_->data(media::VideoFrame::kRGBPlane);
 
@@ -88,15 +117,19 @@ void DecoderRowBased::DecodeBytes(const std::string& encoded_bytes) {
     stride = -stride;
   }
 
-  // TODO(ajwong): This should be bytes_per_dst_pixel shouldn't this.?
-  uint8* out = rect_begin +
-      stride * (clip_.y() + row_y_) +
-      bytes_per_src_pixel_ * clip_.x();
+  uint8* out = rect_begin + stride * (clip_.y() + row_y_) +
+      kBytesPerPixel * clip_.x();
 
   // Consume all the data in the message.
   bool decompress_again = true;
   int used = 0;
   while (decompress_again && used < in_size) {
+    if (row_y_ >= clip_.height()) {
+      state_ = kError;
+      LOG(WARNING) << "Too much data is received for the given rectangle.";
+      return DECODE_ERROR;
+    }
+
     int written = 0;
     int consumed = 0;
     // TODO(ajwong): This assume source and dest stride are the same, which is
@@ -114,6 +147,24 @@ void DecoderRowBased::DecodeBytes(const std::string& encoded_bytes) {
       out += stride;
     }
   }
+
+  if (state_ == kDone && row_y_ < clip_.height()) {
+    state_ = kError;
+    LOG(WARNING) << "Received LAST_PACKET, but didn't get enough data.";
+    return DECODE_ERROR;
+  }
+
+  return state_ == kDone ? DECODE_DONE : DECODE_IN_PROGRESS;
+}
+
+UpdatedRects* DecoderRowBased::GetUpdatedRects() {
+  UpdatedRects* rects = new UpdatedRects();
+  rects->push_back(clip_);
+  return rects;
+}
+
+VideoPacketFormat::Encoding DecoderRowBased::Encoding() {
+  return encoding_;
 }
 
 }  // namespace remoting