Implement screen capturer for MediaStream API.

content/browser/renderer_host/media/screen_capturer.cc

+// 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.
+
+#include "content/browser/renderer_host/media/screen_capturer.h"
+
+#include "remoting/capturer/capture_data.h"
+#include "remoting/capturer/mouse_cursor_shape.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkDevice.h"
+
+namespace content {
+
+namespace {
+const int kBytesPerPixel = 4;
+}  // namespace
+
+ScreenCapturer::ScreenCapturer()
+    : capture_thread_("Screen Capture Thread"),
+      event_handler_(NULL),
+      capture_pending_(false),
+      waiting_frame_size_(false),
+      started_(false) {
+  name_.device_name = "Screen";
+}
+
+ScreenCapturer::~ScreenCapturer() {

[Wez, 2013/01/10 00:58:14]

If the caller hasn't called DeAllocate then we still have a timer that expects
to be torn down on the capture thread but won't be - CHECK() that the capture
thread is not running here?

[Sergey Ulanov, 2013/01/11 23:46:20]

Added DeAllocate.

+}
+
+void ScreenCapturer::Allocate(int width, int height,
+                              int frame_rate,
+                              EventHandler* event_handler) {

[Wez, 2013/01/10 00:58:14]

nit: Consider [D]CHECKing the parameters. e.g. frame_rate == 0 would result in a
divide-by-zero fault on the capture thread.

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+  capture_thread_.Start();

[Wez, 2013/01/10 00:58:14]

Is there no other thread or thread pool we can allocate from to avoid a
thread-per-capturer?

[Sergey Ulanov, 2013/01/11 23:46:20]

Changed this code to use blocking IO pool.

+
+  // base::Unretained is safe because we control lifetime of the thread.
+  capture_thread_.message_loop()->PostTask(
+      FROM_HERE, base::Bind(&ScreenCapturer::DoAllocate, base::Unretained(this),
+                            frame_rate, event_handler));
+}
+
+void ScreenCapturer::Start() {

[wjia(left Chromium), 2013/01/07 22:15:26]

check capture_thread_.IsRunning() here too?

[Sergey Ulanov, 2013/01/09 20:35:30]

Done.

+  capture_thread_.message_loop()->PostTask(
+      FROM_HERE, base::Bind(&ScreenCapturer::DoStart, base::Unretained(this)));
+}
+
+void ScreenCapturer::Stop() {
+  capture_thread_.message_loop()->PostTask(
+      FROM_HERE, base::Bind(&ScreenCapturer::DoStop, base::Unretained(this)));
+}
+
+void ScreenCapturer::DeAllocate() {
+  if (capture_thread_.IsRunning()) {
+    capture_thread_.message_loop()->PostTask(
+        FROM_HERE, base::Bind(&ScreenCapturer::DoDeAllocate,
+                              base::Unretained(this)));
+    capture_thread_.Stop();

[Wez, 2013/01/10 00:58:14]

Does it matter that this might block e.g. for half a second if a screen capture
is in-progress on the thread and the host is slow?

[Sergey Ulanov, 2013/01/11 23:46:20]

Removed capture_thread_. thread pool is used instead now.

+  }
+}
+
+const media::VideoCaptureDevice::Name& ScreenCapturer::device_name() {
+  return name_;
+}
+
+void ScreenCapturer::OnCaptureCompleted(
+    scoped_refptr<remoting::CaptureData> capture_data) {
+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  DCHECK(capture_pending_);
+  capture_pending_ = false;
+
+  if (waiting_frame_size_) {
+    frame_size_ = capture_data->size();
+    waiting_frame_size_ = false;
+
+    // Invoke OnFrameInfo().

[Wez, 2013/01/10 00:58:14]

nit: Please make this more descriptive, e.g. "Inform the EventHandler of the
frame dimensions".

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+    media::VideoCaptureCapability caps;
+    caps.width = frame_size_.width();
+    caps.height = frame_size_.height();
+    caps.frame_rate = frame_rate_;
+    caps.color = media::VideoCaptureCapability::kARGB;
+    caps.expected_capture_delay =
+        base::Time::kMillisecondsPerSecond / frame_rate_;

[Wez, 2013/01/10 00:58:14]

Does it matter that the host platform might not be able to actually match this
rate?

[Sergey Ulanov, 2013/01/11 23:46:20]

I don't think so. In either case we have no way to know it in advance.

+    caps.interlaced = false;
+    event_handler_->OnFrameInfo(caps);

[Wez, 2013/01/10 00:58:14]

This calls the EventHandler on the capture thread - there's no mention of thread
restrictions in the VideoCaptureDevice API comments, so I suspect that that's
not safe.

[Sergey Ulanov, 2013/01/11 23:46:20]

That's the same behavior that we have in device video capturers. The
EventHandler implementation is supposed to handles this properly (see
content::VideoCaptureController).

+  }
+
+  if (!started_)

[Wez, 2013/01/10 00:58:14]

This is happening on the same thread that DoStop() gets processed on (capture
thread), so it'll always be true at this point, surely?

[Sergey Ulanov, 2013/01/11 23:46:20]

OnFrameInfo() needs to be called in response to Allocate() before Start() is
called, so DoCapture() is called from DoAllocate(), before DoStart().

+    return;

[Wez, 2013/01/10 00:58:14]

Check this before the |waiting_frame_size_| check - the EventHandler probably
doesn't expect to see OnFrameInfo() after it has Stop()ed the capturer.

[Sergey Ulanov, 2013/01/11 23:46:20]

See my previous comment. OnFrameInfo() doesn't depend on Start()/Stop()

+  size_t buffer_size =
+      frame_size_.width() * frame_size_.height() *
+      remoting::CaptureData::kBytesPerPixel;
+
+  if (capture_data->size() == frame_size_) {
+    // Invalidate resized frame buffer if we previously allocated it.

[Wez, 2013/01/10 00:58:14]

Reword this comment and place it before the if, e.g.

"If the captured frame matches the requested size, we don't need to resize it."

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+    resized_bitmap_.reset();
+    event_handler_->OnIncomingCapturedFrame(
+        capture_data->data(), buffer_size, base::Time::Now());
+    return;
+  }
+
+  // In case screen size has changed we need to resize the image. Resized image

[Wez, 2013/01/10 00:58:14]

Do we really have no way to inform the caller of the change in resolution?  How
is this handled for tab-capture?  It would be great to avoid all this scaling
code - for now I'd be fine with throwing OnError() in this case, to be honest.

[Sergey Ulanov, 2013/01/11 23:46:20]

No. The interface was designed for webcams and they don't change resolution
unless requested to.

+  // is stored to |resized_bitmap_|. Only regions of the screen that are
+  // changing are copied.
+
+  bool redraw_all = false;

[Wez, 2013/01/10 00:58:14]

Why not re-set the CaptureData's rects to contain a single full-frame rect and
avoid this special-case?

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+
+  if (resized_bitmap_.width() != frame_size_.width() ||
+      resized_bitmap_.height() != frame_size_.height()) {
+    resized_bitmap_.setConfig(SkBitmap::kARGB_8888_Config,
+                              frame_size_.width(), frame_size_.height());
+    resized_bitmap_.setIsOpaque(true);
+    resized_bitmap_.allocPixels();
+    redraw_all = true;
+  }
+
+  float scale_x = static_cast<float>(frame_size_.width()) /
+      capture_data->size().width();

[Wez, 2013/01/10 00:58:14]

Our capturers won't produce 0x0 frames, but if they _did_ this would give NaN...
Perhaps ignore frames w/ "empty" dimensions?

[Sergey Ulanov, 2013/01/11 23:46:20]

Added DCHECK on top of this method.

+  float scale_y = static_cast<float>(frame_size_.height()) /
+      capture_data->size().height();
+  float scale;
+  float x, y;
+  // Center the image in case aspect ratio is different.
+  if (scale_x > scale_y) {
+    scale = scale_y;
+    x = (scale_x - scale_y) / scale * frame_size_.width() / 2.0;
+    y = 0;

[Wez, 2013/01/10 00:58:14]

nit: 0.0f

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+  } else {
+    scale = scale_x;
+    x = 0;
+    y = (scale_y - scale_x) / scale * frame_size_.height() / 2.0;
+  }
+
+  SkDevice device(resized_bitmap_);
+  SkCanvas canvas(&device);
+  canvas.scale(scale, scale);

[Wez, 2013/01/10 00:58:14]

Add a comment before this block to summarize the process, i.e. that we wrap the
resized bitmap buffer in a Skia device and use a scaling canvas to draw to it,
etc.

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+
+  if (redraw_all) {
+    SkBitmap source_bitmap;
+    source_bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                            capture_data->size().width(),
+                            capture_data->size().height());
+    source_bitmap.setIsOpaque(true);
+    source_bitmap.setPixels(capture_data->data());
+    canvas.drawBitmap(source_bitmap, x / scale, y / scale, NULL);
+  } else {
+    int source_stride = capture_data->stride();
+    for (SkRegion::Iterator i(capture_data->dirty_region());
+         !i.done(); i.next()) {
+      SkBitmap source_bitmap;
+      source_bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                              i.rect().width(), i.rect().height(),
+                              source_stride);
+      source_bitmap.setIsOpaque(true);
+      source_bitmap.setPixels(
+          capture_data->data() + i.rect().top() * source_stride +
+          i.rect().left() * remoting::CaptureData::kBytesPerPixel);
+      canvas.drawBitmap(source_bitmap, i.rect().left() + x / scale,
+                        i.rect().top() + y / scale, NULL);

[Wez, 2013/01/10 00:58:14]

Does this do the right thing wrt sub-pixel destination coordinates?

[Sergey Ulanov, 2013/01/11 23:46:20]

Yes, as far as I know, but it uses simple nearest neighbour scaling.

+    }
+  }
+
+  event_handler_->OnIncomingCapturedFrame(
+      reinterpret_cast<uint8*>(resized_bitmap_.getPixels()), buffer_size,
+      base::Time::Now());
+}
+
+void ScreenCapturer::OnCursorShapeChanged(
+    scoped_ptr<remoting::MouseCursorShape> cursor_shape) {
+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+}
+
+void ScreenCapturer::DoAllocate(int frame_rate,
+                                EventHandler* event_handler) {
+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  frame_rate_ = frame_rate;
+  event_handler_ = event_handler;
+  if (!video_frame_capturer_)
+    video_frame_capturer_ = remoting::VideoFrameCapturer::Create();
+  if (video_frame_capturer_)
+    video_frame_capturer_->Start(this);
+  waiting_frame_size_ = true;

[Wez, 2013/01/10 00:58:14]

nit: If you need this, set it before Start()ing the capturer, so that it's clear
that it'll be true when the first frame is received.

[Wez, 2013/01/10 00:58:14]

nit: Add blank lines around frame_rate and event_handler init and before
DoCapture, to make this function more readable.

[Sergey Ulanov, 2013/01/11 23:46:20]

Start() doesn't really start anything. DoCapture() calls Capture() that actually
captures the first frame we use for OnFrameInfo() callback. I've added a comment
to make it clearer.

[Sergey Ulanov, 2013/01/11 23:46:20]

Done.

+  DoCapture();
+}
+
+void ScreenCapturer::DoStart() {
+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  started_ = true;
+  if (video_frame_capturer_) {
+    timer_.reset(new base::RepeatingTimer<ScreenCapturer>());
+    timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(1) / frame_rate_,
+                 base::Bind(&ScreenCapturer::DoCapture,
+                            base::Unretained(this)));
+  }
+}
+
+void ScreenCapturer::DoStop() {
+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  started_ = false;
+  if (video_frame_capturer_) {
+    timer_.reset();
+    video_frame_capturer_->Stop();

[wjia(left Chromium), 2013/01/07 22:15:26]

Is this an synchronous API? ScreenCapturer wouldn't receive any frame from
video_frame_capturer_ after Stop() is called.

[Sergey Ulanov, 2013/01/09 20:35:30]

Yes.

+  }
+}
+
+void ScreenCapturer::DoDeAllocate() {

[Wez, 2013/01/10 00:58:14]

ALso deallocate the resize buffer here.

[Sergey Ulanov, 2013/01/11 23:46:20]

Added it in DoStop.

+  DCHECK(capture_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  video_frame_capturer_.reset();
+  event_handler_ = NULL;
+  waiting_frame_size_ = false;
+  started_ = false;

[Wez, 2013/01/10 00:58:14]

We should already be stopped via Stop?

[Sergey Ulanov, 2013/01/11 23:46:20]

DeAllocate can be called without calling Stop().

+  capture_pending_ = true;

[Wez, 2013/01/10 00:58:14]

Why should capture_pending be true when deallocated?

[Sergey Ulanov, 2013/01/11 23:46:20]

Fixed now.

+}
+
+void ScreenCapturer::DoCapture() {
+  if (capture_pending_)
+    return;
+  capture_pending_ = true;
+  video_frame_capturer_->CaptureFrame();
+}
+
+}  // namespace content