Added video capture capabilities retrieval and caching to VideoCaptureManager

content/browser/renderer_host/media/video_capture_manager.h

 // Copyright (c) 2012 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.
 
 // VideoCaptureManager is used to open/close, start/stop, enumerate available
 // video capture devices, and manage VideoCaptureController's.
 // All functions are expected to be called from Browser::IO thread. Some helper
 // functions (*OnDeviceThread) will dispatch operations to the device thread.
 // VideoCaptureManager will open OS dependent instances of VideoCaptureDevice.
 // A device can only be opened once.
@@ skipping 64 matching lines @@
                              const DoneCB& done_cb);
 
   // Called by VideoCaptureHost to remove |client_handler|. If this is the last
   // client of the device, the |controller| and its VideoCaptureDevice may be
   // destroyed. The client must not access |controller| after calling this
   // function.
   void StopCaptureForClient(VideoCaptureController* controller,
                             VideoCaptureControllerID client_id,
                             VideoCaptureControllerEventHandler* client_handler);
 
+  // Retrieve the available capture capabilities for a particular device. The
+  // capabilities are cached during GetAvailableDevicesOnDeviceThread() and
+  // updated on DoStartDeviceOnDeviceThread(). The call is asynchronous and the
+  // reply comes via OnDeviceCapabilitiesEnumerated().

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

nit: nice comment in general but I don't think you need to explain the private
implementation in detail.  It would be good to mention on what thread the caller
should expect a callback and when.  The callback (as far as the caller is
concerned) is actually DeviceCapabilitiesEnumerated on the listener interface
and not OnDeviceCapabilitiesEnumerated which is a part of the private
implementation.

[mcasas, 2013/10/28 12:50:08]

Boosted prosaic skills :) + corrected callback name.

+  void EnumerateDeviceCapabilities(const StreamDeviceInfo& device_info);
+
  private:
   virtual ~VideoCaptureManager();
   struct DeviceEntry;
 
   // Check to see if |entry| has no clients left on its controller. If so,
   // remove it from the list of devices, and delete it asynchronously. |entry|
   // may be freed by this function.
   void DestroyDeviceEntryIfNoClients(DeviceEntry* entry);
 
   // Helpers to report an event to our Listener.
   void OnOpened(MediaStreamType type, int capture_session_id);
   void OnClosed(MediaStreamType type, int capture_session_id);
   void OnDevicesEnumerated(MediaStreamType stream_type,
                            const media::VideoCaptureDevice::Names& names);
+  void OnDeviceCapabilitiesEnumerated(
+      const StreamDeviceInfo& device_info,
+      const media::VideoCaptureCapabilities& capabilities);
 
   // Find a DeviceEntry by its device ID and type, if it is already opened.
   DeviceEntry* GetDeviceEntryForMediaStreamDevice(
       const MediaStreamDevice& device_info);
 
   // Find a DeviceEntry entry for the indicated session, creating a fresh one
   // if necessary. Returns NULL if the session id is invalid.
   DeviceEntry* GetOrCreateDeviceEntry(int capture_session_id);
 
   // Find the DeviceEntry that owns a particular controller pointer.
   DeviceEntry* GetDeviceEntryForController(
       const VideoCaptureController* controller);
 
   bool IsOnDeviceThread() const;
 
   // Queries and returns the available device IDs.
   media::VideoCaptureDevice::Names GetAvailableDevicesOnDeviceThread(
       MediaStreamType stream_type);
 
+  // Queries and returns the video capture capabilities of a device.
+  media::VideoCaptureCapabilities GetDeviceCapabilitiesOnDeviceThread(
+      const StreamDeviceInfo& device_info) const;
+
   // Create and Start a new VideoCaptureDevice, storing the result in
   // |entry->video_capture_device|. Ownership of |client| passes to
   // the device.
   void DoStartDeviceOnDeviceThread(
       DeviceEntry* entry,
       const media::VideoCaptureCapability& capture_params,
       scoped_ptr<media::VideoCaptureDevice::Client> client);
 
   // Stop and destroy the VideoCaptureDevice held in
   // |entry->video_capture_device|.
@@ skipping 42 matching lines @@
   // Set to true if using fake video capture devices for testing, false by
   // default. This is only used for the MEDIA_DEVICE_VIDEO_CAPTURE device type.
   bool use_fake_device_;
 
   // We cache the enumerated video capture devices in
   // GetAvailableDevicesOnDeviceThread() and then later look up the requested ID
   // when a device is created in DoStartDeviceOnDeviceThread(). Used only on the
   // device thread.
   media::VideoCaptureDevice::Names video_capture_devices_;
 
+  // Local cache of video capture capabilities, indexed by unique id. It is
+  // created in GetAvailableDevicesOnDeviceThread(), updated in
+  // DoStartDeviceOnDeviceThread() and retrieved for a particular device id in
+  // EnumerateDeviceCapabilities(), read only.

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

is this last part true?  It looks like EnumerateDeviceCapabilities doesn't touch
this variable and it really shouldn't since it does not run on the device
thread.

[mcasas, 2013/10/28 12:50:08]

Correct, this comment has not been updated after I moved from ignoring-threads
implementation to now-let's-respect-threads implementation. It should read
"retrieved _via_..." since that's what clients of the class will see.

+  std::map<std::string, media::VideoCaptureCapabilities>
+      video_capture_capabilities_;

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

Instead of an extra map, can we change video_capture_devices_ to be a list of
VideoCaptureDevice::Name + VideoCaptureCapabilities?  (note singular in both
cases).
As is, we have to be careful about keeping these two containers in sync.
Something like:

struct VideoCaptureDevice {
  media::VideoCaptureDevice::Name name;
  media::VideoCaptureCapabilities capabilities;
};

std::list<VideoDevice> video_capture_devices_;

(std::vector might also be fine)

[mcasas, 2013/10/28 12:50:08]

Done. It turned out to be a class - first due to Chromium style (complex
constructor due to non-POD members), and bc it needed a two param constructor
for the vector. Had to add a method outside this class called
FindDeviceInfoById(), which might have rendered useless Name::FindById().

+
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureManager);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_VIDEO_CAPTURE_MANAGER_H_