Don't dismiss picture buffers on rez change until they are available

This CL fixes a race where the code would call DismissPictureBuffer()
on all output_picture_buffers_ once it detected a resolution change.
However some of those picture buffers had yet to be displayed by the
client. The client code, when asked to dismiss a picture buffer, only
tries to detect if it is currently displaying that frame, and if not
goes ahead and deletes the texture. This patch defers the dismissing
of the picture buffers until the client calls RecyclePictureBuffer
back with that stale buffer ID, meaning that the texture has been
displayed and can be safely deleted.

TEST: navigate to
http://sr-repro.s3.amazonaws.com/gpudecode/repro-gpu-mbr.html, before
CL there may be series of black frames when playing the video and it
switches from 720p to 1080p. After the CL the black frames are gone.
BUG=404817

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

Cr-Commit-Position: refs/heads/master@{#293616}

content/common/gpu/media/dxva_video_decode_accelerator.cc

@@ -558,12 +558,25 @@ void DXVAVideoDecodeAccelerator::ReusePictureBuffer(
   RETURN_AND_NOTIFY_ON_FAILURE((state_ != kUninitialized),
       "Invalid state: " << state_, ILLEGAL_STATE,);
 
-  if (output_picture_buffers_.empty())
+  if (output_picture_buffers_.empty() && stale_output_picture_buffers_.empty())
     return;
 
   OutputBuffers::iterator it = output_picture_buffers_.find(picture_buffer_id);
-  RETURN_AND_NOTIFY_ON_FAILURE(it != output_picture_buffers_.end(),
-      "Invalid picture id: " << picture_buffer_id, INVALID_ARGUMENT,);
+  // If we didn't find the picture id in the |output_picture_buffers_| map we
+  // try the |stale_output_picture_buffers_| map, as this may have been an
+  // output picture buffer from before a resolution change, that at resolution
+  // change time had yet to be displayed. The client is calling us back to tell
+  // us that we can now recycle this picture buffer, so if we were waiting to
+  // dispose of it we now can.
+  if (it == output_picture_buffers_.end()) {
+    it = stale_output_picture_buffers_.find(picture_buffer_id);
+    RETURN_AND_NOTIFY_ON_FAILURE(it != stale_output_picture_buffers_.end(),
+        "Invalid picture id: " << picture_buffer_id, INVALID_ARGUMENT,);
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&DXVAVideoDecodeAccelerator::DeferredDismissStaleBuffer,
+            weak_this_factory_.GetWeakPtr(), picture_buffer_id));
+    return;
+  }
 
   it->second->ReusePictureBuffer();
   ProcessPendingSamples();
@@ -996,6 +1009,7 @@ void DXVAVideoDecodeAccelerator::Invalidate() {
     return;
   weak_this_factory_.InvalidateWeakPtrs();
   output_picture_buffers_.clear();
+  stale_output_picture_buffers_.clear();
   pending_output_samples_.clear();
   pending_input_buffers_.clear();
   decoder_.Release();
@@ -1172,8 +1186,7 @@ void DXVAVideoDecodeAccelerator::HandleResolutionChanged(int width,
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::DismissStaleBuffers,
-                 weak_this_factory_.GetWeakPtr(),
-                 output_picture_buffers_));
+                 weak_this_factory_.GetWeakPtr()));
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
@@ -1181,20 +1194,35 @@ void DXVAVideoDecodeAccelerator::HandleResolutionChanged(int width,
                  weak_this_factory_.GetWeakPtr(),
                  width,
                  height));
-
-  output_picture_buffers_.clear();
 }
 
-void DXVAVideoDecodeAccelerator::DismissStaleBuffers(
-    const OutputBuffers& picture_buffers) {
-  OutputBuffers::const_iterator index;
+void DXVAVideoDecodeAccelerator::DismissStaleBuffers() {
+  OutputBuffers::iterator index;
 
-  for (index = picture_buffers.begin();
-       index != picture_buffers.end();
+  for (index = output_picture_buffers_.begin();
+       index != output_picture_buffers_.end();
        ++index) {
-    DVLOG(1) << "Dismissing picture id: " << index->second->id();
-    client_->DismissPictureBuffer(index->second->id());
+    if (index->second->available()) {
+      DVLOG(1) << "Dismissing picture id: " << index->second->id();
+      client_->DismissPictureBuffer(index->second->id());
+    } else {
+      // Move to |stale_output_picture_buffers_| for deferred deletion.
+      stale_output_picture_buffers_.insert(
+          std::make_pair(index->first, index->second));
+    }
   }
+
+  output_picture_buffers_.clear();
+}
+
+void DXVAVideoDecodeAccelerator::DeferredDismissStaleBuffer(
+    int32 picture_buffer_id) {
+  OutputBuffers::iterator it = stale_output_picture_buffers_.find(
+      picture_buffer_id);
+  DCHECK(it != stale_output_picture_buffers_.end());
+  DVLOG(1) << "Dismissing picture id: " << it->second->id();
+  client_->DismissPictureBuffer(it->second->id());
+  stale_output_picture_buffers_.erase(it);
 }
 
 }  // namespace content

content/common/gpu/media/dxva_video_decode_accelerator.h

@@ -151,7 +151,10 @@ class CONTENT_EXPORT DXVAVideoDecodeAccelerator
   typedef std::map<int32, linked_ptr<DXVAPictureBuffer> > OutputBuffers;
 
   // Tells the client to dismiss the stale picture buffers passed in.
-  void DismissStaleBuffers(const OutputBuffers& picture_buffers);
+  void DismissStaleBuffers();
+
+  // Called after the client indicates we can recycle a stale picture buffer.
+  void DeferredDismissStaleBuffer(int32 picture_buffer_id);
 
   // To expose client callbacks from VideoDecodeAccelerator.
   media::VideoDecodeAccelerator::Client* client_;
@@ -196,6 +199,12 @@ class CONTENT_EXPORT DXVAVideoDecodeAccelerator
   // The key is the picture buffer id.
   OutputBuffers output_picture_buffers_;
 
+  // After a resolution change there may be a few output buffers which have yet
+  // to be displayed so they cannot be dismissed immediately. We move them from
+  // |output_picture_buffers_| to this map so they may be dismissed once they
+  // become available.
+  OutputBuffers stale_output_picture_buffers_;
+
   // Set to true if we requested picture slots from the client.
   bool pictures_requested_;