Added supported formats caching to VideoCaptureManager, and

adapted code to cideo_capture_types.{h,cc} reorganization.

Rewritten http://crrev.com/29423003 for the new structure of
video_capture_types.h: The device's capabilities are turned into
a vector of supported formats. No new code is added, the
previously reviewed one is adapted.

VideoCaptureCapability class is not used anymore, so removed (I
tried to make very evident every time I use VideoCaptureFormat
as a supported format).

VideoCaptureLinux and FakeVCD are also adapted, originally the
code for retrieving their caps comes from http://crrev.com/24079003

1 VCM unittest disabled for Mac Valgrind: Underlying FakeVCD
hits a SIGSEGV at the end of this test in Mac Valgrind bot,
see http://crbug.com/319955. Possibly associated to FakeVCD
races, re-enable this test after http://crbug.com/323893, 
ongoing http://crrev.com/91523002.


NOTE: Time ago a bug was filed against this code due to chrome OS
new user's take-a-picture would not show anything. I just verified
this seems to work now.

BUG=289494,309554

Review URL: https://codereview.chromium.org/91343002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240395 0039d316-1c4b-4281-b951-d872f2087c98

20 files changed, 346 insertions, 142 deletions

diff --git a/content/browser/renderer_host/media/video_capture_controller.cc b/content/browser/renderer_host/media/video_capture_controller.cc
index f5d703f..cc0e86b 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -175,6 +175,10 @@ void VideoCaptureController::AddClient(
            << ", " << session_id
            << ")";
 
+  // If this is the first client added to the controller, cache the parameters.
+  if (!controller_clients_.size())
+    video_capture_format_ = params.requested_format;
+
   // Signal error in case device is already in error state.
   if (state_ == VIDEO_CAPTURE_STATE_ERROR) {
     event_handler->OnError(id);
@@ -251,6 +255,12 @@ void VideoCaptureController::ReturnBuffer(
   buffer_pool_->RelinquishConsumerHold(buffer_id, 1);
 }
 
+const media::VideoCaptureFormat&
+VideoCaptureController::GetVideoCaptureFormat() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  return video_capture_format_;
+}
+
 scoped_refptr<media::VideoCaptureDevice::Client::Buffer>
 VideoCaptureController::VideoCaptureDeviceClient::ReserveOutputBuffer(
     media::VideoFrame::Format format,
diff --git a/content/browser/renderer_host/media/video_capture_controller.h b/content/browser/renderer_host/media/video_capture_controller.h
index badab3b..8658cde 100644
--- a/content/browser/renderer_host/media/video_capture_controller.h
+++ b/content/browser/renderer_host/media/video_capture_controller.h
@@ -104,6 +104,8 @@ class CONTENT_EXPORT VideoCaptureController {
                     VideoCaptureControllerEventHandler* event_handler,
                     int buffer_id);
 
+  const media::VideoCaptureFormat& GetVideoCaptureFormat() const;
+
  private:
   class VideoCaptureDeviceClient;
 
@@ -141,6 +143,8 @@ class CONTENT_EXPORT VideoCaptureController {
   // state which stops the flow of data to clients.
   VideoCaptureState state_;
 
+  media::VideoCaptureFormat video_capture_format_;
+
   base::WeakPtrFactory<VideoCaptureController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureController);
diff --git a/content/browser/renderer_host/media/video_capture_host.h b/content/browser/renderer_host/media/video_capture_host.h
index c1f8e3c..b71aac37 100644
--- a/content/browser/renderer_host/media/video_capture_host.h
+++ b/content/browser/renderer_host/media/video_capture_host.h
@@ -61,10 +61,6 @@
 #include "content/public/browser/browser_message_filter.h"
 #include "ipc/ipc_message.h"
 
-namespace media {
-class VideoCaptureCapability;
-}
-
 namespace content {
 class MediaStreamManager;
 
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index ff71016..670f0a0 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
+#include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
@@ -44,10 +45,20 @@ VideoCaptureManager::DeviceEntry::DeviceEntry(
 
 VideoCaptureManager::DeviceEntry::~DeviceEntry() {}
 
+VideoCaptureManager::DeviceInfo::DeviceInfo() {}
+
+VideoCaptureManager::DeviceInfo::DeviceInfo(
+    const media::VideoCaptureDevice::Name& name,
+    const media::VideoCaptureFormats& supported_formats)
+    : name(name),
+      supported_formats(supported_formats) {}
+
+VideoCaptureManager::DeviceInfo::~DeviceInfo() {}
+
 VideoCaptureManager::VideoCaptureManager()
     : listener_(NULL),
       new_capture_session_id_(1),
-      artificial_device_source_for_testing_ (DISABLED) {
+      artificial_device_source_for_testing_(DISABLED) {
 }
 
 VideoCaptureManager::~VideoCaptureManager() {
@@ -74,9 +85,10 @@ void VideoCaptureManager::EnumerateDevices(MediaStreamType stream_type) {
   DCHECK(listener_);
   base::PostTaskAndReplyWithResult(
       device_loop_, FROM_HERE,
-      base::Bind(&VideoCaptureManager::GetAvailableDevicesOnDeviceThread, this,
-                 stream_type),
-      base::Bind(&VideoCaptureManager::OnDevicesEnumerated, this, stream_type));
+      base::Bind(&VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread,
+                 this, stream_type, devices_info_cache_),
+      base::Bind(&VideoCaptureManager::OnDevicesInfoEnumerated, this,
+                 stream_type));
 }
 
 int VideoCaptureManager::Open(const StreamDeviceInfo& device_info) {
@@ -154,21 +166,20 @@ void VideoCaptureManager::DoStartDeviceOnDeviceThread(
       // We look up the device id from the renderer in our local enumeration
       // since the renderer does not have all the information that might be
       // held in the browser-side VideoCaptureDevice::Name structure.
-      media::VideoCaptureDevice::Name* found =
-          video_capture_devices_.FindById(entry->id);
+      DeviceInfo* found = FindDeviceInfoById(entry->id, devices_info_cache_);
       if (found) {
         switch (artificial_device_source_for_testing_) {
           case DISABLED:
             video_capture_device.reset(
-                media::VideoCaptureDevice::Create(*found));
+                media::VideoCaptureDevice::Create(found->name));
             break;
           case TEST_PATTERN:
             video_capture_device.reset(
-                media::FakeVideoCaptureDevice::Create(*found));
+                media::FakeVideoCaptureDevice::Create(found->name));
             break;
           case Y4M_FILE:
             video_capture_device.reset(
-                media::FileVideoCaptureDevice::Create(*found));
+                media::FileVideoCaptureDevice::Create(found->name));
             break;
         }
       }
@@ -271,6 +282,34 @@ void VideoCaptureManager::StopCaptureForClient(
   DestroyDeviceEntryIfNoClients(entry);
 }
 
+void VideoCaptureManager::GetDeviceSupportedFormats(
+    int capture_session_id,
+    media::VideoCaptureFormats* supported_formats) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  supported_formats->clear();
+
+  std::map<int, MediaStreamDevice>::iterator it =
+      sessions_.find(capture_session_id);
+  DCHECK(it != sessions_.end());
+  DVLOG(1) << "GetDeviceSupportedFormats for device: " << it->second.name;
+
+  DeviceInfo* device_in_use =
+      FindDeviceInfoById(it->second.id, devices_info_cache_);
+  if (device_in_use) {
+    DeviceEntry* const existing_device =
+        GetDeviceEntryForMediaStreamDevice(it->second);
+    if (!existing_device) {
+      // If the device is not in use, return all its cached supported formats.
+      *supported_formats = device_in_use->supported_formats;
+      return;
+    }
+    // Otherwise, get the video capture parameters in use from the controller
+    // associated to the device.
+    supported_formats->push_back(
+        existing_device->video_capture_controller->GetVideoCaptureFormat());
+  }
+}
+
 void VideoCaptureManager::DoStopDeviceOnDeviceThread(DeviceEntry* entry) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StopDeviceTime");
   DCHECK(IsOnDeviceThread());
@@ -300,24 +339,25 @@ void VideoCaptureManager::OnClosed(MediaStreamType stream_type,
   listener_->Closed(stream_type, capture_session_id);
 }
 
-void VideoCaptureManager::OnDevicesEnumerated(
+void VideoCaptureManager::OnDevicesInfoEnumerated(
     MediaStreamType stream_type,
-    const media::VideoCaptureDevice::Names& device_names) {
+    const DeviceInfos& new_devices_info_cache) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (!listener_) {
     // Listener has been removed.
     return;
   }
+  devices_info_cache_ = new_devices_info_cache;
 
-  // Transform from VCD::Name to StreamDeviceInfo.
+  // Walk the |devices_info_cache_| and transform from VCD::Name to
+  // StreamDeviceInfo for return purposes.
   StreamDeviceInfoArray devices;
-  for (media::VideoCaptureDevice::Names::const_iterator it =
-           device_names.begin(); it != device_names.end(); ++it) {
+  for (DeviceInfos::const_iterator it = devices_info_cache_.begin();
+       it != devices_info_cache_.end(); ++it) {
     devices.push_back(StreamDeviceInfo(
-        stream_type, it->GetNameAndModel(), it->id()));
+        stream_type, it->name.GetNameAndModel(), it->name.id()));
   }
-
   listener_->DevicesEnumerated(stream_type, devices);
 }
 
@@ -325,14 +365,14 @@ bool VideoCaptureManager::IsOnDeviceThread() const {
   return device_loop_->BelongsToCurrentThread();
 }
 
-media::VideoCaptureDevice::Names
-VideoCaptureManager::GetAvailableDevicesOnDeviceThread(
-    MediaStreamType stream_type) {
+VideoCaptureManager::DeviceInfos
+VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread(
+    MediaStreamType stream_type,
+    const DeviceInfos& old_device_info_cache) {
   SCOPED_UMA_HISTOGRAM_TIMER(
-      "Media.VideoCaptureManager.GetAvailableDevicesTime");
+      "Media.VideoCaptureManager.GetAvailableDevicesInfoOnDeviceThreadTime");
   DCHECK(IsOnDeviceThread());
-  media::VideoCaptureDevice::Names result;
-
+  media::VideoCaptureDevice::Names names_snapshot;
   switch (stream_type) {
     case MEDIA_DEVICE_VIDEO_CAPTURE:
       // Cache the latest enumeration of video capture devices.
@@ -340,20 +380,15 @@ VideoCaptureManager::GetAvailableDevicesOnDeviceThread(
       // enumerate the devices again.
       switch (artificial_device_source_for_testing_) {
         case DISABLED:
-          media::VideoCaptureDevice::GetDeviceNames(&result);
+          media::VideoCaptureDevice::GetDeviceNames(&names_snapshot);
           break;
         case TEST_PATTERN:
-          media::FakeVideoCaptureDevice::GetDeviceNames(&result);
+          media::FakeVideoCaptureDevice::GetDeviceNames(&names_snapshot);
           break;
         case Y4M_FILE:
-          media::FileVideoCaptureDevice::GetDeviceNames(&result);
+          media::FileVideoCaptureDevice::GetDeviceNames(&names_snapshot);
           break;
       }
-
-      // TODO(nick): The correctness of device start depends on this cache being
-      // maintained, but it seems a little odd to keep a cache here. Can we
-      // eliminate it?
-      video_capture_devices_ = result;
       break;
 
     case MEDIA_DESKTOP_VIDEO_CAPTURE:
@@ -364,7 +399,48 @@ VideoCaptureManager::GetAvailableDevicesOnDeviceThread(
       NOTREACHED();
       break;
   }
-  return result;
+
+  // Construct |new_devices_info_cache| with the cached devices that are still
+  // present in the system, and remove their names from |names_snapshot|, so we
+  // keep there the truly new devices.
+  DeviceInfos new_devices_info_cache;
+  for (DeviceInfos::const_iterator it_device_info =
+           old_device_info_cache.begin();
+       it_device_info != old_device_info_cache.end(); ++it_device_info) {
+     for (media::VideoCaptureDevice::Names::iterator it =
+              names_snapshot.begin();
+          it != names_snapshot.end(); ++it) {
+      if (it_device_info->name.id() == it->id()) {
+        new_devices_info_cache.push_back(*it_device_info);
+        names_snapshot.erase(it);
+        break;
+      }
+    }
+  }
+
+  // Get the supported capture formats for the new devices in |names_snapshot|.
+  for (media::VideoCaptureDevice::Names::const_iterator it =
+           names_snapshot.begin();
+       it != names_snapshot.end(); ++it) {
+    media::VideoCaptureFormats supported_formats;
+    DeviceInfo device_info(*it, media::VideoCaptureFormats());
+    switch (artificial_device_source_for_testing_) {
+      case DISABLED:
+        media::VideoCaptureDevice::GetDeviceSupportedFormats(
+            *it, &(device_info.supported_formats));
+        break;
+      case TEST_PATTERN:
+        media::FakeVideoCaptureDevice::GetDeviceSupportedFormats(
+            *it, &(device_info.supported_formats));
+        break;
+      case Y4M_FILE:
+        media::FileVideoCaptureDevice::GetDeviceSupportedFormats(
+            *it, &(device_info.supported_formats));
+        break;
+    }
+    new_devices_info_cache.push_back(device_info);
+  }
+  return new_devices_info_cache;
 }
 
 VideoCaptureManager::DeviceEntry*
@@ -430,7 +506,7 @@ VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
   // Check if another session has already opened this device. If so, just
   // use that opened device.
   DeviceEntry* const existing_device =
-       GetDeviceEntryForMediaStreamDevice(device_info);
+      GetDeviceEntryForMediaStreamDevice(device_info);
   if (existing_device) {
     DCHECK_EQ(device_info.type, existing_device->stream_type);
     return existing_device;
@@ -445,4 +521,15 @@ VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
   return new_device;
 }
 
+VideoCaptureManager::DeviceInfo* VideoCaptureManager::FindDeviceInfoById(
+    const std::string& id,
+    DeviceInfos& device_vector) {
+  for (DeviceInfos::iterator it = device_vector.begin();
+       it != device_vector.end(); ++it) {
+    if (it->name.id() == id)
+      return &(*it);
+  }
+  return NULL;
+}
+
 }  // namespace content
diff --git a/content/browser/renderer_host/media/video_capture_manager.h b/content/browser/renderer_host/media/video_capture_manager.h
index 82c6153..5f3630e 100644
--- a/content/browser/renderer_host/media/video_capture_manager.h
+++ b/content/browser/renderer_host/media/video_capture_manager.h
@@ -83,10 +83,28 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
                             VideoCaptureControllerID client_id,
                             VideoCaptureControllerEventHandler* client_handler);
 
+  // Retrieves the available capture supported formats for a particular device.
+  // The supported formats are cached during device(s) enumeration.
+  void GetDeviceSupportedFormats(int capture_session_id,
+                                 media::VideoCaptureFormats* supported_formats);
+
  private:
   virtual ~VideoCaptureManager();
   struct DeviceEntry;
 
+  // This data structure is a convenient wrap of a devices' name and associated
+  // video capture supported formats.
+  struct DeviceInfo {
+    DeviceInfo();
+    DeviceInfo(const media::VideoCaptureDevice::Name& name,
+               const media::VideoCaptureFormats& supported_formats);
+    ~DeviceInfo();
+
+    media::VideoCaptureDevice::Name name;
+    media::VideoCaptureFormats supported_formats;
+  };
+  typedef std::vector<DeviceInfo> DeviceInfos;
+
   // 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.
@@ -95,8 +113,9 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // 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 OnDevicesInfoEnumerated(
+      MediaStreamType stream_type,
+      const DeviceInfos& new_devices_info_cache);
 
   // Find a DeviceEntry by its device ID and type, if it is already opened.
   DeviceEntry* GetDeviceEntryForMediaStreamDevice(
@@ -112,9 +131,13 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
 
   bool IsOnDeviceThread() const;
 
-  // Queries and returns the available device IDs.
-  media::VideoCaptureDevice::Names GetAvailableDevicesOnDeviceThread(
-      MediaStreamType stream_type);
+  // Queries the Names of the devices in the system; the formats supported by
+  // the new devices are also queried, and consolidated with the copy of the
+  // local device info cache passed. The consolidated list of devices and
+  // supported formats is returned.
+  DeviceInfos GetAvailableDevicesInfoOnDeviceThread(
+      MediaStreamType stream_type,
+      const DeviceInfos& old_device_info_cache);
 
   // Create and Start a new VideoCaptureDevice, storing the result in
   // |entry->video_capture_device|. Ownership of |client| passes to
@@ -128,6 +151,9 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // |entry->video_capture_device|.
   void DoStopDeviceOnDeviceThread(DeviceEntry* entry);
 
+  DeviceInfo* FindDeviceInfoById(const std::string& id,
+                                 DeviceInfos& device_vector);
+
   // The message loop of media stream device thread, where VCD's live.
   scoped_refptr<base::MessageLoopProxy> device_loop_;
 
@@ -168,6 +194,15 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   typedef std::set<DeviceEntry*> DeviceEntries;
   DeviceEntries devices_;
 
+  // Local cache of the enumerated video capture devices' names and capture
+  // supported formats. A snapshot of the current devices and their capabilities
+  // is composed in GetAvailableDevicesInfoOnDeviceThread() --coming
+  // from EnumerateDevices()--, and this snapshot is used to update this list in
+  // OnDevicesInfoEnumerated(). GetDeviceSupportedFormats() will
+  // use this list if the device is not started, otherwise it will retrieve the
+  // active device capture format from the VideoCaptureController associated.
+  DeviceInfos devices_info_cache_;
+
   // For unit testing and for performance/quality tests, a test device can be
   // used instead of a real one. The device can be a simple fake device (a
   // rolling pacman), or a file that is played in a loop continuously. This only
@@ -178,12 +213,6 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
     Y4M_FILE
   } artificial_device_source_for_testing_;
 
-  // 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_;
-
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureManager);
 };
 
diff --git a/content/browser/renderer_host/media/video_capture_manager_unittest.cc b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
index e4d3bb2..bc89159 100644
--- a/content/browser/renderer_host/media/video_capture_manager_unittest.cc
+++ b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
@@ -16,6 +16,7 @@
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/common/media/media_stream_options.h"
+#include "media/video/capture/fake_video_capture_device.h"
 #include "media/video/capture/video_capture_device.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -196,6 +197,97 @@ TEST_F(VideoCaptureManagerTest, OpenTwice) {
   vcm_->Unregister();
 }
 
+// Connect and disconnect devices.
+TEST_F(VideoCaptureManagerTest, ConnectAndDisconnectDevices) {
+  StreamDeviceInfoArray devices;
+  int number_of_devices_keep =
+      media::FakeVideoCaptureDevice::NumberOfFakeDevices();
+
+  InSequence s;
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1).WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 2u);
+
+  // Simulate we remove 1 fake device.
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(1);
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1).WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 1u);
+
+  // Simulate we add 2 fake devices.
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(3);
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1).WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 3u);
+
+  vcm_->Unregister();
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(number_of_devices_keep);
+}
+
+// Enumerate devices and open the first, then check the list of supported
+// formats. Then start the opened device. The capability list should be reduced
+// to just one format, and this should be the one used when configuring-starting
+// the device. Finally stop the device and check that the capabilities have been
+// restored.
+TEST_F(VideoCaptureManagerTest, ManipulateDeviceAndCheckCapabilities) {
+  StreamDeviceInfoArray devices;
+
+  InSequence s;
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1).WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+
+  EXPECT_CALL(*listener_, Opened(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(1);
+  int video_session_id = vcm_->Open(devices.front());
+  message_loop_->RunUntilIdle();
+
+  // When the device has been opened, we should see all the devices'
+  // supported formats.
+  media::VideoCaptureFormats supported_formats;
+  vcm_->GetDeviceSupportedFormats(video_session_id, &supported_formats);
+  ASSERT_EQ(devices.size(), 2u);
+  ASSERT_GT(supported_formats.size(), 1u);
+  EXPECT_GT(supported_formats[0].frame_size.width(), 1);
+  EXPECT_GT(supported_formats[0].frame_size.height(), 1);
+  EXPECT_GT(supported_formats[0].frame_rate, 1);
+  EXPECT_GT(supported_formats[1].frame_size.width(), 1);
+  EXPECT_GT(supported_formats[1].frame_size.height(), 1);
+  EXPECT_GT(supported_formats[1].frame_rate, 1);
+
+  VideoCaptureControllerID client_id = StartClient(video_session_id, true);
+  message_loop_->RunUntilIdle();
+  // After StartClient(), device's supported formats should be reduced to one.
+  vcm_->GetDeviceSupportedFormats(video_session_id, &supported_formats);
+  ASSERT_EQ(supported_formats.size(), 1u);
+  EXPECT_GT(supported_formats[0].frame_size.width(), 1);
+  EXPECT_GT(supported_formats[0].frame_size.height(), 1);
+  EXPECT_GT(supported_formats[0].frame_rate, 1);
+
+  EXPECT_CALL(*listener_, Closed(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(1);
+  StopClient(client_id);
+  // After StopClient(), the device's list of supported formats should be
+  // restored to the original one.
+  vcm_->GetDeviceSupportedFormats(video_session_id, &supported_formats);
+  ASSERT_GT(supported_formats.size(), 1u);
+  EXPECT_GT(supported_formats[0].frame_size.width(), 1);
+  EXPECT_GT(supported_formats[0].frame_size.height(), 1);
+  EXPECT_GT(supported_formats[0].frame_rate, 1);
+  EXPECT_GT(supported_formats[1].frame_size.width(), 1);
+  EXPECT_GT(supported_formats[1].frame_size.height(), 1);
+  EXPECT_GT(supported_formats[1].frame_rate, 1);
+
+  vcm_->Close(video_session_id);
+  message_loop_->RunUntilIdle();
+  vcm_->Unregister();
+}
+
 // Open two different devices.
 TEST_F(VideoCaptureManagerTest, OpenTwo) {
   StreamDeviceInfoArray devices;
diff --git a/content/common/media/video_capture_messages.h b/content/common/media/video_capture_messages.h
index 30ac62c..55cbb38 100644
--- a/content/common/media/video_capture_messages.h
+++ b/content/common/media/video_capture_messages.h
@@ -20,10 +20,6 @@ IPC_STRUCT_TRAITS_BEGIN(media::VideoCaptureParams)
   IPC_STRUCT_TRAITS_MEMBER(allow_resolution_change)
 IPC_STRUCT_TRAITS_END()
 
-IPC_STRUCT_TRAITS_BEGIN(media::VideoCaptureCapability)
-  IPC_STRUCT_TRAITS_MEMBER(supported_format)
-IPC_STRUCT_TRAITS_END()
-
 // TODO(nick): device_id in these messages is basically just a route_id. We
 // should shift to IPC_MESSAGE_ROUTED and use MessageRouter in the filter impls.
 
diff --git a/media/video/capture/android/video_capture_device_android.cc b/media/video/capture/android/video_capture_device_android.cc
index c45a930..adfa9a3 100644
--- a/media/video/capture/android/video_capture_device_android.cc
+++ b/media/video/capture/android/video_capture_device_android.cc
@@ -55,7 +55,7 @@ void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
 
 // static
 void VideoCaptureDevice::GetDeviceSupportedFormats(const Name& device,
-    VideoCaptureCapabilities* formats) {
+    VideoCaptureFormats* formats) {
   NOTIMPLEMENTED();
 }
 
diff --git a/media/video/capture/fake_video_capture_device.cc b/media/video/capture/fake_video_capture_device.cc
index 08c4b36..a87514d 100644
--- a/media/video/capture/fake_video_capture_device.cc
+++ b/media/video/capture/fake_video_capture_device.cc
@@ -8,7 +8,6 @@
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "media/audio/fake_audio_input_stream.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -26,30 +25,43 @@ bool FakeVideoCaptureDevice::fail_next_create_ = false;
 base::subtle::Atomic32 FakeVideoCaptureDevice::number_of_devices_ =
     kNumberOfFakeDevices;
 
+// static
+size_t FakeVideoCaptureDevice::NumberOfFakeDevices(void) {
+  return number_of_devices_;
+}
+
+// static
 void FakeVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
   // Empty the name list.
   device_names->erase(device_names->begin(), device_names->end());
 
   int number_of_devices = base::subtle::NoBarrier_Load(&number_of_devices_);
   for (int32 n = 0; n < number_of_devices; n++) {
-    Name name("fake_device_" + base::IntToString(n),
-              "/dev/video" + base::IntToString(n));
+    Name name(base::StringPrintf("fake_device_%d", n),
+              base::StringPrintf("/dev/video%d", n));
     device_names->push_back(name);
   }
 }
 
+// static
 void FakeVideoCaptureDevice::GetDeviceSupportedFormats(
     const Name& device,
-    VideoCaptureCapabilities* formats) {
-  VideoCaptureCapability capture_format_640x480;
-  capture_format_640x480.supported_format.frame_size.SetSize(640, 480);
-  capture_format_640x480.supported_format.frame_rate =
-      1000 / kFakeCaptureTimeoutMs;
-  capture_format_640x480.supported_format.pixel_format =
-      media::PIXEL_FORMAT_I420;
-  formats->push_back(capture_format_640x480);
+    VideoCaptureFormats* supported_formats) {
+
+  supported_formats->clear();
+  VideoCaptureFormat capture_format_640x480;
+  capture_format_640x480.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_format_640x480.frame_size.SetSize(640, 480);
+  capture_format_640x480.frame_rate = 1000 / kFakeCaptureTimeoutMs;
+  supported_formats->push_back(capture_format_640x480);
+  VideoCaptureFormat capture_format_320x240;
+  capture_format_320x240.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_format_320x240.frame_size.SetSize(320, 240);
+  capture_format_320x240.frame_rate = 1000 / kFakeCaptureTimeoutMs;
+  supported_formats->push_back(capture_format_320x240);
 }
 
+// static
 VideoCaptureDevice* FakeVideoCaptureDevice::Create(const Name& device_name) {
   if (fail_next_create_) {
     fail_next_create_ = false;
@@ -57,7 +69,7 @@ VideoCaptureDevice* FakeVideoCaptureDevice::Create(const Name& device_name) {
   }
   int number_of_devices = base::subtle::NoBarrier_Load(&number_of_devices_);
   for (int32 n = 0; n < number_of_devices; ++n) {
-    std::string possible_id = "/dev/video" + base::IntToString(n);
+    std::string possible_id = base::StringPrintf("/dev/video%d", n);
     if (device_name.id().compare(possible_id) == 0) {
       return new FakeVideoCaptureDevice();
     }
@@ -65,10 +77,12 @@ VideoCaptureDevice* FakeVideoCaptureDevice::Create(const Name& device_name) {
   return NULL;
 }
 
+// static
 void FakeVideoCaptureDevice::SetFailNextCreate() {
   fail_next_create_ = true;
 }
 
+// static
 void FakeVideoCaptureDevice::SetNumberOfFakeDevices(size_t number_of_devices) {
   base::subtle::NoBarrier_AtomicExchange(&number_of_devices_,
                                          number_of_devices);
diff --git a/media/video/capture/fake_video_capture_device.h b/media/video/capture/fake_video_capture_device.h
index 2a884a3..399a682 100644
--- a/media/video/capture/fake_video_capture_device.h
+++ b/media/video/capture/fake_video_capture_device.h
@@ -26,10 +26,11 @@ class MEDIA_EXPORT FakeVideoCaptureDevice : public VideoCaptureDevice {
   // return NULL;
   static void SetFailNextCreate();
   static void SetNumberOfFakeDevices(size_t number_of_devices);
+  static size_t NumberOfFakeDevices();
 
   static void GetDeviceNames(Names* device_names);
   static void GetDeviceSupportedFormats(const Name& device,
-                                        VideoCaptureCapabilities* formats);
+                                        VideoCaptureFormats* supported_formats);
 
   // VideoCaptureDevice implementation.
   virtual void AllocateAndStart(const VideoCaptureParams& params,
diff --git a/media/video/capture/file_video_capture_device.cc b/media/video/capture/file_video_capture_device.cc
index 329e0fd..6f118d2 100644
--- a/media/video/capture/file_video_capture_device.cc
+++ b/media/video/capture/file_video_capture_device.cc
@@ -159,11 +159,11 @@ void FileVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
 
 void FileVideoCaptureDevice::GetDeviceSupportedFormats(
     const Name& device,
-    VideoCaptureCapabilities* formats) {
+    VideoCaptureFormats* supported_formats) {
   base::PlatformFile file = OpenFileForRead(GetFilePathFromCommandLine());
-  VideoCaptureCapability capture_capability;
-  ParseFileAndExtractVideoFormat(file, &capture_capability.supported_format);
-  formats->push_back(capture_capability);
+  VideoCaptureFormat capture_format;
+  ParseFileAndExtractVideoFormat(file, &capture_format);
+  supported_formats->push_back(capture_format);
 
   CHECK(base::ClosePlatformFile(file));
 }
diff --git a/media/video/capture/file_video_capture_device.h b/media/video/capture/file_video_capture_device.h
index d03fd7f..06e6033 100644
--- a/media/video/capture/file_video_capture_device.h
+++ b/media/video/capture/file_video_capture_device.h
@@ -31,7 +31,7 @@ class MEDIA_EXPORT FileVideoCaptureDevice : public VideoCaptureDevice {
   static VideoCaptureDevice* Create(const Name& device_name);
   static void GetDeviceNames(Names* device_names);
   static void GetDeviceSupportedFormats(const Name& device,
-                                        VideoCaptureCapabilities* formats);
+                                        VideoCaptureFormats* supported_formats);
 
   // VideoCaptureDevice implementation, class methods.
   virtual ~FileVideoCaptureDevice();
diff --git a/media/video/capture/linux/video_capture_device_linux.cc b/media/video/capture/linux/video_capture_device_linux.cc
index 8b029ca..b2717b8 100644
--- a/media/video/capture/linux/video_capture_device_linux.cc
+++ b/media/video/capture/linux/video_capture_device_linux.cc
@@ -146,29 +146,23 @@ void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
 
 void VideoCaptureDevice::GetDeviceSupportedFormats(
     const Name& device,
-    VideoCaptureCapabilities* formats) {
-
+    VideoCaptureFormats* supported_formats) {
   if (device.id().empty())
     return;
   int fd;
-  VideoCaptureCapabilities capture_formats;
-  if ((fd = open(device.id().c_str(), O_RDONLY)) < 0) {
-    // Failed to open this device.
+  if ((fd = open(device.id().c_str(), O_RDONLY)) < 0)
     return;
-  }
-
-  formats->clear();
 
-  VideoCaptureCapability capture_capability;
-  // Retrieve the caps one by one, first get colorspace, then sizes, then
-  // framerates. See http://linuxtv.org/downloads/v4l-dvb-apis for reference.
+  supported_formats->clear();
+  // Retrieve the caps one by one, first get pixel format, then sizes, then
+  // frame rates. See http://linuxtv.org/downloads/v4l-dvb-apis for reference.
   v4l2_fmtdesc pixel_format = {};
   pixel_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   while (ioctl(fd, VIDIOC_ENUM_FMT, &pixel_format) == 0) {
-    capture_capability.supported_format.pixel_format =
+    VideoCaptureFormat supported_format;
+    supported_format.pixel_format =
         V4l2ColorToVideoCaptureColorFormat((int32)pixel_format.pixelformat);
-    if (capture_capability.supported_format.pixel_format ==
-        PIXEL_FORMAT_UNKNOWN) {
+    if (supported_format.pixel_format == PIXEL_FORMAT_UNKNOWN) {
       continue;
     }
 
@@ -176,7 +170,7 @@ void VideoCaptureDevice::GetDeviceSupportedFormats(
     frame_size.pixel_format = pixel_format.pixelformat;
     while (ioctl(fd, VIDIOC_ENUM_FRAMESIZES, &frame_size) == 0) {
       if (frame_size.type == V4L2_FRMSIZE_TYPE_DISCRETE) {
-        capture_capability.supported_format.frame_size.SetSize(
+        supported_format.frame_size.SetSize(
             frame_size.discrete.width, frame_size.discrete.height);
       } else if (frame_size.type == V4L2_FRMSIZE_TYPE_STEPWISE) {
         // TODO(mcasas): see http://crbug.com/249953, support these devices.
@@ -192,11 +186,11 @@ void VideoCaptureDevice::GetDeviceSupportedFormats(
       while (ioctl(fd, VIDIOC_ENUM_FRAMEINTERVALS, &frame_interval) == 0) {
         if (frame_interval.type == V4L2_FRMIVAL_TYPE_DISCRETE) {
           if (frame_interval.discrete.numerator != 0) {
-            capture_capability.supported_format.frame_rate =
+            supported_format.frame_rate =
                 static_cast<float>(frame_interval.discrete.denominator) /
                 static_cast<float>(frame_interval.discrete.numerator);
           } else {
-            capture_capability.supported_format.frame_rate = 0;
+            supported_format.frame_rate = 0;
           }
         } else if (frame_interval.type == V4L2_FRMIVAL_TYPE_CONTINUOUS) {
           // TODO(mcasas): see http://crbug.com/249953, support these devices.
@@ -207,7 +201,7 @@ void VideoCaptureDevice::GetDeviceSupportedFormats(
           NOTIMPLEMENTED();
           break;
         }
-        formats->push_back(capture_capability);
+        supported_formats->push_back(supported_format);
         ++frame_interval.index;
       }
       ++frame_size.index;
diff --git a/media/video/capture/mac/video_capture_device_mac.mm b/media/video/capture/mac/video_capture_device_mac.mm
index 91c14ae..dba4fa1 100644
--- a/media/video/capture/mac/video_capture_device_mac.mm
+++ b/media/video/capture/mac/video_capture_device_mac.mm
@@ -83,7 +83,7 @@ void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
 
 // static
 void VideoCaptureDevice::GetDeviceSupportedFormats(const Name& device,
-    VideoCaptureCapabilities* formats) {
+    VideoCaptureFormats* formats) {
   NOTIMPLEMENTED();
 }
 
@@ -203,10 +203,17 @@ bool VideoCaptureDeviceMac::Init() {
   DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   DCHECK_EQ(state_, kNotInitialized);
 
+  // TODO(mcasas): The following check might not be necessary; if the device has
+  // disappeared after enumeration and before coming here, opening would just
+  // fail but not necessarily produce a crash.
   Names device_names;
   GetDeviceNames(&device_names);
-  Name* found = device_names.FindById(device_name_.id());
-  if (!found)
+  Names::iterator it = device_names.begin();
+  for (; it != device_names.end(); ++it) {
+    if (it->id() == device_name_.id())
+      break;
+  }
+  if (it == device_names.end())
     return false;
 
   if (AVFoundationGlue::IsAVFoundationSupported()) {
diff --git a/media/video/capture/video_capture_device.cc b/media/video/capture/video_capture_device.cc
index cd1f535..c370d09 100644
--- a/media/video/capture/video_capture_device.cc
+++ b/media/video/capture/video_capture_device.cc
@@ -17,15 +17,6 @@ const std::string VideoCaptureDevice::Name::GetNameAndModel() const {
   return device_name_ + suffix;
 }
 
-VideoCaptureDevice::Name*
-VideoCaptureDevice::Names::FindById(const std::string& id) {
-  for (iterator it = begin(); it != end(); ++it) {
-    if (it->id() == id)
-      return &(*it);
-  }
-  return NULL;
-}
-
 VideoCaptureDevice::~VideoCaptureDevice() {}
 
 }  // namespace media
diff --git a/media/video/capture/video_capture_device.h b/media/video/capture/video_capture_device.h
index b582093..295401c 100644
--- a/media/video/capture/video_capture_device.h
+++ b/media/video/capture/video_capture_device.h
@@ -112,14 +112,7 @@ class MEDIA_EXPORT VideoCaptureDevice {
   };
 
   // Manages a list of Name entries.
-  class MEDIA_EXPORT Names
-      : public NON_EXPORTED_BASE(std::list<Name>) {
-   public:
-    // Returns NULL if no entry was found by that ID.
-    Name* FindById(const std::string& id);
-
-    // Allow generated copy constructor and assignment.
-  };
+  typedef std::list<Name> Names;
 
   class MEDIA_EXPORT Client {
    public:
@@ -199,14 +192,14 @@ class MEDIA_EXPORT VideoCaptureDevice {
   // Gets the names of all video capture devices connected to this computer.
   static void GetDeviceNames(Names* device_names);
 
-  // Gets the capabilities 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.
+  // 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,
-                                        VideoCaptureCapabilities* formats);
+                                        VideoCaptureFormats* supported_formats);
 
-  // Prepare the camera for use. After this function has been called no other
+  // 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,
diff --git a/media/video/capture/video_capture_device_unittest.cc b/media/video/capture/video_capture_device_unittest.cc
index 510fe1d..5e05ad4 100644
--- a/media/video/capture/video_capture_device_unittest.cc
+++ b/media/video/capture/video_capture_device_unittest.cc
@@ -367,12 +367,12 @@ TEST_F(VideoCaptureDeviceTest, GetDeviceSupportedFormats) {
     DVLOG(1) << "No camera available. Exiting test.";
     return;
   }
-  VideoCaptureCapabilities capture_capabilities;
+  VideoCaptureFormats supported_formats;
   VideoCaptureDevice::Names::iterator names_iterator;
   for (names_iterator = names_.begin(); names_iterator != names_.end();
        ++names_iterator) {
     VideoCaptureDevice::GetDeviceSupportedFormats(*names_iterator,
-                                                  &capture_capabilities);
+                                                  &supported_formats);
     // Nothing to test here since we cannot forecast the hardware capabilities.
   }
 }
@@ -411,20 +411,22 @@ TEST_F(VideoCaptureDeviceTest, FakeGetDeviceSupportedFormats) {
   VideoCaptureDevice::Names names;
   FakeVideoCaptureDevice::GetDeviceNames(&names);
 
-  VideoCaptureCapabilities capture_capabilities;
+  VideoCaptureFormats supported_formats;
   VideoCaptureDevice::Names::iterator names_iterator;
 
   for (names_iterator = names.begin(); names_iterator != names.end();
        ++names_iterator) {
     FakeVideoCaptureDevice::GetDeviceSupportedFormats(*names_iterator,
-                                                      &capture_capabilities);
-    EXPECT_GE(capture_capabilities.size(), 1u);
-    EXPECT_EQ(capture_capabilities[0].supported_format.frame_size.width(), 640);
-    EXPECT_EQ(capture_capabilities[0].supported_format.frame_size.height(),
-              480);
-    EXPECT_EQ(capture_capabilities[0].supported_format.pixel_format,
-              media::PIXEL_FORMAT_I420);
-    EXPECT_GE(capture_capabilities[0].supported_format.frame_rate, 20);
+                                                      &supported_formats);
+    EXPECT_EQ(supported_formats.size(), 2u);
+    EXPECT_EQ(supported_formats[0].frame_size.width(), 640);
+    EXPECT_EQ(supported_formats[0].frame_size.height(), 480);
+    EXPECT_EQ(supported_formats[0].pixel_format, media::PIXEL_FORMAT_I420);
+    EXPECT_GE(supported_formats[0].frame_rate, 20);
+    EXPECT_EQ(supported_formats[1].frame_size.width(), 320);
+    EXPECT_EQ(supported_formats[1].frame_size.height(), 240);
+    EXPECT_EQ(supported_formats[1].pixel_format, media::PIXEL_FORMAT_I420);
+    EXPECT_GE(supported_formats[1].frame_rate, 20);
   }
 }
 
diff --git a/media/video/capture/video_capture_types.cc b/media/video/capture/video_capture_types.cc
index 96e5236..aee3865 100644
--- a/media/video/capture/video_capture_types.cc
+++ b/media/video/capture/video_capture_types.cc
@@ -31,6 +31,4 @@ bool VideoCaptureFormat::IsValid() const {
 
 VideoCaptureParams::VideoCaptureParams() : allow_resolution_change(false) {}
 
-VideoCaptureCapability::VideoCaptureCapability() {}
-
 }  // namespace media
diff --git a/media/video/capture/video_capture_types.h b/media/video/capture/video_capture_types.h
index d06c1b4..6a4f453 100644
--- a/media/video/capture/video_capture_types.h
+++ b/media/video/capture/video_capture_types.h
@@ -32,7 +32,8 @@ enum VideoPixelFormat {
 
 // Video capture format specification.
 // This class is used by the video capture device to specify the format of every
-// frame captured and returned to a client.
+// frame captured and returned to a client. It is also used to specify a
+// supported capture format by a device.
 class MEDIA_EXPORT VideoCaptureFormat {
  public:
   VideoCaptureFormat();
@@ -49,6 +50,8 @@ class MEDIA_EXPORT VideoCaptureFormat {
   VideoPixelFormat pixel_format;
 };
 
+typedef std::vector<VideoCaptureFormat> VideoCaptureFormats;
+
 // Parameters for starting video capture.
 // This class is used by the client of a video capture device to specify the
 // format of frames in which the client would like to have captured frames
@@ -64,19 +67,6 @@ class MEDIA_EXPORT VideoCaptureParams {
   bool allow_resolution_change;
 };
 
-// Capabilities describe the format a camera captures video in.
-// This class is used by the video capture device to report the formats in which
-// it is capable of capturing frames.
-class MEDIA_EXPORT VideoCaptureCapability {
- public:
-  VideoCaptureCapability();
-
-  // Supported resolution and format.
-  VideoCaptureFormat supported_format;
-};
-
-typedef std::vector<VideoCaptureCapability> VideoCaptureCapabilities;
-
 }  // namespace media
 
 #endif  // MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_TYPES_H_
diff --git a/media/video/capture/win/video_capture_device_win.cc b/media/video/capture/win/video_capture_device_win.cc
index cdb10ff..00056a70 100644
--- a/media/video/capture/win/video_capture_device_win.cc
+++ b/media/video/capture/win/video_capture_device_win.cc
@@ -166,7 +166,7 @@ void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
 
 // static
 void VideoCaptureDevice::GetDeviceSupportedFormats(const Name& device,
-    VideoCaptureCapabilities* formats) {
+    VideoCaptureFormats* formats) {
   NOTIMPLEMENTED();
 }