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

content/common/gpu/media/vaapi_h264_decoder.cc

Index: content/common/gpu/media/vaapi_h264_decoder.cc
diff --git a/content/common/gpu/media/vaapi_h264_decoder.cc b/content/common/gpu/media/vaapi_h264_decoder.cc
index cb716b29fb75c32969b160cbd644919ceb60394d..66cfe938ad8f2427bb7884ae903860be0d9ccaf4 100644
--- a/content/common/gpu/media/vaapi_h264_decoder.cc
+++ b/content/common/gpu/media/vaapi_h264_decoder.cc
@@ -20,7 +20,7 @@
       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 {                                                     \
@@ -371,6 +371,10 @@ void VaapiH264Decoder::Reset() {
   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>();
 
@@ -416,10 +420,11 @@ void VaapiH264Decoder::Destroy() {
         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;
@@ -600,7 +605,11 @@ bool VaapiH264Decoder::CreateVASurfaces() {
                                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;
 }
@@ -617,6 +626,21 @@ void VaapiH264Decoder::DestroyVASurfaces() {
   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));
@@ -995,6 +1019,14 @@ bool VaapiH264Decoder::QueueSlice(H264SliceHeader* slice_hdr) {
   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.
 
@@ -1021,44 +1053,41 @@ bool VaapiH264Decoder::DecodePicture() {
                                        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.