media: GpuMemoryBufferVideoFramePool uses BindToCurrentLoop()

media/video/gpu_memory_buffer_video_frame_pool.cc

 // Copyright 2015 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/video/gpu_memory_buffer_video_frame_pool.h"
 
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <list>
 #include <utility>
 
 #include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/containers/stack_container.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/linked_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/trace_event.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/renderers/gpu_video_accelerator_factories.h"
 #include "third_party/libyuv/include/libyuv.h"
 #include "ui/gfx/buffer_format_util.h"
 #include "ui/gl/trace_util.h"
 
 namespace media {
 
 // Implementation of a pool of GpuMemoryBuffers used to back VideoFrames.
 class GpuMemoryBufferVideoFramePool::PoolImpl
     : public base::RefCountedThreadSafe<
@@ skipping 95 matching lines @@
   }
 
   // Get the resources needed for a frame out of the pool, or create them if
   // necessary.
   // This also drops the LRU resources that can't be reuse for this frame.
   FrameResources* GetOrCreateFrameResources(const gfx::Size& size,
                                             VideoPixelFormat format);
 
   // Callback called when a VideoFrame generated with GetFrameResources is no
   // longer referenced.
-  // This could be called by any thread.
+  // This could be called on the thread where |media_task_runner_| is current.

[Daniele Castagna, 2016/03/21 18:45:19]

nit: s/could/must.

[dshwang, 2016/03/21 19:04:38]

Done.

   void MailboxHoldersReleased(FrameResources* frame_resources,
                               const gpu::SyncToken& sync_token);
 
-  // Return frame resources to the pool. This has to be called on the thread
-  // where |media_task_runner_| is current.
-  void ReturnFrameResources(FrameResources* frame_resources);
-
   // Delete resources. This has to be called on the thread where |task_runner|
   // is current.
   static void DeleteFrameResources(GpuVideoAcceleratorFactories* gpu_factories,
                                    FrameResources* frame_resources);
 
   // Task runner associated to the GL context provided by |gpu_factories_|.
   scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
   // Task runner used to asynchronously copy planes.
   scoped_refptr<base::TaskRunner> worker_task_runner_;
 
@@ skipping 422 matching lines @@
   const GLuint64 fence_sync = gles2->InsertFenceSyncCHROMIUM();
   gles2->OrderingBarrierCHROMIUM();
 
   gpu::SyncToken sync_token;
   gles2->GenUnverifiedSyncTokenCHROMIUM(fence_sync, sync_token.GetData());
   for (size_t i = 0; i < num_planes; i += planes_per_copy)
     mailbox_holders[i].sync_token = sync_token;
 
   scoped_refptr<VideoFrame> frame;
 
-  auto release_mailbox_callback =
-      base::Bind(&PoolImpl::MailboxHoldersReleased, this, frame_resources);
+  auto release_mailbox_callback = BindToCurrentLoop(
+      base::Bind(&PoolImpl::MailboxHoldersReleased, this, frame_resources));

[dshwang, 2016/03/21 18:31:23]

No, this patch doesn't have a race, because I use BindToCurrentLoop().
BindToCurrentLoop() is helper function to avoid two redundant callback. :)

[Daniele Castagna, 2016/03/21 18:45:19]

Ooh, sorry, I missed the BindToCurrentLoop. This is definitely better than the
helper function.
Do you mind mentioning it in the CL description?

[dshwang, 2016/03/21 19:04:38]

Done.

 
   // Create the VideoFrame backed by native textures.
   gfx::Size visible_size = video_frame->visible_rect().size();
   switch (output_format_) {
     case PIXEL_FORMAT_I420:
       frame = VideoFrame::WrapYUV420NativeTextures(
           mailbox_holders[VideoFrame::kYPlane],
           mailbox_holders[VideoFrame::kUPlane],
           mailbox_holders[VideoFrame::kVPlane], release_mailbox_callback,
           coded_size, gfx::Rect(visible_size), video_frame->natural_size(),
@@ skipping 120 matching lines @@
   gpu::gles2::GLES2Interface* gles2 = lock->ContextGL();
 
   for (PlaneResource& plane_resource : frame_resources->plane_resources) {
     if (plane_resource.image_id)
       gles2->DestroyImageCHROMIUM(plane_resource.image_id);
     if (plane_resource.texture_id)
       gles2->DeleteTextures(1, &plane_resource.texture_id);
   }
 }
 
 // Called when a VideoFrame is no longer references.

[Daniele Castagna, 2016/03/21 18:45:19]

nit: s/references/referenced.

[dshwang, 2016/03/21 19:04:38]

Done.

+// Put back the resources in the pool.
 void GpuMemoryBufferVideoFramePool::PoolImpl::MailboxHoldersReleased(
     FrameResources* frame_resources,
-    const gpu::SyncToken& sync_token) {
-  // Return the resource on the media thread.
-  media_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&PoolImpl::ReturnFrameResources, this, frame_resources));
-}
-
-// Put back the resources in the pool.
-void GpuMemoryBufferVideoFramePool::PoolImpl::ReturnFrameResources(
-    FrameResources* frame_resources) {
+    const gpu::SyncToken& release_sync_token) {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
   auto it = std::find(resources_pool_.begin(), resources_pool_.end(),
                       frame_resources);
   DCHECK(it != resources_pool_.end());
   // We want the pool to behave in a FIFO way.
   // This minimizes the chances of locking the buffer that might be
   // still needed for drawing.
   std::swap(*it, resources_pool_.back());
+
+  gpu_factories_->WaitSyncToken(release_sync_token);
   frame_resources->SetIsInUse(false);
 }
 
 GpuMemoryBufferVideoFramePool::GpuMemoryBufferVideoFramePool() {}
 
 GpuMemoryBufferVideoFramePool::GpuMemoryBufferVideoFramePool(
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
     const scoped_refptr<base::TaskRunner>& worker_task_runner,
     GpuVideoAcceleratorFactories* gpu_factories)
     : pool_impl_(
           new PoolImpl(media_task_runner, worker_task_runner, gpu_factories)) {
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       pool_impl_.get(), "GpuMemoryBufferVideoFramePool", media_task_runner);
 }
 
 GpuMemoryBufferVideoFramePool::~GpuMemoryBufferVideoFramePool() {
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       pool_impl_.get());
 }
 
 void GpuMemoryBufferVideoFramePool::MaybeCreateHardwareFrame(
     const scoped_refptr<VideoFrame>& video_frame,
     const FrameReadyCB& frame_ready_cb) {
   DCHECK(video_frame);
   pool_impl_->CreateHardwareFrame(video_frame, frame_ready_cb);
 }
 
 }  // namespace media