Add support for multiple V4L2 video devices of the same type.

media/gpu/generic_v4l2_device.cc

Index: media/gpu/generic_v4l2_device.cc
diff --git a/media/gpu/generic_v4l2_device.cc b/media/gpu/generic_v4l2_device.cc
index c7d94904a1d3709050d405bd091a1c5774dbc7bc..4d96dd3c06fce992be1289360fc6651a2f21dfec 100644
--- a/media/gpu/generic_v4l2_device.cc
+++ b/media/gpu/generic_v4l2_device.cc
@@ -15,11 +15,13 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
+#include <algorithm>
 #include <memory>
 
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "base/posix/eintr_wrapper.h"
+#include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "media/gpu/generic_v4l2_device.h"
@@ -41,13 +43,6 @@ static const base::FilePath::CharType kV4l2Lib[] =
 
 namespace media {
 
-namespace {
-const char kDecoderDevice[] = "/dev/video-dec";
-const char kEncoderDevice[] = "/dev/video-enc";
-const char kImageProcessorDevice[] = "/dev/image-proc0";
-const char kJpegDecoderDevice[] = "/dev/jpeg-dec";
-}
-
 GenericV4L2Device::GenericV4L2Device(Type type) : V4L2Device(type) {
 #if defined(USE_LIBV4L2)
   use_libv4l2_ = false;
@@ -55,13 +50,11 @@ GenericV4L2Device::GenericV4L2Device(Type type) : V4L2Device(type) {
 }
 
 GenericV4L2Device::~GenericV4L2Device() {
-#if defined(USE_LIBV4L2)
-  if (use_libv4l2_ && device_fd_.is_valid())
-    v4l2_close(device_fd_.release());
-#endif
+  CloseDevice();
 }
 
 int GenericV4L2Device::Ioctl(int request, void* arg) {
+  DCHECK(device_fd_.is_valid());
 #if defined(USE_LIBV4L2)
   if (use_libv4l2_)
     return HANDLE_EINTR(v4l2_ioctl(device_fd_.get(), request, arg));
@@ -99,6 +92,7 @@ void* GenericV4L2Device::Mmap(void* addr,
                               int prot,
                               int flags,
                               unsigned int offset) {
+  DCHECK(device_fd_.is_valid());
   return mmap(addr, len, prot, flags, device_fd_.get(), offset);
 }
 
@@ -136,48 +130,34 @@ bool GenericV4L2Device::ClearDevicePollInterrupt() {
 }
 
 bool GenericV4L2Device::Initialize() {
-  const char* device_path = NULL;
   static bool v4l2_functions_initialized = PostSandboxInitialization();
   if (!v4l2_functions_initialized) {
     LOG(ERROR) << "Failed to initialize LIBV4L2 libs";
     return false;
   }
 
-  switch (type_) {
-    case kDecoder:
-      device_path = kDecoderDevice;
-      break;
-    case kEncoder:
-      device_path = kEncoderDevice;
-      break;
-    case kImageProcessor:
-      device_path = kImageProcessorDevice;
-      break;
-    case kJpegDecoder:
-      device_path = kJpegDecoderDevice;
-      break;
-  }
+  return true;
+}
 
-  DVLOG(2) << "Initialize(): opening device: " << device_path;
-  // Open the video device.
-  device_fd_.reset(
-      HANDLE_EINTR(open(device_path, O_RDWR | O_NONBLOCK | O_CLOEXEC)));
-  if (!device_fd_.is_valid()) {
+bool GenericV4L2Device::Open(uint32_t v4l2_pixfmt) {
+  std::string path = GetDevicePathFor(type_, v4l2_pixfmt);
+
+  if (path.empty()) {
+    DVLOG(1) << "No devices supporting " << std::hex << v4l2_pixfmt;

[kcwu, 2016/10/06 10:39:59]

prepend "0x" 

[Pawel Osciak, 2016/10/07 08:30:23]

Done.

     return false;
   }
-#if defined(USE_LIBV4L2)
-  if (type_ == kEncoder &&
-      HANDLE_EINTR(v4l2_fd_open(device_fd_.get(), V4L2_DISABLE_CONVERSION)) !=
-          -1) {
-    DVLOG(2) << "Using libv4l2 for " << device_path;
-    use_libv4l2_ = true;
+
+  if (!OpenDevicePath(path)) {
+    LOG(ERROR) << "Failed opening " << path;
+    return false;
   }
-#endif
 
   device_poll_interrupt_fd_.reset(eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC));
   if (!device_poll_interrupt_fd_.is_valid()) {
+    LOG(ERROR) << "Failed creating a poll interrupt fd";
     return false;
   }
+
   return true;
 }
 
@@ -310,10 +290,103 @@ GLenum GenericV4L2Device::GetTextureTarget() {
 uint32_t GenericV4L2Device::PreferredInputFormat() {
   // TODO(posciak): We should support "dontcare" returns here once we
   // implement proper handling (fallback, negotiation) for this in users.
-  CHECK_EQ(type_, kEncoder);
+  CHECK_EQ(type_, Type::kEncoder);
   return V4L2_PIX_FMT_NV12M;
 }
 
+std::vector<uint32_t> GenericV4L2Device::GetSupportedImageProcessorPixelformats(
+    v4l2_buf_type buf_type) {
+  std::vector<uint32_t> supported_pixelformats;

[kcwu, 2016/10/06 10:39:59]

DCHECK_EQ(type_, Type::kImageProcessor);

[Pawel Osciak, 2016/10/07 08:30:23]

Hm, thinking more about it, I think I can reorganize type_ in general.

+
+  const auto& devices = GetDevicesForType(Type::kImageProcessor);
+  for (const auto& device : devices) {
+    if (!OpenDevicePath(device.first)) {
+      LOG(ERROR) << "Failed opening " << device.first;
+      continue;
+    }
+
+    std::vector<uint32_t> pixelformats =
+        EnumerateSupportedPixelformats(buf_type);
+
+    supported_pixelformats.insert(supported_pixelformats.end(),
+                                  pixelformats.begin(), pixelformats.end());
+  }

[kcwu, 2016/10/06 10:39:59]

CloseDevice();

[Pawel Osciak, 2016/10/07 08:30:23]

Done.

+
+  return supported_pixelformats;
+}
+
+VideoDecodeAccelerator::SupportedProfiles
+GenericV4L2Device::GetSupportedDecodeProfiles(const size_t num_formats,
+                                              const uint32_t pixelformats[]) {
+  VideoDecodeAccelerator::SupportedProfiles supported_profiles;
+
+  const auto& devices = GetDevicesForType(Type::kDecoder);
+  for (const auto& device : devices) {
+    if (!OpenDevicePath(device.first)) {
+      LOG(ERROR) << "Failed opening " << device.first;
+      continue;
+    }
+
+    const auto& profiles =
+        EnumerateSupportedDecodeProfiles(num_formats, pixelformats);
+    supported_profiles.insert(supported_profiles.end(), profiles.begin(),
+                              profiles.end());
+  }

[kcwu, 2016/10/06 10:39:59]

CloseDevice();

[Pawel Osciak, 2016/10/07 08:30:23]

Done.

+
+  return supported_profiles;
+}
+
+VideoEncodeAccelerator::SupportedProfiles
+GenericV4L2Device::GetSupportedEncodeProfiles() {
+  VideoEncodeAccelerator::SupportedProfiles supported_profiles;
+
+  const auto& devices = GetDevicesForType(Type::kEncoder);
+  for (const auto& device : devices) {
+    if (!OpenDevicePath(device.first)) {
+      LOG(ERROR) << "Failed opening " << device.first;
+      continue;
+    }
+
+    const auto& profiles = EnumerateSupportedEncodeProfiles();
+    supported_profiles.insert(supported_profiles.end(), profiles.begin(),
+                              profiles.end());
+  }

[kcwu, 2016/10/06 10:39:59]

CloseDevice();

[Pawel Osciak, 2016/10/07 08:30:23]

Done.

+
+  return supported_profiles;
+}
+
+bool GenericV4L2Device::IsJpegDecodingSupported() {
+  const auto& devices = GetDevicesForType(Type::kJpegDecoder);
+  return !devices.empty();
+}
+
+bool GenericV4L2Device::OpenDevicePath(const std::string& path) {
+  CloseDevice();

[kcwu, 2016/10/06 10:39:59]

I know you auto close before open. But I'm wondering is this a good idea to
encourage to write code without close?
If so, it may leave fd opened unnecessary long time like GetSupported*Profiles
above.

[Pawel Osciak, 2016/10/07 08:30:23]

Yes, this is not a good style. Fixing.

+
+  device_fd_.reset(
+      HANDLE_EINTR(open(path.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC)));
+  if (!device_fd_.is_valid())
+    return false;
+
+#if defined(USE_LIBV4L2)
+  if (type_ == Type::kEncoder &&
+      HANDLE_EINTR(v4l2_fd_open(device_fd_.get(), V4L2_DISABLE_CONVERSION)) !=
+          -1) {
+    DVLOG(2) << "Using libv4l2 for " << path;
+    use_libv4l2_ = true;
+  }
+#endif
+  return true;
+}
+
+void GenericV4L2Device::CloseDevice() {
+#if defined(USE_LIBV4L2)
+  if (use_libv4l2_ && device_fd_.is_valid())
+    v4l2_close(device_fd_.release());
+#endif
+  device_fd_.reset();
+}
+
 // static
 bool GenericV4L2Device::PostSandboxInitialization() {
 #if defined(USE_LIBV4L2)
@@ -326,4 +399,82 @@ bool GenericV4L2Device::PostSandboxInitialization() {
 #endif
 }
 
+void GenericV4L2Device::EnumerateDevicesForType(Type type) {
+  static const std::string kDecoderDevicePattern = "/dev/video-dec";
+  static const std::string kEncoderDevicePattern = "/dev/video-enc";
+  static const std::string kImageProcessorDevicePattern = "/dev/image-proc";
+  static const std::string kJpegDecoderDevicePattern = "/dev/jpeg-dec";
+
+  std::string device_pattern;
+  v4l2_buf_type buf_type;
+  switch (type) {
+    case Type::kDecoder:
+      device_pattern = kDecoderDevicePattern;
+      buf_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+      break;
+    case Type::kEncoder:
+      device_pattern = kEncoderDevicePattern;
+      buf_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+      break;
+    case Type::kImageProcessor:
+      device_pattern = kImageProcessorDevicePattern;
+      buf_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+      break;
+    case Type::kJpegDecoder:
+      device_pattern = kJpegDecoderDevicePattern;
+      buf_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+      break;
+  }
+
+  std::vector<std::string> candidate_paths;
+
+  // TODO(posciak): Remove this legacy unnumbered device once
+  // all platforms are updated to use numbered devices.
+  candidate_paths.push_back(device_pattern);
+
+  for (int i = 0; i < 10; ++i) {

[kcwu, 2016/10/06 10:39:59]

Is it possible that number > 9? Add comments if you intended to ignore them.

[Pawel Osciak, 2016/10/07 08:30:23]

Unfortunately we can't use an actual FileEnumerator, as we are already sandboxed
here and can't query directory contents. I'll add a comment.

+    candidate_paths.push_back(
+        base::StringPrintf("%s%d", device_pattern.c_str(), i));
+  }
+
+  Devices devices;
+  for (const auto& path : candidate_paths) {
+    if (!OpenDevicePath(path))
+      continue;
+
+    const auto& supported_pixelformats =
+        EnumerateSupportedPixelformats(buf_type);
+    if (supported_pixelformats.empty())
+      continue;

[kcwu, 2016/10/06 10:39:59]

CloseDevice?

[Pawel Osciak, 2016/10/07 08:30:23]

Done.

+
+    DVLOG(1) << "Found device: " << path;
+    devices.push_back(std::make_pair(path, supported_pixelformats));
+    CloseDevice();
+  }
+
+  DCHECK_EQ(devices_by_type_.count(type), 0u);
+  devices_by_type_[type] = devices;
+}
+
+const GenericV4L2Device::Devices& GenericV4L2Device::GetDevicesForType(
+    Type type) {
+  if (devices_by_type_.count(type) == 0)
+    EnumerateDevicesForType(type);
+
+  DCHECK_NE(devices_by_type_.count(type), 0u);

[kcwu, 2016/10/06 10:39:59]

Is this always true?

[Pawel Osciak, 2016/10/07 08:30:23]

Yes. EnumerateDevicesForType() will always insert into devices_by_type_[type],
but the value of the element (typedef Devices) may be an empty vector if no
devices of type are present. This is intentional to indicate there are no
Devices of given Type and we check for this in callers of this method.

+  return devices_by_type_[type];
+}
+
+std::string GenericV4L2Device::GetDevicePathFor(Type type, uint32_t pixfmt) {
+  const Devices& devices = GetDevicesForType(type);
+
+  for (const auto& device : devices) {
+    if (std::find(device.second.begin(), device.second.end(), pixfmt) !=
+        device.second.end())
+      return device.first;
+  }
+
+  return std::string();
+}
+
 }  //  namespace media