Revert "Enable SharedImages for VTVideoDecodeAccelerator"

This reverts commit 8b7817c7.

Reason for revert: assertion failures in the default configuration of webgl_conformance_tests per crbug.com/1132730 .

Original change's description:
> Enable SharedImages for VTVideoDecodeAccelerator
>
> Add a method to media::VideoDecodeAccelerator to indicate whether
> SharedImage-backed picture buffers are supported. Make this return
> true only for the VTVideoDecodeAccelerator.
>
> Add a parameter to media::PictureBufferManager::CreatePictureBuffers
> to indicate if SharedImage-backed PictureBuffers are desired.
>
> Hook the two of these together in the ProvidePictureBuffersAsync
> method of media::VdaVideoDecoder.
>
> Video playback fails with the Metal-based command decoder, because
> it binds IOSurfaces as TEXTURE_2D, not TEXTURE_RECTANGLE. Plumb
> through a flag to specify if TEXTURE_RECTANGLE support is present.
>
> Bug: 1108909
> Change-Id: I1e42aa38e122714517724cee87eb51987645e410
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427545
> Commit-Queue: ccameron <ccameron@chromium.org>
> Reviewed-by: ccameron <ccameron@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#811077}

TBR=ccameron@chromium.org,sandersd@chromium.org,jonahr@google.com

Change-Id: I21aa3be7737dc3a2d3d9d0d0cceb4fea2b1fca54
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1108909, 1132730
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435779Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811306}

media/gpu/command_buffer_helper.cc

@@ -199,15 +199,6 @@ class CommandBufferHelperImpl
         ->is_passthrough_cmd_decoder();
   }
 
-  bool SupportsTextureRectangle() const override {
-    if (!stub_)
-      return false;
-    return stub_->decoder_context()
-        ->GetFeatureInfo()
-        ->feature_flags()
-        .arb_texture_rectangle;
-  }
-
  private:
   ~CommandBufferHelperImpl() override {
     DVLOG(1) << __func__;

media/gpu/command_buffer_helper.h

@@ -137,9 +137,6 @@ class MEDIA_GPU_EXPORT CommandBufferHelper
   // Is the backing command buffer passthrough (versus validating).
   virtual bool IsPassthrough() const = 0;
 
-  // Does this command buffer support ARB_texture_rectangle.
-  virtual bool SupportsTextureRectangle() const = 0;
-
  protected:
   explicit CommandBufferHelper(
       scoped_refptr<base::SequencedTaskRunner> task_runner);

media/gpu/gpu_video_decode_accelerator_helpers.h

@@ -112,9 +112,6 @@ struct MEDIA_GPU_EXPORT GpuVideoDecodeGLClient {
 
   // Whether or not the command buffer is passthrough.
   bool is_passthrough = false;
-
-  // Whether or not ARB_texture_rectangle is present.
-  bool supports_arb_texture_rectangle = false;
 };
 
 // Convert vector of VDA::SupportedProfile to vector of

media/gpu/ipc/service/gpu_video_decode_accelerator.cc

@@ -191,10 +191,6 @@ GpuVideoDecodeAccelerator::GpuVideoDecodeAccelerator(
       base::BindRepeating(&CreateAbstractTexture, stub_->AsWeakPtr());
   gl_client_.is_passthrough =
       stub_->decoder_context()->GetFeatureInfo()->is_passthrough_cmd_decoder();
-  gl_client_.supports_arb_texture_rectangle = stub_->decoder_context()
-                                                  ->GetFeatureInfo()
-                                                  ->feature_flags()
-                                                  .arb_texture_rectangle;
 }
 
 GpuVideoDecodeAccelerator::~GpuVideoDecodeAccelerator() {

media/gpu/ipc/service/picture_buffer_manager.cc

@@ -13,7 +13,6 @@
 #include "base/logging.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
-#include "build/build_config.h"
 #include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/video_util.h"
 
@@ -28,6 +27,11 @@ int32_t NextID(int32_t* counter) {
   return value;
 }
 
+bool UseSharedImage() {
+  // TODO(https://crbug.com/1108909): Enable shared image use on macOS.
+  return false;
+}
+
 class PictureBufferManagerImpl : public PictureBufferManager {
  public:
   explicit PictureBufferManagerImpl(
@@ -75,8 +79,7 @@ class PictureBufferManagerImpl : public PictureBufferManager {
       VideoPixelFormat pixel_format,
       uint32_t planes,
       gfx::Size texture_size,
-      uint32_t texture_target,
-      bool use_shared_image) override {
+      uint32_t texture_target) override {
     DVLOG(2) << __func__;
     DCHECK(gpu_task_runner_);
     DCHECK(gpu_task_runner_->BelongsToCurrentThread());
@@ -84,7 +87,7 @@ class PictureBufferManagerImpl : public PictureBufferManager {
     DCHECK(planes);
     DCHECK_LE(planes, static_cast<uint32_t>(VideoFrame::kMaxPlanes));
 
-    if (!use_shared_image) {
+    if (!UseSharedImage()) {
       // TODO(sandersd): Consider requiring that CreatePictureBuffers() is
       // called with the context current.
       if (!command_buffer_helper_->MakeContextCurrent()) {
@@ -95,10 +98,9 @@ class PictureBufferManagerImpl : public PictureBufferManager {
 
     std::vector<PictureBuffer> picture_buffers;
     for (uint32_t i = 0; i < count; i++) {
-      PictureBufferData picture_data = {pixel_format, texture_size,
-                                        use_shared_image};
+      PictureBufferData picture_data = {pixel_format, texture_size};
 
-      if (!use_shared_image) {
+      if (!UseSharedImage()) {
         for (uint32_t j = 0; j < planes; j++) {
           // Create a texture for this plane.
           GLuint service_id = command_buffer_helper_->CreateTexture(
@@ -217,8 +219,7 @@ class PictureBufferManagerImpl : public PictureBufferManager {
 
     // If this |picture| has a SharedImage, then keep a reference to the
     // SharedImage in |picture_buffer_data| and update the gpu::MailboxHolder.
-    DCHECK_EQ(picture_buffer_data.use_shared_image,
-              !!picture.scoped_shared_image());
+    DCHECK_EQ(UseSharedImage(), !!picture.scoped_shared_image());
     if (auto scoped_shared_image = picture.scoped_shared_image()) {
       picture_buffer_data.scoped_shared_image = scoped_shared_image;
       picture_buffer_data.mailbox_holders[0] =
@@ -349,7 +350,6 @@ class PictureBufferManagerImpl : public PictureBufferManager {
   struct PictureBufferData {
     VideoPixelFormat pixel_format;
     gfx::Size texture_size;
-    bool use_shared_image = false;
     std::vector<GLuint> service_ids;
     gpu::MailboxHolder mailbox_holders[VideoFrame::kMaxPlanes];
     scoped_refptr<Picture::ScopedSharedImage> scoped_shared_image;

media/gpu/ipc/service/picture_buffer_manager.h

@@ -62,7 +62,6 @@ class PictureBufferManager
   // |planes|: Number of image planes (textures) in the picture.
   // |texture_size|: Size of textures to create.
   // |texture_target|: Type of textures to create.
-  // |use_shared_image|: True if the created buffers should use shared images.
   //
   // Must be called on the GPU thread.
   //
@@ -78,8 +77,7 @@ class PictureBufferManager
       VideoPixelFormat pixel_format,
       uint32_t planes,
       gfx::Size texture_size,
-      uint32_t texture_target,
-      bool use_shared_image) = 0;
+      uint32_t texture_target) = 0;
 
   // Dismisses a picture buffer from the pool.
   //

media/gpu/ipc/service/picture_buffer_manager_unittest.cc

@@ -41,8 +41,7 @@ class PictureBufferManagerImplTest : public testing::Test {
 
   std::vector<PictureBuffer> CreateARGBPictureBuffers(uint32_t count) {
     return pbm_->CreatePictureBuffers(count, PIXEL_FORMAT_ARGB, 1,
-                                      gfx::Size(320, 240), GL_TEXTURE_2D,
-                                      false /* use_shared_image */);
+                                      gfx::Size(320, 240), GL_TEXTURE_2D);
   }
 
   PictureBuffer CreateARGBPictureBuffer() {

media/gpu/ipc/service/vda_video_decoder.cc

@@ -76,8 +76,6 @@ std::unique_ptr<VideoDecodeAccelerator> CreateAndInitializeVda(
       &CommandBufferHelper::MakeContextCurrent, command_buffer_helper);
   gl_client.bind_image = base::BindRepeating(&BindImage, command_buffer_helper);
   gl_client.is_passthrough = command_buffer_helper->IsPassthrough();
-  gl_client.supports_arb_texture_rectangle =
-      command_buffer_helper->SupportsTextureRectangle();
 
   std::unique_ptr<GpuVideoDecodeAcceleratorFactory> factory =
       GpuVideoDecodeAcceleratorFactory::Create(gl_client);
@@ -541,8 +539,7 @@ void VdaVideoDecoder::ProvidePictureBuffersAsync(uint32_t count,
 
   std::vector<PictureBuffer> picture_buffers =
       picture_buffer_manager_->CreatePictureBuffers(
-          count, pixel_format, planes, texture_size, texture_target,
-          vda_->SupportsSharedImagePictureBuffers());
+          count, pixel_format, planes, texture_size, texture_target);
   if (picture_buffers.empty()) {
     parent_task_runner_->PostTask(
         FROM_HERE,

media/gpu/mac/vt_video_decode_accelerator_mac.cc

@@ -1533,10 +1533,7 @@ bool VTVideoDecodeAccelerator::SendFrame(const Frame& frame) {
     gpu::Mailbox mailbox = gpu::Mailbox::GenerateForSharedImage();
 
     gpu::SharedImageBackingGLCommon::InitializeGLTextureParams gl_params;
-    // ANGLE-on-Metal exposes IOSurfaces via GL_TEXTURE_2D. Be robust to that.
-    gl_params.target = gl_client_.supports_arb_texture_rectangle
-                           ? GL_TEXTURE_RECTANGLE_ARB
-                           : GL_TEXTURE_2D;
+    gl_params.target = GL_TEXTURE_RECTANGLE_ARB;
     gl_params.internal_format = gl_format;
     gl_params.format = gl_format;
     gl_params.type = GL_UNSIGNED_BYTE;
@@ -1570,7 +1567,7 @@ bool VTVideoDecodeAccelerator::SendFrame(const Frame& frame) {
         gpu_task_runner_);
     scoped_shared_image = scoped_refptr<Picture::ScopedSharedImage>(
         new Picture::ScopedSharedImage(
-            mailbox, gl_params.target,
+            mailbox, GL_TEXTURE_RECTANGLE_ARB,
             std::move(destroy_shared_image_callback)));
   } else {
     if (!gl_client_.bind_image.Run(picture_info->client_texture_id,
@@ -1690,10 +1687,6 @@ bool VTVideoDecodeAccelerator::TryToSetupDecodeOnSeparateThread(
   return false;
 }
 
-bool VTVideoDecodeAccelerator::SupportsSharedImagePictureBuffers() const {
-  return true;
-}
-
 // static
 VideoDecodeAccelerator::SupportedProfiles
 VTVideoDecodeAccelerator::GetSupportedProfiles() {

media/gpu/mac/vt_video_decode_accelerator_mac.h

@@ -63,7 +63,6 @@ class VTVideoDecodeAccelerator : public VideoDecodeAccelerator,
       const base::WeakPtr<Client>& decode_client,
       const scoped_refptr<base::SingleThreadTaskRunner>& decode_task_runner)
       override;
-  bool SupportsSharedImagePictureBuffers() const override;
 
   // MemoryDumpProvider implementation.
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,

media/gpu/test/fake_command_buffer_helper.cc

@@ -168,8 +168,4 @@ bool FakeCommandBufferHelper::IsPassthrough() const {
   return false;
 }
 
-bool FakeCommandBufferHelper::SupportsTextureRectangle() const {
-  return false;
-}
-
 }  // namespace media

media/gpu/test/fake_command_buffer_helper.h

@@ -62,7 +62,6 @@ class FakeCommandBufferHelper : public CommandBufferHelper {
                         base::OnceClosure done_cb) override;
   void SetWillDestroyStubCB(WillDestroyStubCB will_destroy_stub_cb) override;
   bool IsPassthrough() const override;
-  bool SupportsTextureRectangle() const override;
 
  private:
   ~FakeCommandBufferHelper() override;

media/video/video_decode_accelerator.cc

@@ -101,10 +101,6 @@ GLenum VideoDecodeAccelerator::GetSurfaceInternalFormat() const {
   return GL_RGBA;
 }
 
-bool VideoDecodeAccelerator::SupportsSharedImagePictureBuffers() const {
-  return false;
-}
-
 VideoDecodeAccelerator::SupportedProfile::SupportedProfile()
     : profile(media::VIDEO_CODEC_PROFILE_UNKNOWN), encrypted_only(false) {}
 

media/video/video_decode_accelerator.h

@@ -414,10 +414,6 @@ class MEDIA_EXPORT VideoDecodeAccelerator {
   // TODO(dshwang): after moving to D3D11, remove this. crbug.com/438691
   virtual GLenum GetSurfaceInternalFormat() const;
 
-  // Returns true if the decoder supports SharedImage backed picture buffers.
-  // May be called on any thread at any time.
-  virtual bool SupportsSharedImagePictureBuffers() const;
-
  protected:
   // Do not delete directly; use Destroy() or own it with a scoped_ptr, which
   // will Destroy() it properly by default.