V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.

Review-Url: https://codereview.chromium.org/2335573002
Cr-Commit-Position: refs/heads/master@{#420027}

media/gpu/generic_v4l2_device.cc

@@ -296,7 +296,11 @@ EGLImageKHR GenericV4L2Device::CreateEGLImage(
 
 EGLBoolean GenericV4L2Device::DestroyEGLImage(EGLDisplay egl_display,
                                               EGLImageKHR egl_image) {
-  return eglDestroyImageKHR(egl_display, egl_image);
+  EGLBoolean result = eglDestroyImageKHR(egl_display, egl_image);
+  if (result != EGL_TRUE) {
+    LOG(WARNING) << "Destroy EGLImage failed.";
+  }
+  return result;
 }
 
 GLenum GenericV4L2Device::GetTextureTarget() {

media/gpu/v4l2_video_decode_accelerator.cc

@@ -186,9 +186,6 @@ V4L2VideoDecodeAccelerator::~V4L2VideoDecodeAccelerator() {
   DCHECK(!decoder_thread_.IsRunning());
   DCHECK(!device_poll_thread_.IsRunning());
 
-  DestroyInputBuffers();
-  DestroyOutputBuffers();
-
   // These maps have members that should be manually destroyed, e.g. file
   // descriptors, mmap() segments, etc.
   DCHECK(input_buffer_map_.empty());
@@ -432,8 +429,7 @@ void V4L2VideoDecodeAccelerator::CreateEGLImages(
                   << " index=" << i
                   << " texture_id=" << buffers[i].texture_ids()[0];
       for (EGLImageKHR image : egl_images) {
-        if (egl_image_device_->DestroyEGLImage(egl_display_, image) != EGL_TRUE)
-          DVLOGF(1) << "DestroyEGLImage failed.";
+        egl_image_device_->DestroyEGLImage(egl_display_, image);
       }
       NOTIFY_ERROR(PLATFORM_FAILURE);
       return;
@@ -1576,6 +1572,9 @@ void V4L2VideoDecodeAccelerator::DestroyTask() {
 
   // Set our state to kError.  Just in case.
   decoder_state_ = kError;
+
+  DestroyInputBuffers();
+  DestroyOutputBuffers();
 }
 
 bool V4L2VideoDecodeAccelerator::StartDevicePoll() {
@@ -1689,12 +1688,13 @@ void V4L2VideoDecodeAccelerator::StartResolutionChange() {
   if (image_processor_)
     image_processor_.release()->Destroy();
 
-  // Post a task to clean up buffers on child thread. This will also ensure
-  // that we won't accept ReusePictureBuffer() anymore after that.
-  child_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&V4L2VideoDecodeAccelerator::ResolutionChangeDestroyBuffers,
-                 weak_this_));
+  if (!DestroyOutputBuffers()) {
+    LOGF(ERROR) << "Failed destroying output buffers.";
+    NOTIFY_ERROR(PLATFORM_FAILURE);
+    return;
+  }
+
+  FinishResolutionChange();
 }
 
 void V4L2VideoDecodeAccelerator::FinishResolutionChange() {
@@ -2127,7 +2127,7 @@ bool V4L2VideoDecodeAccelerator::CreateOutputBuffers() {
 
 void V4L2VideoDecodeAccelerator::DestroyInputBuffers() {
   DVLOGF(3);
-  DCHECK(child_task_runner_->BelongsToCurrentThread());
+  DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
   DCHECK(!input_streamon_);
 
   for (size_t i = 0; i < input_buffer_map_.size(); ++i) {
@@ -2150,7 +2150,7 @@ void V4L2VideoDecodeAccelerator::DestroyInputBuffers() {
 
 bool V4L2VideoDecodeAccelerator::DestroyOutputBuffers() {
   DVLOGF(3);
-  DCHECK(child_task_runner_->BelongsToCurrentThread());
+  DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
   DCHECK(!output_streamon_);
   bool success = true;
 
@@ -2158,11 +2158,10 @@ bool V4L2VideoDecodeAccelerator::DestroyOutputBuffers() {
     OutputRecord& output_record = output_buffer_map_[i];
 
     if (output_record.egl_image != EGL_NO_IMAGE_KHR) {
-      if (egl_image_device_->DestroyEGLImage(
-              egl_display_, output_record.egl_image) != EGL_TRUE) {
-        DVLOGF(1) << "DestroyEGLImage failed.";
-        success = false;
-      }
+      child_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(base::IgnoreResult(&V4L2Device::DestroyEGLImage), device_,
+                     egl_display_, output_record.egl_image));
     }
 
     if (output_record.egl_sync != EGL_NO_SYNC_KHR) {
@@ -2199,22 +2198,6 @@ bool V4L2VideoDecodeAccelerator::DestroyOutputBuffers() {
   return success;
 }
 
-void V4L2VideoDecodeAccelerator::ResolutionChangeDestroyBuffers() {
-  DCHECK(child_task_runner_->BelongsToCurrentThread());
-  DVLOGF(3);
-
-  if (!DestroyOutputBuffers()) {
-    LOGF(ERROR) << "Failed destroying output buffers.";
-    NOTIFY_ERROR(PLATFORM_FAILURE);
-    return;
-  }
-
-  // Finish resolution change on decoder thread.
-  decoder_thread_.task_runner()->PostTask(
-      FROM_HERE, base::Bind(&V4L2VideoDecodeAccelerator::FinishResolutionChange,
-                            base::Unretained(this)));
-}
-
 void V4L2VideoDecodeAccelerator::SendPictureReady() {
   DVLOGF(3);
   DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());

media/gpu/v4l2_video_decode_accelerator.h

@@ -133,8 +133,8 @@ class MEDIA_GPU_EXPORT V4L2VideoDecodeAccelerator
     kInitialized,    // Initialize() returned true; ready to start decoding.
     kDecoding,       // DecodeBufferInitial() successful; decoding frames.
     kResetting,      // Presently resetting.
-    // Performing resolution change, all remaining pre-change frames decoded
-    // and processed.
+    // Performing resolution change and waiting for image processor to return
+    // all frames.
     kChangingResolution,
     // Requested new PictureBuffers via ProvidePictureBuffers(), awaiting
     // AssignPictureBuffers().
@@ -329,6 +329,15 @@ class MEDIA_GPU_EXPORT V4L2VideoDecodeAccelerator
   bool CreateInputBuffers();
   bool CreateOutputBuffers();
 
+  // Destroy buffers.
+  void DestroyInputBuffers();
+  // In contrast to DestroyInputBuffers, which is called only on destruction,
+  // we call DestroyOutputBuffers also during playback, on resolution change.
+  // Even if anything fails along the way, we still want to go on and clean
+  // up as much as possible, so return false if this happens, so that the
+  // caller can error out on resolution change.
+  bool DestroyOutputBuffers();
+
   // Set input and output formats before starting decode.
   bool SetupFormats();
   // Return a usable input format of image processor. Return 0 if not found.
@@ -342,16 +351,6 @@ class MEDIA_GPU_EXPORT V4L2VideoDecodeAccelerator
   // Methods run on child thread.
   //
 
-  // Destroy buffers.
-  void DestroyInputBuffers();
-  // In contrast to DestroyInputBuffers, which is called only from destructor,
-  // we call DestroyOutputBuffers also during playback, on resolution change.
-  // Even if anything fails along the way, we still want to go on and clean
-  // up as much as possible, so return false if this happens, so that the
-  // caller can error out on resolution change.
-  bool DestroyOutputBuffers();
-  void ResolutionChangeDestroyBuffers();
-
   // Send decoded pictures to PictureReady.
   void SendPictureReady();