color: Ensure that VideoResourceUpdater give consistent colors

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 428 matching lines @@
   ResourceFormat output_resource_format;
   if (input_frame_format == media::PIXEL_FORMAT_Y16) {
     // Unable to display directly as yuv planes so convert it to RGBA for
     // compositing.
     output_resource_format = RGBA_8888;
   } else {
     // Can be composited directly from yuv planes.
     output_resource_format =
         resource_provider_->YuvResourceFormat(bits_per_channel);
   }
+  gfx::ColorSpace output_color_space = video_frame->ColorSpace();
 
   // 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 || texture_needs_rgb_conversion) {
+    output_color_space = output_color_space.GetAsFullRangeRGB();
     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
@@ skipping 10 matching lines @@
         output_plane_resource_size.height() > max_resource_size) {
       // This output plane has invalid geometry. Clean up and return an empty
       // external resources.
       for (ResourceList::iterator resource_it : plane_resources)
         resource_it->remove_ref();
       return VideoFrameExternalResources();
     }
 
     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);
+        output_plane_resource_size, output_resource_format, output_color_space,
+        software_compositor, is_immutable, video_frame->unique_id(), i);
 
     resource_it->add_ref();
     plane_resources.push_back(resource_it);
   }
 
   VideoFrameExternalResources external_resources;
 
   external_resources.bits_per_channel = bits_per_channel;
 
   if (software_compositor || texture_needs_rgb_conversion) {
@@ skipping 36 matching lines @@
           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());
+      mailbox.set_color_space(output_color_space);
       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;
   }
 
   std::unique_ptr<HalfFloatMaker> half_float_maker;
   if (resource_provider_->YuvResourceFormat(bits_per_channel) ==
@@ skipping 81 matching lines @@
       resource_provider_->CopyToResource(plane_resource.resource_id(), pixels,
                                          resource_size_pixels);
       plane_resource.SetUniqueId(video_frame->unique_id(), i);
     }
 
     // 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());
+    mailbox.set_color_space(output_color_space);
     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;
   return external_resources;
 }
 
 // static
@@ skipping 12 matching lines @@
   // |video_frame|'s previous sync point using the current GL context.
   SyncTokenClientImpl client(updater->context_provider_->ContextGL(),
                              sync_token);
   video_frame->UpdateReleaseSyncToken(&client);
 }
 
 // Create a copy of a texture-backed source video frame in a new GL_TEXTURE_2D
 // texture.
 void VideoResourceUpdater::CopyPlaneTexture(
     media::VideoFrame* video_frame,
+    const gfx::ColorSpace& resource_color_space,
     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;
 
   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,
+                                resource_color_space, 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(
       mailbox_holder.texture_target, mailbox_holder.mailbox.name);
   gl->CopySubTextureCHROMIUM(
       src_texture_id, 0, GL_TEXTURE_2D, lock.texture_id(), 0, 0, 0, 0, 0,
       output_plane_resource_size.width(), output_plane_resource_size.height(),
       false, false, false);
   gl->DeleteTextures(1, &src_texture_id);
 
   // Done with the source video frame texture at this point.
   video_frame->UpdateReleaseSyncToken(&client);
 
   // VideoResourceUpdater shares a context with the compositor so a
   // sync token is not required.
   TextureMailbox mailbox(resource->mailbox(), gpu::SyncToken(), GL_TEXTURE_2D,
                          video_frame->coded_size(), false, false);
-  mailbox.set_color_space(video_frame->ColorSpace());
+  mailbox.set_color_space(resource_color_space);
   external_resources->mailboxes.push_back(mailbox);
 
   external_resources->release_callbacks.push_back(
       base::Bind(&RecycleResource, AsWeakPtr(), resource->resource_id()));
 }
 
 VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
     scoped_refptr<media::VideoFrame> video_frame) {
   TRACE_EVENT0("cc", "VideoResourceUpdater::CreateForHardwarePlanes");
   DCHECK(video_frame->HasTextures());
   if (!context_provider_)
     return VideoFrameExternalResources();
 
   VideoFrameExternalResources external_resources;
   if (video_frame->metadata()->IsTrue(
           media::VideoFrameMetadata::READ_LOCK_FENCES_ENABLED)) {
     external_resources.read_lock_fences_enabled = true;
   }
+  gfx::ColorSpace resource_color_space = video_frame->ColorSpace();
 
   external_resources.type = ResourceTypeForVideoFrame(video_frame.get());
   if (external_resources.type == VideoFrameExternalResources::NONE) {
     DLOG(ERROR) << "Unsupported Texture format"
                 << media::VideoPixelFormatToString(video_frame->format());
     return external_resources;
   }
+  if (external_resources.type == VideoFrameExternalResources::YUV_RESOURCE)
+    resource_color_space = resource_color_space.GetAsFullRangeRGB();

[hubbe, 2017/03/13 17:53:45]

If the YUV->RGB matrix applied by the hardware doesn't match the one requested
by the color space, we should correct for that by using custom primaries.

Not sure how we find out what matrix the hardware will be using though.
Maybe add a TODO somewhere?

Same thing applies in other places where you use GetAsFullRangeRGB().

[ccameron, 2017/03/13 22:16:55]

Yes, hardware-performed YUV->RGB at sample time will be harder to ensure
correctness on. Fortunately we've stopped doing this on Mac, but we may do it on
other platforms.

The thing that I have in mind is stuff that does YUV->RGB inside the command
buffer, via this structure
https://cs.chromium.org/chromium/src/ui/gl/yuv_to_rgb_converter.h?rcl=7d5e412...

For that case we can ensure that things match, if we send to the command buffer
the parameters of the transformation (which can be computed from a
gfx::ColorTransform).

 
   const size_t num_planes = media::VideoFrame::NumPlanes(video_frame->format());
   for (size_t i = 0; i < num_planes; ++i) {
     const gpu::MailboxHolder& mailbox_holder = video_frame->mailbox_holder(i);
     if (mailbox_holder.mailbox.IsZero())
       break;
 
     if (video_frame->metadata()->IsTrue(
             media::VideoFrameMetadata::COPY_REQUIRED)) {
-      CopyPlaneTexture(video_frame.get(), mailbox_holder, &external_resources);
+      CopyPlaneTexture(video_frame.get(), resource_color_space, mailbox_holder,
+                       &external_resources);
     } else {
       TextureMailbox mailbox(mailbox_holder.mailbox, mailbox_holder.sync_token,
                              mailbox_holder.texture_target,
                              video_frame->coded_size(),
                              video_frame->metadata()->IsTrue(
                                  media::VideoFrameMetadata::ALLOW_OVERLAY),
                              false);
-      mailbox.set_color_space(video_frame->ColorSpace());
+      mailbox.set_color_space(resource_color_space);
 #if defined(OS_ANDROID)
       mailbox.set_is_backed_by_surface_texture(video_frame->metadata()->IsTrue(
           media::VideoFrameMetadata::SURFACE_TEXTURE));
       mailbox.set_wants_promotion_hint(video_frame->metadata()->IsTrue(
           media::VideoFrameMetadata::WANTS_PROMOTION_HINT));
 #endif
       external_resources.mailboxes.push_back(mailbox);
       external_resources.release_callbacks.push_back(
           base::Bind(&ReturnTexture, AsWeakPtr(), video_frame));
     }
@@ skipping 29 matching lines @@
   if (lost_resource) {
     resource_it->clear_refs();
     updater->DeleteResource(resource_it);
     return;
   }
 
   resource_it->remove_ref();
 }
 
 }  // namespace cc