PPAPI: implement GetSupportedPreviewSizes

ppapi/api/private/ppb_image_capture_private.idl

Index: ppapi/api/private/ppb_image_capture_private.idl
diff --git a/ppapi/api/private/ppb_image_capture_private.idl b/ppapi/api/private/ppb_image_capture_private.idl
index 28432af5ddea1018e1e6f50ffd1077975df2f703..d3946fa93f9ff5f9fd620dccdd3fa6e2947ab473 100644
--- a/ppapi/api/private/ppb_image_capture_private.idl
+++ b/ppapi/api/private/ppb_image_capture_private.idl
@@ -11,7 +11,7 @@
 [generate_thunk]
 
 label Chrome {
-  M39 = 0.1
+  M42 = 0.1
 };
 
 /**
@@ -22,14 +22,11 @@ label Chrome {
  * actual data is available.
  *
  * Parameters:
- *  |user_data| The same pointer that was passed into <code>
- *  PPB_ImageCapture_Private.Create()</code>.
  *  |sequence_id| The sequence ID of the image capture, same as the one from
  *  CaptureStillImage.
  */
 typedef void PPB_ImageCapture_Private_ShutterCallback(
-    [inout] mem_t user_data,
-    [in] int64_t sequence_id);
+    [in] uint64_t sequence_id);
 
 /**
  * Callback function for <code>PPB_ImageCapture_Private.CaptureStillImage
@@ -38,16 +35,13 @@ typedef void PPB_ImageCapture_Private_ShutterCallback(
  * </code> for more information.
  *
  * Parameters:
- *  |user_data| The same pointer that was passed into <code>
- *  PPB_ImageCapture_Private.Create()</code>.
  *  |sequence_id| The sequence ID of the image capture, same as the one from
  *  CaptureStillImage.
  *  |preview| A <code>PP_Resource</code> corresponding to a VideoFrame
  *  resource used to store the preview image.
  */
 typedef void PPB_ImageCapture_Private_PreviewCallback(
-    [inout] mem_t user_data,
-    [in] int64_t sequence_id,
+    [in] uint64_t sequence_id,
     [in] PP_Resource preview);
 
 /**
@@ -56,16 +50,13 @@ typedef void PPB_ImageCapture_Private_PreviewCallback(
  * PPB_ImageCapture_Private.CaptureStillImage</code> for more information.
  *
  * Parameters:
- *  |user_data| The same pointer that was passed into <code>
- *  PPB_ImageCapture_Private.Create()</code>.
  *  |sequence_id| The sequence ID of the image capture, same as the one from
  *  CaptureStillImage.
  *  |jpeg| A <code>PP_Resource</code> corresponding to a VideoFrame
  *  resource used to store the JPEG image.
  */
 typedef void PPB_ImageCapture_Private_JpegCallback(
-    [inout] mem_t user_data,
-    [in] int64_t sequence_id,
+    [in] uint64_t sequence_id,
     [in] PP_Resource jpeg);
 
 /**
@@ -73,28 +64,26 @@ typedef void PPB_ImageCapture_Private_JpegCallback(
  * </code> to indicate the image capture has failed.
  *
  * Parameters:
- *  |user_data| The same pointer that was passed into <code>
- *  PPB_ImageCapture_Private.Create()</code>.
  *  |sequence_id| The sequence ID of the image capture, same as the one from
  *  CaptureStillImage.
  *  |int32_t| An error code from <code>pp_errors.h</code>.
  */
 typedef void PPB_ImageCapture_Private_ErrorCallback(
-    [inout] mem_t user_data,
-    [in] int64_t sequence_id,
+    [in] uint64_t sequence_id,
     [in] int32_t pp_error);
 
 /**
  * To capture a still image with this class, use the following steps.
  * 1. Get a PPB_ImageCapture_Private object by Create().
- * 2. Call GetCameraCapabilities to get the supported preview sizes.
- * 3. For optimal performance, set one of the supported preview size as the
+ * 2. Open() the device with camera source ID.
+ * 3. Call GetCameraCapabilities() to get the supported preview sizes.
+ * 4. For optimal performance, set one of the supported preview size as the
  *    constraints of getUserMedia. Use the created MediaStreamVideoTrack for
  *    camera previews.
- * 4. Set the same preview size and other settings by SetConfig.
- * 5. Call CaptureStillImage to capture a still image. Play the shutter sound in
- *    the shutter callback. The image from the preview callback can be used for
- *    display. JPEG image will be returned to the JPEG callback.
+ * 5. Set the same preview size and other settings by SetConfig().
+ * 6. Call CaptureStillImage() to capture a still image. Play the shutter sound
+ *    in the shutter callback. The image from the preview callback can be used
+ *    for displaying preview. JPEG image will be returned by the JPEG callback.
  */
 interface PPB_ImageCapture_Private {
   /**
@@ -102,23 +91,11 @@ interface PPB_ImageCapture_Private {
    *
    * @param[in] instance A <code>PP_Instance</code> identifying one instance
    * of a module.
-   * @param[in] camera_source_id A <code>PP_Var</code> identifying a camera
-   * source. The type is string. The ID can be obtained from
-   * MediaStreamTrack.getSources() or MediaStreamVideoTrack.id. If a
-   * MediaStreamVideoTrack is associated with the same source and the track
-   * is closed, this PPB_ImageCapture_Private object can still do image capture.
-   * @param[in] error_callback A <code>PPB_ImageCapture_Private_ErrorCallback
-   * </code> callback to indicate the image capture has failed.
-   * @param[inout] user_data An opaque pointer that will be passed to the
-   * callbacks of PPB_ImageCapture_Private.
    *
    * @return A <code>PP_Resource</code> corresponding to a
    * PPB_ImageCapture_Private resource if successful, 0 if failed.
    */
-  PP_Resource Create([in] PP_Instance instance,
-                     [in] PP_Var camera_source_id,
-                     [in] PPB_ImageCapture_Private_ErrorCallback error_callback,
-                     [inout] mem_t user_data);
+  PP_Resource Create([in] PP_Instance instance);
 
   /**
    * Determines if a resource is an image capture resource.
@@ -132,6 +109,26 @@ interface PPB_ImageCapture_Private {
   PP_Bool IsImageCapture([in] PP_Resource resource);
 
   /**
+   * Opens a video capture device.
+   *
+   * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
+   * image capture resource.
+   * @param[in] device_id A <code>PP_Var</code> identifying a camera device. The
+   * type is string. The ID can be obtained from MediaStreamTrack.getSources()
+   * or MediaStreamVideoTrack.id. If a MediaStreamVideoTrack is associated with
+   * the same source and the track is closed, this PPB_ImageCapture_Private
+   * object can still do image capture.
+   * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
+   * completion of <code>Open()</code>.
+   *
+   * @return An error code from <code>pp_errors.h</code>.
+   */
+  int32_t Open(
+      [in] PP_Resource image_capture,
+      [in] PP_Var device_id,
+      [in] PP_CompletionCallback callback);
+
+  /**
    * Disconnects from the camera and cancels all pending capture requests.
    * After this returns, no callbacks will be called. If <code>
    * PPB_ImageCapture_Private</code> is destroyed and is not closed yet, this
@@ -140,13 +137,8 @@ interface PPB_ImageCapture_Private {
    *
    * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
    * image capture resource.
-   * @param[in] callback <code>PP_CompletionCallback</code> to be called upon
-   * completion of <code>Close()</code>.
-   *
-   * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    */
-  int32_t Close([in] PP_Resource resource,
-                [in] PP_CompletionCallback callback);
+  void Close([in] PP_Resource image_capture);
 
   /**
    * Sets the configuration of the image capture.
@@ -156,7 +148,8 @@ interface PPB_ImageCapture_Private {
    * image capture resource.
    * @param[in] config A <code>PP_ImageCaptureConfig_Private</code> object.
    * @param[in] callback <code>PP_CompletionCallback</code> to be called upon
-   * completion of <code>SetConfig()</code>.
+   * completion of <code>SetConfig()</code>. This callback will only run if the
+   * return code is <code>PP_OK_COMPLETIONPENDING</code>.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    * Returns <code>PP_ERROR_INPROGRESS</code> if there is a pending call of
@@ -176,7 +169,8 @@ interface PPB_ImageCapture_Private {
    * the current image capture config on success. Otherwise, the values will not
    * be changed.
    * @param[in] callback <code>PP_CompletionCallback</code> to be called upon
-   * completion of <code>GetConfig()</code>.
+   * completion of <code>GetConfig()</code>. This callback will only run if the
+   * return code is <code>PP_OK_COMPLETIONPENDING</code>.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    */
@@ -195,7 +189,8 @@ interface PPB_ImageCapture_Private {
    * storing the image capture capabilities on success. Otherwise, the value
    * will not be changed.
    * @param[in] callback <code>PP_CompletionCallback</code> to be called upon
-   * completion of <code>GetCameraCapabilities()</code>.
+   * completion of <code>GetCameraCapabilities()</code>. This callback will only
+   * run if the return code is <code>PP_OK_COMPLETIONPENDING</code>.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    */
@@ -204,6 +199,16 @@ interface PPB_ImageCapture_Private {
                                 [in] PP_CompletionCallback callback);
 
   /**
+   * Allows the browser to reuse preview buffer and JPEG buffer that was
+   * previously sent by |preview_calback| and |jpeg_callback| registered in
+   * <code>PPB_ImageCapture_Private.CaptureStillImage()</code>.
+   *
+   * @return An int32_t containing a result code from <code>pp_errors.h</code>.
+   */
+  int32_t ReuseBuffers(
+      [in] PP_Resource image_capture);

[dmichael (off chromium), 2015/01/28 00:00:35]

I would rather this worked like our existing ImageData cache. When the plugin
finishes with an ImageData resource, they just de-reference it and it
automatically goes in to a cache for possible reuse. In that case, you wouldn't
need this function. (Admittedly, this is different because the browser is giving
the plugin an ImageData, rather than the other way around...  so the existing
ImageDataCache might not work. )

[wuchengli, 2015/01/28 08:51:42]

JPEG size is dynamic. If we want to make plugin give browser an ImageData, we
need another API to get the maximum JPEG size from the driver because the max
will be different for different driver. We'll also need to pass two ImageData to
CaptureStillImage.

The benefits of ImageData cache is to allow multiple Jpeg ImagaData.
ReuseBuffers assumes only one JPEG buffer at a time.

In the future (not gonna happen any time soon), ImageData can be backed by a
different memory that can be accessed by the driver efficiently, e.g. dmabuf. So
we may want to allocate the memory and decide the type in the implementation,
not in the plugin.

Justin. This change should belong to another CL. This CL should only contain
GetSupportedPreviewSizes. Also, FYI, this is ImageDataCache Dave talked about.
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppb_im...

https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppb_im...

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

Will move this change to a separate CL.

[Justin Chuang, 2015/02/02 14:29:24]

Move the changes and discussions to 887403003

+
+  /**
    * Captures a still JPEG image from the camera.
    *
    * Triggers an asynchronous image capture. The camera will initiate a series
@@ -214,9 +219,8 @@ interface PPB_ImageCapture_Private {
    * image has been captured. The preview callback occurs when a scaled, fully
    * processed preview image is available. The JPEG callback occurs when the
    * compressed image is available. If there is an error after the capture is in
-   * progress, the error callback passed to <code>
-   * PPB_ImageCapture_Private.Create()</code> will be invoked. All the callbacks
-   * are invoked by the thread that calls this function.
+   * progress, the error callback will be invoked. All the callbacks are invoked
+   * by the thread that calls this function.

[dmichael (off chromium), 2015/01/28 00:00:35]

It's probably worth noting that there must be a PPB_MessageLoop bound to this
thread that will run to dispatch the callbacks.

[Justin Chuang, 2015/02/02 14:29:24]

Move the changes and discussions to 887403003

    *
    * The size of the preview image in preview callback is determined by
    * <code>PPB_ImageCaptureConfig_Private.SetPreviewSize</code>. The format is
@@ -242,6 +246,8 @@ interface PPB_ImageCapture_Private {
    * @param[in] jpeg_callback A <code>
    * PPB_ImageCapture_Private_JpegCallback</code> callback to return captured
    * JPEG image.
+   * @param[in] error_callback A <code>PPB_ImageCapture_Private_ErrorCallback
+   * </code> callback to indicate <code>CaptureStillImage()</code> has failed.
    * @param[out] sequence_id The sequence ID is a unique monotonically
    * increasing value starting from 0, incremented every time a new request like
    * image capture is submitted.
@@ -255,5 +261,6 @@ interface PPB_ImageCapture_Private {
       [in] PPB_ImageCapture_Private_ShutterCallback shutter_callback,
       [in] PPB_ImageCapture_Private_PreviewCallback preview_callback,
       [in] PPB_ImageCapture_Private_JpegCallback jpeg_callback,
-      [out] int64_t sequence_id);
+      [in] PPB_ImageCapture_Private_ErrorCallback error_callback,

[dmichael (off chromium), 2015/01/28 00:00:35]

Can you look at the way PPB_Messaging::RegisterMessageLoop and
PPP_MessageHandler work?

I'm afraid you might find what you have too limiting, because there's no way to
specify what thread the callbacks should be invoked on.

You might want to have two parameters here to replace the callback function
pointers:
const PPP_ImageCapture_Private* callbacks,
PP_Resource message_loop
|callbacks| is a struct containing all the callback function pointers,
message_loop is a PPB_MessageLoop resource where you want the calls to be
invoked.

[wuchengli, 2015/01/28 08:51:42]

This was an unfinished discussion in a previous CL when you were OOO. :) The
benefits of PPP_ImageCapture is that plugin doesn't need to declare lots of
callbacks. On the other hand, the current method allows the plugin only
implementing the callbacks it needs. PPB_Audio_Callback is also a callback.
Though we have lots of callbacks and probably we should change to
PPP_ImageCapture. I'm OK with either way.

Agree with specifying a thread to invoke on. This should belong to another CL.

[dmichael (off chromium), 2015/01/29 19:22:00]

You could do that as well by letting the user provide a NULL pointer for any
entry in the struct they don't want to implement.
Right, it's not without precedent. But putting everything in a PPP struct will
make versioning more straightforward if you have to update the interface. It's a
newer pattern we didn't have when PPB_Audio was done.
sgtm

[Justin Chuang, 2015/02/02 14:29:24]

Move the changes and discussions to 887403003

+      [out] uint64_t sequence_id);
 };