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

cc/resources/video_resource_updater.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/video_resource_updater.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 155 matching lines @@
     : context_provider_(context_provider),
       resource_provider_(resource_provider) {
 }
 
 VideoResourceUpdater::~VideoResourceUpdater() {
   for (const PlaneResource& plane_resource : all_resources_)
     resource_provider_->DeleteResource(plane_resource.resource_id());
 }
 
 VideoResourceUpdater::ResourceList::iterator
+VideoResourceUpdater::RecycleOrAllocateResource(
+    const gfx::Size& resource_size,
+    ResourceFormat resource_format,
+    const gfx::ColorSpace& color_space,
+    bool software_resource,
+    bool immutable_hint,
+    int unique_id,
+    int plane_index) {
+  ResourceList::iterator recyclable_resource = all_resources_.end();
+  for (auto it = all_resources_.begin(); it != all_resources_.end(); ++it) {
+    // If the plane index is valid (positive, or 0, meaning all planes)
+    // then we are allowed to return a referenced resource that already
+    // contains the right frame data. It's safe to reuse it even if
+    // resource_provider_ holds some references to it, because those
+    // references are read-only.
+    if (plane_index != -1 && it->Matches(unique_id, plane_index)) {
+      DCHECK(it->resource_size() == resource_size);
+      DCHECK(it->resource_format() == resource_format);
+      DCHECK(it->mailbox().IsZero() == software_resource);
+      return it;
+    }
+
+    // Otherwise check whether this is an unreferenced resource of the
+    // right format that we can recycle. Remember it, but don't return
+    // immediately, because we still want to find any reusable
+    // resources.
+    if (!it->has_refs() && it->resource_size() == resource_size &&
+        it->resource_format() == resource_format &&
+        it->mailbox().IsZero() == software_resource) {
+      if (software_resource) {
+        // This extra check is needed because resources backed by SharedMemory
+        // are not ref-counted, unlike mailboxes. Full discussion in
+        // codereview.chromium.org/145273021.
+        if (!resource_provider_->InUseByConsumer(it->resource_id()))
+          recyclable_resource = it;
+      } else if (immutable_hint ==
+                 (resource_provider_->GetTextureHint(it->resource_id()) ==

[danakj, 2016/09/12 22:06:53]

There's nothing innately GPU-only about this is there? The idea of
ResourceProvider is it mostly abstracts away gpu vs software things, so we
should try to not branch on that when dealing with ResourceProvider, so I feel
like we should do this in the software_resource case too (ie always do it).

[Tobias Sargeant, 2016/09/12 22:29:45]

What do you think of ResourceProvider::IsImmutable, which always returns true
for software resources, and uses GetTextureHint for GPU resources? Given that
is_immutable means that the GL texture can't be respecified, it seems like it
follows that all software resources are immutable.

[danakj, 2016/09/12 22:38:15]

I think they already are, since Resource constructors set them to immutable by
default. Who knows what fancy shared memory things we invent in the future
though.

+                  ResourceProvider::TEXTURE_HINT_IMMUTABLE)) {
+        recyclable_resource = it;
+      }
+    }
+  }
+
+  if (recyclable_resource != all_resources_.end())
+    return recyclable_resource;
+
+  // There was nothing available to reuse or recycle. Allocate a new resource.
+  return AllocateResource(resource_size, resource_format, color_space,
+                          !software_resource, immutable_hint);
+}
+
+VideoResourceUpdater::ResourceList::iterator
 VideoResourceUpdater::AllocateResource(const gfx::Size& plane_size,
                                        ResourceFormat format,
                                        const gfx::ColorSpace& color_space,
                                        bool has_mailbox,
                                        bool immutable_hint) {
   // TODO(danakj): Abstract out hw/sw resource create/delete from
   // ResourceProvider and stop using ResourceProvider in this class.
   const ResourceId resource_id = resource_provider_->CreateResource(
       plane_size, immutable_hint ? ResourceProvider::TEXTURE_HINT_IMMUTABLE
                                  : ResourceProvider::TEXTURE_HINT_DEFAULT,
       format, color_space);
-  if (resource_id == 0)
-    return all_resources_.end();
+  DCHECK_NE(resource_id, 0u);
 
   gpu::Mailbox mailbox;
   if (has_mailbox) {
     DCHECK(context_provider_);
 
     gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
 
     gl->GenMailboxCHROMIUM(mailbox.name);
     ResourceProvider::ScopedWriteLockGL lock(resource_provider_, resource_id,
                                              false);
@@ skipping 92 matching lines @@
   if (!media::IsYuvPlanar(input_frame_format)) {
     NOTREACHED() << media::VideoPixelFormatToString(input_frame_format);
     return VideoFrameExternalResources();
   }
 
   const bool software_compositor = context_provider_ == NULL;
 
   ResourceFormat output_resource_format =
       resource_provider_->YuvResourceFormat(bits_per_channel);
 
+  // If GPU compositing is enabled, but the output resource format
+  // returned by the resource provider is RGBA_8888, then a GPU driver
+  // bug workaround requires that YUV frames must be converted to RGB
+  // before texture upload.
+  bool texture_needs_rgb_conversion =
+      !software_compositor &&
+      output_resource_format == ResourceFormat::RGBA_8888;
   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
   // conversion here. That involves an extra copy of each frame to a bitmap.
   // Obviously, this is suboptimal and should be addressed once ubercompositor
   // starts shaping up.
-  if (software_compositor) {
+  if (software_compositor || texture_needs_rgb_conversion) {
     output_resource_format = kRGBResourceFormat;
     output_plane_count = 1;
+    bits_per_channel = 8;
   }
 
   // Drop recycled resources that are the wrong format.
   for (auto it = all_resources_.begin(); it != all_resources_.end();) {
     if (!it->has_refs() && it->resource_format() != output_resource_format)
       DeleteResource(it++);
     else
       ++it;
   }
 
   const int max_resource_size = resource_provider_->max_texture_size();
   std::vector<ResourceList::iterator> plane_resources;
   for (size_t i = 0; i < output_plane_count; ++i) {
     gfx::Size output_plane_resource_size =
         SoftwarePlaneDimension(video_frame.get(), software_compositor, i);
     if (output_plane_resource_size.IsEmpty() ||
         output_plane_resource_size.width() > max_resource_size ||
         output_plane_resource_size.height() > max_resource_size) {
       break;
     }
 
-    // Try recycle a previously-allocated resource.
-    ResourceList::iterator resource_it = all_resources_.end();
-    for (auto it = all_resources_.begin(); it != all_resources_.end(); ++it) {
-      if (it->resource_size() == output_plane_resource_size &&
-          it->resource_format() == output_resource_format) {
-        if (it->Matches(video_frame->unique_id(), i)) {
-          // Bingo, we found a resource that already contains the data we are
-          // planning to put in it. It's safe to reuse it even if
-          // resource_provider_ holds some references to it, because those
-          // references are read-only.
-          resource_it = it;
-          break;
-        }
-
-        // This extra check is needed because resources backed by SharedMemory
-        // are not ref-counted, unlike mailboxes. Full discussion in
-        // codereview.chromium.org/145273021.
-        const bool in_use =
-            software_compositor &&
-            resource_provider_->InUseByConsumer(it->resource_id());
-        if (!it->has_refs() && !in_use) {
-          // We found a resource with the correct size that we can overwrite.
-          resource_it = it;
-        }
-      }
-    }
-
-    // Check if we need to allocate a new resource.
-    if (resource_it == all_resources_.end()) {
-      const bool is_immutable = true;
-      resource_it = AllocateResource(
-          output_plane_resource_size, output_resource_format,
-          video_frame->ColorSpace(), !software_compositor, is_immutable);
-    }
-    if (resource_it == all_resources_.end())
-      break;
+    const bool is_immutable = true;
+    ResourceList::iterator resource_it = RecycleOrAllocateResource(
+        output_plane_resource_size, output_resource_format,
+        video_frame->ColorSpace(), software_compositor, is_immutable,
+        video_frame->unique_id(), i);
 
     resource_it->add_ref();
     plane_resources.push_back(resource_it);
   }
 
-  if (plane_resources.size() != output_plane_count) {
-    // Allocation failed, nothing will be returned so restore reference counts.
-    for (ResourceList::iterator resource_it : plane_resources)
-      resource_it->remove_ref();
-    return VideoFrameExternalResources();
-  }
-
   VideoFrameExternalResources external_resources;
 
   external_resources.bits_per_channel = bits_per_channel;
 
-  if (software_compositor) {
+  if (software_compositor || texture_needs_rgb_conversion) {
     DCHECK_EQ(plane_resources.size(), 1u);
     PlaneResource& plane_resource = *plane_resources[0];
     DCHECK_EQ(plane_resource.resource_format(), kRGBResourceFormat);
-    DCHECK(plane_resource.mailbox().IsZero());
+    DCHECK_EQ(software_compositor, plane_resource.mailbox().IsZero());
 
     if (!plane_resource.Matches(video_frame->unique_id(), 0)) {
       // We need to transfer data from |video_frame| to the plane resource.
-      if (!video_renderer_)
-        video_renderer_.reset(new media::SkCanvasVideoRenderer);
+      if (software_compositor) {
+        if (!video_renderer_)
+          video_renderer_.reset(new media::SkCanvasVideoRenderer);
 
-      ResourceProvider::ScopedWriteLockSoftware lock(
-          resource_provider_, plane_resource.resource_id());
-      SkCanvas canvas(lock.sk_bitmap());
-      // This is software path, so canvas and video_frame are always backed
-      // by software.
-      video_renderer_->Copy(video_frame, &canvas, media::Context3D());
+        ResourceProvider::ScopedWriteLockSoftware lock(
+            resource_provider_, plane_resource.resource_id());
+        SkCanvas canvas(lock.sk_bitmap());
+        // This is software path, so canvas and video_frame are always backed
+        // by software.
+        video_renderer_->Copy(video_frame, &canvas, media::Context3D());
+      } else {
+        size_t bytes_per_row = ResourceUtil::CheckedWidthInBytes<size_t>(
+            video_frame->coded_size().width(), ResourceFormat::RGBA_8888);
+        size_t needed_size = bytes_per_row * video_frame->coded_size().height();
+        if (upload_pixels_.size() < needed_size)
+          upload_pixels_.resize(needed_size);
+
+        media::SkCanvasVideoRenderer::ConvertVideoFrameToRGBPixels(
+            video_frame.get(), &upload_pixels_[0], bytes_per_row);
+
+        resource_provider_->CopyToResource(plane_resource.resource_id(),
+                                           &upload_pixels_[0],
+                                           plane_resource.resource_size());
+      }
       plane_resource.SetUniqueId(video_frame->unique_id(), 0);
     }
 
-    external_resources.software_resources.push_back(
-        plane_resource.resource_id());
-    external_resources.software_release_callback =
-        base::Bind(&RecycleResource, AsWeakPtr(), plane_resource.resource_id());
-    external_resources.type = VideoFrameExternalResources::SOFTWARE_RESOURCE;
+    if (software_compositor) {
+      external_resources.software_resources.push_back(
+          plane_resource.resource_id());
+      external_resources.software_release_callback = base::Bind(
+          &RecycleResource, AsWeakPtr(), plane_resource.resource_id());
+      external_resources.type = VideoFrameExternalResources::SOFTWARE_RESOURCE;
+    } else {
+      // VideoResourceUpdater shares a context with the compositor so
+      // a sync token is not required.
+      TextureMailbox mailbox(plane_resource.mailbox(), gpu::SyncToken(),
+                             resource_provider_->GetResourceTextureTarget(
+                                 plane_resource.resource_id()));
+      mailbox.set_color_space(video_frame->ColorSpace());
+      external_resources.mailboxes.push_back(mailbox);
+      external_resources.release_callbacks.push_back(base::Bind(
+          &RecycleResource, AsWeakPtr(), plane_resource.resource_id()));
+      external_resources.type = VideoFrameExternalResources::RGBA_RESOURCE;
+    }
     return external_resources;
   }
 
   for (size_t i = 0; i < plane_resources.size(); ++i) {
     PlaneResource& plane_resource = *plane_resources[i];
     // Update each plane's resource id with its content.
     DCHECK_EQ(plane_resource.resource_format(),
               resource_provider_->YuvResourceFormat(bits_per_channel));
 
     if (!plane_resource.Matches(video_frame->unique_id(), i)) {
       // We need to transfer data from |video_frame| to the plane resource.
       // TODO(reveman): Can use GpuMemoryBuffers here to improve performance.
 
       // The |resource_size_pixels| is the size of the resource we want to
       // upload to.
       gfx::Size resource_size_pixels = plane_resource.resource_size();
       // The |video_stride_bytes| is the width of the video frame we are
       // uploading (including non-frame data to fill in the stride).
       int video_stride_bytes = video_frame->stride(i);
 
-      size_t bytes_per_row = ResourceUtil::UncheckedWidthInBytes<size_t>(
+      size_t bytes_per_row = ResourceUtil::CheckedWidthInBytes<size_t>(
           resource_size_pixels.width(), plane_resource.resource_format());
       // Use 4-byte row alignment (OpenGL default) for upload performance.
       // Assuming that GL_UNPACK_ALIGNMENT has not changed from default.
       size_t upload_image_stride =
-          MathUtil::UncheckedRoundUp<size_t>(bytes_per_row, 4u);
+          MathUtil::CheckedRoundUp<size_t>(bytes_per_row, 4u);
 
       bool needs_conversion = false;
       int shift = 0;
 
       // LUMINANCE_F16 uses half-floats, so we always need a conversion step.
       if (plane_resource.resource_format() == LUMINANCE_F16) {
         needs_conversion = true;
         // Note that the current method of converting integers to half-floats
         // stops working if you have more than 10 bits of data.
         DCHECK_LE(bits_per_channel, 10);
@@ skipping 68 matching lines @@
       //
       // PLEASE NOTE: This doesn't work if bits_per_channel is > 10.
       // PLEASE NOTE: All planes are assumed to use the same multiplier/offset.
       external_resources.offset = 0.5f;
       // Max value from input data.
       int max_input_value = (1 << bits_per_channel) - 1;
       // 2 << 11 = 2048 would be 1.0 with our exponent.
       external_resources.multiplier = 2048.0 / max_input_value;
     }
 
+    // VideoResourceUpdater shares a context with the compositor so a
+    // sync token is not required.
     TextureMailbox mailbox(plane_resource.mailbox(), gpu::SyncToken(),
                            resource_provider_->GetResourceTextureTarget(
                                plane_resource.resource_id()));
     mailbox.set_color_space(video_frame->ColorSpace());
     external_resources.mailboxes.push_back(mailbox);
     external_resources.release_callbacks.push_back(base::Bind(
         &RecycleResource, AsWeakPtr(), plane_resource.resource_id()));
   }
 
   external_resources.type = VideoFrameExternalResources::YUV_RESOURCE;
@@ skipping 26 matching lines @@
     const gpu::MailboxHolder& mailbox_holder,
     VideoFrameExternalResources* external_resources) {
   gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
   SyncTokenClientImpl client(gl, mailbox_holder.sync_token);
 
   const gfx::Size output_plane_resource_size = video_frame->coded_size();
   // 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;
 
-  // 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,
-                                video_frame->ColorSpace(), true, is_immutable);
-  }
-
+  const bool is_immutable = false;
+  const int no_unique_id = 0;
+  const int no_plane_index = -1;  // Do not recycle referenced textures.
+  VideoResourceUpdater::ResourceList::iterator resource =
+      RecycleOrAllocateResource(output_plane_resource_size, copy_target_format,
+                                video_frame->ColorSpace(), false, is_immutable,
+                                no_unique_id, no_plane_index);
   resource->add_ref();
 
   ResourceProvider::ScopedWriteLockGL lock(resource_provider_,
                                            resource->resource_id(), false);
   DCHECK_EQ(
       resource_provider_->GetResourceTextureTarget(resource->resource_id()),
       (GLenum)GL_TEXTURE_2D);
 
   gl->WaitSyncTokenCHROMIUM(mailbox_holder.sync_token.GetConstData());
   uint32_t src_texture_id = gl->CreateAndConsumeTextureCHROMIUM(
@@ skipping 71 matching lines @@
 void VideoResourceUpdater::RecycleResource(
     base::WeakPtr<VideoResourceUpdater> updater,
     ResourceId resource_id,
     const gpu::SyncToken& sync_token,
     bool lost_resource,
     BlockingTaskRunner* main_thread_task_runner) {
   if (!updater.get()) {
     // Resource was already deleted.
     return;
   }
-
   const ResourceList::iterator resource_it = std::find_if(
       updater->all_resources_.begin(), updater->all_resources_.end(),
       [resource_id](const PlaneResource& plane_resource) {
         return plane_resource.resource_id() == resource_id;
       });
   if (resource_it == updater->all_resources_.end())
     return;
 
   ContextProvider* context_provider = updater->context_provider_;
   if (context_provider && sync_token.HasData()) {
     context_provider->ContextGL()->WaitSyncTokenCHROMIUM(
         sync_token.GetConstData());
   }
 
   if (lost_resource) {
     resource_it->clear_refs();
     updater->DeleteResource(resource_it);
     return;
   }
 
   resource_it->remove_ref();
 }
 
 }  // namespace cc