Always call OMX_FreeBuffer(), even in ERRORING.

content/common/gpu/media/omx_video_decode_accelerator.cc

Index: content/common/gpu/media/omx_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/omx_video_decode_accelerator.cc b/content/common/gpu/media/omx_video_decode_accelerator.cc
index 1d3be0193438392f5fbf3497dcc8f8dfea335212..a991d3fac559d9b9087735b40d0737209d933727 100644
--- a/content/common/gpu/media/omx_video_decode_accelerator.cc
+++ b/content/common/gpu/media/omx_video_decode_accelerator.cc
@@ -605,12 +605,7 @@ void OmxVideoDecodeAccelerator::OnReachedIdleInDestroying() {
 
   BeginTransitionToState(OMX_StateLoaded);
 
-  // TODO(fischman): evaluate what these conditionals are doing.  What happens
-  // if they're false??
-  if (!input_buffers_at_component_)
-    FreeInputBuffers();
-  if (!output_buffers_at_component_)
-    FreeOutputBuffers();
+  FreeOMXBuffers();
 }
 
 void OmxVideoDecodeAccelerator::OnReachedLoadedInDestroying() {
@@ -622,6 +617,7 @@ void OmxVideoDecodeAccelerator::OnReachedLoadedInDestroying() {
 
 void OmxVideoDecodeAccelerator::OnReachedInvalidInErroring() {
   client_state_ = OMX_StateInvalid;
+  FreeOMXBuffers();
   ShutdownComponent();
 }
 
@@ -715,30 +711,22 @@ bool OmxVideoDecodeAccelerator::AllocateOutputBuffers() {
   return true;
 }
 
-void OmxVideoDecodeAccelerator::FreeInputBuffers() {
+void OmxVideoDecodeAccelerator::FreeOMXBuffers() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
-  // Calls to OMX to free buffers.
-  OMX_ERRORTYPE result;
-  OMX_BUFFERHEADERTYPE* omx_buffer;
   while (!free_input_buffers_.empty()) {
-    omx_buffer = free_input_buffers_.front();
+    OMX_BUFFERHEADERTYPE* omx_buffer = free_input_buffers_.front();
     free_input_buffers_.pop();
-    result = OMX_FreeBuffer(component_handle_, input_port_, omx_buffer);
+    OMX_ERRORTYPE result =
+        OMX_FreeBuffer(component_handle_, input_port_, omx_buffer);
     RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer", PLATFORM_FAILURE,);
   }
-}
-
-void OmxVideoDecodeAccelerator::FreeOutputBuffers() {
-  DCHECK_EQ(message_loop_, MessageLoop::current());
-  // Calls to OMX to free buffers.
-  OMX_ERRORTYPE result;
-
   for (OutputPictureById::iterator it = pictures_.begin();
        it != pictures_.end(); ++it) {
     OMX_BUFFERHEADERTYPE* omx_buffer = it->second.omx_buffer_header;
     DCHECK(omx_buffer);
     delete reinterpret_cast<media::Picture*>(omx_buffer->pAppPrivate);
-    result = OMX_FreeBuffer(component_handle_, output_port_, omx_buffer);
+    OMX_ERRORTYPE result =
+        OMX_FreeBuffer(component_handle_, output_port_, omx_buffer);
     RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer", PLATFORM_FAILURE,);

[marcheu, 2012/08/14 22:11:25]

Can we keep iterating even if we fail a FreeBuffer call? That way we can still
try to free the other pictures, and destroy the eglImages.

[Ami GONE FROM CHROMIUM, 2012/08/15 04:08:30]

I really dislike this sort of programming.
I guess you're going for belt&suspenders here, but I don't like writing code
which "should" never get run.  Do you know of any concrete ways in which an
OMX_FreeBuffer call can fail and yet have further OMX_FB calls be useful, or the
DestroyEglImage below be useful?

At any rate, done.

     texture_to_egl_image_translator_->DestroyEglImage(egl_display_,
                                                       it->second.egl_image);