[Chromoting] Implement DualBufferFrameConsumer

remoting/client/dual_buffer_frame_consumer.cc

+// 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 (!buffers_[0]) {
+    return;
+  }
+  DCHECK(buffers_[0]->size().equals(buffers_[1]->size()));
+  std::unique_ptr<webrtc::DesktopFrame> full_frame(

[Sergey Ulanov, 2016/07/21 19:14:25]

Add some comments to explain what happens here.

[Yuwei, 2016/07/22 03:38:22]

Done.

+      webrtc::BasicDesktopFrame::CopyOf(*buffers_[0]));
+  webrtc::DesktopRect desktop_rect =
+        webrtc::DesktopRect::MakeSize(buffers_[0]->size());
+  for (webrtc::DesktopRegion::Iterator i(buffer_1_mask_); !i.IsAtEnd();
+       i.Advance()) {
+    CopyRGB32Rect(buffers_[0]->data(), buffers_[1]->stride(), desktop_rect,

[Sergey Ulanov, 2016/07/21 19:14:26]

Please use DesktopFrame::CopyPixelsFrom().

[Yuwei, 2016/07/22 03:38:22]

Done.

+                  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());
+
+  // Both buffers are reallocated whenever screen size changes.
+  if (!buffers_[0] || !buffers_[0]->size().equals(size)) {
+    buffers_[0] = webrtc::SharedDesktopFrame::Wrap(
+        base::WrapUnique(new webrtc::BasicDesktopFrame(size)));
+    buffers_[1] = webrtc::SharedDesktopFrame::Wrap(
+        base::WrapUnique(new webrtc::BasicDesktopFrame(size)));
+    buffer_1_mask_.Clear();
+    current_buffer_ = 0;
+  } else {
+    current_buffer_ = (current_buffer_ + 1) % 2;
+  }
+  return buffers_[current_buffer_]->Share();
+}
+
+void DualBufferFrameConsumer::DrawFrame(
+    std::unique_ptr<webrtc::DesktopFrame> frame,
+    const base::Closure& done) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  webrtc::SharedDesktopFrame* shared_frame =
+      reinterpret_cast<webrtc::SharedDesktopFrame*> (frame.get());
+  if (shared_frame->GetUnderlyingFrame() == buffers_[1]->GetUnderlyingFrame()) {
+    buffer_1_mask_.AddRegion(frame->updated_region());
+  } else if (shared_frame->GetUnderlyingFrame() ==
+      buffers_[0]->GetUnderlyingFrame()) {
+    buffer_1_mask_.Subtract(frame->updated_region());
+  } else {
+    // This may happen if the resolution is changed but a previous shared frame

[Sergey Ulanov, 2016/07/21 19:14:25]

Currently SoftwareVideoRenderer never processes more than one frame at a time,
so you can replace this with NOTREACHED(). (also if the renderer did try
decoding multiple frames in parallel you wouldn't want to drop this frame here -
it's still better to render it before the newer frames with updated size are
decoded.)

[Yuwei, 2016/07/22 03:38:22]

Removed the else branch.

The problem is that SoftwareVideoRenderer::ProcessVideoPacket calls
AllocateFrame and DrawFrame asynchronously. NOTREACHED() will be problematic if
this happens:

1. Allocate frame A
2. Allocate frame B, resolution changed
3. Draw frame A
4. Draw frame B

Step 3 will hit NOTREACHED() since its underlying buffer is already replaced in
DualBufferFrameConsumer...

[Sergey Ulanov, 2016/07/22 18:04:19]

Ugh. I was wrong when I wrote that SoftwareVideoRenderer doesn't process
multiple frames parallel. Sorry. The way SoftwareVideoRenderer is actually
implemented it may have multiple decode requests queued. It's possible to fix it
in SoftwareVideoRenderer by adding a frame queue, but I don't think it's
necessary.

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

I think the fact is that multiple frames can be processed by
SoftwareVideoRenderer at the same time but no two frames can be in the same
processing stage (receiving, decoding or rendering) nor their processing order
in each stage will alter since tasks are queued up in each thread. I don't think
we need to make any change to SoftwareVideoRenderer...

+    // with different resolution is received. In this case it is save to ignore
+    // the frame since a new full desktop frame will be received soon.
+    if (done) {
+      done.Run();
+    }
+    return;
+  }
+  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));
+    if (done) {

[Sergey Ulanov, 2016/07/21 19:14:26]

I don't think you need this check. DrawFrame() should only be called with a
non-null done callback.

[Yuwei, 2016/07/22 03:38:22]

Done. Removed.

+      done.Run();

[Sergey Ulanov, 2016/07/21 19:14:26]

We cannot assume that the frame have been rendered here. You want to pass that
done closure to callback_ so it's called when the frame is actually rendered
(i.e. after eglSwapBuffer() has returned). That callback is used to measure
rendering latency.

[Yuwei, 2016/07/21 20:00:41]

I guess the measurement will be messed up if another frame comes when the first
frame is being uploaded and then eglSwapBuffer is run after the second frame is
rendered?

[Sergey Ulanov, 2016/07/21 20:22:48]

To make it work correctly the renderer will need to keep queue of callbacks for
all pending frames and call them after eglSwapBuffer() completes. In other words
if a frame dropped before being rendered then it's considered to be rendered
only when the next frame is rendered. That's what we have in
PepperVideoRenderer3D.

[Yuwei, 2016/07/22 03:38:22]

Done.

+    }
+    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