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 46dbbc95e4e0cc6999f121afe6f9f7dff515444a..4a362ac74fd4e324fcf060c4e84b562d1f22e765 100644
--- a/cc/resources/video_resource_updater.cc
+++ b/cc/resources/video_resource_updater.cc
@@ -245,6 +245,77 @@ static gfx::Size SoftwarePlaneDimension(
   return gfx::Size(plane_width, plane_height);
 }
 
+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->ref_count == 0 && !it->mailbox.IsZero() &&
+        it->resource_format == resource_format &&
+        it->resource_size == resource_size &&
+        immutable_hint ==
+            (resource_provider_->GetTextureHint(it->resource_id) ==
+             ResourceProvider::TEXTURE_HINT_IMMUTABLE)) {
+      return it;
+    }
+  }
+
+  return AllocateResource(resource_size, resource_format, true, immutable_hint);
+}
+
+// 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(
+    const scoped_refptr<media::VideoFrame>& video_frame) {

[dcheng, 2016/03/08 18:33:15]

Nit: this function doesn't appear to take ownership, so consider passing
|video_frame| as a raw pointer.

[Tobias Sargeant, 2016/03/09 11:50:58]

Done. Also changed the other private functions so that they are consistent.

+  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->ref_count;
+
+  size_t bytes_per_row = ResourceUtil::UncheckedWidthInBytes<size_t>(

[dcheng, 2016/03/08 18:33:15]

Not really related to this CL, but it's hard for me to see why the Unchecked
versions are safe here (I assume they're safe though, since they're already used
elsewhere in this file)

[Tobias Sargeant, 2016/03/09 11:50:58]

I agree that it is not obvious. This was copying the style from elsewhere in
this file. I assume that it's ok because video_frame must be in a consistent
state, with a correctly sized chunk of memory allocated for the frame data. If
we were going to overflow here, then that should have been impossible.

I couldn't see any advice in the discussion of the CL that introduced the other
instance. cc'ing prashant.n for comment.

+      output_plane_resource_size.width(), ResourceFormat::RGBA_8888);
+  size_t upload_image_stride =
+      MathUtil::UncheckedRoundUp<size_t>(bytes_per_row, 4u);
+  size_t needed_size =
+      upload_image_stride * output_plane_resource_size.height();
+  if (upload_pixels_.size() < needed_size)
+    upload_pixels_.resize(needed_size);
+
+  ResourceProvider::ScopedWriteLockGL lock(resource_provider_,
+                                           resource->resource_id);
+  media::SkCanvasVideoRenderer::ConvertVideoFrameToRGBPixels(
+      video_frame.get(), &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));
+  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(
     const scoped_refptr<media::VideoFrame>& video_frame) {
   TRACE_EVENT0("cc", "VideoResourceUpdater::CreateForSoftwarePlanes");
@@ -296,6 +367,11 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
   ResourceFormat output_resource_format =
       resource_provider_->YuvResourceFormat(bits_per_channel);
 
+  if (output_resource_format == ResourceFormat::RGBA_8888) {
+    // YUV frames must be converted to RGB before texture upload.
+    return CreateRGBTextureForSoftwarePlanes(video_frame);
+  }
+
   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
@@ -558,29 +634,11 @@ void VideoResourceUpdater::CopyPlaneTexture(
   // The copy needs to be a direct transfer of pixel data, so we use an RGBA8
   // target to avoid loss of precision or dropping any alpha component.
   const ResourceFormat copy_target_format = ResourceFormat::RGBA_8888;
+  const bool is_immutable = false;
 
   // 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->ref_count == 0 &&
-        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);
-  }
+  ResourceList::iterator resource = RecycleOrAllocateTexture(
+      output_plane_resource_size, copy_target_format, is_immutable);
 
   ++resource->ref_count;