[Chromoting] Implement DualBufferFrameConsumer

remoting/client/dual_buffer_frame_consumer.cc

Index: remoting/client/dual_buffer_frame_consumer.cc
diff --git a/remoting/client/dual_buffer_frame_consumer.cc b/remoting/client/dual_buffer_frame_consumer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..83bbd479a3ea46f77515894c395d696442f1dc8d
--- /dev/null
+++ b/remoting/client/dual_buffer_frame_consumer.cc
@@ -0,0 +1,116 @@
+// Copyright 2016 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/dual_buffer_frame_consumer.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/memory/ptr_util.h"
+#include "remoting/base/util.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+#include "third_party/webrtc/modules/desktop_capture/shared_desktop_frame.h"
+
+namespace remoting {
+
+DualBufferFrameConsumer::DualBufferFrameConsumer(
+    base::Callback<void(std::unique_ptr<webrtc::DesktopFrame>)> callback,
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    protocol::FrameConsumer::PixelFormat format)
+    : callback_(callback),
+      task_runner_(task_runner),
+      pixel_format_(format),
+      weak_factory_(this) {
+  weak_ptr_ = weak_factory_.GetWeakPtr();
+  thread_checker_.DetachFromThread();
+}
+
+DualBufferFrameConsumer::~DualBufferFrameConsumer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void DualBufferFrameConsumer::RequestFullDesktopFrame() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!buffer_0_) {
+    return;
+  }
+  DCHECK(buffer_0_->size().equals(buffer_1_->size()));
+  std::unique_ptr<webrtc::DesktopFrame> full_frame(
+      webrtc::BasicDesktopFrame::CopyOf(*buffer_0_));
+  webrtc::DesktopRect desktop_rect =
+        webrtc::DesktopRect::MakeSize(buffer_0_->size());
+  for (webrtc::DesktopRegion::Iterator i(buffer_1_mask_); !i.IsAtEnd();
+       i.Advance()) {
+    CopyRGB32Rect(buffer_1_->data(), buffer_1_->stride(), desktop_rect,
+                  full_frame->data(), full_frame->stride(), desktop_rect,
+                  i.rect());
+  }
+  full_frame->mutable_updated_region()->SetRect(desktop_rect);
+
+  RunRenderCallback(std::move(full_frame), base::Closure());
+}
+
+std::unique_ptr<webrtc::DesktopFrame> DualBufferFrameConsumer::AllocateFrame(
+    const webrtc::DesktopSize& size) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!buffer_0_ || !buffer_0_->size().equals(size)) {
+    buffer_0_ = webrtc::SharedDesktopFrame::Wrap(

[Sergey Ulanov, 2016/07/20 18:39:26]

add comment that both buffers are reallocated whenever screen size changes.

[Yuwei, 2016/07/21 00:07:32]

Done.

+        base::WrapUnique(new webrtc::BasicDesktopFrame(size)));
+    buffer_1_ = webrtc::SharedDesktopFrame::Wrap(
+        base::WrapUnique(new webrtc::BasicDesktopFrame(size)));
+    buffer_1_mask_.Clear();
+    next_buffer_ = 0;
+  }
+  next_buffer_ = (next_buffer_ + 1) % 2;

[Sergey Ulanov, 2016/07/20 18:39:26]

I think you want to synchronize buffers here (or maybe somewhere else).
Otherwise you end up rendering changes between frames N+1 and N+2 on frame N,
i.e. the changes between N and N+1 and not included in what you get in the end.
And to implement it you need to save updated_region() from the frame you get in
DrawFrame() somewhere.

[Yuwei, 2016/07/20 19:12:43]

Do you mean that when rendering is slower than decoding, then when the next
rendering happens, it may end up rendering the buffer when it is being updated?
I think this may just cause screen tearing very rarely. We can add locks for
this if we like...
I think this is fine. SharedDesktopFrame will only share the buffer, and each
SharedDesktopFrame will have its own updated_region(). Uploading a region that
is being updated between [N+1, N+2] will just cause screen tearing. Other than
that I don't think having part of [N+1, N+2] image for the updated_region in [N,
N+1] iteration will accumulate error.

+  if (next_buffer_ == 0) {

[Sergey Ulanov, 2016/07/20 18:39:26]

If you replace buffer_?_ with buffers_[] then this would be 'return
buffers_[(next_buffer_+1)%2]->Share();'

[Yuwei, 2016/07/21 00:07:32]

Done.

+    return buffer_1_->Share();
+  } else {
+    return buffer_0_->Share();
+  }
+}
+
+void DualBufferFrameConsumer::DrawFrame(
+    std::unique_ptr<webrtc::DesktopFrame> frame,
+    const base::Closure& done) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if ((reinterpret_cast<webrtc::SharedDesktopFrame*> (frame.get()))

[Yuwei, 2016/07/18 22:38:35]

This assumes |frame| must come from AllocateFrame. Looks like there is no
documented restrictions in FrameConsumer but I think this must be true?

[Sergey Ulanov, 2016/07/20 18:39:26]

Yes, this is a valid assumption. Caller must use AllocateFrame() to allocate
frames.

[Yuwei, 2016/07/21 00:07:32]

Acknowledged.

+      ->GetUnderlyingFrame() == buffer_1_->GetUnderlyingFrame()) {
+    buffer_1_mask_.AddRegion(frame->updated_region());
+  } else {
+    buffer_1_mask_.Subtract(frame->updated_region());
+  }
+  RunRenderCallback(std::move(frame), done);
+}
+
+protocol::FrameConsumer::PixelFormat
+DualBufferFrameConsumer::GetPixelFormat() {
+  return pixel_format_;
+}
+
+base::WeakPtr<DualBufferFrameConsumer> DualBufferFrameConsumer::GetWeakPtr() {
+  return weak_ptr_;
+}
+
+void DualBufferFrameConsumer::RunRenderCallback(
+    std::unique_ptr<webrtc::DesktopFrame> frame,
+    const base::Closure& done) {
+  if (!task_runner_) {
+    callback_.Run(std::move(frame));
+    done.Run();
+    return;
+  }
+  task_runner_->PostTaskAndReply(
+      FROM_HERE, base::Bind(callback_, base::Passed(&frame)),
+      base::Bind(&DualBufferFrameConsumer::OnFrameRendered, GetWeakPtr(),
+                 done));
+}
+
+void DualBufferFrameConsumer::OnFrameRendered(const base::Closure& done) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (done) {
+    done.Run();
+  }
+}
+
+}  // namespace remoting