Commit 4889f186 authored by watk's avatar watk Committed by Commit bot

Invalidate VideoRendererImpl's weak ptrs before resetting the decoder

Previously VideoRendererImpl waited for the VideoFrameStream::Reset() to
complete before invalidating its frame callback weak pointers, which
meant that it could receive frames that it didn't want between Flush()
and OnVideoFrameStreamResetDone(). For simplicity, now the weak pointers
are invalidated at the same time that Reset() is called, so it's
guaranteed that no frames will be received after Flush().

Also included in this change is reordering the deletion of the video
frame queue and resetting of the VideoFrameStream to save VDAs doing
wasted work.

BUG=640800

Review-Url: https://codereview.chromium.org/2273033003
Cr-Commit-Position: refs/heads/master@{#414285}
parent 0c448a7d
...@@ -93,8 +93,6 @@ void VideoRendererImpl::Flush(const base::Closure& callback) { ...@@ -93,8 +93,6 @@ void VideoRendererImpl::Flush(const base::Closure& callback) {
flush_cb_ = callback; flush_cb_ = callback;
state_ = kFlushing; state_ = kFlushing;
// This is necessary if the |video_frame_stream_| has already seen an end of
// stream and needs to drain it before flushing it.
if (buffering_state_ != BUFFERING_HAVE_NOTHING) { if (buffering_state_ != BUFFERING_HAVE_NOTHING) {
buffering_state_ = BUFFERING_HAVE_NOTHING; buffering_state_ = BUFFERING_HAVE_NOTHING;
task_runner_->PostTask( task_runner_->PostTask(
...@@ -104,11 +102,18 @@ void VideoRendererImpl::Flush(const base::Closure& callback) { ...@@ -104,11 +102,18 @@ void VideoRendererImpl::Flush(const base::Closure& callback) {
received_end_of_stream_ = false; received_end_of_stream_ = false;
rendered_end_of_stream_ = false; rendered_end_of_stream_ = false;
algorithm_->Reset(); // Reset |video_frame_stream_| and drop any pending read callbacks from it.
pending_read_ = false;
frame_callback_weak_factory_.InvalidateWeakPtrs();
video_frame_stream_->Reset( video_frame_stream_->Reset(
base::Bind(&VideoRendererImpl::OnVideoFrameStreamResetDone, base::Bind(&VideoRendererImpl::OnVideoFrameStreamResetDone,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
// To avoid unnecessary work by VDAs, only delete queued frames after
// resetting |video_frame_stream_|. If this is done in the opposite order VDAs
// will get a bunch of ReusePictureBuffer() calls before the Reset(), which
// they may use to output more frames that won't be used.
algorithm_->Reset();
} }
void VideoRendererImpl::StartPlayingFrom(base::TimeDelta timestamp) { void VideoRendererImpl::StartPlayingFrom(base::TimeDelta timestamp) {
...@@ -337,9 +342,7 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status, ...@@ -337,9 +342,7 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
base::AutoLock auto_lock(lock_); base::AutoLock auto_lock(lock_);
DCHECK_NE(state_, kUninitialized); DCHECK_EQ(state_, kPlaying);
DCHECK_NE(state_, kFlushed);
CHECK(pending_read_); CHECK(pending_read_);
pending_read_ = false; pending_read_ = false;
...@@ -352,14 +355,6 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status, ...@@ -352,14 +355,6 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
return; return;
} }
// Already-queued VideoFrameStream ReadCB's can fire after various state
// transitions have happened; in that case just drop those frames
// immediately.
if (state_ == kFlushing)
return;
DCHECK_EQ(state_, kPlaying);
// Can happen when demuxers are preparing for a new Seek(). // Can happen when demuxers are preparing for a new Seek().
if (!frame) { if (!frame) {
DCHECK_EQ(status, VideoFrameStream::DEMUXER_READ_ABORTED); DCHECK_EQ(status, VideoFrameStream::DEMUXER_READ_ABORTED);
...@@ -520,11 +515,6 @@ void VideoRendererImpl::OnVideoFrameStreamResetDone() { ...@@ -520,11 +515,6 @@ void VideoRendererImpl::OnVideoFrameStreamResetDone() {
DCHECK(!rendered_end_of_stream_); DCHECK(!rendered_end_of_stream_);
DCHECK_EQ(buffering_state_, BUFFERING_HAVE_NOTHING); DCHECK_EQ(buffering_state_, BUFFERING_HAVE_NOTHING);
// Pending read might be true if an async video frame copy is in flight.
pending_read_ = false;
// Drop any pending calls to FrameReady() and
// FrameReadyForCopyingToGpuMemoryBuffers()
frame_callback_weak_factory_.InvalidateWeakPtrs();
state_ = kFlushed; state_ = kFlushed;
base::ResetAndReturn(&flush_cb_).Run(); base::ResetAndReturn(&flush_cb_).Run();
} }
......
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