VAVDA: Destroy vaapi buffers after we're done drawing.

content/common/gpu/media/vaapi_h264_decoder.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 <dlfcn.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "content/common/gpu/gl_scoped_binders.h"
 #include "content/common/gpu/media/vaapi_h264_decoder.h"
 #include "third_party/libva/va/va.h"
 #include "third_party/libva/va/va_x11.h"
 #include "ui/gl/gl_bindings.h"
 
 #define VA_LOG_ON_ERROR(va_res, err_msg)                   \
   do {                                                     \
     if ((va_res) != VA_STATUS_SUCCESS) {                   \
       DVLOG(1) << err_msg                                  \
                << " VA error: " << VAAPI_ErrorStr(va_res); \
     }                                                      \
-  } while(0)
+  } while (0)
 
 #define VA_SUCCESS_OR_RETURN(va_res, err_msg, ret)         \
   do {                                                     \
     if ((va_res) != VA_STATUS_SUCCESS) {                   \
       DVLOG(1) << err_msg                                  \
                << " VA error: " << VAAPI_ErrorStr(va_res); \
       return (ret);                                        \
     }                                                      \
   } while (0)
 
@@ skipping 330 matching lines @@
   frame_num_ = 0;
   prev_frame_num_ = -1;
   prev_frame_num_offset_ = -1;
 
   prev_ref_has_memmgmnt5_ = false;
   prev_ref_top_field_order_cnt_ = -1;
   prev_ref_pic_order_cnt_msb_ = -1;
   prev_ref_pic_order_cnt_lsb_ = -1;
   prev_ref_field_ = H264Picture::FIELD_NONE;
 
+  // When called from the constructor, although va_display_ is invalid,
+  // |pending_slice_bufs_| and |pending_va_bufs_| are empty.
+  DestroyPendingBuffers();
+
   pending_slice_bufs_ = std::queue<VABufferID>();
   pending_va_bufs_ = std::queue<VABufferID>();
 
   ref_pic_list0_.clear();
   ref_pic_list1_.clear();
 
   for (POCToDecodeSurfaces::iterator it = poc_to_decode_surfaces_.begin();
        it != poc_to_decode_surfaces_.end(); ) {
     int poc = it->second->poc();
     // Must be incremented before UnassignSurfaceFromPoC as this call
@@ skipping 25 matching lines @@
     case kDecoding:
     case kAfterReset:
     case kError:
       destroy_surfaces = true;
       // fallthrough
     case kInitialized:
       if (!make_context_current_.Run())
         break;
       if (destroy_surfaces)
         DestroyVASurfaces();
+      DestroyPendingBuffers();
       va_res = VAAPI_DestroyConfig(va_display_, va_config_id_);
       VA_LOG_ON_ERROR(va_res, "vaDestroyConfig failed");
-       va_res = VAAPI_Terminate(va_display_);
-       VA_LOG_ON_ERROR(va_res, "vaTerminate failed");
+      va_res = VAAPI_Terminate(va_display_);
+      VA_LOG_ON_ERROR(va_res, "vaTerminate failed");
       // fallthrough
     case kUninitialized:
       break;
   }
 
   state_ = kUninitialized;
 }
 
 // Maps Profile enum values to VaProfile values.
 bool VaapiH264Decoder::SetProfile(media::VideoCodecProfile profile) {
@@ skipping 160 matching lines @@
                                          GetRequiredNumOfPictures(),
                                          va_surface_ids_);
   VA_SUCCESS_OR_RETURN(va_res, "vaCreateSurfaces failed", false);
 
   DCHECK(decode_surfaces_.empty());
   // And create a context associated with them.
   va_res = VAAPI_CreateContext(va_display_, va_config_id_,
                                pic_width_, pic_height_, VA_PROGRESSIVE,
                                va_surface_ids_, GetRequiredNumOfPictures(),
                                &va_context_id_);
-  VA_SUCCESS_OR_RETURN(va_res, "vaCreateContext failed", false);
+
+  VA_SUCCESS_OR_RETURN(
+      va_res, "vaCreateContext failed",
+      (VAAPI_DestroySurfaces(va_display_, va_surface_ids_,
+      GetRequiredNumOfPictures()),  false));

[piman, 2012/08/03 22:29:46]

Let's not go overboard here, this is really unreadable IMO - a () expression
with side effects that's executed conditionally from inside a macro?

How about instead:
if (va_res != VA_STATUS_SUCCESS) {
  DVLOG(1) << ...
  VAAPI_DestroySurfaces(va_display_, va_surface_ids_,
GetRequiredNumOfPictures());
  return false;
}

[Ami GONE FROM CHROMIUM, 2012/08/03 22:33:08]

Hah :)  That's what was there when I suggested the comma operator.
I actually think this is *more* readable this way, but I don't think it's worth
arguing over (I made my original suggestion as a "FWIW" but I should have named
it a "nit").
Do what y'all like.

 
   return true;
 }
 
 void VaapiH264Decoder::DestroyVASurfaces() {
   DCHECK(state_ == kDecoding || state_ == kError || state_ == kAfterReset);
   decode_surfaces_.clear();
 
   VAStatus va_res = VAAPI_DestroyContext(va_display_, va_context_id_);
   VA_LOG_ON_ERROR(va_res, "vaDestroyContext failed");
 
   va_res = VAAPI_DestroySurfaces(va_display_, va_surface_ids_,
                                  GetRequiredNumOfPictures());
   VA_LOG_ON_ERROR(va_res, "vaDestroySurfaces failed");
 }
 
+void VaapiH264Decoder::DestroyPendingBuffers() {
+  while (!pending_slice_bufs_.empty()) {
+    VABufferID buffer = pending_slice_bufs_.front();
+    VAStatus va_res = VAAPI_DestroyBuffer(va_display_, buffer);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyBuffer failed");
+    pending_slice_bufs_.pop();
+  }
+  while (!pending_va_bufs_.empty()) {
+    VABufferID buffer = pending_va_bufs_.front();
+    VAStatus va_res = VAAPI_DestroyBuffer(va_display_, buffer);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyBuffer failed");
+    pending_va_bufs_.pop();
+  }
+}
+
 // Fill |va_pic| with default/neutral values.
 static void InitVAPicture(VAPictureH264* va_pic) {
   memset(va_pic, 0, sizeof(*va_pic));
   va_pic->picture_id = VA_INVALID_ID;
   va_pic->flags = VA_PICTURE_H264_INVALID;
 }
 
 void VaapiH264Decoder::FillVAPicture(VAPictureH264 *va_pic, H264Picture* pic) {
   POCToDecodeSurfaces::iterator iter = poc_to_decode_surfaces_.find(
       pic->pic_order_cnt);
@@ skipping 358 matching lines @@
 
   if (!SendVASliceParam(slice_hdr))
     return false;
 
   if (!SendSliceData(slice_hdr->nalu_data, slice_hdr->nalu_size))
     return false;
 
   return true;
 }
 
+void VaapiH264Decoder::DestroyBuffers(VABufferID* va_buffers,
+                                      size_t num_va_buffers) {
+  for (size_t i = 0; i < num_va_buffers; ++i) {
+    VAStatus va_res = VAAPI_DestroyBuffer(va_display_, va_buffers[i]);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyBuffer failed");
+  }
+}
+
 // TODO(posciak) start using vaMapBuffer instead of vaCreateBuffer wherever
 // possible.
 
 bool VaapiH264Decoder::DecodePicture() {
   DCHECK(!frame_ready_at_hw_);
   DCHECK(curr_pic_.get());
 
   static const size_t kMaxVABuffers = 32;
   DCHECK_LE(pending_va_bufs_.size(), kMaxVABuffers);
   DCHECK_LE(pending_slice_bufs_.size(), kMaxVABuffers);
 
   DVLOG(4) << "Pending VA bufs to commit: " << pending_va_bufs_.size();
   DVLOG(4) << "Pending slice bufs to commit: " << pending_slice_bufs_.size();
 
   // Find the surface associated with the picture to be decoded.
   DCHECK(pending_slice_bufs_.size());
   DecodeSurface* dec_surface =
       poc_to_decode_surfaces_[curr_pic_->pic_order_cnt];
   DVLOG(4) << "Decoding POC " << curr_pic_->pic_order_cnt
            << " into surface " << dec_surface->va_surface_id();
 
   // Get ready to decode into surface.
   VAStatus va_res = VAAPI_BeginPicture(va_display_, va_context_id_,
                                        dec_surface->va_surface_id());
   VA_SUCCESS_OR_RETURN(va_res, "vaBeginPicture failed", false);
 
-  // Put buffer IDs for pending parameter buffers into buffers[].
-  VABufferID buffers[kMaxVABuffers];
-  size_t num_buffers = pending_va_bufs_.size();
-  for (size_t i = 0; i < num_buffers && i < kMaxVABuffers; ++i) {
-    buffers[i] = pending_va_bufs_.front();
+  // Put buffer IDs for pending parameter buffers into va_buffers[].
+  VABufferID va_buffers[kMaxVABuffers];
+  size_t num_va_buffers = pending_va_bufs_.size();
+  for (size_t i = 0; i < num_va_buffers && i < kMaxVABuffers; ++i) {
+    va_buffers[i] = pending_va_bufs_.front();
     pending_va_bufs_.pop();
   }
+  ScopedClosureRunner va_buffers_deleter(
+      base::Bind(&VaapiH264Decoder::DestroyBuffers, this, va_buffers,
+      num_va_buffers));
 
   // And send them to the HW decoder.
-  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, buffers,
-                               num_buffers);
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, va_buffers,
+                               num_va_buffers);
   VA_SUCCESS_OR_RETURN(va_res, "vaRenderPicture for va_bufs failed", false);
 
-  DVLOG(4) << "Committed " << num_buffers << "VA buffers";
+  DVLOG(4) << "Committed " << num_va_buffers << "VA buffers";
 
-  for (size_t i = 0; i < num_buffers; ++i) {
-    va_res = VAAPI_DestroyBuffer(va_display_, buffers[i]);
-    VA_SUCCESS_OR_RETURN(va_res, "vaDestroyBuffer for va_bufs failed", false);
-  }
-
-  // Put buffer IDs for pending slice data buffers into buffers[].
-  num_buffers = pending_slice_bufs_.size();
-  for (size_t i = 0; i < num_buffers && i < kMaxVABuffers; ++i) {
-    buffers[i] = pending_slice_bufs_.front();
+  // Put buffer IDs for pending slice data buffers into slice_buffers[].
+  VABufferID slice_buffers[kMaxVABuffers];
+  size_t num_slice_buffers = pending_slice_bufs_.size();
+  for (size_t i = 0; i < num_slice_buffers && i < kMaxVABuffers; ++i) {
+    slice_buffers[i] = pending_slice_bufs_.front();
     pending_slice_bufs_.pop();
   }
+  ScopedClosureRunner slice_buffers_deleter(
+      base::Bind(&VaapiH264Decoder::DestroyBuffers, this, slice_buffers,
+      num_slice_buffers));
 
   // And send them to the Hw decoder.
-  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, buffers,
-                               num_buffers);
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, slice_buffers,
+                               num_slice_buffers);
   VA_SUCCESS_OR_RETURN(va_res, "vaRenderPicture for slices failed", false);
 
-  DVLOG(4) << "Committed " << num_buffers << "slice buffers";
-
-  for (size_t i = 0; i < num_buffers; ++i) {
-    va_res = VAAPI_DestroyBuffer(va_display_, buffers[i]);
-    VA_SUCCESS_OR_RETURN(va_res, "vaDestroyBuffer for slices failed", false);
-  }
+  DVLOG(4) << "Committed " << num_slice_buffers << "slice buffers";
 
   // Instruct HW decoder to start processing committed buffers (decode this
   // picture). This does not block until the end of decode.
   va_res = VAAPI_EndPicture(va_display_, va_context_id_);
   VA_SUCCESS_OR_RETURN(va_res, "vaEndPicture failed", false);
 
   // Used to notify clients that we had sufficient data to start decoding
   // a new frame.
   frame_ready_at_hw_ = true;
   return true;
@@ skipping 1020 matching lines @@
     }
   }
 }
 
 // static
 size_t VaapiH264Decoder::GetRequiredNumOfPictures() {
   return kNumReqPictures;
 }
 
 }  // namespace content