From 073af203fdbeebdbf092b07f411417cc2cc0ed51 Mon Sep 17 00:00:00 2001 From: Vikas Soni Date: Sat, 2 Feb 2019 03:39:11 +0000 Subject: [PATCH] Refactor CodecImage, ImageReader and SurfaceTexture. Refactor CodecImage, ImageReader and SurfaceTexture to make it usable by SharedImage code in future. Added a bool parameter to hint if SurfaceTexture/ImageReader should bind egl images to the texture target or not. Bug: 900963 Change-Id: I4630d4432a850427b029b9f05460b0e7e2459a3f Reviewed-on: https://chromium-review.googlesource.com/c/1447013 Commit-Queue: vikas soni Reviewed-by: Frank Liberato Cr-Commit-Position: refs/heads/master@{#628566} --- media/gpu/android/codec_image.cc | 14 ++++++++++---- media/gpu/android/codec_image.h | 11 +++++++---- media/gpu/android/codec_image_unittest.cc | 10 +++++----- media/gpu/android/image_reader_gl_owner.cc | 11 +++++++---- media/gpu/android/image_reader_gl_owner.h | 2 +- media/gpu/android/mock_texture_owner.h | 2 +- media/gpu/android/surface_texture_gl_owner.cc | 5 ++++- media/gpu/android/surface_texture_gl_owner.h | 2 +- media/gpu/android/texture_owner.h | 4 +++- 9 files changed, 39 insertions(+), 22 deletions(-) diff --git a/media/gpu/android/codec_image.cc b/media/gpu/android/codec_image.cc index 05eaef91c5f82f..5cf2987c03a190 100644 --- a/media/gpu/android/codec_image.cc +++ b/media/gpu/android/codec_image.cc @@ -202,8 +202,11 @@ bool CodecImage::RenderToTextureOwnerBackBuffer() { return true; } -bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode) { +bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode, + bool bind_egl_image) { DCHECK(texture_owner_); + DCHECK(bind_egl_image); + if (phase_ == Phase::kInFrontBuffer) return true; if (phase_ == Phase::kInvalidated) @@ -221,14 +224,17 @@ bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode) { std::unique_ptr scoped_make_current = MakeCurrentIfNeeded(texture_owner_.get()); // If we have to switch contexts, then we always want to restore the - // bindings. + // bindings. Also if bind_egl_image is set to false, we do no need to restore + // any bindings since UpdateTexImage will not bind any egl image to the + // texture target. bool should_restore_bindings = - bindings_mode == BindingsMode::kRestore || !!scoped_make_current; + (bindings_mode == BindingsMode::kRestore || !!scoped_make_current) && + bind_egl_image; GLint bound_service_id = 0; if (should_restore_bindings) glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &bound_service_id); - texture_owner_->UpdateTexImage(); + texture_owner_->UpdateTexImage(bind_egl_image); if (should_restore_bindings) glBindTexture(GL_TEXTURE_EXTERNAL_OES, bound_service_id); return true; diff --git a/media/gpu/android/codec_image.h b/media/gpu/android/codec_image.h index 8aaeaa6356ebe1..f435e1b69b20e7 100644 --- a/media/gpu/android/codec_image.h +++ b/media/gpu/android/codec_image.h @@ -109,11 +109,14 @@ class MEDIA_GPU_EXPORT CodecImage : public gpu::gles2::GLStreamTextureImage { // frame available event before calling UpdateTexImage(). Passing // BindingsMode::kDontRestore skips the work of restoring the current texture // bindings if the texture owner's context is already current. Otherwise, - // this switches contexts and preserves the texture bindings. - // Returns true if the buffer is in the front buffer. Returns false if the - // buffer was invalidated. + // this switches contexts and preserves the texture bindings. Setting + // |bind_egl_image| = false skips doing the egl bindings to the texture target + // during texture update. |bind_egl_image| must always be true when using a + // SurfaceTexture backed CodecImage(TextureOwner). Returns true if the buffer + // is in the front buffer. Returns false if the buffer was invalidated. enum class BindingsMode { kRestore, kDontRestore }; - bool RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode); + bool RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode, + bool bind_egl_image = true); // Renders this image to the overlay. Returns true if the buffer is in the // overlay front buffer. Returns false if the buffer was invalidated. diff --git a/media/gpu/android/codec_image_unittest.cc b/media/gpu/android/codec_image_unittest.cc index 3271bfe5d60b2d..a0f9027195619c 100644 --- a/media/gpu/android/codec_image_unittest.cc +++ b/media/gpu/android/codec_image_unittest.cc @@ -156,7 +156,7 @@ TEST_F(CodecImageTest, CopyTexImageTriggersFrontBufferRendering) { InSequence s; EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true)); EXPECT_CALL(*texture_owner_, WaitForFrameAvailable()); - EXPECT_CALL(*texture_owner_, UpdateTexImage()); + EXPECT_CALL(*texture_owner_, UpdateTexImage(true)); i->CopyTexImage(GL_TEXTURE_EXTERNAL_OES); ASSERT_TRUE(i->was_rendered_to_front_buffer()); } @@ -166,7 +166,7 @@ TEST_F(CodecImageTest, GetTextureMatrixTriggersFrontBufferRendering) { InSequence s; EXPECT_CALL(*codec_, ReleaseOutputBuffer(_, true)); EXPECT_CALL(*texture_owner_, WaitForFrameAvailable()); - EXPECT_CALL(*texture_owner_, UpdateTexImage()); + EXPECT_CALL(*texture_owner_, UpdateTexImage(true)); EXPECT_CALL(*texture_owner_, GetTransformMatrix(_)); float matrix[16]; i->GetTextureMatrix(matrix); @@ -245,7 +245,7 @@ TEST_F(CodecImageTest, RenderToFrontBufferRestoresTextureBindings) { glGenTextures(1, &pre_bound_texture); glBindTexture(GL_TEXTURE_EXTERNAL_OES, pre_bound_texture); auto i = NewImage(kTextureOwner); - EXPECT_CALL(*texture_owner_, UpdateTexImage()); + EXPECT_CALL(*texture_owner_, UpdateTexImage(true)); i->RenderToFrontBuffer(); GLint post_bound_texture = 0; glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &post_bound_texture); @@ -264,7 +264,7 @@ TEST_F(CodecImageTest, RenderToFrontBufferRestoresGLContext) { auto i = NewImage(kTextureOwner); // Our context should not be current when UpdateTexImage() is called. - EXPECT_CALL(*texture_owner_, UpdateTexImage()).WillOnce(Invoke([&]() { + EXPECT_CALL(*texture_owner_, UpdateTexImage(true)).WillOnce(Invoke([&](bool) { ASSERT_FALSE(context->IsCurrent(surface.get())); })); i->RenderToFrontBuffer(); @@ -299,7 +299,7 @@ TEST_F(CodecImageTest, GetAHardwareBuffer) { EXPECT_EQ(texture_owner_->get_a_hardware_buffer_count, 0); EXPECT_FALSE(i->was_rendered_to_front_buffer()); - EXPECT_CALL(*texture_owner_, UpdateTexImage()); + EXPECT_CALL(*texture_owner_, UpdateTexImage(true)); i->GetAHardwareBuffer(); EXPECT_EQ(texture_owner_->get_a_hardware_buffer_count, 1); EXPECT_TRUE(i->was_rendered_to_front_buffer()); diff --git a/media/gpu/android/image_reader_gl_owner.cc b/media/gpu/android/image_reader_gl_owner.cc index 056864f639f6db..2971cd5eaf6e1e 100644 --- a/media/gpu/android/image_reader_gl_owner.cc +++ b/media/gpu/android/image_reader_gl_owner.cc @@ -185,7 +185,7 @@ gl::ScopedJavaSurface ImageReaderGLOwner::CreateJavaSurface() const { return gl::ScopedJavaSurface::AcquireExternalSurface(j_surface); } -void ImageReaderGLOwner::UpdateTexImage() { +void ImageReaderGLOwner::UpdateTexImage(bool bind_egl_image) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); // If we've lost the texture, then do nothing. @@ -251,9 +251,12 @@ void ImageReaderGLOwner::UpdateTexImage() { current_image_fence_ = std::move(scoped_acquire_fence_fd); current_image_bound_ = false; - // TODO(khushalsagar): This should be on the public API so that we only bind - // the texture if we were going to render it without an overlay. - EnsureTexImageBound(); + // Skip generating and binding egl image if bind_egl_image is false. + if (bind_egl_image) { + // TODO(khushalsagar): This should be on the public API so that we only bind + // the texture if we were going to render it without an overlay. + EnsureTexImageBound(); + } } void ImageReaderGLOwner::EnsureTexImageBound() { diff --git a/media/gpu/android/image_reader_gl_owner.h b/media/gpu/android/image_reader_gl_owner.h index f3d485718d6ce9..48b4c97804aeef 100644 --- a/media/gpu/android/image_reader_gl_owner.h +++ b/media/gpu/android/image_reader_gl_owner.h @@ -34,7 +34,7 @@ class MEDIA_GPU_EXPORT ImageReaderGLOwner : public TextureOwner { gl::GLContext* GetContext() const override; gl::GLSurface* GetSurface() const override; gl::ScopedJavaSurface CreateJavaSurface() const override; - void UpdateTexImage() override; + void UpdateTexImage(bool bind_egl_image) override; void GetTransformMatrix(float mtx[16]) override; void ReleaseBackBuffers() override; void SetReleaseTimeToNow() override; diff --git a/media/gpu/android/mock_texture_owner.h b/media/gpu/android/mock_texture_owner.h index 25815abeffc240..adcff47a4b0082 100644 --- a/media/gpu/android/mock_texture_owner.h +++ b/media/gpu/android/mock_texture_owner.h @@ -28,7 +28,7 @@ class MockTextureOwner : public TextureOwner { MOCK_CONST_METHOD0(GetContext, gl::GLContext*()); MOCK_CONST_METHOD0(GetSurface, gl::GLSurface*()); MOCK_CONST_METHOD0(CreateJavaSurface, gl::ScopedJavaSurface()); - MOCK_METHOD0(UpdateTexImage, void()); + MOCK_METHOD1(UpdateTexImage, void(bool bind_egl_image)); MOCK_METHOD1(GetTransformMatrix, void(float mtx[16])); MOCK_METHOD0(ReleaseBackBuffers, void()); MOCK_METHOD0(SetReleaseTimeToNow, void()); diff --git a/media/gpu/android/surface_texture_gl_owner.cc b/media/gpu/android/surface_texture_gl_owner.cc index a692b2b426af2b..af0da9b4de9df9 100644 --- a/media/gpu/android/surface_texture_gl_owner.cc +++ b/media/gpu/android/surface_texture_gl_owner.cc @@ -68,8 +68,11 @@ gl::ScopedJavaSurface SurfaceTextureGLOwner::CreateJavaSurface() const { return gl::ScopedJavaSurface(surface_texture_.get()); } -void SurfaceTextureGLOwner::UpdateTexImage() { +// bind_egl_image is a no-op for surface texture since it always binds the egl +// image under the hood. +void SurfaceTextureGLOwner::UpdateTexImage(bool bind_egl_image) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + DCHECK(bind_egl_image); if (surface_texture_) surface_texture_->UpdateTexImage(); } diff --git a/media/gpu/android/surface_texture_gl_owner.h b/media/gpu/android/surface_texture_gl_owner.h index 91f0ad6fae7be3..a65c48e1ad9e97 100644 --- a/media/gpu/android/surface_texture_gl_owner.h +++ b/media/gpu/android/surface_texture_gl_owner.h @@ -33,7 +33,7 @@ class MEDIA_GPU_EXPORT SurfaceTextureGLOwner : public TextureOwner { gl::GLContext* GetContext() const override; gl::GLSurface* GetSurface() const override; gl::ScopedJavaSurface CreateJavaSurface() const override; - void UpdateTexImage() override; + void UpdateTexImage(bool bind_egl_image) override; void GetTransformMatrix(float mtx[16]) override; void ReleaseBackBuffers() override; void SetReleaseTimeToNow() override; diff --git a/media/gpu/android/texture_owner.h b/media/gpu/android/texture_owner.h index a2004262b1c03f..c2619ccb73d51e 100644 --- a/media/gpu/android/texture_owner.h +++ b/media/gpu/android/texture_owner.h @@ -69,7 +69,9 @@ class MEDIA_GPU_EXPORT TextureOwner virtual gl::ScopedJavaSurface CreateJavaSurface() const = 0; // Update the texture image using the latest available image data. - virtual void UpdateTexImage() = 0; + // |bind_egl_image| hints the underlying implementation whether an egl image + // should be bound to the texture target or not. + virtual void UpdateTexImage(bool bind_egl_image = true) = 0; // Transformation matrix if any associated with the texture image. virtual void GetTransformMatrix(float mtx[16]) = 0;