Add support for Tegra V4L2 VDA

content/common/gpu/media/exynos_v4l2_video_device.cc

Index: content/common/gpu/media/exynos_v4l2_video_device.cc
diff --git a/content/common/gpu/media/exynos_v4l2_video_device.cc b/content/common/gpu/media/exynos_v4l2_video_device.cc
index ef40cf0e6854e4a7a9b90aa24d038bfb60977432..5be18a09037baa56cffc8ea4b5f66f8f4b5d37e3 100644
--- a/content/common/gpu/media/exynos_v4l2_video_device.cc
+++ b/content/common/gpu/media/exynos_v4l2_video_device.cc
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
+
 #include <fcntl.h>
+#include <libdrm/drm_fourcc.h>
 #include <linux/videodev2.h>
 #include <poll.h>
 #include <sys/eventfd.h>
@@ -12,6 +14,7 @@
 #include "base/debug/trace_event.h"
 #include "base/posix/eintr_wrapper.h"
 #include "content/common/gpu/media/exynos_v4l2_video_device.h"
+#include "ui/gl/gl_bindings.h"
 
 namespace content {
 
@@ -114,4 +117,65 @@ bool ExynosV4L2Device::Initialize() {
   }
   return true;
 }
+
+EGLImageKHR ExynosV4L2Device::CreateEGLImage(EGLDisplay egl_display,
+                                             GLuint texture_id,
+                                             gfx::Size frame_buffer_size,
+                                             unsigned int buffer_index) {
+  DVLOG(3) << "CreateEGLImage()";
+  int dmabuf_fds[2] = {-1, -1};

[Pawel Osciak, 2014/03/27 09:09:50]

As I mentioned before, num_planes should be passed to this method and not
hardcoded.

[shivdasp, 2014/03/27 09:33:22]

I think this is do-able quickly.Done.
On 2014/03/27 09:09:50, Pawel Osciak wrote:

[sheu, 2014/03/27 09:40:41]

I'd disagree here -- if we're defining a preferred output format then we should
know the plane count here.  I think that's the simplest way around the issue.

[shivdasp, 2014/03/27 10:06:46]

I think the num_planes depend upon the output format. So if we choose to use a
different output format, we wouldn't need to change the device implementation if
we use the parameterized num_planes here. I think that's what Pawel thinks.
But I guess that's partially true since there is rest of the code which would
need to change if num planes change. Filling up the attrs etc.
I am okay either ways. Let me know.

[Pawel Osciak, 2014/03/28 05:33:53]

Yes this is exactly what I mean. There is no need to hardcode both, because the
driver should return that value in G_FMT once we set the correct one. So there
is no need for this class to have a method for getting number of planes. V4L2VDA
can just call G_FMT to find out what it is.
We still need it, since V4L2VDA needs to know for other calls, like qbuf. For
Exynos num planes would be 2, but I think you didn't change the Tegra's
preferred format. You said before that it was not using 2-plane NV12M?
That's why using G_FMT in V4L2VDA and taking num_planes from there is the most
universal (and API-conformant), while simple enough way I would say.

[shivdasp, 2014/03/28 05:42:38]

Okay I will make a change to send in the num_planes as an additional argument to
CreateEGLImage(). That's the only change I hope.
We made change in the buffer allocation to have Tegra's preferred format same as
Exynos. Anyways since we now have moved code related to output formats into
device specific changing either ways will not affect another.

+  for (size_t i = 0; i < arraysize(dmabuf_fds); ++i) {
+    // Export the DMABUF fd so we can export it as a texture.
+    struct v4l2_exportbuffer expbuf;
+    memset(&expbuf, 0, sizeof(expbuf));
+    expbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+    expbuf.index = buffer_index;
+    expbuf.plane = i;
+    expbuf.flags = O_CLOEXEC;
+    if (HANDLE_EINTR(Ioctl(VIDIOC_EXPBUF, &expbuf) != 0)) {
+      return EGL_NO_IMAGE_KHR;

[Pawel Osciak, 2014/03/27 09:09:50]

If this fails on any other than the first fd, we will never close the ones we've
already exported and leak.

Please replace all close() calls in this method (including the ones on
successful return) with a base::ScopedClosureRunner():

static void CloseDmabufFds(linked_ptr<std::vector<int> > dmabuf_fds) {
  for (size_t i = 0; i < dmabuf_fds->size(); ++i)
    close(dmabuf_fds->at(i));
}

And in this method:

linked_ptr<std::vector<int> > dmabuf_fds(new std::vector<int>(num_planes, -1));
base::Closure dmabuf_fds_cb = 
    base::Bind(&CloseDmabufFds, dmabuf_fds);
base::ScopedClosureRunner dmabuf_fds_closer(dmabuf_fds_cb);

And here expbufs assigning to dmabuf_fds->at(i).

[shivdasp, 2014/03/27 09:33:22]

I need to understand the usage here. If I understand each of the fds are added
in the linked list and then deleted on failure.

[sheu, 2014/03/27 09:40:41]

See above about preferred format.

In the case we go hard-coded on the number of planes, then dmabufds can be an
array of ScopedFD and we get the same auto-destruction behavior when they go out
of scope.  i.e.:

ScopedFD dma_bufs[2];

or if it's not hard-coded, even:

scoped_ptr<ScopedFD[]> dma_bufs(new ScopedFd[num_planes]);

[shivdasp, 2014/03/27 10:06:46]

Using ScopedFD looks easier, let me make that change. Thanks.

[Pawel Osciak, 2014/03/28 05:33:53]

Yes, sorry, I forgot we had ScopedFD

+    }
+    dmabuf_fds[i] = expbuf.fd;
+  }
+  EGLint attrs[] = {
+      EGL_WIDTH,                     0, EGL_HEIGHT,                    0,
+      EGL_LINUX_DRM_FOURCC_EXT,      0, EGL_DMA_BUF_PLANE0_FD_EXT,     0,
+      EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0, EGL_DMA_BUF_PLANE0_PITCH_EXT,  0,
+      EGL_DMA_BUF_PLANE1_FD_EXT,     0, EGL_DMA_BUF_PLANE1_OFFSET_EXT, 0,
+      EGL_DMA_BUF_PLANE1_PITCH_EXT,  0, EGL_NONE, };
+  attrs[1] = frame_buffer_size.width();
+  attrs[3] = frame_buffer_size.height();
+  attrs[5] = DRM_FORMAT_NV12;

[Pawel Osciak, 2014/03/27 09:09:50]

This shouldn't be hardcoded either actually...

How about instead of num_planes, size, etc. we just pass v4l2_format to this
method and extract all info from there?
And in this case we can DCHECK on V4L2_PIX_FMT_NV12M and then hardcode this DRM
define for now.

[shivdasp, 2014/03/27 09:33:22]

How do you propose we remove the hardcoding of DRM_FORMAT_NV12, is there a G_FMT
anywhere ? Since it is anyways in device implementation, it is confined for
Exynos only.
Hmm...  I think sending the complete v4l2_format struct here may not help.
Either we populate the v4l2_format before the call or in this class. I prefer
having it in this class.

[sheu, 2014/03/27 09:40:41]

See above about preferred format.

[Pawel Osciak, 2014/03/28 05:33:53]

Ok, let's keep it for now and figure it out later. No need to block this CL on
this.
That's exactly what I hoped for.
It's the driver who should populate it on G_FMT.
Yes exactly, agreed.

[shivdasp, 2014/03/28 05:42:38]

I understand there is no more change required here now atleast in this CL.

+  attrs[7] = dmabuf_fds[0];
+  attrs[9] = 0;
+  attrs[11] = frame_buffer_size.width();
+  attrs[13] = dmabuf_fds[1];
+  attrs[15] = 0;
+  attrs[17] = frame_buffer_size.width();
+
+  EGLImageKHR egl_image = eglCreateImageKHR(
+      egl_display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT, NULL, attrs);
+  if (egl_image == EGL_NO_IMAGE_KHR) {
+    close(dmabuf_fds[0]);
+    close(dmabuf_fds[1]);
+    return egl_image;
+  }
+  glBindTexture(GL_TEXTURE_EXTERNAL_OES, texture_id);
+  glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES, egl_image);
+
+  // Close the fds now
+  close(dmabuf_fds[0]);
+  close(dmabuf_fds[1]);
+  return egl_image;
+}
+
+EGLBoolean ExynosV4L2Device::DestroyEGLImage(EGLDisplay egl_display,
+                                             EGLImageKHR egl_image) {
+  return eglDestroyImageKHR(egl_display, egl_image);
+}
+
+GLenum ExynosV4L2Device::GetTextureTarget() { return GL_TEXTURE_EXTERNAL_OES; }
+
+uint32 ExynosV4L2Device::PreferredOutputFormat() { return V4L2_PIX_FMT_NV12M; }
+
 }  //  namespace content