media: Enabling direct rendering for VP9

media/filters/vpx_video_decoder.cc

Index: media/filters/vpx_video_decoder.cc
diff --git a/media/filters/vpx_video_decoder.cc b/media/filters/vpx_video_decoder.cc
index 7d71d787d56f3f192ed8ca3eeb3fe117bc953f8b..a4e17f64abbeaf9afc635d6736fd23b2a5318a0d 100644
--- a/media/filters/vpx_video_decoder.cc
+++ b/media/filters/vpx_video_decoder.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ -18,6 +19,7 @@
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
+#include "media/base/limits.h"
 #include "media/base/media_switches.h"
 #include "media/base/pipeline.h"
 #include "media/base/video_decoder_config.h"
@@ -30,6 +32,7 @@
 #define VPX_CODEC_DISABLE_COMPAT 1
 extern "C" {
 #include "third_party/libvpx/source/libvpx/vpx/vpx_decoder.h"
+#include "third_party/libvpx/source/libvpx/vpx/vpx_external_frame_buffer.h"
 #include "third_party/libvpx/source/libvpx/vpx/vp8dx.h"
 }
 
@@ -67,13 +70,152 @@ static int GetThreadCount(const VideoDecoderConfig& config) {
   return decode_threads;
 }
 
+// Maximum number of frame buffers that can be used (by both chromium and libvpx
+// combined) for VP9 Decoding.
+// TODO(vigneshv): Investigate if this can be relaxed to a higher number.
+static const int kVP9MaxFrameBuffers = VP9_MAXIMUM_REF_BUFFERS +
+                                       VPX_MAXIMUM_WORK_BUFFERS +
+                                       limits::kMaxVideoFrames;
+
+// Reference counted frame buffers used for VP9 decoding. Reference counting is
+// done manually because both chromium and libvpx has to release this before a
+// buffer can be re-used.
+struct VP9FrameBuffer {

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: Make this decl private inside the MemoryPool since nothing outside the pool
really needs to be aware of it.

[vignesh, 2014/02/04 01:38:10]

Done.

+  uint8* data;
+  uint64 size;
+  uint32 ref_cnt;
+};
+
+class VpxVideoDecoder::MemoryPool
+    : public base::RefCountedThreadSafe<VpxVideoDecoder::MemoryPool> {
+ public:
+  MemoryPool(VpxVideoDecoder* decoder);
+
+  // Callback that will be called by libvpx when it needs a frame buffer.
+  // Parameters:
+  // |user_priv|  Private data passed to libvpx (pointer to memory pool).
+  // |min_size|   Minimum size needed by libvpx to decompress the next frame.
+  // |fb|         Pointer to the frame buffer to update.
+  // Returns 0 on success. Returns < 0 on failure.
+  static int32 GetVP9FrameBuffer(void* user_priv, size_t min_size,
+                                 vpx_codec_frame_buffer* fb);
+
+  // Callback that will be called by libvpx when the frame buffer is no more

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: s/more/longer/

[vignesh, 2014/02/04 01:38:10]

Done.

+  // being used by libvpx. Parameters:
+  // |user_priv|  Private data passed to libvpx (pointer to memory pool).
+  // |fb|         Pointer to the frame buffer that's being released.
+  static int32 ReleaseVP9FrameBuffer(void *user_priv,
+                                     vpx_codec_frame_buffer *fb);
+
+  // Generates a "no_longer_needed" closure that holds a reference
+  // to this pool.
+  base::Closure frame_callback(VP9FrameBuffer* frame_buffer);
+
+  // Sets |decoder_| to NULL.
+  void ClearDecoder();
+
+ private:
+  friend class base::RefCountedThreadSafe<VpxVideoDecoder::MemoryPool>;
+  ~MemoryPool();
+
+  // Method that gets called when a VideoFrame that references this pool gets
+  // destroyed.
+  void OnVideoFrameDestroyed(VP9FrameBuffer* frame_buffer);
+
+  // Pointer to the VpxVideoDecoder object that has this object.
+  VpxVideoDecoder* decoder_;
+
+  // Frame buffers to be used by libvpx for VP9 Decoding.
+  std::vector<VP9FrameBuffer*> frame_buffers_;
+
+  DISALLOW_COPY_AND_ASSIGN(MemoryPool);
+};
+
+VpxVideoDecoder::MemoryPool::MemoryPool(VpxVideoDecoder* decoder)
+    : decoder_(decoder) {
+}
+
+VpxVideoDecoder::MemoryPool::~MemoryPool() {
+  for (size_t i = 0; i < frame_buffers_.size(); ++i) {
+    delete[] frame_buffers_[i]->data;
+    delete frame_buffers_[i];
+  }
+}
+
+int32 VpxVideoDecoder::MemoryPool::GetVP9FrameBuffer(
+    void* user_priv, size_t min_size, vpx_codec_frame_buffer* fb) {
+  if (user_priv == NULL || fb == NULL)

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: These look like they should be DCHECKs since I think it's likely a bug in
libvpx if these are NULL.

[vignesh, 2014/02/04 01:38:10]

Done.

+    return -1;
+
+  VpxVideoDecoder::MemoryPool* memory_pool =
+      static_cast<VpxVideoDecoder::MemoryPool*>(user_priv);
+

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

Put the rest of this method in a private method and just call that method here
so you can get rid of all the memory_pool references.

[vignesh, 2014/02/04 01:38:10]

Done.

+  // Check if a free frame buffer exists.
+  int i = 0;
+  for (i = 0; i < memory_pool->frame_buffers_.size(); ++i) {

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: s/i = 0// since you initialize it on the line above.

[vignesh, 2014/02/04 01:38:10]

Done.

+    if (memory_pool->frame_buffers_[i]->ref_cnt == 0)
+      break;
+  }
+
+  if (i == memory_pool->frame_buffers_.size()) {
+    // Maximum number of frame buffers reached.
+    if (i == kVP9MaxFrameBuffers)
+      return -1;
+
+    // Create a new frame buffer.
+    memory_pool->frame_buffers_.push_back(new VP9FrameBuffer());
+  }
+
+  // Reallocate the frame buffer if necessary.
+  if (memory_pool->frame_buffers_[i]->size < min_size) {
+    delete[] memory_pool->frame_buffers_[i]->data;

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

Just use std::vector<uint8> for data so you can replace this and the 4 lines
below with frame_buffers_[i]->resize(min_size).

[vignesh, 2014/02/04 01:38:10]

Done.

+    memory_pool->frame_buffers_[i]->data = new uint8[min_size];
+    if (!memory_pool->frame_buffers_[i]->data)
+      return -1;
+    memory_pool->frame_buffers_[i]->size = min_size;
+  }
+
+  fb->data = memory_pool->frame_buffers_[i]->data;
+  fb->size = memory_pool->frame_buffers_[i]->size;
+  ++memory_pool->frame_buffers_[i]->ref_cnt;
+
+  // Set the frame buffer's private data to point at the external frame buffer.
+  fb->frame_priv = static_cast<void*>(memory_pool->frame_buffers_[i]);
+  return 0;
+}
+
+int32 VpxVideoDecoder::MemoryPool::ReleaseVP9FrameBuffer(
+    void *user_priv, vpx_codec_frame_buffer *fb) {
+  VP9FrameBuffer* frame_buffer = static_cast<VP9FrameBuffer*>(fb->frame_priv);
+  --frame_buffer->ref_cnt;

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

Is this method and GetVP9FrameBuffer() ALWAYS called on the same thread as
vpx_codec_decode()? If not, then this isn't thread-safe.

[vignesh, 2014/02/04 01:38:10]

yes, both the methods are always called on the same thread as vpx_codec_decode.

+  return 0;
+}
+
+base::Closure VpxVideoDecoder::MemoryPool::frame_callback(

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: This isn't a trivial accessor so it should have a CamelCase name like
CreateFrameCallback().

[vignesh, 2014/02/04 01:38:10]

Done.

+    VP9FrameBuffer* frame_buffer) {
+  ++frame_buffer->ref_cnt;
+  return BindToCurrentLoop(
+             base::Bind(&MemoryPool::OnVideoFrameDestroyed, this,
+                        frame_buffer));
+}
+
+void VpxVideoDecoder::MemoryPool::OnVideoFrameDestroyed(
+    VP9FrameBuffer* frame_buffer) {
+  --frame_buffer->ref_cnt;
+}
+
+void VpxVideoDecoder::MemoryPool::ClearDecoder() {
+  decoder_ = NULL;
+}
+
 VpxVideoDecoder::VpxVideoDecoder(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
     : task_runner_(task_runner),
       weak_factory_(this),
       state_(kUninitialized),
       vpx_codec_(NULL),
-      vpx_codec_alpha_(NULL) {
+      vpx_codec_alpha_(NULL),
+      memory_pool_(NULL) {

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: You don't need this for a scoped_refptr. It is already initialized to null.

[vignesh, 2014/02/04 01:38:10]

Done.

 }
 
 VpxVideoDecoder::~VpxVideoDecoder() {
@@ -142,6 +284,20 @@ bool VpxVideoDecoder::ConfigureDecoder(const VideoDecoderConfig& config) {
   if (!vpx_codec_)
     return false;
 
+  // We use our own buffers for VP9 so that there is no need to copy data after
+  // decoding.
+  if (config.codec() == kCodecVP9) {
+    memory_pool_ = new MemoryPool(this);
+    if (vpx_codec_set_external_frame_buffer_functions(
+            vpx_codec_,
+            &MemoryPool::GetVP9FrameBuffer,
+            &MemoryPool::ReleaseVP9FrameBuffer,
+            memory_pool_)) {
+      LOG(ERROR) << "Failed to configure external buffers.";
+      return false;
+    }
+  }
+
   if (config.format() == VideoFrame::YV12A) {
     vpx_codec_alpha_ = InitializeVpxContext(vpx_codec_alpha_, config);
     if (!vpx_codec_alpha_)
@@ -156,6 +312,10 @@ void VpxVideoDecoder::CloseDecoder() {
     vpx_codec_destroy(vpx_codec_);
     delete vpx_codec_;
     vpx_codec_ = NULL;
+    if (memory_pool_) {
+      memory_pool_->ClearDecoder();
+      memory_pool_ = NULL;
+    }
   }
   if (vpx_codec_alpha_) {
     vpx_codec_destroy(vpx_codec_alpha_);
@@ -342,6 +502,23 @@ void VpxVideoDecoder::CopyVpxImageTo(const vpx_image* vpx_image,
 
   gfx::Size size(vpx_image->d_w, vpx_image->d_h);
 
+  if (!vpx_codec_alpha_ && memory_pool_) {
+    *video_frame = VideoFrame::WrapExternalYuvData(
+                       VideoFrame::YV12,
+                       size, gfx::Rect(size), config_.natural_size(),
+                       vpx_image->stride[VPX_PLANE_Y],
+                       vpx_image->stride[VPX_PLANE_U],
+                       vpx_image->stride[VPX_PLANE_V],
+                       vpx_image->planes[VPX_PLANE_Y],
+                       vpx_image->planes[VPX_PLANE_U],
+                       vpx_image->planes[VPX_PLANE_V],
+                       kNoTimestamp(),
+                       memory_pool_->frame_callback(

[acolwell GONE FROM CHROMIUM, 2014/02/04 00:48:35]

nit: Change the signature of this method so you don't need the cast here. It's a
little cleaner to have the cast inside the method since the MemoryPool is really
the one that "knows" this is a VP9FrameBuffer*.

[vignesh, 2014/02/04 01:38:10]

Done.

+                           static_cast<VP9FrameBuffer*>(
+                               vpx_image->ext_fb_priv)));
+    return;
+  }
+
   *video_frame = frame_pool_.CreateFrame(
       vpx_codec_alpha_ ? VideoFrame::YV12A : VideoFrame::YV12,
       size,