Add a GpuMemoryBuffer pool that creates hardware backed VideoFrames.

media/video/gpu_memory_buffer_video_frame_pool.cc

+// Copyright (c) 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 <list>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/containers/stack_container.h"
+#include "base/location.h"
+#include "base/memory/linked_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/trace_event/trace_event.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "media/renderers/gpu_video_accelerator_factories.h"
+
+namespace media {
+
+// Implementation of a pool of GpuMemoryBuffers used to back VideoFrames.
+class GpuMemoryBufferVideoFramePool::PoolImpl
+    : public base::RefCountedThreadSafe<
+          GpuMemoryBufferVideoFramePool::PoolImpl> {
+ public:
+  // 'task_runner' is associated to the thread where the context of

[DaleCurtis, 2015/05/14 00:58:58]

Style is || instead of '' for designating variables.

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+  // GLES2Interface returned by 'gpu_factories' lives.
+  // 'gpu_factories' is an interface to GPU related operation and can be
+  // null.
+  PoolImpl(const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+           const scoped_refptr<GpuVideoAcceleratorFactories>& gpu_factories)
+      : task_runner_(task_runner), gpu_factories_(gpu_factories) {}
+
+  // Takes a software VideoFrame and returns a VideoFrame backed by native
+  // textures if possible.
+  // The data contained in video_frame is copied into the returned frame.
+  const scoped_refptr<VideoFrame> CreateHardwareFrame(

[DaleCurtis, 2015/05/14 00:58:57]

Remove const

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+      scoped_refptr<VideoFrame> video_frame);

[DaleCurtis, 2015/05/14 00:58:58]

const&

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+
+ private:
+  friend class base::RefCountedThreadSafe<
+      GpuMemoryBufferVideoFramePool::PoolImpl>;
+  ~PoolImpl();
+
+  // Resource to represent a plane.
+  struct PlaneResource {
+    gfx::Size size;
+    scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer;
+    unsigned texture_id = 0u;
+    unsigned image_id = 0u;
+    gpu::Mailbox mailbox;
+  };
+
+  // All the resources needed to compose a frame.
+  struct FrameResources {
+    FrameResources(VideoFrame::Format format, const gfx::Size& size)
+        : format(format), size(size) {}
+    bool in_use = true;
+    VideoFrame::Format format;
+    gfx::Size size;
+    PlaneResource plane_resources[VideoFrame::kMaxPlanes];
+  };
+
+  // Return true if 'resources' can be used to represent a frame for
+  // specific 'format' and 'size'.
+  static bool IsFrameResourcesCompatible(const FrameResources* resources,
+                                         const gfx::Size& size,
+                                         VideoFrame::Format format) {
+    return size == resources->size && format == resources->format;
+  }
+
+  // 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,
+                                            VideoFrame::Format format);
+
+  // Callback called when a VideoFrame generated with GetFrameResources is no
+  // longer referenced.
+  // This could be called by any thread.
+  void MailboxHoldersReleased(FrameResources* frame_resources,
+                              uint32 sync_point);
+
+  // Return frame resources to the pool. This has to be called on the thread
+  // where '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(
+      const scoped_refptr<GpuVideoAcceleratorFactories>& gpu_factories,
+      FrameResources* frame_resources);
+
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  scoped_refptr<GpuVideoAcceleratorFactories> gpu_factories_;
+
+  // Pool of resources.
+  std::list<FrameResources*> resources_pool_;
+
+  unsigned texture_target_ = GL_TEXTURE_2D;
+  DISALLOW_COPY_AND_ASSIGN(PoolImpl);
+};
+
+namespace {
+
+// Copy a buffer info a GpuMemoryBuffer.
+// bytes_per_row is expected to be less or equal than the strides of the two

[DaleCurtis, 2015/05/14 00:58:57]

bytes_per_row and all other variables in comments, should have || around them. 
Please find all instances in this file and correct.

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+// buffers.
+void CopyPlaneToGpuMemoryBuffer(int rows,
+                                int bytes_per_row,
+                                const uint8* source,
+                                int source_stride,
+                                gfx::GpuMemoryBuffer* buffer) {
+  TRACE_EVENT2("media", "CopyPlaneToGpuMemoryBuffer", "bytes_per_row",
+               bytes_per_row, "rows", rows);
+
+  DCHECK(buffer);
+  DCHECK(source);
+  void* data = nullptr;
+  CHECK(buffer->Map(&data));
+  uint8* mapped_buffer = static_cast<uint8*>(data);
+  int dest_stride = 0;
+  buffer->GetStride(&dest_stride);
+  DCHECK_NE(dest_stride, 0);
+  DCHECK_LE(bytes_per_row, std::abs(dest_stride));
+  DCHECK_LE(bytes_per_row, source_stride);
+  for (int row = 0; row < rows; ++row) {
+    memcpy(mapped_buffer + dest_stride * row, source + source_stride * row,
+           bytes_per_row);
+  }
+  buffer->Unmap();
+}
+
+}  // unnamed namespace
+
+// Creates a VideoFrame backed by native textures starting from a software

[DaleCurtis, 2015/05/14 00:58:58]

Again, strongly prefer if the comment was reflowed into the function body.

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+// VideoFrame.
+// The data contained in video_frame is copied into the returned VideoFrame.
+// This method first tries to reuse resources and drops the incompatible ones.
+// If resources are not available, they will be created.
+// The returned VideoFrame will call
+// GpuMemoryBufferVideoFramePool::PoolImpl::MailboxHoldersReleased once the
+// VideoFrame is not referenced anymore and the resources will be put back
+// in the pool.
+const scoped_refptr<VideoFrame>
+GpuMemoryBufferVideoFramePool::PoolImpl::CreateHardwareFrame(

[DaleCurtis, 2015/05/14 00:58:57]

Fix const, const& stuff

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+    scoped_refptr<VideoFrame> video_frame) {
+  if (!gpu_factories_)
+    return video_frame;
+
+  gpu::gles2::GLES2Interface* gles2 = gpu_factories_->GetGLES2Interface();
+  if (!gles2)
+    return video_frame;
+
+  VideoFrame::Format format = video_frame->format();
+  size_t planes = VideoFrame::NumPlanes(format);
+  DCHECK(video_frame->visible_rect().origin().IsOrigin());
+  gfx::Size size = video_frame->visible_rect().size();
+  gpu::MailboxHolder mailbox_holders[VideoFrame::kMaxPlanes];
+  FrameResources* frame_resources = GetOrCreateFrameResources(size, format);
+
+  for (size_t i = 0; i < planes; ++i) {
+    PlaneResource& plane_resource = frame_resources->plane_resources[i];
+    CopyPlaneToGpuMemoryBuffer(VideoFrame::Rows(i, format, size.height()),
+                               VideoFrame::RowBytes(i, format, size.width()),
+                               video_frame->data(i), video_frame->stride(i),
+                               plane_resource.gpu_memory_buffer.get());
+
+    // Bind the texture and create or rebind the image.
+    gles2->BindTexture(texture_target_, plane_resource.texture_id);
+    if (plane_resource.gpu_memory_buffer && !plane_resource.image_id) {
+      const size_t width = VideoFrame::Columns(i, format, size.width());
+      const size_t height = VideoFrame::Rows(i, format, size.height());
+      plane_resource.image_id = gles2->CreateImageCHROMIUM(
+          plane_resource.gpu_memory_buffer->AsClientBuffer(), width, height,
+          GL_R8_EXT);
+    } else {
+      gles2->ReleaseTexImage2DCHROMIUM(texture_target_,
+                                       plane_resource.image_id);
+    }
+    gles2->BindTexImage2DCHROMIUM(texture_target_, plane_resource.image_id);
+    mailbox_holders[i] =
+        gpu::MailboxHolder(plane_resource.mailbox, texture_target_, 0);
+  }
+
+  unsigned sync_point = gles2->InsertSyncPointCHROMIUM();
+  for (size_t i = 0; i < planes; ++i) {
+    mailbox_holders[i].sync_point = sync_point;
+  }
+
+  return VideoFrame::WrapYUV420NativeTextures(
+      mailbox_holders[VideoFrame::kYPlane],
+      mailbox_holders[VideoFrame::kUPlane],
+      mailbox_holders[VideoFrame::kVPlane],
+      base::Bind(&PoolImpl::MailboxHoldersReleased, this, frame_resources),
+      size, video_frame->visible_rect(), video_frame->natural_size(),
+      video_frame->timestamp(), video_frame->allow_overlay());
+}
+
+// Destroy all the resources posting one task per FrameResources
+// to the 'task_runner_'.
+GpuMemoryBufferVideoFramePool::PoolImpl::~PoolImpl() {
+  // Delete all the resources on the media thread.
+  while (!resources_pool_.empty()) {
+    FrameResources* frame_resources = resources_pool_.front();
+    resources_pool_.pop_front();
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&PoolImpl::DeleteFrameResources, gpu_factories_,
+                              base::Owned(frame_resources)));
+  }
+}
+
+// Tries to find the resources in the pool or create them.
+// Incompatible resources will be dropped.
+GpuMemoryBufferVideoFramePool::PoolImpl::FrameResources*

[DaleCurtis, 2015/05/14 00:58:58]

Why not scoped_ptr?

[Daniele Castagna, 2015/05/14 17:30:26]

As per reveman's suggestion we decided to leave FrameResources ownership in the
pool and just mark the object as in_use.
Getting a FrameResouces gives you just a pointer to FrameResources and doesn't
transfer the ownership.

This will make it easier to account memory usage of the pool and to deal with
context lost situations once we address that in media.

[DaleCurtis, 2015/05/14 18:52:13]

I'm not sure about that logic, but up to you :) Someone calling this in the
future may forget.

[Daniele Castagna, 2015/05/14 19:24:16]

I agree that it can be confusing, at least this method is private to PoolImpl...

How would you change it? scoped_refptrs everywhere?

[DaleCurtis, 2015/05/14 20:04:49]

I was expected that resources_pool_ would be a ScopedVector, it's fine to return
a raw pointer here though.  Vector is fine if you use std::remove_if to swap
elements to the back for removal to avoid erases at places other than the end.

+GpuMemoryBufferVideoFramePool::PoolImpl::GetOrCreateFrameResources(
+    const gfx::Size& size,
+    VideoFrame::Format format) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  auto it = resources_pool_.begin();
+  while (it != resources_pool_.end()) {
+    FrameResources* frame_resources = *it;
+    if (!frame_resources->in_use) {
+      if (IsFrameResourcesCompatible(frame_resources, size, format)) {
+        frame_resources->in_use = true;
+        return frame_resources;
+      } else {
+        resources_pool_.erase(it++);
+        DeleteFrameResources(gpu_factories_, frame_resources);
+        delete frame_resources;
+      }
+    } else {
+      it++;
+    }
+  }
+
+  // Create the resources.
+  gpu::gles2::GLES2Interface* gles2 = gpu_factories_->GetGLES2Interface();
+  DCHECK(gles2);
+  gles2->ActiveTexture(GL_TEXTURE0);
+  size_t planes = VideoFrame::NumPlanes(format);
+  FrameResources* frame_resources = new FrameResources(format, size);
+  resources_pool_.push_back(frame_resources);
+  for (size_t i = 0; i < planes; ++i) {

[DaleCurtis, 2015/05/14 00:58:57]

Don't need i it looks like, so why not for (const auto& plane_resource :
frame_resources->plane_resources)

[Daniele Castagna, 2015/05/14 17:30:26]

|planes| might be less than arraysize(frame_resources->plane_resources);

+    PlaneResource& plane_resource = frame_resources->plane_resources[i];
+    const size_t width = VideoFrame::Columns(i, format, size.width());
+    const size_t height = VideoFrame::Rows(i, format, size.height());
+    const gfx::Size plane_size(width, height);
+    plane_resource.gpu_memory_buffer = gpu_factories_->AllocateGpuMemoryBuffer(
+        plane_size, gfx::GpuMemoryBuffer::R_8, gfx::GpuMemoryBuffer::MAP);
+
+    gles2->GenTextures(1, &plane_resource.texture_id);
+    gles2->BindTexture(texture_target_, plane_resource.texture_id);
+    gles2->TexParameteri(texture_target_, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    gles2->TexParameteri(texture_target_, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    gles2->TexParameteri(texture_target_, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    gles2->TexParameteri(texture_target_, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    gles2->GenMailboxCHROMIUM(plane_resource.mailbox.name);
+    gles2->ProduceTextureCHROMIUM(texture_target_, plane_resource.mailbox.name);
+  }
+  return frame_resources;
+}
+
+// static
+void GpuMemoryBufferVideoFramePool::PoolImpl::DeleteFrameResources(
+    const scoped_refptr<GpuVideoAcceleratorFactories>& gpu_factories,
+    FrameResources* frame_resources) {
+  // TODO(dcastagna): As soon as the context lost is dealt with in media,

[DaleCurtis, 2015/05/14 00:58:57]

Seems pretty important to fix sooner than later :)

[Daniele Castagna, 2015/05/14 17:30:26]

Absolutely. I talked with posciak@ about this and discovered that a gpu process
restart with hardware decoding would result in broken videos. :(

I think we should deal with the context lost for media hardware decoding first,
and then this pool could follow.

[DaleCurtis, 2015/05/14 18:52:13]

Depends on the failure mode, if it dangles use-after-frees then it has to be
fixed before we can turn this on.

[Daniele Castagna, 2015/05/14 19:24:16]

There is no use-after-frees and the resources are deleted when the context is
lost.

As the TODO says, we shouldn't even call this callback if the context is lost.

+  // make sure that we won't execture this callback (use a weak pointer to
+  // the old context).
+  gpu::gles2::GLES2Interface* gles2 = gpu_factories->GetGLES2Interface();
+  if (!gles2)
+    return;
+
+  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.
+void GpuMemoryBufferVideoFramePool::PoolImpl::MailboxHoldersReleased(
+    FrameResources* frame_resources,
+    uint32 sync_point) {
+  // Return the resource on the media thread.
+  task_runner_->PostTask(FROM_HERE, base::Bind(&PoolImpl::ReturnFrameResources,
+                                               this, frame_resources));
+}
+
+// Put back the resoruces in the pool.
+void GpuMemoryBufferVideoFramePool::PoolImpl::ReturnFrameResources(
+    FrameResources* frame_resources) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  auto it = std::find(resources_pool_.begin(), resources_pool_.end(),

[DaleCurtis, 2015/05/14 00:58:58]

These will be released extremely frequently, so you'll want to do some profiling
to ensure this find() isn't too expensive.

[Daniele Castagna, 2015/05/14 17:30:27]

I'll take a look at this before enabling the pool.
resource_pool_ size is really little (size in the order of 10), it's really
unlikely this is going to be an issue.

+                      frame_resources);
+  DCHECK(it != resources_pool_.end());

[DaleCurtis, 2015/05/14 00:58:57]

DCHECK_NE?

[Daniele Castagna, 2015/05/14 17:30:26]

That doesn't work with iterators. :(

+  // 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());
+  frame_resources->in_use = false;
+}
+
+GpuMemoryBufferVideoFramePool::GpuMemoryBufferVideoFramePool(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    const scoped_refptr<GpuVideoAcceleratorFactories>& gpu_factories)
+    : pool_impl_(new PoolImpl(task_runner, gpu_factories)) {
+}
+
+GpuMemoryBufferVideoFramePool::~GpuMemoryBufferVideoFramePool() {
+}
+
+scoped_refptr<VideoFrame>
+GpuMemoryBufferVideoFramePool::MaybeCreateHardwareFrame(
+    const scoped_refptr<VideoFrame>& video_frame) {
+  switch (video_frame->format()) {
+    // Supported cases.
+    case VideoFrame::YV12:
+    case VideoFrame::I420:
+      return pool_impl_->CreateHardwareFrame(video_frame);
+    // Unsupported cases.
+    case media::VideoFrame::YV12A:
+    case media::VideoFrame::YV16:
+    case media::VideoFrame::YV12J:
+    case media::VideoFrame::YV12HD:
+    case media::VideoFrame::YV24:
+#if defined(VIDEO_HOLE)
+    case media::VideoFrame::HOLE:
+#endif  // defined(VIDEO_HOLE)
+    case media::VideoFrame::ARGB:
+      return video_frame;

[DaleCurtis, 2015/05/14 00:58:57]

I'd just do nothing here and change the return at the end of the function to
always return video_frame.

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+
+    // Unacceptable inputs.
+    case media::VideoFrame::NATIVE_TEXTURE:
+    case media::VideoFrame::UNKNOWN:
+    case media::VideoFrame::NV12:
+      NOTREACHED();

[DaleCurtis, 2015/05/14 00:58:58]

Again remove this, these falls under the unsupported cases. 

[Daniele Castagna, 2015/05/14 17:30:26]

Done.

+  }
+  NOTREACHED();
+  return scoped_refptr<VideoFrame>();
+}
+
+}  // namespace media