PPAPI: implement GetSupportedPreviewSizes

ppapi/api/private/ppb_image_capture_private.idl

 /* 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.
  */
 
 /**
  * Defines the <code>PPB_ImageCapture_Private</code> interface. Used for
  * acquiring a single still image from a camera source.
  */
 
 [generate_thunk]
 
 label Chrome {
-  M39 = 0.1
+  M42 = 0.1
 };
 
 /**
  * Callback function for <code>PPB_ImageCapture_Private.CaptureStillImage
  * </code> to indicate the image has been captured from the sensor. This is a
  * good opportunity to play a shutter sound or give other feedback of camera
  * operation. This will occur after the image was captured, but before the
  * 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
  * </code> to deliver a preview image. The client can use this to show the
  * captured image. See <code>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.
  *  |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);
 
 /**
  * Callback function for <code>PPB_ImageCapture_Private.CaptureStillImage
  * </code> to deliver a still JPEG image. See <code>
  * 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);
 
 /**
  * Callback function for <code>PPB_ImageCapture_Private.CaptureStillImage
  * </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 {
   /**
    * Creates a PPB_ImageCapture_Private resource.
    *
    * @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.
    *
    * @param[in] resource The <code>PP_Resource</code> to test.
    *
    * @return A <code>PP_Bool</code> with <code>PP_TRUE</code> if the given
    * resource is an image capture resource or <code>PP_FALSE</code>
    * otherwise.
    */
   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
    * function will be automatically called. Calling this more than once has no
    * effect.
    *
    * @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.
    * If <code>SetConfig()</code> is not called, default settings will be used.
    *
    * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
    * 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
    * <code>SetConfig()</code> or <code>CaptureStillImage()</code>.
    * If an error is returned, the configuration will not be changed.
    */
   int32_t SetConfig([in] PP_Resource image_capture,
                     [in] PP_Resource config,
                     [in] PP_CompletionCallback callback);
 
   /**
    * Gets the configuration of the image capture.
    *
    * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
    * image capture resource.
    * @param[out] config A <code>PP_ImageCaptureConfig_Private</code> for storing
    * 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>.
    */
   int32_t GetConfig([in] PP_Resource image_capture,
                     [out] PP_Resource config,
                     [in] PP_CompletionCallback callback);

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

Is it overkill to have the config as a separate resource? Would it be better to
just have 2 Gets and 2 Sets here?

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

Some settings have to be sent to the driver atomically. For example, apps want
to reduce exposure time and increase gain together. Having two image functions,
SetExposure and SetGain, in image capture class won't be atomic.

FYI. Config cannot be a simple struct because API may be added in different
chrome releases and only resource can have release function decorators if I
understand correctly.

Capabilities are camera characteristics that do not change. It can have lots of
things in the future like supported Jpeg sizes, supported JPEG sizes, focus
length, min aperture, max fps, and etc. Apps can just get capabilities once. It
should not be merged to config so GetConfig can be faster.

 
   /**
    * Gets the camera capabilities.
    *
    * The camera capabilities do not change for a given camera source.
    *
    * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
    * image capture resource.
    * @param[out] capabilities A <code>PPB_CameraCapabilities_Private</code> for
    * 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>.
    */
   int32_t GetCameraCapabilities([in] PP_Resource image_capture,
                                 [out] PP_Resource capabilities,
                                 [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
    * of callbacks to the application as the image capture progresses. The
    * callbacks will be invoked in the order of shutter callback, preview
    * callback, and JPEG callback. The shutter callback occurs after the image is
    * captured. This can be used to trigger a sound to let the user know that
    * 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
    * decided by the camera and can be got from <code>PPB_VideoFrame.GetFormat
    * </code>. The size of the JPEG image is determined by <code>
    * PPB_ImageCaptureConfig_Private.SetJpegSize</code>.
    *
    * The camera may need to stop and re-start streaming during image capture. If
    * some MediaStreamVideoTrack are associated with the camera source, they will
    * receive mute and unmute events. The mute event will be received before all
    * the callbacks. The unmute event will be received after all the callbacks.
    * The preview image will not be sent to the video tracks associated with the
    * camera.
    *
    * @param[in] image_capture A <code>PP_Resource</code> corresponding to an
    * image capture resource.
    * @param[in] shutter_callback A <code>
    * PPB_ImageCapture_Private_ShutterCallback</code> callback to indicate the
    * image has been taken.
    * @param[in] preview_callback A <code>
    * PPB_ImageCapture_Private_PreviewCallback</code> callback to return a
    * preview of the captured image.
    * @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.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    * PP_OK means the callbacks will be triggered. Other values mean the
    * callbacks will not be triggered.
    */
   int32_t CaptureStillImage(
       [in] PP_Resource image_capture,
       [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);
 };