[Chromoting] Implement DualBufferFrameConsumer

remoting/client/shared_frame_video_renderer.h

+// 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.
+
+#ifndef REMOTING_CLIENT_OPENGL_GL_SOFTWARE_VIDEO_RENDERER_H_
+#define REMOTING_CLIENT_OPENGL_GL_SOFTWARE_VIDEO_RENDERER_H_
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
+#include "remoting/client/software_video_renderer.h"
+#include "remoting/protocol/frame_consumer.h"
+
+namespace webrtc {
+class SharedDesktopFrame;
+}
+
+namespace remoting {
+
+// This class uses one single BasicDesktopFrame buffer to decode all video
+// frames using the underlying video renderer.
+// There is no synchronization to prevent screen tearing if decoding and

[Yuwei, 2016/07/15 20:57:12]

I'm not sure whether screen tearing is a concern I need to fix. Looks like the
host capturer also occasionally produces screen tearing...

[Sergey Ulanov, 2016/07/15 22:14:48]

Ideally we want to avoid it. ScreenCaptures don't have this problem. They use a
set of two buffers and the consumer is allowed to hold only on of them at a
time.
It should be possible to implement the same approach with two rotating buffers.

[Yuwei, 2016/07/15 22:55:32]

It will work if we just use the buffers to store updated regions and simply use
glTexSubImage2d to upload only the updated regions.

The weird problem is that in some cases like the phone get rotated during a
session, the OpenGL surface and context will be destroyed and recreated, and the
GPU memory will lose the current desktop frame. Not sure how we can restore
current frame in that case...

[Yuwei, 2016/07/16 02:24:56]

Hmm... I think it is possible to keep track of rectangle areas that are in
buffer 2 not in buffer 1. In that way we can restore the current frame easily.
Is there already efficient code for merging rectangles?

[Yuwei, 2016/07/16 02:31:00]

NVM... DesktopRegion::AddRect and DesktopRegion::Subtract should be good
enough...

[Yuwei, 2016/07/18 18:53:47]

Done. Used dual buffers.

+// rendering happen at the same time.
+// This class should be used and destroyed on the same thread. If |task_runner|
+// is null |callback| will be run directly upon the stack of DrawFrame,
+// otherwise it will be run on the thread of |task_runner|.
+class SharedFrameVideoRenderer : public protocol::FrameConsumer,

[Sergey Ulanov, 2016/07/15 22:14:48]

Do you actually need to implement VideoRenderer interface?
As far as I can tell you just want to have a FrameConsumer that reuses the same
buffer. SoftwareVideoRenderer can be used with any FrameConsumer.

[Yuwei, 2016/07/15 22:55:32]

This is basically managing to give VideoRenderer and FrameConsumer the same
lifecycle.

The problem here is SoftwareVideoRenderer takes in a raw pointer to the
FrameConsumer meaning that the renderer must not outlive FrameConsumer. When the
caller calls DisplayUpdaterFactory::CreateVideoRenderer(), they would expect
that the video renderer's lifecycle is independent on any unexposed object...

I'm not sure whether is it possible or flexible enough to make
SoftwareVideoRenderer own the FrameConsumer or take a WeakPtr of it...

[Yuwei, 2016/07/18 18:53:47]

Done. Made SoftwareVideoRenderer able to own a FrameConsumer.

Now we are able to turn JniVideoRenderer back to JniFrameConsumer. I'll do a
follow up CL for this.

+                                 public protocol::VideoRenderer {
+ public:
+  SharedFrameVideoRenderer(
+      base::Callback<void(std::unique_ptr<webrtc::DesktopFrame>)> callback,
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+      PixelFormat format);
+  ~SharedFrameVideoRenderer() override;
+
+  // The underlying video renderer must be set before using this class.
+  void SetUnderlyingVideoRenderer(
+      std::unique_ptr<protocol::VideoRenderer> renderer);
+
+  // VideoRenderer interface. Function calls will just be forwarded to the
+  // underlying video renderer.
+  bool Initialize(const ClientContext& client_context,
+                  protocol::FrameStatsConsumer* perf_tracker) override;
+  void OnSessionConfig(const protocol::SessionConfig& config) override;
+  protocol::VideoStub* GetVideoStub() override;
+  protocol::FrameConsumer* GetFrameConsumer() override;
+  protocol::FrameStatsConsumer* GetFrameStatsConsumer() override;
+
+  // FrameConsumer interface.
+  std::unique_ptr<webrtc::DesktopFrame> AllocateFrame(
+      const webrtc::DesktopSize& size) override;
+  void DrawFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
+                 const base::Closure& done) override;
+  PixelFormat GetPixelFormat() override;
+
+ private:
+  void OnFrameRendered(const base::Closure& done);
+
+  std::unique_ptr<protocol::VideoRenderer> underlying_video_renderer_;
+
+  std::unique_ptr<webrtc::SharedDesktopFrame> shared_frame_;
+  base::Callback<void(std::unique_ptr<webrtc::DesktopFrame>)> callback_;
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  PixelFormat pixel_format_;
+  base::ThreadChecker thread_checker_;
+  base::WeakPtrFactory<SharedFrameVideoRenderer> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(SharedFrameVideoRenderer);
+};
+
+}  // namespace remoting
+#endif  // REMOTING_CLIENT_OPENGL_GL_SOFTWARE_VIDEO_RENDERER_H_