Improving the decoder pipeline.

remoting/client/frame_producer.h

+// 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.
+
+#ifndef REMOTING_CLIENT_FRAME_PRODUCER_H_
+#define REMOTING_CLIENT_FRAME_PRODUCER_H_
+
+#include "base/callback_forward.h"
+#include "third_party/skia/include/core/SkRect.h"
+#include "third_party/skia/include/core/SkRegion.h"
+#include "third_party/skia/include/core/SkSize.h"
+
+namespace pp {
+class ImageData;
+} // namespace pp
+
+namespace remoting {
+
+class FrameProducer {
+ public:
+  FrameProducer() {}
+  virtual ~FrameProducer() {}
+
+  // This routine drains the queue returning all pending buffers to
+  // the consumer.

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

nit: "Request that all pending buffers be returned to the consumer via
ReturnBuffer"?

Consider renaming this to ReturnBuffers() or RequestReturnBuffers().

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

Done.

+  virtual void DrainQueue(const base::Closure& done) = 0;
+
+  // This routine submits an image buffer for subsequent drawing. Once any part
+  // of the frame within the clipping area is invalidated, the producer copies
+  // the updated pixels to one of the pending buffers and return it to
+  // the consumer via the FrameConsumer::PaintBuffer() call.
+  //
+  // The passed buffer must be large enough to hold the whole clipping area.

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

I think this method needs a better name, and description!

The method is used to add output buffers for the producer to use.  Although
there is a queue in the implementation, it's not strictly necessary for the
producer to use a queue, I don't think.

How about AddOutputBuffer()?

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

AddBuffer() because no user method calls these buffers output buffers.

[Wez, 2012/02/23 00:11:09]

Wouldn't DrawBuffer be more appropriate, then, since the buffer gets drawn?

[alexeypa (please no reviews), 2012/02/23 17:10:33]

FrameProducer::AddBuffer -> FrameProducer::DrawBuffer
FrameConsumer::PaintBuffer -> FrameConsumer::ApplyBuffer

+  virtual void EnqueueBuffer(pp::ImageData* buffer) = 0;
+
+  // The frame consumer can call this routine requesting the frame to be
+  // repainted. The specified region of the frame is will be invalided.
+  // The region is given in client's coordinates relative to the beginning of
+  // the frame.

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

nit: Comment style, e.g:

"Requests that the specified region of the frame be refreshed as soon as
possible"

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

Done.

+  virtual void Invalidate(const SkRegion& region) = 0;

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

Consider calling this RequestRegion() or InvalidateRegion().

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

Done.

+
+  // The frame consumer calls this function to notify the producer about view
+  // area changes. This can result in producer immediately returning some of or
+  // all enqueued buffers to the consumer for reuse or reallocation.

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

nit: Comment style, e.g:

"Notifies the producer of changes to the output view size or visible (clip)
area."

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

Done.

+  virtual void SetView(const SkISize& view_size, const SkIRect& clip_area) = 0;

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

"View" is a Pepper term; in abstract terms this sets the output size, and
visible/clip area.  How about SetOutputSizeAndClip()?

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

It should be SetScalingFactorAndClipArea() then. I think it is unnecessary long
and is as informative as SetView() is. "View" is a pretty generic term. It is
used in Pepper API. It is used in a bunch of other cases. 

[Wez, 2012/02/23 00:11:09]

The API doesn't take a scaling factor, it takes an output size and an output
clip area; hence my suggesting SetOutputSizeAndClip().

The problem with View is that it's Pepper-specific, and doesn't mean output size
and clip even in that context.

[alexeypa (please no reviews), 2012/02/23 17:10:33]

Done.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FrameProducer);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_CLIENT_FRAME_PRODUCER_H_