Commit 17392d78 authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

Fix broken MediaCodec VP8 drain when using asynchronous API.

We invalidated the callback we give to MediaCodecBridge for asynchronous
API callbacks during MCVD::Destroy(), so we never get notified when the
EOS we queued as input completes. Thus leaking VP8 players continuously!

We could change to not invalidating the WeakPtr for the asynchronous
callback, but since draining is for a KitKat era issue, lets drop the
drain entirely when using the asynchrous API (N+).

BUG=1006092
TEST=vp8 instances are no longer leaked.
R=liberato

Change-Id: I3ea9c6013686000f176891eb8e645a60430adc71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906116
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarFrank Liberato <liberato@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714033}
parent 76f8bf1c
......@@ -246,6 +246,10 @@ void MediaCodecVideoDecoder::Destroy() {
TRACE_EVENT0("media", "MediaCodecVideoDecoder::Destroy");
// Cancel pending callbacks.
//
// WARNING: This will lose the callback we've given to MediaCodecBridge for
// asynchronous notifications; so we must not leave this function with any
// work necessary from StartTimerOrPumpCodec().
weak_factory_.InvalidateWeakPtrs();
if (media_crypto_context_) {
......@@ -265,6 +269,10 @@ void MediaCodecVideoDecoder::Destroy() {
codec_allocator_weak_factory_.InvalidateWeakPtrs();
CancelPendingDecodes(DecodeStatus::ABORTED);
StartDrainingCodec(DrainType::kForDestroy);
// Per the WARNING above. Validate that no draining work remains.
if (using_async_api_)
DCHECK(!drain_type_.has_value());
}
void MediaCodecVideoDecoder::Initialize(const VideoDecoderConfig& config,
......@@ -1021,7 +1029,7 @@ void MediaCodecVideoDecoder::StartDrainingCodec(DrainType drain_type) {
// TODO(watk): Strongly consider blacklisting VP8 (or specific MediaCodecs)
// instead. Draining is responsible for a lot of complexity.
if (decoder_config_.codec() != kCodecVP8 || !codec_ || codec_->IsFlushed() ||
codec_->IsDrained()) {
codec_->IsDrained() || using_async_api_) {
// If the codec isn't already drained or flushed, then we have to remember
// that we owe it a flush. We also have to remember not to deliver any
// output buffers that might still be in progress in the codec.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment