VaapiVideoAccelerator: make Vaapi accelerator work with ozone

content/common/gpu/media/gpu_video_decode_accelerator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Pawel Osciak, 2014/08/25 01:13:23]

With TEST=None in CL description, how was this tested?

You haven't modified video_decode_accelerator_unittest.cc. Please make it
compile on ozone, fix it as needed and make sure it passes all test cases and
that display from it works as well. Please also make sure that this works
properly with dynamic resolution change.

Also please make sure everything still works on X.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/gpu/media/gpu_video_decode_accelerator.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 12 matching lines @@
 #include "ui/gl/gl_surface_egl.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #include "content/common/gpu/media/dxva_video_decode_accelerator.h"
 #elif defined(OS_MACOSX)
 #include "content/common/gpu/media/vt_video_decode_accelerator.h"
 #elif defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL) && defined(USE_X11)
 #include "content/common/gpu/media/v4l2_video_decode_accelerator.h"
 #include "content/common/gpu/media/v4l2_video_device.h"
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
+#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
 #include "content/common/gpu/media/vaapi_video_decode_accelerator.h"
-#include "ui/gl/gl_context_glx.h"
 #include "ui/gl/gl_implementation.h"
 #elif defined(USE_OZONE)
 #include "media/ozone/media_ozone_platform.h"
 #elif defined(OS_ANDROID)
 #include "content/common/gpu/media/android_video_decode_accelerator.h"
 #endif
 
 #include "ui/gfx/size.h"
 
 namespace content {
@@ skipping 218 matching lines @@
     SendCreateDecoderReply(init_done_msg, false);
     return;
   }
   video_decode_accelerator_.reset(new V4L2VideoDecodeAccelerator(
       gfx::GLSurfaceEGL::GetHardwareDisplay(),
       stub_->decoder()->GetGLContext()->GetHandle(),
       weak_factory_for_io_.GetWeakPtr(),
       make_context_current_,
       device.Pass(),
       io_message_loop_));
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
+#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
+#if defined(USE_X11)

[Pawel Osciak, 2014/08/25 01:13:23]

Do we need to make the X11 decision here? Could VaapiVDA fail initialization if
its display backend fails to initialize if something is not supported?

   if (gfx::GetGLImplementation() != gfx::kGLImplementationDesktopGL) {
     VLOG(1) << "HW video decode acceleration not available without "
                "DesktopGL (GLX).";
     SendCreateDecoderReply(init_done_msg, false);
     return;
   }
-  gfx::GLContextGLX* glx_context =
-      static_cast<gfx::GLContextGLX*>(stub_->decoder()->GetGLContext());
-  video_decode_accelerator_.reset(new VaapiVideoDecodeAccelerator(
-      glx_context->display(), make_context_current_));
-#elif defined(USE_OZONE)
-  media::MediaOzonePlatform* platform =

[spang, 2014/08/22 17:20:23]

I would like to move the vaapi & v4l2 code out of content/common/gpu/media/, and
let the platform construct the VDA it wants.

So rather than removing this MediaOzonePlatform path, we should make it work
properly on Intel.

[Pawel Osciak, 2014/08/25 01:13:23]

Could you please explain why and how/to where would you like to move those VDAs?
They are not really platform-dependent, only their display backend may be. And
they don't care if we are ozone or not either...

I agree we should abstract and move away perhaps the picture/display-related
code that depends on the backend. But the VDAs should stay here, they are
abstract enough.

[spang, 2014/08/28 19:52:08]

I was thinking we should move it to media/ somewhere and let the platform
instantiate it from media/ozone/.

They are platform dependent, in that you may or may not have an implementation
on your platform, as well as in their integration with the display. It seems
cleaner to just have content ask for a working VDA object, and let the platform
instantiate something appropriate.

So that's the model I was proposing: move the implementations to media/, and
give platforms control of building them. And if chromium porters somewhere have
some other API than V4L2 or VA, they can use that to.

[Pawel Osciak, 2014/08/29 11:26:29]

Agreed, but as you said, VDA impl choice is orthogonal to display backend
choice. One VDA may be using different backends, and the choice criteria will
not be the same either.

I'd prefer if we could keep this one only about the backend choice for display
for VaapiVDA please. I think that is complicated enough already :)
Sure, this is worth discussing, but this would definitely be out of scope of
this change :)

-      media::MediaOzonePlatform::GetInstance();
-  video_decode_accelerator_.reset(platform->CreateVideoDecodeAccelerator(
-      make_context_current_));
-  if (!video_decode_accelerator_) {
+#else
+  if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
+    VLOG(1) << "HW video decode acceleration not available without "

[Pawel Osciak, 2014/08/25 01:13:23]

Please use DVLOG or DLOG

+               "EGLGLES2.";
     SendCreateDecoderReply(init_done_msg, false);
     return;
   }
+#endif // USE_X11
+  video_decode_accelerator_.reset(new VaapiVideoDecodeAccelerator(
+      stub_->decoder()->GetGLContext(), make_context_current_));
 #elif defined(OS_ANDROID)
   video_decode_accelerator_.reset(new AndroidVideoDecodeAccelerator(
       stub_->decoder()->AsWeakPtr(),
       make_context_current_));
 #else
   NOTIMPLEMENTED() << "HW video decode acceleration not available.";
   SendCreateDecoderReply(init_done_msg, false);
   return;
 #endif
 
@@ skipping 201 matching lines @@
   return stub_->channel()->Send(message);
 }
 
 void GpuVideoDecodeAccelerator::SendCreateDecoderReply(IPC::Message* message,
                                                        bool succeeded) {
   GpuCommandBufferMsg_CreateVideoDecoder::WriteReplyParams(message, succeeded);
   Send(message);
 }
 
 }  // namespace content