media: Optimize HW Video to 2D Canvas copy.

media/filters/skcanvas_video_renderer.cc

Index: media/filters/skcanvas_video_renderer.cc
diff --git a/media/filters/skcanvas_video_renderer.cc b/media/filters/skcanvas_video_renderer.cc
index 9dcf67a7ada815ea478a3ef9c00777c3ad04b918..41485d90b5237c9bca97cde3e0ce2e4f84dec3d9 100644
--- a/media/filters/skcanvas_video_renderer.cc
+++ b/media/filters/skcanvas_video_renderer.cc
@@ -5,11 +5,17 @@
 #include "media/filters/skcanvas_video_renderer.h"
 
 #include "base/logging.h"
+#include "gpu/GLES2/gl2extchromium.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/video_frame.h"
 #include "media/base/yuv_convert.h"
+#include "skia/ext/refptr.h"
 #include "third_party/libyuv/include/libyuv.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkImageGenerator.h"
+#include "third_party/skia/include/gpu/GrContext.h"
+#include "third_party/skia/include/gpu/SkGrPixelRef.h"
 #include "ui/gfx/skbitmap_operations.h"
 
 // Skia internal format depends on a platform. On Android it is ABGR, on others
@@ -28,7 +34,15 @@
 
 namespace media {
 
-static bool IsYUV(media::VideoFrame::Format format) {
+namespace {
+
+// This class keeps two temporary resources; software bitmap, hardware bitmap.
+// If both bitmap are created and then only software bitmap is updated every
+// frame, hardware bitmap outlives until the media player dies. So we delete
+// a temporary resource if it is not used for 3 sec.
+const int kTemporaryResourceDeletionDelay = 3;  // Seconds;

[scherkus (not reviewing), 2014/10/15 17:10:23]

note this is in media time, which has little relation to actual wall clock time

can you provide some more details on why we need this? for example, what would
happen if we didn't have this code?

+
+bool IsYUV(media::VideoFrame::Format format) {
   switch (format) {
     case VideoFrame::YV12:
     case VideoFrame::YV16:
@@ -49,7 +63,7 @@ static bool IsYUV(media::VideoFrame::Format format) {
   return false;
 }
 
-static bool IsJPEGColorSpace(media::VideoFrame::Format format) {
+bool IsJPEGColorSpace(media::VideoFrame::Format format) {
   switch (format) {
     case VideoFrame::YV12J:
       return true;
@@ -70,12 +84,12 @@ static bool IsJPEGColorSpace(media::VideoFrame::Format format) {
   return false;
 }
 
-static bool IsYUVOrNative(media::VideoFrame::Format format) {
+bool IsYUVOrNative(media::VideoFrame::Format format) {
   return IsYUV(format) || format == media::VideoFrame::NATIVE_TEXTURE;
 }
 
 // Converts a |video_frame| to raw |rgb_pixels|.
-static void ConvertVideoFrameToRGBPixels(
+void ConvertVideoFrameToRGBPixels(
     const scoped_refptr<media::VideoFrame>& video_frame,
     void* rgb_pixels,
     size_t row_bytes) {
@@ -205,6 +219,94 @@ static void ConvertVideoFrameToRGBPixels(
   }
 }
 
+bool EnsureTextureBackedSkBitmap(GrContext* gr,

[scherkus (not reviewing), 2014/10/15 17:10:23]

I know you lifted this code from WebMediaPlayerAndroid, but can we rename this,
add docs, and clean it up a bit? It's a small style thing, but I don't really
like these "ensure" style methods as it gets unclear as to what they exactly are
doing and what returning "false" means (it looks like it's for saying that it
failed to allocate)


for example, I'd find it more clear to split this into two functions:

bool IsSkBitmapProperlySizedTexture(const SkBitmap* bitmap, const gfx::Size&
size) {
  // existing initial checks
}

bool AllocateSkBitmapTexture(GrContext* gr, SkBitmap* bitmap, const gfx::Size&
size) {
  // rest of allocation code
}


It'd help clean up the comments/code around line 266 in
CovertVideoFrameToTexture()

+                                 SkBitmap* bitmap,
+                                 const gfx::Size& size) {
+  if (!bitmap->getTexture() || bitmap->width() != size.width() ||
+      bitmap->height() != size.height()) {
+    if (!gr)
+      return false;
+    GrTextureDesc desc;
+    // Use kRGBA_8888_GrPixelConfig, not kSkia8888_GrPixelConfig, because
+    // glCopyTextureChromium doesn't support GL_BGRA_EXT as internal format.
+    desc.fConfig = kRGBA_8888_GrPixelConfig;
+    desc.fFlags = kRenderTarget_GrTextureFlagBit | kNoStencil_GrTextureFlagBit;
+    desc.fSampleCnt = 0;
+    desc.fOrigin = kTopLeft_GrSurfaceOrigin;
+    desc.fWidth = size.width();
+    desc.fHeight = size.height();
+    GrAutoScratchTexture scratch_texture(
+        gr, desc, GrContext::kExact_ScratchTexMatch);
+    skia::RefPtr<GrTexture> texture = skia::AdoptRef(scratch_texture.detach());
+    if (!texture.get())
+      return false;
+
+    SkImageInfo info = SkImageInfo::MakeN32Premul(desc.fWidth, desc.fHeight);
+    SkGrPixelRef* pixelRef = SkNEW_ARGS(SkGrPixelRef, (info, texture.get()));
+    if (!pixelRef)
+      return false;
+    bitmap->setInfo(info);
+    bitmap->setPixelRef(pixelRef)->unref();
+  }
+
+  return true;
+}
+
+bool ConvertVideoFrameToTexture(

[scherkus (not reviewing), 2014/10/15 17:10:23]

considering all the various combinations of sw vs. hw, let's try to have these
method names be as exact as possible

I'd also recommend changing the name Convert to Copy considering it is actually
a copy

e.g., CopyVideoFrameTextureToSkBitmapTexture(...)

+    VideoFrame* video_frame,
+    SkBitmap* bitmap,
+    SkCanvasVideoRenderer::Context3DProvider* context_provider) {
+  DCHECK(context_provider && context_provider->gl &&
+         context_provider->gr_context &&
+         video_frame->format() == VideoFrame::NATIVE_TEXTURE);
+
+  // Check if we could reuse existing texture based bitmap.
+  // Otherwise, release existing texture based bitmap and allocate
+  // a new one based on video size.
+  if (!EnsureTextureBackedSkBitmap(context_provider->gr_context,
+                                   bitmap,
+                                   video_frame->visible_rect().size())) {
+    return false;
+  }
+
+  unsigned textureId =

[scherkus (not reviewing), 2014/10/15 17:10:24]

textureId -> texture_id

+      static_cast<unsigned>((bitmap->getTexture())->getTextureHandle());
+  // If CopyVideoFrameToTexture() changes the state of the 'textureId', it's
+  // needed to invalidate the state cached in skia, but currently the state
+  // isn't changed.
+  SkCanvasVideoRenderer::CopyVideoFrameToTexture(context_provider->gl,
+                                                 video_frame,
+                                                 textureId,
+                                                 0,
+                                                 GL_RGBA,
+                                                 GL_UNSIGNED_BYTE,
+                                                 true,
+                                                 false);
+  return true;
+}
+
+class SyncPointClientImpl : public VideoFrame::SyncPointClient {
+ public:
+  explicit SyncPointClientImpl(gpu::gles2::GLES2Interface* gl) : gl_(gl) {}
+  virtual ~SyncPointClientImpl() {}
+  virtual uint32 InsertSyncPoint() OVERRIDE {
+    return gl_->InsertSyncPointCHROMIUM();
+  }
+  virtual void WaitSyncPoint(uint32 sync_point) OVERRIDE {
+    gl_->WaitSyncPointCHROMIUM(sync_point);
+  }
+
+ private:
+  gpu::gles2::GLES2Interface* gl_;
+};
+
+bool IsNull(SkCanvasVideoRenderer::Context3DProvider* context_provider) {
+  return !context_provider ||
+         !(context_provider->gl && context_provider->gr_context);
+}
+
+}  // anonymous namespace
+
 // Generates an RGB image from a VideoFrame.
 class VideoImageGenerator : public SkImageGenerator {
  public:
@@ -289,18 +391,22 @@ class VideoImageGenerator : public SkImageGenerator {
 };
 
 SkCanvasVideoRenderer::SkCanvasVideoRenderer()
-    : generator_(NULL), last_frame_timestamp_(media::kNoTimestamp()) {
+    : generator_(NULL),
+      last_frame_timestamp_(media::kNoTimestamp()),
+      accelerated_last_frame_timestamp_(media::kNoTimestamp()) {
   last_frame_.setIsVolatile(true);
 }
 
 SkCanvasVideoRenderer::~SkCanvasVideoRenderer() {}
 
-void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame,
-                                  SkCanvas* canvas,
-                                  const gfx::RectF& dest_rect,
-                                  uint8 alpha,
-                                  SkXfermode::Mode mode,
-                                  VideoRotation video_rotation) {
+void SkCanvasVideoRenderer::Paint(
+    const scoped_refptr<VideoFrame>& video_frame,
+    SkCanvas* canvas,
+    const gfx::RectF& dest_rect,
+    uint8 alpha,
+    SkXfermode::Mode mode,
+    VideoRotation video_rotation,
+    SkCanvasVideoRenderer::Context3DProvider* context_provider) {

[scherkus (not reviewing), 2014/10/15 17:10:23]

we can drop SkCanvasVideoRenderer::, right?

   if (alpha == 0) {
     return;
   }
@@ -313,32 +419,56 @@ void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame,
 
   // Paint black rectangle if there isn't a frame available or the
   // frame has an unexpected format.
-  if (!video_frame.get() || !IsYUVOrNative(video_frame->format())) {
+  if (!video_frame.get() || video_frame->natural_size().IsEmpty() ||
+      !IsYUVOrNative(video_frame->format())) {
     canvas->drawRect(dest, paint);
     canvas->flush();
     return;
   }
 
+  bool accelerated = false;
+  if (!IsNull(context_provider) &&
+      video_frame->format() == media::VideoFrame::NATIVE_TEXTURE &&
+      canvas->getGrContext()) {
+    // TODO(dshwang): Android video decoder doesn't update the timestamp on a
+    // VideoFrame. To reduce redundant copy, Android should update the
+    // timestamp.
+    if (accelerated_last_frame_.isNull() ||
+        video_frame->timestamp() != accelerated_last_frame_timestamp_ ||
+        video_frame->timestamp() == base::TimeDelta()) {
+      accelerated = ConvertVideoFrameToTexture(
+          video_frame.get(), &accelerated_last_frame_, context_provider);
+      if (accelerated)
+        accelerated_last_frame_timestamp_ = video_frame->timestamp();
+    } else {
+      DCHECK(accelerated_last_frame_.getTexture());
+      accelerated = true;
+    }
+  }
+
   // Check if we should convert and update |last_frame_|.
-  if (last_frame_.isNull() ||
-      video_frame->timestamp() != last_frame_timestamp_) {
-    generator_ = new VideoImageGenerator(video_frame);
+  if (!accelerated) {
+    if (last_frame_.isNull() ||
+        video_frame->timestamp() != last_frame_timestamp_) {
+      generator_ = new VideoImageGenerator(video_frame);
+
+      // Note: This takes ownership of |generator_|.
+      if (!SkInstallDiscardablePixelRef(generator_, &last_frame_)) {
+        NOTREACHED();
+      }
+      DCHECK(video_frame->visible_rect().width() == last_frame_.width() &&
+             video_frame->visible_rect().height() == last_frame_.height());
 
-    // Note: This takes ownership of |generator_|.
-    if (!SkInstallDiscardablePixelRef(generator_, &last_frame_)) {
-      NOTREACHED();
+      last_frame_timestamp_ = video_frame->timestamp();
+    } else if (generator_) {
+      generator_->set_frame(video_frame);
     }
-    DCHECK(video_frame->visible_rect().width() == last_frame_.width() &&
-           video_frame->visible_rect().height() == last_frame_.height());
-
-    last_frame_timestamp_ = video_frame->timestamp();
-  } else if (generator_) {
-    generator_->set_frame(video_frame);
   }
 
   paint.setXfermodeMode(mode);
   paint.setFilterLevel(SkPaint::kLow_FilterLevel);
 
+  SkBitmap& target_frame = accelerated ? accelerated_last_frame_ : last_frame_;
   bool need_transform =
       video_rotation != VIDEO_ROTATION_0 ||
       dest_rect.size() != video_frame->visible_rect().size() ||
@@ -371,12 +501,12 @@ void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame,
           gfx::SizeF(rotated_dest_size.height(), rotated_dest_size.width());
     }
     canvas->scale(
-        SkFloatToScalar(rotated_dest_size.width() / last_frame_.width()),
-        SkFloatToScalar(rotated_dest_size.height() / last_frame_.height()));
-    canvas->translate(-SkFloatToScalar(last_frame_.width() * 0.5f),
-                      -SkFloatToScalar(last_frame_.height() * 0.5f));
+        SkFloatToScalar(rotated_dest_size.width() / target_frame.width()),
+        SkFloatToScalar(rotated_dest_size.height() / target_frame.height()));
+    canvas->translate(-SkFloatToScalar(target_frame.width() * 0.5f),
+                      -SkFloatToScalar(target_frame.height() * 0.5f));
   }
-  canvas->drawBitmap(last_frame_, 0, 0, &paint);
+  canvas->drawBitmap(target_frame, 0, 0, &paint);
   if (need_transform)
     canvas->restore();
   canvas->flush();
@@ -384,6 +514,7 @@ void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame,
   // |video_frame| not to be outlived.
   if (generator_)
     generator_->set_frame(NULL);
+  CleanUpTemporaryBuffers();
 }
 
 void SkCanvasVideoRenderer::Copy(const scoped_refptr<VideoFrame>& video_frame,
@@ -393,7 +524,66 @@ void SkCanvasVideoRenderer::Copy(const scoped_refptr<VideoFrame>& video_frame,
         video_frame->visible_rect(),
         0xff,
         SkXfermode::kSrc_Mode,
-        media::VIDEO_ROTATION_0);
+        media::VIDEO_ROTATION_0,
+        NULL);
+}
+
+void SkCanvasVideoRenderer::CleanUpTemporaryBuffers() {
+  static const base::TimeDelta buffer_time =
+      base::TimeDelta::FromSeconds(kTemporaryResourceDeletionDelay);
+  // Find the latest time stamp.
+  base::TimeDelta last_timestamp =
+      accelerated_last_frame_timestamp_ > last_frame_timestamp_
+          ? accelerated_last_frame_timestamp_
+          : last_frame_timestamp_;
+  // Delete a too old resource.
+  if (last_timestamp > last_frame_timestamp_ + buffer_time &&
+      !last_frame_.isNull()) {
+    last_frame_.reset();
+  }
+  if (last_timestamp > accelerated_last_frame_timestamp_ + buffer_time &&
+      !accelerated_last_frame_.isNull()) {
+    accelerated_last_frame_.reset();
+  }
+}
+
+// static
+void SkCanvasVideoRenderer::CopyVideoFrameToTexture(

[scherkus (not reviewing), 2014/10/15 17:10:23]

ditto for precise naming

this is much more like CopyVideoFrameTextureToGlTexture()

+    gpu::gles2::GLES2Interface* gl,
+    VideoFrame* video_frame,
+    unsigned int texture,
+    unsigned int level,
+    unsigned int internal_format,
+    unsigned int type,
+    bool premultiply_alpha,
+    bool flip_y) {
+  DCHECK(video_frame && video_frame->format() == VideoFrame::NATIVE_TEXTURE);
+  const gpu::MailboxHolder* mailbox_holder = video_frame->mailbox_holder();
+  DCHECK(mailbox_holder->texture_target == GL_TEXTURE_2D ||
+         mailbox_holder->texture_target == GL_TEXTURE_EXTERNAL_OES);
+
+  gl->WaitSyncPointCHROMIUM(mailbox_holder->sync_point);
+  uint32 source_texture = gl->CreateAndConsumeTextureCHROMIUM(
+      mailbox_holder->texture_target, mailbox_holder->mailbox.name);
+
+  // The video is stored in a unmultiplied format, so premultiply
+  // if necessary.
+  gl->PixelStorei(GL_UNPACK_PREMULTIPLY_ALPHA_CHROMIUM, premultiply_alpha);
+  // Application itself needs to take care of setting the right |flip_y|
+  // value down to get the expected result.
+  // "flip_y == true" means to reverse the video orientation while
+  // "flip_y == false" means to keep the intrinsic orientation.
+  gl->PixelStorei(GL_UNPACK_FLIP_Y_CHROMIUM, flip_y);
+  gl->CopyTextureCHROMIUM(
+      GL_TEXTURE_2D, source_texture, texture, level, internal_format, type);
+  gl->PixelStorei(GL_UNPACK_FLIP_Y_CHROMIUM, false);
+  gl->PixelStorei(GL_UNPACK_PREMULTIPLY_ALPHA_CHROMIUM, false);
+
+  gl->DeleteTextures(1, &source_texture);
+  gl->Flush();
+
+  SyncPointClientImpl client(gl);
+  video_frame->UpdateReleaseSyncPoint(&client);
 }
 
 }  // namespace media