chromeos: decode video into NV12 format instead of RGBA in vaapi decoder

media/gpu/vaapi_drm_picture.cc

Index: media/gpu/vaapi_drm_picture.cc
diff --git a/media/gpu/vaapi_drm_picture.cc b/media/gpu/vaapi_drm_picture.cc
index b6e1f13731f76f636ec235ec48e8b0a81207506b..6220f134616be58b4b42b43beaf44ad80c491278 100644
--- a/media/gpu/vaapi_drm_picture.cc
+++ b/media/gpu/vaapi_drm_picture.cc
@@ -50,6 +50,9 @@ static unsigned BufferFormatToInternalFormat(gfx::BufferFormat format) {
     case gfx::BufferFormat::YVU_420:
       return GL_RGB_YCRCB_420_CHROMIUM;
 
+    case gfx::BufferFormat::YUV_420_BIPLANAR:
+      return GL_RGB_YCBCR_420V_CHROMIUM;
+
     default:
       NOTREACHED();
       return GL_BGRA_EXT;
@@ -104,8 +107,11 @@ bool VaapiDrmPicture::Initialize() {
 bool VaapiDrmPicture::Allocate(gfx::BufferFormat format) {
   ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance();
   ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone();
-  pixmap_ = factory->CreateNativePixmap(gfx::kNullAcceleratedWidget, size_,
-                                        format, gfx::BufferUsage::SCANOUT);
+  // TODO(dshwang): make it scanout when cros kernel supports NV12 overlay.
+  // crbug.com/683347

[gurchetansingh, 2017/06/22 23:31:59]

Shouldn't you be able to query drm plane resources whether or not this
particular kernel supports scanout?  We'll have certain  kernels (v3.18) that
will not support NV12 and other kernels (v4.4) that will

[dshwang, 2017/06/23 01:07:05]

there is good news and bad news.
bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
chromium can try to create NV12 framebuffer in startup time to make it
queryable.

good news: We never use NV12 framebuffer for v3.18. We will enable hardware
overlay only if the kernel version is >=4.4.

[gurchetansingh, 2017/06/23 03:18:33]

I don't understand.  drmModeGetPlaneResources should give you all the
information you need.  Chrome already queries from it (see
hardware_display_plane_manager.cc).  Can you access that info somehow?

The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and it
fails on start up?  Where is this code?  IMO that's much more hacky than
drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
reasons).

[dshwang, 2017/06/23 23:25:14]

Oh, that's good to know. It works. I'll make
ClientNativePixmapFactoryDmabuf::IsConfigurationSupported() reflect the result
of drmModeGetPlaneResources in runtime in another CL.

Note: drmModeGetPlaneResources say the following format list is overlayable in
Apollo Lake.
DRM_FORMAT_C8
DRM_FORMAT_RGB565
DRM_FORMAT_XRGB8888
DRM_FORMAT_XBGR8888
DRM_FORMAT_ARGB8888
DRM_FORMAT_ABGR8888
DRM_FORMAT_XRGB2101010
DRM_FORMAT_XBGR2101010
DRM_FORMAT_YUYV
DRM_FORMAT_YVYU
DRM_FORMAT_UYVY
DRM_FORMAT_VYUY

[gurchetansingh, 2017/06/24 01:50:48]

If NV12 is not overlay-able yet, that's fine.  Let's take what the kernel gives
us and not hard-code.  Where is IsConfigurationSupported called in this part of
the code-base?  Maybe we should think about how we want to wire up the
plane/overlay resource sharing first ... dcastagna@ any opinions?

[Daniele Castagna, 2017/06/26 18:06:43]

I'm not sure we want to dynamically pick the decoded format based on what is
supported for HW overlays at this point. A format that can be used as HW
overlays might not be the optimal for compositing/decoding, and we don't know
what we're doing with the decoded buffers at this point.

[dshwang, 2017/06/28 02:08:34]

In my opinion, we should choose NV12 always because it's the most efficient
format for vaapi decoders. cc will try to make it overlay later. If the hardware
(i.e. display controller) can overlay it, cc will do it.

+  pixmap_ =
+      factory->CreateNativePixmap(gfx::kNullAcceleratedWidget, size_, format,
+                                  gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);

[gurchetansingh, 2017/06/22 23:31:59]

What GBM use flags is the NV12 buffer allocated with?

[dshwang, 2017/06/23 01:07:05]

gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...

[gurchetansingh, 2017/06/23 03:18:33]

That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
(texture) from this buffer.  Are you only allocating LINEAR b/c of the
BO_USE_TEXTURE issue in minigbm?  Can you try the minigbm fix I suggested?

[dshwang, 2017/06/23 23:25:14]

Does "NV12 is Y-tiled in the upstream kernel" mean that scanout framebuffer need
y-tiling? That's true, but it requires on-going patches about NV12 in upstream
kernel.
https://patchwork.freedesktop.org/series/25377/
It's because vaapi requires linear buffer currently.
gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR to
BO_USE_LINEAR.
If I use tiling buffer, some glitch happen because vaapi lib cannot handle it.
I'll raise this issue to intel media team, but it will take long time. If this
CL is landed, it will help me a lot to provide reproducible test to the team.
Yes, I submitted https://chromium-review.googlesource.com/c/546884/

[gurchetansingh, 2017/06/24 01:50:48]

So the buffer is 

1) imported into VAAPI, 
2) a video stream is decoded into the NV12 buffer 
3) A glitch happens -- what kind of a glitch?

I noticed on the Android side tiled YV12 buffers on i915 cause artifacts. Since
one of the goals of overlays is end to end lossless tiled compression, IMO we
should really get to the bottom why this not working.  Can you file a bug
against the VAAPI team if they are Chromium committers?

In regards to test ability, we could just wire up one the drm-tests to use the
video API as well.  I don't want to commit things just so we can test.  

[dshwang, 2017/06/28 02:08:34]

I could not reproduce glitch anymore after repo sync. next patch uses tiling
texture, which is gfx::BufferUsage::GPU_READ. When I find the reason, I'll
report.

next patch set changes here to choose usage in runtime.

   if (!pixmap_) {
     DVLOG(1) << "Failed allocating a pixmap";
     return false;
@@ -137,7 +143,9 @@ bool VaapiDrmPicture::DownloadFromSurface(
 }
 
 bool VaapiDrmPicture::AllowOverlay() const {
-  return true;
+  // TODO(dshwang): make it true when cros kernel supports NV12 overlay.

[gurchetansingh, 2017/06/24 01:50:48]

Should be a runtime decision based on plane resources.

[Daniele Castagna, 2017/06/26 18:06:43]

This is really just some metadata that ends up in VideoFrame and should be ok to
always leave it to true.
the DrmOverlayValidator in Ozone will make the final call if we can use it as
overlay or if we need compositing.

DS, why did you change this?

[dshwang, 2017/06/28 02:08:34]

new patch set remove this change. as usage is chosen in runtime, it returns
either true or false in runtime.

+  // crbug.com/683347
+  return false;
 }
 
 }  // namespace media