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

Unified Diff: media/filters/renderer_impl.cc

Issue 870693002: Require Renderer::Initialize() to return a status code via callback. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 11 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/filters/renderer_impl.cc
diff --git a/media/filters/renderer_impl.cc b/media/filters/renderer_impl.cc
index a2b0d99ec5f003c7fb14a1f3d1dde3acfec17b79..8fd8c8ef10d49bd42ad80c76903e6e7cfeca9dfe 100644
--- a/media/filters/renderer_impl.cc
+++ b/media/filters/renderer_impl.cc
@@ -50,11 +50,12 @@ RendererImpl::~RendererImpl() {
video_renderer_.reset();
audio_renderer_.reset();
+ DCHECK(init_cb_.is_null());
xhwang 2015/01/23 00:22:01 hmm, what if Pipeline::StopTask() is called during
DaleCurtis 2015/01/23 20:31:21 Right, this is possible. Changed this to run init_
FireAllPendingCallbacks();
}
void RendererImpl::Initialize(DemuxerStreamProvider* demuxer_stream_provider,
- const base::Closure& init_cb,
+ const PipelineStatusCB& init_cb,
const StatisticsCB& statistics_cb,
const BufferingStateCB& buffering_state_cb,
const PaintCB& paint_cb,
@@ -262,16 +263,19 @@ void RendererImpl::OnAudioRendererInitializeDone(PipelineStatus status) {
DVLOG(1) << __FUNCTION__ << ": " << status;
DCHECK(task_runner_->BelongsToCurrentThread());
- if (status != PIPELINE_OK)
- OnError(status);
xhwang 2015/01/23 00:22:01 I agree this is confusing! :)
-
// OnError() may be fired at any time by the renderers, even if they thought
// they initialized successfully (due to delayed output device setup).
xhwang 2015/01/23 00:22:01 Ouch, this is painful :(
if (state_ != STATE_INITIALIZING) {
+ DCHECK(init_cb_.is_null());
audio_renderer_.reset();
return;
}
+ if (status != PIPELINE_OK) {
+ base::ResetAndReturn(&init_cb_).Run(status);
+ return;
+ }
+
DCHECK(!init_cb_.is_null());
InitializeVideoRenderer();
}
@@ -308,12 +312,10 @@ void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
DVLOG(1) << __FUNCTION__ << ": " << status;
DCHECK(task_runner_->BelongsToCurrentThread());
- if (status != PIPELINE_OK)
- OnError(status);
-
// OnError() may be fired at any time by the renderers, even if they thought
// they initialized successfully (due to delayed output device setup).
if (state_ != STATE_INITIALIZING) {
+ DCHECK(init_cb_.is_null());
audio_renderer_.reset();
video_renderer_.reset();
return;
@@ -321,6 +323,11 @@ void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
DCHECK(!init_cb_.is_null());
+ if (status != PIPELINE_OK) {
+ base::ResetAndReturn(&init_cb_).Run(status);
+ return;
+ }
+
if (audio_renderer_) {
time_source_ = audio_renderer_->GetTimeSource();
} else {
@@ -331,7 +338,7 @@ void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
state_ = STATE_PLAYING;
DCHECK(time_source_);
DCHECK(audio_renderer_ || video_renderer_);
- base::ResetAndReturn(&init_cb_).Run();
+ base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
}
void RendererImpl::FlushAudioRenderer() {
@@ -549,20 +556,21 @@ void RendererImpl::OnError(PipelineStatus error) {
if (state_ == STATE_ERROR)
return;
+ const State old_state = state_;
state_ = STATE_ERROR;
+ if (old_state == STATE_INITIALIZING) {
+ base::ResetAndReturn(&init_cb_).Run(error);
+ return;
+ }
+
// Pipeline will destroy |this| as the result of error.
base::ResetAndReturn(&error_cb_).Run(error);
-
FireAllPendingCallbacks();
}
void RendererImpl::FireAllPendingCallbacks() {
DCHECK(task_runner_->BelongsToCurrentThread());
-
- if (!init_cb_.is_null())
- base::ResetAndReturn(&init_cb_).Run();
-
if (!flush_cb_.is_null())
base::ResetAndReturn(&flush_cb_).Run();
}

Powered by Google App Engine
This is Rietveld 408576698