Add still image capture interface for VideoCaptureDevice.

media/video/capture/video_capture_device.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

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

Please add a more detailed explanation to the CL description.
Please add an implementation of this to FakeVideoCaptureDevice and implement a
unittest using that.

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

CL description updated.

I have another CL for adding the implementation for video_capture_device_linux
and also the unittest to run it. 

I think it is good to add the implementation/test of still image capture for the
FakeVideoCaptureDevice. But let me do it in another CL.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // VideoCaptureDevice is the abstract base class for realizing video capture
 // device support in Chromium. It provides the interface for OS dependent
 // implementations.
 // The class is created and functions are invoked on a thread owned by
 // VideoCaptureManager. Capturing is done on other threads, depending on the OS
 // specific implementation.
 
@@ skipping 197 matching lines @@
         const VideoCaptureFormat& buffer_format,
         const scoped_refptr<media::VideoFrame>& frame,
         base::TimeTicks timestamp) = 0;
 
     // An error has occurred that cannot be handled and VideoCaptureDevice must
     // be StopAndDeAllocate()-ed. |reason| is a text description of the error.
     virtual void OnError(const std::string& reason) = 0;
 
     // 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.
   // Return NULL if the hardware is not available.
   static VideoCaptureDevice* Create(
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
       const Name& device_name);
   virtual ~VideoCaptureDevice();
 
   // Gets the names of all video capture devices connected to this computer.
   static void GetDeviceNames(Names* device_names);
 
   // Gets the supported formats of a particular device attached to the system.
   // This method should be called before allocating or starting a device. In
   // case format enumeration is not supported, or there was a problem, the
   // formats array will be empty.
   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.
   virtual void AllocateAndStart(const VideoCaptureParams& params,
                                 scoped_ptr<Client> client) = 0;
 
   // Deallocates the camera, possibly asynchronously.
   //
   // This call requires the device to do the following things, eventually: put
   // camera hardware into a state where other applications could use it, free
   // the memory associated with capture, and delete the |client| pointer passed
   // into AllocateAndStart.
   //
   // If deallocation is done asynchronously, then the device implementation must
   // ensure that a subsequent AllocateAndStart() operation targeting the same ID
   // would be sequenced through the same task runner, so that deallocation
   // happens first.
   virtual void StopAndDeAllocate() = 0;
 
   // Gets the power line frequency from the current system time zone if this is
   // 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;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_DEVICE_H_