Pepper: Fix texture management in VideoDecoderShim on Reset.

- Use a hash_set to track available textures. This prevents weird behavior
if the plugin recycles a texture twice.
- Fix Reset, so all textures are made available on completion.
- Fix the plugin, which had a bug that allowed pictures to jump the queue
and didn't behave correctly on Reset.

BUG=281689

Review URL: https://codereview.chromium.org/337743003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277548 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/pepper/pepper_video_decoder_host.cc

@@ -337,12 +337,14 @@ void PepperVideoDecoderHost::NotifyEndOfBitstreamBuffer(
 }
 
 void PepperVideoDecoderHost::NotifyFlushDone() {
+  DCHECK(pending_decodes_.empty());
   host()->SendReply(flush_reply_context_,
                     PpapiPluginMsg_VideoDecoder_FlushReply());
   flush_reply_context_ = ppapi::host::ReplyMessageContext();
 }
 
 void PepperVideoDecoderHost::NotifyResetDone() {
+  DCHECK(pending_decodes_.empty());
   host()->SendReply(reset_reply_context_,
                     PpapiPluginMsg_VideoDecoder_ResetReply());
   reset_reply_context_ = ppapi::host::ReplyMessageContext();

content/renderer/pepper/video_decoder_shim.cc

@@ -386,7 +386,7 @@ void VideoDecoderShim::AssignPictureBuffers(
     // Map the plugin texture id to the local texture id.
     uint32_t plugin_texture_id = buffers[i].texture_id();
     texture_id_map_[plugin_texture_id] = local_texture_ids[i];
-    available_textures_.push_back(plugin_texture_id);
+    available_textures_.insert(plugin_texture_id);
   }
   pending_texture_mailboxes_.clear();
   SendPictures();
@@ -398,7 +398,7 @@ void VideoDecoderShim::ReusePictureBuffer(int32 picture_buffer_id) {
   if (textures_to_dismiss_.find(texture_id) != textures_to_dismiss_.end()) {
     DismissTexture(texture_id);
   } else if (texture_id_map_.find(texture_id) != texture_id_map_.end()) {
-    available_textures_.push_back(texture_id);
+    available_textures_.insert(texture_id);
     SendPictures();
   } else {
     NOTREACHED();
@@ -442,18 +442,19 @@ void VideoDecoderShim::OnDecodeComplete(int32_t result, uint32_t decode_id) {
   DCHECK(RenderThreadImpl::current());
   DCHECK(host_);
 
+  if (result == PP_ERROR_RESOURCE_FAILED) {
+    host_->NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
+    return;
+  }
+
   num_pending_decodes_--;
   completed_decodes_.push(decode_id);
 
-  if (result == PP_OK) {
-    // If frames are being queued because we're out of textures, don't notify
-    // the host that decode has completed. This exerts "back pressure" to keep
-    // the host from sending buffers that will cause pending_frames_ to grow.
-    if (pending_frames_.empty())
-      NotifyCompletedDecodes();
-  } else if (result == PP_ERROR_RESOURCE_FAILED) {
-    host_->NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
-  }
+  // If frames are being queued because we're out of textures, don't notify
+  // the host that decode has completed. This exerts "back pressure" to keep
+  // the host from sending buffers that will cause pending_frames_ to grow.
+  if (pending_frames_.empty())
+    NotifyCompletedDecodes();
 }
 
 void VideoDecoderShim::OnOutputComplete(scoped_ptr<PendingFrame> frame) {
@@ -470,8 +471,7 @@ void VideoDecoderShim::OnOutputComplete(scoped_ptr<PendingFrame> frame) {
            ++it) {
         textures_to_dismiss_.insert(it->second);
       }
-      for (std::vector<uint32_t>::const_iterator it =
-               available_textures_.begin();
+      for (TextureIdSet::const_iterator it = available_textures_.begin();
            it != available_textures_.end();
            ++it) {
         DismissTexture(*it);
@@ -501,8 +501,9 @@ void VideoDecoderShim::SendPictures() {
   while (!pending_frames_.empty() && !available_textures_.empty()) {
     const linked_ptr<PendingFrame>& frame = pending_frames_.front();
 
-    uint32_t texture_id = available_textures_.back();
-    available_textures_.pop_back();
+    TextureIdSet::iterator it = available_textures_.begin();
+    uint32_t texture_id = *it;
+    available_textures_.erase(it);
 
     uint32_t local_texture_id = texture_id_map_[texture_id];
     gpu::gles2::GLES2Interface* gles2 = context_provider_->ContextGL();
@@ -544,6 +545,16 @@ void VideoDecoderShim::OnResetComplete() {
     pending_frames_.pop();
   NotifyCompletedDecodes();
 
+  // Dismiss any old textures now.
+  while (!textures_to_dismiss_.empty())
+    DismissTexture(*textures_to_dismiss_.begin());
+  // Make all textures available.
+  for (TextureIdMap::const_iterator it = texture_id_map_.begin();
+       it != texture_id_map_.end();
+       ++it) {
+    available_textures_.insert(it->first);
+  }
+
   state_ = DECODING;
   host_->NotifyResetDone();
 }

content/renderer/pepper/video_decoder_shim.h

@@ -98,9 +98,9 @@ class VideoDecoderShim : public media::VideoDecodeAccelerator {
   typedef base::hash_map<uint32_t, uint32_t> TextureIdMap;
   TextureIdMap texture_id_map_;
   // Available textures (these are plugin ids.)
-  std::vector<uint32_t> available_textures_;
-  // Track textures that are no longer needed (these are plugin ids.)
   typedef base::hash_set<uint32_t> TextureIdSet;
+  TextureIdSet available_textures_;
+  // Track textures that are no longer needed (these are plugin ids.)
   TextureIdSet textures_to_dismiss_;
   // Mailboxes for pending texture requests, to write to plugin's textures.
   std::vector<gpu::Mailbox> pending_texture_mailboxes_;

ppapi/examples/video_decode/video_decode.cc

@@ -53,6 +53,15 @@ struct Shader {
 class Decoder;
 class MyInstance;
 
+struct PendingPicture {
+  PendingPicture(Decoder* decoder, const PP_VideoPicture& picture)
+      : decoder(decoder), picture(picture) {}
+  ~PendingPicture() {}
+
+  Decoder* decoder;
+  PP_VideoPicture picture;
+};
+
 class MyInstance : public pp::Instance, public pp::Graphics3DClient {
  public:
   MyInstance(PP_Instance instance, pp::Module* module);
@@ -106,14 +115,15 @@ class MyInstance : public pp::Instance, public pp::Graphics3DClient {
   void CreateRectangleARBProgramOnce();
   Shader CreateProgram(const char* vertex_shader, const char* fragment_shader);
   void CreateShader(GLuint program, GLenum type, const char* source, int size);
-  void PaintFinished(int32_t result, Decoder* decoder, PP_VideoPicture picture);
+  void PaintNextPicture();
+  void PaintFinished(int32_t result);
 
   pp::Size plugin_size_;
   bool is_painting_;
   // When decode outpaces render, we queue up decoded pictures for later
-  // painting.  Elements are <decoder,picture>.
-  typedef std::queue<std::pair<Decoder*, PP_VideoPicture> > PictureQueue;
-  PictureQueue pictures_pending_paint_;
+  // painting.
+  typedef std::queue<PendingPicture> PendingPictureQueue;
+  PendingPictureQueue pending_pictures_;
 
   int num_frames_rendered_;
   PP_TimeTicks first_frame_delivered_ticks_;
@@ -143,7 +153,8 @@ class Decoder {
   ~Decoder();
 
   int id() const { return id_; }
-  bool decoding() const { return !flushing_ && !resetting_; }
+  bool flushing() const { return flushing_; }
+  bool resetting() const { return resetting_; }
 
   void Reset();
   void RecyclePicture(const PP_VideoPicture& picture);
@@ -238,7 +249,6 @@ Decoder::~Decoder() {
 void Decoder::InitializeDone(int32_t result) {
   assert(decoder_);
   assert(result == PP_OK);
-  assert(decoding());
   Start();
 }
 
@@ -258,6 +268,7 @@ void Decoder::Start() {
 
 void Decoder::Reset() {
   assert(decoder_);
+  assert(!resetting_);
   resetting_ = true;
   decoder_->Reset(callback_factory_.NewCallback(&Decoder::ResetDone));
 }
@@ -298,7 +309,7 @@ void Decoder::DecodeDone(int32_t result) {
   if (result == PP_ERROR_ABORTED)
     return;
   assert(result == PP_OK);
-  if (decoding())
+  if (!flushing_ && !resetting_)
     DecodeNextFrame();
 }
 
@@ -316,12 +327,14 @@ void Decoder::PictureReady(int32_t result, PP_VideoPicture picture) {
 void Decoder::FlushDone(int32_t result) {
   assert(decoder_);
   assert(result == PP_OK || result == PP_ERROR_ABORTED);
+  assert(flushing_);
   flushing_ = false;
 }
 
 void Decoder::ResetDone(int32_t result) {
   assert(decoder_);
   assert(result == PP_OK);
+  assert(resetting_);
   resetting_ = false;
 
   Start();
@@ -385,10 +398,15 @@ bool MyInstance::HandleInputEvent(const pp::InputEvent& event) {
       pp::MouseInputEvent mouse_event(event);
       // Reset all decoders on mouse down.
       if (mouse_event.GetButton() == PP_INPUTEVENT_MOUSEBUTTON_LEFT) {
+        // Reset decoders.
         for (size_t i = 0; i < video_decoders_.size(); i++) {
-          if (video_decoders_[i]->decoding())
+          if (!video_decoders_[i]->resetting())
             video_decoders_[i]->Reset();
         }
+
+        // Clear pending pictures.
+        while (!pending_pictures_.empty())
+          pending_pictures_.pop();
       }
       return true;
     }
@@ -409,13 +427,20 @@ void MyInstance::PaintPicture(Decoder* decoder,
                               const PP_VideoPicture& picture) {
   if (first_frame_delivered_ticks_ == -1)
     assert((first_frame_delivered_ticks_ = core_if_->GetTimeTicks()) != -1);
-  if (is_painting_) {
-    pictures_pending_paint_.push(std::make_pair(decoder, picture));
-    return;
-  }
 
+  pending_pictures_.push(PendingPicture(decoder, picture));
+  if (!is_painting_)
+    PaintNextPicture();
+}
+
+void MyInstance::PaintNextPicture() {
   assert(!is_painting_);
   is_painting_ = true;
+
+  const PendingPicture& next = pending_pictures_.front();
+  Decoder* decoder = next.decoder;
+  const PP_VideoPicture& picture = next.picture;
+
   int x = 0;
   int y = 0;
   int half_width = plugin_size_.width() / 2;
@@ -450,14 +475,11 @@ void MyInstance::PaintPicture(Decoder* decoder,
   gles2_if_->UseProgram(graphics_3d, 0);
 
   last_swap_request_ticks_ = core_if_->GetTimeTicks();
-  assert(PP_OK_COMPLETIONPENDING ==
-         context_->SwapBuffers(callback_factory_.NewCallback(
-             &MyInstance::PaintFinished, decoder, picture)));
+  context_->SwapBuffers(
+      callback_factory_.NewCallback(&MyInstance::PaintFinished));
 }
 
-void MyInstance::PaintFinished(int32_t result,
-                               Decoder* decoder,
-                               PP_VideoPicture picture) {
+void MyInstance::PaintFinished(int32_t result) {
   assert(result == PP_OK);
   swap_ticks_ += core_if_->GetTimeTicks() - last_swap_request_ticks_;
   is_painting_ = false;
@@ -470,14 +492,20 @@ void MyInstance::PaintFinished(int32_t result,
                        << ", fps: " << fps
                        << ", with average ms/swap of: " << ms_per_swap;
   }
+
+  // If the decoders were reset, this will be empty.
+  if (pending_pictures_.empty())
+    return;
+
+  const PendingPicture& next = pending_pictures_.front();
+  Decoder* decoder = next.decoder;
+  const PP_VideoPicture& picture = next.picture;
   decoder->RecyclePicture(picture);
+  pending_pictures_.pop();
+
   // Keep painting as long as we have pictures.
-  if (!pictures_pending_paint_.empty()) {
-    std::pair<Decoder*, PP_VideoPicture> pending =
-        pictures_pending_paint_.front();
-    pictures_pending_paint_.pop();
-    PaintPicture(pending.first, pending.second);
-  }
+  if (!pending_pictures_.empty())
+    PaintNextPicture();
 }
 
 void MyInstance::InitGL() {

ppapi/proxy/video_decoder_resource.cc

@@ -471,6 +471,10 @@ void VideoDecoderResource::OnPluginMsgResetComplete(
     const ResourceMessageReplyParams& params) {
   // All shm buffers should have been made available by now.
   DCHECK_EQ(shm_buffers_.size(), available_shm_buffers_.size());
+  // Received pictures are no longer valid.
+  while (!received_pictures_.empty())
+    received_pictures_.pop();
+
   scoped_refptr<TrackedCallback> callback;
   callback.swap(reset_callback_);
   callback->Run(params.result());