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

Unified Diff: media/renderers/video_renderer_impl.cc

Issue 2273033003: Invalidate VideoRendererImpl's weak ptrs before resetting the decoder (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 4 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/renderers/video_renderer_impl.cc
diff --git a/media/renderers/video_renderer_impl.cc b/media/renderers/video_renderer_impl.cc
index 5e667a37c101cba9465250c54a3c0e73a141d720..a64769796c80565a2a1afb721ceae7ec731a5bd7 100644
--- a/media/renderers/video_renderer_impl.cc
+++ b/media/renderers/video_renderer_impl.cc
@@ -93,8 +93,6 @@ void VideoRendererImpl::Flush(const base::Closure& callback) {
flush_cb_ = callback;
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) {
buffering_state_ = BUFFERING_HAVE_NOTHING;
task_runner_->PostTask(
@@ -104,11 +102,18 @@ void VideoRendererImpl::Flush(const base::Closure& callback) {
received_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(
base::Bind(&VideoRendererImpl::OnVideoFrameStreamResetDone,
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) {
@@ -337,9 +342,7 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
DCHECK(task_runner_->BelongsToCurrentThread());
base::AutoLock auto_lock(lock_);
- DCHECK_NE(state_, kUninitialized);
- DCHECK_NE(state_, kFlushed);
-
+ DCHECK_EQ(state_, kPlaying);
CHECK(pending_read_);
pending_read_ = false;
@@ -352,14 +355,6 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
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().
if (!frame) {
DCHECK_EQ(status, VideoFrameStream::DEMUXER_READ_ABORTED);
@@ -520,11 +515,6 @@ void VideoRendererImpl::OnVideoFrameStreamResetDone() {
DCHECK(!rendered_end_of_stream_);
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;
base::ResetAndReturn(&flush_cb_).Run();
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698