iOS Chromoting Client

remoting/ios/bridge/frame_consumer_bridge.cc

+// Copyright 2014 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/ios/bridge/frame_consumer_bridge.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/synchronization/waitable_event.h"
+#include "remoting/base/util.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+
+namespace remoting {
+
+FrameConsumerBridge::FrameConsumerBridge(FrameConsumerBridgeCallback callback)
+    : callback_(callback), frame_producer_(NULL) {}
+
+void FrameConsumerBridge::Initialize(FrameProducer* producer) {
+  frame_producer_ = producer;
+}
+
+void FrameConsumerBridge::ApplyBuffer(const webrtc::DesktopSize& view_size,
+                                      const webrtc::DesktopRect& clip_area,
+                                      webrtc::DesktopFrame* buffer,
+                                      const webrtc::DesktopRegion& region,
+                                      const webrtc::DesktopRegion& shape) {
+
+  if (!view_size_.equals(view_size)) {
+    // Drop the frame, since the data belongs to the previous generation,
+    // before SetSourceSize() called SetOutputSizeAndClip().
+    FreeBuffer(buffer);
+    return;
+  }
+
+  callback_.Run(view_size, buffer, region);
+
+  // Recycle |buffer| by returning the |frame_producer_| as the next buffer
+  if (frame_producer_) {

[dcaiafa, 2014/03/19 01:14:15]

Is it valid for frame_producer_ to be NULL? If not, add a DCHECK and remove
if-block, here and below.

[aboone, 2014/03/21 16:42:07]

It can be NULL between a call to the constructor and a call to initialize.  But
I believe you're saying to disallow all workflows that would have that result,
which is understandable.

Done.

+    frame_producer_->DrawBuffer(buffer);

[dcaiafa, 2014/03/19 01:14:15]

I don't think you can reuse the buffer here. In the current setup, won't the
callback process the buffer asynchronously? The callback is the new owner of the
buffer, and its signature should be updated to reflect that (use scoped_ptr).

[aboone, 2014/03/21 16:42:07]

The callback is not a task post, its just an indirect function call which is
synchronous.  A series of functions ensues, and the buffer ends up at
[HostController applyFrame] which copies only the needed data so GL can access
it on the next paint.

The strategy you're envisioning uses more memory, but less memory writes.  If I
have time which I probably won't, I'll come back to this and try it out.

+  }
+}
+
+void FrameConsumerBridge::ReturnBuffer(webrtc::DesktopFrame* buffer) {
+  FreeBuffer(buffer);
+}
+
+void FrameConsumerBridge::SetSourceSize(const webrtc::DesktopSize& source_size,
+                                        const webrtc::DesktopVector& dpi) {
+  view_size_ = source_size;
+  webrtc::DesktopRect clip_area = webrtc::DesktopRect::MakeSize(view_size_);
+  if (frame_producer_) {
+    frame_producer_->SetOutputSizeAndClip(view_size_, clip_area);
+  }
+
+  // Allocate buffer and start drawing frames onto it.
+  AllocateBuffer();

[dcaiafa, 2014/03/19 01:14:15]

The name of the method is confusing. It's main purpose is not to allocate
buffers. Maybe rename it to "DrawBuffer"?

[aboone, 2014/03/21 16:42:07]

Renamed to DrawWithNewBuffer

+}
+
+FrameConsumerBridge::PixelFormat FrameConsumerBridge::GetPixelFormat() {
+  return FORMAT_RGBA;
+}
+
+void FrameConsumerBridge::AllocateBuffer() {
+  webrtc::DesktopFrame* buffer = new webrtc::BasicDesktopFrame(view_size_);
+  buffers_.push_back(buffer);
+  if (frame_producer_) {
+    frame_producer_->DrawBuffer(buffer);
+  }
+}
+
+void FrameConsumerBridge::FreeBuffer(webrtc::DesktopFrame* buffer) {
+  ScopedVector<webrtc::DesktopFrame>::iterator it =
+      std::find(buffers_.begin(), buffers_.end(), buffer);
+
+  if (it != buffers_.end()) {
+    buffers_.erase(it);
+  }
+}
+
+}  // namespace remoting