Improving the decoder pipeline.

remoting/client/rectangle_update_decoder.cc

Index: remoting/client/rectangle_update_decoder.cc
diff --git a/remoting/client/rectangle_update_decoder.cc b/remoting/client/rectangle_update_decoder.cc
index 673c2a037061f78d23afbd4c916b6c5995e750a7..3581f1d1c1af1ba5d3257335a4d40c56c3ca87af 100644
--- a/remoting/client/rectangle_update_decoder.cc
+++ b/remoting/client/rectangle_update_decoder.cc
@@ -10,6 +10,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
+#include "ppapi/cpp/image_data.h"
 #include "remoting/base/decoder.h"
 #include "remoting/base/decoder_row_based.h"
 #include "remoting/base/decoder_vp8.h"
@@ -17,6 +18,7 @@
 #include "remoting/client/frame_consumer.h"
 #include "remoting/protocol/session_config.h"
 
+using base::Passed;
 using remoting::protocol::ChannelConfig;
 using remoting::protocol::SessionConfig;
 
@@ -27,8 +29,8 @@ RectangleUpdateDecoder::RectangleUpdateDecoder(
     : message_loop_(message_loop),
       consumer_(consumer),
       screen_size_(SkISize::Make(0, 0)),
-      clip_rect_(SkIRect::MakeEmpty()),
-      decoder_needs_reset_(false) {
+      view_size_(SkISize::Make(0, 0)),
+      clip_area_(SkIRect::MakeEmpty()) {
 }
 
 RectangleUpdateDecoder::~RectangleUpdateDecoder() {
@@ -56,24 +58,19 @@ void RectangleUpdateDecoder::DecodePacket(const VideoPacket* packet,
                               this, packet, done));
     return;
   }
-  AllocateFrame(packet, done);
-}
 
-void RectangleUpdateDecoder::AllocateFrame(const VideoPacket* packet,
-                                           const base::Closure& done) {
-  if (!message_loop_->BelongsToCurrentThread()) {
-    message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::AllocateFrame,
-                              this, packet, done));
-    return;
-  }
   base::ScopedClosureRunner done_runner(done);
+  bool decoder_needs_reset = false;
 
   // If the packet includes a screen size, store it.
   if (packet->format().has_screen_width() &&
       packet->format().has_screen_height()) {
-    screen_size_.set(packet->format().screen_width(),
-                     packet->format().screen_height());
+    SkISize screen_size = SkISize::Make(packet->format().screen_width(),
+                                        packet->format().screen_height());
+    if (screen_size_ != screen_size) {
+      screen_size_ = screen_size;
+      decoder_needs_reset = true;
+    }
   }
 
   // If we've never seen a screen size, ignore the packet.
@@ -81,42 +78,9 @@ void RectangleUpdateDecoder::AllocateFrame(const VideoPacket* packet,
     return;
   }
 
-  // Ensure the output frame is the right size.
-  SkISize frame_size = SkISize::Make(0, 0);
-  if (frame_)
-    frame_size.set(frame_->width(), frame_->height());
-
-  // Allocate a new frame, if necessary.
-  if ((!frame_) || (screen_size_ != frame_size)) {
-    if (frame_) {
-      consumer_->ReleaseFrame(frame_);
-      frame_ = NULL;
-    }
-
-    consumer_->AllocateFrame(
-        media::VideoFrame::RGB32, screen_size_, &frame_,
-        base::Bind(&RectangleUpdateDecoder::ProcessPacketData,
-                   this, packet, done_runner.Release()));
-    decoder_needs_reset_ = true;
-    return;
-  }
-  ProcessPacketData(packet, done_runner.Release());
-}
-
-void RectangleUpdateDecoder::ProcessPacketData(
-    const VideoPacket* packet, const base::Closure& done) {
-  if (!message_loop_->BelongsToCurrentThread()) {
-    message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::ProcessPacketData,
-                              this, packet, done));
-    return;
-  }
-  base::ScopedClosureRunner done_runner(done);
-
-  if (decoder_needs_reset_) {
-    decoder_->Reset();
-    decoder_->Initialize(frame_);
-    decoder_needs_reset_ = false;
+  if (decoder_needs_reset) {
+    decoder_->Initialize(screen_size_);
+    consumer_->SetScreenSize(screen_size_);
   }
 
   if (!decoder_->IsReadyForData()) {
@@ -126,109 +90,102 @@ void RectangleUpdateDecoder::ProcessPacketData(
   }
 
   if (decoder_->DecodePacket(packet) == Decoder::DECODE_DONE)
-    SubmitToConsumer();
+    DoPaint();
 }
 
-void RectangleUpdateDecoder::SetOutputSize(const SkISize& size) {
-  if (!message_loop_->BelongsToCurrentThread()) {
-    message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::SetOutputSize,
-                              this, size));
+void RectangleUpdateDecoder::DoPaint() {
+  if (buffers_.empty())
     return;
-  }
 
-  // TODO(wez): Refresh the frame only if the ratio has changed.
-  if (frame_) {
-    SkIRect frame_rect = SkIRect::MakeWH(frame_->width(), frame_->height());
-    refresh_region_.op(frame_rect, SkRegion::kUnion_Op);
-  }
+  // Draw the invalidated region to the buffer.
+  pp::ImageData* buffer = buffers_.front();
+  SkRegion output_region;
+  decoder_->RenderFrame(view_size_, clip_area_,
+                        reinterpret_cast<uint8*>(buffer->data()),
+                        buffer->stride(),
+                        &output_region);
 
-  // TODO(hclam): If the scale ratio has changed we should reallocate a
-  // VideoFrame of different size. However if the scale ratio is always
-  // smaller than 1.0 we can use the same video frame.
-  if (decoder_.get()) {
-    decoder_->SetOutputSize(size);
-    RefreshFullFrame();
+  // Notify the consumer that painting is done.
+  if (!output_region.isEmpty()) {
+    buffers_.pop();
+    consumer_->PaintBuffer(view_size_, clip_area_, buffer, output_region);
   }
 }
 
-void RectangleUpdateDecoder::UpdateClipRect(const SkIRect& new_clip_rect) {
+void RectangleUpdateDecoder::DrainQueue(const base::Closure& done) {
   if (!message_loop_->BelongsToCurrentThread()) {
     message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::UpdateClipRect,
-                              this, new_clip_rect));
+        FROM_HERE, base::Bind(&RectangleUpdateDecoder::DrainQueue, this, done));
     return;
   }
 
-  if (new_clip_rect == clip_rect_ || !decoder_.get())
-    return;
-
-  // TODO(wez): Only refresh newly-exposed portions of the frame.
-  if (frame_) {
-    SkIRect frame_rect = SkIRect::MakeWH(frame_->width(), frame_->height());
-    refresh_region_.op(frame_rect, SkRegion::kUnion_Op);
+  while (!buffers_.empty()) {
+    consumer_->ReturnBuffer(buffers_.front());
+    buffers_.pop();
   }
 
-  clip_rect_ = new_clip_rect;
-  decoder_->SetClipRect(new_clip_rect);
-
-  // TODO(wez): Defer refresh so that multiple events can be batched.
-  DoRefresh();
+  if (!done.is_null())
+    done.Run();
 }
 
-void RectangleUpdateDecoder::RefreshFullFrame() {
+void RectangleUpdateDecoder::EnqueueBuffer(pp::ImageData* buffer) {
   if (!message_loop_->BelongsToCurrentThread()) {
     message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::RefreshFullFrame, this));
+        FROM_HERE, base::Bind(&RectangleUpdateDecoder::EnqueueBuffer,
+                              this, buffer));
     return;
   }
 
-  // If a video frame or the decoder is not allocated yet then don't
-  // save the refresh rectangle to avoid wasted computation.
-  if (!frame_ || !decoder_.get())
-    return;
-
-  SkIRect frame_rect = SkIRect::MakeWH(frame_->width(), frame_->height());
-  refresh_region_.op(frame_rect, SkRegion::kUnion_Op);
-
-  DoRefresh();
-}
-
-void RectangleUpdateDecoder::SubmitToConsumer() {
-  // A frame is not allocated yet, we can reach here because of a refresh
-  // request.
-  if (!frame_)
-    return;
-
-  SkRegion* dirty_region = new SkRegion;
-  decoder_->GetUpdatedRegion(dirty_region);
+  DCHECK(clip_area_.width() <= buffer->size().width() &&
+         clip_area_.height() <= buffer->size().height());
 
-  consumer_->OnPartialFrameOutput(frame_, dirty_region, base::Bind(
-      &RectangleUpdateDecoder::OnFrameConsumed, this, dirty_region));
+  buffers_.push(buffer);
+  DoPaint();
 }
 
-void RectangleUpdateDecoder::DoRefresh() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-
-  if (refresh_region_.isEmpty())
+void RectangleUpdateDecoder::Invalidate(const SkRegion& region) {
+  if (!message_loop_->BelongsToCurrentThread()) {
+    message_loop_->PostTask(
+        FROM_HERE, base::Bind(&RectangleUpdateDecoder::Invalidate,
+                              this, region));
     return;
+  }
 
-  decoder_->RefreshRegion(refresh_region_);
-  refresh_region_.setEmpty();
-  SubmitToConsumer();
+  if (decoder_.get()) {
+    decoder_->Invalidate(view_size_, region);
+    DoPaint();
+  }
 }
 
-void RectangleUpdateDecoder::OnFrameConsumed(SkRegion* region) {
+void RectangleUpdateDecoder::SetView(const SkISize& view_size,
+                                     const SkIRect& clip_area) {
   if (!message_loop_->BelongsToCurrentThread()) {
     message_loop_->PostTask(
-        FROM_HERE, base::Bind(&RectangleUpdateDecoder::OnFrameConsumed,
-                              this, region));
+        FROM_HERE, base::Bind(&RectangleUpdateDecoder::SetView,
+                              this, view_size, clip_area));
     return;
   }
 
-  delete region;
+  // The whole frame needs to be repainted if the scaling factor has changed.
+  // Otherwise the newly exposed parts of the frame will be automatically
+  // updated because they haven't been validated by RenderFrame() yet.

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

This sentence sounds the wrong way around?

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

Yeah. I was trying to say that if the scaling is the same we don't need to do
anything special. But it is better to say that by not saying anything. :-)

+  if (view_size_ != view_size && decoder_.get()) {
+    SkRegion region;
+    region.op(SkIRect::MakeSize(view_size), SkRegion::kUnion_Op);
+    decoder_->Invalidate(view_size, region);
+  }
+
+  if (view_size_ != view_size ||
+      clip_area_ != clip_area) {
+    view_size_ = view_size;
+    clip_area_ = clip_area;
 
-  DoRefresh();
+    // Return buffers to the consumer for reuse/reallocation.
+    while (!buffers_.empty()) {
+      consumer_->ReturnBuffer(buffers_.front());
+      buffers_.pop();
+    }
+  }
 }
 
 }  // namespace remoting