Skip to content

Commit

Permalink
[GPU] Make StreamTextureSharedImageInterface not subclass GLImage
Browse files Browse the repository at this point in the history 
StreamTextureSharedImageInterface subclasses GLImage, which is
problematic as we are trying to eliminate GLImage :).

Preceding CLs have eliminated all client references to
StreamTextureSharedImageInterface as a GLImage. This CL eliminates
StreamTextureSharedImageInterface subclassing GLImage. It is a no-op
functionally.

Bug: 1310020
Change-Id: I351738a554ef65ff58d995b3377eca987f7375e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3748101
Reviewed-by: vikas soni <vikassoni@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084872}
  • Loading branch information
@colinblundell
colinblundell authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent a3b2f9f commit 46d0916
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 253 deletions.
1 change: 1 addition & 0 deletions gpu/command_buffer/service/shared_image/android_video_image_backing.cc
View file
Expand Up @@ -18,6 +18,7 @@
#include "gpu/config/gpu_finch_features.h"
#include "gpu/vulkan/vulkan_device_queue.h"
#include "gpu/vulkan/vulkan_implementation.h"
#include "ui/gfx/gpu_fence.h"

namespace gpu {

Expand Down
12 changes: 9 additions & 3 deletions gpu/command_buffer/service/stream_texture_shared_image_interface.h
View file
Expand Up @@ -6,9 +6,11 @@
#define GPU_COMMAND_BUFFER_SERVICE_STREAM_TEXTURE_SHARED_IMAGE_INTERFACE_H_

#include <memory>

#include "base/android/scoped_hardware_buffer_fence_sync.h"
#include "gpu/gpu_gles2_export.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gl/gl_bindings.h"
#include "ui/gl/gl_image.h"

namespace base::android {
class ScopedHardwareBufferFenceSync;
Expand All @@ -20,7 +22,8 @@ class TextureBase;

// This class is a specialized GLImage that lets SharedImageVideo draw video
// frames.
class GPU_GLES2_EXPORT StreamTextureSharedImageInterface : public gl::GLImage {
class GPU_GLES2_EXPORT StreamTextureSharedImageInterface
: public base::RefCounted<StreamTextureSharedImageInterface> {
public:
enum class BindingsMode {
// Binds image to the texture with service id. Doesn't alter current gl
Expand Down Expand Up @@ -72,7 +75,10 @@ class GPU_GLES2_EXPORT StreamTextureSharedImageInterface : public gl::GLImage {
GetAHardwareBuffer() = 0;

protected:
~StreamTextureSharedImageInterface() override = default;
virtual ~StreamTextureSharedImageInterface() = default;

private:
friend class base::RefCounted<StreamTextureSharedImageInterface>;
};

} // namespace gpu
Expand Down
64 changes: 0 additions & 64 deletions gpu/ipc/service/stream_texture_android.cc
View file
Expand Up @@ -170,33 +170,6 @@ bool StreamTexture::TextureOwnerBindsTextureOnUpdate() {
return texture_owner_->binds_texture_on_update();
}

bool StreamTexture::CopyTexImage(unsigned target) {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
if (target != GL_TEXTURE_EXTERNAL_OES)
return false;

if (!texture_owner_.get())
return false;

GLint texture_id;
glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &texture_id);

// CopyTexImage will only be called for TextureOwner's SurfaceTexture
// implementation which binds texture to TextureOwner's texture_id on update.
// Also ensure that the CopyTexImage() is always called on TextureOwner's
// context.
DCHECK(texture_owner_->binds_texture_on_update());
DCHECK(texture_owner_->GetContext()->IsCurrent(nullptr));
if (texture_id > 0 &&
static_cast<unsigned>(texture_id) != texture_owner_->GetTextureId())
return false;

// UpdateTexImage happens via OnFrameAvailable callback now. And this code
// only runs if |texture_owner| binds texture on update, so there is nothing
// else to do here.
return true;
}

void StreamTexture::OnFrameAvailable() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
has_pending_frame_ = true;
Expand Down Expand Up @@ -240,19 +213,6 @@ void StreamTexture::OnFrameAvailable() {
}
}

gfx::Size StreamTexture::GetSize() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
return coded_size_;
}

unsigned StreamTexture::GetInternalFormat() {
return GL_RGBA;
}

unsigned StreamTexture::GetDataType() {
return GL_UNSIGNED_BYTE;
}

void StreamTexture::StartListening(
mojo::PendingAssociatedRemote<mojom::StreamTextureClient> client) {
client_.Bind(std::move(client));
Expand Down Expand Up @@ -305,30 +265,6 @@ void StreamTexture::UpdateRotatedVisibleSize(
OnFrameAvailable();
}

StreamTexture::BindOrCopy StreamTexture::ShouldBindOrCopy() {
return COPY;
}

bool StreamTexture::BindTexImage(unsigned target) {
NOTREACHED();
return false;
}

void StreamTexture::ReleaseTexImage(unsigned target) {
}

bool StreamTexture::CopyTexSubImage(unsigned target,
const gfx::Point& offset,
const gfx::Rect& rect) {
return false;
}

void StreamTexture::OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
uint64_t process_tracing_id,
const std::string& dump_name) {
// TODO(ericrk): Add OnMemoryDump for GLImages. crbug.com/514914
}

std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
StreamTexture::GetAHardwareBuffer() {
DCHECK(texture_owner_);
Expand Down
21 changes: 2 additions & 19 deletions gpu/ipc/service/stream_texture_android.h
View file
Expand Up @@ -22,7 +22,6 @@
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "ui/gl/android/surface_texture.h"
#include "ui/gl/gl_image.h"

namespace gfx {
class Size;
Expand Down Expand Up @@ -63,24 +62,6 @@ class StreamTexture : public StreamTextureSharedImageInterface,
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::WeakPtr<StreamTexture> weak_stream_texture);

// gl::GLImage implementation:
gfx::Size GetSize() override;
unsigned GetInternalFormat() override;
unsigned GetDataType() override;
BindOrCopy ShouldBindOrCopy() override;
bool BindTexImage(unsigned target) override;
void ReleaseTexImage(unsigned target) override;
bool CopyTexImage(unsigned target) override;
bool CopyTexSubImage(unsigned target,
const gfx::Point& offset,
const gfx::Rect& rect) override;
void SetColorSpace(const gfx::ColorSpace& color_space) override {}
void OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
uint64_t process_tracing_id,
const std::string& dump_name) override;
std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
GetAHardwareBuffer() override;

// gpu::StreamTextureSharedImageInterface implementation.
void ReleaseResources() override {}
bool IsUsingGpuMemory() const override;
Expand All @@ -89,6 +70,8 @@ class StreamTexture : public StreamTextureSharedImageInterface,
TextureBase* GetTextureBase() const override;
void NotifyOverlayPromotion(bool promotion, const gfx::Rect& bounds) override;
bool RenderToOverlay() override;
std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
GetAHardwareBuffer() override;
bool TextureOwnerBindsTextureOnUpdate() override;

gpu::Mailbox CreateSharedImage(const gfx::Size& coded_size);
Expand Down
90 changes: 0 additions & 90 deletions media/gpu/android/codec_image.cc
View file
Expand Up @@ -59,92 +59,6 @@ void CodecImage::NotifyUnused() {
unused_cbs_.clear();
}

gfx::Size CodecImage::GetSize() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
return coded_size_;
}

unsigned CodecImage::GetInternalFormat() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
return GL_RGBA;
}

unsigned CodecImage::GetDataType() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
return GL_UNSIGNED_BYTE;
}

CodecImage::BindOrCopy CodecImage::ShouldBindOrCopy() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);

// If we're using an overlay, then pretend it's bound. That way, we'll get
// calls to ScheduleOverlayPlane. Otherwise, CopyTexImage needs to be called.
return is_texture_owner_backed_ ? COPY : BIND;
}

bool CodecImage::BindTexImage(unsigned target) {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
DCHECK_EQ(BIND, ShouldBindOrCopy());
return true;
}

void CodecImage::ReleaseTexImage(unsigned target) {}

bool CodecImage::CopyTexImage(unsigned target) {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);

// This method is only called for SurfaceTexture implementation which can't be
// thread-safe. Hence the lock which ensures thread safety should be null.
DCHECK(!GetDrDcLockPtr());

TRACE_EVENT0("media", "CodecImage::CopyTexImage");
DCHECK_EQ(COPY, ShouldBindOrCopy());

if (target != GL_TEXTURE_EXTERNAL_OES)
return false;

if (!output_buffer_renderer_)
return true;

GLint texture_id = 0;
glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &texture_id);

// CopyTexImage will only be called for TextureOwner's SurfaceTexture
// implementation which binds texture to TextureOwner's texture_id on update.
DCHECK(output_buffer_renderer_->texture_owner()->binds_texture_on_update());
if (texture_id > 0 &&
static_cast<unsigned>(texture_id) !=
output_buffer_renderer_->texture_owner()->GetTextureId()) {
return false;
}

// Our hypothesis is that in actuality the rendering to the front buffer and
// binding of the image, if possible, 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_
->render_to_front_buffer_will_be_noop_for_debugging()) {
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.
output_buffer_renderer_->RenderToTextureOwnerFrontBuffer(
BindingsMode::kBindImage,
output_buffer_renderer_->texture_owner()->GetTextureId());
return true;
}

bool CodecImage::CopyTexSubImage(unsigned target,
const gfx::Point& offset,
const gfx::Rect& rect) {
return false;
}

void CodecImage::NotifyOverlayPromotion(bool promotion,
const gfx::Rect& bounds) {
AssertAcquiredDrDcLock();
Expand Down Expand Up @@ -175,10 +89,6 @@ void CodecImage::NotifyOverlayPromotion(bool promotion,
}
}

void CodecImage::OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
uint64_t process_tracing_id,
const std::string& dump_name) {}

void CodecImage::ReleaseResources() {
DCHECK_CALLED_ON_VALID_THREAD(gpu_main_thread_checker_);
ReleaseCodecBuffer();
Expand Down
20 changes: 2 additions & 18 deletions media/gpu/android/codec_image.h
View file
Expand Up @@ -67,24 +67,6 @@ class MEDIA_GPU_EXPORT CodecImage
// replace previous callbacks. Order of callbacks is not guaranteed.
void AddUnusedCB(UnusedCB unused_cb);

// gl::GLImage implementation
gfx::Size GetSize() override;
unsigned GetInternalFormat() override;
unsigned GetDataType() override;
BindOrCopy ShouldBindOrCopy() override;
bool BindTexImage(unsigned target) override;
void ReleaseTexImage(unsigned target) override;
bool CopyTexImage(unsigned target) override;
bool CopyTexSubImage(unsigned target,
const gfx::Point& offset,
const gfx::Rect& rect) override;
void SetColorSpace(const gfx::ColorSpace& color_space) override {}
void OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
uint64_t process_tracing_id,
const std::string& dump_name) override;
std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
GetAHardwareBuffer() override;

// Notify us that we're no longer in-use for display, and may be pointed at
// another output buffer via a call to Initialize.
void NotifyUnused();
Expand All @@ -99,6 +81,8 @@ class MEDIA_GPU_EXPORT CodecImage
// 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() override;
std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
GetAHardwareBuffer() override;
bool TextureOwnerBindsTextureOnUpdate() override;

// Whether the codec buffer has been rendered to the front buffer.
Expand Down
59 changes: 0 additions & 59 deletions media/gpu/android/codec_image_unittest.cc
View file
Expand Up @@ -181,42 +181,6 @@ TEST_F(CodecImageTest, ImageStartsUnrendered) {
ASSERT_FALSE(i->was_rendered_to_front_buffer());
}

TEST_F(CodecImageTest, CopyTexImageIsInvalidForOverlayImages) {
auto i = NewImage(kOverlay);
ASSERT_NE(gl::GLImage::COPY, i->ShouldBindOrCopy());
}

TEST_F(CodecImageTest, CopyTexImageFailsIfTargetIsNotOES) {
auto i = NewImage(kTextureOwner);
ASSERT_FALSE(i->CopyTexImage(GL_TEXTURE_2D));
}

TEST_F(CodecImageTest, CopyTexImageFailsIfTheWrongTextureIsBound) {
auto i = NewImage(kTextureOwner);
GLuint wrong_texture_id;
glGenTextures(1, &wrong_texture_id);
glBindTexture(GL_TEXTURE_EXTERNAL_OES, wrong_texture_id);
ASSERT_FALSE(i->CopyTexImage(GL_TEXTURE_EXTERNAL_OES));
}

TEST_F(CodecImageTest, CopyTexImageCanBeCalledRepeatedly) {
auto i = NewImage(kTextureOwner);
ASSERT_TRUE(i->CopyTexImage(GL_TEXTURE_EXTERNAL_OES));
ASSERT_TRUE(i->CopyTexImage(GL_TEXTURE_EXTERNAL_OES));
}

TEST_F(CodecImageTest, CopyTexImageTriggersFrontBufferRendering) {
auto i = NewImage(kTextureOwner);
// Verify that the release comes before the wait.
InSequence s;
EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true));
EXPECT_CALL(*codec_buffer_wait_coordinator_, WaitForFrameAvailable());
EXPECT_CALL(*codec_buffer_wait_coordinator_->texture_owner(),
UpdateTexImage());
i->CopyTexImage(GL_TEXTURE_EXTERNAL_OES);
ASSERT_TRUE(i->was_rendered_to_front_buffer());
}

TEST_F(CodecImageTest, CanRenderTextureOwnerImageToBackBuffer) {
auto i = NewImage(kTextureOwner);
ASSERT_TRUE(i->RenderToTextureOwnerBackBuffer());
Expand Down Expand Up @@ -345,27 +309,4 @@ TEST_F(CodecImageTest, RenderAfterUnusedDoesntCrash) {
codec_buffer_wait_coordinator_->texture_owner()->GetTextureId()));
}

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, gfx::ColorSpace::CreateSRGB());
auto buffer_renderer = std::make_unique<CodecOutputBufferRenderer>(
std::move(buffer), nullptr,
features::NeedThreadSafeAndroidMedia()
? base::MakeRefCounted<gpu::RefCountedLockForTest>()
: nullptr);

scoped_refptr<CodecImage> image = new CodecImage(
coded_size, features::NeedThreadSafeAndroidMedia()
? base::MakeRefCounted<gpu::RefCountedLockForTest>()
: nullptr);
image->Initialize(std::move(buffer_renderer), false,
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

0 comments on commit 46d0916

Please sign in to comment.