Revert "V4L2SVDA: Move allocation from GPU Child thread to decoder thread."

content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc

Index: content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc b/content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc
index f48fc934813295f35d25886462f464ddcf46aed6..1a967bd5aa4e01b0c0984b7c63acff4c440cba74 100644
--- a/content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc
+++ b/content/common/gpu/media/v4l2_slice_video_decode_accelerator.cc
@@ -406,6 +406,7 @@ V4L2SliceVideoDecodeAccelerator::V4L2SliceVideoDecodeAccelerator(
       surface_set_change_pending_(false),
       picture_clearing_count_(0),
       make_context_current_(make_context_current),
+      pictures_assigned_(false, false),
       egl_display_(egl_display),
       egl_context_(egl_context),
       weak_this_factory_(this) {
@@ -546,6 +547,11 @@ void V4L2SliceVideoDecodeAccelerator::Destroy() {
         FROM_HERE, base::Bind(&V4L2SliceVideoDecodeAccelerator::DestroyTask,
                               base::Unretained(this)));
 
+    // Wake up decoder thread in case we are waiting in CreateOutputBuffers
+    // for client to provide pictures. Since this is Destroy, we won't be
+    // getting them anymore (AssignPictureBuffers won't be called).
+    pictures_assigned_.Signal();
+
     // Wait for tasks to finish/early-exit.
     decoder_thread_.Stop();
   }
@@ -747,6 +753,12 @@ bool V4L2SliceVideoDecodeAccelerator::CreateOutputBuffers() {
                  client_, num_pictures, coded_size_,
                  device_->GetTextureTarget()));
 
+  // Wait for the client to call AssignPictureBuffers() on the Child thread.
+  // We do this, because if we continue decoding without finishing buffer
+  // allocation, we may end up Resetting before AssignPictureBuffers arrives,
+  // resulting in unnecessary complications and subtle bugs.
+  pictures_assigned_.Wait();
+
   return true;
 }
 
@@ -773,7 +785,7 @@ void V4L2SliceVideoDecodeAccelerator::DestroyInputBuffers() {
 }
 
 void V4L2SliceVideoDecodeAccelerator::DismissPictures(
-    const std::vector<int32_t>& picture_buffer_ids,
+    std::vector<int32_t> picture_buffer_ids,
     base::WaitableEvent* done) {
   DVLOGF(3);
   DCHECK(child_task_runner_->BelongsToCurrentThread());
@@ -954,29 +966,11 @@ void V4L2SliceVideoDecodeAccelerator::ProcessPendingEventsIfNeeded() {
   // We always first process the surface set change, as it is an internal
   // event from the decoder and interleaving it with external requests would
   // put the decoder in an undefined state.
-  if (surface_set_change_pending_) {
-    if (!FinishSurfaceSetChange())
-      return;
-  }
-  DCHECK(!surface_set_change_pending_);
+  FinishSurfaceSetChangeIfNeeded();
 
   // Process external (client) requests.
-  if (decoder_flushing_) {
-    if (!FinishFlush())
-      return;
-  }
-  DCHECK(!decoder_flushing_);
-
-  if (decoder_resetting_) {
-    if (!FinishReset())
-      return;
-  }
-  DCHECK(!decoder_resetting_);
-
-  if (state_ == kIdle)
-    state_ = kDecoding;
-
-  ScheduleDecodeBufferTaskIfNeeded();
+  FinishFlushIfNeeded();
+  FinishResetIfNeeded();
 }
 
 void V4L2SliceVideoDecodeAccelerator::ReuseInputBuffer(int index) {
@@ -1303,21 +1297,21 @@ void V4L2SliceVideoDecodeAccelerator::InitiateSurfaceSetChange() {
   DVLOGF(2);
   DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
 
+  DCHECK_EQ(state_, kDecoding);
   state_ = kIdle;
 
   DCHECK(!surface_set_change_pending_);
   surface_set_change_pending_ = true;
 
-  ProcessPendingEventsIfNeeded();
+  FinishSurfaceSetChangeIfNeeded();
 }
 
-bool V4L2SliceVideoDecodeAccelerator::FinishSurfaceSetChange() {
+void V4L2SliceVideoDecodeAccelerator::FinishSurfaceSetChangeIfNeeded() {
   DVLOGF(2);
   DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
 
-  DCHECK(surface_set_change_pending_);
-  if (!surfaces_at_device_.empty())
-    return false;
+  if (!surface_set_change_pending_ || !surfaces_at_device_.empty())
+    return;
 
   DCHECK_EQ(state_, kIdle);
   DCHECK(decoder_display_queue_.empty());
@@ -1330,7 +1324,7 @@ bool V4L2SliceVideoDecodeAccelerator::FinishSurfaceSetChange() {
   // Keep input queue running while we switch outputs.
   if (!StopDevicePoll(true)) {
     NOTIFY_ERROR(PLATFORM_FAILURE);
-    return false;
+    return;
   }
 
   // This will return only once all buffers are dismissed and destroyed.
@@ -1339,21 +1333,24 @@ bool V4L2SliceVideoDecodeAccelerator::FinishSurfaceSetChange() {
   // after displaying.
   if (!DestroyOutputs(true)) {
     NOTIFY_ERROR(PLATFORM_FAILURE);
-    return false;
+    return;
   }
 
   if (!CreateOutputBuffers()) {
     NOTIFY_ERROR(PLATFORM_FAILURE);
-    return false;
+    return;
   }
 
-  // At this point we can safely say the surface set has been changed, even
-  // though we haven't received the actual buffers via AssignPictureBuffers()
-  // yet. We will not start decoding without having surfaces available,
-  // and will schedule a decode task once the client provides the buffers.
+  if (!StartDevicePoll()) {
+    NOTIFY_ERROR(PLATFORM_FAILURE);
+    return;
+  }
+
+  DVLOGF(3) << "Surface set change finished";
+
   surface_set_change_pending_ = false;
-  DVLOG(3) << "Surface set change finished";
-  return true;
+  state_ = kDecoding;
+  ScheduleDecodeBufferTaskIfNeeded();
 }
 
 bool V4L2SliceVideoDecodeAccelerator::DestroyOutputs(bool dismiss) {
@@ -1443,18 +1440,6 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffers(
   DVLOGF(3);
   DCHECK(child_task_runner_->BelongsToCurrentThread());
 
-  decoder_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(
-          &V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask,
-          base::Unretained(this), buffers));
-}
-
-void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
-    const std::vector<media::PictureBuffer>& buffers) {
-  DVLOGF(3);
-  DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, kDecoding);
-
   const uint32_t req_buffer_count = decoder_->GetRequiredNumOfPictures();
 
   if (buffers.size() < req_buffer_count) {
@@ -1465,6 +1450,17 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
     return;
   }
 
+  if (!make_context_current_.Run()) {
+    DLOG(ERROR) << "could not make context current";
+    NOTIFY_ERROR(PLATFORM_FAILURE);
+    return;
+  }
+
+  gfx::ScopedTextureBinder bind_restore(GL_TEXTURE_EXTERNAL_OES, 0);
+
+  // It's safe to manipulate all the buffer state here, because the decoder
+  // thread is waiting on pictures_assigned_.
+
   // Allocate the output buffers.
   struct v4l2_requestbuffers reqbufs;
   memset(&reqbufs, 0, sizeof(reqbufs));
@@ -1479,66 +1475,9 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
     return;
   }
 
-  child_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&V4L2SliceVideoDecodeAccelerator::CreateEGLImages,
-                            weak_this_, buffers, output_format_fourcc_,
-                            output_planes_count_));
-}
-
-void V4L2SliceVideoDecodeAccelerator::CreateEGLImages(
-    const std::vector<media::PictureBuffer>& buffers,
-    uint32_t output_format_fourcc,
-    size_t output_planes_count) {
-  DVLOGF(3);
-  DCHECK(child_task_runner_->BelongsToCurrentThread());
-
-  if (!make_context_current_.Run()) {
-    DLOG(ERROR) << "could not make context current";
-    NOTIFY_ERROR(PLATFORM_FAILURE);
-    return;
-  }
-
-  gfx::ScopedTextureBinder bind_restore(GL_TEXTURE_EXTERNAL_OES, 0);
-
-  std::vector<EGLImageKHR> egl_images;
-  for (size_t i = 0; i < buffers.size(); ++i) {
-    EGLImageKHR egl_image = device_->CreateEGLImage(egl_display_,
-                                                    egl_context_,
-                                                    buffers[i].texture_id(),
-                                                    buffers[i].size(),
-                                                    i,
-                                                    output_format_fourcc,
-                                                    output_planes_count);
-    if (egl_image == EGL_NO_IMAGE_KHR) {
-      LOGF(ERROR) << "Could not create EGLImageKHR";
-      for (const auto& image_to_destroy : egl_images)
-        device_->DestroyEGLImage(egl_display_, image_to_destroy);
-
-      NOTIFY_ERROR(PLATFORM_FAILURE);
-      return;
-    }
-
-    egl_images.push_back(egl_image);
-  }
-
-  decoder_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(
-          &V4L2SliceVideoDecodeAccelerator::AssignEGLImages,
-          base::Unretained(this), buffers, egl_images));
-}
-
-void V4L2SliceVideoDecodeAccelerator::AssignEGLImages(
-    const std::vector<media::PictureBuffer>& buffers,
-    const std::vector<EGLImageKHR>& egl_images) {
-  DVLOGF(3);
-  DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(buffers.size(), egl_images.size());
-
-  DCHECK(free_output_buffers_.empty());
-  DCHECK(output_buffer_map_.empty());
-
   output_buffer_map_.resize(buffers.size());
 
+  DCHECK(free_output_buffers_.empty());
   for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
     DCHECK(buffers[i].size() == coded_size_);
 
@@ -1550,18 +1489,25 @@ void V4L2SliceVideoDecodeAccelerator::AssignEGLImages(
     DCHECK_EQ(output_record.picture_id, -1);
     DCHECK_EQ(output_record.cleared, false);
 
-    output_record.egl_image = egl_images[i];
+    EGLImageKHR egl_image = device_->CreateEGLImage(
+        egl_display_, egl_context_, buffers[i].texture_id(),
+        buffers[i].size(), i, output_format_fourcc_, output_planes_count_);
+    if (egl_image == EGL_NO_IMAGE_KHR) {
+      LOGF(ERROR) << "Could not create EGLImageKHR";
+      // Ownership of EGLImages allocated in previous iterations of this loop
+      // has been transferred to output_buffer_map_. After we error-out here
+      // the destructor will handle their cleanup.
+      NOTIFY_ERROR(PLATFORM_FAILURE);
+      return;
+    }
+
+    output_record.egl_image = egl_image;
     output_record.picture_id = buffers[i].id();
     free_output_buffers_.push_back(i);
     DVLOGF(3) << "buffer[" << i << "]: picture_id=" << output_record.picture_id;
   }
 
-  if (!StartDevicePoll()) {
-    NOTIFY_ERROR(PLATFORM_FAILURE);
-    return;
-  }
-
-  ProcessPendingEventsIfNeeded();
+  pictures_assigned_.Signal();
 }
 
 void V4L2SliceVideoDecodeAccelerator::ReusePictureBuffer(
@@ -1675,17 +1621,18 @@ void V4L2SliceVideoDecodeAccelerator::InitiateFlush() {
 
   decoder_flushing_ = true;
 
-  ProcessPendingEventsIfNeeded();
+  decoder_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&V4L2SliceVideoDecodeAccelerator::FinishFlushIfNeeded,
+                 base::Unretained(this)));
 }
 
-bool V4L2SliceVideoDecodeAccelerator::FinishFlush() {
+void V4L2SliceVideoDecodeAccelerator::FinishFlushIfNeeded() {
   DVLOGF(3);
   DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
 
-  DCHECK(decoder_flushing_);
-
-  if (!surfaces_at_device_.empty())
-    return false;
+  if (!decoder_flushing_ || !surfaces_at_device_.empty())
+    return;
 
   DCHECK_EQ(state_, kIdle);
 
@@ -1703,13 +1650,14 @@ bool V4L2SliceVideoDecodeAccelerator::FinishFlush() {
 
   SendPictureReady();
 
-  decoder_flushing_ = false;
-  DVLOGF(3) << "Flush finished";
-
   child_task_runner_->PostTask(FROM_HERE,
                                base::Bind(&Client::NotifyFlushDone, client_));
 
-  return true;
+  decoder_flushing_ = false;
+
+  DVLOGF(3) << "Flush finished";
+  state_ = kDecoding;
+  ScheduleDecodeBufferTaskIfNeeded();
 }
 
 void V4L2SliceVideoDecodeAccelerator::Reset() {
@@ -1745,16 +1693,15 @@ void V4L2SliceVideoDecodeAccelerator::ResetTask() {
   while (!decoder_input_queue_.empty())
     decoder_input_queue_.pop();
 
-  ProcessPendingEventsIfNeeded();
+  FinishResetIfNeeded();
 }
 
-bool V4L2SliceVideoDecodeAccelerator::FinishReset() {
+void V4L2SliceVideoDecodeAccelerator::FinishResetIfNeeded() {
   DVLOGF(3);
   DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
 
-  DCHECK(decoder_resetting_);
-  if (!surfaces_at_device_.empty())
-    return false;
+  if (!decoder_resetting_ || !surfaces_at_device_.empty())
+    return;
 
   DCHECK_EQ(state_, kIdle);
   DCHECK(!decoder_flushing_);
@@ -1778,12 +1725,14 @@ bool V4L2SliceVideoDecodeAccelerator::FinishReset() {
   }
 
   decoder_resetting_ = false;
-  DVLOGF(3) << "Reset finished";
 
   child_task_runner_->PostTask(FROM_HERE,
                                base::Bind(&Client::NotifyResetDone, client_));
 
-  return true;
+  DVLOGF(3) << "Reset finished";
+
+  state_ = kDecoding;
+  ScheduleDecodeBufferTaskIfNeeded();
 }
 
 void V4L2SliceVideoDecodeAccelerator::SetErrorState(Error error) {