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..5ad9760155ef533fec6fa1c6bcc6f984ca6aaf5d 100644
--- a/content/common/gpu/media/vaapi_h264_decoder.cc
+++ b/content/common/gpu/media/vaapi_h264_decoder.cc
@@ -31,6 +31,12 @@
     }                                                      \
   } while (0)
 
+#define VA_LOG_ERROR(va_res, err_msg)                     \
+  do {                                                    \
+    DVLOG(1) << err_msg                                   \
+             << " VA error: " << VAAPI_ErrorStr(va_res);  \
+  } while (0)
+
 namespace content {
 
 void *vaapi_handle = dlopen("libva.so", RTLD_NOW);
@@ -371,6 +377,21 @@ 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.
+  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();
+  }
+
   pending_slice_bufs_ = std::queue<VABufferID>();
   pending_va_bufs_ = std::queue<VABufferID>();
 
@@ -418,8 +439,8 @@ void VaapiH264Decoder::Destroy() {
         DestroyVASurfaces();
       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 +621,14 @@ 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);
+
+  if (va_res != VA_STATUS_SUCCESS) {
+    VAAPI_DestroySurfaces(va_display_, va_surface_ids_,
+                          GetRequiredNumOfPictures());
+    DVLOG(1) << "vaCreateContext failed"
+             << " VA error: " << VAAPI_ErrorStr(va_res);
+    return false;
+  }
 
   return true;
 }
@@ -995,6 +1023,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,49 +1057,53 @@ bool VaapiH264Decoder::DecodePicture() {
                                        dec_surface->va_surface_id());
   VA_SUCCESS_OR_RETURN(va_res, "vaBeginPicture failed", false);

[Pawel Osciak, 2012/08/03 18:06:39]

Actually, here we've already created va buffers, so they still have to be
deleted. That means you need to delete the pending_*_buffers in Destroy() as
well, like you do in Reset(). Same thing applies at line 1072, where the other
queue of pending buffers is not cleaned up.

 
-  // 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();
   }
 
   // And send them to the HW decoder.
-  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, buffers,
-                               num_buffers);
-  VA_SUCCESS_OR_RETURN(va_res, "vaRenderPicture for va_bufs failed", false);
-
-  DVLOG(4) << "Committed " << num_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);
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, va_buffers,
+                               num_va_buffers);
+  if (va_res != VA_STATUS_SUCCESS) {
+    DestroyBuffers(va_buffers, num_va_buffers);
+    VA_LOG_ERROR(va_res, "vaRenderPicture for va_bufs failed");
+    return 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();
+  DVLOG(4) << "Committed " << num_va_buffers << "VA buffers";
+
+  // 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();
   }
 
   // And send them to the Hw decoder.
-  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, buffers,
-                               num_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);
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, slice_buffers,
+                               num_slice_buffers);
+  if (va_res != VA_STATUS_SUCCESS) {
+    DestroyBuffers(va_buffers, num_va_buffers);
+    DestroyBuffers(slice_buffers, num_slice_buffers);
+    VA_LOG_ERROR(va_res, "vaRenderPicture for slices failed");
+    return 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);
+  VA_LOG_ON_ERROR(va_res, "vaEndPicture failed");
+
+  // Now that EndPicture has passed we can destroy our buffers.
+  DestroyBuffers(va_buffers, num_va_buffers);
+  DestroyBuffers(slice_buffers, num_slice_buffers);
 
   // Used to notify clients that we had sufficient data to start decoding
   // a new frame.