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 e270335504a523131f55f0f05325f98eeb79806d..15ac6c8a68c00bd63fa3668af0323c5500f399f8 100644
--- a/media/filters/vpx_video_decoder.cc
+++ b/media/filters/vpx_video_decoder.cc
@@ -18,6 +18,7 @@
 #include "media/base/bind_to_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 +31,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 +69,22 @@ static int GetThreadCount(const VideoDecoderConfig& config) {
   return decode_threads;
 }
 
+// Maximum number of frame buffers allowed to be used by libvpx for VP9
+// decoding. 8 (libvpx reference buffers) + 1 (work buffer) + kMaxVideoFrames (
+// jitter buffers).
+static const int kVP9MaxFrameBuffers = 9 + limits::kMaxVideoFrames;

[acolwell GONE FROM CHROMIUM, 2013/12/17 19:07:03]

Shouldn't libvpx advertise the minimum required number of frame buffers it needs
instead of us hardcoding it here?

[vignesh, 2013/12/18 18:01:02]

Done.

+
+// Minimum libvpx decoder ABI version required to support direct rendering.
+static const int kLibvpxMinABIExternalBuffers = 6;
+
 VpxVideoDecoder::VpxVideoDecoder(
     const scoped_refptr<base::MessageLoopProxy>& message_loop)
     : message_loop_(message_loop),
       weak_factory_(this),
       state_(kUninitialized),
       vpx_codec_(NULL),
-      vpx_codec_alpha_(NULL) {
+      vpx_codec_alpha_(NULL),
+      frame_buffers_(NULL) {
 }
 
 VpxVideoDecoder::~VpxVideoDecoder() {
@@ -124,6 +135,21 @@ static vpx_codec_ctx* InitializeVpxContext(vpx_codec_ctx* context,
   return context;
 }
 
+int32 VpxVideoDecoder::ReallocVP9FrameBuffer(void *user_priv, size_t new_size,
+                                             vpx_codec_frame_buffer *fb) {
+  if (!fb)
+    return -1;
+  if (fb->data)
+    delete[] fb->data;

[acolwell GONE FROM CHROMIUM, 2013/12/17 19:07:03]

This is not safe. libvpx only knows when it no longer needs this reference. It
has no clue about whether a VideoFrame is still around that contains pointers
into this data.

[vignesh, 2013/12/18 18:01:02]

The way libvpx designed to do this is under the following assumption:
 * libvpx needs 9 buffers (8 reference buffers + 1 work buffer) for decoding
VP9.
 * the application passes in 9 + x buffers to libvpx, where x is the number of
buffers (frames) that the application expects to hold on to after decoding.
libvpx ensures that it won't call Realloc (or overwrite) on the x most recent
frames. So assuming that's how chromium works (it does not hold on to more than
kMaxVideoFrames), this behavior is correct. If not, we will have to fix it.

+  fb->data = new uint8[new_size];
+  if (!fb->data) {
+    fb->size = 0;
+    return -1;
+  }
+  fb->size = new_size;
+  return VPX_CODEC_OK;
+}
+
 bool VpxVideoDecoder::ConfigureDecoder(const VideoDecoderConfig& config) {
   const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
   bool can_handle = false;
@@ -142,6 +168,21 @@ 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 &&
+      VPX_DECODER_ABI_VERSION >= kLibvpxMinABIExternalBuffers) {
+    frame_buffers_ = new vpx_codec_frame_buffer[kVP9MaxFrameBuffers]();
+    if (vpx_codec_set_frame_buffers(vpx_codec_,
+                                    frame_buffers_,
+                                    kVP9MaxFrameBuffers,
+                                    VpxVideoDecoder::ReallocVP9FrameBuffer,
+                                    NULL)) {
+      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 +197,12 @@ void VpxVideoDecoder::CloseDecoder() {
     vpx_codec_destroy(vpx_codec_);
     delete vpx_codec_;
     vpx_codec_ = NULL;
+    if (frame_buffers_) {

[acolwell GONE FROM CHROMIUM, 2013/12/17 19:07:03]

This is not safe. It assumes that all VideoFrames have been destroyed when the
CloseDecoder() method has been called. This is definitely not true when
MediaSource is used and there are config changes. Frames will likely outlive the
decoder instance. You need to provide a closure to WrapExternalYuvData so that
you can determine when all the frames have been released.

[vignesh, 2013/12/18 18:01:02]

Addressing this issue by introducing MemoryPool class as you suggested.

+      for (int i = 0; i < kVP9MaxFrameBuffers; i++)
+        delete[] frame_buffers_[i].data;
+      delete[] frame_buffers_;
+      frame_buffers_ = NULL;
+    }
   }
   if (vpx_codec_alpha_) {
     vpx_codec_destroy(vpx_codec_alpha_);
@@ -342,6 +389,22 @@ void VpxVideoDecoder::CopyVpxImageTo(const vpx_image* vpx_image,
 
   gfx::Size size(vpx_image->d_w, vpx_image->d_h);
 
+  if (!vpx_codec_alpha_ &&

[acolwell GONE FROM CHROMIUM, 2013/12/17 19:07:03]

Why isn't alpha included in the 0-copy code path?

[vignesh, 2013/12/18 18:01:02]

libvpx takes in external buffers only for VP9 as of now. So we have to do a copy
for VP8 (as alpha is VP8 only).

+      VPX_DECODER_ABI_VERSION >= kLibvpxMinABIExternalBuffers) {
+    *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(),
+                       base::Closure());
+    return;
+  }
+
   *video_frame = frame_pool_.CreateFrame(
       vpx_codec_alpha_ ? VideoFrame::YV12A : VideoFrame::YV12,
       size,
@@ -361,8 +424,6 @@ void VpxVideoDecoder::CopyVpxImageTo(const vpx_image* vpx_image,
              vpx_image->stride[VPX_PLANE_V],
              (vpx_image->d_h + 1) / 2,
              video_frame->get());
-  if (!vpx_codec_alpha_)
-    return;
   if (!vpx_image_alpha) {
     MakeOpaqueAPlane(
         vpx_image->stride[VPX_PLANE_Y], vpx_image->d_h, video_frame->get());