Plumb color space from MediaCodec to MCVD.

Bug: 1049192
Change-Id: I49ca6860718a2a015cfccf07150bd3aa4344b781
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042042
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740406}

media/base/android/java/src/org/chromium/media/MediaCodecBridge.java

@@ -200,6 +200,24 @@ class MediaCodecBridge {
         private int channelCount() {
             return mFormat.getInteger(MediaFormat.KEY_CHANNEL_COUNT);
         }
+
+        @CalledByNative("GetOutputFormatResult")
+        private int colorStandard() {
+            if (!mFormat.containsKey(MediaFormat.KEY_COLOR_STANDARD)) return -1;
+            return mFormat.getInteger(MediaFormat.KEY_COLOR_STANDARD);
+        }
+
+        @CalledByNative("GetOutputFormatResult")
+        private int colorRange() {
+            if (!mFormat.containsKey(MediaFormat.KEY_COLOR_RANGE)) return -1;
+            return mFormat.getInteger(MediaFormat.KEY_COLOR_RANGE);
+        }
+
+        @CalledByNative("GetOutputFormatResult")
+        private int colorTransfer() {
+            if (!mFormat.containsKey(MediaFormat.KEY_COLOR_TRANSFER)) return -1;
+            return mFormat.getInteger(MediaFormat.KEY_COLOR_TRANSFER);
+        }
     }
 
     // Warning: This class may execute on an arbitrary thread for the lifetime

media/base/android/media_codec_bridge.h

@@ -19,6 +19,7 @@
 #include "media/base/encryption_pattern.h"
 #include "media/base/encryption_scheme.h"
 #include "media/base/media_export.h"
+#include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace media {
@@ -75,6 +76,13 @@ class MEDIA_EXPORT MediaCodecBridge {
   // or MEDIA_CODEC_OK otherwise.
   virtual MediaCodecStatus GetOutputChannelCount(int* channel_count) = 0;
 
+  // Fills in |color_space| with the color space of the decoded video.  This
+  // is valid after DequeueOutputBuffer() signals a format change.  Will return
+  // MEDIA_CODEC_OK on success, with |color_space| initialized, or
+  // MEDIA_CODEC_ERROR with |color_space| unmodified otherwise.
+  virtual MediaCodecStatus GetOutputColorSpace(
+      gfx::ColorSpace* color_space) = 0;
+
   // Submits a byte array to the given input buffer. Call this after getting an
   // available buffer from DequeueInputBuffer(). If |data| is NULL, it assumes
   // the input buffer has already been populated (but still obeys |size|).

media/base/android/media_codec_bridge_impl.cc

@@ -357,6 +357,86 @@ MediaCodecStatus MediaCodecBridgeImpl::GetOutputChannelCount(
   return status;
 }
 
+MediaCodecStatus MediaCodecBridgeImpl::GetOutputColorSpace(
+    gfx::ColorSpace* color_space) {
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> result =
+      Java_MediaCodecBridge_getOutputFormat(env, j_bridge_);
+  MediaCodecStatus status = static_cast<MediaCodecStatus>(
+      Java_GetOutputFormatResult_status(env, result));
+  if (status != MEDIA_CODEC_OK)
+    return status;
+
+  // TODO(liberato): Consider consolidating these to save JNI hops.  However,
+  // since this is called only rarely, it's clearer this way.
+  int standard = Java_GetOutputFormatResult_colorStandard(env, result);
+  int range = Java_GetOutputFormatResult_colorRange(env, result);
+  int transfer = Java_GetOutputFormatResult_colorTransfer(env, result);
+  gfx::ColorSpace::PrimaryID primary_id;
+  gfx::ColorSpace::TransferID transfer_id;
+  gfx::ColorSpace::MatrixID matrix_id = gfx::ColorSpace::MatrixID::RGB;
+  gfx::ColorSpace::RangeID range_id;
+
+  switch (standard) {
+    case 1:  // MediaFormat.COLOR_STANDARD_BT709:
+      primary_id = gfx::ColorSpace::PrimaryID::BT709;
+      break;
+    case 2:  // MediaFormat.COLOR_STANDARD_BT601_PAL:
+      primary_id = gfx::ColorSpace::PrimaryID::BT470BG;
+      break;
+    case 4:  // MediaFormat.COLOR_STANDARD_BT601_NTSC:
+      primary_id = gfx::ColorSpace::PrimaryID::SMPTE170M;
+      break;
+    case 6:  // MediaFormat.COLOR_STANDARD_BT2020
+      primary_id = gfx::ColorSpace::PrimaryID::BT2020;
+      break;
+    default:
+      DVLOG(3) << __func__ << ": unsupported primary in p: " << standard
+               << " r: " << range << " t: " << transfer;
+      return MEDIA_CODEC_ERROR;
+  }
+
+  switch (transfer) {
+    case 1:  // MediaFormat.COLOR_TRANSFER_LINEAR
+      // TODO(liberato): LINEAR or LINEAR_HDR?
+      // Based on https://android.googlesource.com/platform/frameworks/native/
+      //            +/master/libs/nativewindow/include/android/data_space.h#57
+      // we pick LINEAR_HDR.
+      transfer_id = gfx::ColorSpace::TransferID::LINEAR_HDR;
+      break;
+    case 3:  // MediaFormat.COLOR_TRANSFER_SDR_VIDEO
+      transfer_id = gfx::ColorSpace::TransferID::SMPTE170M;
+      break;
+    case 6:  // MediaFormat.COLOR_TRANSFER_ST2084
+      transfer_id = gfx::ColorSpace::TransferID::SMPTEST2084;
+      break;
+    case 7:  // MediaFormat.COLOR_TRANSFER_HLG
+      transfer_id = gfx::ColorSpace::TransferID::ARIB_STD_B67;
+      break;
+    default:
+      DVLOG(3) << __func__ << ": unsupported transfer in p: " << standard
+               << " r: " << range << " t: " << transfer;
+      return MEDIA_CODEC_ERROR;
+  }
+
+  switch (range) {
+    case 1:  // MediaFormat.COLOR_RANGE_FULL
+      range_id = gfx::ColorSpace::RangeID::FULL;
+      break;
+    case 2:  // MediaFormat.COLOR_RANGE_LIMITED
+      range_id = gfx::ColorSpace::RangeID::LIMITED;
+      break;
+    default:
+      DVLOG(3) << __func__ << ": unsupported range in p: " << standard
+               << " r: " << range << " t: " << transfer;
+      return MEDIA_CODEC_ERROR;
+  }
+
+  *color_space = gfx::ColorSpace(primary_id, transfer_id, matrix_id, range_id);
+
+  return MEDIA_CODEC_OK;
+}
+
 MediaCodecStatus MediaCodecBridgeImpl::QueueInputBuffer(
     int index,
     const uint8_t* data,

media/base/android/media_codec_bridge_impl.h

@@ -110,6 +110,7 @@ class MEDIA_EXPORT MediaCodecBridgeImpl : public MediaCodecBridge {
   MediaCodecStatus GetOutputSize(gfx::Size* size) override;
   MediaCodecStatus GetOutputSamplingRate(int* sampling_rate) override;
   MediaCodecStatus GetOutputChannelCount(int* channel_count) override;
+  MediaCodecStatus GetOutputColorSpace(gfx::ColorSpace* color_space) override;
   MediaCodecStatus QueueInputBuffer(int index,
                                     const uint8_t* data,
                                     size_t data_size,

media/base/android/mock_media_codec_bridge.h

@@ -30,6 +30,8 @@ class MockMediaCodecBridge : public MediaCodecBridge,
   MOCK_METHOD1(GetOutputSize, MediaCodecStatus(gfx::Size* size));
   MOCK_METHOD1(GetOutputSamplingRate, MediaCodecStatus(int* sampling_rate));
   MOCK_METHOD1(GetOutputChannelCount, MediaCodecStatus(int* channel_count));
+  MOCK_METHOD1(GetOutputColorSpace,
+               MediaCodecStatus(gfx::ColorSpace* color_space));
   MOCK_METHOD4(QueueInputBuffer,
                MediaCodecStatus(int index,
                                 const uint8_t* data,

media/gpu/android/codec_wrapper.cc

@@ -97,6 +97,9 @@ class CodecWrapperImpl : public base::RefCountedThreadSafe<CodecWrapperImpl> {
   // while we're already flushed?
   bool elided_eos_pending_ = false;
 
+  // Most recently reported color space.
+  gfx::ColorSpace color_space_ = gfx::ColorSpace::CreateSRGB();
+
   // Task runner on which we'll release codec buffers without rendering.  May be
   // null to always do this on the calling task runner.
   scoped_refptr<base::SequencedTaskRunner> release_task_runner_;
@@ -106,12 +109,18 @@ class CodecWrapperImpl : public base::RefCountedThreadSafe<CodecWrapperImpl> {
 
 CodecOutputBuffer::CodecOutputBuffer(scoped_refptr<CodecWrapperImpl> codec,
                                      int64_t id,
-                                     const gfx::Size& size)
-    : codec_(std::move(codec)), id_(id), size_(size) {}
+                                     const gfx::Size& size,
+                                     const gfx::ColorSpace& color_space)
+    : codec_(std::move(codec)),
+      id_(id),
+      size_(size),
+      color_space_(color_space) {}
 
 // For testing.
-CodecOutputBuffer::CodecOutputBuffer(int64_t id, const gfx::Size& size)
-    : id_(id), size_(size) {}
+CodecOutputBuffer::CodecOutputBuffer(int64_t id,
+                                     const gfx::Size& size,
+                                     const gfx::ColorSpace& color_space)
+    : id_(id), size_(size), color_space_(color_space) {}
 
 CodecOutputBuffer::~CodecOutputBuffer() {
   // While it will work if we re-release the buffer, since CodecWrapper handles
@@ -331,8 +340,8 @@ CodecWrapperImpl::DequeueStatus CodecWrapperImpl::DequeueOutputBuffer(
 
         int64_t buffer_id = next_buffer_id_++;
         buffer_ids_[buffer_id] = index;
-        *codec_buffer =
-            base::WrapUnique(new CodecOutputBuffer(this, buffer_id, size_));
+        *codec_buffer = base::WrapUnique(
+            new CodecOutputBuffer(this, buffer_id, size_, color_space_));
         return DequeueStatus::kOk;
       }
       case MEDIA_CODEC_TRY_AGAIN_LATER: {
@@ -347,6 +356,12 @@ CodecWrapperImpl::DequeueStatus CodecWrapperImpl::DequeueOutputBuffer(
           state_ = State::kError;
           return DequeueStatus::kError;
         }
+        if (codec_->GetOutputColorSpace(&color_space_) == MEDIA_CODEC_ERROR) {
+          // If we get back an unsupported color space, then just default to
+          // sRGB for < 720p, or 709 otherwise.  It's better than nothing.
+          color_space_ = size_.width() >= 1280 ? gfx::ColorSpace::CreateREC709()
+                                               : gfx::ColorSpace::CreateSRGB();
+        }
         continue;
       }
       case MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED: {

media/gpu/android/codec_wrapper.h

@@ -44,6 +44,9 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer {
   // The size of the image.
   gfx::Size size() const { return size_; }
 
+  // Color space of the image.
+  const gfx::ColorSpace& color_space() const { return color_space_; }
+
   // Note that you can't use the first ctor, since CodecWrapperImpl isn't
   // defined here.  Use the second, and it'll be nullptr.
   template <typename... Args>
@@ -58,15 +61,19 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer {
   friend class CodecWrapperImpl;
   CodecOutputBuffer(scoped_refptr<CodecWrapperImpl> codec,
                     int64_t id,
-                    const gfx::Size& size);
+                    const gfx::Size& size,
+                    const gfx::ColorSpace& color_space);
 
   // For testing, since CodecWrapperImpl isn't available.  Uses nullptr.
-  CodecOutputBuffer(int64_t id, const gfx::Size& size);
+  CodecOutputBuffer(int64_t id,
+                    const gfx::Size& size,
+                    const gfx::ColorSpace& color_space);
 
   scoped_refptr<CodecWrapperImpl> codec_;
   int64_t id_;
   bool was_rendered_ = false;
   gfx::Size size_;
+  gfx::ColorSpace color_space_;
   DISALLOW_COPY_AND_ASSIGN(CodecOutputBuffer);
 };
 

media/gpu/android/codec_wrapper_unittest.cc

@@ -356,4 +356,29 @@ TEST_F(CodecWrapperTest, CodecWrapperPostsReleaseToProvidedThread) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST_F(CodecWrapperTest, CodecWrapperGetsColorSpaceFromCodec) {
+  // CodecWrapper should provide the color space that's reported by the bridge.
+  EXPECT_CALL(*codec_, DequeueOutputBuffer(_, _, _, _, _, _, _))
+      .WillOnce(Return(MEDIA_CODEC_OUTPUT_FORMAT_CHANGED))
+      .WillOnce(Return(MEDIA_CODEC_OK));
+  gfx::ColorSpace color_space{gfx::ColorSpace::CreateHDR10()};
+  EXPECT_CALL(*codec_, GetOutputColorSpace(_))
+      .WillOnce(DoAll(SetArgPointee<0>(color_space), Return(MEDIA_CODEC_OK)));
+  auto codec_buffer = DequeueCodecOutputBuffer();
+  ASSERT_EQ(codec_buffer->color_space(), color_space);
+}
+
+TEST_F(CodecWrapperTest, CodecWrapperDefaultsToSRGB) {
+  // If MediaCodec doesn't provide a color space, then CodecWrapper should
+  // default to sRGB for sanity.
+  // CodecWrapper should provide the color space that's reported by the bridge.
+  EXPECT_CALL(*codec_, DequeueOutputBuffer(_, _, _, _, _, _, _))
+      .WillOnce(Return(MEDIA_CODEC_OUTPUT_FORMAT_CHANGED))
+      .WillOnce(Return(MEDIA_CODEC_OK));
+  EXPECT_CALL(*codec_, GetOutputColorSpace(_))
+      .WillOnce(Return(MEDIA_CODEC_ERROR));
+  auto codec_buffer = DequeueCodecOutputBuffer();
+  ASSERT_EQ(codec_buffer->color_space(), gfx::ColorSpace::CreateSRGB());
+}
+
 }  // namespace media

media/gpu/android/direct_shared_image_video_provider.cc

@@ -237,14 +237,11 @@ bool GpuSharedImageVideoFactory::CreateImageInternal(
   }
 
   // Create a shared image.
-  // TODO(vikassoni): Hardcoding colorspace to SRGB. Figure how if media has a
-  // colorspace and wire it here.
   // TODO(vikassoni): This shared image need to be thread safe eventually for
   // webview to work with shared images.
   auto shared_image = std::make_unique<gpu::SharedImageVideo>(
-      mailbox, size, gfx::ColorSpace::CreateSRGB(), std::move(image),
-      std::move(texture), std::move(shared_context),
-      false /* is_thread_safe */);
+      mailbox, size, spec.color_space, std::move(image), std::move(texture),
+      std::move(shared_context), false /* is_thread_safe */);
 
   // Register it with shared image mailbox as well as legacy mailbox. This
   // keeps |shared_image| around until its destruction cb is called.

media/gpu/android/shared_image_video_provider.cc

@@ -15,7 +15,8 @@ SharedImageVideoProvider::ImageSpec::~ImageSpec() = default;
 
 bool SharedImageVideoProvider::ImageSpec::operator==(
     const ImageSpec& rhs) const {
-  return size == rhs.size && generation_id == rhs.generation_id;
+  return size == rhs.size && generation_id == rhs.generation_id &&
+         color_space == rhs.color_space;
 }
 
 bool SharedImageVideoProvider::ImageSpec::operator!=(

media/gpu/android/shared_image_video_provider.h

@@ -38,6 +38,9 @@ class MEDIA_GPU_EXPORT SharedImageVideoProvider {
     // Size of the underlying texture.
     gfx::Size size;
 
+    // Color space used for the SharedImage.
+    gfx::ColorSpace color_space;
+
     // This is a hack to allow us to discard pooled images if the TextureOwner
     // changes.  We don't want to keep a ref to the TextureOwner here, so we
     // just use a generation counter.  Note that this is temporary anyway; we

media/gpu/android/video_frame_factory_impl.cc

@@ -145,6 +145,7 @@ void VideoFrameFactoryImpl::CreateVideoFrame(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   gfx::Size coded_size = output_buffer->size();
+  const gfx::ColorSpace& color_space = output_buffer->color_space();
   gfx::Rect visible_rect(coded_size);
 
   // The pixel format doesn't matter here as long as it's valid for texture
@@ -165,8 +166,9 @@ void VideoFrameFactoryImpl::CreateVideoFrame(
     return;
   }
 
-  // Update the current spec to match the size.
+  // Update the current spec to match the size and color space.
   image_spec_.size = coded_size;
+  image_spec_.color_space = color_space;
 
   auto image_ready_cb = base::BindOnce(
       &VideoFrameFactoryImpl::CreateVideoFrame_OnImageReady,
@@ -202,6 +204,8 @@ void VideoFrameFactoryImpl::CreateVideoFrame_OnImageReady(
   if (!thiz)
     return;
 
+  gfx::ColorSpace color_space = output_buffer->color_space();
+
   // Initialize the CodecImage to use this output buffer.  Note that we're not
   // on the gpu main thread here, but it's okay since CodecImage is not being
   // used at this point.  Alternatively, we could post it, or hand it off to the
@@ -227,7 +231,7 @@ void VideoFrameFactoryImpl::CreateVideoFrame_OnImageReady(
       &VideoFrameFactoryImpl::CreateVideoFrame_Finish, thiz,
       std::move(output_cb), timestamp, coded_size, natural_size,
       std::move(codec_buffer_wait_coordinator), pixel_format, overlay_mode,
-      enable_threaded_texture_mailboxes, std::move(record));
+      enable_threaded_texture_mailboxes, color_space, std::move(record));
 
   // TODO(liberato): Use |ycbcr_helper_| as a signal about whether we're
   // supposed to get YCbCr info or not, rather than requiring the provider to
@@ -277,6 +281,7 @@ void VideoFrameFactoryImpl::CreateVideoFrame_Finish(
     VideoPixelFormat pixel_format,
     OverlayMode overlay_mode,
     bool enable_threaded_texture_mailboxes,
+    const gfx::ColorSpace& color_space,
     SharedImageVideoProvider::ImageRecord record) {
   gpu::MailboxHolder mailbox_holders[VideoFrame::kMaxPlanes];
   mailbox_holders[0] = gpu::MailboxHolder(record.mailbox, gpu::SyncToken(),
@@ -291,6 +296,8 @@ void VideoFrameFactoryImpl::CreateVideoFrame_Finish(
   // For Vulkan.
   frame->set_ycbcr_info(ycbcr_info_);
 
+  frame->set_color_space(color_space);
+
   // If, for some reason, we failed to create a frame, then fail.  Note that we
   // don't need to call |release_cb|; dropping it is okay since the api says so.
   if (!frame) {

media/gpu/android/video_frame_factory_impl.h

@@ -109,6 +109,7 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
       VideoPixelFormat pixel_format,
       OverlayMode overlay_mode,
       bool enable_threaded_texture_mailboxes,
+      const gfx::ColorSpace& color_space,
       SharedImageVideoProvider::ImageRecord record);
 
   MaybeRenderEarlyManager* mre_manager() const { return mre_manager_.get(); }

media/gpu/android/video_frame_factory_impl_unittest.cc

@@ -89,14 +89,20 @@ class VideoFrameFactoryImplTest : public testing::Test {
 
   ~VideoFrameFactoryImplTest() override = default;
 
+  struct {
+    gfx::Size coded_size{100, 100};
+    gfx::Rect visible_rect{coded_size};
+    gfx::Size natural_size{coded_size};
+    gfx::ColorSpace color_space{gfx::ColorSpace::CreateSCRGBLinear()};
+  } video_frame_params_;
+
   void RequestVideoFrame() {
-    gfx::Size coded_size(100, 100);
-    gfx::Rect visible_rect(coded_size);
-    gfx::Size natural_size(coded_size);
-    auto output_buffer = CodecOutputBuffer::CreateForTesting(0, coded_size);
-    ASSERT_TRUE(
-        VideoFrame::IsValidConfig(PIXEL_FORMAT_ARGB, VideoFrame::STORAGE_OPAQUE,
-                                  coded_size, visible_rect, natural_size));
+    auto output_buffer = CodecOutputBuffer::CreateForTesting(
+        0, video_frame_params_.coded_size, video_frame_params_.color_space);
+    ASSERT_TRUE(VideoFrame::IsValidConfig(
+        PIXEL_FORMAT_ARGB, VideoFrame::STORAGE_OPAQUE,
+        video_frame_params_.coded_size, video_frame_params_.visible_rect,
+        video_frame_params_.natural_size));
 
     // Save a copy in case the test wants it.
     output_buffer_raw_ = output_buffer.get();
@@ -108,10 +114,13 @@ class VideoFrameFactoryImplTest : public testing::Test {
     output_buffer_raw_ = output_buffer.get();
     EXPECT_CALL(*image_provider_raw_, MockRequestImage());
 
-    impl_->CreateVideoFrame(
-        std::move(output_buffer), base::TimeDelta(), natural_size,
-        PromotionHintAggregator::NotifyPromotionHintCB(), output_cb_.Get());
+    impl_->CreateVideoFrame(std::move(output_buffer), base::TimeDelta(),
+                            video_frame_params_.natural_size,
+                            PromotionHintAggregator::NotifyPromotionHintCB(),
+                            output_cb_.Get());
     base::RunLoop().RunUntilIdle();
+
+    // TODO(liberato): Verify that it requested a shared image.
   }
 
   // |release_cb_called_flag| will be set when the record's |release_cb| runs.
@@ -189,7 +198,8 @@ TEST_F(VideoFrameFactoryImplTest, CreateVideoFrameFailsIfUnsupportedFormat) {
   gfx::Size coded_size(limits::kMaxDimension + 1, limits::kMaxDimension + 1);
   gfx::Rect visible_rect(coded_size);
   gfx::Size natural_size(0, 0);
-  auto output_buffer = CodecOutputBuffer::CreateForTesting(0, coded_size);
+  auto output_buffer =
+      CodecOutputBuffer::CreateForTesting(0, coded_size, gfx::ColorSpace());
   ASSERT_FALSE(VideoFrame::IsValidConfig(PIXEL_FORMAT_ARGB,
                                          VideoFrame::STORAGE_OPAQUE, coded_size,
                                          visible_rect, natural_size));
@@ -227,6 +237,11 @@ TEST_F(VideoFrameFactoryImplTest, CreateVideoFrameSucceeds) {
   EXPECT_EQ(codec_image->get_codec_output_buffer_for_testing(),
             output_buffer_raw_);
 
+  EXPECT_EQ(frame->coded_size(), video_frame_params_.coded_size);
+  EXPECT_EQ(frame->natural_size(), video_frame_params_.natural_size);
+  EXPECT_EQ(frame->visible_rect(), video_frame_params_.visible_rect);
+  EXPECT_EQ(frame->ColorSpace(), video_frame_params_.color_space);
+
   // Destroy the VideoFrame, and verify that our release cb is called.
   EXPECT_FALSE(release_cb_called_flag);
   frame = nullptr;