PPAPI: implement GetSupportedPreviewSizes

content/renderer/pepper/pepper_platform_image_capture.cc

Index: content/renderer/pepper/pepper_platform_image_capture.cc
diff --git a/content/renderer/pepper/pepper_platform_video_capture.cc b/content/renderer/pepper/pepper_platform_image_capture.cc
similarity index 52%
copy from content/renderer/pepper/pepper_platform_video_capture.cc
copy to content/renderer/pepper/pepper_platform_image_capture.cc
index 0d011f3fdc4217392897e9975df7cadd9f6eb66e..2e4c4295c4807f427ce1c2b7980f96041a38c1c4 100644
--- a/content/renderer/pepper/pepper_platform_video_capture.cc
+++ b/content/renderer/pepper/pepper_platform_image_capture.cc
@@ -1,27 +1,29 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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 "content/renderer/pepper/pepper_platform_video_capture.h"
+#include "content/renderer/pepper/pepper_platform_image_capture.h"
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "content/renderer/media/video_capture_impl_manager.h"
+#include "content/renderer/pepper/pepper_image_capture_host.h"
 #include "content/renderer/pepper/pepper_media_device_manager.h"
-#include "content/renderer/pepper/pepper_video_capture_host.h"
 #include "content/renderer/render_frame_impl.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/base/bind_to_current_loop.h"
+#include "content/renderer/pepper/gfx_conversion.h"
 #include "url/gurl.h"
 
 namespace content {
 
-PepperPlatformVideoCapture::PepperPlatformVideoCapture(
+PepperPlatformImageCapture::PepperPlatformImageCapture(
     int render_frame_id,
     const std::string& device_id,
     const GURL& document_url,
-    PepperVideoCaptureHost* handler)
+    PepperImageCaptureHost* handler)
     : render_frame_id_(render_frame_id),
       device_id_(device_id),
       session_id_(0),
@@ -34,47 +36,33 @@ PepperPlatformVideoCapture::PepperPlatformVideoCapture(
   PepperMediaDeviceManager* const device_manager = GetMediaDeviceManager();
   if (device_manager) {
     pending_open_device_id_ = device_manager->OpenDevice(
-        PP_DEVICETYPE_DEV_VIDEOCAPTURE,
+        PP_DEVICETYPE_DEV_VIDEOCAPTURE,  // TODO(jchuang): add new type?

[wuchengli, 2015/01/19 14:05:03]

I don't think we need to add a new type. Remove TODO.

[Justin Chuang, 2015/01/26 15:00:50]

Done.

         device_id,
         document_url,
-        base::Bind(&PepperPlatformVideoCapture::OnDeviceOpened,
+        base::Bind(&PepperPlatformImageCapture::OnDeviceOpened,
                    weak_factory_.GetWeakPtr()));
     pending_open_device_ = true;
   }
 }
 
-void PepperPlatformVideoCapture::StartCapture(
-    const media::VideoCaptureParams& params) {
+void PepperPlatformImageCapture::GetPreviewSizes() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!stop_capture_cb_.is_null())
-    return;
+
   VideoCaptureImplManager* manager =
       RenderThreadImpl::current()->video_capture_impl_manager();
-  stop_capture_cb_ =
-      manager->StartCapture(session_id_,
-                            params,
-                            media::BindToCurrentLoop(base::Bind(
-                                &PepperPlatformVideoCapture::OnStateUpdate,
-                                weak_factory_.GetWeakPtr())),
-                            media::BindToCurrentLoop(base::Bind(
-                                &PepperPlatformVideoCapture::OnFrameReady,
-                                weak_factory_.GetWeakPtr())));
-}
-
-void PepperPlatformVideoCapture::StopCapture() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (stop_capture_cb_.is_null())
-    return;
-  stop_capture_cb_.Run();
-  stop_capture_cb_.Reset();
+  manager->GetDeviceSupportedFormats(
+      session_id_,
+      media::BindToCurrentLoop(
+          base::Bind(
+              &PepperPlatformImageCapture::OnDeviceSupportedFormatsEnumerated,
+              weak_factory_.GetWeakPtr())));
 }
 
-void PepperPlatformVideoCapture::DetachEventHandler() {
+void PepperPlatformImageCapture::DetachEventHandler() {
   handler_ = NULL;
-  StopCapture();
+
   if (!release_device_cb_.is_null()) {
-    release_device_cb_.Run();
-    release_device_cb_.Reset();
+    base::ResetAndReturn(&release_device_cb_).Run();
   }
   if (!label_.empty()) {
     PepperMediaDeviceManager* const device_manager = GetMediaDeviceManager();
@@ -91,16 +79,20 @@ void PepperPlatformVideoCapture::DetachEventHandler() {
   }
 }
 
-PepperPlatformVideoCapture::~PepperPlatformVideoCapture() {
-  DCHECK(stop_capture_cb_.is_null());
+PepperPlatformImageCapture::~PepperPlatformImageCapture() {
   DCHECK(release_device_cb_.is_null());
   DCHECK(label_.empty());
   DCHECK(!pending_open_device_);
 }
 
-void PepperPlatformVideoCapture::OnDeviceOpened(int request_id,
+void PepperPlatformImageCapture::OnDeviceOpened(int request_id,
                                                 bool succeeded,
                                                 const std::string& label) {
+  if (!handler_) {
+    NOTREACHED();

[wuchengli, 2015/01/19 14:05:03]

I'd prefer following PepperPlatformVideoCapture::OnDeviceOpened unless you are
sure about this. Otherwise remove this if check and add if (handler_) in line
109.

[Justin Chuang, 2015/01/26 15:00:50]

I think the code is correct. handler_ is deregistered immediately before
PepperPlatformImageCapture is destroyed in upper layer. No harm to add
NONREACHED() here.  But I'm also okay to remove it since it shouldn't assert.

[wuchengli, 2015/01/29 13:52:14]

You are right. PepperPlatformImageCapture::DetachEventHandler set |handler_| to
NULL and call device_manager->CancelOpenDevice. This shouldn't happen. Let's
change it to DCHECK(handler_);

[Justin Chuang, 2015/02/02 08:43:44]

Done.

+    return;
+  }
+
   pending_open_device_ = false;
   pending_open_device_id_ = -1;
 
@@ -109,50 +101,37 @@ void PepperPlatformVideoCapture::OnDeviceOpened(int request_id,
   if (succeeded) {
     label_ = label;
     session_id_ = device_manager->GetSessionID(
-        PP_DEVICETYPE_DEV_VIDEOCAPTURE, label);
+        PP_DEVICETYPE_DEV_VIDEOCAPTURE, label);  // TODO: new device type?

[wuchengli, 2015/01/19 14:05:03]

Remove TODO.

[Justin Chuang, 2015/01/26 15:00:50]

Done.

     VideoCaptureImplManager* manager =
         RenderThreadImpl::current()->video_capture_impl_manager();
     release_device_cb_ = manager->UseDevice(session_id_);
   }
 
-  if (handler_)
-    handler_->OnInitialized(succeeded);
+  handler_->OnPlatformInitialized(succeeded);
 }
 
-void PepperPlatformVideoCapture::OnStateUpdate(VideoCaptureState state) {
-  if (!handler_)
+void PepperPlatformImageCapture::OnDeviceSupportedFormatsEnumerated(
+    const media::VideoCaptureFormats& formats) {
+  if (!handler_) {
+    NOTREACHED();

[wuchengli, 2015/01/19 14:05:03]

PepperPlatformVideoCapture::OnStateUpdate doesn't have NOTREACHED(). Is it
possible PepperImageCaptureHost::Close() is called before
OnDeviceSupportedFormatsEnumerated arrives?

[Justin Chuang, 2015/01/26 15:00:50]

dittos. The pointer to this object is passed as weak pointer.  And it should be
invoked in the same loop.

[wuchengli, 2015/01/29 13:52:14]

That's correct. Then this can be changed to DCHECK(handler_);

[Justin Chuang, 2015/02/02 08:43:44]

Done.

     return;
-  switch (state) {
-    case VIDEO_CAPTURE_STATE_STARTED:
-      handler_->OnStarted();
-      break;
-    case VIDEO_CAPTURE_STATE_STOPPED:
-      handler_->OnStopped();
-      break;
-    case VIDEO_CAPTURE_STATE_PAUSED:
-      handler_->OnPaused();
-      break;
-    case VIDEO_CAPTURE_STATE_ERROR:
-      handler_->OnError();
-      break;
-    default:
-      NOTREACHED() << "Unexpected state: " << state << ".";
   }
-}
 
-void PepperPlatformVideoCapture::OnFrameReady(
-    const scoped_refptr<media::VideoFrame>& frame,
-    const media::VideoCaptureFormat& format,
-    const base::TimeTicks& estimated_capture_time) {
-  if (handler_ && !stop_capture_cb_.is_null())
-    handler_->OnFrameReady(frame, format);
+  // TODO(jchuang): Do we need fallback if the device doesn't support format

[wuchengli, 2015/01/19 14:05:03]

Remove TODO. VideoCaptureImpl::GetDeviceSupportedFormats doesn't say some
devices won't supports this. Also, this API will only be used by CrOS, which
supports GetDeviceSupportedFormats.

[Justin Chuang, 2015/01/26 15:00:50]

Why it won't happen on CrOS? It depends on how the camera module implements UVC.

Anyway, I'll remove the TODO, and let's discuss error handling of format
enumeration in pepper_image_capture.cc

+  // enumeration (when formats.size() == 0)?
+  std::vector<PP_Size> sizes;
+  for (media::VideoCaptureFormats::const_iterator it = formats.begin();
+       it != formats.end(); it++) {
+    sizes.push_back(PP_FromGfxSize(it->frame_size));

[wuchengli, 2015/01/19 14:05:03]

We should du-dup here. Right? Some frame sizes will be the same.

[Justin Chuang, 2015/01/26 15:00:50]

Thanks. Can you paste result with dup? I don't see it on my webcam.

[wuchengli, 2015/01/29 13:52:14]

Sorry. By dup I meant same frame size but different fps or same frame size but
different format.
VideoCaptureDeviceFactoryLinux::GetDeviceSupportedFormats adds all combination
of format * size * fps. I'm sure some devices have formats like 640x480 30fps
and 640x380 15fps. We should ask Michael what he needs. I'll send an email.

https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...

+  }
+  handler_->OnPlatformPreviewSizesEnumerated(sizes);
 }
 
-PepperMediaDeviceManager* PepperPlatformVideoCapture::GetMediaDeviceManager() {
+PepperMediaDeviceManager* PepperPlatformImageCapture::GetMediaDeviceManager() {
   RenderFrameImpl* const render_frame =
       RenderFrameImpl::FromRoutingID(render_frame_id_);
-  return render_frame ?
-      PepperMediaDeviceManager::GetForRenderFrame(render_frame).get() : NULL;
+  return render_frame
+      ? PepperMediaDeviceManager::GetForRenderFrame(render_frame).get() : NULL;
 }
 
 }  // namespace content