vaapi: cleanup a few comments and var names

This CL cleans up a few things that I realized while
debugging other CLs in the area:
- |available_va_surfaces_| is a std::list on ToT, but has
 queue semantics, so it's changed int his CL.
-  s/output_buffers_/available_va_surfaces_/ because that's
 what they are.
- s/TryOutputSurface/TryOutputPicture/ because that's what
 the method does.
- s/pictures_/picture_map_/ to better define what it is. After
 dcastagna@ suggestion, it's made a base::small_map in PS7

Comments updated throughout these variables.

No change in behaviour intended, but tested nonetheless with
vp8/9-h264 crosvideo playback.

Bug: 822346
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;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2117ceac29f7d6ed23d698c4abd5c72e0140ae47
Reviewed-on: https://chromium-review.googlesource.com/988512
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547552}

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -129,8 +129,8 @@ 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()) {
+  PictureMap::iterator it = picture_map_.find(picture_buffer_id);
+  if (it == picture_map_.end()) {
     VLOGF(4) << "Picture id " << picture_buffer_id << " does not exist";
     return NULL;
   }
@@ -245,22 +245,22 @@ void VaapiVideoDecodeAccelerator::OutputPicture(
   }
 }
 
-void VaapiVideoDecodeAccelerator::TryOutputSurface() {
+void VaapiVideoDecodeAccelerator::TryOutputPicture() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Handle Destroy() arriving while pictures are queued for output.
   if (!client_)
     return;
 
-  if (pending_output_cbs_.empty() || output_buffers_.empty())
+  if (pending_output_cbs_.empty() || available_picture_buffers_.empty())
     return;
 
   OutputCB output_cb = pending_output_cbs_.front();
   pending_output_cbs_.pop();
 
-  VaapiPicture* picture = PictureById(output_buffers_.front());
+  VaapiPicture* picture = PictureById(available_picture_buffers_.front());
   DCHECK(picture);
-  output_buffers_.pop();
+  available_picture_buffers_.pop();
 
   output_cb.Run(picture);
 
@@ -409,9 +409,9 @@ void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange(
     return;
 
   if (!pending_output_cbs_.empty() ||
-      pictures_.size() != available_va_surfaces_.size()) {
+      picture_map_.size() != available_va_surfaces_.size()) {
     // Either: Not all |pending_output_cbs_| have been executed yet
-    // (i.e. they're waiting for resources in TryOutputSurface()), or |client_|
+    // (i.e. they're waiting for resources in TryOutputPicture()), or |client_|
     // hasn't returned all the |available_va_surfaces_| (via
     // RecycleVASurfaceID), In any case, give some time for both to happen.
     DVLOGF(2) << "Awaiting pending output/surface release callbacks to finish";
@@ -427,16 +427,16 @@ void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange(
 
   // All surfaces released, destroy them and dismiss all PictureBuffers.
   awaiting_va_surfaces_recycle_ = false;
-  available_va_surfaces_.clear();
+  available_va_surfaces_ = {};
   vaapi_wrapper_->DestroySurfaces();
 
-  for (Pictures::iterator iter = pictures_.begin(); iter != pictures_.end();
-       ++iter) {
+  for (PictureMap::iterator iter = picture_map_.begin();
+       iter != picture_map_.end(); ++iter) {
     VLOGF(2) << "Dismissing picture id: " << iter->first;
     if (client_)
       client_->DismissPictureBuffer(iter->first);
   }
-  pictures_.clear();
+  picture_map_.clear();
 
   // And ask for a new set as requested.
   VLOGF(2) << "Requesting " << requested_num_pics_
@@ -482,7 +482,7 @@ void VaapiVideoDecodeAccelerator::RecycleVASurfaceID(
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
 
-  available_va_surfaces_.push_back(va_surface_id);
+  available_va_surfaces_.push(va_surface_id);
   decoder_thread_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&VaapiVideoDecodeAccelerator::DecodeTask,
                                 base::Unretained(this)));
@@ -492,10 +492,9 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
     const std::vector<PictureBuffer>& buffers) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
-  DCHECK(pictures_.empty());
+  DCHECK(picture_map_.empty());
 
-  while (!output_buffers_.empty())
-    output_buffers_.pop();
+  available_picture_buffers_ = {};
 
   RETURN_AND_NOTIFY_ON_FAILURE(
       buffers.size() >= requested_num_pics_,
@@ -510,7 +509,8 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
                                      buffers.size(), &va_surface_ids),
       "Failed creating VA Surfaces", PLATFORM_FAILURE, );
   DCHECK_EQ(va_surface_ids.size(), buffers.size());
-  available_va_surfaces_.assign(va_surface_ids.begin(), va_surface_ids.end());
+  for (const auto id : va_surface_ids)
+    available_va_surfaces_.push(id);
 
   for (size_t i = 0; i < buffers.size(); ++i) {
     uint32_t client_id = !buffers[i].client_texture_ids().empty()
@@ -534,12 +534,11 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
       RETURN_AND_NOTIFY_ON_FAILURE(
           picture->Allocate(vaapi_picture_factory_->GetBufferFormat()),
           "Failed to allocate memory for a VaapiPicture", PLATFORM_FAILURE, );
-      output_buffers_.push(buffers[i].id());
+      available_picture_buffers_.push(buffers[i].id());
     }
-    bool inserted =
-        pictures_.insert(std::make_pair(buffers[i].id(), std::move(picture)))
-            .second;
-    DCHECK(inserted);
+    const auto result = picture_map_.emplace(
+        std::make_pair(buffers[i].id(), std::move(picture)));
+    DCHECK(result.second);
   }
 
   // Resume DecodeTask if it is still in decoding state.
@@ -612,8 +611,8 @@ void VaapiVideoDecodeAccelerator::ReusePictureBuffer(
   --num_frames_at_client_;
   TRACE_COUNTER1("media,gpu", "Vaapi frames at client", num_frames_at_client_);
 
-  output_buffers_.push(picture_buffer_id);
-  TryOutputSurface();
+  available_picture_buffers_.push(picture_buffer_id);
+  TryOutputPicture();
 }
 
 void VaapiVideoDecodeAccelerator::FlushTask() {
@@ -830,7 +829,7 @@ void VaapiVideoDecodeAccelerator::VASurfaceReady(
       base::Bind(&VaapiVideoDecodeAccelerator::OutputPicture, weak_this_,
                  va_surface, bitstream_id, visible_rect));
 
-  TryOutputSurface();
+  TryOutputPicture();
 }
 
 scoped_refptr<VASurface> VaapiVideoDecodeAccelerator::CreateVASurface() {
@@ -844,7 +843,7 @@ scoped_refptr<VASurface> VaapiVideoDecodeAccelerator::CreateVASurface() {
   scoped_refptr<VASurface> va_surface(new VASurface(
       available_va_surfaces_.front(), requested_pic_size_,
       vaapi_wrapper_->va_surface_format(), va_surface_release_cb_));
-  available_va_surfaces_.pop_front();
+  available_va_surfaces_.pop();
 
   return va_surface;
 }

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"
@@ -159,7 +160,7 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
                      VaapiPicture* picture);
 
   // Try to OutputPicture() if we have both a ready surface and picture.
-  void TryOutputSurface();
+  void TryOutputPicture();
 
   // Called when a VASurface is no longer in use by the decoder or is not being
   // synced/waiting to be synced to a picture. Returns it to available surfaces
@@ -189,19 +190,15 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
   State state_;
   Config::OutputMode output_mode_;
 
-  // Queue of input InputBuffers (PictureBuffer ids) to decode.
+  // Queue of InputBuffers (BitstreamBuffer id and data) to decode.
   base::queue<std::unique_ptr<InputBuffer>> input_buffers_;
   // Current InputBuffer at |decoder_|.
   // Only accessed on |decoder_thread_task_runner_| (needs no |lock_|)
   std::unique_ptr<InputBuffer> curr_input_buffer_;
 
-  // VA Surfaces no longer in use that can be passed back to the decoder for
-  // reuse, once it requests them.
-  std::list<VASurfaceID> available_va_surfaces_;
-
-  // Queue for incoming output buffers (texture ids).
-  using OutputBuffers = base::queue<int32_t>;
-  OutputBuffers output_buffers_;
+  // VASurfaceIDs available to use by |decoder_|, i.e. not assigned to a
+  // VASurface -- active between CreateVASurface() - VASurfaceReady().
+  base::queue<VASurfaceID> available_va_surfaces_;
 
   std::unique_ptr<VaapiPictureFactory> vaapi_picture_factory_;
 
@@ -209,14 +206,16 @@ 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.
+  // Queue of PictureBuffer ids available to be sent to |client_| via
+  // OutputPicture() (|client_| returns them via ReusePictureBuffer()).
+  base::queue<int32_t> available_picture_buffers_;
+  // Available VaapiPictures allocated by |client_| in AssignPictureBuffers()
+  // (and/or TryFinishSurfaceSetChange()). These pictures are indexed by the
+  // |available_picture_buffers_|.
+  using PictureMap =
+      base::small_map<std::map<int32_t, std::unique_ptr<VaapiPicture>>>;
+  PictureMap picture_map_;
+  // Returns the VaapiPicture indexed by |picture_buffer_id|, or nullptr.
   VaapiPicture* PictureById(int32_t picture_buffer_id);
 
   // Pending output requests from the decoder. When it indicates that we should