Add VideoCaptureDevice::GetDeviceSupportedFormats to interface + implementation for Linux and Fake

media/video/capture/linux/video_capture_device_linux.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 "media/video/capture/linux/video_capture_device_linux.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #if defined(OS_OPENBSD)
 #include <sys/videoio.h>
@@ skipping 78 matching lines @@
   GetListOfUsableFourCCs(false, &usable_fourccs);
 
   memset(&fmtdesc, 0, sizeof(v4l2_fmtdesc));
   fmtdesc.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
   while (ioctl(fd, VIDIOC_ENUM_FMT, &fmtdesc) == 0) {
     if (std::find(usable_fourccs.begin(), usable_fourccs.end(),
                   fmtdesc.pixelformat) != usable_fourccs.end())
       return true;
 
-    fmtdesc.index++;
+    ++fmtdesc.index;
   }
   return false;
 }
 
 void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
   int fd = -1;
 
   // Empty the name list.
   device_names->clear();
 
@@ skipping 135 matching lines @@
   // Make sure no buffers are still allocated.
   // This can happen (theoretically) if an error occurs when trying to stop
   // the camera.
   DeAllocateVideoBuffers();
 }
 
 const VideoCaptureDevice::Name& VideoCaptureDeviceLinux::device_name() {
   return device_name_;
 }
 
+void VideoCaptureDeviceLinux::GetDeviceSupportedFormats(
+    VideoCaptureFormats* capture_formats) {

[perkj_chrome, 2013/09/13 10:33:05]

To be threadsafe - all v4l2 calls in this file is done on a separate thread
|v4l2_thread_| - this is since v4l2 require us to pull frames from a camera. 

You will need to decide on if you want to call this before or after Allocate- 
Allocate open the driver so you should not do it again.

Would it make more sence to move this code to OnAllocate  and let the supported
formats be returned in the observer interface? 
Also consider moving the requirement for a capture format to the Start method? 

[mcasas, 2013/09/24 15:02:27]

Done.

+

[scherkus (not reviewing), 2013/09/12 17:56:00]

remove blank line

[mcasas, 2013/09/24 15:02:27]

Done.

+  if (device_name_.id().empty())
+    return;
+  int fd = open(device_name_.id().c_str(), O_RDONLY);

[scherkus (not reviewing), 2013/09/12 17:56:00]

use file_util::ScopedFD

[scherkus (not reviewing), 2013/09/12 17:56:00]

this should be wrapped with HANDLE_EINTR()

[mcasas, 2013/09/24 15:02:27]

Done.

+  if (fd < 0)
+    return;

[scherkus (not reviewing), 2013/09/12 17:56:00]

DPLOG(ERROR) << "Couldn't open " << device_name_.id();

DPLOG will provide the posix last error message

[mcasas, 2013/09/24 15:02:27]

Done.

+
+  media::VideoCaptureCapability capture_format;

[scherkus (not reviewing), 2013/09/12 17:56:00]

remove media::

[mcasas, 2013/09/24 15:02:27]

Done.

+
+  v4l2_capability device;
+  // Test if this is a V4L2 capture device.
+  if ((ioctl(fd, VIDIOC_QUERYCAP, &device) == 0) &&
+      (device.capabilities & V4L2_CAP_VIDEO_CAPTURE) &&
+      !(device.capabilities & V4L2_CAP_VIDEO_OUTPUT)) {
+    // Retrieve the caps one by one, first get colorspace, then sizes, then

[scherkus (not reviewing), 2013/09/12 17:56:00]

instead of having this large block of code indented, check for the error
condition and return early

[mcasas, 2013/09/24 15:02:27]

Done.

+    // framerates. See http://linuxtv.org/downloads/v4l-dvb-apis for reference.
+    v4l2_fmtdesc pixel_format = {};
+    pixel_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+    while (ioctl(fd, VIDIOC_ENUM_FMT, &pixel_format) == 0) {
+      capture_format.color = media::V4l2ColorToVideoCaptureColorFormat(

[scherkus (not reviewing), 2013/09/12 17:56:00]

remove media::

[scherkus (not reviewing), 2013/09/12 17:56:00]

do we have to cast here?

it seems we don't have to on line 418 and as far as I can tell the types are the
same

[mcasas, 2013/09/24 15:02:27]

Done.

[mcasas, 2013/09/24 15:02:27]

Done.

+          static_cast<int32>(pixel_format.pixelformat));
+
+      v4l2_frmsizeenum frame_size = {};
+      frame_size.pixel_format = pixel_format.pixelformat;
+      while (ioctl(fd, VIDIOC_ENUM_FRAMESIZES, &frame_size) == 0) {
+        if (frame_size.type == V4L2_FRMSIZE_TYPE_DISCRETE) {
+          capture_format.width = frame_size.discrete.width;
+          capture_format.height = frame_size.discrete.height;
+        } else if (frame_size.type == V4L2_FRMSIZE_TYPE_STEPWISE) {
+          // What to do here? We get a min-max pair for width and height, and a
+          // step for each dimensions. Is not a crazy format, ViVi uses it.
+          // TODO(mcasas) see crbug.com/249953

[scherkus (not reviewing), 2013/09/12 17:56:00]

nit: we format TODOs with a : after the )

also can you elaborate / provide a summary of what to do in addition to linking
to the bug?

[scherkus (not reviewing), 2013/09/12 17:56:00]

nit: use http:// in front of crbug.com (many text editors 'n web browsers will
link-ify the result when viewing the source)

[mcasas, 2013/09/24 15:02:27]

Done.

[mcasas, 2013/09/24 15:02:27]

Is an easy elaborate: support these devices :) 

+          NOTIMPLEMENTED();
+        } else if (frame_size.type == V4L2_FRMSIZE_TYPE_CONTINUOUS) {
+          // Same here as the case before: TODO(mcasas) see crbug.com/249953

[scherkus (not reviewing), 2013/09/12 17:56:00]

ditto

[mcasas, 2013/09/24 15:02:27]

Done.

+          NOTIMPLEMENTED();

[scherkus (not reviewing), 2013/09/12 17:56:00]

if these are unimplemented, do you mean to continue executing and insert a
result?

is there a use case for inserting a bunch of VideoCaptureCapabilities with 0
width/height?

[mcasas, 2013/09/24 15:02:27]

I don't know this particular case. The proposed use case for this function as a
whole is to add the getUserMedia call to decide if a particular resolution (and
frame rate etc) as requested by the user, is possible. Right now there is no
information at all coming from the device to support this decision. So if we get
any information out of it, it would be already an improvement. That was my
rationale for all these NOTIMPLEMENTED and let it continue, deferring the
semantics to higher levels...

+        }
+        v4l2_frmivalenum frame_interval = {};
+        frame_interval.pixel_format = pixel_format.pixelformat;
+        frame_interval.width = frame_size.discrete.width;
+        frame_interval.height = frame_size.discrete.height;
+        while (ioctl(fd, VIDIOC_ENUM_FRAMEINTERVALS, &frame_interval) == 0) {
+          if (frame_interval.type == V4L2_FRMIVAL_TYPE_DISCRETE) {
+            capture_format.frame_rate =
+                (frame_interval.discrete.numerator != 0)

[scherkus (not reviewing), 2013/09/12 17:56:00]

I'd just use an if () statement here -- the ?: operator isn't making things easy
to read in this case

[mcasas, 2013/09/24 15:02:27]

Done.

+                    ? (static_cast<float>(frame_interval.discrete.denominator) /
+                       static_cast<float>(frame_interval.discrete.numerator))
+                    : 0;
+
+          } else if (frame_interval.type == V4L2_FRMIVAL_TYPE_CONTINUOUS) {
+            // Same here as the case above: TODO(mcasas) see crbug.com/249953

[scherkus (not reviewing), 2013/09/12 17:56:00]

nit: try to start TODO comments at the beginning of a comment

// TODO(mcasa): Same here as above, see http://crbug.com/249953

[mcasas, 2013/09/24 15:02:27]

Done.

+            NOTIMPLEMENTED();
+            break;
+          } else if (frame_interval.type == V4L2_FRMIVAL_TYPE_STEPWISE) {
+            // Same here as the case before: TODO(mcasas) see crbug.com/249953
+            NOTIMPLEMENTED();

[scherkus (not reviewing), 2013/09/12 17:56:00]

ditto... is there a use case for inserting VideoCaptureCapabilities with an
undefined frame rate?

[mcasas, 2013/09/24 15:02:27]

This is an easier case: yes, some devices might not be willing to share the
video frame rates (think auto exposure types), but if width+height are present,
that'd be an improvement.

+            break;
+          }
+          capture_formats->insert(capture_formats->end(), capture_format);

[scherkus (not reviewing), 2013/09/12 17:56:00]

push_back() ?

[mcasas, 2013/09/24 15:02:27]

Done.

+          ++frame_interval.index;
+        }
+        ++frame_size.index;
+      }
+      ++pixel_format.index;
+    }
+  } else {
+    // The selected device is not video capture as we like it.
+    capture_formats->clear();
+  }
+  close(fd);
+}
+
 void VideoCaptureDeviceLinux::OnAllocate(int width,
                                          int height,
                                          int frame_rate,
                                          EventHandler* observer) {
   DCHECK_EQ(v4l2_thread_.message_loop(), base::MessageLoop::current());
 
   observer_ = observer;
 
   // Need to open camera with O_RDWR after Linux kernel 3.3.
   if ((device_fd_ = open(device_name_.id().c_str(), O_RDWR)) < 0) {
@@ skipping 20 matching lines @@
                          &v4l2_formats);
 
   v4l2_fmtdesc fmtdesc;
   memset(&fmtdesc, 0, sizeof(v4l2_fmtdesc));
   fmtdesc.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
   // Enumerate image formats.
   std::list<int>::iterator best = v4l2_formats.end();
   while (ioctl(device_fd_, VIDIOC_ENUM_FMT, &fmtdesc) == 0) {
     best = std::find(v4l2_formats.begin(), best, fmtdesc.pixelformat);
-    fmtdesc.index++;
+    ++fmtdesc.index;
   }
 
   if (best == v4l2_formats.end()) {
     SetErrorState("Failed to find a supported camera format.");
     return;
   }
 
   // Set format and frame size now.
   v4l2_format video_fmt;
   memset(&video_fmt, 0, sizeof(video_fmt));
@@ skipping 136 matching lines @@
     }
     v4l2_thread_.message_loop()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&VideoCaptureDeviceLinux::OnCaptureTask,
                    base::Unretained(this)),
         base::TimeDelta::FromMilliseconds(kCaptureSelectWaitMs));
   }
 
   // Check if select timeout.
   if (result == 0) {
-    timeout_count_++;
+    ++timeout_count_;
     if (timeout_count_ >= kContinuousTimeoutLimit) {
       SetErrorState(base::StringPrintf(
           "Continuous timeout %d times", timeout_count_));
       timeout_count_ = 0;
       return;
     }
   } else {
     timeout_count_ = 0;
   }
 
@@ skipping 40 matching lines @@
   }
 
   if (r_buffer.count > kMaxVideoBuffers) {
     r_buffer.count = kMaxVideoBuffers;
   }
 
   buffer_pool_size_ = r_buffer.count;
 
   // Map the buffers.
   buffer_pool_ = new Buffer[r_buffer.count];
-  for (unsigned int i = 0; i < r_buffer.count; i++) {
+  for (unsigned int i = 0; i < r_buffer.count; ++i) {
     v4l2_buffer buffer;
     memset(&buffer, 0, sizeof(buffer));
     buffer.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
     buffer.memory = V4L2_MEMORY_MMAP;
     buffer.index = i;
 
     if (ioctl(device_fd_, VIDIOC_QUERYBUF, &buffer) < 0) {
       return false;
     }
 
@@ skipping 11 matching lines @@
     }
   }
   return true;
 }
 
 void VideoCaptureDeviceLinux::DeAllocateVideoBuffers() {
   if (!buffer_pool_)
     return;
 
   // Unmaps buffers.
-  for (int i = 0; i < buffer_pool_size_; i++) {
+  for (int i = 0; i < buffer_pool_size_; ++i) {
     munmap(buffer_pool_[i].start, buffer_pool_[i].length);
   }
   v4l2_requestbuffers r_buffer;
   memset(&r_buffer, 0, sizeof(r_buffer));
   r_buffer.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   r_buffer.memory = V4L2_MEMORY_MMAP;
   r_buffer.count = 0;
 
   if (ioctl(device_fd_, VIDIOC_REQBUFS, &r_buffer) < 0) {
     SetErrorState("Failed to reset buf.");
   }
 
   delete [] buffer_pool_;
   buffer_pool_ = NULL;
   buffer_pool_size_ = 0;
 }
 
 void VideoCaptureDeviceLinux::SetErrorState(const std::string& reason) {
   DVLOG(1) << reason;
   state_ = kError;
   observer_->OnError();
 }
 
 }  // namespace media