Add still image capture interface for VideoCaptureDevice.

media/video/capture/video_capture_device.h

Index: media/video/capture/video_capture_device.h
diff --git a/media/video/capture/video_capture_device.h b/media/video/capture/video_capture_device.h
index 382dad6cdc461d7d6d20bf04ab8e7e91693ae503..105087ee41f60425362afe84f5de4b2e27751c6e 100644
--- a/media/video/capture/video_capture_device.h
+++ b/media/video/capture/video_capture_device.h
@@ -216,6 +216,33 @@ class MEDIA_EXPORT VideoCaptureDevice {
 
     // VideoCaptureDevice requests the |message| to be logged.
     virtual void OnLog(const std::string& message) {}
+
+    // The video stream has been muted. After this callback, no more
+    // OnIncomingCapturedData() will be called. This may happen when
+    // CaptureImage() has called. After the still image has been captured, the

[perkj_chrome, 2014/09/17 11:25:58]

s has been

[Pawel Osciak, 2014/09/17 11:29:42]

s/has called/was called/
Is there any other case that this may happen? If not, don't say "may happen".

[Owen Lin, 2014/09/18 06:04:15]

Hi, Pawel, I was thinking if the implementation doesn't require the video stream
to stop. We won't call OnMute() and OnUnmute(). 

Do you think it's a case we need to consider or we must pause the video stream
when take still image.

+    // client will get notified by OnUnmute() and the video stream will be
+    // resumed.
+    virtual void OnMute() {}
+
+    // The video stream has resumed.
+    virtual void OnUnmute() {}
+  };
+
+  class MEDIA_EXPORT ImageClient {

[Pawel Osciak, 2014/09/17 11:29:42]

StillImageClient here and everywhere?

[wuchengli, 2014/09/18 01:26:39]

Image is symmetric to video. For example, ImageCaptureFormats vs
VideoCaptureFormats. StillImageCaptureFormats doesn't look good.

+   public:
+    virtual ~ImageClient() {}
+
+    // Callback function to notify the client the caputred image is available.

[Pawel Osciak, 2014/09/17 11:29:42]

s/caputred/captured

[Owen Lin, 2014/09/18 06:04:15]

Done.

+    virtual void OnIncomingCapturedData(const uint8* data,

[Pawel Osciak, 2014/09/17 11:29:42]

Let's call this differently, in case one class wants to subclass both Clients.

[Owen Lin, 2014/09/18 06:04:15]

It is unlikely to happen. Both function "AllocateAndStart" and
"InitializeImageCapture" will take the ownership of its client (It is passed as
a scoped_ptr). So we cannot pass the same client to those two functions.

+                                        int length,
+                                        const ImageCaptureFormat& format,
+                                        int rotation,

[Pawel Osciak, 2014/09/17 11:29:42]

Please specify the parameters in the doc above.

[Owen Lin, 2014/09/18 06:04:15]

Done.

+                                        base::TimeTicks timestamp) = 0;
+
+    // Callback function to notify client the failure of the image capture.
+    // The VideoCaptureDevice must be StopAndDeAllocate()-ed. |reason| is a
+    // text description of the error.
+    virtual void OnError(const std::string& reason) = 0;

[wuchengli, 2014/09/17 06:54:29]

OK. Let's keep this for now. PPAPI uses the error codes from pp_errors.h. If we
need an integer error code, we can change it in the future.

https://codereview.chromium.org/391323002/diff/400001/ppapi/api/private/ppb_i...

[Pawel Osciak, 2014/09/17 11:29:42]

I'm wondering if we need to have a separate OnError? The difference between this
and the regular client is that we have many OnIncomingCapturedData() there and
checking for error in the impl of it would be ugly and slightly inefficient. In
this case we only have one OnIncomingCapturedData, which can either be a success
or failure, so maybe checking for error or success would be ok in its
implementation?
Just wondering what others think about this...

[wuchengli, 2014/09/18 01:26:39]

One thing to consider is we'll soon add doAutoFocus and setting camera
parameters. Autofocus needs error callback. If setting parameters is
asynchronous, it may also need error callback.

Another way is to combine Client and ImageClient. But Client already has lots of
functions. Maybe duplicating OnError is not too bad given autofocus will also
use it.

[Owen Lin, 2014/09/18 06:04:15]

Good point. I was thinking that. The main reason I do this way is to handle the
error not in the scope of taking a picture (e.g., allocation fail in
InitializeImageCapture).

Now, I am thinking maybe we can put the device into an error state and report
the error when CaputreImage is called.

In that case, we can remove the entire ImageClient and use callbacks in these
functions.

How do you think?

[wuchengli, 2014/09/18 08:55:08]

I think having ImageClient interface is more consistent with
VideoCaptureDevice::Client. Both callbacks and ImageClient work. We should
choose one that is more consistent. As I said, we'll add autofocus and
parameters setting, which will require lots of callbacks.

   };
 
   // Creates a VideoCaptureDevice object.
@@ -235,6 +262,12 @@ class MEDIA_EXPORT VideoCaptureDevice {
   static void GetDeviceSupportedFormats(const Name& device,
                                         VideoCaptureFormats* supported_formats);
 
+  // Gets the supported formats for still image of a particular device attached
+  // to the system.
+  static void GetDeviceSupportedImageFormats(

[Pawel Osciak, 2014/09/17 11:29:42]

StillImageFormats?

[Owen Lin, 2014/09/18 06:04:15]

Let's keep using Image for consistency.

+      const Name& device,
+      ImageCaptureFormats* supported_formats);

[Pawel Osciak, 2014/09/17 11:29:42]

What is the expectation for lifetime/ownership of this pointer? Please document
how this works.

[Owen Lin, 2014/09/18 06:04:15]

ImageCaputreFormats is actullay a vector<ImageCaptureFormat>.
Here, it's an output. Caller should provide it.

+
   // Prepares the camera for use. After this function has been called no other
   // applications can use the camera. StopAndDeAllocate() must be called before
   // the object is deleted.
@@ -258,6 +291,30 @@ class MEDIA_EXPORT VideoCaptureDevice {
   // defined, otherwise returns 0.
   int GetPowerLineFrequencyForLocation() const;
 
+  // Initializes the device for still image capture for the given image format.
+  //
+  // This function must be called between AllocateAndStart() and
+  // StopAndDeAllocate().
+  virtual void InitializeImageCapture(const ImageCaptureFormat& image_format,
+                                      scoped_ptr<ImageClient> client) {}
+
+  // Releases resources for image capture.
+  //
+  // The ImageClient passed from InitializeImageCapture would be freed. This
+  // method must be called between InitializeImageCapture() and
+  // StopAndDeAllocate().
+  virtual void ReleaseImageCapture() {}
+
+  // Gets one image from the device asynchronously.
+  //
+  // It will return the image by the ImageClient::OnIncomingCapturedData()
+  // callback. If the video stream has to be stopped to capture the still image,
+  // the Client::OnMute() and Client::OnUnmute() will be called.
+  //
+  // This function must be called between InitializeImageCapture() and
+  // ReleaseImageCapture().
+  virtual void CaptureImage() {}
+
  protected:
   static const int kPowerLine50Hz = 50;
   static const int kPowerLine60Hz = 60;