V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

media/gpu/v4l2_video_decode_accelerator.cc

 // Copyright 2014 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 "media/gpu/v4l2_video_decode_accelerator.h"
 
 #include <dlfcn.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/videodev2.h>
@@ skipping 168 matching lines @@
       egl_image_format_fourcc_(0),
       egl_image_planes_count_(0),
       weak_this_factory_(this) {
   weak_this_ = weak_this_factory_.GetWeakPtr();
 }
 
 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());
   DCHECK(output_buffer_map_.empty());
 }
 
 bool V4L2VideoDecodeAccelerator::Initialize(const Config& config,
                                             Client* client) {
   DVLOGF(3) << "profile: " << config.profile;
   DCHECK(child_task_runner_->BelongsToCurrentThread());
@@ skipping 1367 matching lines @@
   decoder_frames_at_client_ = 0;
   while (!decoder_input_queue_.empty())
     decoder_input_queue_.pop();
   decoder_flushing_ = false;
 
   if (image_processor_)
     image_processor_.release()->Destroy();
 
   // Set our state to kError.  Just in case.
   decoder_state_ = kError;
+
+  DestroyInputBuffers();
+  DestroyOutputBuffers();
 }
 
 bool V4L2VideoDecodeAccelerator::StartDevicePoll() {
   DVLOGF(3);
   DCHECK(!device_poll_thread_.IsRunning());
   DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
 
   // Start up the device poll thread and schedule its first DevicePollTask().
   if (!device_poll_thread_.Start()) {
     LOGF(ERROR) << "Device thread failed to start";
@@ skipping 93 matching lines @@
   decoder_state_ = kChangingResolution;
 
   if (!image_processor_bitstream_buffer_ids_.empty()) {
     DVLOGF(3) << "Wait image processor to finish before destroying buffers.";
     return;
   }
 
   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() {
   DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
   DCHECK_EQ(decoder_state_, kChangingResolution);
   DVLOGF(3);
 
   if (decoder_state_ == kError) {
     DVLOGF(2) << "early out: kError state";
     return;
@@ skipping 409 matching lines @@
   // done with this one. After we get the buffers, we'll go back into kIdle and
   // kick off further event processing, and eventually go back into kDecoding
   // once no more events are pending (if any).
   decoder_state_ = kAwaitingPictureBuffers;
 
   return true;
 }
 
 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) {
     if (input_buffer_map_[i].address != NULL) {
       device_->Munmap(input_buffer_map_[i].address,
                       input_buffer_map_[i].length);
     }
   }
 
   struct v4l2_requestbuffers reqbufs;
   memset(&reqbufs, 0, sizeof(reqbufs));
   reqbufs.count = 0;
   reqbufs.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
   reqbufs.memory = V4L2_MEMORY_MMAP;
   IOCTL_OR_LOG_ERROR(VIDIOC_REQBUFS, &reqbufs);
 
   input_buffer_map_.clear();
   free_input_buffers_.clear();
 }
 
 bool V4L2VideoDecodeAccelerator::DestroyOutputBuffers() {
   DVLOGF(3);
-  DCHECK(child_task_runner_->BelongsToCurrentThread());
+  DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
   DCHECK(!output_streamon_);
   bool success = true;
 
   for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
     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_,

[kcwu, 2016/09/12 10:59:48]

Add log if destroy failed at least.

[wuchengli, 2016/09/12 11:15:36]

V4L2SVDA has three base::IgnoreResult(&V4L2Device::DestroyEGLImage). I'll add in
GenericV4L2Device::DestroyEGLImage and see what Pawel thinks.

+                     egl_display_, output_record.egl_image));
     }
 
     if (output_record.egl_sync != EGL_NO_SYNC_KHR) {
       if (eglDestroySyncKHR(egl_display_, output_record.egl_sync) != EGL_TRUE) {

[kcwu, 2016/09/12 10:59:48]

Is it okay to call eglDestroySyncKHR on non-child thread?

[wuchengli, 2016/09/12 11:15:36]

Yes. It's OK. Also see line 1296. eglDestroySyncKHR is called on decode thread.

         DVLOGF(1) << "eglDestroySyncKHR failed.";
         success = false;
       }
     }
 
     DVLOGF(1) << "dismissing PictureBuffer id=" << output_record.picture_id;
     child_task_runner_->PostTask(
         FROM_HERE, base::Bind(&Client::DismissPictureBuffer, client_,
                               output_record.picture_id));
   }
@@ skipping 12 matching lines @@
   while (!free_output_buffers_.empty())
     free_output_buffers_.pop();
   output_buffer_queued_count_ = 0;
   // The client may still hold some buffers. The texture holds a reference to
   // the buffer. It is OK to free the buffer and destroy EGLImage here.
   decoder_frames_at_client_ = 0;
 
   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());
   bool resetting_or_flushing =
       (decoder_state_ == kResetting || decoder_flushing_);
   while (pending_picture_ready_.size() > 0) {
     bool cleared = pending_picture_ready_.front().cleared;
     const Picture& picture = pending_picture_ready_.front().picture;
     if (cleared && picture_clearing_count_ == 0) {
       // This picture is cleared. It can be posted to a thread different than
@@ skipping 69 matching lines @@
       StartResolutionChange();
   }
 }
 
 void V4L2VideoDecodeAccelerator::ImageProcessorError() {
   LOGF(ERROR) << "Image processor error";
   NOTIFY_ERROR(PLATFORM_FAILURE);
 }
 
 }  // namespace media