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 <[email protected]> Reviewed-by: Frank Liberato <[email protected]> Cr-Commit-Position: refs/heads/master@{#628566}
feiyue1206 · Feb 2, 2019 · 073af20 · 073af20
1 parent c34be7a
commit 073af20
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 22 deletions.
diff --git 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<ui::ScopedMakeCurrent> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;