Improving the decoder pipeline.

remoting/base/decoder_row_based.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/base/decoder_row_based.h"
 
 #include "base/logging.h"
 #include "remoting/base/decompressor.h"
 #include "remoting/base/decompressor_zlib.h"
 #include "remoting/base/decompressor_verbatim.h"
@@ skipping 16 matching lines @@
                              VideoPacketFormat::ENCODING_VERBATIM);
 }
 
 DecoderRowBased::DecoderRowBased(Decompressor* decompressor,
                                  VideoPacketFormat::Encoding encoding)
     : state_(kUninitialized),
       clip_(SkIRect::MakeEmpty()),
       decompressor_(decompressor),
       encoding_(encoding),
       row_pos_(0),
-      row_y_(0) {
+      row_y_(0),
+      screen_size_(SkISize::Make(0, 0)) {
 }
 
 DecoderRowBased::~DecoderRowBased() {
 }
 
-void DecoderRowBased::Reset() {
-  frame_ = NULL;
-  decompressor_->Reset();
-  state_ = kUninitialized;
-  updated_region_.setEmpty();
-}
-
 bool DecoderRowBased::IsReadyForData() {
   switch (state_) {
     case kUninitialized:
     case kError:
       return false;
     case kReady:
     case kProcessing:
     case kPartitionDone:
     case kDone:
       return true;
   }
   NOTREACHED();
   return false;
 }
 
-void DecoderRowBased::Initialize(scoped_refptr<media::VideoFrame> frame) {
-  // Make sure we are not currently initialized.
-  CHECK_EQ(kUninitialized, state_);
+void DecoderRowBased::Initialize(const SkISize& screen_size) {
+  decompressor_->Reset();
+  updated_region_.setEmpty();
+  screen_buffer_.reset(NULL);
 
-  if (frame->format() != media::VideoFrame::RGB32) {
-    LOG(WARNING) << "DecoderRowBased only supports RGB32.";
-    state_ = kError;
-    return;
+  screen_size_ = screen_size;
+  // Allocate the screen buffer, if necessary.
+  if (!screen_size_.isZero()) {

[Wez, 2012/02/17 23:42:17]

isZero() -> isEmpty()

[alexeypa (please no reviews), 2012/02/21 23:00:44]

Done.

+    screen_buffer_.reset(new uint8[
+        screen_size_.width() * screen_size_.height() * kBytesPerPixel]);
   }
 
-  frame_ = frame;
   state_ = kReady;
 }
 
 Decoder::DecodeResult DecoderRowBased::DecodePacket(const VideoPacket* packet) {
   UpdateStateForPacket(packet);
 
   if (state_ == kError) {
     return DECODE_ERROR;
   }
 
+  // The input buffer is a linear vector of pixel data.

[Wez, 2012/02/17 23:42:17]

nit: This comment doesn't really add anything to the following lines

[alexeypa (please no reviews), 2012/02/21 23:00:44]

Done.

   const uint8* in = reinterpret_cast<const uint8*>(packet->data().data());
   const int in_size = packet->data().size();
+  const int row_size = clip_.width() * kBytesPerPixel;
 
-  const int row_size = clip_.width() * kBytesPerPixel;
-  int stride = frame_->stride(media::VideoFrame::kRGBPlane);
-  uint8* rect_begin = frame_->data(media::VideoFrame::kRGBPlane);
-
-  uint8* out = rect_begin + stride * (clip_.fTop + row_y_) +
-      kBytesPerPixel * clip_.fLeft;
+  // Access the pixel buffer directly.

[Wez, 2012/02/17 23:42:17]

nit: This comment doesn't really add anything

[alexeypa (please no reviews), 2012/02/21 23:00:44]

Done.

+  int out_stride = screen_size_.width() * kBytesPerPixel;
+  uint8* out = screen_buffer_.get() + out_stride * (clip_.top() + row_y_) +
+      kBytesPerPixel * clip_.left();
 
   // 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
-    // incorrect.
     decompress_again = decompressor_->Process(
         in + used, in_size - used, out + row_pos_, row_size - row_pos_,
         &consumed, &written);
     used += consumed;
     row_pos_ += written;
 
     // If this row is completely filled then move onto the next row.
     if (row_pos_ == row_size) {
       ++row_y_;
       row_pos_ = 0;
-      out += stride;
+      out += out_stride;
     }
   }
 
   if (state_ == kPartitionDone || state_ == kDone) {
     if (row_y_ < clip_.height()) {
       state_ = kError;
       LOG(WARNING) << "Received LAST_PACKET, but didn't get enough data.";
       return DECODE_ERROR;
     }
 
@@ skipping 12 matching lines @@
   if (state_ == kError) {
     return;
   }
 
   if (packet->flags() & VideoPacket::FIRST_PACKET) {
     if (state_ != kReady && state_ != kDone && state_ != kPartitionDone) {
       state_ = kError;
       LOG(WARNING) << "Received unexpected FIRST_PACKET.";
       return;
     }
-    state_ = kProcessing;
 
     // Reset the buffer location status variables on the first packet.
     clip_.setXYWH(packet->format().x(), packet->format().y(),
                   packet->format().width(), packet->format().height());
+    if (!SkIRect::MakeSize(screen_size_).contains(clip_)) {
+      state_ = kError;
+      LOG(WARNING) << "Invalid clipping area received.";

[Wez, 2012/02/17 23:42:17]

If this is never valid then consider a DCHECK, or NOTREACHED

[alexeypa (please no reviews), 2012/02/21 23:00:44]

This should never happen but this data arrives from the network so we should be
able to handle any sort of bogus data.

+      return;
+    }
+
+    state_ = kProcessing;
     row_pos_ = 0;
     row_y_ = 0;
   }
 
   if (state_ != kProcessing) {
     state_ = kError;
     LOG(WARNING) << "Received unexpected packet.";
     return;
   }
 
@@ skipping 11 matching lines @@
       state_ = kError;
       LOG(WARNING) << "Received unexpected LAST_PARTITION.";
       return;
     }
     state_ = kDone;
   }
 
   return;
 }
 
-void DecoderRowBased::GetUpdatedRegion(SkRegion* region) {
-  region->swap(updated_region_);
-  updated_region_.setEmpty();
-}
-
 VideoPacketFormat::Encoding DecoderRowBased::Encoding() {
   return encoding_;
 }
 
+void DecoderRowBased::Invalidate(const SkISize& view_size,
+                                 const SkRegion& region) {
+  updated_region_.op(region, SkRegion::kUnion_Op);
+}
+
+void DecoderRowBased::RenderFrame(const SkISize& view_size,
+                                  const SkIRect& clip_area,
+                                  uint8* image_buffer,
+                                  int image_stride,
+                                  SkRegion* output_region) {
+  output_region->setEmpty();
+
+  // TODO(alexeypa): implement scaling in the case when |view_size| does
+  // not match |screen_size_|.

[Wez, 2012/02/17 23:42:17]

nit: Consider changing this to a comment bemoaning the lack of scaling, rather
than a statement of intent to implement it.  Or add a feature-tracking bug... ;)

[alexeypa (please no reviews), 2012/02/21 23:00:44]

Done.

+  SkIRect clip_rect = SkIRect::MakeSize(screen_size_);
+  if (!clip_rect.intersect(clip_area))
+    return;
+
+  int screen_stride = screen_size_.width() * kBytesPerPixel;
+
+  for (SkRegion::Iterator i(updated_region_); !i.done(); i.next()) {
+    SkIRect rect(i.rect());
+    if (!rect.intersect(clip_rect))
+      continue;
+
+    CopyRGB32Rect(screen_buffer_.get(), screen_stride,
+                  clip_rect,
+                  image_buffer, image_stride,
+                  clip_area,
+                  rect);
+    output_region->op(rect, SkRegion::kUnion_Op);
+  }
+
+  updated_region_.setEmpty();
+}
+
 }  // namespace remoting