VaVDA: preparation for decode on client NativePixmaps

This CL extracts a few cleanups/preparations of crrev.com/c/1171566
(that is a big CL and might get reverted a few times).

- VaapiPicture::va_surface_id() allows for accessing the underlying
 VASurfaceID, if this is present (otherwise we return VA_INVALID_ID).

- Pictures is made a base::small_map (originally crrev.com/c/988512),
 PictureById() is removed in favour of using base::ContainsKey()
 on 3 callsites.

Bug: 822346, 717265
Test: v_d_a_unittest vp8/vp9/h264 passing on nautilus
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I18a0335462b321d25550d295cb628b4ddd3e2269
Reviewed-on: https://chromium-review.googlesource.com/1180295Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584256}

media/gpu/vaapi/vaapi_picture.cc

@@ -4,6 +4,8 @@
 
 #include "media/gpu/vaapi/vaapi_picture.h"
 
+#include <va/va.h>
+
 #include "media/gpu/vaapi/vaapi_wrapper.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_implementation.h"
@@ -36,4 +38,8 @@ bool VaapiPicture::AllowOverlay() const {
   return false;
 }
 
+VASurfaceID VaapiPicture::va_surface_id() const {
+  return VA_INVALID_ID;
+}
+
 }  // namespace media

media/gpu/vaapi/vaapi_picture.h

@@ -22,6 +22,8 @@
 
 namespace media {
 
+using VASurfaceID = unsigned int;
+
 class VASurface;
 class VaapiWrapper;
 
@@ -49,6 +51,9 @@ class MEDIA_GPU_EXPORT VaapiPicture {
   virtual bool DownloadFromSurface(
       const scoped_refptr<VASurface>& va_surface) = 0;
 
+  // Returns the associated VASurfaceID, if any, or VA_INVALID_ID.
+  virtual VASurfaceID va_surface_id() const;
+
  protected:
   VaapiPicture(const scoped_refptr<VaapiWrapper>& vaapi_wrapper,
                const MakeGLContextCurrentCallback& make_context_current_cb,

media/gpu/vaapi/vaapi_picture_native_pixmap.cc

@@ -46,6 +46,10 @@ bool VaapiPictureNativePixmap::AllowOverlay() const {
   return true;
 }
 
+VASurfaceID VaapiPictureNativePixmap::va_surface_id() const {
+  return va_surface_->id();
+}
+
 unsigned VaapiPictureNativePixmap::BufferFormatToInternalFormat(
     gfx::BufferFormat format) const {
   switch (format) {

media/gpu/vaapi/vaapi_picture_native_pixmap.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include "base/memory/ref_counted.h"
+#include "media/gpu/vaapi/va_surface.h"
 #include "media/gpu/vaapi/vaapi_picture.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/gfx/geometry/size.h"
@@ -41,6 +42,7 @@ class VaapiPictureNativePixmap : public VaapiPicture {
   // VaapiPicture implementation.
   bool DownloadFromSurface(const scoped_refptr<VASurface>& va_surface) override;
   bool AllowOverlay() const override;
+  VASurfaceID va_surface_id() const override;
 
   unsigned BufferFormatToInternalFormat(gfx::BufferFormat format) const;
 

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -128,17 +128,6 @@ void VaapiVideoDecodeAccelerator::NotifyError(Error error) {
   }
 }
 
-VaapiPicture* VaapiVideoDecodeAccelerator::PictureById(
-    int32_t picture_buffer_id) {
-  Pictures::iterator it = pictures_.find(picture_buffer_id);
-  if (it == pictures_.end()) {
-    DVLOGF(4) << "Picture id " << picture_buffer_id << " does not exist";
-    return NULL;
-  }
-
-  return it->second.get();
-}
-
 VaapiVideoDecodeAccelerator::VaapiVideoDecodeAccelerator(
     const MakeGLContextCurrentCallback& make_context_current_cb,
     const BindGLImageCallback& bind_image_cb)
@@ -258,11 +247,11 @@ void VaapiVideoDecodeAccelerator::TryOutputSurface() {
   if (pending_output_cbs_.empty() || output_buffers_.empty())
     return;
 
-  OutputCB output_cb = pending_output_cbs_.front();
+  OutputCB output_cb = std::move(pending_output_cbs_.front());
   pending_output_cbs_.pop();
 
-  VaapiPicture* picture = PictureById(output_buffers_.front());
-  DCHECK(picture);
+  DCHECK(base::ContainsKey(pictures_, output_buffers_.front()));
+  VaapiPicture* picture = pictures_[output_buffers_.front()].get();
   output_buffers_.pop();
 
   output_cb.Run(picture);
@@ -508,8 +497,7 @@ void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange() {
   available_va_surfaces_.clear();
   vaapi_wrapper_->DestroySurfaces();
 
-  for (Pictures::iterator iter = pictures_.begin(); iter != pictures_.end();
-       ++iter) {
+  for (auto iter = pictures_.begin(); iter != pictures_.end(); ++iter) {
     VLOGF(2) << "Dismissing picture id: " << iter->first;
     if (client_)
       client_->DismissPictureBuffer(iter->first);
@@ -601,10 +589,9 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
           "Failed to allocate memory for a VaapiPicture", PLATFORM_FAILURE, );
       output_buffers_.push(buffers[i].id());
     }
-    bool inserted =
-        pictures_.insert(std::make_pair(buffers[i].id(), std::move(picture)))
-            .second;
-    DCHECK(inserted);
+
+    DCHECK(!base::ContainsKey(pictures_, buffers[i].id()));
+    pictures_[buffers[i].id()] = std::move(picture);
 
     available_va_surfaces_.push_back(va_surface_ids[i]);
     surfaces_available_.Signal();
@@ -633,8 +620,7 @@ void VaapiVideoDecodeAccelerator::ImportBufferForPicture(
     return;
   }
 
-  VaapiPicture* picture = PictureById(picture_buffer_id);
-  if (!picture) {
+  if (!pictures_.count(picture_buffer_id)) {
     CloseGpuMemoryBufferHandle(gpu_memory_buffer_handle);
 
     // It's possible that we've already posted a DismissPictureBuffer for this
@@ -646,6 +632,7 @@ void VaapiVideoDecodeAccelerator::ImportBufferForPicture(
     return;
   }
 
+  VaapiPicture* picture = pictures_[picture_buffer_id].get();
   if (!picture->ImportGpuMemoryBufferHandle(
           VideoPixelFormatToGfxBufferFormat(pixel_format),
           gpu_memory_buffer_handle)) {
@@ -667,7 +654,7 @@ void VaapiVideoDecodeAccelerator::ReusePictureBuffer(
   TRACE_EVENT1("media,gpu", "VAVDA::ReusePictureBuffer", "Picture id",
                picture_buffer_id);
 
-  if (!PictureById(picture_buffer_id)) {
+  if (!pictures_.count(picture_buffer_id)) {
     // It's possible that we've already posted a DismissPictureBuffer for this
     // picture, but it has not yet executed when this ReusePictureBuffer
     // was posted to us by the client. In that case just ignore this (we've

media/gpu/vaapi/vaapi_video_decode_accelerator.h

@@ -18,6 +18,7 @@
 #include <vector>
 
 #include "base/containers/queue.h"
+#include "base/containers/small_map.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -220,15 +221,11 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
   scoped_refptr<VaapiWrapper> vaapi_wrapper_;
   std::unique_ptr<AcceleratedVideoDecoder> decoder_;
 
-  // All allocated Pictures, regardless of their current state. Pictures are
-  // allocated once using |create_vaapi_picture_callback_| and destroyed at the
-  // end of decode. Comes after |vaapi_wrapper_| to ensure all pictures are
-  // destroyed before said |vaapi_wrapper_| is destroyed.
-  using Pictures = std::map<int32_t, std::unique_ptr<VaapiPicture>>;
-  Pictures pictures_;
-
-  // Return a VaapiPicture associated with given client-provided id.
-  VaapiPicture* PictureById(int32_t picture_buffer_id);
+  // All allocated VaapiPictures, regardless of their current state. Pictures
+  // are allocated at AssignPictureBuffers() and are kept until dtor or
+  // TryFinishSurfaceSetChange(). Comes after |vaapi_wrapper_| to ensure all
+  // pictures are destroyed before this is destroyed.
+  base::small_map<std::map<int32_t, std::unique_ptr<VaapiPicture>>> pictures_;
 
   // VA Surfaces no longer in use that can be passed back to the decoder for
   // reuse, once it requests them.
@@ -242,7 +239,7 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
   // to use, we'll execute the callback passing the Picture. The callback
   // will put the contents of the surface into the picture and return it to
   // the client, releasing the surface as well.
-  // If we don't have any available Pictures at the time when the decoder
+  // If we don't have any available |pictures_| at the time when the decoder
   // requests output, we'll store the request on pending_output_cbs_ queue for
   // later and run it once the client gives us more textures
   // via ReusePictureBuffer().
@@ -250,7 +247,7 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
   base::queue<OutputCB> pending_output_cbs_;
 
   // ChildThread's task runner.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   // WeakPtr<> pointing to |this| for use in posting tasks from the decoder
   // thread back to the ChildThread.  Because the decoder thread is a member of