Revert "Switch VideoFrameFactoryImpl to use a SharedImageVideoProvider"

This reverts commit 579b12b9.

Reason for revert: crbug.com/968470 (want to revert before branch cut)

Original change's description:
> Switch VideoFrameFactoryImpl to use a SharedImageVideoProvider
> 
> This CL creates DirectSharedImageVideoProvider, which does a hop to
> the gpu main thread for every SharedImageVideo request.  This is
> almost identical to what was happening before, just refactored.
> 
> GpuVideoFrameFactory is now an implementation detail of the provider
> rather than VideoFrameFactoryImpl.
> 
> A follow-up CL will provide an implementation of
> SharedImageVideoProvider that maintains a pool of SharedImageVideo
> objects to provide without hopping to the gpu main thread on the
> critical path.  It will still post a "MaybeRenderEarly" to the main
> thread, but that doesn't need to hold up delivery of the VideoFrame
> to the renderer.
> 
> Change-Id: Ia318862daf610327e716515f020b3eeb934dd012
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628154
> Commit-Queue: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#663361}

TBR=dalecurtis@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

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

media/gpu/BUILD.gn

@@ -127,8 +127,6 @@ component("gpu") {
       "android/promotion_hint_aggregator_impl.h",
       "android/shared_image_video.cc",
       "android/shared_image_video.h",
-      "android/shared_image_video_provider.cc",
-      "android/shared_image_video_provider.h",
       "android/surface_chooser_helper.cc",
       "android/surface_chooser_helper.h",
       "android/surface_texture_gl_owner.cc",

media/gpu/android/codec_image.cc

@@ -41,24 +41,20 @@ std::unique_ptr<ui::ScopedMakeCurrent> MakeCurrentIfNeeded(
 
 }  // namespace
 
-CodecImage::CodecImage() = default;
+CodecImage::CodecImage(
+    std::unique_ptr<CodecOutputBuffer> output_buffer,
+    scoped_refptr<TextureOwner> texture_owner,
+    PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb)
+    : phase_(Phase::kInCodec),
+      output_buffer_(std::move(output_buffer)),
+      texture_owner_(std::move(texture_owner)),
+      promotion_hint_cb_(std::move(promotion_hint_cb)) {}
 
 CodecImage::~CodecImage() {
   if (destruction_cb_)
     std::move(destruction_cb_).Run(this);
 }
 
-void CodecImage::Initialize(
-    std::unique_ptr<CodecOutputBuffer> output_buffer,
-    scoped_refptr<TextureOwner> texture_owner,
-    PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb) {
-  DCHECK(output_buffer);
-  phase_ = Phase::kInCodec;
-  output_buffer_ = std::move(output_buffer);
-  texture_owner_ = std::move(texture_owner);
-  promotion_hint_cb_ = std::move(promotion_hint_cb);
-}
-
 void CodecImage::SetDestructionCb(DestructionCb destruction_cb) {
   destruction_cb_ = std::move(destruction_cb);
 }

media/gpu/android/codec_image.h

@@ -33,16 +33,9 @@ class MEDIA_GPU_EXPORT CodecImage : public gpu::gles2::GLStreamTextureImage {
   // since CodecImageGroup calls the same cb for multiple images.
   using DestructionCb = base::RepeatingCallback<void(CodecImage*)>;
 
-  CodecImage();
-
-  // (Re-)Initialize this CodecImage to use |output_buffer| et. al.
-  //
-  // 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,
-      scoped_refptr<TextureOwner> texture_owner,
-      PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb);
+  CodecImage(std::unique_ptr<CodecOutputBuffer> output_buffer,
+             scoped_refptr<TextureOwner> texture_owner,
+             PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb);
 
   void SetDestructionCb(DestructionCb destruction_cb);
 
@@ -139,7 +132,7 @@ class MEDIA_GPU_EXPORT CodecImage : public gpu::gles2::GLStreamTextureImage {
   bool RenderToOverlay();
 
   // The phase of the image buffer's lifecycle.
-  Phase phase_ = Phase::kInvalidated;
+  Phase phase_;
 
   // The buffer backing this image.
   std::unique_ptr<CodecOutputBuffer> output_buffer_;

media/gpu/android/codec_image_group_unittest.cc

@@ -43,7 +43,10 @@ class CodecImageGroupWithDestructionHook : public CodecImageGroup {
 // CodecImage with a mocked ReleaseCodecBuffer.
 class MockCodecImage : public CodecImage {
  public:
-  MockCodecImage() = default;
+  MockCodecImage()
+      : CodecImage(nullptr,
+                   nullptr,
+                   PromotionHintAggregator::NotifyPromotionHintCB()) {}
 
   MOCK_METHOD0(ReleaseCodecBuffer, void());
 

media/gpu/android/codec_image_unittest.cc

@@ -81,8 +81,7 @@ class CodecImageTest : public testing::Test {
       CodecImage::DestructionCb destruction_cb = base::DoNothing()) {
     std::unique_ptr<CodecOutputBuffer> buffer;
     wrapper_->DequeueOutputBuffer(nullptr, nullptr, &buffer);
-    scoped_refptr<CodecImage> image = new CodecImage();
-    image->Initialize(
+    scoped_refptr<CodecImage> image = new CodecImage(
         std::move(buffer), kind == kTextureOwner ? texture_owner_ : nullptr,
         base::BindRepeating(&PromotionHintReceiver::OnPromotionHint,
                             base::Unretained(&promotion_hint_receiver_)));

media/gpu/android/shared_image_video_provider.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/shared_image_video_provider.h"
-
-namespace media {
-
-SharedImageVideoProvider::ImageSpec::ImageSpec(
-    gfx::Size our_size,
-    scoped_refptr<CodecImageGroup> group)
-    : size(our_size), image_group(std::move(group)) {}
-SharedImageVideoProvider::ImageSpec::ImageSpec(const ImageSpec&) = default;
-SharedImageVideoProvider::ImageSpec::~ImageSpec() = default;
-
-SharedImageVideoProvider::ImageRecord::ImageRecord() = default;
-SharedImageVideoProvider::ImageRecord::ImageRecord(ImageRecord&&) = default;
-SharedImageVideoProvider::ImageRecord::~ImageRecord() = default;
-
-}  // namespace media

media/gpu/android/shared_image_video_provider.h

-// 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.
-
-#ifndef MEDIA_GPU_ANDROID_SHARED_IMAGE_VIDEO_PROVIDER_H_
-#define MEDIA_GPU_ANDROID_SHARED_IMAGE_VIDEO_PROVIDER_H_
-
-#include "base/callback.h"
-#include "gpu/command_buffer/service/mailbox_manager.h"
-#include "gpu/ipc/common/vulkan_ycbcr_info.h"
-#include "media/gpu/android/codec_image_group.h"
-#include "media/gpu/android/promotion_hint_aggregator.h"
-#include "media/gpu/media_gpu_export.h"
-#include "ui/gfx/geometry/size.h"
-
-namespace gpu {
-struct SyncToken;
-}  // namespace gpu
-
-namespace media {
-
-// Provider class for shared images.
-class MEDIA_GPU_EXPORT SharedImageVideoProvider {
- public:
-  // Description of the underlying properties of the shared image.
-  struct ImageSpec {
-    ImageSpec(gfx::Size size, scoped_refptr<CodecImageGroup> group);
-    ImageSpec(const ImageSpec&);
-    ~ImageSpec();
-
-    // Size of the underlying texture.
-    gfx::Size size;
-
-    // Image group to which the CodecImage should belong.  In other words, this
-    // indicates that the image will share the same underlying TextureOwner.
-    scoped_refptr<CodecImageGroup> image_group;
-
-    // TODO: Include other properties, if they matter, like texture format.
-
-    bool operator==(const ImageSpec&);
-  };
-
-  using ReleaseCB = base::OnceCallback<void(const gpu::SyncToken&)>;
-
-  // Description of the image that's being provided to the client.
-  struct ImageRecord {
-    ImageRecord();
-    ImageRecord(ImageRecord&&);
-    ~ImageRecord();
-
-    // Mailbox to which this shared image is bound.
-    gpu::Mailbox mailbox;
-
-    // Sampler conversion information which is used in vulkan context.
-    base::Optional<gpu::VulkanYCbCrInfo> ycbcr_info;
-
-    // Release callback.  When this is called (or dropped), the image will be
-    // considered unused.
-    ReleaseCB release_cb;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(ImageRecord);
-  };
-
-  SharedImageVideoProvider() = default;
-  virtual ~SharedImageVideoProvider() = default;
-
-  using ImageReadyCB = base::OnceCallback<void(ImageRecord)>;
-
-  // Call |cb| when we have a shared image that matches |spec|.  We may call
-  // |cb| back before returning, or we might post it for later.
-  // |output_buffer|, |texture_owner|, and |promotion_hint_cb| probably should
-  // not be provided to the provider, but need to be refactored a bit first.
-  // They will be used to configure the SharedImageVideo to display the buffer
-  // on the given texture, and return promotion hints.
-  virtual void RequestImage(
-      ImageReadyCB cb,
-      const ImageSpec& spec,
-      std::unique_ptr<CodecOutputBuffer> output_buffer,
-      scoped_refptr<TextureOwner> texture_owner,
-      PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb) = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(SharedImageVideoProvider);
-};
-
-}  // namespace media
-
-#endif  // MEDIA_GPU_ANDROID_SHARED_IMAGE_VIDEO_PROVIDER_H_

media/gpu/android/video_frame_factory_impl.h

@@ -18,7 +18,6 @@
 #include "media/base/video_frame.h"
 #include "media/gpu/android/codec_image.h"
 #include "media/gpu/android/codec_wrapper.h"
-#include "media/gpu/android/shared_image_video_provider.h"
 #include "media/gpu/android/surface_texture_gl_owner.h"
 #include "media/gpu/android/video_frame_factory.h"
 #include "media/gpu/gles2_decoder_helper.h"
@@ -27,6 +26,7 @@
 
 namespace media {
 class CodecImageGroup;
+class GpuVideoFrameFactory;
 
 // VideoFrameFactoryImpl creates CodecOutputBuffer backed VideoFrames and tries
 // to eagerly render them to their surface to release the buffers back to the
@@ -40,7 +40,8 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
   // cleaned up properly. The release callback may be called from any thread.
   using ImageReadyCB =
       base::OnceCallback<void(gpu::Mailbox mailbox,
-                              VideoFrame::ReleaseMailboxCB release_cb)>;
+                              VideoFrame::ReleaseMailboxCB release_cb,
+                              base::Optional<gpu::VulkanYCbCrInfo> ycbcr_info)>;
 
   // |get_stub_cb| will be run on |gpu_task_runner|.
   VideoFrameFactoryImpl(
@@ -80,9 +81,12 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
                            VideoPixelFormat pixel_format,
                            OverlayMode overlay_mode,
                            bool enable_threaded_texture_mailboxes,
-                           SharedImageVideoProvider::ImageRecord record);
+                           gpu::Mailbox mailbox,
+                           VideoFrame::ReleaseMailboxCB release_cb,
+                           base::Optional<gpu::VulkanYCbCrInfo> ycbcr_info);
 
-  std::unique_ptr<SharedImageVideoProvider> image_provider_;
+  // The gpu thread side of the implementation.
+  std::unique_ptr<GpuVideoFrameFactory> gpu_video_frame_factory_;
   scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner_;
   GetStubCb get_stub_cb_;
 
@@ -94,65 +98,47 @@ class MEDIA_GPU_EXPORT VideoFrameFactoryImpl : public VideoFrameFactory {
   // Is the sync mailbox manager enabled?
   bool enable_threaded_texture_mailboxes_ = false;
 
-  // Current group that new CodecImages should belong to.  Do not use this on
-  // our thread; everything must be posted to the gpu main thread, including
-  // destruction of it.
-  scoped_refptr<CodecImageGroup> image_group_;
-
   SEQUENCE_CHECKER(sequence_checker_);
   DISALLOW_COPY_AND_ASSIGN(VideoFrameFactoryImpl);
 };
 
-// GpuSharedImageVideoFactory creates SharedImageVideo objects.  It must be run
-// on the gpu main thread.
-//
-// GpuSharedImageVideoFactory is an implementation detail of
-// DirectSharedImageVideoProvider.  It really should be split out into its own
-// file from here, but in the interest of making CL diffs more readable, that
-// is left for later.
-class GpuSharedImageVideoFactory
+// GpuVideoFrameFactory is an implementation detail of VideoFrameFactoryImpl. It
+// may be created on any thread but only accessed on the gpu thread thereafter.
+class GpuVideoFrameFactory
     : public gpu::CommandBufferStub::DestructionObserver {
  public:
-  GpuSharedImageVideoFactory();
-  ~GpuSharedImageVideoFactory() override;
-
-  // TODO(liberato): Now that this is used as part of an image provider, it
-  // doesn't really make sense for it to call back with a TextureOwner.  That
-  // should be handled by VideoFrameFactoryImpl if it wants.
-  void Initialize(VideoFrameFactory::OverlayMode overlay_mode,
-                  VideoFrameFactory::GetStubCb get_stub_cb,
-                  VideoFrameFactory::InitCb init_cb);
-
-  // Similar to SharedImageVideoProvider::ImageReadyCB, but provides additional
-  // details for the provider that's using us.
-  using FactoryImageReadyCB =
-      base::OnceCallback<void(SharedImageVideoProvider::ImageRecord record,
-                              scoped_refptr<CodecImage> codec_image)>;
-
-  // Creates a SharedImage for |spec|, and returns it via the callback.
-  // TODO(liberato): |texture_owner| is only needed to get the service id, to
-  // create the per-frame texture.  All of that is only needed for legacy
-  // mailbox support, where we have to have one texture per CodecImage.
-  void CreateImage(FactoryImageReadyCB cb,
-                   const SharedImageVideoProvider::ImageSpec& spec,
-                   scoped_refptr<TextureOwner> texture_owner);
+  GpuVideoFrameFactory();
+  ~GpuVideoFrameFactory() override;
+
+  scoped_refptr<TextureOwner> Initialize(
+      VideoFrameFactory::OverlayMode overlay_mode,
+      VideoFrameFactory::GetStubCb get_stub_cb);
+
+  // Creates a SharedImage for |output_buffer|, and returns it via the callback.
+  void CreateImage(
+      std::unique_ptr<CodecOutputBuffer> output_buffer,
+      scoped_refptr<TextureOwner> texture_owner,
+      PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb,
+      VideoFrameFactoryImpl::ImageReadyCB image_ready_cb,
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+
+  // Set our image group.  Must be called before the first call to
+  // CreateVideoFrame occurs.
+  void SetImageGroup(scoped_refptr<CodecImageGroup> image_group);
 
  private:
   // Creates a SharedImage for |mailbox|, and returns success or failure.
-  bool CreateImageInternal(const SharedImageVideoProvider::ImageSpec& spec,
-                           scoped_refptr<TextureOwner> texture_owner,
-                           gpu::Mailbox mailbox,
-                           scoped_refptr<CodecImage> image);
+  bool CreateImageInternal(
+      std::unique_ptr<CodecOutputBuffer> output_buffer,
+      scoped_refptr<TextureOwner> texture_owner,
+      gpu::Mailbox mailbox,
+      PromotionHintAggregator::NotifyPromotionHintCB promotion_hint_cb);
 
   void OnWillDestroyStub(bool have_context) override;
 
   // Removes |image| from |images_|.
   void OnImageDestructed(CodecImage* image);
 
-  // Set our image group.  Must be called before the first call to
-  // CreateVideoFrame occurs.
-  void SetImageGroup(scoped_refptr<CodecImageGroup> image_group);
-
   // Outstanding images that should be considered for early rendering.
   std::vector<CodecImage*> images_;
 
@@ -170,8 +156,8 @@ class GpuSharedImageVideoFactory
   base::Optional<gpu::VulkanYCbCrInfo> ycbcr_info_;
 
   THREAD_CHECKER(thread_checker_);
-  base::WeakPtrFactory<GpuSharedImageVideoFactory> weak_factory_;
-  DISALLOW_COPY_AND_ASSIGN(GpuSharedImageVideoFactory);
+  base::WeakPtrFactory<GpuVideoFrameFactory> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(GpuVideoFrameFactory);
 };
 
 namespace internal {