VideoCaptureDeviceLinux: Add support for SPLANE+DMABUF V4L2 type capture

media/video/capture/linux/v4l2_capture_delegate_single_plane.cc

Index: media/video/capture/linux/v4l2_capture_delegate_single_plane.cc
diff --git a/media/video/capture/linux/v4l2_capture_delegate_single_plane.cc b/media/video/capture/linux/v4l2_capture_delegate_single_plane.cc
index e2e7e5b0da16a9c4b934b83bdfa567a1f1ab39d6..b650ba34c8006578902486a82fc196f70a7c1eb3 100644
--- a/media/video/capture/linux/v4l2_capture_delegate_single_plane.cc
+++ b/media/video/capture/linux/v4l2_capture_delegate_single_plane.cc
@@ -4,10 +4,27 @@
 
 #include "media/video/capture/linux/v4l2_capture_delegate_single_plane.h"
 
+#include <linux/version.h>
 #include <sys/mman.h>
 
 namespace media {
 
+V4L2CaptureDelegateSinglePlane::V4L2CaptureDelegateSinglePlane(
+    const VideoCaptureDevice::Name& device_name,
+    const scoped_refptr<base::SingleThreadTaskRunner>& v4l2_task_runner,
+    int power_line_frequency,
+    bool try_to_use_dma_buf)
+    : V4L2CaptureDelegate(device_name, v4l2_task_runner, power_line_frequency) {
+  memory_type_ = V4L2_MEMORY_MMAP;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 8, 0)
+  if (try_to_use_dma_buf)
+    memory_type_ = V4L2_MEMORY_DMABUF;
+#endif
+}
+
+V4L2CaptureDelegateSinglePlane::~V4L2CaptureDelegateSinglePlane() {
+}
+
 scoped_refptr<V4L2CaptureDelegate::BufferTracker>
 V4L2CaptureDelegateSinglePlane::CreateBufferTracker() const {
   return make_scoped_refptr(new BufferTrackerSPlane());
@@ -25,8 +42,34 @@ bool V4L2CaptureDelegateSinglePlane::FillV4L2Format(
 }
 
 void V4L2CaptureDelegateSinglePlane::FinishFillingV4L2Buffer(
-    v4l2_buffer* buffer) const {
+    v4l2_buffer* buffer,
+    bool for_enqueue) const {
+  buffer->memory = memory_type_;
   buffer->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 8, 0)
+  if (memory_type_ == V4L2_MEMORY_MMAP)
+    return;
+
+  if (!for_enqueue) {
+    DCHECK(buffer->m.fd);  // For Dequeue, API should have filled |fd| in.

[Pawel Osciak, 2015/06/15 10:34:55]

We call this before calling DQBUF, so it couldn't have been filled yet?

Also, fd = 0 is a valid fd, while -1 is not. The former would fail, while the
latter would not.

[mcasas, 2015/06/17 01:30:53]

First time this method is called, it's with 
|for_enqueue|==true, the second time is called
with |for_enqueue|==false, see 
V4L2CaptureDelegate::MapAndQueueBuffer(...)
Done.

+    return;
+  }
+  // For enqueueing, need to reserve an output buffer, keep it locally and pass
+  // its |fd| into the V4L2 API.
+  scoped_ptr<VideoCaptureDevice::Client::Buffer> capture_buffer =

[Pawel Osciak, 2015/06/15 10:34:55]

Do we need to allocate buffers on each QBUF? Could we allocate them once, when
we REQBUFS?

Further, I'd suggest adding Client::Buffers to Plane. Then we could look them up
by buffer.index. With that, we could get rid of std::find, and also we could not
fail on SendBuffer. Moreover, we could just look at that common structure and
wouldn't have to check memory type, just see if fd != -1 or if start != nullptr
and make a decision based on that.

Also, we shouldn't need for_capture then, just fill it always (cheap).

[mcasas, 2015/06/17 01:30:53]

We don't allocate a Buffers on each QBUF, we 
ReserveOutputBuffer(). The first time we call this
method, VideoCaptureBufferPool indeed allocates,
but subsequent calls, assuming same call params,
i.e. same format and resolution, do not allocate.
I see your points. I don't like the way BufferTracker can
be two things depending on the values of its Planes' 
members. I think it is already bad enough that it can
be either |fd| != -1 or |start| != nullptr (but not both).

The suggestion you made in l.128 seems to go in that
direction as well: have a BufferTracker that can be either
or but not both. I would suggest an abstract class 
BufferTracker with two implementations, MmapBufferTracker
and DmaBufBufferTracker, WDYT?

+      client()->ReserveOutputBuffer(media::PIXEL_FORMAT_GPUMEMORYBUFFER,
+                                    capture_format().frame_size);
+  if (capture_buffer && capture_buffer->GetType() == gfx::OZONE_NATIVE_BUFFER) {
+    buffer->m.fd = capture_buffer->AsPlatformFile();
+
+    allocated_buffers_.push_back(capture_buffer.release());
+    DVLOG(1) << "Sizeof allocated_buffers_ " << allocated_buffers_.size();
+  } else {
+    DLOG(WARNING) << "Uh oh, platform does not support Dma-Buf allocation, "

[Pawel Osciak, 2015/06/15 10:34:55]

We should propagate errors and SetErrorState() in higher layers.

[mcasas, 2015/06/17 01:30:53]

This is a Warning, and it was meant to be to help
development, so I refactored the code to better
reflect the meaning.

+                  << "or has run out of buffers :-?";
+  }
+#endif
 }
 
 void V4L2CaptureDelegateSinglePlane::SetPayloadSize(
@@ -35,20 +78,58 @@ void V4L2CaptureDelegateSinglePlane::SetPayloadSize(
   buffer_tracker->SetPlanePayloadSize(0, buffer.bytesused);
 }
 
+void V4L2CaptureDelegateSinglePlane::FinishFillingV4L2RequestBuffers(
+    v4l2_requestbuffers* request) const {
+  request->memory = memory_type_;
+}
+
 void V4L2CaptureDelegateSinglePlane::SendBuffer(
     const scoped_refptr<BufferTracker>& buffer_tracker,
     const v4l2_format& format) const {
-  client()->OnIncomingCapturedData(
-      buffer_tracker->GetPlaneStart(0),
+  DVLOG(1) << __FUNCTION__;
+
+  if (memory_type_ == V4L2_MEMORY_MMAP) {
+    const size_t data_length = format.fmt.pix.sizeimage;
+    DCHECK_GE(data_length, capture_format().ImageAllocationSize());
+    client()->OnIncomingCapturedData(
+        buffer_tracker->GetPlaneStart(0),
       buffer_tracker->GetPlanePayloadSize(0),
-      capture_format(),
-      rotation(),
-      base::TimeTicks::Now());
+        capture_format(),
+        rotation(),
+        base::TimeTicks::Now());
+    return;
+  }
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 8, 0)
+  // Search for the |fd| used for capture in |allocated_buffers_| and send it
+  // to client() if found. Otherwise is an error.
+  const int incoming_fd = buffer_tracker->GetFd(0);
+  ScopedVector<VideoCaptureDevice::Client::Buffer>::iterator used_buffer =
+      std::find_if(allocated_buffers_.begin(), allocated_buffers_.end(),
+                   [incoming_fd](const VideoCaptureDevice::Client::Buffer* b) {

[Pawel Osciak, 2015/06/15 10:34:55]

const Buffer& possible?

[mcasas, 2015/06/17 01:30:53]

ScopedVector keeps pointers, from which you can
make_scoped_ptr() as in l.116.

+                     return incoming_fd == b->AsPlatformFile();
+                   });
+  if (used_buffer == allocated_buffers_.end()) {
+    DLOG(ERROR) << "Uh oh, captured |fd| is not found in the list :?";
+    return;

[Pawel Osciak, 2015/06/15 10:34:55]

Need to propagate the error.

[mcasas, 2015/06/17 01:30:53]

Same as before, this was meant to be for 
helping development, not for production code.
Using SetErrorState() comes with a series of
troubles on its own, as you can see it is a
private method, so we'll need to surface it etc.
Let me ask you the complementary question:
is an V4L2 driver able to capture in an |fd|
that has not been fed, given the configuration
in this CL?

+  }
+  client()->OnIncomingCapturedBuffer(make_scoped_ptr(*used_buffer),
+                                     capture_format(), base::TimeTicks::Now());
+  allocated_buffers_.weak_erase(used_buffer);

[Pawel Osciak, 2015/06/15 10:34:55]

Perhaps we could use std::vector<scoped_refptr<>> to simplify this?

[mcasas, 2015/06/17 01:30:53]

I wish! ReserveOutputBuffer() returns a scoped_ptr<>
and VideoCaptureDevice::Client::Buffer has no
ref counting semantics.

+#endif
 }
 
 bool V4L2CaptureDelegateSinglePlane::BufferTrackerSPlane::Init(
     int fd,
     const v4l2_buffer& buffer) {
+  DVLOG(1) << __FUNCTION__;
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 8, 0)
+  if (buffer.memory == V4L2_MEMORY_DMABUF) {
+    AddNonMmapedPlane(buffer.m.fd);

[Pawel Osciak, 2015/06/15 10:34:55]

Would it be possible to have one AddPlane() method, which could look at
buffer.memory and decide what to use, to simplify and reduce the number of
ifdefs?

[mcasas, 2015/06/17 01:30:53]

Wouldn't it move this ifdef somewhere else, and
force every caller (including V4L2CaptureDelegateMultiPlane,
who is oblivious to DmaBufs so far) to indicate a
fd? And, in this method, we'd still need to avoid
mmap() for V4L2_MEMORY_DMABUFs.

+    return true;
+  }
+#endif
+
   // Some devices require mmap() to be called with both READ and WRITE.
   // See http://crbug.com/178582.
   void* const start = mmap(NULL, buffer.length, PROT_READ | PROT_WRITE,