Add vaapi_h264_decoder_test.

content/common/gpu/media/vaapi_wrapper.cc

Index: content/common/gpu/media/vaapi_wrapper.cc
diff --git a/content/common/gpu/media/vaapi_wrapper.cc b/content/common/gpu/media/vaapi_wrapper.cc
index 53e8b6952424edadf8ed13f136486ed029884ce9..7c892a6ed9f7c07463a1f3b30c539e67540b5a5e 100644
--- a/content/common/gpu/media/vaapi_wrapper.cc
+++ b/content/common/gpu/media/vaapi_wrapper.cc
@@ -114,6 +114,14 @@ typedef VAStatus (*VaapiRenderPicture)(VADisplay dpy,
                                        int num_buffers);
 typedef VAStatus (*VaapiSyncSurface)(VADisplay dpy, VASurfaceID render_target);
 typedef VAStatus (*VaapiTerminate)(VADisplay dpy);
+typedef VAStatus (*VaapiMapBuffer)(VADisplay dpy,

[Pawel Osciak, 2013/11/29 04:25:13]

Please maintain lexicographical order.

[chihchung, 2013/11/29 14:06:30]

Done.

+                                   VABufferID buf_id,
+                                   void** pbuf);
+typedef VAStatus (*VaapiUnmapBuffer)(VADisplay dpy, VABufferID buf_id);
+typedef VAStatus (*VaapiDeriveImage)(VADisplay dpy,
+                                     VASurfaceID surface,
+                                     VAImage* image);
+typedef VAStatus (*VaapiDestroyImage)(VADisplay dpy, VAImageID image);
 
 #define VAAPI_SYM(name, handle) Vaapi##name VAAPI_##name = NULL
 
@@ -136,6 +144,10 @@ VAAPI_SYM(PutSurface, vaapi_x11_handle);
 VAAPI_SYM(RenderPicture, vaapi_handle);
 VAAPI_SYM(SyncSurface, vaapi_x11_handle);
 VAAPI_SYM(Terminate, vaapi_handle);
+VAAPI_SYM(MapBuffer, vaapi_handle);
+VAAPI_SYM(UnmapBuffer, vaapi_handle);
+VAAPI_SYM(DeriveImage, vaapi_handle);
+VAAPI_SYM(DestroyImage, vaapi_handle);
 
 #undef VAAPI_SYM
 
@@ -433,6 +445,85 @@ bool VaapiWrapper::PutSurfaceIntoPixmap(VASurfaceID va_surface_id,
   return true;
 }
 
+static scoped_refptr<media::VideoFrame> CopyNV12ToI420(VAImage* image,

[Pawel Osciak, 2013/11/29 04:25:13]

To be honest I feel now that all the conversion code should go to the test file.
We don't use it outside the test and it'd be better not to have it in the
compiled Chrome. We should keep test-related changes in the wrapper to the
minimum.

Since the test has to understand vaapi anyway, just maybe have a
VaapiWrapper::GetVaImage to derive image here returning VAImage and also a
VW::ReturnVaImage to free it and do conversions and copies outside of wrapper.

What do you think?

[chihchung, 2013/11/29 14:06:30]

I prefer current code because (1) The linker should be able to remove unused
functions for compiled chrome (2) The GetVaImage function will need to return
two parameters, VAImage and void* mem, because the test code doesn't have access
to VAAPI_MapBuffer. But I am OK to change it if you think it's better.

[Pawel Osciak, 2013/12/03 08:52:06]

1) We just did an interesting experiment where we found out that those methods
are indeed optimized out, but their string literals are kept
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=192).
I would still insist on removing CopyNV12ToI420 from this class, even if for
code clarity. Pretty please? :)

2) True, but unfortunately necessary I'd say. (Technically you could instead
wrap NV12 into VideoFrame and convert later, but VF doesn't have support for
NV12 and it's probably not a good idea to add it just for this test).

[chihchung, 2013/12/03 10:57:40]

Done.

+                                                       void* mem) {
+  DVLOG(1) << "CopyNV12ToI420 width=" << image->width
+           << ", height=" << image->height;
+
+  const gfx::Size coded_size(image->width, image->height);
+  const gfx::Rect visible_rect(image->width, image->height);
+  const gfx::Size natural_size(image->width, image->height);
+
+  scoped_refptr<media::VideoFrame> frame =
+      media::VideoFrame::CreateFrame(media::VideoFrame::I420,

[Pawel Osciak, 2013/11/29 04:25:13]

Normally I'd suggest adding a
VideoFrame::WrapVAImage(), but it's probably a better idea to contain this here
and not pollute VideoFrame with code used just for testing.

+                                     coded_size,
+                                     visible_rect,
+                                     natural_size,
+                                     base::TimeDelta());
+
+  uint8_t* mem_byte_ptr = static_cast<uint8_t*>(mem);
+  uint8_t* srcY = mem_byte_ptr + image->offsets[0];
+  uint8_t* srcU = mem_byte_ptr + image->offsets[1];
+  uint8_t* srcV = srcU + 1;
+
+  uint8_t* dstY = frame->data(media::VideoFrame::kYPlane);
+  uint8_t* dstU = frame->data(media::VideoFrame::kUPlane);
+  uint8_t* dstV = frame->data(media::VideoFrame::kVPlane);
+
+  for (int i = 0; i < image->height; i++) {
+    memcpy(dstY, srcY, image->width);
+    srcY += image->pitches[0];
+    dstY += frame->stride(media::VideoFrame::kYPlane);
+
+    if ((i & 1) == 1)

[Pawel Osciak, 2013/11/29 04:25:13]

It's obvious for us, but maybe add a comment why this is.

[chihchung, 2013/11/29 14:06:30]

Done.

+      continue;
+    for (int j = 0, k = 0; j < image->width; j += 2, k++) {
+      dstU[k] = srcU[j];

[Pawel Osciak, 2013/11/29 04:25:13]

Did a quick peek into vagetimage and it looks like it doesn't do image
conversion based on given image.format. You'd have to use VPP infra in vaapi.
Not worth it.

+      dstV[k] = srcV[j];
+    }
+    srcU += image->pitches[1];
+    srcV += image->pitches[1];
+    dstU += frame->stride(media::VideoFrame::kUPlane);
+    dstV += frame->stride(media::VideoFrame::kVPlane);
+  }
+
+  return frame;
+}
+
+scoped_refptr<media::VideoFrame> VaapiWrapper::VideoFrameFromVASurface(
+    VASurfaceID va_surface_id) {
+  base::AutoLock auto_lock(va_lock_);
+
+  VAStatus va_res = VAAPI_SyncSurface(va_display_, va_surface_id);
+  VA_SUCCESS_OR_RETURN(va_res, "Failed syncing surface", NULL);
+
+  VAImage image;
+  void* mem = NULL;
+  scoped_refptr<media::VideoFrame> result;

[Pawel Osciak, 2013/11/29 04:25:13]

s/result/frame or something like that :)

[chihchung, 2013/11/29 14:06:30]

Done.

+
+  // Derive a VAImage from the VASurface
+  va_res = VAAPI_DeriveImage(va_display_, va_surface_id, &image);
+  VA_LOG_ON_ERROR(va_res, "vaDeriveImage failed");
+  if (va_res == VA_STATUS_SUCCESS) {

[Pawel Osciak, 2013/11/29 04:25:13]

Early return from here instead maybe to save on indents?

[chihchung, 2013/11/29 14:06:30]

Done.

+    // Check if the format is NV12
+    if (image.format.fourcc == VA_FOURCC_NV12) {
+      // Map the buffer
+      va_res = VAAPI_MapBuffer(va_display_, image.buf, &mem);
+      VA_LOG_ON_ERROR(va_res, "vaMapBuffer failed");
+      if (va_res == VA_STATUS_SUCCESS) {
+        result = CopyNV12ToI420(&image, mem);
+        // Unmap the buffer
+        VAAPI_UnmapBuffer(va_display_, image.buf);
+      }
+    } else {
+      LOG(ERROR) << "unexpected image format: " << image.format.fourcc;

[Pawel Osciak, 2013/11/29 04:25:13]

Please use DVLOG() instead so as to not pollute the logs and fail at call site.

[chihchung, 2013/11/29 14:06:30]

Done.

+    }
+    VAAPI_DestroyImage(va_display_, image.image_id);
+  }
+
+  return result;
+}
+
 // static
 bool VaapiWrapper::PostSandboxInitialization() {
   vaapi_handle = dlopen("libva.so.1", RTLD_NOW);
@@ -468,6 +559,10 @@ bool VaapiWrapper::PostSandboxInitialization() {
   VAAPI_DLSYM_OR_RETURN_ON_ERROR(RenderPicture, vaapi_handle);
   VAAPI_DLSYM_OR_RETURN_ON_ERROR(SyncSurface, vaapi_handle);
   VAAPI_DLSYM_OR_RETURN_ON_ERROR(Terminate, vaapi_handle);
+  VAAPI_DLSYM_OR_RETURN_ON_ERROR(MapBuffer, vaapi_handle);
+  VAAPI_DLSYM_OR_RETURN_ON_ERROR(UnmapBuffer, vaapi_handle);
+  VAAPI_DLSYM_OR_RETURN_ON_ERROR(DeriveImage, vaapi_handle);
+  VAAPI_DLSYM_OR_RETURN_ON_ERROR(DestroyImage, vaapi_handle);
 #undef VAAPI_DLSYM
 
   return true;