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

Unified Diff: media/gpu/android_video_decode_accelerator.cc

Issue 2461073002: Use MediaCodec.setOutputSurface() for fullscreen transitions on M. (Closed)
Patch Set: Rework logic. Fix nits. Created 4 years, 1 month 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: media/gpu/android_video_decode_accelerator.cc
diff --git a/media/gpu/android_video_decode_accelerator.cc b/media/gpu/android_video_decode_accelerator.cc
index 11adbd8189c59bc4f157815a5828ce9f55d11f23..0e65a89222bc48a4e7cd36308b90a99f505f2f88 100644
--- a/media/gpu/android_video_decode_accelerator.cc
+++ b/media/gpu/android_video_decode_accelerator.cc
@@ -169,7 +169,7 @@ class AVDAManager {
// is assumed to be on the way out, so we fail its allocation request.
bool AllocateSurface(int surface_id, AndroidVideoDecodeAccelerator* avda) {
// Nobody has to wait for no surface.
- if (surface_id == AndroidVideoDecodeAccelerator::Config::kNoSurfaceID)
+ if (surface_id == SurfaceManager::kNoSurfaceID)
return true;
auto iter = surface_waiter_map_.find(surface_id);
@@ -225,8 +225,7 @@ class AVDAManager {
// defer the surface creation until the codec is actually used if we know no
// software fallback exists.
bool ShouldDeferSurfaceCreation(int surface_id, VideoCodec codec) {
- return surface_id == AndroidVideoDecodeAccelerator::Config::kNoSurfaceID &&
- codec == kCodecH264 &&
+ return surface_id == SurfaceManager::kNoSurfaceID && codec == kCodecH264 &&
g_avda_codec_allocator.Get().IsAnyRegisteredAVDA() &&
(base::android::BuildInfo::GetInstance()->sdk_int() <= 18 ||
base::SysInfo::IsLowEndDevice());
@@ -319,6 +318,7 @@ AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
deferred_initialization_pending_(false),
codec_needs_reset_(false),
defer_surface_creation_(false),
+ surface_id_(SurfaceManager::kNoSurfaceID),
weak_this_factory_(this) {}
AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
@@ -397,15 +397,21 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
return false;
}
+ // If SetSurface() was called before initialize, pick up the surface.
+ if (pending_surface_id_) {
+ surface_id_ = pending_surface_id_.value();
+ pending_surface_id_.reset();
+ }
+
// If we're low on resources, we may decide to defer creation of the surface
// until the codec is actually used.
- if (g_avda_manager.Get().ShouldDeferSurfaceCreation(config_.surface_id,
+ if (g_avda_manager.Get().ShouldDeferSurfaceCreation(surface_id_,
codec_config_->codec_)) {
DCHECK(!deferred_initialization_pending_);
// We should never be here if a SurfaceView is required.
- DCHECK_EQ(config_.surface_id, Config::kNoSurfaceID);
- DCHECK(g_avda_manager.Get().AllocateSurface(config_.surface_id, this));
+ DCHECK_EQ(surface_id_, SurfaceManager::kNoSurfaceID);
+ DCHECK(g_avda_manager.Get().AllocateSurface(surface_id_, this));
defer_surface_creation_ = true;
NotifyInitializationComplete(true);
@@ -420,7 +426,7 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
return false;
}
- if (g_avda_manager.Get().AllocateSurface(config_.surface_id, this)) {
+ if (g_avda_manager.Get().AllocateSurface(surface_id_, this)) {
// We have successfully owned the surface, so finish initialization now.
return InitializePictureBufferManager();
}
@@ -446,8 +452,7 @@ bool AndroidVideoDecodeAccelerator::InitializePictureBufferManager() {
return false;
}
- codec_config_->surface_ =
- picture_buffer_manager_.Initialize(config_.surface_id);
+ codec_config_->surface_ = picture_buffer_manager_.Initialize(surface_id_);
if (codec_config_->surface_.IsEmpty())
return false;
@@ -654,6 +659,17 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
return false;
}
+ // If we're waiting to switch surfaces pause output release until we have all
+ // picture buffers returned. This is so we can ensure the right flags are set
+ // on the picture buffers returned to the client.
+ if (pending_surface_id_) {
+ DCHECK_GE(base::android::BuildInfo::GetInstance()->sdk_int(), 23);
watk 2016/11/08 22:59:03 Feels like this DCHECK belongs in SetSurface(). i.
DaleCurtis 2016/11/08 23:50:11 Moved to the codec bridge.
+ if (picture_buffer_manager_.ArePicturesOutstanding())
+ return false;
+ if (!UpdateSurface())
+ return false;
+ }
+
bool eos = false;
base::TimeDelta presentation_timestamp;
int32_t buf_index = 0;
@@ -887,7 +903,7 @@ void AndroidVideoDecodeAccelerator::RequestPictureBuffers() {
client_->ProvidePictureBuffers(
kNumPictureBuffers, PIXEL_FORMAT_UNKNOWN, 1,
picture_buffer_manager_.GetPictureBufferSize(),
- picture_buffer_manager_.GetTextureTarget());
+ AVDAPictureBufferManager::kTextureTarget);
}
}
@@ -1241,6 +1257,18 @@ void AndroidVideoDecodeAccelerator::Reset() {
}
}
+void AndroidVideoDecodeAccelerator::SetSurface(int32_t surface_id) {
+ if (surface_id == surface_id_) {
+ pending_surface_id_.reset();
+ return;
+ }
+
+ // Surface changes never take affect immediately, they will be handled during
watk 2016/11/08 22:59:03 s/affect/effect/
DaleCurtis 2016/11/08 23:50:11 Done.
+ // DequeOutput() once we get to a good switch point or immediately during an
+ // OnDestroyingSurface() call.
+ pending_surface_id_ = surface_id;
+}
+
void AndroidVideoDecodeAccelerator::Destroy() {
DVLOG(1) << __FUNCTION__;
DCHECK(thread_checker_.CalledOnValidThread());
@@ -1274,7 +1302,7 @@ void AndroidVideoDecodeAccelerator::ActualDestroy() {
// We no longer care about |surface_id|, in case we did before. It's okay
// if we have no surface and/or weren't the owner or a waiter.
- g_avda_manager.Get().DeallocateSurface(config_.surface_id, this);
+ g_avda_manager.Get().DeallocateSurface(surface_id_, this);
// Note that async codec construction might still be in progress. In that
// case, the codec will be deleted when it completes once we invalidate all
@@ -1308,9 +1336,29 @@ void AndroidVideoDecodeAccelerator::OnDestroyingSurface(int surface_id) {
TRACE_EVENT0("media", "AVDA::OnDestroyingSurface");
DVLOG(1) << __FUNCTION__ << " surface_id: " << surface_id;
- if (surface_id != config_.surface_id)
+ if (surface_id != surface_id_)
return;
+ // If the API is available avoid having to restart the decoder in order to
+ // leave full screen. If we don't clear the surface immediately during this
watk 2016/11/08 22:59:03 s/full screen/fullscreen for consistency
DaleCurtis 2016/11/08 23:50:10 Done.
+ // callback, the MediaCodec will throw an error as the surface is destroyed.
+ if (base::android::BuildInfo::GetInstance()->sdk_int() >= 23) {
+ // Since we can't wait for a transition, we must invalidate all outstanding
+ // picture buffers to avoid putting the GL system in a broken state.
+ picture_buffer_manager_.ReleaseCodecBuffers(output_picture_buffers_);
+
+ // Switch away from the surface being destroyed to a surface texture.
+ DCHECK_NE(surface_id_, SurfaceManager::kNoSurfaceID);
+
+ // The leaving full screen notification may come in before this point.
watk 2016/11/08 22:59:03 s/full screen/fullscreen
DaleCurtis 2016/11/08 23:50:11 Done.
+ if (pending_surface_id_)
+ DCHECK_EQ(pending_surface_id_.value(), SurfaceManager::kNoSurfaceID);
+
+ pending_surface_id_ = SurfaceManager::kNoSurfaceID;
+ UpdateSurface();
+ return;
+ }
+
// If we're currently asynchronously configuring a codec, it will be destroyed
// when configuration completes and it notices that |state_| has changed to
// SURFACE_DESTROYED.
@@ -1584,4 +1632,32 @@ bool AndroidVideoDecodeAccelerator::IsMediaCodecSoftwareDecodingForbidden()
codec_config_->codec_ == kCodecVP9);
}
+bool AndroidVideoDecodeAccelerator::UpdateSurface() {
+ DCHECK_GE(base::android::BuildInfo::GetInstance()->sdk_int(), 23);
+ DCHECK(pending_surface_id_);
+ DCHECK_NE(surface_id_, pending_surface_id_.value());
+
+ // Ensure the current context is active when switching surfaces; we may need
+ // to create a new texture.
+ if (!make_context_current_cb_.Run()) {
+ POST_ERROR(PLATFORM_FAILURE,
+ "Failed to make this decoder's GL context current when "
+ "switching surfaces.");
+ return false;
+ }
+
+ surface_id_ = pending_surface_id_.value();
+ codec_config_->surface_ =
+ picture_buffer_manager_.Initialize(pending_surface_id_.value());
+ if (codec_config_->surface_.IsEmpty()) {
+ POST_ERROR(PLATFORM_FAILURE, "An error occurred while switching surfaces.");
+ return false;
+ }
+
+ if (media_codec_)
+ media_codec_->SetSurface(codec_config_->surface_.j_surface().obj());
+ pending_surface_id_.reset();
+ return true;
+}
+
} // namespace media

Powered by Google App Engine
This is Rietveld 408576698