With this CL, the time for PepperVideoSourceHost::SendGetFrameReply drops from about 4ms to 0.7ms for each 720p frame.

If the current video frame is of the same size as the previous frame, reuse the shared memory and overwrite the previous frame. Pepper does not hold onto a frame after it has been processed, and it indicates processing is done by sending a new request, so overwriting the previous frame should be fine.

BUG=423365

Review URL: https://codereview.chromium.org/653873003

Cr-Commit-Position: refs/heads/master@{#301657}

5 files changed, 94 insertions, 41 deletions

diff --git a/content/renderer/pepper/pepper_video_source_host.cc b/content/renderer/pepper/pepper_video_source_host.cc
index bdad965..dd22573 100644
--- a/content/renderer/pepper/pepper_video_source_host.cc
+++ b/content/renderer/pepper/pepper_video_source_host.cc
@@ -12,6 +12,7 @@
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/host/dispatch_host_message.h"
 #include "ppapi/host/ppapi_host.h"
+#include "ppapi/proxy/host_dispatcher.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/proxy/ppb_image_data_proxy.h"
 #include "ppapi/shared_impl/scoped_pp_resource.h"
@@ -52,6 +53,7 @@ PepperVideoSourceHost::PepperVideoSourceHost(RendererPpapiHost* host,
       get_frame_pending_(false),
       weak_factory_(this) {
   frame_receiver_ = new FrameReceiver(weak_factory_.GetWeakPtr());
+  memset(&shared_image_desc_, 0, sizeof(shared_image_desc_));
 }
 
 PepperVideoSourceHost::~PepperVideoSourceHost() { Close(); }
@@ -119,45 +121,85 @@ void PepperVideoSourceHost::SendGetFrameReply() {
   const int dst_width = frame->visible_rect().width();
   const int dst_height = frame->visible_rect().height();
 
-  PP_ImageDataDesc image_desc;
+  // Note: We try to reuse the shared memory for the previous frame here. This
+  // means that the previous frame may be overwritten and is no longer valid
+  // after calling this function again.
   IPC::PlatformFileForTransit image_handle;
   uint32_t byte_count;
-  ppapi::ScopedPPResource resource(
-      ppapi::ScopedPPResource::PassRef(),
-      ppapi::proxy::PPB_ImageData_Proxy::CreateImageData(
-          pp_instance(),
-          ppapi::PPB_ImageData_Shared::SIMPLE,
-          PP_IMAGEDATAFORMAT_BGRA_PREMUL,
-          PP_MakeSize(dst_width, dst_height),
-          false /* init_to_zero */,
-          &image_desc,
-          &image_handle,
-          &byte_count));
-  if (!resource.get()) {
-    SendGetFrameErrorReply(PP_ERROR_FAILED);
-    return;
+  if (shared_image_.get() && dst_width == shared_image_->width() &&
+      dst_height == shared_image_->height()) {
+    // We have already allocated the correct size in shared memory. We need to
+    // duplicate the handle for IPC however, which will close down the
+    // duplicated handle when it's done.
+    int local_fd = 0;
+    if (shared_image_->GetSharedMemory(&local_fd, &byte_count) != PP_OK) {
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
+
+    ppapi::proxy::HostDispatcher* dispatcher =
+        ppapi::proxy::HostDispatcher::GetForInstance(pp_instance());
+    if (!dispatcher) {
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
+
+#if defined(OS_WIN)
+    image_handle = dispatcher->ShareHandleWithRemote(
+        reinterpret_cast<HANDLE>(static_cast<intptr_t>(local_fd)), false);
+#elif defined(OS_POSIX)
+    image_handle = dispatcher->ShareHandleWithRemote(local_fd, false);
+#else
+#error Not implemented.
+#endif
+  } else {
+    // We need to allocate new shared memory.
+    shared_image_ = NULL;  // Release any previous image.
+
+    ppapi::ScopedPPResource resource(
+        ppapi::ScopedPPResource::PassRef(),
+        ppapi::proxy::PPB_ImageData_Proxy::CreateImageData(
+            pp_instance(),
+            ppapi::PPB_ImageData_Shared::SIMPLE,
+            PP_IMAGEDATAFORMAT_BGRA_PREMUL,
+            PP_MakeSize(dst_width, dst_height),
+            false /* init_to_zero */,
+            &shared_image_desc_,
+            &image_handle,
+            &byte_count));
+    if (!resource) {
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
+
+    ppapi::thunk::EnterResourceNoLock<ppapi::thunk::PPB_ImageData_API>
+        enter_resource(resource, false);
+    if (enter_resource.failed()) {
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
+
+    shared_image_ = static_cast<PPB_ImageData_Impl*>(enter_resource.object());
+    if (!shared_image_.get()) {
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
+
+    DCHECK(!shared_image_->IsMapped());  // New memory should not be mapped.
+    if (!shared_image_->Map() || !shared_image_->GetMappedBitmap() ||
+        !shared_image_->GetMappedBitmap()->getPixels()) {
+      shared_image_ = NULL;
+      SendGetFrameErrorReply(PP_ERROR_FAILED);
+      return;
+    }
   }
 
-  ppapi::thunk::EnterResourceNoLock<ppapi::thunk::PPB_ImageData_API>
-      enter_resource(resource, false);
-  if (enter_resource.failed()) {
-    SendGetFrameErrorReply(PP_ERROR_FAILED);
-    return;
-  }
-
-  PPB_ImageData_Impl* image_data =
-      static_cast<PPB_ImageData_Impl*>(enter_resource.object());
-  ImageDataAutoMapper mapper(image_data);
-  if (!mapper.is_valid()) {
-    SendGetFrameErrorReply(PP_ERROR_FAILED);
-    return;
-  }
-
-  const SkBitmap* bitmap = image_data->GetMappedBitmap();
+  const SkBitmap* bitmap = shared_image_->GetMappedBitmap();
   if (!bitmap) {
     SendGetFrameErrorReply(PP_ERROR_FAILED);
     return;
   }
+
   uint8_t* bitmap_pixels = static_cast<uint8_t*>(bitmap->getPixels());
   if (!bitmap_pixels) {
     SendGetFrameErrorReply(PP_ERROR_FAILED);
@@ -194,10 +236,10 @@ void PepperVideoSourceHost::SendGetFrameReply() {
                      dst_height);
 
   ppapi::HostResource host_resource;
-  host_resource.SetHostResource(pp_instance(), resource.get());
+  host_resource.SetHostResource(pp_instance(), shared_image_->GetReference());
 
   // Convert a video timestamp to a PP_TimeTicks (a double, in seconds).
-  PP_TimeTicks timestamp = frame->timestamp().InSecondsF();
+  const PP_TimeTicks timestamp = frame->timestamp().InSecondsF();
 
   ppapi::proxy::SerializedHandle serialized_handle;
   serialized_handle.set_shmem(image_handle, byte_count);
@@ -205,12 +247,9 @@ void PepperVideoSourceHost::SendGetFrameReply() {
 
   host()->SendReply(reply_context_,
                     PpapiPluginMsg_VideoSource_GetFrameReply(
-                        host_resource, image_desc, timestamp));
+                        host_resource, shared_image_desc_, timestamp));
 
   reply_context_ = ppapi::host::ReplyMessageContext();
-
-  // Keep a reference once we know this method succeeds.
-  resource.Release();
 }
 
 void PepperVideoSourceHost::SendGetFrameErrorReply(int32_t error) {
@@ -228,6 +267,8 @@ void PepperVideoSourceHost::Close() {
 
   source_handler_.reset(NULL);
   stream_url_.clear();
+
+  shared_image_ = NULL;
 }
 
 }  // namespace content
diff --git a/content/renderer/pepper/pepper_video_source_host.h b/content/renderer/pepper/pepper_video_source_host.h
index 13d7d13..6010aad 100644
--- a/content/renderer/pepper/pepper_video_source_host.h
+++ b/content/renderer/pepper/pepper_video_source_host.h
@@ -13,6 +13,7 @@
 #include "content/common/content_export.h"
 #include "content/renderer/media/video_source_handler.h"
 #include "ppapi/c/pp_time.h"
+#include "ppapi/c/ppb_image_data.h"
 #include "ppapi/host/host_message_context.h"
 #include "ppapi/host/resource_host.h"
 
@@ -20,6 +21,7 @@ struct PP_ImageDataDesc;
 
 namespace content {
 
+class PPB_ImageData_Impl;
 class RendererPpapiHost;
 
 class CONTENT_EXPORT PepperVideoSourceHost : public ppapi::host::ResourceHost {
@@ -78,6 +80,13 @@ class CONTENT_EXPORT PepperVideoSourceHost : public ppapi::host::ResourceHost {
   std::string stream_url_;
   scoped_refptr<media::VideoFrame> last_frame_;
   bool get_frame_pending_;
+  // We use only one ImageData resource in order to avoid allocating
+  // shared memory repeatedly. We send the same one each time the plugin
+  // requests a frame. For this to work, the plugin must finish using
+  // the ImageData it receives prior to calling GetFrame, and not access
+  // the ImageData until it gets its next callback to GetFrame.
+  scoped_refptr<PPB_ImageData_Impl> shared_image_;
+  PP_ImageDataDesc shared_image_desc_;
 
   base::WeakPtrFactory<PepperVideoSourceHost> weak_factory_;
 
diff --git a/ppapi/api/private/ppb_video_source_private.idl b/ppapi/api/private/ppb_video_source_private.idl
index 3fc5872..e57eedd 100644
--- a/ppapi/api/private/ppb_video_source_private.idl
+++ b/ppapi/api/private/ppb_video_source_private.idl
@@ -61,7 +61,8 @@ interface PPB_VideoSource_Private {
                [in] PP_CompletionCallback callback);
 
   /**
-   * Gets a frame from the video source.
+   * Gets a frame from the video source. The returned image data is only valid
+   * until the next call to GetFrame.
    * The image data resource inside the returned frame will have its reference
    * count incremented by one and must be managed by the plugin.
    *
diff --git a/ppapi/c/private/ppb_video_source_private.h b/ppapi/c/private/ppb_video_source_private.h
index a3ba071..e2c715b 100644
--- a/ppapi/c/private/ppb_video_source_private.h
+++ b/ppapi/c/private/ppb_video_source_private.h
@@ -4,7 +4,7 @@
  */
 
 /* From private/ppb_video_source_private.idl,
- *   modified Thu Apr 25 11:51:30 2013.
+ *   modified Mon Oct 27 16:13:24 2014.
  */
 
 #ifndef PPAPI_C_PRIVATE_PPB_VIDEO_SOURCE_PRIVATE_H_
@@ -80,7 +80,8 @@ struct PPB_VideoSource_Private_0_1 {
                   struct PP_Var stream_url,
                   struct PP_CompletionCallback callback);
   /**
-   * Gets a frame from the video source.
+   * Gets a frame from the video source. The returned image data is only valid
+   * until the next call to GetFrame.
    * The image data resource inside the returned frame will have its reference
    * count incremented by one and must be managed by the plugin.
    *
diff --git a/ppapi/cpp/private/video_source_private.h b/ppapi/cpp/private/video_source_private.h
index 3d919bf..487703e 100644
--- a/ppapi/cpp/private/video_source_private.h
+++ b/ppapi/cpp/private/video_source_private.h
@@ -61,7 +61,8 @@ class VideoSource_Private : public Resource {
   int32_t Open(const Var& stream_url,
                const CompletionCallback& cc);
 
-  /// Gets a frame from the video source.
+  /// Gets a frame from the video source. The returned frame is only valid
+  /// until the next call to GetFrame.
   ///
   /// @param[out] frame A <code>VideoFrame_Private</code> to hold a video
   /// frame from the source.