Get coded size for MediaCodec path

This CL modifies MediaCodec path to get use real coded_size and
visible_rect in VideoFrame instead of relying on texture matrix
transform.

To achieve this we utilize similar approach as in YCbCrHelper.
Helper is renamed to FrameInfoHelper. During CreateVideoFrame
VideoFrameFactoryImpl will check if it has cached FrameInfo for
current visible size that comes from media codec. If not it
will post task to FrameInfoHelper to get it. To avoid
unnecessary frame renders FrameInfoHelper also does similar
caching. If info is not cached it will Render frame early to
get the info.

To be able render frame before creation CodecImage and
SharedImageVideo the render logic is extracted from CodecImage
to CodecOutputBufferRenderer.

Bug: 1076564
Change-Id: I5baac124f46d78a89ae4300d5a01a9b9ab04af2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2196922Reviewed-by: Jonathan Backer <backer@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769028}

gpu/command_buffer/service/shared_image_video.cc

@@ -103,7 +103,7 @@ void SharedImageVideo::OnContextLost() {
 }
 
 base::Optional<VulkanYCbCrInfo> SharedImageVideo::GetYcbcrInfo(
-    StreamTextureSharedImageInterface* stream_texture_sii,
+    TextureOwner* texture_owner,
     scoped_refptr<SharedContextState> context_state) {
   // For non-vulkan context, return null.
   if (!context_state->GrContextIsVulkan())
@@ -111,7 +111,7 @@ base::Optional<VulkanYCbCrInfo> SharedImageVideo::GetYcbcrInfo(
 
   // GetAHardwareBuffer() renders the latest image and gets AHardwareBuffer
   // from it.
-  auto scoped_hardware_buffer = stream_texture_sii->GetAHardwareBuffer();
+  auto scoped_hardware_buffer = texture_owner->GetAHardwareBuffer();
   if (!scoped_hardware_buffer) {
     return base::nullopt;
   }

gpu/command_buffer/service/shared_image_video.h

@@ -54,7 +54,7 @@ class GPU_GLES2_EXPORT SharedImageVideo
   // Returns ycbcr information. This is only valid in vulkan context and
   // nullopt for other context.
   static base::Optional<VulkanYCbCrInfo> GetYcbcrInfo(
-      StreamTextureSharedImageInterface* stream_texture_sii,
+      TextureOwner* texture_owner,
       scoped_refptr<SharedContextState> context_state);
 
  protected:

gpu/command_buffer/service/stream_texture_shared_image_interface.h

@@ -17,6 +17,20 @@ class TextureBase;
 class GPU_GLES2_EXPORT StreamTextureSharedImageInterface
     : public gles2::GLStreamTextureImage {
  public:
+  enum class BindingsMode {
+    // Ensures that the TextureOwner's texture is bound to the latest image, if
+    // it requires explicit binding.
+    kEnsureTexImageBound,
+
+    // Updates the current image but does not bind it. If updating the image
+    // implicitly binds the texture, the current bindings will be restored.
+    kRestoreIfBound,
+
+    // Updates the current image but does not bind it. If updating the image
+    // implicitly binds the texture, the current bindings will not be restored.
+    kDontRestoreIfBound
+  };
+
   // Release the underlying resources. This should be called when the image is
   // not longer valid or the context is lost.
   virtual void ReleaseResources() = 0;
@@ -41,20 +55,6 @@ class GPU_GLES2_EXPORT StreamTextureSharedImageInterface
 
  protected:
   ~StreamTextureSharedImageInterface() override = default;
-
-  enum class BindingsMode {
-    // Ensures that the TextureOwner's texture is bound to the latest image, if
-    // it requires explicit binding.
-    kEnsureTexImageBound,
-
-    // Updates the current image but does not bind it. If updating the image
-    // implicitly binds the texture, the current bindings will be restored.
-    kRestoreIfBound,
-
-    // Updates the current image but does not bind it. If updating the image
-    // implicitly binds the texture, the current bindings will not be restored.
-    kDontRestoreIfBound
-  };
 };
 
 }  // namespace gpu

gpu/ipc/service/stream_texture_android.cc

@@ -239,7 +239,8 @@ void StreamTexture::OnFrameAvailable() {
     visible_rect_ = visible_rect;
 
     auto mailbox = CreateSharedImage(coded_size);
-    auto ycbcr_info = SharedImageVideo::GetYcbcrInfo(this, context_state_);
+    auto ycbcr_info =
+        SharedImageVideo::GetYcbcrInfo(texture_owner_.get(), context_state_);
 
     channel_->Send(new GpuStreamTextureMsg_FrameWithInfoAvailable(
         route_id_, mailbox, coded_size, visible_rect, ycbcr_info));

media/gpu/BUILD.gn

@@ -109,6 +109,8 @@ component("gpu") {
       "android/codec_image.h",
       "android/codec_image_group.cc",
       "android/codec_image_group.h",
+      "android/codec_output_buffer_renderer.cc",
+      "android/codec_output_buffer_renderer.h",
       "android/codec_surface_bundle.cc",
       "android/codec_surface_bundle.h",
       "android/codec_wrapper.cc",
@@ -117,6 +119,8 @@ component("gpu") {
       "android/device_info.h",
       "android/direct_shared_image_video_provider.cc",
       "android/direct_shared_image_video_provider.h",
+      "android/frame_info_helper.cc",
+      "android/frame_info_helper.h",
       "android/maybe_render_early_manager.cc",
       "android/maybe_render_early_manager.h",
       "android/media_codec_video_decoder.cc",
@@ -133,8 +137,6 @@ component("gpu") {
       "android/video_frame_factory.h",
       "android/video_frame_factory_impl.cc",
       "android/video_frame_factory_impl.h",
-      "android/ycbcr_helper.cc",
-      "android/ycbcr_helper.h",
     ]
     libs += [ "android" ]
     deps += [

media/gpu/android/codec_image.cc

@@ -16,31 +16,6 @@
 #include "ui/gl/scoped_make_current.h"
 
 namespace media {
-namespace {
-
-// Makes |texture_owner|'s context current if it isn't already.
-std::unique_ptr<ui::ScopedMakeCurrent> MakeCurrentIfNeeded(
-    gpu::TextureOwner* texture_owner) {
-  gl::GLContext* context = texture_owner->GetContext();
-  // Note: this works for virtual contexts too, because IsCurrent() returns true
-  // if their shared platform context is current, regardless of which virtual
-  // context is current.
-  if (context->IsCurrent(nullptr))
-    return nullptr;
-
-  auto scoped_current = std::make_unique<ui::ScopedMakeCurrent>(
-      context, texture_owner->GetSurface());
-  // Log an error if ScopedMakeCurrent failed for debugging
-  // https://crbug.com/878042.
-  // TODO(ericrk): Remove this once debugging is completed.
-  if (!context->IsCurrent(nullptr)) {
-    LOG(ERROR) << "Failed to make context current in CodecImage. Subsequent "
-                  "UpdateTexImage may fail.";
-  }
-  return scoped_current;
-}
-
-}  // namespace
 
 CodecImage::CodecImage() = default;
 
@@ -49,12 +24,11 @@ CodecImage::~CodecImage() {
 }
 
 void CodecImage::Initialize(
-    std::unique_ptr<CodecOutputBuffer> output_buffer,
+    std::unique_ptr<CodecOutputBufferRenderer> output_buffer_renderer,
     scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator,
     PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb) {
-  DCHECK(output_buffer);
-  phase_ = Phase::kInCodec;
-  output_buffer_ = std::move(output_buffer);
+  DCHECK(output_buffer_renderer);
+  output_buffer_renderer_ = std::move(output_buffer_renderer);
   codec_buffer_wait_coordinator_ = std::move(codec_buffer_wait_coordinator);
   promotion_hint_cb_ = std::move(promotion_hint_cb);
 }
@@ -80,7 +54,8 @@ 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_ ? output_buffer_->size() : gfx::Size(1, 1);
+  return output_buffer_renderer_ ? output_buffer_renderer_->size()
+                                 : gfx::Size(1, 1);
 }
 
 unsigned CodecImage::GetInternalFormat() {
@@ -119,7 +94,11 @@ bool CodecImage::CopyTexImage(unsigned target) {
           codec_buffer_wait_coordinator_->texture_owner()->GetTextureId()))
     return false;
 
-  RenderToTextureOwnerFrontBuffer(BindingsMode::kEnsureTexImageBound);
+  if (!output_buffer_renderer_)
+    return true;
+
+  output_buffer_renderer_->RenderToTextureOwnerFrontBuffer(
+      BindingsMode::kEnsureTexImageBound);
   return true;
 }
 
@@ -183,23 +162,13 @@ void CodecImage::OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
                               const std::string& dump_name) {}
 
 void CodecImage::GetTextureMatrix(float matrix[16]) {
-  // Default to identity.
-  static constexpr float kYInvertedIdentity[16]{
+  static constexpr float kIdentity[16]{
       1, 0, 0, 0,  //
-      0, -1, 0, 0,  //
+      0, 1, 0, 0,  //
       0, 0, 1, 0,  //
-      0, 1,  0, 1   //
+      0, 0, 0, 1   //
   };
-  memcpy(matrix, kYInvertedIdentity, sizeof(kYInvertedIdentity));
-  if (!codec_buffer_wait_coordinator_)
-    return;
-
-  // The matrix is available after we render to the front buffer. If that fails
-  // we'll return the matrix from the previous frame, which is more likely to be
-  // correct than the identity matrix anyway.
-  RenderToTextureOwnerFrontBuffer(BindingsMode::kDontRestoreIfBound);
-  codec_buffer_wait_coordinator_->texture_owner()->GetTransformMatrix(matrix);
-  YInvertMatrix(matrix);
+  memcpy(matrix, kIdentity, sizeof(kIdentity));
 }
 
 void CodecImage::NotifyPromotionHint(bool promotion_hint,
@@ -222,8 +191,10 @@ void CodecImage::ReleaseResources() {
 }
 
 bool CodecImage::IsUsingGpuMemory() const {
+  if (!output_buffer_renderer_)
+    return false;
   // Only the images which are bound to texture accounts for gpu memory.
-  return was_tex_image_bound_;
+  return output_buffer_renderer_->was_tex_image_bound();
 }
 
 void CodecImage::UpdateAndBindTexImage() {
@@ -239,120 +210,33 @@ gpu::TextureBase* CodecImage::GetTextureBase() const {
 }
 
 bool CodecImage::RenderToFrontBuffer() {
-  // This code is used to trigger early rendering of the image before it is used
-  // for compositing, there is no need to bind the image.
-  return codec_buffer_wait_coordinator_
-             ? RenderToTextureOwnerFrontBuffer(BindingsMode::kRestoreIfBound)
-             : RenderToOverlay();
+  if (!output_buffer_renderer_)
+    return false;
+  return output_buffer_renderer_->RenderToFrontBuffer();
 }
 
 bool CodecImage::RenderToTextureOwnerBackBuffer(BlockingMode blocking_mode) {
-  DCHECK_NE(phase_, Phase::kInFrontBuffer);
-  if (phase_ == Phase::kInBackBuffer)
-    return true;
-  if (phase_ == Phase::kInvalidated)
-    return false;
-
-  // Normally, we should have a wait coordinator if we're called.  However, if
-  // the renderer is torn down (either VideoFrameSubmitter or the whole process)
-  // before we get returns back from viz, then we can be notified that we're
-  // no longer in use (erroneously) when the VideoFrame is destroyed.  So, if
-  // we don't have a wait coordinator, then just fail.
-  if (!codec_buffer_wait_coordinator_)
+  if (!output_buffer_renderer_)
     return false;
 
-  // Wait for a previous frame available so we don't confuse it with the one
-  // we're about to release.
-  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable()) {
-    if (blocking_mode == BlockingMode::kForbidBlocking)
-      return false;
-    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
-  }
-  if (!output_buffer_->ReleaseToSurface()) {
-    phase_ = Phase::kInvalidated;
-    return false;
-  }
-  phase_ = Phase::kInBackBuffer;
-  codec_buffer_wait_coordinator_->SetReleaseTimeToNow();
-  return true;
+  return output_buffer_renderer_->RenderToTextureOwnerBackBuffer(blocking_mode);
 }
 
 bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode) {
-  // Normally, we should have a wait coordinator if we're called.  However, if
-  // the renderer is torn down (either VideoFrameSubmitter or the whole process)
-  // before we get returns back from viz, then we can be notified that we're
-  // no longer in use (erroneously) when the VideoFrame is destroyed.  So, if
-  // we don't have a wait coordinator, then just fail.
-  if (!codec_buffer_wait_coordinator_)
+  if (!output_buffer_renderer_)
     return false;
-
-  if (phase_ == Phase::kInFrontBuffer) {
-    EnsureBoundIfNeeded(bindings_mode);
-    return true;
-  }
-  if (phase_ == Phase::kInvalidated)
-    return false;
-
-  // Render it to the back buffer if it's not already there.
-  if (!RenderToTextureOwnerBackBuffer())
-    return false;
-
-  // The image is now in the back buffer, so promote it to the front buffer.
-  phase_ = Phase::kInFrontBuffer;
-  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable())
-    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
-
-  std::unique_ptr<ui::ScopedMakeCurrent> scoped_make_current =
-      MakeCurrentIfNeeded(
-          codec_buffer_wait_coordinator_->texture_owner().get());
-  // If updating the image will implicitly update the texture bindings then
-  // restore if requested or the update needed a context switch.
-  bool should_restore_bindings =
-      codec_buffer_wait_coordinator_->texture_owner()
-          ->binds_texture_on_update() &&
-      (bindings_mode == BindingsMode::kRestoreIfBound || !!scoped_make_current);
-
-  GLint bound_service_id = 0;
-  if (should_restore_bindings)
-    glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &bound_service_id);
-  codec_buffer_wait_coordinator_->texture_owner()->UpdateTexImage();
-  EnsureBoundIfNeeded(bindings_mode);
-  if (should_restore_bindings)
-    glBindTexture(GL_TEXTURE_EXTERNAL_OES, bound_service_id);
-  return true;
-}
-
-void CodecImage::EnsureBoundIfNeeded(BindingsMode mode) {
-  DCHECK(codec_buffer_wait_coordinator_);
-
-  if (codec_buffer_wait_coordinator_->texture_owner()
-          ->binds_texture_on_update()) {
-    was_tex_image_bound_ = true;
-    return;
-  }
-  if (mode != BindingsMode::kEnsureTexImageBound)
-    return;
-  codec_buffer_wait_coordinator_->texture_owner()->EnsureTexImageBound();
-  was_tex_image_bound_ = true;
+  return output_buffer_renderer_->RenderToTextureOwnerFrontBuffer(
+      bindings_mode);
 }
 
 bool CodecImage::RenderToOverlay() {
-  if (phase_ == Phase::kInFrontBuffer)
-    return true;
-  if (phase_ == Phase::kInvalidated)
+  if (!output_buffer_renderer_)
     return false;
-
-  if (!output_buffer_->ReleaseToSurface()) {
-    phase_ = Phase::kInvalidated;
-    return false;
-  }
-  phase_ = Phase::kInFrontBuffer;
-  return true;
+  return output_buffer_renderer_->RenderToOverlay();
 }
 
 void CodecImage::ReleaseCodecBuffer() {
-  output_buffer_ = nullptr;
-  phase_ = Phase::kInvalidated;
+  output_buffer_renderer_.reset();
 }
 
 std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
@@ -369,9 +253,7 @@ CodecImage::GetAHardwareBuffer() {
 }
 
 gfx::Rect CodecImage::GetCropRect() {
-  if (!codec_buffer_wait_coordinator_)
   return gfx::Rect();
-  return codec_buffer_wait_coordinator_->texture_owner()->GetCropRect();
 }
 
 bool CodecImage::HasMutableState() const {

media/gpu/android/codec_image.h

@@ -16,7 +16,7 @@
 #include "gpu/command_buffer/service/gl_stream_texture_image.h"
 #include "gpu/command_buffer/service/stream_texture_shared_image_interface.h"
 #include "media/gpu/android/codec_buffer_wait_coordinator.h"
-#include "media/gpu/android/codec_wrapper.h"
+#include "media/gpu/android/codec_output_buffer_renderer.h"
 #include "media/gpu/android/promotion_hint_aggregator.h"
 #include "media/gpu/media_gpu_export.h"
 
@@ -33,8 +33,7 @@ namespace media {
 class MEDIA_GPU_EXPORT CodecImage
     : public gpu::StreamTextureSharedImageInterface {
  public:
-  // Whether RenderToTextureOwnerBackBuffer may block or not.
-  enum class BlockingMode { kForbidBlocking, kAllowBlocking };
+  using BlockingMode = CodecOutputBufferRenderer::BlockingMode;
 
   // Callback to notify that a codec image is now unused in the sense of not
   // being out for display.  This lets us signal interested folks once a video
@@ -54,7 +53,7 @@ class MEDIA_GPU_EXPORT CodecImage
   // May be called on a random thread, but only if the CodecImage is otherwise
   // not in use.
   void Initialize(
-      std::unique_ptr<CodecOutputBuffer> output_buffer,
+      std::unique_ptr<CodecOutputBufferRenderer> output_buffer_renderer,
       scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator,
       PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb);
 
@@ -123,12 +122,11 @@ class MEDIA_GPU_EXPORT CodecImage
 
   // Whether the codec buffer has been rendered to the front buffer.
   bool was_rendered_to_front_buffer() const {
-    return phase_ == Phase::kInFrontBuffer;
+    return output_buffer_renderer_
+               ? output_buffer_renderer_->was_rendered_to_front_buffer()
+               : false;
   }
 
-  // Whether the TextureOwner's texture is in the front buffer and bound to the
-  // latest image.
-  bool was_tex_image_bound() const { return was_tex_image_bound_; }
 
   // Whether this image is backed by a texture owner.
   // We want to check for texture_owner owned by
@@ -164,7 +162,9 @@ class MEDIA_GPU_EXPORT CodecImage
   virtual void ReleaseCodecBuffer();
 
   CodecOutputBuffer* get_codec_output_buffer_for_testing() const {
-    return output_buffer_.get();
+    return output_buffer_renderer_
+               ? output_buffer_renderer_->get_codec_output_buffer_for_testing()
+               : nullptr;
   }
 
  protected:
@@ -173,22 +173,12 @@ class MEDIA_GPU_EXPORT CodecImage
  private:
   FRIEND_TEST_ALL_PREFIXES(CodecImageTest, RenderAfterUnusedDoesntCrash);
 
-  // The lifecycle phases of an image.
-  // The only possible transitions are from left to right. Both
-  // kInFrontBuffer and kInvalidated are terminal.
-  enum class Phase { kInCodec, kInBackBuffer, kInFrontBuffer, kInvalidated };
+  std::unique_ptr<CodecOutputBufferRenderer> output_buffer_renderer_;
 
   // Renders this image to the texture owner front buffer by first rendering
   // it to the back buffer if it's not already there, and then waiting for the
   // frame available event before calling UpdateTexImage().
   bool RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode);
-  void EnsureBoundIfNeeded(BindingsMode mode);
-
-  // The phase of the image buffer's lifecycle.
-  Phase phase_ = Phase::kInvalidated;
-
-  // The buffer backing this image.
-  std::unique_ptr<CodecOutputBuffer> output_buffer_;
 
   // The CodecBufferWaitCoordinator that |output_buffer_| will be rendered to.
   // Or null, if this image is backed by an overlay.
@@ -202,8 +192,6 @@ class MEDIA_GPU_EXPORT CodecImage
 
   std::vector<UnusedCB> unused_cbs_;
 
-  bool was_tex_image_bound_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(CodecImage);
 };
 

media/gpu/android/codec_image_unittest.cc

@@ -86,10 +86,15 @@ class CodecImageTest : public testing::Test {
       CodecImage::UnusedCB unused_cb = base::DoNothing()) {
     std::unique_ptr<CodecOutputBuffer> buffer;
     wrapper_->DequeueOutputBuffer(nullptr, nullptr, &buffer);
+
+    auto codec_buffer_wait_coordinator =
+        kind == kTextureOwner ? codec_buffer_wait_coordinator_ : nullptr;
+    auto buffer_renderer = std::make_unique<CodecOutputBufferRenderer>(
+        std::move(buffer), codec_buffer_wait_coordinator);
+
     scoped_refptr<CodecImage> image = new CodecImage();
     image->Initialize(
-        std::move(buffer),
-        kind == kTextureOwner ? codec_buffer_wait_coordinator_ : nullptr,
+        std::move(buffer_renderer), codec_buffer_wait_coordinator,
         base::BindRepeating(&PromotionHintReceiver::OnPromotionHint,
                             base::Unretained(&promotion_hint_receiver_)));
 
@@ -216,48 +221,6 @@ TEST_F(CodecImageTestExplicitBind, CopyTexImageTriggersFrontBufferRendering) {
   ASSERT_TRUE(i->was_rendered_to_front_buffer());
 }
 
-TEST_F(CodecImageTest, GetTextureMatrixTriggersFrontBufferRendering) {
-  auto i = NewImage(kTextureOwner);
-  InSequence s;
-  EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true));
-  EXPECT_CALL(*codec_buffer_wait_coordinator_, WaitForFrameAvailable());
-  EXPECT_CALL(*codec_buffer_wait_coordinator_->texture_owner(),
-              UpdateTexImage());
-  EXPECT_CALL(*codec_buffer_wait_coordinator_->texture_owner(),
-              GetTransformMatrix(_));
-  float matrix[16];
-  i->GetTextureMatrix(matrix);
-  ASSERT_TRUE(i->was_rendered_to_front_buffer());
-}
-
-TEST_F(CodecImageTestExplicitBind,
-       GetTextureMatrixTriggersFrontBufferRendering) {
-  // GetTextureMatrix should not bind the image.
-  codec_buffer_wait_coordinator_->texture_owner()->expect_update_tex_image =
-      false;
-
-  auto i = NewImage(kTextureOwner);
-  InSequence s;
-  EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true));
-  EXPECT_CALL(*codec_buffer_wait_coordinator_, WaitForFrameAvailable());
-  EXPECT_CALL(*codec_buffer_wait_coordinator_->texture_owner(),
-              UpdateTexImage());
-  EXPECT_CALL(*codec_buffer_wait_coordinator_->texture_owner(),
-              GetTransformMatrix(_));
-  float matrix[16];
-  i->GetTextureMatrix(matrix);
-  ASSERT_TRUE(i->was_rendered_to_front_buffer());
-}
-
-TEST_F(CodecImageTest, GetTextureMatrixReturnsIdentityForOverlayImages) {
-  auto i = NewImage(kOverlay);
-  float matrix[16]{0};
-  i->GetTextureMatrix(matrix);
-  // See GetTextureMatrix() for the expected result.
-  ASSERT_EQ(matrix[0], 1);
-  ASSERT_EQ(matrix[5], -1);
-}
-
 TEST_F(CodecImageTest, ScheduleOverlayPlaneTriggersFrontBufferRendering) {
   auto i = NewImage(kOverlay);
   EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true));
@@ -412,15 +375,6 @@ TEST_F(CodecImageTest, GetAHardwareBufferAfterRelease) {
   EXPECT_FALSE(i->GetAHardwareBuffer());
 }
 
-TEST_F(CodecImageTest, GetCropRect) {
-  auto i = NewImage(kTextureOwner);
-  EXPECT_EQ(
-      codec_buffer_wait_coordinator_->texture_owner()->get_crop_rect_count, 0);
-  i->GetCropRect();
-  EXPECT_EQ(
-      codec_buffer_wait_coordinator_->texture_owner()->get_crop_rect_count, 1);
-}
-
 TEST_F(CodecImageTest, RenderAfterUnusedDoesntCrash) {
   auto i = NewImage(kTextureOwner);
   i->NotifyUnused();

media/gpu/android/codec_output_buffer_renderer.cc

+// Copyright 2020 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 "media/gpu/android/codec_output_buffer_renderer.h"
+#include <string.h>
+
+#include "base/android/scoped_hardware_buffer_fence_sync.h"
+#include "base/bind_helpers.h"
+#include "gpu/command_buffer/service/gles2_cmd_decoder.h"
+#include "gpu/command_buffer/service/texture_manager.h"
+#include "ui/gl/gl_context.h"
+#include "ui/gl/scoped_make_current.h"
+
+namespace media {
+namespace {
+
+// Makes |texture_owner|'s context current if it isn't already.
+std::unique_ptr<ui::ScopedMakeCurrent> MakeCurrentIfNeeded(
+    gpu::TextureOwner* texture_owner) {
+  gl::GLContext* context = texture_owner->GetContext();
+  // Note: this works for virtual contexts too, because IsCurrent() returns true
+  // if their shared platform context is current, regardless of which virtual
+  // context is current.
+  if (context->IsCurrent(nullptr))
+    return nullptr;
+
+  auto scoped_current = std::make_unique<ui::ScopedMakeCurrent>(
+      context, texture_owner->GetSurface());
+  // Log an error if ScopedMakeCurrent failed for debugging
+  // https://crbug.com/878042.
+  // TODO(ericrk): Remove this once debugging is completed.
+  if (!context->IsCurrent(nullptr)) {
+    LOG(ERROR) << "Failed to make context current in CodecImage. Subsequent "
+                  "UpdateTexImage may fail.";
+  }
+  return scoped_current;
+}
+
+}  // namespace
+
+CodecOutputBufferRenderer::CodecOutputBufferRenderer(
+    std::unique_ptr<CodecOutputBuffer> output_buffer,
+    scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator)
+    : output_buffer_(std::move(output_buffer)),
+      codec_buffer_wait_coordinator_(std::move(codec_buffer_wait_coordinator)) {
+
+}
+
+CodecOutputBufferRenderer::~CodecOutputBufferRenderer() = default;
+
+bool CodecOutputBufferRenderer::RenderToTextureOwnerBackBuffer(
+    BlockingMode blocking_mode) {
+  DCHECK_NE(phase_, Phase::kInFrontBuffer);
+  if (phase_ == Phase::kInBackBuffer)
+    return true;
+  if (phase_ == Phase::kInvalidated)
+    return false;
+
+  // Normally, we should have a wait coordinator if we're called.  However, if
+  // the renderer is torn down (either VideoFrameSubmitter or the whole process)
+  // before we get returns back from viz, then we can be notified that we're
+  // no longer in use (erroneously) when the VideoFrame is destroyed.  So, if
+  // we don't have a wait coordinator, then just fail.
+  if (!codec_buffer_wait_coordinator_)
+    return false;
+
+  // Wait for a previous frame available so we don't confuse it with the one
+  // we're about to release.
+  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable()) {
+    if (blocking_mode == BlockingMode::kForbidBlocking)
+      return false;
+    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
+  }
+  if (!output_buffer_->ReleaseToSurface()) {
+    phase_ = Phase::kInvalidated;
+    return false;
+  }
+  phase_ = Phase::kInBackBuffer;
+  codec_buffer_wait_coordinator_->SetReleaseTimeToNow();
+  return true;
+}
+
+bool CodecOutputBufferRenderer::RenderToTextureOwnerFrontBuffer(
+    BindingsMode bindings_mode) {
+  // Normally, we should have a wait coordinator if we're called.  However, if
+  // the renderer is torn down (either VideoFrameSubmitter or the whole process)
+  // before we get returns back from viz, then we can be notified that we're
+  // no longer in use (erroneously) when the VideoFrame is destroyed.  So, if
+  // we don't have a wait coordinator, then just fail.
+  if (!codec_buffer_wait_coordinator_)
+    return false;
+
+  if (phase_ == Phase::kInFrontBuffer) {
+    EnsureBoundIfNeeded(bindings_mode);
+    return true;
+  }
+  if (phase_ == Phase::kInvalidated)
+    return false;
+
+  // Render it to the back buffer if it's not already there.
+  if (!RenderToTextureOwnerBackBuffer())
+    return false;
+
+  // The image is now in the back buffer, so promote it to the front buffer.
+  phase_ = Phase::kInFrontBuffer;
+  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable())
+    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
+
+  std::unique_ptr<ui::ScopedMakeCurrent> scoped_make_current =
+      MakeCurrentIfNeeded(
+          codec_buffer_wait_coordinator_->texture_owner().get());
+  // If updating the image will implicitly update the texture bindings then
+  // restore if requested or the update needed a context switch.
+  bool should_restore_bindings =
+      codec_buffer_wait_coordinator_->texture_owner()
+          ->binds_texture_on_update() &&
+      (bindings_mode == BindingsMode::kRestoreIfBound || !!scoped_make_current);
+
+  GLint bound_service_id = 0;
+  if (should_restore_bindings)
+    glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &bound_service_id);
+  codec_buffer_wait_coordinator_->texture_owner()->UpdateTexImage();
+  EnsureBoundIfNeeded(bindings_mode);
+  if (should_restore_bindings)
+    glBindTexture(GL_TEXTURE_EXTERNAL_OES, bound_service_id);
+  return true;
+}
+
+void CodecOutputBufferRenderer::EnsureBoundIfNeeded(BindingsMode mode) {
+  DCHECK(codec_buffer_wait_coordinator_);
+
+  if (codec_buffer_wait_coordinator_->texture_owner()
+          ->binds_texture_on_update()) {
+    was_tex_image_bound_ = true;
+    return;
+  }
+  if (mode != BindingsMode::kEnsureTexImageBound)
+    return;
+  codec_buffer_wait_coordinator_->texture_owner()->EnsureTexImageBound();
+  was_tex_image_bound_ = true;
+}
+
+bool CodecOutputBufferRenderer::RenderToOverlay() {
+  if (phase_ == Phase::kInFrontBuffer)
+    return true;
+  if (phase_ == Phase::kInvalidated)
+    return false;
+
+  if (!output_buffer_->ReleaseToSurface()) {
+    phase_ = Phase::kInvalidated;
+    return false;
+  }
+  phase_ = Phase::kInFrontBuffer;
+  return true;
+}
+
+bool CodecOutputBufferRenderer::RenderToFrontBuffer() {
+  // This code is used to trigger early rendering of the image before it is used
+  // for compositing, there is no need to bind the image.
+  return codec_buffer_wait_coordinator_
+             ? RenderToTextureOwnerFrontBuffer(BindingsMode::kRestoreIfBound)
+             : RenderToOverlay();
+}
+
+}  // namespace media

media/gpu/android/codec_output_buffer_renderer.h

+// Copyright 2020 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.
+
+#ifndef MEDIA_GPU_ANDROID_CODEC_OUTPUT_BUFFER_RENDERER_H_
+#define MEDIA_GPU_ANDROID_CODEC_OUTPUT_BUFFER_RENDERER_H_
+
+#include <stdint.h>
+
+#include <memory>
+
+#include "gpu/command_buffer/service/stream_texture_shared_image_interface.h"
+#include "media/gpu/android/codec_buffer_wait_coordinator.h"
+#include "media/gpu/android/codec_wrapper.h"
+#include "media/gpu/media_gpu_export.h"
+
+namespace media {
+
+// A class that holds CodecOutputBuffer and renders it to TextureOwner or
+// overlay as necessary. Unit tests for this class are part of CodecImage unit
+// tests.
+class MEDIA_GPU_EXPORT CodecOutputBufferRenderer {
+ public:
+  using BindingsMode = gpu::StreamTextureSharedImageInterface::BindingsMode;
+  // Whether RenderToTextureOwnerBackBuffer may block or not.
+  enum class BlockingMode { kForbidBlocking, kAllowBlocking };
+
+  CodecOutputBufferRenderer(
+      std::unique_ptr<CodecOutputBuffer> output_buffer,
+      scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator);
+  ~CodecOutputBufferRenderer();
+
+  CodecOutputBufferRenderer(const CodecOutputBufferRenderer&) = delete;
+  CodecOutputBufferRenderer& operator=(const CodecOutputBufferRenderer&) =
+      delete;
+
+  // Renders this image to the overlay. Returns true if the buffer is in the
+  // overlay front buffer. Returns false if the buffer was invalidated.
+  bool RenderToOverlay();
+
+  // Renders this image to the texture owner front buffer by first rendering
+  // it to the back buffer if it's not already there, and then waiting for the
+  // frame available event before calling UpdateTexImage().
+  bool RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode);
+
+  // Renders this image to the front buffer of its backing surface.
+  // Returns true if the buffer is in the front buffer. Returns false if the
+  // buffer was invalidated. After an image is invalidated it's no longer
+  // possible to render it.
+  bool RenderToFrontBuffer();
+
+  // Renders this image to the back buffer of its texture owner. Only valid if
+  // is_texture_owner_backed(). Returns true if the buffer is in the back
+  // buffer. Returns false if the buffer was invalidated.
+  // |blocking_mode| indicates whether this should (a) wait for any previously
+  // pending rendered frame before rendering this one, or (b) fail if a wait
+  // is required.
+  bool RenderToTextureOwnerBackBuffer(
+      BlockingMode blocking_mode = BlockingMode::kAllowBlocking);
+
+  // Whether the codec buffer has been rendered to the front buffer.
+  bool was_rendered_to_front_buffer() const {
+    return phase_ == Phase::kInFrontBuffer;
+  }
+
+  gfx::Size size() const { return output_buffer_->size(); }
+
+  bool was_tex_image_bound() const { return was_tex_image_bound_; }
+
+  scoped_refptr<gpu::TextureOwner> texture_owner() const {
+    return codec_buffer_wait_coordinator_
+               ? codec_buffer_wait_coordinator_->texture_owner()
+               : nullptr;
+  }
+
+  CodecOutputBuffer* get_codec_output_buffer_for_testing() const {
+    return output_buffer_.get();
+  }
+
+ private:
+  // The lifecycle phases of an buffer.
+  // The only possible transitions are from left to right. Both
+  // kInFrontBuffer and kInvalidated are terminal.
+  enum class Phase { kInCodec, kInBackBuffer, kInFrontBuffer, kInvalidated };
+
+  void EnsureBoundIfNeeded(BindingsMode mode);
+
+  // The phase of the image buffer's lifecycle.
+  Phase phase_ = Phase::kInCodec;
+
+  // The buffer backing this image.
+  std::unique_ptr<CodecOutputBuffer> output_buffer_;
+
+  // The CodecBufferWaitCoordinator that |output_buffer_| will be rendered to.
+  // Or null, if this image is backed by an overlay.
+  scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator_;
+
+  bool was_tex_image_bound_ = false;
+};
+
+}  // namespace media
+#endif  // MEDIA_GPU_ANDROID_CODEC_OUTPUT_BUFFER_RENDERER_H

media/gpu/android/frame_info_helper.cc

+// Copyright 2019 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 "media/gpu/android/frame_info_helper.h"
+
+#include "gpu/command_buffer/service/shared_image_video.h"
+#include "gpu/ipc/service/command_buffer_stub.h"
+#include "gpu/ipc/service/gpu_channel.h"
+#include "gpu/ipc/service/gpu_channel_manager.h"
+
+namespace media {
+
+FrameInfoHelper::FrameInfo::FrameInfo() = default;
+FrameInfoHelper::FrameInfo::~FrameInfo() = default;
+FrameInfoHelper::FrameInfo::FrameInfo(FrameInfo&&) = default;
+FrameInfoHelper::FrameInfo::FrameInfo(const FrameInfoHelper::FrameInfo&) =
+    default;
+FrameInfoHelper::FrameInfo& FrameInfoHelper::FrameInfo::operator=(
+    const FrameInfoHelper::FrameInfo&) = default;
+
+// Concrete implementation of FrameInfoHelper that renders output buffers and
+// gets the FrameInfo they need.
+class FrameInfoHelperImpl : public FrameInfoHelper,
+                            public gpu::CommandBufferStub::DestructionObserver {
+ public:
+  FrameInfoHelperImpl(SharedImageVideoProvider::GetStubCB get_stub_cb) {
+    stub_ = get_stub_cb.Run();
+    if (stub_)
+      stub_->AddDestructionObserver(this);
+  }
+
+  ~FrameInfoHelperImpl() override {
+    if (stub_)
+      stub_->RemoveDestructionObserver(this);
+  }
+
+  void GetFrameInfo(
+      std::unique_ptr<CodecOutputBufferRenderer> buffer_renderer,
+      base::OnceCallback<
+          void(std::unique_ptr<CodecOutputBufferRenderer>, FrameInfo, bool)> cb)
+      override {
+    if (!buffer_renderer) {
+      std::move(cb).Run(nullptr, FrameInfo(), false);
+      return;
+    }
+
+    auto texture_owner = buffer_renderer->texture_owner();
+
+    FrameInfo info;
+
+    // Indicates that the FrameInfo is reliable and can be cached by caller.
+    // It's true if we either return cached values or we attempted to render
+    // frame and succeeded.
+    bool success = true;
+
+    // We default to visible size if if we can't get real size
+    info.coded_size = buffer_renderer->size();
+    info.visible_rect = gfx::Rect(info.coded_size);
+
+    if (texture_owner) {
+      if (visible_size_ == buffer_renderer->size()) {
+        info = frame_info_;
+      } else if (buffer_renderer->RenderToTextureOwnerFrontBuffer(
+                     CodecOutputBufferRenderer::BindingsMode::
+                         kDontRestoreIfBound)) {
+        visible_size_ = buffer_renderer->size();
+        texture_owner->GetCodedSizeAndVisibleRect(
+            visible_size_, &frame_info_.coded_size, &frame_info_.visible_rect);
+
+        frame_info_.ycbcr_info = GetYCbCrInfo(texture_owner.get());
+        info = frame_info_;
+      } else {
+        // We attempted to render frame and failed, mark request as failed so
+        // caller won't cache best-guess values.
+        success = false;
+      }
+    }
+
+    std::move(cb).Run(std::move(buffer_renderer), frame_info_, success);
+  }
+
+  void OnWillDestroyStub(bool have_context) override {
+    DCHECK(stub_);
+    stub_ = nullptr;
+  }
+
+ private:
+  // Gets YCbCrInfo from last rendered frame.
+  base::Optional<gpu::VulkanYCbCrInfo> GetYCbCrInfo(
+      gpu::TextureOwner* texture_owner) {
+    gpu::ContextResult result;
+    if (!stub_)
+      return base::nullopt;
+
+    auto shared_context =
+        stub_->channel()->gpu_channel_manager()->GetSharedContextState(&result);
+    auto context_provider =
+        (result == gpu::ContextResult::kSuccess) ? shared_context : nullptr;
+    if (!context_provider)
+      return base::nullopt;
+
+    return gpu::SharedImageVideo::GetYcbcrInfo(texture_owner, context_provider);
+  }
+
+  gpu::CommandBufferStub* stub_ = nullptr;
+
+  FrameInfo frame_info_;
+  gfx::Size visible_size_;
+};
+
+// static
+base::SequenceBound<FrameInfoHelper> FrameInfoHelper::Create(
+    scoped_refptr<base::SequencedTaskRunner> gpu_task_runner,
+    SharedImageVideoProvider::GetStubCB get_stub_cb) {
+  return base::SequenceBound<FrameInfoHelperImpl>(std::move(gpu_task_runner),
+                                                  std::move(get_stub_cb));
+}
+
+}  // namespace media

media/gpu/android/ycbcr_helper.h → media/gpu/android/frame_info_helper.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MEDIA_GPU_ANDROID_YCBCR_HELPER_H_
-#define MEDIA_GPU_ANDROID_YCBCR_HELPER_H_
+#ifndef MEDIA_GPU_ANDROID_FRAME_INFO_HELPER_H_
+#define MEDIA_GPU_ANDROID_FRAME_INFO_HELPER_H_
 
 #include "base/optional.h"
 #include "base/threading/sequence_bound.h"
@@ -14,35 +14,50 @@
 namespace media {
 
 // Helper class to fetch YCbCrInfo for Vulkan from a CodecImage.
-class MEDIA_GPU_EXPORT YCbCrHelper {
+class MEDIA_GPU_EXPORT FrameInfoHelper {
  public:
-  using OptionalInfo = base::Optional<gpu::VulkanYCbCrInfo>;
+  struct FrameInfo {
+    FrameInfo();
+    ~FrameInfo();
 
-  static base::SequenceBound<YCbCrHelper> Create(
+    FrameInfo(FrameInfo&&);
+    FrameInfo(const FrameInfo&);
+    FrameInfo& operator=(const FrameInfo&);
+
+    gfx::Size coded_size;
+    gfx::Rect visible_rect;
+    base::Optional<gpu::VulkanYCbCrInfo> ycbcr_info;
+  };
+
+  static base::SequenceBound<FrameInfoHelper> Create(
       scoped_refptr<base::SequencedTaskRunner> gpu_task_runner,
       SharedImageVideoProvider::GetStubCB get_stub_cb);
 
-  virtual ~YCbCrHelper() = default;
+  virtual ~FrameInfoHelper() = default;
 
-  // Call |cb| with the YCbCrInfo (or nullopt, if we can't get it).  Will render
-  // |codec_image_holder| to the front buffer if it hasn't successfully gotten
-  // the YCbCrInfo on a previous call.  Otherwise, will return the cached
-  // YCbCrInfo and leave |codec_image_holder| unmodified.  Once we call |cb|
-  // with a non-nullopt YCbCrInfo, we will always return that same value; there
-  // is no need to call us afterwards.
+  // Call |cb| with the FrameInfo.  Will render |buffer_renderer| to the front
+  // buffer if we don't have frame info cached. For Vulkan this also will
+  // attempt to get YCbCrInfo and cache it.  If all necessary info is cached the
+  // call will leave buffer_renderer intact and it can be rendered later.
+  // Rendering can fail for reasons. This function will make best efforts to
+  // fill FrameInfo which can be used to create VideoFrame, but shouldn't be
+  // cached by caller. Last parameter in |cb| is bool that indicates that info
+  // is reliable.
   //
   // While this API might seem to be out of its Vulkan mind, it's this
   // complicated to (a) prevent rendering frames out of order to the front
   // buffer, and (b) make it easy to handle the fact that sometimes, we just
   // can't get a YCbCrInfo from a CodecImage due to timeouts.
-  virtual void GetYCbCrInfo(
-      scoped_refptr<CodecImageHolder> codec_image_holder,
-      base::OnceCallback<void(OptionalInfo ycbcr_info)> cb) = 0;
+  virtual void GetFrameInfo(
+      std::unique_ptr<CodecOutputBufferRenderer> buffer_renderer,
+      base::OnceCallback<void(std::unique_ptr<CodecOutputBufferRenderer>,
+                              FrameInfo,
+                              bool)> cb) = 0;
 
  protected:
-  YCbCrHelper() = default;
+  FrameInfoHelper() = default;
 };
 
 }  // namespace media
 
-#endif  // MEDIA_GPU_ANDROID_YCBCR_HELPER_H_
+#endif  // MEDIA_GPU_ANDROID_FRAME_INFO_HELPER_H_

media/gpu/android/video_frame_factory_impl.h

@@ -16,10 +16,10 @@
 #include "media/gpu/android/codec_buffer_wait_coordinator.h"
 #include "media/gpu/android/codec_image.h"
 #include "media/gpu/android/codec_wrapper.h"
+#include "media/gpu/android/frame_info_helper.h"
 #include "media/gpu/android/maybe_render_early_manager.h"
 #include "media/gpu/android/shared_image_video_provider.h"
 #include "media/gpu/android/video_frame_factory.h"
-#include "media/gpu/android/ycbcr_helper.h"
 #include "media/gpu/media_gpu_export.h"
 #include "ui/gl/gl_bindings.h"
 
@@ -41,13 +41,18 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
       base::OnceCallback<void(gpu::Mailbox mailbox,
                               VideoFrame::ReleaseMailboxCB release_cb)>;
 
+  using ImageWithInfoReadyCB =
+      base::OnceCallback<void(std::unique_ptr<CodecOutputBufferRenderer>,
+                              FrameInfoHelper::FrameInfo,
+                              SharedImageVideoProvider::ImageRecord)>;
+
   // |get_stub_cb| will be run on |gpu_task_runner|.
   VideoFrameFactoryImpl(
       scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner,
       const gpu::GpuPreferences& gpu_preferences,
       std::unique_ptr<SharedImageVideoProvider> image_provider,
       std::unique_ptr<MaybeRenderEarlyManager> mre_manager,
-      base::SequenceBound<YCbCrHelper> ycbcr_helper);
+      base::SequenceBound<FrameInfoHelper> frame_info_helper);
   ~VideoFrameFactoryImpl() override;
 
   void Initialize(OverlayMode overlay_mode, InitCB init_cb) override;
@@ -68,6 +73,8 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
   }
 
  private:
+  void RequestImage(std::unique_ptr<CodecOutputBufferRenderer> buffer_renderer,
+                    ImageWithInfoReadyCB image_ready_cb);
   // ImageReadyCB that will construct a VideoFrame, and forward it to
   // |output_cb| if construction succeeds.  This is static for two reasons.
   // First, we want to snapshot the state of the world when the request is made,
@@ -83,33 +90,23 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
       base::WeakPtr<VideoFrameFactoryImpl> thiz,
       OnceOutputCB output_cb,
       base::TimeDelta timestamp,
-      gfx::Size coded_size,
       gfx::Size natural_size,
-      std::unique_ptr<CodecOutputBuffer> output_buffer,
       scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator,
       PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb,
       VideoPixelFormat pixel_format,
       OverlayMode overlay_mode,
       bool enable_threaded_texture_mailboxes,
       scoped_refptr<base::SequencedTaskRunner> gpu_task_runner,
+      std::unique_ptr<CodecOutputBufferRenderer> output_buffer_renderer,
+      FrameInfoHelper::FrameInfo frame_info,
       SharedImageVideoProvider::ImageRecord record);
 
-  // Callback to receive YCbCrInfo from |provider_| while creating a VideoFrame.
-  void CreateVideoFrame_OnYCbCrInfo(base::OnceClosure completion_cb,
-                                    YCbCrHelper::OptionalInfo ycbcr_info);
-
-  // Really create the VideoFrame, once we've tried to get the YCbCrInfo if it's
-  // needed for it.
-  void CreateVideoFrame_Finish(
-      OnceOutputCB output_cb,
-      base::TimeDelta timestamp,
-      gfx::Size coded_size,
-      gfx::Size natural_size,
+  void CreateVideoFrame_OnFrameInfoReady(
+      ImageWithInfoReadyCB image_ready_cb,
       scoped_refptr<CodecBufferWaitCoordinator> codec_buffer_wait_coordinator,
-      VideoPixelFormat pixel_format,
-      OverlayMode overlay_mode,
-      bool enable_threaded_texture_mailboxes,
-      SharedImageVideoProvider::ImageRecord record);
+      std::unique_ptr<CodecOutputBufferRenderer> output_buffer_renderer,
+      FrameInfoHelper::FrameInfo frame_info,
+      bool success);
 
   MaybeRenderEarlyManager* mre_manager() const { return mre_manager_.get(); }
 
@@ -131,11 +128,12 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
 
   std::unique_ptr<MaybeRenderEarlyManager> mre_manager_;
 
-  // Sampler conversion information which is used in vulkan context.
-  YCbCrHelper::OptionalInfo ycbcr_info_;
+  // Caches FrameInfo and visible size it was cached for.
+  gfx::Size visible_size_;
+  FrameInfoHelper::FrameInfo frame_info_;
 
   // Optional helper to get the Vulkan YCbCrInfo.
-  base::SequenceBound<YCbCrHelper> ycbcr_helper_;
+  base::SequenceBound<FrameInfoHelper> frame_info_helper_;
 
   // The current image spec that we'll use to request images.
   SharedImageVideoProvider::ImageSpec image_spec_;

media/gpu/android/video_frame_factory_impl_unittest.cc

@@ -41,21 +41,49 @@ class MockMaybeRenderEarlyManager : public MaybeRenderEarlyManager {
   MOCK_METHOD0(MaybeRenderEarly, void());
 };
 
-class MockYCbCrHelper : public YCbCrHelper, public DestructionObservable {
+class MockFrameInfoHelper : public FrameInfoHelper,
+                            public DestructionObservable {
  public:
-  MockYCbCrHelper(MockYCbCrHelper** thiz) { *thiz = this; }
-
-  void GetYCbCrInfo(
-      scoped_refptr<CodecImageHolder> codec_image_holder,
-      base::OnceCallback<void(OptionalInfo ycbcr_info)> cb) override {
-    MockGetYCbCrInfo(codec_image_holder);
+  MockFrameInfoHelper(MockFrameInfoHelper** thiz) { *thiz = this; }
+
+  void GetFrameInfo(
+      std::unique_ptr<CodecOutputBufferRenderer> buffer_renderer,
+      base::OnceCallback<
+          void(std::unique_ptr<CodecOutputBufferRenderer>, FrameInfo, bool)> cb)
+      override {
+    MockGetFrameInfo(buffer_renderer.get());
     cb_ = std::move(cb);
+    buffer_renderer_ = std::move(buffer_renderer);
+
+    if (run_callback_automatically_) {
+      RunWithYcbCrInfo(true);
+      base::RunLoop().RunUntilIdle();
+    }
+  }
+
+  void RunWithYcbCrInfo(bool success) {
+    DCHECK(buffer_renderer_);
+
+    FrameInfo info;
+    info.coded_size = buffer_renderer_->size();
+    info.visible_rect = gfx::Rect(info.coded_size);
+
+    std::move(cb_).Run(std::move(buffer_renderer_), info, success);
+  }
+
+  void set_run_callback_automatically(bool run_callback_automatically) {
+    run_callback_automatically_ = run_callback_automatically;
   }
 
-  MOCK_METHOD1(MockGetYCbCrInfo,
-               void(scoped_refptr<CodecImageHolder> codec_image_holder));
+  MOCK_METHOD1(MockGetFrameInfo,
+               void(CodecOutputBufferRenderer* buffer_renderer));
 
-  base::OnceCallback<void(OptionalInfo ycbcr_info)> cb_;
+ private:
+  bool run_callback_automatically_ = true;
+  base::OnceCallback<
+      void(std::unique_ptr<CodecOutputBufferRenderer>, FrameInfo, bool)>
+      cb_;
+  std::unique_ptr<CodecOutputBufferRenderer> buffer_renderer_;
 };
 
 class VideoFrameFactoryImplTest : public testing::Test {
@@ -68,8 +96,8 @@ class VideoFrameFactoryImplTest : public testing::Test {
     auto mre_manager = std::make_unique<MockMaybeRenderEarlyManager>();
     mre_manager_raw_ = mre_manager.get();
 
-    auto ycbcr_helper =
-        base::SequenceBound<MockYCbCrHelper>(task_runner_, &ycbcr_helper_raw_);
+    auto ycbcr_helper = base::SequenceBound<MockFrameInfoHelper>(
+        task_runner_, &ycbcr_helper_raw_);
     base::RunLoop().RunUntilIdle();  // Init |ycbcr_helper_raw_|.
     ycbcr_destruction_observer_ =
         ycbcr_helper_raw_->CreateDestructionObserver();
@@ -148,7 +176,7 @@ class VideoFrameFactoryImplTest : public testing::Test {
   // Sent to |impl_| by RequestVideoFrame..
   base::MockCallback<VideoFrameFactory::OnceOutputCB> output_cb_;
 
-  MockYCbCrHelper* ycbcr_helper_raw_ = nullptr;
+  MockFrameInfoHelper* ycbcr_helper_raw_ = nullptr;
   std::unique_ptr<DestructionObserver> ycbcr_destruction_observer_;
 
   gpu::GpuPreferences gpu_preferences_;
@@ -244,77 +272,72 @@ TEST_F(VideoFrameFactoryImplTest,
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_F(VideoFrameFactoryImplTest, DoesNotCallYCbCrHelperIfNotVulkan) {
-  EXPECT_CALL(*ycbcr_helper_raw_, MockGetYCbCrInfo(_)).Times(0);
-  RequestVideoFrame();
-  auto image_record = MakeImageRecord();
-  image_record.is_vulkan = false;
-  image_provider_raw_->ProvideOneRequestedImage(&image_record);
-  base::RunLoop().RunUntilIdle();
-}
+TEST_F(VideoFrameFactoryImplTest, DoesCallFrameInfoHelperIfVulkan) {
+  // We will be driving callback by ourselves in this test.
+  ycbcr_helper_raw_->set_run_callback_automatically(false);
+  // Expect call to get info for the first frame.
+  EXPECT_CALL(*ycbcr_helper_raw_, MockGetFrameInfo(_)).Times(1);
 
-TEST_F(VideoFrameFactoryImplTest, DoesCallYCbCrHelperIfVulkan) {
   RequestVideoFrame();
-  auto image_record = MakeImageRecord();
-  base::OnceCallback<void(YCbCrHelper::OptionalInfo)> cb;
-  EXPECT_CALL(*ycbcr_helper_raw_,
-              MockGetYCbCrInfo(image_record.codec_image_holder))
-      .Times(1);
-  image_record.is_vulkan = true;
-  image_provider_raw_->ProvideOneRequestedImage(&image_record);
+
+  // Provide info.  It should send image request.
+  ycbcr_helper_raw_->RunWithYcbCrInfo(true);
   base::RunLoop().RunUntilIdle();
 
-  // Provide YCbCrInfo.  It should provide the VideoFrame too.
+  testing::Mock::VerifyAndClearExpectations(ycbcr_helper_raw_);
+
+  // Fulfilling image request should provide video frame.
   EXPECT_CALL(output_cb_, Run(_)).Times(1);
-  gpu::VulkanYCbCrInfo ycbcr;
-  std::move(ycbcr_helper_raw_->cb_).Run(ycbcr);
+
+  auto image_record = MakeImageRecord();
+  image_provider_raw_->ProvideOneRequestedImage(&image_record);
   base::RunLoop().RunUntilIdle();
-  // It's okay if the ycbcr helper is destroyed.  If not, then verify
-  // expectations explicitly now.
-  if (ycbcr_destruction_observer_->destructed())
-    ycbcr_helper_raw_ = nullptr;
-  else
-    testing::Mock::VerifyAndClearExpectations(ycbcr_helper_raw_);
 
   // Verify that no more calls happen, since we don't want thread hops on every
   // frame.  Note that multiple could be dispatched before now.  It should still
   // send along a VideoFrame, though.
+  EXPECT_CALL(*ycbcr_helper_raw_, MockGetFrameInfo(_)).Times(0);
+  EXPECT_CALL(output_cb_, Run(_)).Times(1);
+
   RequestVideoFrame();
   auto other_image_record = MakeImageRecord();
   // If the helper hasn't been destroyed, then we don't expect it to be called.
-  if (ycbcr_helper_raw_)
-    EXPECT_CALL(*ycbcr_helper_raw_, MockGetYCbCrInfo(_)).Times(0);
-  EXPECT_CALL(output_cb_, Run(_)).Times(1);
   image_provider_raw_->ProvideOneRequestedImage(&other_image_record);
   base::RunLoop().RunUntilIdle();
 }
 
 TEST_F(VideoFrameFactoryImplTest, NullYCbCrInfoDoesntCrash) {
-  // Sending a null YCbCrInfo then requesting a frame shouldn't cause a crash.
-  // See https://crbug.com/1007196 .
+  // We will be driving callback by ourselves in this test.
+  ycbcr_helper_raw_->set_run_callback_automatically(false);
+
+  // Expect call to get info for the first frame.
+  EXPECT_CALL(*ycbcr_helper_raw_, MockGetFrameInfo(_)).Times(1);
+
   RequestVideoFrame();
+
+  // Provide info.  It should send image request.
+  ycbcr_helper_raw_->RunWithYcbCrInfo(false);
+  base::RunLoop().RunUntilIdle();
+
+  testing::Mock::VerifyAndClearExpectations(ycbcr_helper_raw_);
+
+  // Fulfilling image request should provide video frame.
+  EXPECT_CALL(output_cb_, Run(_)).Times(1);
+
   auto image_record = MakeImageRecord();
-  EXPECT_CALL(*ycbcr_helper_raw_,
-              MockGetYCbCrInfo(image_record.codec_image_holder))
-      .Times(1);
-  image_record.is_vulkan = true;
   image_provider_raw_->ProvideOneRequestedImage(&image_record);
   base::RunLoop().RunUntilIdle();
 
-  // Provide an empty YCbCrInfo.
+  // Verify that we will get call to GetFrameInfo as previous one failed.
+  EXPECT_CALL(*ycbcr_helper_raw_, MockGetFrameInfo(_)).Times(1);
   EXPECT_CALL(output_cb_, Run(_)).Times(1);
-  std::move(ycbcr_helper_raw_->cb_).Run(base::nullopt);
-  base::RunLoop().RunUntilIdle();
 
-  // It shouldn't crash on the next frame.  crbug.com/1007196
   RequestVideoFrame();
+  ycbcr_helper_raw_->RunWithYcbCrInfo(true);
+  base::RunLoop().RunUntilIdle();
+
   auto other_image_record = MakeImageRecord();
-  other_image_record.is_vulkan = true;
-  // Should still call the helper, since it didn't get YCbCrInfo last time.
-  EXPECT_CALL(*ycbcr_helper_raw_,
-              MockGetYCbCrInfo(other_image_record.codec_image_holder))
-      .Times(1);
-  // Since we aren't sending YCbCr info, it won't call us back with a frame.
+  // If the helper hasn't been destroyed, then we don't expect it to be called.
   image_provider_raw_->ProvideOneRequestedImage(&other_image_record);
   base::RunLoop().RunUntilIdle();
 }

media/gpu/android/ycbcr_helper.cc

-// Copyright 2019 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 "media/gpu/android/ycbcr_helper.h"
-
-#include "gpu/command_buffer/service/shared_image_video.h"
-#include "gpu/ipc/service/command_buffer_stub.h"
-#include "gpu/ipc/service/gpu_channel.h"
-#include "gpu/ipc/service/gpu_channel_manager.h"
-
-namespace media {
-
-// Concrete implementation of YCbCrHelper that renders output buffers and gets
-// the YCbCrInfo they need.
-class YCbCrHelperImpl : public YCbCrHelper,
-                        public gpu::CommandBufferStub::DestructionObserver {
- public:
-  YCbCrHelperImpl(SharedImageVideoProvider::GetStubCB get_stub_cb) {
-    stub_ = get_stub_cb.Run();
-    if (stub_)
-      stub_->AddDestructionObserver(this);
-  }
-
-  ~YCbCrHelperImpl() override {
-    if (stub_)
-      stub_->RemoveDestructionObserver(this);
-  }
-
-  // YCbCrHelper
-  void GetYCbCrInfo(
-      scoped_refptr<CodecImageHolder> codec_image_holder,
-      base::OnceCallback<void(OptionalInfo ycbcr_info)> cb) override {
-    // If we don't have the info cached, then try to get it.  If we have gotten
-    // it, then don't try again.  Assume that our caller asked for it before it
-    // got the results back.  We don't want to render more frames to the front
-    // buffer if we don't need to.
-    if (!ycbcr_info_)
-      ycbcr_info_ = RenderImageAndGetYCbCrInfo(std::move(codec_image_holder));
-
-    // Whether we got it or not, send it along.
-    std::move(cb).Run(ycbcr_info_);
-  }
-
-  void OnWillDestroyStub(bool have_context) override {
-    DCHECK(stub_);
-    stub_ = nullptr;
-  }
-
- private:
-  // Render the codec output buffer, and use it to get the YCbCrInfo.
-  OptionalInfo RenderImageAndGetYCbCrInfo(
-      scoped_refptr<CodecImageHolder> codec_image_holder) {
-    gpu::ContextResult result;
-    if (!stub_)
-      return base::nullopt;
-
-    auto shared_context =
-        stub_->channel()->gpu_channel_manager()->GetSharedContextState(&result);
-    auto context_provider =
-        (result == gpu::ContextResult::kSuccess) ? shared_context : nullptr;
-    if (!context_provider)
-      return base::nullopt;
-
-    return gpu::SharedImageVideo::GetYcbcrInfo(
-        codec_image_holder->codec_image_raw(), context_provider);
-  }
-
-  gpu::CommandBufferStub* stub_ = nullptr;
-
-  OptionalInfo ycbcr_info_;
-};
-
-// static
-base::SequenceBound<YCbCrHelper> YCbCrHelper::Create(
-    scoped_refptr<base::SequencedTaskRunner> gpu_task_runner,
-    SharedImageVideoProvider::GetStubCB get_stub_cb) {
-  return base::SequenceBound<YCbCrHelperImpl>(std::move(gpu_task_runner),
-                                              std::move(get_stub_cb));
-}
-
-}  // namespace media

media/mojo/services/gpu_mojo_media_client.cc

@@ -241,8 +241,8 @@ std::unique_ptr<VideoDecoder> GpuMojoMediaClient::CreateVideoDecoder(
       // ignored.  If we can tell that here, then VideoFrameFactory can use it
       // as a signal about whether it's supposed to get YCbCrInfo rather than
       // requiring the provider to set |is_vulkan| in the ImageRecord.
-      auto ycbcr_helper =
-          YCbCrHelper::Create(gpu_task_runner_, std::move(get_stub_cb));
+      auto frame_info_helper =
+          FrameInfoHelper::Create(gpu_task_runner_, std::move(get_stub_cb));
       video_decoder = std::make_unique<MediaCodecVideoDecoder>(
           gpu_preferences_, gpu_feature_info_, media_log->Clone(),
           DeviceInfo::GetInstance(),
@@ -253,7 +253,7 @@ std::unique_ptr<VideoDecoder> GpuMojoMediaClient::CreateVideoDecoder(
           std::make_unique<VideoFrameFactoryImpl>(
               gpu_task_runner_, gpu_preferences_, std::move(image_provider),
               MaybeRenderEarlyManager::Create(gpu_task_runner_),
-              std::move(ycbcr_helper)));
+              std::move(frame_info_helper)));
 
 #elif BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
       if (IsNewAcceleratedVideoDecoderUsed(gpu_preferences_)) {