VaapiVideoAccelerator: make Vaapi accelerator work with ozone

content/common/gpu/media/vaapi_video_decode_accelerator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/synchronization/waitable_event.h"
-#include "base/threading/non_thread_safe.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/media/vaapi_video_decode_accelerator.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/video/picture.h"
 #include "ui/gl/gl_bindings.h"
-#include "ui/gl/scoped_binders.h"
 
 static void ReportToUMA(
     content::VaapiH264Decoder::VAVDAH264DecoderFailure failure) {
   UMA_HISTOGRAM_ENUMERATION(
       "Media.VAVDAH264.DecoderFailure",
       failure,
       content::VaapiH264Decoder::VAVDA_H264_DECODER_FAILURES_MAX);
 }
 
 namespace content {
@@ skipping 25 matching lines @@
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &VaapiVideoDecodeAccelerator::Cleanup, weak_this_));
 
   DVLOG(1) << "Notifying of error " << error;
   if (client_) {
     client_->NotifyError(error);
     client_ptr_factory_.reset();
   }
 }
 
-// TFPPicture allocates X Pixmaps and binds them to textures passed
-// in PictureBuffers from clients to them. TFPPictures are created as
-// a consequence of receiving a set of PictureBuffers from clients and released
-// at the end of decode (or when a new set of PictureBuffers is required).
-//
-// TFPPictures are used for output, contents of VASurfaces passed from decoder
-// are put into the associated pixmap memory and sent to client.
-class VaapiVideoDecodeAccelerator::TFPPicture : public base::NonThreadSafe {
- public:
-  ~TFPPicture();
-
-  static linked_ptr<TFPPicture> Create(
-      const base::Callback<bool(void)>& make_context_current,
-      const GLXFBConfig& fb_config,
-      Display* x_display,
-      int32 picture_buffer_id,
-      uint32 texture_id,
-      gfx::Size size);
-
-  int32 picture_buffer_id() {
-    return picture_buffer_id_;
-  }
-
-  gfx::Size size() {
-    return size_;
-  }
-
-  int x_pixmap() {
-    return x_pixmap_;
-  }
-
-  // Bind texture to pixmap. Needs to be called every frame.
-  bool Bind();
-
- private:
-  TFPPicture(const base::Callback<bool(void)>& make_context_current,
-             Display* x_display,
-             int32 picture_buffer_id,
-             uint32 texture_id,
-             gfx::Size size);
-
-  bool Initialize(const GLXFBConfig& fb_config);
-
-  base::Callback<bool(void)> make_context_current_;
-
-  Display* x_display_;
-
-  // Output id for the client.
-  int32 picture_buffer_id_;
-  uint32 texture_id_;
-
-  gfx::Size size_;
-
-  // Pixmaps bound to this texture.
-  Pixmap x_pixmap_;
-  GLXPixmap glx_pixmap_;
-
-  DISALLOW_COPY_AND_ASSIGN(TFPPicture);
-};
-
-VaapiVideoDecodeAccelerator::TFPPicture::TFPPicture(
-    const base::Callback<bool(void)>& make_context_current,
-    Display* x_display,
-    int32 picture_buffer_id,
-    uint32 texture_id,
-    gfx::Size size)
-    : make_context_current_(make_context_current),
-      x_display_(x_display),
-      picture_buffer_id_(picture_buffer_id),
-      texture_id_(texture_id),
-      size_(size),
-      x_pixmap_(0),
-      glx_pixmap_(0) {
-  DCHECK(!make_context_current_.is_null());
-};
-
-linked_ptr<VaapiVideoDecodeAccelerator::TFPPicture>
-VaapiVideoDecodeAccelerator::TFPPicture::Create(
-    const base::Callback<bool(void)>& make_context_current,
-    const GLXFBConfig& fb_config,
-    Display* x_display,
-    int32 picture_buffer_id,
-    uint32 texture_id,
-    gfx::Size size) {
-
-  linked_ptr<TFPPicture> tfp_picture(
-      new TFPPicture(make_context_current, x_display, picture_buffer_id,
-                     texture_id, size));
-
-  if (!tfp_picture->Initialize(fb_config))
-    tfp_picture.reset();
-
-  return tfp_picture;
-}
-
-bool VaapiVideoDecodeAccelerator::TFPPicture::Initialize(
-    const GLXFBConfig& fb_config) {
-  DCHECK(CalledOnValidThread());
-  if (!make_context_current_.Run())
-    return false;
-
-  XWindowAttributes win_attr;
-  int screen = DefaultScreen(x_display_);
-  XGetWindowAttributes(x_display_, RootWindow(x_display_, screen), &win_attr);
-  //TODO(posciak): pass the depth required by libva, not the RootWindow's depth
-  x_pixmap_ = XCreatePixmap(x_display_, RootWindow(x_display_, screen),
-                            size_.width(), size_.height(), win_attr.depth);
-  if (!x_pixmap_) {
-    DVLOG(1) << "Failed creating an X Pixmap for TFP";
-    return false;
-  }
-
-  static const int pixmap_attr[] = {
-    GLX_TEXTURE_TARGET_EXT, GLX_TEXTURE_2D_EXT,
-    GLX_TEXTURE_FORMAT_EXT, GLX_TEXTURE_FORMAT_RGB_EXT,
-    GL_NONE,
-  };
-
-  glx_pixmap_ = glXCreatePixmap(x_display_, fb_config, x_pixmap_, pixmap_attr);
-  if (!glx_pixmap_) {
-    // x_pixmap_ will be freed in the destructor.
-    DVLOG(1) << "Failed creating a GLX Pixmap for TFP";
-    return false;
-  }
-
-  return true;
-}
-
-VaapiVideoDecodeAccelerator::TFPPicture::~TFPPicture() {
-  DCHECK(CalledOnValidThread());
-  // Unbind surface from texture and deallocate resources.
-  if (glx_pixmap_ && make_context_current_.Run()) {
-    glXReleaseTexImageEXT(x_display_, glx_pixmap_, GLX_FRONT_LEFT_EXT);
-    glXDestroyPixmap(x_display_, glx_pixmap_);
-  }
-
-  if (x_pixmap_)
-    XFreePixmap(x_display_, x_pixmap_);
-  XSync(x_display_, False);  // Needed to work around buggy vdpau-driver.
-}
-
-bool VaapiVideoDecodeAccelerator::TFPPicture::Bind() {
-  DCHECK(CalledOnValidThread());
-  DCHECK(x_pixmap_);
-  DCHECK(glx_pixmap_);
-  if (!make_context_current_.Run())
-    return false;
-
-  gfx::ScopedTextureBinder texture_binder(GL_TEXTURE_2D, texture_id_);
-  glXBindTexImageEXT(x_display_, glx_pixmap_, GLX_FRONT_LEFT_EXT, NULL);
-
-  return true;
-}
-
-VaapiVideoDecodeAccelerator::TFPPicture*
-    VaapiVideoDecodeAccelerator::TFPPictureById(int32 picture_buffer_id) {
-  TFPPictures::iterator it = tfp_pictures_.find(picture_buffer_id);
-  if (it == tfp_pictures_.end()) {
-    DVLOG(1) << "Picture id " << picture_buffer_id << " does not exist";
-    return NULL;
-  }
-
-  return it->second.get();
+VaapiPictureProvider::Picture* VaapiVideoDecodeAccelerator::PictureById(
+    int32 picture_buffer_id) {
+  VaapiPictureProvider::Picture* picture = pictures_.get(picture_buffer_id);
+  return picture;
 }
 
 VaapiVideoDecodeAccelerator::VaapiVideoDecodeAccelerator(
-    Display* x_display,
+    gfx::GLContext* gl_context,
     const base::Callback<bool(void)>& make_context_current)
-    : x_display_(x_display),
+    : gl_context_(gl_context),
       make_context_current_(make_context_current),
       state_(kUninitialized),
       input_ready_(&lock_),
       surfaces_available_(&lock_),
       message_loop_(base::MessageLoop::current()),
       decoder_thread_("VaapiDecoderThread"),
       num_frames_at_client_(0),
       num_stream_bufs_at_decoder_(0),
       finish_flush_pending_(false),
       awaiting_va_surfaces_recycle_(false),
       requested_num_pics_(0),
       weak_this_factory_(this) {
   weak_this_ = weak_this_factory_.GetWeakPtr();
   va_surface_release_cb_ = media::BindToCurrentLoop(
       base::Bind(&VaapiVideoDecodeAccelerator::RecycleVASurfaceID, weak_this_));
 }
 
 VaapiVideoDecodeAccelerator::~VaapiVideoDecodeAccelerator() {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
-}
-
-class XFreeDeleter {
- public:
-  void operator()(void* x) const {
-    ::XFree(x);
-  }
-};
-
-bool VaapiVideoDecodeAccelerator::InitializeFBConfig() {
-  const int fbconfig_attr[] = {
-    GLX_DRAWABLE_TYPE, GLX_PIXMAP_BIT,
-    GLX_BIND_TO_TEXTURE_TARGETS_EXT, GLX_TEXTURE_2D_BIT_EXT,
-    GLX_BIND_TO_TEXTURE_RGB_EXT, GL_TRUE,
-    GLX_Y_INVERTED_EXT, GL_TRUE,
-    GL_NONE,
-  };
-
-  int num_fbconfigs;
-  scoped_ptr<GLXFBConfig, XFreeDeleter> glx_fb_configs(
-      glXChooseFBConfig(x_display_, DefaultScreen(x_display_), fbconfig_attr,
-                        &num_fbconfigs));
-  if (!glx_fb_configs)
-    return false;
-  if (!num_fbconfigs)
-    return false;
-
-  fb_config_ = glx_fb_configs.get()[0];
-  return true;
+  // Make sure pictures are destroyed before the picture provider is.
+  pictures_.clear();

[Pawel Osciak, 2014/09/24 11:27:12]

Looks like each Picture should rather have a refptr to the provider.

[llandwerlin-old, 2014/10/02 14:14:23]

Right, thanks, the comment is misleading here (leftover from previous version of
the CL). But with the changes you asked for (every picture should be able to
destroy itself) no pointer to the VaapiPictureProvider is required.

I'll add a comment to mention that pictures need to be destroyed before the
VaapiWrapper otherwise their VADisplay might be invalid.

 }
 
 bool VaapiVideoDecodeAccelerator::Initialize(media::VideoCodecProfile profile,
                                              Client* client) {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
 
   client_ptr_factory_.reset(new base::WeakPtrFactory<Client>(client));
   client_ = client_ptr_factory_->GetWeakPtr();
 
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(state_, kUninitialized);
   DVLOG(2) << "Initializing VAVDA, profile: " << profile;
 
-  if (!make_context_current_.Run())
-    return false;
-
-  if (!InitializeFBConfig()) {
-    DVLOG(1) << "Could not get a usable FBConfig";
+#if defined(USE_X11)
+  if (gfx::GetGLImplementation() != gfx::kGLImplementationDesktopGL) {
+    DVLOG(1) << "HW video decode acceleration not available without "
+               "DesktopGL (GLX).";
     return false;
   }
+#else
+  if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
+    DVLOG(1) << "HW video decode acceleration not available without "
+               "EGLGLES2.";
+    return false;
+  }
+#endif // USE_X11
 
   vaapi_wrapper_ = VaapiWrapper::Create(
       VaapiWrapper::kDecode,
       profile,
-      x_display_,
       base::Bind(&ReportToUMA, content::VaapiH264Decoder::VAAPI_ERROR));
 
   if (!vaapi_wrapper_.get()) {
     DVLOG(1) << "Failed initializing VAAPI";
     return false;
   }
 
+  vaapi_picture_provider_ = VaapiPictureProvider::Create(
+      vaapi_wrapper_->GetDisplay(),

[Pawel Osciak, 2014/09/24 11:27:12]

Could the provider get its own display? Or better, as mentioned before, use its
own VaapiWrapper?

[llandwerlin-old, 2014/10/02 14:14:23]

You need to have the all VA objects (VASurface/VAContext/etc...) allocated from
the same display object (parent object) in order to do operations across them
(VPP stuff or call vaPutSurface()). If it's not the case, you will experience
crashes in libva.

Would you like me to give the VaapiWrapper object instead?

[Pawel Osciak, 2014/10/08 08:17:21]

Yes, please. I'd really prefer to use a common VaapiWrapper. Then we wouldn't
have to worry about ownership, lifetimes, synchronization and same display...?

+      gl_context_,
+      make_context_current_);
+
+  if (!vaapi_picture_provider_.get()) {
+    DVLOG(1) << "Failed initializing VAAPI Picture Provider";
+    return false;
+  }
+
   decoder_.reset(
       new VaapiH264Decoder(
           vaapi_wrapper_.get(),
           media::BindToCurrentLoop(base::Bind(
               &VaapiVideoDecodeAccelerator::SurfaceReady, weak_this_)),
           base::Bind(&ReportToUMA)));
 
   CHECK(decoder_thread_.Start());
   decoder_thread_proxy_ = decoder_thread_.message_loop_proxy();
 
@@ skipping 14 matching lines @@
   pending_output_cbs_.push(
       base::Bind(&VaapiVideoDecodeAccelerator::OutputPicture,
                  weak_this_, va_surface, input_id));
 
   TryOutputSurface();
 }
 
 void VaapiVideoDecodeAccelerator::OutputPicture(
     const scoped_refptr<VASurface>& va_surface,
     int32 input_id,
-    TFPPicture* tfp_picture) {
+    VaapiPictureProvider::Picture* picture) {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
 
-  int32 output_id  = tfp_picture->picture_buffer_id();
+  int32 output_id = picture->picture_buffer_id();
 
   TRACE_EVENT2("Video Decoder", "VAVDA::OutputSurface",
                "input_id", input_id,
                "output_id", output_id);
 
   DVLOG(3) << "Outputting VASurface " << va_surface->id()
            << " into pixmap bound to picture buffer id " << output_id;
 
-  RETURN_AND_NOTIFY_ON_FAILURE(tfp_picture->Bind(),
-                               "Failed binding texture to pixmap",
-                               PLATFORM_FAILURE, );
-
   RETURN_AND_NOTIFY_ON_FAILURE(
-      vaapi_wrapper_->PutSurfaceIntoPixmap(va_surface->id(),
-                                           tfp_picture->x_pixmap(),
-                                           tfp_picture->size()),
-      "Failed putting surface into pixmap", PLATFORM_FAILURE, );
+      vaapi_picture_provider_->PutSurfaceIntoPicture(va_surface->id(), picture),
+      "Failed putting surface into pixmap",
+      PLATFORM_FAILURE, );
 
   // Notify the client a picture is ready to be displayed.
   ++num_frames_at_client_;
   TRACE_COUNTER1("Video Decoder", "Textures at client", num_frames_at_client_);
   DVLOG(4) << "Notifying output picture id " << output_id
            << " for input "<< input_id << " is ready";
   // TODO(posciak): Use visible size from decoder here instead
   // (crbug.com/402760).
   if (client_)
     client_->PictureReady(
-        media::Picture(output_id, input_id, gfx::Rect(tfp_picture->size())));
+        media::Picture(output_id, input_id, gfx::Rect(picture->size())));
 }
 
 void VaapiVideoDecodeAccelerator::TryOutputSurface() {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
 
   // Handle Destroy() arriving while pictures are queued for output.
   if (!client_)
     return;
 
   if (pending_output_cbs_.empty() || output_buffers_.empty())
     return;
 
   OutputCB output_cb = pending_output_cbs_.front();
   pending_output_cbs_.pop();
 
-  TFPPicture* tfp_picture = TFPPictureById(output_buffers_.front());
-  DCHECK(tfp_picture);
+  VaapiPictureProvider::Picture* picture = PictureById(output_buffers_.front());
+  DCHECK(picture);
   output_buffers_.pop();
 
-  output_cb.Run(tfp_picture);
+  output_cb.Run(picture);
 
   if (finish_flush_pending_ && pending_output_cbs_.empty())
     FinishFlush();
 }
 
 void VaapiVideoDecodeAccelerator::MapAndQueueNewInputBuffer(
     const media::BitstreamBuffer& bitstream_buffer) {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
   TRACE_EVENT1("Video Decoder", "MapAndQueueNewInputBuffer", "input_id",
       bitstream_buffer.id());
@@ skipping 182 matching lines @@
   TryFinishSurfaceSetChange();
 }
 
 void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange() {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
 
   if (!awaiting_va_surfaces_recycle_)
     return;
 
   if (!pending_output_cbs_.empty() ||
-      tfp_pictures_.size() != available_va_surfaces_.size()) {
+      pictures_.size() != available_va_surfaces_.size()) {
     // Either:
     // 1. Not all pending pending output callbacks have been executed yet.
     // Wait for the client to return enough pictures and retry later.
     // 2. The above happened and all surface release callbacks have been posted
     // as the result, but not all have executed yet. Post ourselves after them
     // to let them release surfaces.
     DVLOG(2) << "Awaiting pending output/surface release callbacks to finish";
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange, weak_this_));
     return;
   }
 
   // All surfaces released, destroy them and dismiss all PictureBuffers.
   awaiting_va_surfaces_recycle_ = false;
   available_va_surfaces_.clear();
   vaapi_wrapper_->DestroySurfaces();
 
-  for (TFPPictures::iterator iter = tfp_pictures_.begin();
-       iter != tfp_pictures_.end(); ++iter) {
+  for (Pictures::iterator iter = pictures_.begin(); iter != pictures_.end();
+       ++iter) {
     DVLOG(2) << "Dismissing picture id: " << iter->first;
     if (client_)
       client_->DismissPictureBuffer(iter->first);
   }
-  tfp_pictures_.clear();
+  pictures_.clear();
 
   // And ask for a new set as requested.
   DVLOG(1) << "Requesting " << requested_num_pics_ << " pictures of size: "
            << requested_pic_size_.ToString();
 
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &Client::ProvidePictureBuffers, client_,
       requested_num_pics_, requested_pic_size_, GL_TEXTURE_2D));
 }
 
@@ skipping 39 matching lines @@
 
   available_va_surfaces_.push_back(va_surface_id);
   surfaces_available_.Signal();
 }
 
 void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
     const std::vector<media::PictureBuffer>& buffers) {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
 
   base::AutoLock auto_lock(lock_);
-  DCHECK(tfp_pictures_.empty());
+  DCHECK(pictures_.empty());
 
   while (!output_buffers_.empty())
     output_buffers_.pop();
 
   RETURN_AND_NOTIFY_ON_FAILURE(
       buffers.size() == requested_num_pics_,
       "Got an invalid number of picture buffers. (Got " << buffers.size()
       << ", requested " << requested_num_pics_ << ")", INVALID_ARGUMENT, );
   DCHECK(requested_pic_size_ == buffers[0].size());
 
   std::vector<VASurfaceID> va_surface_ids;
   RETURN_AND_NOTIFY_ON_FAILURE(
       vaapi_wrapper_->CreateSurfaces(requested_pic_size_,
                                      buffers.size(),
                                      &va_surface_ids),
       "Failed creating VA Surfaces", PLATFORM_FAILURE, );
   DCHECK_EQ(va_surface_ids.size(), buffers.size());
 
+  RETURN_AND_NOTIFY_ON_FAILURE(
+      vaapi_picture_provider_->SetCodedSurfacesSize(requested_pic_size_),
+      "Failed to update VA picture provider surface size", PLATFORM_FAILURE, );
+
   for (size_t i = 0; i < buffers.size(); ++i) {
     DVLOG(2) << "Assigning picture id: " << buffers[i].id()
              << " to texture id: " << buffers[i].texture_id()
              << " VASurfaceID: " << va_surface_ids[i];
 
-    linked_ptr<TFPPicture> tfp_picture(
-        TFPPicture::Create(make_context_current_, fb_config_, x_display_,
-                           buffers[i].id(), buffers[i].texture_id(),
-                           requested_pic_size_));
+    scoped_ptr<VaapiPictureProvider::Picture> picture(
+        vaapi_picture_provider_->CreatePicture(buffers[i].id(),
+                                               buffers[i].texture_id(),
+                                               requested_pic_size_));
 
     RETURN_AND_NOTIFY_ON_FAILURE(
-        tfp_picture.get(), "Failed assigning picture buffer to a texture.",
+        picture.get(),
+        "Failed assigning picture buffer to a texture.",
         PLATFORM_FAILURE, );
 
-    bool inserted = tfp_pictures_.insert(std::make_pair(
-        buffers[i].id(), tfp_picture)).second;
-    DCHECK(inserted);
+    DCHECK(!pictures_.contains(buffers[i].id()));

[Pawel Osciak, 2014/09/24 11:27:12]

I think this should be s/!// ?

Are you testing on a Debug build?

[llandwerlin-old, 2014/10/02 14:14:23]

I might be wrong, but the purpose here is to check that the caller didn't
provide 2 buffers with the same id. Therefore it should be "not contained".

I'm using the version 9999 of chromeos-chrome package, not sure if that enables
dchecks.

[Pawel Osciak, 2014/10/08 08:17:21]

Ah you are right, sorry.
As for building chrome, I'd recommend
http://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chro....

+    pictures_.set(buffers[i].id(), picture.Pass());
 
     output_buffers_.push(buffers[i].id());
     available_va_surfaces_.push_back(va_surface_ids[i]);
     surfaces_available_.Signal();
   }
 
   state_ = kDecoding;
   decoder_thread_proxy_->PostTask(FROM_HERE, base::Bind(
       &VaapiVideoDecodeAccelerator::DecodeTask, base::Unretained(this)));
 }
@@ skipping 186 matching lines @@
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
   Cleanup();
   delete this;
 }
 
 bool VaapiVideoDecodeAccelerator::CanDecodeOnIOThread() {
   return false;
 }
 
 }  // namespace content