Use texture-backed VideoFrane pipeline for Aura desktop capturing.

content/browser/media/capture/content_video_capture_device_core.cc

Index: content/browser/media/capture/content_video_capture_device_core.cc
diff --git a/content/browser/media/capture/content_video_capture_device_core.cc b/content/browser/media/capture/content_video_capture_device_core.cc
index b8fbe8d1b6ce80edf3c559732c8e49558b07341e..67ea197edbc239c0fcde67984091021218e99286 100644
--- a/content/browser/media/capture/content_video_capture_device_core.cc
+++ b/content/browser/media/capture/content_video_capture_device_core.cc
@@ -48,12 +48,19 @@ ThreadSafeCaptureOracle::ThreadSafeCaptureOracle(
       oracle_(oracle.Pass()),
       params_(params),
       capture_size_updated_(false) {
-  // Frame dimensions must each be an even integer since the client wants (or
-  // will convert to) YUV420.
-  capture_size_ = gfx::Size(
-      MakeEven(params.requested_format.frame_size.width()),
-      MakeEven(params.requested_format.frame_size.height()));
-  frame_rate_ = params.requested_format.frame_rate;
+  switch (params_.requested_format.pixel_format) {
+    case media::PIXEL_FORMAT_I420:
+      video_frame_format_ = media::VideoFrame::I420;
+      break;
+    case media::PIXEL_FORMAT_TEXTURE:
+      video_frame_format_ = media::VideoFrame::NATIVE_TEXTURE;
+      break;
+    default:
+      NOTREACHED() << "Invalid pixel_format "
+                   << params_.requested_format.pixel_format;
+      video_frame_format_ = media::VideoFrame::UNKNOWN;

[Ami GONE FROM CHROMIUM, 2014/04/01 18:03:38]

This needs to be handled elsewhere with some early-outs, no?

[hshi1, 2014/04/24 21:22:35]

I'll add a format check in the caller
(ContentVideoCaptureDeviceCore::AllocateAndStart) where I can early-out by
calling client->OnError.

In that case this "default" will never be reached. Do you want me to remove the
NOTREACHED() here?

[Ami GONE FROM CHROMIUM, 2014/04/24 22:02:38]

no code is best code so if you can delete it then that's great.
I suspect that you can't delete the default: clause without accounting for all
the enum values, so I think l.59-62 will turn into:
LOG(FATAL) << "Unpossible! " << params_.requested_format.pixel_format;

+      break;
+  }
 }
 
 ThreadSafeCaptureOracle::~ThreadSafeCaptureOracle() {}
@@ -69,7 +76,8 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     return false;  // Capture is stopped.
 
   scoped_refptr<media::VideoCaptureDevice::Client::Buffer> output_buffer =
-      client_->ReserveOutputBuffer(media::VideoFrame::I420, capture_size_);
+      client_->ReserveOutputBuffer(video_frame_format_,
+                                   params_.requested_format.frame_size);
   const bool should_capture =
       oracle_->ObserveEventAndDecideCapture(event, event_time);
   const bool content_is_dirty =
@@ -110,27 +118,30 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
   TRACE_EVENT_ASYNC_BEGIN2("mirroring", "Capture", output_buffer.get(),
                            "frame_number", frame_number,
                            "trigger", event_name);
-  *storage = media::VideoFrame::WrapExternalPackedMemory(
-      media::VideoFrame::I420,
-      capture_size_,
-      gfx::Rect(capture_size_),
-      capture_size_,
-      static_cast<uint8*>(output_buffer->data()),
-      output_buffer->size(),
-      base::SharedMemory::NULLHandle(),
-      base::TimeDelta(),
-      base::Closure());
+  // NATIVE_TEXTURE frames wrap a texture mailbox, which we don't have at the
+  // moment.  We do not construct those frames.
+  if (video_frame_format_ != media::VideoFrame::NATIVE_TEXTURE) {

[sheu, 2014/04/01 00:11:26]

This part is a little icky, in that we return (or don't return) a VideoFrame
based on whether the format is NATIVE_TEXTURE or not.  I'd like to return a
VideoFrame or not unconditionally; the problem is that here I can't construct a
NATIVE_TEXTURE frame yet, as we won't know the coded_size until the
CopyOutputRequest returns with the captured texture.

Suggestions?  Do you think it would make sense for CopyOutputRequest to be able
to specify a texture size, if we provide it a mailbox to capture into?

[Ami GONE FROM CHROMIUM, 2014/04/01 18:03:38]

Is it necessary to populate *storage here, considering the frame is passed to
DidCopyOutput anyway?
(IOW can we populate it in DCO() unconditionally)

[sheu, 2014/04/21 20:39:39]

This frame population uses a lot of state that is internal to the
ThreadSafeCaptureOracle.  We'd have to break out that state in a clean way to
populate the VideoFrame in DCO.

+    *storage = media::VideoFrame::WrapExternalPackedMemory(
+        video_frame_format_,
+        params_.requested_format.frame_size,
+        gfx::Rect(params_.requested_format.frame_size),
+        params_.requested_format.frame_size,
+        static_cast<uint8*>(output_buffer->data()),
+        output_buffer->size(),
+        base::SharedMemory::NULLHandle(),
+        base::TimeDelta(),
+        base::Closure());
+  }
   *callback = base::Bind(&ThreadSafeCaptureOracle::DidCaptureFrame,
                          this,
-                         output_buffer,
-                         *storage,
-                         frame_number);
+                         frame_number,
+                         output_buffer);
   return true;
 }
 
 gfx::Size ThreadSafeCaptureOracle::GetCaptureSize() const {
   base::AutoLock guard(lock_);
-  return capture_size_;
+  return params_.requested_format.frame_size;
 }
 
 void ThreadSafeCaptureOracle::UpdateCaptureSize(const gfx::Size& source_size) {
@@ -146,11 +157,11 @@ void ThreadSafeCaptureOracle::UpdateCaptureSize(const gfx::Size& source_size) {
         source_size.height() > params_.requested_format.frame_size.height()) {
       gfx::Rect capture_rect = media::ComputeLetterboxRegion(
           gfx::Rect(params_.requested_format.frame_size), source_size);
-      capture_size_ = gfx::Size(MakeEven(capture_rect.width()),
-                                MakeEven(capture_rect.height()));
+      params_.requested_format.frame_size.SetSize(
+          MakeEven(capture_rect.width()), MakeEven(capture_rect.height()));
     } else {
-      capture_size_ = gfx::Size(MakeEven(source_size.width()),
-                                MakeEven(source_size.height()));
+      params_.requested_format.frame_size.SetSize(
+          MakeEven(source_size.width()), MakeEven(source_size.height()));
     }
     capture_size_updated_ = true;
   }
@@ -168,9 +179,9 @@ void ThreadSafeCaptureOracle::ReportError(const std::string& reason) {
 }
 
 void ThreadSafeCaptureOracle::DidCaptureFrame(
+    int frame_number,
     const scoped_refptr<media::VideoCaptureDevice::Client::Buffer>& buffer,
     const scoped_refptr<media::VideoFrame>& frame,
-    int frame_number,
     base::TimeTicks timestamp,
     bool success) {
   base::AutoLock guard(lock_);
@@ -183,12 +194,9 @@ void ThreadSafeCaptureOracle::DidCaptureFrame(
 
   if (success) {
     if (oracle_->CompleteCapture(frame_number, timestamp)) {
-      client_->OnIncomingCapturedVideoFrame(
-          buffer,
-          media::VideoCaptureFormat(
-              capture_size_, frame_rate_, media::PIXEL_FORMAT_I420),
-          frame,
-          timestamp);
+      media::VideoCaptureFormat format = params_.requested_format;
+      format.frame_size = frame->coded_size();
+      client_->OnIncomingCapturedVideoFrame(buffer, format, frame, timestamp);
     }
   }
 }
@@ -211,14 +219,12 @@ void ContentVideoCaptureDeviceCore::AllocateAndStart(
     return;
   }
 
-  if (params.requested_format.frame_size.width() < kMinFrameWidth ||
-      params.requested_format.frame_size.height() < kMinFrameHeight) {
-    std::string error_msg =
-        "invalid frame size: " + params.requested_format.frame_size.ToString();
-    DVLOG(1) << error_msg;
-    client->OnError(error_msg);
-    return;
-  }
+  media::VideoCaptureParams new_params = params;
+  // Frame dimensions must each be an even integer since the client wants (or
+  // will convert to) YUV420.
+  new_params.requested_format.frame_size.SetSize(
+      MakeEven(params.requested_format.frame_size.width()),
+      MakeEven(params.requested_format.frame_size.height()));
 
   base::TimeDelta capture_period = base::TimeDelta::FromMicroseconds(
       1000000.0 / params.requested_format.frame_rate + 0.5);
@@ -227,16 +233,17 @@ void ContentVideoCaptureDeviceCore::AllocateAndStart(
       new VideoCaptureOracle(capture_period,
                              kAcceleratedSubscriberIsSupported));
   oracle_proxy_ =
-      new ThreadSafeCaptureOracle(client.Pass(), oracle.Pass(), params);
+      new ThreadSafeCaptureOracle(client.Pass(), oracle.Pass(), new_params);
 
   // Starts the capture machine asynchronously.
   BrowserThread::PostTaskAndReplyWithResult(
-      BrowserThread::UI, FROM_HERE,
+      BrowserThread::UI,
+      FROM_HERE,
       base::Bind(&VideoCaptureMachine::Start,
                  base::Unretained(capture_machine_.get()),
-                 oracle_proxy_),
-      base::Bind(&ContentVideoCaptureDeviceCore::CaptureStarted,
-                 AsWeakPtr()));
+                 oracle_proxy_,
+                 new_params),
+      base::Bind(&ContentVideoCaptureDeviceCore::CaptureStarted, AsWeakPtr()));
 
   TransitionStateTo(kCapturing);
 }