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

Unified Diff: media/renderers/video_renderer_impl.cc

Issue 1247973002: Fix race condition when accessing time_progressing_. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 5 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: media/renderers/video_renderer_impl.cc
diff --git a/media/renderers/video_renderer_impl.cc b/media/renderers/video_renderer_impl.cc
index 1160b65dae5773ddcbd7bba5226c319f218543b2..5023b6b2406b7bdb44d377d9eb4226bfcbacc1a2 100644
--- a/media/renderers/video_renderer_impl.cc
+++ b/media/renderers/video_renderer_impl.cc
@@ -213,7 +213,8 @@ scoped_refptr<VideoFrame> VideoRendererImpl::Render(
// end of stream, or have frames available. We also don't want to do this in
// background rendering mode unless this isn't the first background render
// tick and we haven't seen any decoded frames since the last one.
- const size_t effective_frames = MaybeFireEndedCallback();
+ const size_t effective_frames =
+ MaybeFireEndedCallback(!render_first_frame_and_stop_);
xhwang 2015/07/21 21:01:38 Can you explain a little bit more how "inverse of
DaleCurtis 2015/07/22 00:44:43 Done.
if (buffering_state_ == BUFFERING_HAVE_ENOUGH && !received_end_of_stream_ &&
!effective_frames && (!background_rendering ||
(!frames_decoded_ && was_background_rendering_))) {
@@ -496,7 +497,7 @@ void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
// See if we can fire EOS immediately instead of waiting for Render().
if (use_new_video_renderering_path_)
- MaybeFireEndedCallback();
+ MaybeFireEndedCallback(time_progressing_);
} else {
// Maintain the latest frame decoded so the correct frame is displayed
// after prerolling has completed.
@@ -708,13 +709,11 @@ void VideoRendererImpl::MaybeStopSinkAfterFirstPaint() {
DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK(use_new_video_renderering_path_);
- {
- base::AutoLock auto_lock(lock_);
- render_first_frame_and_stop_ = false;
- }
-
if (!time_progressing_ && sink_started_)
StopSink();
+
+ base::AutoLock auto_lock(lock_);
+ render_first_frame_and_stop_ = false;
}
bool VideoRendererImpl::HaveReachedBufferingCap() {
@@ -748,7 +747,7 @@ void VideoRendererImpl::StopSink() {
was_background_rendering_ = false;
}
-size_t VideoRendererImpl::MaybeFireEndedCallback() {
+size_t VideoRendererImpl::MaybeFireEndedCallback(bool time_progressing) {
xhwang 2015/07/21 21:01:38 This must be called under the lock, append _Locked
DaleCurtis 2015/07/22 00:44:43 Done.
// If there's only one frame in the video or Render() was never called, the
// algorithm will have one frame linger indefinitely. So in cases where the
// frame duration is unknown and we've received EOS, fire it once we get down
@@ -760,7 +759,7 @@ size_t VideoRendererImpl::MaybeFireEndedCallback() {
return effective_frames;
// Don't fire ended if time isn't moving and we have frames.
- if (!time_progressing_ && algorithm_->frames_queued())
+ if (!time_progressing && algorithm_->frames_queued())
return effective_frames;
// Fire ended if we have no more effective frames or only ever had one frame.
« media/renderers/video_renderer_impl.h ('K') | « media/renderers/video_renderer_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698