Passing Native Texture backed Video Frame from Renderer to GPU process

content/common/gpu/client/gpu_video_encode_accelerator_host.cc

Index: content/common/gpu/client/gpu_video_encode_accelerator_host.cc
diff --git a/content/common/gpu/client/gpu_video_encode_accelerator_host.cc b/content/common/gpu/client/gpu_video_encode_accelerator_host.cc
index 1cbfeaa025a5a3c300068a36dba5e7d87b91a0db..330566212c303c64daf68760d53d53a029869ab7 100644
--- a/content/common/gpu/client/gpu_video_encode_accelerator_host.cc
+++ b/content/common/gpu/client/gpu_video_encode_accelerator_host.cc
@@ -117,33 +117,17 @@ void GpuVideoEncodeAcceleratorHost::Encode(
   if (!channel_)
     return;
 
-  if (!base::SharedMemory::IsHandleValid(frame->shared_memory_handle())) {
-    NOTIFY_ERROR(kPlatformFailureError)
-        << "Encode(): cannot encode frame not backed by shared memory";
-    return;
-  }
-  base::SharedMemoryHandle handle =
-      channel_->ShareToGpuProcess(frame->shared_memory_handle());
-  if (!base::SharedMemory::IsHandleValid(handle)) {
-    NOTIFY_ERROR(kPlatformFailureError)
-        << "Encode(): failed to duplicate buffer handle for GPU process";
+  bool rv = false;

[mcasas, 2015/06/04 15:20:55]

s/rv/return_val/ or similar?

[emircan, 2015/06/04 22:04:40]

Done.

+  if (frame->storage_type() == media::VideoFrame::STORAGE_TEXTURE)
+    rv = EncodeNativeTexture(frame, force_keyframe);
+  else
+    rv = EncodeSharedMemory(frame, force_keyframe);

[mcasas, 2015/06/04 15:20:55]

What about

const bool encoded_ok = (frame->IsMappable()) 
    ? EncodeSharedMemory(frame, force_keyframe)
    : EncodeNativeTexture(frame, force_keyframe);

The less clients touch explicitly the STORAGE_* the better.

[emircan, 2015/06/04 22:04:40]

IsMappable() covers STORAGE_OWNED_MEMORY and STORAGE_UNOWNED_MEMORY as well, and
they don't provide shared_memory_handle() which we ShareToGpuProcess() and use
in the IPC call. Here we are specifically looking for STORAGE_SHMEM.

+
+  if (!rv) {
+    NOTIFY_ERROR(kPlatformFailureError) << "Encode(): cannot encode frame";
     return;
   }
 
-  // We assume that planar frame data passed here is packed and contiguous.
-  const size_t plane_count = media::VideoFrame::NumPlanes(frame->format());
-  size_t frame_size = 0;
-  for (size_t i = 0; i < plane_count; ++i) {
-    // Cast DCHECK parameters to void* to avoid printing uint8* as a string.
-    DCHECK_EQ(reinterpret_cast<void*>(frame->data(i)),
-              reinterpret_cast<void*>((frame->data(0) + frame_size)))
-        << "plane=" << i;
-    frame_size += frame->stride(i) * frame->rows(i);
-  }
-
-  Send(new AcceleratedVideoEncoderMsg_Encode(
-      encoder_route_id_, next_frame_id_, handle, frame->shared_memory_offset(),
-      frame_size, force_keyframe));
   frame_map_[next_frame_id_] = frame;
 
   // Mask against 30 bits, to avoid (undefined) wraparound on signed integer.
@@ -195,6 +179,50 @@ void GpuVideoEncodeAcceleratorHost::OnWillDeleteImpl() {
   OnChannelError();
 }
 
+bool GpuVideoEncodeAcceleratorHost::EncodeNativeTexture(
+    const scoped_refptr<media::VideoFrame>& frame,
+    bool force_keyframe) {
+  Send(new AcceleratedVideoEncoderMsg_EncodeNativeTexture(

[mcasas, 2015/06/04 15:20:55]

DCHECK_EQ(frame->storage_type(), STORAGE_TEXTURE) ?
I think ATM is the only non-mappable type we can receive
here, i.e. no DMABUFS or HOLEs, correct?

[emircan, 2015/06/04 22:04:40]

Correct. Passing only the mailbox.

+      encoder_route_id_, next_frame_id_, frame->mailbox_holder(0),
+      force_keyframe));
+  return true;
+}
+
+bool GpuVideoEncodeAcceleratorHost::EncodeSharedMemory(
+    const scoped_refptr<media::VideoFrame>& frame,
+    bool force_keyframe) {
+  if (!base::SharedMemory::IsHandleValid(frame->shared_memory_handle())) {

[mcasas, 2015/06/04 15:20:55]

DCHECK_EQ(frame->storage_type(), STORAGE_SHMEM)?
Although is done inside shared_memory_handle(), I don't
think is superfluous to encode such an assumption here.

[emircan, 2015/06/04 22:04:40]

Done.

+    NOTIFY_ERROR(kPlatformFailureError) << "EncodeSharedMemory(): cannot "
+                                           "encode frame with invalid shared "
+                                           "memory handle";
+    return false;
+  }
+  base::SharedMemoryHandle handle =
+      channel_->ShareToGpuProcess(frame->shared_memory_handle());
+  if (!base::SharedMemory::IsHandleValid(handle)) {
+    NOTIFY_ERROR(kPlatformFailureError) << "EncodeSharedMemory(): failed to "
+                                           "duplicate buffer handle for GPU "
+                                           "process";
+    return false;
+  }
+
+  // We assume that planar frame data passed here is packed and contiguous.
+  const size_t plane_count = media::VideoFrame::NumPlanes(frame->format());
+  size_t frame_size = 0;
+  for (size_t i = 0; i < plane_count; ++i) {
+    // Cast DCHECK parameters to void* to avoid printing uint8* as a string.
+    DCHECK_EQ(reinterpret_cast<void*>(frame->data(i)),
+              reinterpret_cast<void*>((frame->data(0) + frame_size)))
+        << "plane=" << i;
+    frame_size += frame->stride(i) * frame->rows(i);
+  }
+
+  Send(new AcceleratedVideoEncoderMsg_EncodeSharedMemory(
+      encoder_route_id_, next_frame_id_, handle,
+      frame->shared_memory_offset(), frame_size, force_keyframe));
+  return true;
+}
+
 void GpuVideoEncodeAcceleratorHost::PostNotifyError(Error error) {
   DCHECK(CalledOnValidThread());
   DVLOG(2) << "PostNotifyError(): error=" << error;