Added video capture capabilities retrieval and caching to VideoCaptureManager

content/browser/renderer_host/media/video_capture_manager.cc

 // 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.
 
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 21 matching lines @@
 VideoCaptureManager::DeviceEntry::DeviceEntry(
     MediaStreamType stream_type,
     const std::string& id,
     scoped_ptr<VideoCaptureController> controller)
     : stream_type(stream_type),
       id(id),
       video_capture_controller(controller.Pass()) {}
 
 VideoCaptureManager::DeviceEntry::~DeviceEntry() {}
 
+VideoCaptureManager::DeviceInfo::DeviceInfo() {}
+
+VideoCaptureManager::DeviceInfo::DeviceInfo(
+    const media::VideoCaptureDevice::Name& name,
+    const media::VideoCaptureCapabilities& capabilities)
+    : device_in_use_(false),
+      capability_in_use_(),
+      name_(name),
+      capabilities_(capabilities) {}
+
+VideoCaptureManager::DeviceInfo::~DeviceInfo() {}
+
 VideoCaptureManager::VideoCaptureManager()
     : listener_(NULL),
       new_capture_session_id_(1),
       use_fake_device_(false) {
 }
 
 VideoCaptureManager::~VideoCaptureManager() {
   DCHECK(devices_.empty());
 }
 
 void VideoCaptureManager::Register(MediaStreamProviderListener* listener,
                                    base::MessageLoopProxy* device_thread_loop) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!listener_);
   DCHECK(!device_loop_.get());
   listener_ = listener;
   device_loop_ = device_thread_loop;
 }
 
 void VideoCaptureManager::Unregister() {
   DCHECK(listener_);
   listener_ = NULL;
 }
 
 void VideoCaptureManager::EnumerateDevices(MediaStreamType stream_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << "VideoCaptureManager::EnumerateDevices, type " << stream_type;
   DCHECK(listener_);
   base::PostTaskAndReplyWithResult(
-      device_loop_, FROM_HERE,
-      base::Bind(&VideoCaptureManager::GetAvailableDevicesOnDeviceThread, this,
+      device_loop_,
+      FROM_HERE,
+      base::Bind(&VideoCaptureManager::GetAvailableDevicesOnDeviceThread,
+                 this,
                  stream_type),
-      base::Bind(&VideoCaptureManager::OnDevicesEnumerated, this, stream_type));
+      base::Bind(
+          &VideoCaptureManager::OnDeviceNamesEnumerated, this, stream_type));
 }
 
 int VideoCaptureManager::Open(const StreamDeviceInfo& device_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(listener_);
 
   // Generate a new id for the session being opened.
   const int capture_session_id = new_capture_session_id_++;
 
   DCHECK(sessions_.find(capture_session_id) == sessions_.end());
@@ skipping 41 matching lines @@
                  capture_session_id));
   sessions_.erase(session_it);
 }
 
 void VideoCaptureManager::UseFakeDevice() {
   use_fake_device_ = true;
 }
 
 void VideoCaptureManager::DoStartDeviceOnDeviceThread(
     DeviceEntry* entry,
+    const media::VideoCaptureDevice::Name* device_name,
     const media::VideoCaptureCapability& capture_params,
     scoped_ptr<media::VideoCaptureDevice::Client> device_client) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(IsOnDeviceThread());
 
   scoped_ptr<media::VideoCaptureDevice> video_capture_device;
   switch (entry->stream_type) {
     case MEDIA_DEVICE_VIDEO_CAPTURE: {
       // 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);
-      if (found) {
-        video_capture_device.reset(use_fake_device_ ?
-            media::FakeVideoCaptureDevice::Create(*found) :
-            media::VideoCaptureDevice::Create(*found));
+      if (device_name) {
+        video_capture_device.reset(use_fake_device_
+            ? media::FakeVideoCaptureDevice::Create(*device_name)
+            : media::VideoCaptureDevice::Create(*device_name));
       }
       break;
     }
     case MEDIA_TAB_VIDEO_CAPTURE: {
       video_capture_device.reset(
           WebContentsVideoCaptureDevice::Create(entry->id));
       break;
     }
     case MEDIA_DESKTOP_VIDEO_CAPTURE: {
 #if defined(ENABLE_SCREEN_CAPTURE)
@@ skipping 46 matching lines @@
     DVLOG(1) << "VideoCaptureManager starting device (type = "
              << entry->stream_type << ", id = " << entry->id << ")";
 
     media::VideoCaptureCapability params_as_capability;
     params_as_capability.width = params.requested_format.width;
     params_as_capability.height = params.requested_format.height;
     params_as_capability.frame_rate = params.requested_format.frame_rate;
     params_as_capability.frame_size_type =
         params.requested_format.frame_size_type;
 
+    DeviceInfo* found = FindDeviceInfoById(entry->id);
+    if (found) {
+      found->device_in_use_ = true;

[ncarter (slow), 2013/11/07 22:43:01]

The VideoCaptureManager already knows whether a particular device is in use or
not, on the basis of whether an active DeviceEntry exists with that id and type,
so I wonder if it's redundant to track it in a bool here in a parallel struct.

[mcasas, 2013/11/08 02:45:21]

That's true. It implies an extra search, but that's not as bad as duplicating 
info. So, done away with |device_in_use|.

+      found->capability_in_use_ = params_as_capability;

[ncarter (slow), 2013/11/07 22:43:01]

Would it be better to just cache the Capability in the Controller?

If we track it in the DeviceInfo, then we wind up with three different structs
that are tracking the current active state of the device: the DeviceEntry, the
Controller, and now the DeviceInfo.

[mcasas, 2013/11/08 02:45:21]

Struct DeviceEntry and struct DeviceInfo actually cache different things, 
the first one being a dynamic id-controller*-device* used during the 
device's runtime, and DeviceInfo which is mostly static, containing names
and capabilities. Moreover they are created on different moments, the
DeviceInfo on early times and DeviceEntry during action (capture) time.
The only touching point is the case of the capability in use. Do you think 
is ok to keep |capability_in_use_| and just remove |device_in_use_|? (L.235)

+    }
     device_loop_->PostTask(FROM_HERE, base::Bind(
         &VideoCaptureManager::DoStartDeviceOnDeviceThread, this,
-        entry, params_as_capability,
+        entry,
+        (found ? & found->name_ : NULL),
+        params_as_capability,
         base::Passed(entry->video_capture_controller->NewDeviceClient())));
   }
   // Run the callback first, as AddClient() may trigger OnFrameInfo().
   done_cb.Run(entry->video_capture_controller->GetWeakPtr());
   entry->video_capture_controller->AddClient(client_id,
                                              client_handler,
                                              client_render_process,
                                              params);
 }
 
@@ skipping 13 matching lines @@
 
   // Detach client from controller.
   int session_id = controller->RemoveClient(client_id, client_handler);
   DVLOG(1) << "VideoCaptureManager::StopCaptureForClient, session_id = "
            << session_id;
 
   // If controller has no more clients, delete controller and device.
   DestroyDeviceEntryIfNoClients(entry);
 }
 
+void VideoCaptureManager::GetDeviceCapabilities(

[ncarter (slow), 2013/11/07 22:43:01]

How does the client of this API know when to call this function? It seems to
become valid only after the device enumeration event occurs. Is that the
intended usage -- you request enumeration, and when the enumeration completes,
you call back to get the extra info here?

[mcasas, 2013/11/08 02:45:21]

Yes exactly. There's a comment in the header file:
// Retrieve the available capture capabilities for a particular device. The
// capabilities are cached during device(s) enumeration and updated as clients
// open/close the device(s).

Next CL(s) will call wire the logic to call this method whenever
RTCVideoCapturer::GetSupportedFormats() is used during capabilities
negotiation in renderer side.

+    int capture_session_id,
+    media::VideoCaptureCapabilities* capabilities) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  capabilities->clear();
+
+  std::map<int, MediaStreamDevice>::iterator it =
+      sessions_.find(capture_session_id);
+  DCHECK(it != sessions_.end());
+  DVLOG(1) << "GetDeviceCapabilities for device: " << it->second.name;
+
+  DeviceInfo* device = FindDeviceInfoById(it->second.id);
+  if (device) {
+    if (device->device_in_use_)
+      capabilities->push_back(device->capability_in_use_);
+    else
+      *capabilities = device->capabilities_;
+  }
+}
+
 void VideoCaptureManager::DoStopDeviceOnDeviceThread(DeviceEntry* entry) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StopDeviceTime");
   DCHECK(IsOnDeviceThread());
   if (entry->video_capture_device) {
     entry->video_capture_device->StopAndDeAllocate();
   }
   entry->video_capture_device.reset();
 }
 
 void VideoCaptureManager::OnOpened(MediaStreamType stream_type,
                                    int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!listener_) {
     // Listener has been removed.
     return;
   }
   listener_->Opened(stream_type, capture_session_id);
 }
 
 void VideoCaptureManager::OnClosed(MediaStreamType stream_type,
                                    int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!listener_) {
     // Listener has been removed.
     return;
   }
   listener_->Closed(stream_type, capture_session_id);
 }
 
-void VideoCaptureManager::OnDevicesEnumerated(
+void VideoCaptureManager::OnDeviceNamesEnumerated(
     MediaStreamType stream_type,
-    const media::VideoCaptureDevice::Names& device_names) {
+    const media::VideoCaptureDevice::Names& names_snapshot) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
   if (!listener_) {

[ncarter (slow), 2013/11/07 22:43:01]

Why do we check the listener_, if we're not invoking the listener?

[mcasas, 2013/11/08 02:45:21]

It's used in OnDeviceNameAndCapabilitiesEnumerated()
but if there's no listener there's no sense in continuing with the
logic at this point. Plus there are even unittests that hit this
transient: |listener_| gets removed between GetAvailableDevicesOnDeviceThread
and this method !

     // Listener has been removed.
     return;
   }
 
-  // Transform from VCD::Name to StreamDeviceInfo.
-  StreamDeviceInfoArray devices;
-  for (media::VideoCaptureDevice::Names::const_iterator it =
-           device_names.begin(); it != device_names.end(); ++it) {
-    devices.push_back(StreamDeviceInfo(
-        stream_type, it->GetNameAndModel(), it->id()));
+  // Eliminate from |devices_info_cache_| all devices not in the snapshot.
+  ScopedVector<DeviceInfo>::iterator it_device_info =
+      devices_info_cache_.begin();
+  while(it_device_info != devices_info_cache_.end()) {

[ncarter (slow), 2013/11/07 22:43:01]

space after while

[mcasas, 2013/11/08 02:45:21]

Done.

+    media::VideoCaptureDevice::Names::const_iterator it;
+    for (it = names_snapshot.begin(); it != names_snapshot.end(); ++it) {
+      if ((*it_device_info)->name_.id() == it->id())
+        break;
+    }
+    if (it == names_snapshot.end())
+      devices_info_cache_.erase(it_device_info++);
+    else
+      ++it_device_info;
   }
 
-  listener_->DevicesEnumerated(stream_type, devices);
+  // Truly new devices are put in |new_device_names|, and we need to retrieve
+  // their capabilities.
+  media::VideoCaptureDevice::Names new_device_names;
+  for (media::VideoCaptureDevice::Names::const_iterator it =
+           names_snapshot.begin(); it != names_snapshot.end(); ++it) {
+    DeviceInfo* device_info = FindDeviceInfoById(it->id());
+    if (!device_info)
+      new_device_names.push_back(*it);
+  }
+  base::PostTaskAndReplyWithResult(
+      device_loop_, FROM_HERE,
+      base::Bind(&VideoCaptureManager::GetDevicesCapabilitiesOnDeviceThread,
+                 this, new_device_names),
+      base::Bind(&VideoCaptureManager::OnDeviceNameAndCapabilitiesEnumerated,
+                 this, stream_type));
+}
+
+void VideoCaptureManager::OnDeviceNameAndCapabilitiesEnumerated(
+    MediaStreamType stream_type,
+    ScopedVector<DeviceInfo> new_devices_info) {
+  DVLOG(1) << "OnDeviceNameAndCapabilitiesEnumerated, #new devices: "
+           << new_devices_info.size();
+
+  ScopedVector<DeviceInfo>::iterator it = new_devices_info.begin();
+  while (it != new_devices_info.end()) {
+    devices_info_cache_.push_back(*it);
+    it = new_devices_info.weak_erase(it);
+  }
+
+  // Walk the |devices_info_cache_|| and transform from VCD::Name to

[ncarter (slow), 2013/11/07 22:43:01]

|| -> |

[mcasas, 2013/11/08 02:45:21]

Done.

+  // StreamDeviceInfo for return purposes.
+  StreamDeviceInfoArray devices;
+  for (ScopedVector<DeviceInfo>::const_iterator it =
+          devices_info_cache_.begin();
+      it != devices_info_cache_.end(); ++it) {

[perkj_chrome, 2013/11/07 10:49:11]

nit: indentation

[mcasas, 2013/11/07 17:14:30]

Ok, clang-format for Chromium style, and we leave it to it.

+    devices.push_back(StreamDeviceInfo(
+        stream_type, (*it)->name_.GetNameAndModel(), (*it)->name_.id()));
+  }
+  // |listener_| might have disappeared while we were scanning the devices.
+  if(listener_)

[ncarter (slow), 2013/11/07 22:43:01]

space after the if

[mcasas, 2013/11/08 02:45:21]

Done.

+    listener_->DevicesEnumerated(stream_type, devices);
 }
 
 bool VideoCaptureManager::IsOnDeviceThread() const {
   return device_loop_->BelongsToCurrentThread();
 }
 
 media::VideoCaptureDevice::Names
 VideoCaptureManager::GetAvailableDevicesOnDeviceThread(
     MediaStreamType stream_type) {
   SCOPED_UMA_HISTOGRAM_TIMER(
       "Media.VideoCaptureManager.GetAvailableDevicesTime");
   DCHECK(IsOnDeviceThread());
-  media::VideoCaptureDevice::Names result;
+  media::VideoCaptureDevice::Names names;
 
   switch (stream_type) {
     case MEDIA_DEVICE_VIDEO_CAPTURE:
-      // Cache the latest enumeration of video capture devices.
-      // We'll refer to this list again in OnOpen to avoid having to
-      // enumerate the devices again.
-      if (!use_fake_device_) {
-        media::VideoCaptureDevice::GetDeviceNames(&result);
-      } else {
-        media::FakeVideoCaptureDevice::GetDeviceNames(&result);
-      }
-
-      // 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;
+      if (!use_fake_device_)
+        media::VideoCaptureDevice::GetDeviceNames(&names);
+      else
+        media::FakeVideoCaptureDevice::GetDeviceNames(&names);
       break;
 
     case MEDIA_DESKTOP_VIDEO_CAPTURE:
       // Do nothing.
       break;
 
     default:
       NOTREACHED();
       break;
   }
-  return result;
+  return names;
+}
+
+ScopedVector<VideoCaptureManager::DeviceInfo>
+VideoCaptureManager::GetDevicesCapabilitiesOnDeviceThread(
+    const media::VideoCaptureDevice::Names& device_names) {
+  DCHECK(IsOnDeviceThread());
+  DVLOG(1) << "GetDevicesCapabilitiesOnDeviceThread";
+
+  ScopedVector<DeviceInfo> device_info_vector;
+  for (media::VideoCaptureDevice::Names::const_iterator it =
+          device_names.begin(); it != device_names.end(); ++it) {
+    media::VideoCaptureCapabilities capabilities;
+    DeviceInfo* device_info = new DeviceInfo((*it),
+        media::VideoCaptureCapabilities());
+    if (!use_fake_device_) {
+      media::VideoCaptureDevice::GetDeviceSupportedFormats(*it,
+          &(device_info->capabilities_));
+    } else {
+      media::FakeVideoCaptureDevice::GetDeviceSupportedFormats(*it,
+          &(device_info->capabilities_));
+    }
+    device_info_vector.push_back(device_info);
+  }
+  return device_info_vector.Pass();
 }
 
 VideoCaptureManager::DeviceEntry*
 VideoCaptureManager::GetDeviceEntryForMediaStreamDevice(
     const MediaStreamDevice& device_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   for (DeviceEntries::iterator it = devices_.begin();
        it != devices_.end(); ++it) {
     DeviceEntry* device = *it;
@@ skipping 26 matching lines @@
              << entry->stream_type << ", id = " << entry->id << ")";
 
     // The DeviceEntry is removed from |devices_| immediately. The controller is
     // deleted immediately, and the device is freed asynchronously. After this
     // point, subsequent requests to open this same device ID will create a new
     // DeviceEntry, VideoCaptureController, and VideoCaptureDevice.
     devices_.erase(entry);
     entry->video_capture_controller.reset();
     device_loop_->PostTask(
         FROM_HERE,
-        base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread, this,
+        base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread,
+                   this,
                    base::Owned(entry)));
+    DeviceInfo* device_info = FindDeviceInfoById(entry->id);
+    if (device_info)
+      device_info->device_in_use_ = false;
   }
 }
 
 VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
     int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   std::map<int, MediaStreamDevice>::iterator session_it =
       sessions_.find(capture_session_id);
   if (session_it == sessions_.end()) {
@@ skipping 12 matching lines @@
 
   scoped_ptr<VideoCaptureController> video_capture_controller(
       new VideoCaptureController());
   DeviceEntry* new_device = new DeviceEntry(device_info.type,
                                             device_info.id,
                                             video_capture_controller.Pass());
   devices_.insert(new_device);
   return new_device;
 }
 
+VideoCaptureManager::DeviceInfo* VideoCaptureManager::FindDeviceInfoById(
+    const std::string& id) {
+  for (ScopedVector<DeviceInfo>::iterator it = devices_info_cache_.begin();
+       it != devices_info_cache_.end(); ++it) {
+    if ((*it)->name_.id() == id)
+      return *it;
+  }
+  return NULL;
+}
+
 }  // namespace content