Add in a new FrameConsumer interface, Decode API, and a RectangleUpdateDecoder abstraction.

This should allow a decoder that can still request the view to allocate frames without being owned by the view itself.  This allows for cleaner threading semantics and reduced coupling of classes.  The new decoder API keeps the decoder from being aware of the network packet types tightening up the API layering.

BUG=52833
TEST=None. This code isn't used yet.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60703

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

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

remoting/base/decoder.h

@@ -90,6 +90,30 @@ class Decoder {
   // of BeginDecode() / EndDecode().
   virtual bool IsStarted() { return started_; }
 
+  // --- NEW API ---
+  // TODO(ajwong): This API is incorrect in the face of a streaming decode
+  // protocol like VP8.  However, it breaks the layering abstraction by
+  // depending on the network packet protocol buffer type. I'm going to go
+  // forward with it as is, and then refactor again to support streaming
+  // decodes.
+
+  // Initializes the decoder to draw into the given |frame|.  The |clip|
+  // specifies the region to draw into.  The clip region must fit inside
+  // the dimensions of frame. Failure to do so will CHECK Fail.
+  virtual void Initialize(scoped_refptr<media::VideoFrame> frame,
+                          const gfx::Rect& clip) {}
+
+  // Reset the decoder to an uninitialized state. Release all references to
+  // the initialized |frame|.  Initialize() must be called before the decoder
+  // is used again.
+  virtual void Reset() {}
+
+  // Feeds more data into the decoder.
+  virtual void DecodeBytes(const std::string& encoded_bytes) {}
+
+  // Returns true if decoder is ready to accept data via ProcessRectangleData.
+  virtual bool IsReadyForData() { return false; }
+
  protected:
   // Every decoder will have two internal states because there are three
   // kinds of messages send to PartialDecode().

remoting/base/protocol/chromotocol.proto

@@ -138,7 +138,7 @@ message RectangleUpdatePacket {
   optional int32 sequence_number = 2 [default = 0];
 
   // This is provided on the first packet of the rectangle data, when
-  // the sequence_number is 0.
+  // the flags has FIRST_PACKET set.
   optional RectangleFormat format = 3;
 
   optional bytes encoded_rect = 4;

remoting/client/frame_consumer.h

+// Copyright (c) 2010 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.
+
+#ifndef REMOTING_CLIENT_FRAME_CONSUMER_H_
+#define REMOTING_CLIENT_FRAME_CONSUMER_H_
+
+namespace remoting {
+
+class FrameConsumer {
+ public:
+  FrameConsumer() {}
+  virtual ~FrameConsumer() {}
+
+  // Request a frame be allocated from the FrameConsumer.
+  //
+  // If a frame cannot be allocated to fit the format, and height/width
+  // requirements, |frame_out| will be set to NULL.
+  //
+  // An allocated frame will have at least the width and height requested, but
+  // may be bigger. Query the retrun frame for the actual frame size, stride,
+  // etc.
+  //
+  // The AllocateFrame call is asynchronous.  From invocation, until when the
+  // |done| callback is invoked, |frame_out| should be considered to be locked
+  // by the FrameConsumer, must remain a valid pointer, and should not be
+  // examined or modified.  After |done| is called, the |frame_out| will
+  // contain a result of the allocation. If a frame could not be allocated,
+  // |frame_out| will be NULL.
+  //
+  // All frames retrieved via the AllocateFrame call must be released by a
+  // corresponding call ReleaseFrame(scoped_refptr<VideoFrame>* frame_out.
+  virtual void AllocateFrame(media::VideoFrame::Format format,
+                             size_t width,
+                             size_t height,
+                             base::TimeDelta timestamp,
+                             base::TimeDelta duration,
+                             scoped_refptr<media::VideoFrame>* frame_out,
+                             Task* done) = 0;
+
+  virtual void ReleaseFrame(media::VideoFrame* frame) = 0;
+
+  // OnPartialFrameOutput() is called every time at least one rectangle of
+  // output is produced.  The |frame| is guaranteed to have valid data for
+  // every region included in the |rects| list.
+  //
+  // Both |frame| and |rects| are guaranteed to be valid until the |done|
+  // callback is invoked.
+  virtual void OnPartialFrameOutput(media::VideoFrame* frame,
+                                    UpdatedRects* rects,
+                                    Task* done) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FrameConsumer);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_CLIENT_FRAME_CONSUMER_H_

remoting/client/rectangle_update_decoder.cc

+// Copyright (c) 2010 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/client/rectangle_update_decoder.h"
+
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "media/base/callback.h"
+#include "remoting/base/decoder.h"
+#include "remoting/base/decoder_verbatim.h"
+#include "remoting/base/decoder_zlib.h"
+#include "remoting/base/protocol/chromotocol.pb.h"
+#include "remoting/base/tracer.h"
+#include "remoting/client/frame_consumer.h"
+
+using media::AutoTaskRunner;
+
+namespace remoting {
+
+namespace {
+
+class PartialFrameCleanup : public Task {
+ public:
+  PartialFrameCleanup(media::VideoFrame* frame, UpdatedRects* rects)
+      : frame_(frame), rects_(rects) {
+  }
+
+  virtual void Run() {
+    delete rects_;
+    frame_ = NULL;
+  }
+
+ private:
+  scoped_refptr<media::VideoFrame> frame_;
+  UpdatedRects* rects_;
+};
+
+}  // namespace
+
+RectangleUpdateDecoder::RectangleUpdateDecoder(MessageLoop* message_loop,
+                                               FrameConsumer* consumer)
+    : message_loop_(message_loop),
+      consumer_(consumer) {
+}
+
+RectangleUpdateDecoder::~RectangleUpdateDecoder() {
+}
+
+void RectangleUpdateDecoder::DecodePacket(const RectangleUpdatePacket& packet,
+                                          Task* done) {
+  if (message_loop_ != MessageLoop::current()) {
+    message_loop_->PostTask(
+        FROM_HERE,
+        NewTracedMethod(this,
+                        &RectangleUpdateDecoder::DecodePacket, packet,
+                        done));
+    return;
+  }
+  AutoTaskRunner done_runner(done);
+
+  TraceContext::tracer()->PrintString("Decode Packet called.");
+
+  if (!IsValidPacket(packet)) {
+    LOG(ERROR) << "Received invalid packet.";
+    return;
+  }
+
+  Task* process_packet_data =
+      NewTracedMethod(this,
+                      &RectangleUpdateDecoder::ProcessPacketData,
+                      packet, done_runner.release());
+
+  if (packet.flags() | RectangleUpdatePacket::FIRST_PACKET) {
+    const RectangleFormat& format = packet.format();
+
+    InitializeDecoder(format, process_packet_data);
+  } else {
+    process_packet_data->Run();
+    delete process_packet_data;
+  }
+}
+
+void RectangleUpdateDecoder::ProcessPacketData(
+    const RectangleUpdatePacket& packet,
+    Task* done) {
+  AutoTaskRunner done_runner(done);
+
+  if (!decoder_->IsReadyForData()) {
+    // TODO(ajwong): This whole thing should move into an invalid state.
+    LOG(ERROR) << "Decoder is unable to process data. Dropping packet.";
+    return;
+  }
+
+  TraceContext::tracer()->PrintString("Executing Decode.");
+  decoder_->DecodeBytes(packet.encoded_rect());
+
+  if (packet.flags() | RectangleUpdatePacket::LAST_PACKET) {
+    decoder_->Reset();
+
+    UpdatedRects* rects = new UpdatedRects();
+
+    // Empty out the list of current updated rects so the decoder can keep
+    // writing new ones while these are processed.
+    rects->swap(updated_rects_);
+
+    consumer_->OnPartialFrameOutput(frame_, rects,
+                                    new PartialFrameCleanup(frame_, rects));
+  }
+}
+
+// static
+bool RectangleUpdateDecoder::IsValidPacket(
+    const RectangleUpdatePacket& packet) {
+  if (!packet.IsInitialized()) {
+    LOG(WARNING) << "Protobuf consistency checks fail.";
+    return false;
+  }
+
+  // First packet must have a format.
+  if (packet.flags() | RectangleUpdatePacket::FIRST_PACKET) {
+    if (!packet.has_format()) {
+      LOG(WARNING) << "First packet must have format.";
+      return false;
+    }
+
+    // TODO(ajwong): Verify that we don't need to whitelist encodings.
+    const RectangleFormat& format = packet.format();
+    if (!format.has_encoding() ||
+        format.encoding() == EncodingInvalid) {
+      LOG(WARNING) << "Invalid encoding specified.";
+      return false;
+    }
+  }
+
+  // We shouldn't generate null packets.
+  if (!packet.has_encoded_rect()) {
+    LOG(WARNING) << "Packet w/o an encoded rectangle received.";
+    return false;
+  }
+
+  return true;
+}
+
+void RectangleUpdateDecoder::InitializeDecoder(const RectangleFormat& format,
+                                               Task* done) {
+  if (message_loop_ != MessageLoop::current()) {
+    message_loop_->PostTask(
+        FROM_HERE,
+        NewTracedMethod(this,
+                        &RectangleUpdateDecoder::InitializeDecoder,
+                        format, done));
+    return;
+  }
+  AutoTaskRunner done_runner(done);
+
+  // Check if we need to request a new frame.
+  if (!frame_ ||
+      frame_->width() < static_cast<size_t>(format.width()) ||
+      frame_->height() < static_cast<size_t>(format.height())) {
+    if (frame_) {
+      TraceContext::tracer()->PrintString("Releasing old frame.");
+      consumer_->ReleaseFrame(frame_);
+      frame_ = NULL;
+    }
+    TraceContext::tracer()->PrintString("Requesting new frame.");
+    consumer_->AllocateFrame(media::VideoFrame::RGB32,
+                             format.width(), format.height(),
+                             base::TimeDelta(), base::TimeDelta(),
+                             &frame_,
+                             NewTracedMethod(
+                                 this,
+                                 &RectangleUpdateDecoder::InitializeDecoder,
+                                 format,
+                                 done_runner.release()));
+    return;
+  }
+
+  // TODO(ajwong): We need to handle the allocator failing to create a frame
+  // and properly disable this class.
+  CHECK(frame_);
+
+  if (decoder_.get()) {
+    // TODO(ajwong): We need to handle changing decoders midstream correctly
+    // since someone may feed us a corrupt stream, or manually create a
+    // stream that changes decoders. At the very leask, we should not
+    // crash the process.
+    //
+    // For now though, assume that only one encoding is used throughout.
+    //
+    // Note, this may be as simple as just deleting the current decoder.
+    // However, we need to verify the flushing semantics of the decoder first.
+    CHECK(decoder_->Encoding() == format.encoding());
+  } else {
+    // Initialize a new decoder based on this message encoding.
+    if (format.encoding() == EncodingNone) {
+      TraceContext::tracer()->PrintString("Creating Verbatim decoder.");
+      decoder_.reset(new DecoderVerbatim());
+    } else if (format.encoding() == EncodingZlib) {
+      TraceContext::tracer()->PrintString("Creating Zlib decoder");
+      decoder_.reset(new DecoderZlib());
+    } else {
+      NOTREACHED() << "Invalid Encoding found: " << format.encoding();
+    }
+  }
+
+  // TODO(ajwong): This can happen in the face of corrupt input data.  Figure
+  // out the right behavior and make this more resilient.
+  CHECK(updated_rects_.empty());
+
+  gfx::Rect rectangle_size(format.x(), format.y(),
+                           format.width(), format.height());
+  updated_rects_.push_back(rectangle_size);
+  decoder_->Initialize(frame_, rectangle_size);
+  TraceContext::tracer()->PrintString("Decoder is Initialized");
+}
+
+}  // namespace remoting

remoting/client/rectangle_update_decoder.h

+// Copyright (c) 2010 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.
+
+#ifndef REMOTING_CLIENT_RECTANGLE_UPDATE_DECODER_H
+#define REMOTING_CLIENT_RECTANGLE_UPDATE_DECODER_H
+
+#include "base/scoped_ptr.h"
+#include "base/task.h"
+#include "media/base/video_frame.h"
+#include "remoting/base/decoder.h"  // For UpdatedRects.
+
+class MessageLoop;
+
+namespace remoting {
+
+class FrameConsumer;
+class RectangleFormat;
+class RectangleUpdatePacket;
+
+// TODO(ajwong): Re-examine this API, especially with regards to how error
+// conditions on each step are reported.  Should they be CHECKs? Logs? Other?
+class RectangleUpdateDecoder {
+ public:
+  RectangleUpdateDecoder(MessageLoop* message_loop,
+                         FrameConsumer* consumer);
+  ~RectangleUpdateDecoder();
+
+  // Decodes the contents of |packet| calling OnPartialFrameOutput() in the
+  // regsitered as data is avaialable. DecodePacket may keep a reference to
+  // |packet| so the |packet| must remain alive and valid until |done| is
+  // executed.
+  //
+  // TODO(ajwong): Should packet be a const pointer to make the lifetime
+  // more clear?
+  void DecodePacket(const RectangleUpdatePacket& packet, Task* done);
+
+ private:
+  static bool IsValidPacket(const RectangleUpdatePacket& packet);
+
+  void InitializeDecoder(const RectangleFormat& format, Task* done);
+
+  void ProcessPacketData(const RectangleUpdatePacket& packet, Task* done);
+
+  // Pointers to infrastructure objects.  Not owned.
+  MessageLoop* message_loop_;
+  FrameConsumer* consumer_;
+
+  scoped_ptr<Decoder> decoder_;
+  UpdatedRects updated_rects_;
+
+  // Framebuffer for the decoder.
+  scoped_refptr<media::VideoFrame> frame_;
+};
+
+}  // namespace remoting
+
+DISABLE_RUNNABLE_METHOD_REFCOUNT(remoting::RectangleUpdateDecoder);
+
+#endif  // REMOTING_CLIENT_RECTANGLE_UPDATE_DECODER_H

remoting/remoting.gyp

@@ -253,11 +253,14 @@
         'client/client_context.h',
         'client/client_util.cc',
         'client/client_util.h',
+        'client/frame_consumer.h',
         'client/host_connection.h',
         'client/input_handler.cc',
         'client/input_handler.h',
         'client/jingle_host_connection.cc',
         'client/jingle_host_connection.h',
+        'client/rectangle_update_decoder.cc',
+        'client/rectangle_update_decoder.h',
       ],
     },  # end of target 'chromoting_client'