Change class VideoCaptureDevice::Name to struct VideoCaptureDeviceDescriptor

media/capture/video/video_capture_device_descriptor.h

Index: media/capture/video/video_capture_device_descriptor.h
diff --git a/media/capture/video/video_capture_device_descriptor.h b/media/capture/video/video_capture_device_descriptor.h
new file mode 100644
index 0000000000000000000000000000000000000000..9964deb1b203128b6cfeefb92ac8aa9976511ead
--- /dev/null
+++ b/media/capture/video/video_capture_device_descriptor.h
@@ -0,0 +1,74 @@
+// 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.
+
+#ifndef MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_DESCRIPTOR_H_
+#define MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_DESCRIPTOR_H_
+
+#include <list>
+#include <string>
+
+#include "media/capture/capture_export.h"
+
+namespace media {
+
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.media
+enum class VideoCaptureApiType {
+  LINUX_V4L2_SINGLE_PLANE,
+  WINDOWS_MEDIA_FOUNDATION,
+  WINDOWS_DIRECT_SHOW,
+  MACOSX_AVFOUNDATION,
+  MACOSX_DECKLINK,
+  ANDROID_API1,
+  ANDROID_API2_LEGACY,
+  ANDROID_API2_FULL,
+  ANDROID_API2_LIMITED,
+  ANDROID_TANGO,
+  API_TYPE_UNKNOWN
+};
+
+enum class VideoCaptureTransportType {
+  // For AVFoundation Api, identify devices that are built-in or USB.
+  MACOSX_USB_OR_BUILT_IN,
+  OTHER_TRANSPORT

[emircan, 2016/07/25 22:56:40]

Can we split OTHER_TRANSPORT into two: MACOSX_OTHER_TRANSPORT, UNKNOWN? UNKNOWN
would be more clear in covering all the other platform cases.

[chfremer, 2016/07/26 16:56:35]

I initially had the same thought, but then I couldn't find a reason why we would
want to distinguish the different platforms. Note that the situation is similar
with API_TYPE_UNKNOWN, where we also don't distinguish different platforms. 

And if we do distinguish, we would need an #idfef in the constructor
implementation to initialize the Mac OSX case differently from the other
platforms.

+};
+
+// Represents information about a capture device as returned by a
+// VideoCaptureDeviceFactory.
+// |device_id| represents a unique id of a physical device. Since the same
+// physical device may be accessible through different APIs |capture_api|
+// disambiguates the API.

[emircan, 2016/07/25 22:56:40]

Earlier there was a comment about using VideoCaptureDevice::GetDeviceNames to
create an instance. Does that apply for GetDeviceDescriptors?

[chfremer, 2016/07/26 16:56:35]

I believe you are referring to this:
  // You should not create an instance of this class directly by e.g. setting
  // various properties directly.  Instead use
  // VideoCaptureDevice::GetDeviceNames to do this for you and if you need to
  // cache your own copy of a name, you can do so via the copy constructor.
  // The reason for this is that a device name might contain platform specific
  // settings that are relevant only to the platform specific implementation of
  // VideoCaptureDevice::Create.

This description seems to be more an explanation of the intended use of the
VideoCaptureDeviceFactory API than an description of the struct itself. I
revised my description now and am trying to convey the same information. I also
put more explanations into the description of VideoDeviceCaptureFactory.

+struct CAPTURE_EXPORT VideoCaptureDeviceDescriptor {
+ public:
+  VideoCaptureDeviceDescriptor();
+  VideoCaptureDeviceDescriptor(const VideoCaptureDeviceDescriptor& other);
+  ~VideoCaptureDeviceDescriptor();
+
+  // These operators are needed due to storing the name in an STL container.
+  // In the shared build, all methods from the STL container will be exported
+  // so even though they're not used, they're still depended upon.
+  bool operator==(const VideoCaptureDeviceDescriptor& other) const {
+    return (other.device_id == device_id) && (other.capture_api == capture_api);
+  }
+  bool operator<(const VideoCaptureDeviceDescriptor& other) const {
+    if (device_id < other.device_id)
+      return true;
+    return capture_api < other.capture_api;
+  }
+
+  const char* GetCaptureApiTypeString() const;
+
+  std::string friendly_name;

[emircan, 2016/07/25 22:56:40]

s/friendly_name/name/? Or drop a comment about why you call it friendly_name.

[chfremer, 2016/07/26 16:56:35]

I chose this, because in the original code in VideoCaptureDevice::Name the
accessor method name() had a comment explaining that this is the "Friendly name
of a device". By choosing this name I was hoping to eliminate the need for the
comment, which apparently did not work out :-).

Is there any name I can choose for this field that is self-explaning enough to
not require a comment?
How would you feel about "display_name"?

For now, I am adding a comment back in.

[emircan, 2016/07/28 21:23:01]

|display_name| sgtm

[chfremer, 2016/07/29 18:09:50]

Done.

+  std::string device_id;
+  VideoCaptureApiType capture_api;
+  VideoCaptureTransportType transport_type;
+
+  // Allow generated copy constructor and assignment.

[emircan, 2016/07/25 22:56:40]

This comment is unnecessary.

[chfremer, 2016/07/26 16:56:35]

removed

+};
+
+typedef std::list<VideoCaptureDeviceDescriptor> VideoCaptureDeviceDescriptors;
+
+}  // namespace media
+
+#endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_DESCRIPTOR_H_