[go: up one dir, main page]

[Merge=M104] [CodecImage] Verify CopyTexImage() is functionally a no-op

We are looking to eliminate StreamTextureSharedImageInterface
subclassing GLImage as part of eliminating GLImage as a public
interface. The blocker to doing so is that
SharedImageVideoSurfaceTexture passes a
StreamTextureSharedImageInterface instance to
AbstractTexture::BindStreamTextureImage(), which operates on the
passed-in object as a GLImage. That invocation will in turn result in
the StreamTextureSharedImageInterface instance's BindTexImage() or its
CopyTexImage() method being invoked.

StreamTextureSharedImageInterface has two implementations, and their
implementations of these methods are as follows:

- StreamTexture::BindTexImage(): no-op
- StreamTexture::CopyTexImage(): no-op
- CodecImage::BindTexImage(): no-op
- CodecImage::CopyTexImage(): renders to front buffer

Hence, we need to focus only on CodecImage::CopyTexImage(). Our
suspicion is that it is currently always the case that the image has
already been rendered to the front buffer at this point, making
CodecImage::CopyTexImage() itself a no-op.

To verify this hypothesis, this CL adds a DumpWithoutCrashing() call
if the hypothesis is violated.

(cherry picked from commit a6030cc74f9c8145bf424ebe89877bb68b4639c0)

Bug: 1310020
Change-Id: I070c696063a5c92506dd33a546055d9f9b078e74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3750920
Reviewed-by: vikas soni <vikassoni@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1022104}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3755296
Auto-Submit: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/branch-heads/5112@{#811}
Cr-Branched-From: b13d3fe7b3c47a56354ef54b221008afa754412e-refs/heads/main@{#1012729}
diff --git a/media/gpu/android/codec_image.cc b/media/gpu/android/codec_image.cc
index f86895b..d81393d 100644
--- a/media/gpu/android/codec_image.cc
+++ b/media/gpu/android/codec_image.cc
@@ -10,6 +10,7 @@
 
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
 #include "base/callback_helpers.h"
+#include "base/debug/dump_without_crashing.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "gpu/config/gpu_finch_features.h"
@@ -117,6 +118,15 @@
     return false;
   }
 
+  // Our hypothesis is that in actuality the rendering to the front buffer and
+  // binding of the image have always already occurred by the time that this
+  // method is called. The below DumpWithoutCrashing() call serves to verify
+  // whether this hypothesis is correct. See crbug.com/1310020 for details.
+  // TODO(crbug.com/1310020): Remove this code as part of removing this entire
+  // function once we have verified that it is indeed no longer needed.
+  if (!output_buffer_renderer_->was_rendered_to_front_buffer())
+    base::debug::DumpWithoutCrashing();
+
   // On some devices GL_TEXTURE_BINDING_EXTERNAL_OES is not supported as
   // glGetIntegerv() parameter. In this case the value of |texture_id| will be
   // zero and we assume that it is properly bound to TextureOwner's texture id.