Commit 6d5e8a2d authored by Ted Meyer's avatar Ted Meyer Committed by Commit Bot

Fix race condition:

There is a race condition between AudioRenderer::Reinitialize and

audio_clock_ to null. Sometimes webmediaplayerimpl can request a
timestamp from the pipeline->renderer->audio_renderer which could race
with an ongoing track switch doing a reinitialize.

AudioRenderer: :CurrentMediaTime where reinitialize was setting
Bug: 863764
Change-Id: I6d6a32be37c42bde37d1d02353e3f43a58ab4e66
Reviewed-on: https://chromium-review.googlesource.com/1142899Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577050}
parent 934be57a
......@@ -271,6 +271,7 @@ base::TimeDelta RendererImpl::GetMediaTime() {
return restarting_audio_time_;
}
}
return time_source_->CurrentMediaTime();
}
......@@ -619,12 +620,12 @@ void RendererImpl::RestartAudioRenderer(
return;
}
audio_renderer_->StartPlaying();
{
base::AutoLock lock(restarting_audio_lock_);
audio_playing_ = true;
pending_audio_track_change_ = false;
}
audio_renderer_->StartPlaying();
std::move(restart_completed_cb).Run();
}
......@@ -646,9 +647,9 @@ void RendererImpl::RestartVideoRenderer(
return;
}
video_renderer_->StartPlayingFrom(time);
video_playing_ = true;
pending_video_track_change_ = false;
video_renderer_->StartPlayingFrom(time);
std::move(restart_completed_cb).Run();
}
......@@ -904,15 +905,9 @@ void RendererImpl::OnVideoOpacityChange(bool opaque) {
}
void RendererImpl::CleanUpTrackChange(base::RepeatingClosure on_finished,
bool* pending_change,
bool* ended,
bool* playing) {
{
// This lock is required for setting pending_audio_track_change_, and has
// no effect when setting pending_video_track_change_.
base::AutoLock lock(restarting_audio_lock_);
*pending_change = *ended = *playing = false;
}
*ended = *playing = false;
std::move(on_finished).Run();
}
......@@ -945,8 +940,7 @@ void RendererImpl::OnSelectedVideoTracksChanged(
pending_video_track_change_ = true;
video_renderer_->Flush(base::BindRepeating(
&RendererImpl::CleanUpTrackChange, weak_this_,
base::Passed(&fix_stream_cb), &pending_video_track_change_, &video_ended_,
&video_playing_));
base::Passed(&fix_stream_cb), &video_ended_, &video_playing_));
}
void RendererImpl::OnEnabledAudioTracksChanged(
......@@ -987,8 +981,7 @@ void RendererImpl::OnEnabledAudioTracksChanged(
audio_renderer_->Flush(base::BindRepeating(
&RendererImpl::CleanUpTrackChange, weak_this_,
base::Passed(&fix_stream_cb), &pending_audio_track_change_, &audio_ended_,
&audio_playing_));
base::Passed(&fix_stream_cb), &audio_ended_, &audio_playing_));
}
} // namespace media
......@@ -148,7 +148,6 @@ class MEDIA_EXPORT RendererImpl : public Renderer {
// Fix state booleans after the stream switching is finished.
void CleanUpTrackChange(base::RepeatingClosure on_finished,
bool* pending_change,
bool* ended,
bool* playing);
......
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