Fix Android black frames from MSE config changes.

content/common/gpu/media/android_video_decode_accelerator.cc

Index: content/common/gpu/media/android_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/android_video_decode_accelerator.cc b/content/common/gpu/media/android_video_decode_accelerator.cc
index d8da2982bf2a15844cbfc44da7556591c6edeadc..e9b61ff88f0f3adb7e32eab7a9790f4435588286 100644
--- a/content/common/gpu/media/android_video_decode_accelerator.cc
+++ b/content/common/gpu/media/android_video_decode_accelerator.cc
@@ -19,6 +19,7 @@
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/media/android_copying_backing_strategy.h"
 #include "content/common/gpu/media/android_deferred_rendering_backing_strategy.h"
+#include "content/common/gpu/media/avda_return_on_failure.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
@@ -48,6 +49,11 @@
     PostError(FROM_HERE, media::VideoDecodeAccelerator::error_code); \
   } while (0)
 
+// Override definition from avda_return_on_failure.h to s/state_provider_/this/.

[DaleCurtis, 2016/03/08 18:29:03]

Just use RETURN_ON_FAILURE directly instead of redefining the macro, that's just
going to cause confusion.

[chcunningham, 2016/03/08 22:19:13]

Done.

+#undef RETURN_NULL_IF_NULL
+#define RETURN_NULL_IF_NULL(ptr, ...) \
+  RETURN_ON_FAILURE(this, ptr, "Got null for " << #ptr, ILLEGAL_STATE, nullptr);
+
 namespace content {
 
 enum { kNumPictureBuffers = media::limits::kMaxVideoFrames + 1 };
@@ -585,25 +591,30 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
         return false;
 
       case media::MEDIA_CODEC_OUTPUT_FORMAT_CHANGED: {
-        if (!output_picture_buffers_.empty()) {
-          // TODO(chcunningham): This will likely dismiss a handful of decoded
-          // frames that have not yet been drawn and returned to us for re-use.
-          // Consider a more complicated design that would wait for them to be
-          // drawn before dismissing.
-          DismissPictureBuffers();
-        }
-
         if (media_codec_->GetOutputSize(&size_) != media::MEDIA_CODEC_OK) {
           POST_ERROR(PLATFORM_FAILURE, "GetOutputSize failed.");
           return false;
         }
+        DVLOG(3) << __FUNCTION__
+                 << " OUTPUT_FORMAT_CHANGED, new size: " << size_.ToString();
+
+        // Don't request picture buffers if we already have some. This avoids
+        // having to dismiss the existing buffers which may actively reference
+        // decoded images. Breaking their connection to the decoded image will
+        // cause rendering of black frames. Instead, we let the existing
+        // PictureBuffers live on and we simply update their size the next time
+        // they're attachted to an image of the new resolution. See the
+        // size update in |SendDecodedFrameToClient| and https://crbug/587994.
+        if (output_picture_buffers_.empty()) {
+          picturebuffers_requested_ = true;

[DaleCurtis, 2016/03/08 18:29:03]

Should this value be double checked to prevent multiple in flight
RequestPictureBuffers?

[chcunningham, 2016/03/08 22:19:13]

Done.

+          base::MessageLoop::current()->PostTask(
+              FROM_HERE,
+              base::Bind(&AndroidVideoDecodeAccelerator::RequestPictureBuffers,
+                         weak_this_factory_.GetWeakPtr()));
+          return false;
+        }
 
-        picturebuffers_requested_ = true;
-        base::MessageLoop::current()->PostTask(
-            FROM_HERE,
-            base::Bind(&AndroidVideoDecodeAccelerator::RequestPictureBuffers,
-                       weak_this_factory_.GetWeakPtr()));
-        return false;
+        return true;
       }
 
       case media::MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED:
@@ -709,14 +720,21 @@ void AndroidVideoDecodeAccelerator::SendDecodedFrameToClient(
   free_picture_ids_.pop();
   TRACE_COUNTER1("media", "AVDA::FreePictureIds", free_picture_ids_.size());
 
-  OutputBufferMap::const_iterator i =
-      output_picture_buffers_.find(picture_buffer_id);
+  OutputBufferMap::iterator i = output_picture_buffers_.find(picture_buffer_id);

[DaleCurtis, 2016/03/08 18:29:03]

This is a fine place for auto if you like.

[chcunningham, 2016/03/08 22:19:13]

Done.

   if (i == output_picture_buffers_.end()) {
     POST_ERROR(PLATFORM_FAILURE,
                "Can't find PictureBuffer id: " << picture_buffer_id);
     return;
   }
 
+  bool size_changed = false;
+  if (i->second.size() != size_) {
+    // Size may have changed due to resolution change since the last time this
+    // PictureBuffer was used.
+    strategy_->UpdatePictureBufferSize(&i->second, size_);
+    size_changed = true;
+  }
+
   // Connect the PictureBuffer to the decoded frame, via whatever
   // mechanism the strategy likes.
   strategy_->UseCodecBufferForPictureBuffer(codec_buffer_index, i->second);
@@ -726,7 +744,8 @@ void AndroidVideoDecodeAccelerator::SendDecodedFrameToClient(
       FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyPictureReady,
                             weak_this_factory_.GetWeakPtr(),
                             media::Picture(picture_buffer_id, bitstream_id,
-                                           gfx::Rect(size_), allow_overlay)));
+                                           gfx::Rect(size_), allow_overlay,
+                                           size_changed)));
 }
 
 void AndroidVideoDecodeAccelerator::Decode(
@@ -1001,6 +1020,20 @@ AndroidVideoDecodeAccelerator::GetGlDecoder() const {
   return gl_decoder_;
 }
 
+gpu::gles2::TextureRef* AndroidVideoDecodeAccelerator::GetTextureForPicture(
+    const media::PictureBuffer& picture_buffer) {
+  RETURN_NULL_IF_NULL(GetGlDecoder());
+  RETURN_NULL_IF_NULL(GetGlDecoder()->GetContextGroup());
+  gpu::gles2::TextureManager* texture_manager =
+      GetGlDecoder()->GetContextGroup()->texture_manager();
+  RETURN_NULL_IF_NULL(texture_manager);
+  gpu::gles2::TextureRef* texture_ref =
+      texture_manager->GetTexture(picture_buffer.internal_texture_id());
+  RETURN_NULL_IF_NULL(texture_ref);
+
+  return texture_ref;
+}
+
 void AndroidVideoDecodeAccelerator::OnFrameAvailable() {
   // Remember: this may be on any thread.
   DCHECK(strategy_);