Call UpdateTexImage after OnFrameAvailable

Both with SurfaceTexture and AImageReader we get only one FrameAvailable
callback before we call UpdateTexImage. This causes the problem when we
released buffer to codec in early rendering, but dropped the frame
later. In this case we did not called UpdateTexImage for that frame and
will not received OnFrameAvailable for next one causing
WaitForFrameAvailable to timeout.

This CL adds UpdateTexImage call after WaitForFrameAvailable in
RenderToTextureOwnerBackBuffer in case we're already waiting for frame.

Bug: 1113203
Change-Id: I32e703d07df94f7170795077773a30b4578c1a86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339495
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797477}

media/gpu/android/codec_image.cc

@@ -186,11 +186,11 @@ bool CodecImage::RenderToFrontBuffer() {
   return output_buffer_renderer_->RenderToFrontBuffer();
 }
 
-bool CodecImage::RenderToTextureOwnerBackBuffer(BlockingMode blocking_mode) {
+bool CodecImage::RenderToTextureOwnerBackBuffer() {
   if (!output_buffer_renderer_)
     return false;
 
-  return output_buffer_renderer_->RenderToTextureOwnerBackBuffer(blocking_mode);
+  return output_buffer_renderer_->RenderToTextureOwnerBackBuffer();
 }
 
 bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode) {

media/gpu/android/codec_image.h

@@ -31,8 +31,6 @@ namespace media {
 class MEDIA_GPU_EXPORT CodecImage
     : public gpu::StreamTextureSharedImageInterface {
  public:
-  using BlockingMode = CodecOutputBufferRenderer::BlockingMode;
-
   // Callback to notify that a codec image is now unused in the sense of not
   // being out for display.  This lets us signal interested folks once a video
   // frame is destroyed and the sync token clears, so that that CodecImage may
@@ -134,11 +132,9 @@ class MEDIA_GPU_EXPORT CodecImage
   // Renders this image to the back buffer of its texture owner. Only valid if
   // is_texture_owner_backed(). Returns true if the buffer is in the back
   // buffer. Returns false if the buffer was invalidated.
-  // |blocking_mode| indicates whether this should (a) wait for any previously
-  // pending rendered frame before rendering this one, or (b) fail if a wait
-  // is required.
-  bool RenderToTextureOwnerBackBuffer(
-      BlockingMode blocking_mode = BlockingMode::kAllowBlocking);
+  // RenderToTextureOwnerBackBuffer() will not block if there is any previously
+  // pending frame and will return false in this case.
+  bool RenderToTextureOwnerBackBuffer();
 
   // Release any codec buffer without rendering, if we have one.
   virtual void ReleaseCodecBuffer();

media/gpu/android/codec_output_buffer_renderer.cc

@@ -7,6 +7,7 @@
 
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
 #include "base/bind_helpers.h"
+#include "base/optional.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "ui/gl/gl_context.h"
@@ -37,6 +38,19 @@ std::unique_ptr<ui::ScopedMakeCurrent> MakeCurrentIfNeeded(
   return scoped_current;
 }
 
+class ScopedRestoreTextureBinding {
+ public:
+  ScopedRestoreTextureBinding() {
+    glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &bound_service_id_);
+  }
+  ~ScopedRestoreTextureBinding() {
+    glBindTexture(GL_TEXTURE_EXTERNAL_OES, bound_service_id_);
+  }
+
+ private:
+  GLint bound_service_id_;
+};
+
 }  // namespace
 
 CodecOutputBufferRenderer::CodecOutputBufferRenderer(
@@ -49,8 +63,7 @@ CodecOutputBufferRenderer::CodecOutputBufferRenderer(
 
 CodecOutputBufferRenderer::~CodecOutputBufferRenderer() = default;
 
-bool CodecOutputBufferRenderer::RenderToTextureOwnerBackBuffer(
-    BlockingMode blocking_mode) {
+bool CodecOutputBufferRenderer::RenderToTextureOwnerBackBuffer() {
   DCHECK_NE(phase_, Phase::kInFrontBuffer);
   if (phase_ == Phase::kInBackBuffer)
     return true;
@@ -65,12 +78,10 @@ bool CodecOutputBufferRenderer::RenderToTextureOwnerBackBuffer(
   if (!codec_buffer_wait_coordinator_)
     return false;
 
-  // Wait for a previous frame available so we don't confuse it with the one
-  // we're about to release.
+  // Don't render frame if one is already pending.
+  // RenderToTextureOwnerFrontBuffer will wait before calling this.
   if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable()) {
-    if (blocking_mode == BlockingMode::kForbidBlocking)
-      return false;
-    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
+    return false;
   }
   if (!output_buffer_->ReleaseToSurface()) {
     phase_ = Phase::kInvalidated;
@@ -98,32 +109,45 @@ bool CodecOutputBufferRenderer::RenderToTextureOwnerFrontBuffer(
   if (phase_ == Phase::kInvalidated)
     return false;
 
-  // Render it to the back buffer if it's not already there.
-  if (!RenderToTextureOwnerBackBuffer())
-    return false;
-
-  // The image is now in the back buffer, so promote it to the front buffer.
-  phase_ = Phase::kInFrontBuffer;
-  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable())
-    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
-
   std::unique_ptr<ui::ScopedMakeCurrent> scoped_make_current =
       MakeCurrentIfNeeded(
           codec_buffer_wait_coordinator_->texture_owner().get());
   // If updating the image will implicitly update the texture bindings then
   // restore if requested or the update needed a context switch.
-  bool should_restore_bindings =
-      codec_buffer_wait_coordinator_->texture_owner()
+  base::Optional<ScopedRestoreTextureBinding> scoped_restore_texture;
+  if (codec_buffer_wait_coordinator_->texture_owner()
           ->binds_texture_on_update() &&
-      (bindings_mode == BindingsMode::kRestoreIfBound || !!scoped_make_current);
+      (bindings_mode == BindingsMode::kRestoreIfBound ||
+       !!scoped_make_current)) {
+    scoped_restore_texture.emplace();
+  }
+
+  // Render it to the back buffer if it's not already there.
+  if (phase_ != Phase::kInBackBuffer) {
+    // Wait for a previous frame available so we don't confuse it with the one
+    // we're about to render.
+    if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable()) {
+      codec_buffer_wait_coordinator_->WaitForFrameAvailable();
+
+      // We must call update tex image if we did get OnFrameAvailable, otherwise
+      // we will stop receiving callbacks (see https://crbug.com/c/1113203)
+      codec_buffer_wait_coordinator_->texture_owner()->UpdateTexImage();
+    }
+    if (!RenderToTextureOwnerBackBuffer()) {
+      // RenderTotextureOwnerBackBuffer can fail now only if ReleaseToSurface
+      // failed.
+      DCHECK(phase_ == Phase::kInvalidated);
+      return false;
+    }
+  }
+
+  // The image is now in the back buffer, so promote it to the front buffer.
+  phase_ = Phase::kInFrontBuffer;
+  if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable())
+    codec_buffer_wait_coordinator_->WaitForFrameAvailable();
 
-  GLint bound_service_id = 0;
-  if (should_restore_bindings)
-    glGetIntegerv(GL_TEXTURE_BINDING_EXTERNAL_OES, &bound_service_id);
   codec_buffer_wait_coordinator_->texture_owner()->UpdateTexImage();
   EnsureBoundIfNeeded(bindings_mode);
-  if (should_restore_bindings)
-    glBindTexture(GL_TEXTURE_EXTERNAL_OES, bound_service_id);
   return true;
 }
 

media/gpu/android/codec_output_buffer_renderer.h

@@ -22,8 +22,6 @@ namespace media {
 class MEDIA_GPU_EXPORT CodecOutputBufferRenderer {
  public:
   using BindingsMode = gpu::StreamTextureSharedImageInterface::BindingsMode;
-  // Whether RenderToTextureOwnerBackBuffer may block or not.
-  enum class BlockingMode { kForbidBlocking, kAllowBlocking };
 
   CodecOutputBufferRenderer(
       std::unique_ptr<CodecOutputBuffer> output_buffer,
@@ -52,11 +50,9 @@ class MEDIA_GPU_EXPORT CodecOutputBufferRenderer {
   // Renders this image to the back buffer of its texture owner. Only valid if
   // is_texture_owner_backed(). Returns true if the buffer is in the back
   // buffer. Returns false if the buffer was invalidated.
-  // |blocking_mode| indicates whether this should (a) wait for any previously
-  // pending rendered frame before rendering this one, or (b) fail if a wait
-  // is required.
-  bool RenderToTextureOwnerBackBuffer(
-      BlockingMode blocking_mode = BlockingMode::kAllowBlocking);
+  // RenderToTextureOwnerBackBuffer() will not block if there is any previously
+  // pending frame and will return false in this case.
+  bool RenderToTextureOwnerBackBuffer();
 
   // Whether the codec buffer has been rendered to the front buffer.
   bool was_rendered_to_front_buffer() const {

media/gpu/android/maybe_render_early_manager.h

@@ -90,8 +90,7 @@ void MEDIA_GPU_EXPORT MaybeRenderEarly(std::vector<Image*>* image_vector_ptr) {
     // Try to render to the back buffer, but don't wait for any previous frame.
     // While this does make it more likely that we'll have to wait the next time
     // we draw, it does prevent us from waiting on frames we don't plan to draw.
-    images[back_buffer_index]->RenderToTextureOwnerBackBuffer(
-        CodecImage::BlockingMode::kForbidBlocking);
+    images[back_buffer_index]->RenderToTextureOwnerBackBuffer();
   }
 }
 

media/gpu/android/maybe_render_early_manager_unittest.cc

@@ -35,20 +35,17 @@ struct MockImage {
     }
 
     if (expectation == kRenderToBackBuffer) {
-      EXPECT_CALL(*this, RenderToTextureOwnerBackBuffer(
-                             CodecImage::BlockingMode::kForbidBlocking))
+      EXPECT_CALL(*this, RenderToTextureOwnerBackBuffer())
           .WillOnce(Return(phase != kInvalidated));
     } else {
-      EXPECT_CALL(*this, RenderToTextureOwnerBackBuffer(
-                             CodecImage::BlockingMode::kForbidBlocking))
-          .Times(0);
+      EXPECT_CALL(*this, RenderToTextureOwnerBackBuffer()).Times(0);
     }
   }
 
   MOCK_METHOD0(was_rendered_to_front_buffer, bool());
   MOCK_METHOD0(is_texture_owner_backed, bool());
   MOCK_METHOD0(RenderToFrontBuffer, bool());
-  MOCK_METHOD1(RenderToTextureOwnerBackBuffer, bool(CodecImage::BlockingMode));
+  MOCK_METHOD0(RenderToTextureOwnerBackBuffer, bool());
 };
 
 class MaybeRenderEarlyTest : public testing::Test {