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)
+    : 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,

[ncarter (slow), 2013/11/09 01:49:11]

The interface to this function gets kind of weird with the addition of the
device_name parameter, because in the MEDIA_DEVICE_VIDEO_CAPTURE case, we never
read entry->id at all.

[mcasas, 2013/11/11 17:39:59]

Done.

     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);

[ncarter (slow), 2013/11/09 01:49:11]

Should we be doing this for only for MEDIA_DEVICE_VIDEO_CAPTURE ?

[mcasas, 2013/11/11 17:39:59]

Done.

+    if (found) {
+      found->capability_in_use_ = params_as_capability;

[ncarter (slow), 2013/11/09 01:49:11]

Because you're mirroring this state into the DeviceInfo, it seems you would need
a corresponding reset when the DeviceEntry goes away in
DestroyDeviceEntryIfNoClients.

For this reason I think it's better to just store params_as_capability in the
DeviceEntry or the controller. It is a valid thing to track in the controller.

[mcasas, 2013/11/11 17:39:59]

Done.

+    }
     device_loop_->PostTask(FROM_HERE, base::Bind(
         &VideoCaptureManager::DoStartDeviceOnDeviceThread, this,
-        entry, params_as_capability,
+        entry,
+        (found ? & found->name_ : NULL),

[ncarter (slow), 2013/11/09 01:49:11]

Isn't this use of found->name_ unsafe? What guarantees that the pointer will
still exist when it's dereferenced on the other thread?

[mcasas, 2013/11/11 17:39:59]

Done.

+        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(
+    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) {
+    DeviceEntry* const existing_device =
+         GetDeviceEntryForMediaStreamDevice(it->second);
+    if (existing_device)
+      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_) {
     // 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()) {
+    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));

[ncarter (slow), 2013/11/09 01:49:11]

Could we make this enumeration work with only one visitation of the device
thread (the current code does IO -> Device -> IO -> Device -> IO)?

A couple approaches come to mind.
 - Always enumerate the capabilities of all devices -- eliminate the caching,
and just return the device info along with the enumeration.
 - Keep a list of already-enumerated device IDs on the Device thread, and only
enumerate the capabilities of each once.

[mcasas, 2013/11/11 17:39:59]

See global comments :) short answer: yes 

+}
+
+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
+  // StreamDeviceInfo for return purposes.
+  StreamDeviceInfoArray devices;
+  for (ScopedVector<DeviceInfo>::const_iterator it =
+           devices_info_cache_.begin();
+       it != devices_info_cache_.end();
+       ++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_)
+    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)));
   }
 }
 
 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);
@@ skipping 13 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