PPAPI: implement GetSupportedPreviewSizes

content/renderer/pepper/pepper_image_capture_host.cc

Index: content/renderer/pepper/pepper_image_capture_host.cc
diff --git a/content/renderer/pepper/pepper_image_capture_host.cc b/content/renderer/pepper/pepper_image_capture_host.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d6f04f9d4eff9d4f1a9cdf445555bc238bc4960d
--- /dev/null
+++ b/content/renderer/pepper/pepper_image_capture_host.cc
@@ -0,0 +1,134 @@
+// 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_image_capture_host.h"
+
+#include "content/renderer/pepper/pepper_media_device_manager.h"

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

is this used?

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

Done.

+#include "content/renderer/pepper/pepper_platform_image_capture.h"
+#include "content/renderer/pepper/renderer_ppapi_host_impl.h"
+#include "content/renderer/render_frame_impl.h"

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

is this used?

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

yes, must be kept

+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/proxy/ppapi_messages.h"
+
+namespace content {
+
+PepperImageCaptureHost::PepperImageCaptureHost(RendererPpapiHostImpl* host,
+                                               PP_Instance instance,
+                                               PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      renderer_ppapi_host_(host) {
+}
+
+PepperImageCaptureHost::~PepperImageCaptureHost() {
+  Close();
+}
+
+bool PepperImageCaptureHost::Init() {
+  return !!renderer_ppapi_host_->GetPluginInstance(pp_instance());
+}
+
+int32_t PepperImageCaptureHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  int32_t result = PP_ERROR_FAILED;
+
+  PPAPI_BEGIN_MESSAGE_MAP(PepperImageCaptureHost, msg)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_ImageCapture_Open, OnOpen)

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

Most other pepper host use 2 space indentation.

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

Done.

+  PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_ImageCapture_Close, OnClose)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(
+      PpapiPluginMsg_ImageCapture_GetPreviewSizes, OnGetPreviewSizes)
+  PPAPI_END_MESSAGE_MAP()
+  return result;
+}
+
+void PepperImageCaptureHost::OnPlatformInitialized(bool succeeded) {

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

s/OnPlatformInitialized/OnInitialized/. I'd prefer being consistent with
PepperVideoCaptureHost. 

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

Done.

+  if (!open_reply_context_.is_valid())
+    return;
+
+  if (succeeded) {
+    open_reply_context_.params.set_result(PP_OK);
+  } else {
+    DetachPlatformImageCapture();
+    open_reply_context_.params.set_result(PP_ERROR_FAILED);
+  }
+
+  host()->SendReply(open_reply_context_,
+                    PpapiPluginMsg_ImageCapture_OpenReply());
+  open_reply_context_ = ppapi::host::ReplyMessageContext();
+}
+
+void PepperImageCaptureHost::OnPlatformPreviewSizesEnumerated(
+    const std::vector<PP_Size>& sizes) {
+  if (!preview_sizes_reply_context_.is_valid())
+    return;
+

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

Do we need to check platform_image_capture_ != NULL here? Is it possible someone
calls Close before preview sizes are enumerated?

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

No. Because the function is called directly in platform_image_capture_ in the
same thread, it won't be NULL, and we've got all data we need in this callback.

If the platform object is closed, the weak pointer in lower layer will become
NULL. Callback won't be invoked.

+  if (sizes.size() > 0)

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

GetDeviceSupportedFormats doesn't say about size == 0 case. I think we should
return PP_OK even when sizes.size() == 0.

https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

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

Yes, but you can see code in media stream treat it as a failed case.
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

I tend to treat it as failed because it is supposed to return a list of
supported resolutions. If it doesn't, then it's error. But it is open question
either higher or lower layer should return PP_ERROR.

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

Agreed to have PP_ERROR here. This API won't work without formats enumeration
support and a camera should have at least one preview size.

From https://codereview.chromium.org/195363002, Mac QTKit and Win DirectShow do
not support formats enumeration. I asked Pawel. On Chrome OS, VIDIOC_ENUM_FMT is
required to be supported.

+    preview_sizes_reply_context_.params.set_result(PP_OK);
+  else
+    preview_sizes_reply_context_.params.set_result(PP_ERROR_FAILED);
+  host()->SendReply(preview_sizes_reply_context_,
+                    PpapiPluginMsg_ImageCapture_GetPreviewSizesReply(sizes));
+  preview_sizes_reply_context_ = ppapi::host::ReplyMessageContext();
+}
+
+int32_t PepperImageCaptureHost::OnOpen(ppapi::host::HostMessageContext* context,
+                                       const std::string& device_id) {
+  if (open_reply_context_.is_valid())
+    return PP_ERROR_INPROGRESS;
+
+  if (platform_image_capture_.get())
+    return PP_ERROR_FAILED;
+
+  GURL document_url = renderer_ppapi_host_->GetDocumentURL(pp_instance());
+  if (!document_url.is_valid())
+    return PP_ERROR_FAILED;
+
+  platform_image_capture_.reset(new PepperPlatformImageCapture(

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

How about using PepperPlatformVideoCapture? We'll only need one API --
GetSupportedPreviewSizes. We can also save some code.

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

Good point. I prefer to put it in PepperPlatformImageCapture so it's possible to
add more Image Capture PPAPI in the future. 
If we don't plan to extend Image Capture PPAPI, we can put "get_resolution_list"
API in Media Stream PPAPI instead. 

In addition, I expect both Image Capture and Video Capture will be converged in
video_capture_impl_manager layer (one layer downward), only the PPAPI host are
different. So there won't be a lot of duplicated code I think.

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

Using PepperPlatformVideoCapture in this CL doesn't mean we cannot add
PepperPlatformImageCapture in the future. Usually a changelist should be
self-contained and add only the necessary code that are used now. If some code
are not used now, they should be added in the future. Having said that, I'm fine
with using PepperPlatformImageCapture in this CL. It's up to you.

+      renderer_ppapi_host_->GetRenderFrameForInstance(pp_instance())
+          ->GetRoutingID(),
+      device_id,
+      document_url,
+      this));
+
+  open_reply_context_ = context->MakeReplyMessageContext();
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperImageCaptureHost::OnClose(
+    ppapi::host::HostMessageContext* context) {
+  return Close();
+}
+
+int32_t PepperImageCaptureHost::OnGetPreviewSizes(
+    ppapi::host::HostMessageContext* context) {
+  if (preview_sizes_reply_context_.is_valid())
+    return PP_ERROR_INPROGRESS;
+
+  if (!platform_image_capture_.get())
+    return PP_ERROR_FAILED;
+
+  preview_sizes_reply_context_ = context->MakeReplyMessageContext();
+  platform_image_capture_->GetPreviewSizes();
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperImageCaptureHost::Close() {

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

Close and DetachPlatformImageCapture are the same. Remove Close and let others
call DetachPlatformImageCapture directly.

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

When we support CaptureStillImage, then it will become two separate functions
because we need to handle operation abortion. I think it's okay to have two
functions look the same for now. It doesn't cause confusions, IMO.

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

This is the same problem like the previous comment. We should only look at the
code at this moment. If something is necessary for supporting CaptureStillImage,
it should be added in that CL, not this one. From this CL, Close and
DetachPlatformImageCapture are the same and one of them should be removed.

+  if (!platform_image_capture_.get())
+    return PP_OK;
+
+  // TODO(jchuang): abort pending image capture.
+  DetachPlatformImageCapture();
+  return PP_OK;
+}
+
+void PepperImageCaptureHost::DetachPlatformImageCapture() {
+  if (platform_image_capture_) {
+    platform_image_capture_->DetachEventHandler();
+    platform_image_capture_.reset();
+  }
+}
+
+}  // namespace content