Linux Video Capture: small cleanup

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>
 #include <poll.h>
 #if defined(OS_OPENBSD)
 #include <sys/videoio.h>
 #else
 #include <linux/videodev2.h>
 #endif
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
 #include <list>
 #include <string>
 
 #include "base/bind.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/scoped_file.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/stringprintf.h"
 
 namespace media {
 
+#define GET_V4L2_FOURCC_CHAR(a, index) ((char)( ((a) >> (8 * index)) & 0xff))
+
 // Max number of video buffers VideoCaptureDeviceLinux can allocate.
 enum { kMaxVideoBuffers = 2 };
 // Timeout in milliseconds v4l2_thread_ blocks waiting for a frame from the hw.
 enum { kCaptureTimeoutMs = 200 };
 // The number of continuous timeouts tolerated before treated as error.
 enum { kContinuousTimeoutLimit = 10 };
 // MJPEG is preferred if the width or height is larger than this.
 enum { kMjpegWidth = 640 };
 enum { kMjpegHeight = 480 };
 // Typical framerate, in fps
 enum { kTypicalFramerate = 30 };
 
 // V4L2 color formats VideoCaptureDeviceLinux support.
 static const int32 kV4l2RawFmts[] = {
   V4L2_PIX_FMT_YUV420,
   V4L2_PIX_FMT_YUYV,
   V4L2_PIX_FMT_UYVY
 };
 
 // USB VID and PID are both 4 bytes long.
 static const size_t kVidPidSize = 4;
 
 // /sys/class/video4linux/video{N}/device is a symlink to the corresponding
 // USB device info directory.
 static const char kVidPathTemplate[] =
     "/sys/class/video4linux/%s/device/../idVendor";
 static const char kPidPathTemplate[] =
     "/sys/class/video4linux/%s/device/../idProduct";
 
-bool ReadIdFile(const std::string path, std::string* id) {
+static bool ReadIdFile(const std::string path, std::string* id) {
   char id_buf[kVidPidSize];
   FILE* file = fopen(path.c_str(), "rb");
   if (!file)
     return false;
   const bool success = fread(id_buf, kVidPidSize, 1, file) == 1;
   fclose(file);
   if (!success)
     return false;
   id->append(id_buf, kVidPidSize);
   return true;
 }
 
+//static
+int VideoCaptureDeviceLinux::Ioctl(const base::ScopedFD& device_fd,
+                                   int request,
+                                   void* arg) {
+  return HANDLE_EINTR(ioctl(device_fd.get(), request, arg));

[perkj_chrome, 2014/12/16 10:34:43]

I think this is less readable than calling HANDLE_EINTR(ioctl(device_fd.get(),
request, arg)) inline. 

codesearch seems to agree with me mostly, but not 100%. 
The virtual method Ioctl you reference seems to be there since there are
different implementations that need to be called.

[mcasas, 2014/12/16 18:08:17]

Reverted then.

Just FTR I'd say it was cleaner to refer to a local Ioctl()
method that encapsulated the HANDLE_EINTR part, and would
homogeneise the <= 0, < 0, == -1 etc comparisons all over,
all meaning "if ok" or "if not ok".

+}
+
 // This function translates Video4Linux pixel formats to Chromium pixel formats,
 // should only support those listed in GetListOfUsableFourCCs.
 // static
-VideoPixelFormat VideoCaptureDeviceLinux::V4l2ColorToVideoCaptureColorFormat(
-    int32 v4l2_fourcc) {
+VideoPixelFormat VideoCaptureDeviceLinux::V4l2FourCcToChromiumPixelFormat(
+    uint32 v4l2_fourcc) {
   VideoPixelFormat result = PIXEL_FORMAT_UNKNOWN;
   switch (v4l2_fourcc) {
     case V4L2_PIX_FMT_YUV420:
       result = PIXEL_FORMAT_I420;
       break;
     case V4L2_PIX_FMT_YUYV:
       result = PIXEL_FORMAT_YUY2;
       break;
     case V4L2_PIX_FMT_UYVY:
       result = PIXEL_FORMAT_UYVY;
       break;
     case V4L2_PIX_FMT_MJPEG:
     case V4L2_PIX_FMT_JPEG:
       result = PIXEL_FORMAT_MJPEG;
       break;
     default:
-      DVLOG(1) << "Unsupported pixel format " << std::hex << v4l2_fourcc;
+      DVLOG(1) << "Unsupported pixel format: "
+          << GET_V4L2_FOURCC_CHAR(v4l2_fourcc, 0)
+          << GET_V4L2_FOURCC_CHAR(v4l2_fourcc, 1)
+          << GET_V4L2_FOURCC_CHAR(v4l2_fourcc, 2)
+          << GET_V4L2_FOURCC_CHAR(v4l2_fourcc, 3);
   }
   return result;
 }
 
 // static
-void VideoCaptureDeviceLinux::GetListOfUsableFourCCs(bool favour_mjpeg,
-                                                     std::list<int>* fourccs) {
+std::list<int> VideoCaptureDeviceLinux::GetListOfUsableFourCCs(
+    bool favour_mjpeg) {
+  std::list<int> fourccs;
   for (size_t i = 0; i < arraysize(kV4l2RawFmts); ++i)
-    fourccs->push_back(kV4l2RawFmts[i]);
+    fourccs.push_back(kV4l2RawFmts[i]);
   if (favour_mjpeg)
-    fourccs->push_front(V4L2_PIX_FMT_MJPEG);
+    fourccs.push_front(V4L2_PIX_FMT_MJPEG);
   else
-    fourccs->push_back(V4L2_PIX_FMT_MJPEG);
+    fourccs.push_back(V4L2_PIX_FMT_MJPEG);
 
   // JPEG works as MJPEG on some gspca webcams from field reports.
   // Put it as the least preferred format.
-  fourccs->push_back(V4L2_PIX_FMT_JPEG);
+  fourccs.push_back(V4L2_PIX_FMT_JPEG);
+  return fourccs;
 }
 
 const std::string VideoCaptureDevice::Name::GetModel() const {
   // |unique_id| is of the form "/dev/video2".  |file_name| is "video2".
   const std::string dev_dir = "/dev/";
   DCHECK_EQ(0, unique_id_.compare(0, dev_dir.length(), dev_dir));
   const std::string file_name =
       unique_id_.substr(dev_dir.length(), unique_id_.length());
 
   const std::string vidPath =
@@ skipping 89 matching lines @@
 
   // Need to open camera with O_RDWR after Linux kernel 3.3.
   device_fd_.reset(HANDLE_EINTR(open(device_name_.id().c_str(), O_RDWR)));
   if (!device_fd_.is_valid()) {
     SetErrorState("Failed to open V4L2 device driver.");
     return;
   }
 
   // Test if this is a V4L2 capture device.
   v4l2_capability cap;
-  if (!((HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QUERYCAP, &cap)) == 0) &&
+  if (!((Ioctl(device_fd_, VIDIOC_QUERYCAP, &cap) == 0) &&
         (cap.capabilities & V4L2_CAP_VIDEO_CAPTURE) &&
         !(cap.capabilities & V4L2_CAP_VIDEO_OUTPUT))) {
     // This is not a V4L2 video capture device.
     device_fd_.reset();
     SetErrorState("This is not a V4L2 video capture device");
     return;
   }
 
   // Get supported video formats in preferred order.
   // For large resolutions, favour mjpeg over raw formats.
-  std::list<int> v4l2_formats;
-  GetListOfUsableFourCCs(width > kMjpegWidth || height > kMjpegHeight,
-                         &v4l2_formats);
+  const std::list<int>& desired_v4l2_formats =
+      GetListOfUsableFourCCs(width > kMjpegWidth || height > kMjpegHeight);
+  std::list<int>::const_iterator best = desired_v4l2_formats.end();
 
   v4l2_fmtdesc fmtdesc = {0};
   fmtdesc.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-
-  // Enumerate image formats.
-  std::list<int>::iterator best = v4l2_formats.end();
-  while (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_ENUM_FMT, &fmtdesc)) ==
-         0) {
-    best = std::find(v4l2_formats.begin(), best, fmtdesc.pixelformat);
-    fmtdesc.index++;
+  for (; Ioctl(device_fd_, VIDIOC_ENUM_FMT, &fmtdesc) == 0; ++fmtdesc.index) {
+    best = std::find(desired_v4l2_formats.begin(), best, fmtdesc.pixelformat);
   }
-
-  if (best == v4l2_formats.end()) {
+  if (best == desired_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(v4l2_format));
   video_fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   video_fmt.fmt.pix.sizeimage = 0;
   video_fmt.fmt.pix.width = width;
   video_fmt.fmt.pix.height = height;
   video_fmt.fmt.pix.pixelformat = *best;
 
-  if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_S_FMT, &video_fmt)) < 0) {
+  if (Ioctl(device_fd_, VIDIOC_S_FMT, &video_fmt) < 0) {
     SetErrorState(
         base::StringPrintf("Failed to set camera format: %s", strerror(errno)));
     return;
   }
 
   // Set capture framerate in the form of capture interval.
   v4l2_streamparm streamparm;
   memset(&streamparm, 0, sizeof(v4l2_streamparm));
   streamparm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   // The following line checks that the driver knows about framerate get/set.
-  if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_G_PARM, &streamparm)) >= 0) {
+  if (Ioctl(device_fd_, VIDIOC_G_PARM, &streamparm) >= 0) {
     // Now check if the device is able to accept a capture framerate set.
     if (streamparm.parm.capture.capability & V4L2_CAP_TIMEPERFRAME) {
       // |frame_rate| is float, approximate by a fraction.
       streamparm.parm.capture.timeperframe.numerator =
           media::kFrameRatePrecision;
       streamparm.parm.capture.timeperframe.denominator = (frame_rate) ?
           (frame_rate * media::kFrameRatePrecision) :
           (kTypicalFramerate * media::kFrameRatePrecision);
 
-      if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_S_PARM, &streamparm)) <
-          0) {
+      if (Ioctl(device_fd_, VIDIOC_S_PARM, &streamparm) < 0) {
         SetErrorState("Failed to set camera framerate");
         return;
       }
       DVLOG(2) << "Actual camera driverframerate: "
                << streamparm.parm.capture.timeperframe.denominator << "/"
                << streamparm.parm.capture.timeperframe.numerator;
     }
   }
   // TODO(mcasas): what should be done if the camera driver does not allow
   // framerate configuration, or the actual one is different from the desired?
 
   // Set anti-banding/anti-flicker to 50/60Hz. May fail due to not supported
   // operation (|errno| == EINVAL in this case) or plain failure.
   const int power_line_frequency = GetPowerLineFrequencyForLocation();
   if ((power_line_frequency == kPowerLine50Hz) ||
       (power_line_frequency == kPowerLine60Hz)) {
     struct v4l2_control control = {};
     control.id = V4L2_CID_POWER_LINE_FREQUENCY;
     control.value = (power_line_frequency == kPowerLine50Hz) ?
                         V4L2_CID_POWER_LINE_FREQUENCY_50HZ :
                         V4L2_CID_POWER_LINE_FREQUENCY_60HZ;
     HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_S_CTRL, &control));
   }
 
   // Store our current width and height.
   capture_format_.frame_size.SetSize(video_fmt.fmt.pix.width,
                                      video_fmt.fmt.pix.height);
   capture_format_.frame_rate = frame_rate;
   capture_format_.pixel_format =
-      V4l2ColorToVideoCaptureColorFormat(video_fmt.fmt.pix.pixelformat);
+      V4l2FourCcToChromiumPixelFormat(video_fmt.fmt.pix.pixelformat);
 
-  // Start capturing.
   if (!AllocateVideoBuffers()) {
-    // Error, We can not recover.
-    SetErrorState("Allocate buffer failed");
+    SetErrorState("Allocate buffers failed");
     return;
   }
 
   // Start UVC camera.
   v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-  if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_STREAMON, &type)) == -1) {
+  if (Ioctl(device_fd_, VIDIOC_STREAMON, &type) == -1) {
     SetErrorState("VIDIOC_STREAMON failed");
     return;
   }
 
   is_capturing_ = true;
   // Post task to start fetching frames from v4l2.
   v4l2_thread_.message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&VideoCaptureDeviceLinux::OnCaptureTask,
                  base::Unretained(this)));
 }
 
 void VideoCaptureDeviceLinux::OnStopAndDeAllocate() {
   DCHECK_EQ(v4l2_thread_.message_loop(), base::MessageLoop::current());
 
   v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-  if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_STREAMOFF, &type)) < 0) {
+  if (Ioctl(device_fd_, VIDIOC_STREAMOFF, &type) < 0) {
     SetErrorState("VIDIOC_STREAMOFF failed");
     return;
   }
-  // We don't dare to deallocate the buffers if we can't stop
-  // the capture device.
+  // We don't dare to deallocate the buffers if we can't stop the capture
+  // device.
   DeAllocateVideoBuffers();
 
-  // We need to close and open the device if we want to change the settings
-  // Otherwise VIDIOC_S_FMT will return error
-  // Sad but true.
+  // We need to close and open the device if we want to change the settings.
+  // Otherwise VIDIOC_S_FMT will return error. Sad but true.
   device_fd_.reset();
   is_capturing_ = false;
   client_.reset();
 }
 
 void VideoCaptureDeviceLinux::OnCaptureTask() {
   DCHECK_EQ(v4l2_thread_.message_loop(), base::MessageLoop::current());
   if (!is_capturing_)
     return;
 
@@ skipping 20 matching lines @@
 
   timeout_count_ = 0;
 
   // Check if the driver has filled a buffer.
   if (device_pfd.revents & POLLIN) {
     v4l2_buffer buffer;
     memset(&buffer, 0, sizeof(buffer));
     buffer.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
     buffer.memory = V4L2_MEMORY_MMAP;
     // Dequeue a buffer.
-    if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_DQBUF, &buffer)) == 0) {
+    if (Ioctl(device_fd_, VIDIOC_DQBUF, &buffer) == 0) {
       client_->OnIncomingCapturedData(
           static_cast<uint8*>(buffer_pool_[buffer.index].start),
           buffer.bytesused,
           capture_format_,
           rotation_,
           base::TimeTicks::Now());
 
       // Enqueue the buffer again.
-      if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QBUF, &buffer)) == -1) {
+      if (Ioctl(device_fd_, VIDIOC_QBUF, &buffer) == -1) {
         SetErrorState(base::StringPrintf(
             "Failed to enqueue capture buffer errno %d", errno));
       }
     } else {
       SetErrorState(base::StringPrintf(
           "Failed to dequeue capture buffer errno %d", errno));
       return;
     }
   }
 
@@ skipping 74 matching lines @@
 }
 
 void VideoCaptureDeviceLinux::SetErrorState(const std::string& reason) {
   DCHECK(!v4l2_thread_.IsRunning() ||
          v4l2_thread_.message_loop() == base::MessageLoop::current());
   is_capturing_ = false;
   client_->OnError(reason);
 }
 
 }  // namespace media