Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1205)

Unified Diff: content/common/gpu/media/android_video_decode_accelerator.cc

Issue 1750213002: Fix Android black frames from MSE config changes. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Feedback: Adding UpdatePictureBufferSize to strategy Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 7deb6f3dc81cbe36be666f2354ac8591e5b71950..bf5af2778c7169782b7e327b472d43e6ef67c9c5 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"
@@ -47,6 +48,11 @@
PostError(FROM_HERE, media::VideoDecodeAccelerator::error_code); \
} while (0)
+// Override definition from avda_return_on_failure.h to s/state_provider_/this/.
+#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 };
@@ -244,16 +250,17 @@ AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
error_sequence_token_(0),
defer_errors_(false),
weak_this_factory_(this) {
- if (UseDeferredRenderingStrategy()) {
- // TODO(liberato, watk): Figure out what we want to do about zero copy for
- // fullscreen external SurfaceView in WebView. http://crbug.com/582170.
- DCHECK(!gl_decoder_->GetContextGroup()->mailbox_manager()->UsesSync());
- DVLOG(1) << __FUNCTION__ << ", using deferred rendering strategy.";
- strategy_.reset(new AndroidDeferredRenderingBackingStrategy(this));
- } else {
- DVLOG(1) << __FUNCTION__ << ", using copy back strategy.";
- strategy_.reset(new AndroidCopyingBackingStrategy(this));
- }
+ // if (UseDeferredRenderingStrategy()) {
+ // // TODO(liberato, watk): Figure out what we want to do about zero copy
+ // for
+ // // fullscreen external SurfaceView in WebView. http://crbug.com/582170.
+ // DCHECK(!gl_decoder_->GetContextGroup()->mailbox_manager()->UsesSync());
+ // DVLOG(1) << __FUNCTION__ << ", using deferred rendering strategy.";
+ // strategy_.reset(new AndroidDeferredRenderingBackingStrategy(this));
+ // } else {
+ DVLOG(1) << __FUNCTION__ << ", using copy back strategy.";
+ strategy_.reset(new AndroidCopyingBackingStrategy(this));
+ // }
}
AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
@@ -574,23 +581,29 @@ 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();
- }
-
- picturebuffers_requested_ = true;
int32_t width, height;
media_codec_->GetOutputFormat(&width, &height);
size_ = gfx::Size(width, height);
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&AndroidVideoDecodeAccelerator::RequestPictureBuffers,
- weak_this_factory_.GetWeakPtr()));
- return false;
+ DVLOG(4) << __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;
+ 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:
@@ -685,14 +698,19 @@ 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);
if (i == output_picture_buffers_.end()) {
POST_ERROR(PLATFORM_FAILURE,
"Can't find PictureBuffer id: " << picture_buffer_id);
return;
}
+ 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_);
+ }
+
// Connect the PictureBuffer to the decoded frame, via whatever
// mechanism the strategy likes.
strategy_->UseCodecBufferForPictureBuffer(codec_buffer_index, i->second);
@@ -974,6 +992,21 @@ AndroidVideoDecodeAccelerator::GetGlDecoder() const {
return gl_decoder_;
}
+gpu::gles2::TextureRef* const
+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_);

Powered by Google App Engine
This is Rietveld 408576698