convert CopyRowsToRGB10Buffer to use gfx::ColorSpace

Also fixes the logic. Before it the code would default
to rec601, and because of a bug it would *always* use
the default.

Now it defaults to rec709, unless rec601 is specified.

Bug: 754986
Change-Id: I94773a3bf0dfeae18e6a2ee8bc36ca884ba590b5
Reviewed-on: https://chromium-review.googlesource.com/998842
Commit-Queue: Fredrik Hubinette <hubbe@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555606}

media/video/gpu_memory_buffer_video_frame_pool.cc

@@ -32,6 +32,7 @@
 #include "media/video/gpu_video_accelerator_factories.h"
 #include "third_party/libyuv/include/libyuv.h"
 #include "ui/gfx/buffer_format_util.h"
+#include "ui/gfx/color_space.h"
 #include "ui/gl/trace_util.h"
 
 namespace media {
@@ -464,7 +465,6 @@ void CopyRowsToRGB10Buffer(bool is_argb,
   base::ScopedClosureRunner done_runner(std::move(done));
   TRACE_EVENT2("media", "CopyRowsToXR30Buffer", "bytes_per_row", width * 2,
                "rows", rows);
-
   if (!output)
     return;
 
@@ -472,11 +472,6 @@ void CopyRowsToRGB10Buffer(bool is_argb,
   DCHECK_LE(width, std::abs(dest_stride / 2));
   DCHECK_EQ(0, first_row % 2);
 
-  int color_space = COLOR_SPACE_UNSPECIFIED;
-  if (source_frame->metadata()->GetInteger(VideoFrameMetadata::COLOR_SPACE,
-                                           &color_space)) {
-    color_space = COLOR_SPACE_UNSPECIFIED;
-  }
   const uint16_t* y_plane = reinterpret_cast<const uint16_t*>(
       source_frame->visible_data(VideoFrame::kYPlane) +
       first_row * source_frame->stride(VideoFrame::kYPlane));
@@ -489,33 +484,31 @@ void CopyRowsToRGB10Buffer(bool is_argb,
       source_frame->visible_data(VideoFrame::kUPlane) +
       first_row / 2 * source_frame->stride(VideoFrame::kUPlane));
   const size_t u_plane_stride = source_frame->stride(VideoFrame::kUPlane) / 2;
-  uint8_t* dest_ar30 = output + first_row * dest_stride;
-
-  switch (color_space) {
-    case COLOR_SPACE_HD_REC709:
-      if (is_argb) {
-        libyuv::H010ToAR30(y_plane, y_plane_stride, u_plane, u_plane_stride,
-                           v_plane, v_plane_stride, dest_ar30, dest_stride,
-                           width, rows);
-      } else {
-        libyuv::H010ToAB30(y_plane, y_plane_stride, u_plane, u_plane_stride,
-                           v_plane, v_plane_stride, dest_ar30, dest_stride,
-                           width, rows);
-      }
-      break;
-    case COLOR_SPACE_UNSPECIFIED:
-    case COLOR_SPACE_JPEG:
-    case COLOR_SPACE_SD_REC601:
-      if (is_argb) {
-        libyuv::I010ToAR30(y_plane, y_plane_stride, u_plane, u_plane_stride,
-                           v_plane, v_plane_stride, dest_ar30, dest_stride,
-                           width, rows);
-      } else {
-        libyuv::I010ToAB30(y_plane, y_plane_stride, u_plane, u_plane_stride,
-                           v_plane, v_plane_stride, dest_ar30, dest_stride,
-                           width, rows);
-      }
-      break;
+  uint8_t* dest_rgb10 = output + first_row * dest_stride;
+
+  SkYUVColorSpace skyuv = kRec709_SkYUVColorSpace;
+  source_frame->ColorSpace().ToSkYUVColorSpace(&skyuv);
+
+  if (skyuv == kRec601_SkYUVColorSpace) {
+    if (is_argb) {
+      libyuv::I010ToAR30(y_plane, y_plane_stride, u_plane, u_plane_stride,
+                         v_plane, v_plane_stride, dest_rgb10, dest_stride,
+                         width, rows);
+    } else {
+      libyuv::I010ToAB30(y_plane, y_plane_stride, u_plane, u_plane_stride,
+                         v_plane, v_plane_stride, dest_rgb10, dest_stride,
+                         width, rows);
+    }
+  } else {
+    if (is_argb) {
+      libyuv::H010ToAR30(y_plane, y_plane_stride, u_plane, u_plane_stride,
+                         v_plane, v_plane_stride, dest_rgb10, dest_stride,
+                         width, rows);
+    } else {
+      libyuv::H010ToAB30(y_plane, y_plane_stride, u_plane, u_plane_stride,
+                         v_plane, v_plane_stride, dest_rgb10, dest_stride,
+                         width, rows);
+    }
   }
 }
 
@@ -910,7 +903,13 @@ void GpuMemoryBufferVideoFramePool::PoolImpl::
 #endif
       break;
     case GpuVideoAcceleratorFactories::OutputFormat::XR30:
+    case GpuVideoAcceleratorFactories::OutputFormat::XB30:
       allow_overlay = true;
+      // We've converted the YUV to RGB, fix the color space.
+      // TODO(hubbe): The libyuv YUV to RGB conversion may not have
+      // honored the color space conversion 100%. We should either fix
+      // libyuv or find a way for later passes to make up the difference.
+      frame->set_color_space(video_frame->ColorSpace().GetAsRGB());
       break;
     default:
       break;

media/video/gpu_memory_buffer_video_frame_pool_unittest.cc

@@ -128,6 +128,15 @@ class GpuMemoryBufferVideoFramePoolTest : public ::testing::Test {
     return video_frame;
   }
 
+  // Note, the X portion is set to 1 since it may use ARGB instead of
+  // XRGB on some platforms.
+  uint32_t as_xr30(uint32_t r, uint32_t g, uint32_t b) {
+    return (3 << 30) | (r << 20) | (g << 10) | b;
+  }
+  uint32_t as_xb30(uint32_t r, uint32_t g, uint32_t b) {
+    return (3 << 30) | (b << 20) | (g << 10) | r;
+  }
+
  protected:
   base::SimpleTestTickClock test_clock_;
   std::unique_ptr<MockGpuVideoAcceleratorFactories> mock_gpu_factories_;
@@ -331,6 +340,62 @@ TEST_F(GpuMemoryBufferVideoFramePoolTest, CreateOneHardwareXR30Frame) {
   EXPECT_EQ(1u, gles2_->gen_textures_count());
   EXPECT_TRUE(frame->metadata()->IsTrue(
       media::VideoFrameMetadata::READ_LOCK_FENCES_ENABLED));
+
+  EXPECT_EQ(1u, mock_gpu_factories_->created_memory_buffers().size());
+  mock_gpu_factories_->created_memory_buffers()[0]->Map();
+
+  void* memory = mock_gpu_factories_->created_memory_buffers()[0]->memory(0);
+  EXPECT_EQ(as_xr30(0, 311, 0), *static_cast<uint32_t*>(memory));
+}
+
+TEST_F(GpuMemoryBufferVideoFramePoolTest, CreateOneHardwareXR30FrameBT709) {
+  scoped_refptr<VideoFrame> software_frame = CreateTestYUVVideoFrame(10, 10);
+  software_frame->set_color_space(gfx::ColorSpace::CreateREC709());
+  scoped_refptr<VideoFrame> frame;
+  mock_gpu_factories_->SetVideoFrameOutputFormat(
+      media::GpuVideoAcceleratorFactories::OutputFormat::XR30);
+  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
+      software_frame, base::BindOnce(MaybeCreateHardwareFrameCallback, &frame));
+
+  RunUntilIdle();
+
+  EXPECT_NE(software_frame.get(), frame.get());
+  EXPECT_EQ(PIXEL_FORMAT_ARGB, frame->format());
+  EXPECT_EQ(1u, frame->NumTextures());
+  EXPECT_EQ(1u, gles2_->gen_textures_count());
+  EXPECT_TRUE(frame->metadata()->IsTrue(
+      media::VideoFrameMetadata::READ_LOCK_FENCES_ENABLED));
+
+  EXPECT_EQ(1u, mock_gpu_factories_->created_memory_buffers().size());
+  mock_gpu_factories_->created_memory_buffers()[0]->Map();
+
+  void* memory = mock_gpu_factories_->created_memory_buffers()[0]->memory(0);
+  EXPECT_EQ(as_xr30(0, 311, 0), *static_cast<uint32_t*>(memory));
+}
+
+TEST_F(GpuMemoryBufferVideoFramePoolTest, CreateOneHardwareXR30FrameBT601) {
+  scoped_refptr<VideoFrame> software_frame = CreateTestYUVVideoFrame(10, 10);
+  software_frame->set_color_space(gfx::ColorSpace::CreateREC601());
+  scoped_refptr<VideoFrame> frame;
+  mock_gpu_factories_->SetVideoFrameOutputFormat(
+      media::GpuVideoAcceleratorFactories::OutputFormat::XR30);
+  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
+      software_frame, base::BindOnce(MaybeCreateHardwareFrameCallback, &frame));
+
+  RunUntilIdle();
+
+  EXPECT_NE(software_frame.get(), frame.get());
+  EXPECT_EQ(PIXEL_FORMAT_ARGB, frame->format());
+  EXPECT_EQ(1u, frame->NumTextures());
+  EXPECT_EQ(1u, gles2_->gen_textures_count());
+  EXPECT_TRUE(frame->metadata()->IsTrue(
+      media::VideoFrameMetadata::READ_LOCK_FENCES_ENABLED));
+
+  EXPECT_EQ(1u, mock_gpu_factories_->created_memory_buffers().size());
+  mock_gpu_factories_->created_memory_buffers()[0]->Map();
+
+  void* memory = mock_gpu_factories_->created_memory_buffers()[0]->memory(0);
+  EXPECT_EQ(as_xr30(0, 543, 0), *static_cast<uint32_t*>(memory));
 }
 
 TEST_F(GpuMemoryBufferVideoFramePoolTest, CreateOneHardwareXB30Frame) {

media/video/mock_gpu_video_accelerator_factories.cc

@@ -99,7 +99,10 @@ MockGpuVideoAcceleratorFactories::CreateGpuMemoryBuffer(
     gfx::BufferUsage /* usage */) {
   if (fail_to_allocate_gpu_memory_buffer_)
     return nullptr;
-  return std::make_unique<GpuMemoryBufferImpl>(size, format);
+  std::unique_ptr<gfx::GpuMemoryBuffer> ret(
+      new GpuMemoryBufferImpl(size, format));
+  created_memory_buffers_.push_back(ret.get());
+  return ret;
 }
 
 std::unique_ptr<base::SharedMemory>

media/video/mock_gpu_video_accelerator_factories.h

@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #include <memory>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -91,6 +92,10 @@ class MockGpuVideoAcceleratorFactories : public GpuVideoAcceleratorFactories {
 
   gpu::gles2::GLES2Interface* GetGLES2Interface() { return gles2_; }
 
+  const std::vector<gfx::GpuMemoryBuffer*>& created_memory_buffers() {
+    return created_memory_buffers_;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MockGpuVideoAcceleratorFactories);
 
@@ -100,6 +105,8 @@ class MockGpuVideoAcceleratorFactories : public GpuVideoAcceleratorFactories {
   bool fail_to_allocate_gpu_memory_buffer_ = false;
 
   gpu::gles2::GLES2Interface* gles2_;
+
+  std::vector<gfx::GpuMemoryBuffer*> created_memory_buffers_;
 };
 
 }  // namespace media

ui/gfx/color_space.cc

@@ -423,6 +423,13 @@ ColorSpace ColorSpace::GetAsFullRangeRGB() const {
   return result;
 }
 
+ColorSpace ColorSpace::GetAsRGB() const {
+  ColorSpace result(*this);
+  if (IsValid())
+    result.matrix_ = MatrixID::RGB;
+  return result;
+}
+
 ColorSpace ColorSpace::GetRasterColorSpace() const {
   // Rasterization can only be done into parametric color spaces.
   if (icc_profile_id_)

ui/gfx/color_space.h

@@ -179,6 +179,9 @@ class COLOR_SPACE_EXPORT ColorSpace {
   // parametric).
   ColorSpace GetParametricApproximation() const;
 
+  // Return this color space with any YUV to RGB conversion stripped off.
+  ColorSpace GetAsRGB() const;
+
   // Return this color space with any range adjust or YUV to RGB conversion
   // stripped off.
   ColorSpace GetAsFullRangeRGB() const;