Use coded_size in the CodecImage::GetSize

After https://crrev.com/c/2196922 we start using real coded size in
VideoFrame and SharedImageVideo. CodecImage was still using size from
CodecOutputBuffer which is visible size. This leads to validation errors
in gles2 decoder because we try to copy larger rect than CodecImage.

This CL changes CodecImage::GetSize to return coded size instead of
visible_size to be in line with VideoFrame and SharedImageVideo.

Bug: 1085359, 1076564
Change-Id: Icd92fb60e95cfbb9fa9ce916097c2b0984b13ebf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213261Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772434}

media/gpu/android/codec_image.cc

@@ -17,7 +17,7 @@
 
 namespace media {
 
-CodecImage::CodecImage() = default;
+CodecImage::CodecImage(const gfx::Size& coded_size) : coded_size_(coded_size) {}
 
 CodecImage::~CodecImage() {
   NotifyUnused();
@@ -51,11 +51,7 @@ void CodecImage::NotifyUnused() {
 }
 
 gfx::Size CodecImage::GetSize() {
-  // Return a nonzero size, to avoid GL errors, even if we dropped the codec
-  // buffer already.  Note that if we dropped it, there's no data in the
-  // texture anyway, so the old size doesn't matter.
-  return output_buffer_renderer_ ? output_buffer_renderer_->size()
-                                 : gfx::Size(1, 1);
+  return coded_size_;
 }
 
 unsigned CodecImage::GetInternalFormat() {

media/gpu/android/codec_image.h

@@ -46,7 +46,7 @@ class MEDIA_GPU_EXPORT CodecImage
   // destroying it.
   using UnusedCB = base::OnceCallback<void(CodecImage*)>;
 
-  CodecImage();
+  CodecImage(const gfx::Size& coded_size);
 
   // (Re-)Initialize this CodecImage to use |output_buffer| et. al.
   //
@@ -187,6 +187,9 @@ class MEDIA_GPU_EXPORT CodecImage
   // The bounds last sent to the overlay.
   gfx::Rect most_recent_bounds_;
 
+  // Coded size of the image.
+  gfx::Size coded_size_;
+
   // Callback to notify about promotion hints and overlay position.
   PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb_;
 

media/gpu/android/codec_image_group_unittest.cc

@@ -20,6 +20,9 @@
 namespace media {
 
 namespace {
+// Size used to create MockCodecImage.
+constexpr gfx::Size kMockImageSize(100, 100);
+
 // Subclass of CodecImageGroup which will notify us when it's destroyed.
 class CodecImageGroupWithDestructionHook : public CodecImageGroup {
  public:
@@ -119,7 +122,7 @@ TEST_F(CodecImageGroupTest, ImagesRetainRefToGroup) {
   bool was_destroyed = false;
   rec.image_group->SetDestructionCallback(
       base::BindOnce([](bool* flag) -> void { *flag = true; }, &was_destroyed));
-  scoped_refptr<CodecImage> image = new MockCodecImage();
+  scoped_refptr<CodecImage> image = new MockCodecImage(kMockImageSize);
   // We're supposed to call this from |gpu_task_runner_|, but all
   // CodecImageGroup really cares about is being single sequence.
   rec.image_group->AddCodecImage(image.get());
@@ -138,8 +141,8 @@ TEST_F(CodecImageGroupTest, ImageGroupDropsForwardsSurfaceDestruction) {
   // also verify that the image group drops its ref to the surface bundle, so
   // that it doesn't prevent destruction of the overlay that provided it.
   Record rec = CreateImageGroup();
-  scoped_refptr<MockCodecImage> image_1 = new MockCodecImage();
-  scoped_refptr<MockCodecImage> image_2 = new MockCodecImage();
+  scoped_refptr<MockCodecImage> image_1 = new MockCodecImage(kMockImageSize);
+  scoped_refptr<MockCodecImage> image_2 = new MockCodecImage(kMockImageSize);
   rec.image_group->AddCodecImage(image_1.get());
   rec.image_group->AddCodecImage(image_2.get());
 

media/gpu/android/codec_image_unittest.cc

@@ -92,7 +92,7 @@ class CodecImageTest : public testing::Test {
     auto buffer_renderer = std::make_unique<CodecOutputBufferRenderer>(
         std::move(buffer), codec_buffer_wait_coordinator);
 
-    scoped_refptr<CodecImage> image = new CodecImage();
+    scoped_refptr<CodecImage> image = new CodecImage(buffer_renderer->size());
     image->Initialize(
         std::move(buffer_renderer), codec_buffer_wait_coordinator,
         base::BindRepeating(&PromotionHintReceiver::OnPromotionHint,
@@ -383,4 +383,20 @@ TEST_F(CodecImageTest, RenderAfterUnusedDoesntCrash) {
       CodecImage::BindingsMode::kEnsureTexImageBound));
 }
 
+TEST_F(CodecImageTest, CodedSizeVsVisibleSize) {
+  const gfx::Size coded_size(128, 128);
+  const gfx::Size visible_size(100, 100);
+  auto buffer = CodecOutputBuffer::CreateForTesting(0, visible_size);
+  auto buffer_renderer =
+      std::make_unique<CodecOutputBufferRenderer>(std::move(buffer), nullptr);
+
+  scoped_refptr<CodecImage> image = new CodecImage(coded_size);
+  image->Initialize(std::move(buffer_renderer), nullptr,
+                    PromotionHintAggregator::NotifyPromotionHintCB());
+
+  // Verify that CodecImage::GetSize returns coded_size and not visible_size
+  // that comes in CodecOutputBuffer size.
+  EXPECT_EQ(image->GetSize(), coded_size);
+}
+
 }  // namespace media

media/gpu/android/direct_shared_image_video_provider.cc

@@ -148,7 +148,7 @@ void GpuSharedImageVideoFactory::CreateImage(
 
   // Generate a shared image mailbox.
   auto mailbox = gpu::Mailbox::GenerateForSharedImage();
-  auto codec_image = base::MakeRefCounted<CodecImage>();
+  auto codec_image = base::MakeRefCounted<CodecImage>(spec.coded_size);
 
   TRACE_EVENT0("media", "GpuSharedImageVideoFactory::CreateVideoFrame");
 
@@ -200,14 +200,14 @@ bool GpuSharedImageVideoFactory::CreateImageInternal(
   if (!group)
     return false;
 
-  const auto& size = spec.size;
+  const auto& coded_size = spec.coded_size;
 
   // Create a Texture and a CodecImage to back it.
   // TODO(liberato): Once legacy mailbox support is removed, we don't need to
   // create this texture.  So, we won't need |texture_owner| either.
   std::unique_ptr<AbstractTexture> texture = decoder_helper_->CreateTexture(
-      GL_TEXTURE_EXTERNAL_OES, GL_RGBA, size.width(), size.height(), GL_RGBA,
-      GL_UNSIGNED_BYTE);
+      GL_TEXTURE_EXTERNAL_OES, GL_RGBA, coded_size.width(), coded_size.height(),
+      GL_RGBA, GL_UNSIGNED_BYTE);
 
   // Attach the image to the texture.
   // Either way, we expect this to be UNBOUND (i.e., decoder-managed).  For
@@ -239,7 +239,7 @@ bool GpuSharedImageVideoFactory::CreateImageInternal(
   // 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),
+      mailbox, coded_size, gfx::ColorSpace::CreateSRGB(), std::move(image),
       std::move(texture), std::move(shared_context),
       false /* is_thread_safe */);
 

media/gpu/android/mock_codec_image.cc

@@ -6,7 +6,8 @@
 
 namespace media {
 
-MockCodecImage::MockCodecImage() = default;
+MockCodecImage::MockCodecImage(const gfx::Size& coded_size)
+    : CodecImage(coded_size) {}
 
 MockCodecImage::~MockCodecImage() = default;
 

media/gpu/android/mock_codec_image.h

@@ -15,7 +15,7 @@ namespace media {
 // CodecImage with a mocked ReleaseCodecBuffer.
 class MockCodecImage : public CodecImage {
  public:
-  MockCodecImage();
+  MockCodecImage(const gfx::Size& coded_size);
 
   MOCK_METHOD0(ReleaseCodecBuffer, void());
 

media/gpu/android/shared_image_video_provider.cc

@@ -9,13 +9,13 @@ namespace media {
 SharedImageVideoProvider::ImageSpec::ImageSpec() = default;
 SharedImageVideoProvider::ImageSpec::ImageSpec(const gfx::Size& our_size,
                                                uint64_t our_generation_id)
-    : size(our_size), generation_id(our_generation_id) {}
+    : coded_size(our_size), generation_id(our_generation_id) {}
 SharedImageVideoProvider::ImageSpec::ImageSpec(const ImageSpec&) = default;
 SharedImageVideoProvider::ImageSpec::~ImageSpec() = default;
 
 bool SharedImageVideoProvider::ImageSpec::operator==(
     const ImageSpec& rhs) const {
-  return size == rhs.size && generation_id == rhs.generation_id;
+  return coded_size == rhs.coded_size && generation_id == rhs.generation_id;
 }
 
 bool SharedImageVideoProvider::ImageSpec::operator!=(

media/gpu/android/shared_image_video_provider.h

@@ -31,12 +31,12 @@ class MEDIA_GPU_EXPORT SharedImageVideoProvider {
   // Description of the underlying properties of the shared image.
   struct ImageSpec {
     ImageSpec();
-    ImageSpec(const gfx::Size& size, uint64_t generation_id);
+    ImageSpec(const gfx::Size& coded_size, uint64_t generation_id);
     ImageSpec(const ImageSpec&);
     ~ImageSpec();
 
     // Size of the underlying texture.
-    gfx::Size size;
+    gfx::Size coded_size;
 
     // 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

media/gpu/android/video_frame_factory_impl.cc

@@ -229,7 +229,7 @@ void VideoFrameFactoryImpl::CreateVideoFrame_OnFrameInfoReady(
   // just want to ask for the same image spec as before to order callback after
   // all RequestImage, so skip updating image_spec_ in this case.
   if (output_buffer_renderer)
-    image_spec_.size = frame_info.coded_size;
+    image_spec_.coded_size = frame_info.coded_size;
 
   auto cb = base::BindOnce(std::move(image_ready_cb),
                            std::move(output_buffer_renderer), frame_info);

media/gpu/android/video_frame_factory_impl_unittest.cc

@@ -156,7 +156,8 @@ class VideoFrameFactoryImplTest : public testing::Test {
             *flag = true;
         },
         base::Unretained(release_cb_called_flag));
-    auto codec_image = base::MakeRefCounted<MockCodecImage>();
+    auto codec_image =
+        base::MakeRefCounted<MockCodecImage>(gfx::Size(100, 100));
     record.codec_image_holder =
         base::MakeRefCounted<CodecImageHolder>(task_runner_, codec_image);
     return record;