Avoid planar YUV resources when one component EGL images are not supported

cc/resources/video_resource_updater.cc

Index: cc/resources/video_resource_updater.cc
diff --git a/cc/resources/video_resource_updater.cc b/cc/resources/video_resource_updater.cc
index 3f943723c3785cd20dd073226b4aba27143841c8..5adeadd40ffb1ece584aeea3aeafd3d17f5312a8 100644
--- a/cc/resources/video_resource_updater.cc
+++ b/cc/resources/video_resource_updater.cc
@@ -172,6 +172,26 @@ VideoResourceUpdater::~VideoResourceUpdater() {
 }
 
 VideoResourceUpdater::ResourceList::iterator
+VideoResourceUpdater::RecycleOrAllocateTexture(const gfx::Size& resource_size,
+                                               ResourceFormat resource_format,
+                                               bool immutable_hint) {
+  for (auto it = all_resources_.begin(); it != all_resources_.end(); ++it) {
+    // Reuse resource if attributes match and the resource is a currently
+    // unreferenced texture.
+    if (it->resource_size() == resource_size &&
+        it->resource_format() == resource_format && !it->mailbox().IsZero() &&
+        !it->has_refs() &&
+        resource_provider_->GetTextureHint(it->resource_id()) !=
+            ResourceProvider::TEXTURE_HINT_IMMUTABLE) {
+      return it;
+    }
+  }
+
+  // Otherwise allocate a new resource.
+  return AllocateResource(resource_size, resource_format, true, immutable_hint);
+}
+
+VideoResourceUpdater::ResourceList::iterator
 VideoResourceUpdater::AllocateResource(const gfx::Size& plane_size,
                                        ResourceFormat format,
                                        bool has_mailbox,
@@ -245,6 +265,54 @@ static gfx::Size SoftwarePlaneDimension(media::VideoFrame* input_frame,
   return gfx::Size(plane_width, plane_height);
 }
 
+// Create an RGB texture by software converting YUV planar data, for the case
+// where YUV planar textures are not renderable by the GPU.
+VideoFrameExternalResources
+VideoResourceUpdater::CreateRGBTextureForSoftwarePlanes(
+    media::VideoFrame* video_frame) {
+  DCHECK(media::IsYuvPlanar(video_frame->format()));
+  gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+  const ResourceFormat resource_format = ResourceFormat::RGBA_8888;
+  const gfx::Size output_plane_resource_size = video_frame->coded_size();
+  const bool is_immutable = false;
+
+  VideoResourceUpdater::ResourceList::iterator resource =
+      RecycleOrAllocateTexture(output_plane_resource_size, resource_format,
+                               is_immutable);
+
+  resource->add_ref();
+
+  size_t bytes_per_row = ResourceUtil::UncheckedWidthInBytes<size_t>(
+      output_plane_resource_size.width(), ResourceFormat::RGBA_8888);
+  size_t upload_image_stride =
+      MathUtil::UncheckedRoundUp<size_t>(bytes_per_row, 4u);

[dcheng, 2016/08/12 22:50:47]

Sorry, I think I've asked this before, but can these calculations overflow? Does
it matter?

[Tobias Sargeant, 2016/08/15 09:33:04]

Ack. Commented in reply.

+  size_t needed_size =
+      upload_image_stride * output_plane_resource_size.height();
+  if (upload_pixels_.size() < needed_size)
+    upload_pixels_.resize(needed_size);
+
+  media::SkCanvasVideoRenderer::ConvertVideoFrameToRGBPixels(
+      video_frame, &upload_pixels_[0], upload_image_stride);
+
+  resource_provider_->CopyToResource(
+      resource->resource_id(), &upload_pixels_[0], resource->resource_size());
+
+  gpu::SyncToken sync_token;
+  const uint64_t fence_sync = gl->InsertFenceSyncCHROMIUM();
+  gl->ShallowFlushCHROMIUM();
+  gl->GenSyncTokenCHROMIUM(fence_sync, sync_token.GetData());
+
+  VideoFrameExternalResources external_resources;
+
+  external_resources.mailboxes.push_back(
+      TextureMailbox(resource->mailbox(), sync_token, GL_TEXTURE_2D,
+                     video_frame->coded_size(), false, false));
+  external_resources.release_callbacks.push_back(
+      base::Bind(&RecycleResource, AsWeakPtr(), resource->resource_id()));
+  external_resources.type = VideoFrameExternalResources::RGBA_RESOURCE;
+  return external_resources;
+}
+
 VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
     scoped_refptr<media::VideoFrame> video_frame) {
   TRACE_EVENT0("cc", "VideoResourceUpdater::CreateForSoftwarePlanes");
@@ -296,6 +364,13 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
   ResourceFormat output_resource_format =
       resource_provider_->YuvResourceFormat(bits_per_channel);
 
+  if (!software_compositor &&

[liberato (no reviews please), 2016/08/12 22:09:23]

is it possible to avoid switching the resource format to 8888 in the first place
when using the software compositor?

[Tobias Sargeant, 2016/08/15 09:33:04]

It will only be set to 8888 for GPU resources, so in a sense the
!software_compositor is redundant. I think I'd like to keep here though, for
readability, unless you think otherwise?

[liberato (no reviews please), 2016/08/16 14:55:46]

makes sense.  probably deserves a comment to that effect.

also, might be more appropriate as a DCHECK.  i don't understand the semantics
of a mismatch between |software_compositor| vs |output_resource_format| well
enough to say which is better.

[Tobias Sargeant, 2016/08/16 15:19:10]

I updated the comment to make the intent clearer.

+      output_resource_format == ResourceFormat::RGBA_8888) {
+    // If the output resource format is RGB, then YUV frames must be converted
+    // to RGB before texture upload.
+    return CreateRGBTextureForSoftwarePlanes(video_frame.get());
+  }
+
   size_t output_plane_count = media::VideoFrame::NumPlanes(input_frame_format);
 
   // TODO(skaslev): If we're in software compositing mode, we do the YUV -> RGB
@@ -563,28 +638,10 @@ void VideoResourceUpdater::CopyPlaneTexture(
   // target to avoid loss of precision or dropping any alpha component.
   const ResourceFormat copy_target_format = ResourceFormat::RGBA_8888;
 
-  // Search for an existing resource to reuse.
-  VideoResourceUpdater::ResourceList::iterator resource = all_resources_.end();
-
-  for (auto it = all_resources_.begin(); it != all_resources_.end(); ++it) {
-    // Reuse resource if attributes match and the resource is a currently
-    // unreferenced texture.
-    if (it->resource_size() == output_plane_resource_size &&
-        it->resource_format() == copy_target_format &&
-        !it->mailbox().IsZero() && !it->has_refs() &&
-        resource_provider_->GetTextureHint(it->resource_id()) !=
-            ResourceProvider::TEXTURE_HINT_IMMUTABLE) {
-      resource = it;
-      break;
-    }
-  }
-
-  // Otherwise allocate a new resource.
-  if (resource == all_resources_.end()) {
-    const bool is_immutable = false;
-    resource = AllocateResource(output_plane_resource_size, copy_target_format,
-                                true, is_immutable);
-  }
+  const bool is_immutable = false;
+  VideoResourceUpdater::ResourceList::iterator resource =
+      RecycleOrAllocateTexture(output_plane_resource_size, copy_target_format,
+                               is_immutable);
 
   resource->add_ref();