[Mojo Video Capture] Use string keys instead of VideoCaptureDeviceDescriptor

services/video_capture/device_factory_media_to_mojo_adapter.cc

 // Copyright 2016 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 "services/video_capture/device_factory_media_to_mojo_adapter.h"
 
 #include <sstream>
 
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
@@ skipping 21 matching lines @@
     std::unique_ptr<media::VideoCaptureDeviceFactory> device_factory,
     const media::VideoCaptureJpegDecoderFactoryCB&
         jpeg_decoder_factory_callback)
     : device_factory_(std::move(device_factory)),
       jpeg_decoder_factory_callback_(jpeg_decoder_factory_callback) {}
 
 DeviceFactoryMediaToMojoAdapter::~DeviceFactoryMediaToMojoAdapter() = default;
 
 void DeviceFactoryMediaToMojoAdapter::EnumerateDeviceDescriptors(
     const EnumerateDeviceDescriptorsCallback& callback) {
-  media::VideoCaptureDeviceDescriptors descriptors;
+  std::vector<media::VideoCaptureDeviceDescriptor> descriptors;
   device_factory_->GetDeviceDescriptors(&descriptors);
-  callback.Run(descriptors);
+  callback.Run(std::move(descriptors));
 }
 
 void DeviceFactoryMediaToMojoAdapter::GetSupportedFormats(
-    const media::VideoCaptureDeviceDescriptor& device_descriptor,
+    const std::string& device_id,
     const GetSupportedFormatsCallback& callback) {
+  std::vector<media::VideoCaptureDeviceDescriptor> descriptors;
+  device_factory_->GetDeviceDescriptors(&descriptors);
+  auto descriptor_iter = std::find_if(
+      descriptors.begin(), descriptors.end(),
+      [&device_id](const media::VideoCaptureDeviceDescriptor& descriptor) {
+        return descriptor.device_id == device_id;
+      });
+  media::VideoCaptureFormats media_formats;
+  if (descriptor_iter != descriptors.end()) {
+    device_factory_->GetSupportedFormats(*descriptor_iter, &media_formats);
+  }
   std::vector<VideoCaptureFormat> result;
-  std::vector<media::VideoCaptureFormat> media_formats;
-  device_factory_->GetSupportedFormats(device_descriptor, &media_formats);
   for (const auto& media_format : media_formats) {
     // The Video Capture Service requires devices to deliver frames either in
     // I420 or MJPEG formats.
     // TODO(chfremer): Add support for Y16 format. See crbug.com/624436.
     if (media_format.pixel_format != media::PIXEL_FORMAT_I420 &&
         media_format.pixel_format != media::PIXEL_FORMAT_MJPEG) {
       continue;
     }
     VideoCaptureFormat format;
     format.frame_size = media_format.frame_size;
     format.frame_rate = media_format.frame_rate;
     if (base::ContainsValue(result, format))
       continue;  // Result already contains this format
     result.push_back(format);
   }
   callback.Run(std::move(result));
 }
 
 void DeviceFactoryMediaToMojoAdapter::CreateDeviceProxy(
-    const media::VideoCaptureDeviceDescriptor& device_descriptor,
+    const std::string& device_id,
     mojom::VideoCaptureDeviceProxyRequest proxy_request,
     const CreateDeviceProxyCallback& callback) {
-  if (active_devices_.find(device_descriptor) != active_devices_.end()) {
+  auto active_device_iter = active_devices_by_id_.find(device_id);
+  if (active_device_iter != active_devices_by_id_.end()) {
     // The requested device is already in use.
     // Revoke the access and close the device, then bind to the new request.
-    ActiveDeviceEntry& device_entry = active_devices_[device_descriptor];
+    ActiveDeviceEntry& device_entry = active_device_iter->second;
     device_entry.binding->Unbind();
     device_entry.device_proxy->Stop();
     device_entry.binding->Bind(std::move(proxy_request));
     device_entry.binding->set_connection_error_handler(base::Bind(
         &DeviceFactoryMediaToMojoAdapter::OnClientConnectionErrorOrClose,
-        base::Unretained(this), device_descriptor));
+        base::Unretained(this), device_id));
     callback.Run(mojom::DeviceAccessResultCode::SUCCESS);
     return;
   }
 
+  // Create device
+  std::vector<media::VideoCaptureDeviceDescriptor> descriptors;
+  device_factory_->GetDeviceDescriptors(&descriptors);
+  auto descriptor_iter = std::find_if(

[yzshen1, 2016/11/14 17:56:21]

Something to consider: it may make sense to extract the code of finding a
specific descriptor, making it part of the device factory API. It seems a waste
of returning a vector and discard most of them immediately.

WDYT?

[chfremer, 2016/11/14 18:17:43]

I completely agree, but I would like to defer that refactoring to a later time.
VideoCaptureDeviceFactory has many platform-dependent implementations. What we
would need is a layer between those platform-specific implementations and the
API exposed to clients, where we can handle the mapping between device ids and
device descriptors.

I am adding crbug.com/665065 to track this.

+      descriptors.begin(), descriptors.end(),
+      [&device_id](const media::VideoCaptureDeviceDescriptor& descriptor) {
+        return descriptor.device_id == device_id;
+      });
+  if (descriptor_iter == descriptors.end()) {
+    callback.Run(mojom::DeviceAccessResultCode::ERROR_DEVICE_NOT_FOUND);
+    return;
+  }
   std::unique_ptr<media::VideoCaptureDevice> media_device =
-      device_factory_->CreateDevice(device_descriptor);
+      device_factory_->CreateDevice(*descriptor_iter);
   if (media_device == nullptr) {
     callback.Run(mojom::DeviceAccessResultCode::ERROR_DEVICE_NOT_FOUND);
     return;
   }
 
-  // Add entry to |active_devices| to keep track of it
+  // Add entry to active_devices to keep track of it
   ActiveDeviceEntry device_entry;
   device_entry.device_proxy = base::MakeUnique<VideoCaptureDeviceProxyImpl>(
       std::move(media_device), jpeg_decoder_factory_callback_);
   device_entry.binding =
       base::MakeUnique<mojo::Binding<mojom::VideoCaptureDeviceProxy>>(
           device_entry.device_proxy.get(), std::move(proxy_request));
   device_entry.binding->set_connection_error_handler(base::Bind(
       &DeviceFactoryMediaToMojoAdapter::OnClientConnectionErrorOrClose,
-      base::Unretained(this), device_descriptor));
-  active_devices_[device_descriptor] = std::move(device_entry);
+      base::Unretained(this), device_id));
+  active_devices_by_id_[device_id] = std::move(device_entry);
 
   callback.Run(mojom::DeviceAccessResultCode::SUCCESS);
 }
 
 void DeviceFactoryMediaToMojoAdapter::OnClientConnectionErrorOrClose(
-    const media::VideoCaptureDeviceDescriptor& descriptor) {
-  active_devices_[descriptor].device_proxy->Stop();
-  active_devices_.erase(descriptor);
+    const std::string& device_id) {
+  active_devices_by_id_[device_id].device_proxy->Stop();
+  active_devices_by_id_.erase(device_id);
 }
 
 }  // namespace video_capture