Set the texture to cleared in VideoDecodeAccelerator::PictureReady.

content/common/gpu/media/gpu_video_decode_accelerator.cc

Index: content/common/gpu/media/gpu_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/gpu_video_decode_accelerator.cc b/content/common/gpu/media/gpu_video_decode_accelerator.cc
index e180f06788303c4d0e02f2bddfb13674d739f8eb..2f0e5788b4ec5d05bfbc28f65084071dca5a3de3 100644
--- a/content/common/gpu/media/gpu_video_decode_accelerator.cc
+++ b/content/common/gpu/media/gpu_video_decode_accelerator.cc
@@ -180,6 +180,13 @@ void GpuVideoDecodeAccelerator::DismissPictureBuffer(
 
 void GpuVideoDecodeAccelerator::PictureReady(
     const media::Picture& picture) {
+  // VDA may call PictureReady on IO thread. SetTextureCleared should run on
+  // the child thread. VDA is responsible to call PictureReady on the child
+  // thread when a picture buffer is delivered the first time.
+  if (child_message_loop_->BelongsToCurrentThread())

[piman, 2013/09/26 16:50:01]

nit: needs brackets per style guide.

[wuchengli, 2013/09/30 16:11:21]

Done.

+    if (!SetTextureCleared(picture))
+      return;

[piman, 2013/09/26 16:50:01]

Is there a way to DCHECK that we don't get here on the IO thread without the
texture having been set cleared? For example check the list that you keep? You'd
need a lock, but only in debug, so that would be ok I think.

[wuchengli, 2013/09/30 16:11:21]

Done.

+
   if (!Send(new AcceleratedVideoDecoderHostMsg_PictureReady(
           host_route_id_,
           picture.picture_buffer_id(),
@@ -327,9 +334,7 @@ void GpuVideoDecodeAccelerator::OnAssignPictureBuffers(
     }
     if (texture_target_ == GL_TEXTURE_EXTERNAL_OES) {
       // GL_TEXTURE_EXTERNAL_OES textures have their dimensions defined by the
-      // underlying EGLImage.  Use |texture_dimensions_| for this size.  The
-      // textures cannot be rendered to or cleared, so we set |cleared| true to
-      // skip clearing.
+      // underlying EGLImage.  Use |texture_dimensions_| for this size.
       texture_manager->SetLevelInfo(texture_ref,
                                     GL_TEXTURE_EXTERNAL_OES,
                                     0,
@@ -340,7 +345,7 @@ void GpuVideoDecodeAccelerator::OnAssignPictureBuffers(
                                     0,
                                     0,
                                     0,
-                                    true);
+                                    false);
     } else {
       // For other targets, texture dimensions should already be defined.
       GLsizei width = 0, height = 0;
@@ -352,11 +357,6 @@ void GpuVideoDecodeAccelerator::OnAssignPictureBuffers(
         return;
       }
     }
-    if (!texture_manager->ClearRenderableLevels(command_decoder, texture_ref)) {
-      DLOG(ERROR) << "Failed to Clear texture id " << texture_ids[i];
-      NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
-      return;
-    }
     uint32 service_texture_id;
     if (!command_decoder->GetServiceTextureId(
             texture_ids[i], &service_texture_id)) {
@@ -367,6 +367,8 @@ void GpuVideoDecodeAccelerator::OnAssignPictureBuffers(
     buffers.push_back(media::PictureBuffer(
         buffer_ids[i], texture_dimensions_, service_texture_id));
   }
+  buffer_ids_ = buffer_ids;
+  texture_ids_ = texture_ids;
   video_decode_accelerator_->AssignPictureBuffers(buffers);
 }
 
@@ -446,4 +448,35 @@ bool GpuVideoDecodeAccelerator::Send(IPC::Message* message) {
   return stub_->channel()->Send(message);
 }
 
+bool GpuVideoDecodeAccelerator::SetTextureCleared(
+    const media::Picture& picture) {
+  DCHECK(child_message_loop_->BelongsToCurrentThread());
+  uint32 i;
+  for (i = 0; i < buffer_ids_.size(); i++)

[piman, 2013/09/26 16:50:01]

nit: needs brackets per style guide

[wuchengli, 2013/09/30 16:11:21]

Done. Code was changed.

+    if (picture.picture_buffer_id() == buffer_ids_[i])
+      break;
+  if (i >= buffer_ids_.size())
+    return true;
+
+  gpu::gles2::TextureManager* texture_manager =
+      stub_->decoder()->GetContextGroup()->texture_manager();
+  gpu::gles2::TextureRef* texture_ref = texture_manager->GetTexture(
+      texture_ids_[i]);

[piman, 2013/09/26 16:50:01]

Can you, instead, keep a scoped_refptr<TextureRef> when you first lookup the
texture in OnAssignPictureBuffers?
It is possible that by the time this executes, the original texture has been
deleted, and another one took its place on the same id. You don't want to touch
the new one.

[wuchengli, 2013/09/30 16:11:21]

Done.

+  if (!texture_ref) {
+    DLOG(ERROR) << "Failed to find texture id " << texture_ids_[i];
+    NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
+    return false;
+  }
+  gpu::gles2::Texture* info = texture_ref->texture();
+  if (info->target() != texture_target_) {
+    DLOG(ERROR) << "Texture target mismatch for texture id " << texture_ids_[i];
+    NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
+    return false;
+  }
+  texture_manager->SetLevelCleared(texture_ref, info->target(), 0, true);
+  buffer_ids_.erase(buffer_ids_.begin() + i);
+  texture_ids_.erase(texture_ids_.begin() + i);

[Ami GONE FROM CHROMIUM, 2013/09/26 16:21:56]

Would require less book-keeping as a vector of pairs (buffer/texture ids) or a
map from buffer id to texture id, FWIW.

[piman, 2013/09/26 16:50:01]

+1 on map, given the linear lookup above.

[wuchengli, 2013/09/30 16:11:21]

Done. Changed to a map.

+  return true;
+}
+
 }  // namespace content