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..9de3782388acc597bd1698ada2fcd736a49da140
--- /dev/null
+++ b/media/capture/video/video_capture_device_descriptor.h
@@ -0,0 +1,75 @@
+// 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 {

[mcasas, 2016/07/26 23:52:03]

s/VideoCaptureApiType/VideoCaptureAPI/ (or Api)

There's no need to further identify with "Type", IMHO.

[chfremer, 2016/07/27 23:10:18]

Done.

+  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

[mcasas, 2016/07/26 23:52:04]

Since it's an enum class, the name will
always prefix this entry, and
VideoCaptureApi::API_TYPE_UNKNOWN, is a
bit verbose, so just make it
VideoCaptureApi::UNKNOWN

[chfremer, 2016/07/27 23:10:17]

Done.

+};
+
+enum class VideoCaptureTransportType {
+  // For AVFoundation Api, identify devices that are built-in or USB.
+  MACOSX_USB_OR_BUILT_IN,
+  OTHER_TRANSPORT
+};
+
+// Represents information about a capture device as returned by
+// VideoCaptureDeviceFactory::EnumerateDeviceDescriptors().
+// |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.
+struct CAPTURE_EXPORT VideoCaptureDeviceDescriptor {

[mcasas, 2016/07/26 23:52:04]

I'd err on the side of making this a class [1]

[1] https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes

[chfremer, 2016/07/27 23:10:18]

After reviewing the styleguide, I still feel struct is the right choice here.
The only behavior implemented here is GetCaptureApiTypeString(), and this does
not really belong here.
Would you feel more comfortable with it being a struct if I moved that method
out?

[mcasas, 2016/07/28 00:05:42]

Hmm I'll leave the decision to you. 

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

Okay, then I'll leave it a struct for now.

+ public:
+  VideoCaptureDeviceDescriptor();
+  VideoCaptureDeviceDescriptor(const std::string friendly_name,
+                               const std::string device_id,
+                               VideoCaptureApiType capture_api);
+  VideoCaptureDeviceDescriptor(const VideoCaptureDeviceDescriptor& other);

[mcasas, 2016/07/26 23:52:04]

We used to have a ctor with a |transport_type|
argument, used in Mac, right? [1]

[1]
https://cs.chromium.org/chromium/src/media/capture/video/video_capture_device...

[chfremer, 2016/07/27 23:10:18]

True. I had removed all constructors for initializing the struct in order to
treat it as "plain old data" and have clients use the empty constructor followed
by assigning to fields by name.

Since emircan@ already suggested the use of emplace_back(), which requires
constructors, I am now adding the other constructors back in and use them
wherever possible.

+  ~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;  // Name that is intended for display in the UI
+  std::string device_id;
+  VideoCaptureApiType capture_api;

[mcasas, 2016/07/26 23:52:04]

Make this three const?

And, if possible, |transport_type| too (if
we bring back the constructor with it).

[chfremer, 2016/07/27 23:10:18]

We need the ability to assign values when we fill default-constructed instances
that are passed into VideoCaptureDeviceFactory::GetDeviceInfo(). This is done in
order to avoid the need for returning a copy of VideoCaptureDeviceInfo from that
method.

[mcasas, 2016/07/28 00:05:42]

Acknowledged.

+  VideoCaptureTransportType transport_type;
+};
+
+typedef std::list<VideoCaptureDeviceDescriptor> VideoCaptureDeviceDescriptors;

[mcasas, 2016/07/26 23:52:03]

using VideoCaptureDeviceDescriptors = std::list<VideoCaptureDeviceDescriptor>;

(typedef is the old thing).

[mcasas, 2016/07/28 00:05:42]

ping

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

Done.

+
+}  // namespace media
+
+#endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_DESCRIPTOR_H_