diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-01 07:13:58 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-01 07:13:58 +0000 |
commit | a8e24d5265840bb47aad860d987d015f195c6599 (patch) | |
tree | dd63c01c3af7a09b1b88605f1b3a8244facfd87b /media | |
parent | 08d358276cfc2b4fda202598bef9f0eff053eb75 (diff) | |
download | chromium_src-a8e24d5265840bb47aad860d987d015f195c6599.zip chromium_src-a8e24d5265840bb47aad860d987d015f195c6599.tar.gz chromium_src-a8e24d5265840bb47aad860d987d015f195c6599.tar.bz2 |
Stopgap fix for crash in issue 53867 comment 15
gwtquake lets the renderer create > 300 threads (one per audio element?), and eventually thread creation fails. This CL makes the media code more robust against thread creation failure (currently, it just crashes the renderer).
The Real Fix probably is to have a thread pool for media stuff instead of one thread per media object. And maybe threads just leak under some circumstances. I will file a follow-up bug for that case, hopefully with a reduced test case.
BUG=53867,61293
TEST=Completing the first level in gwtquake shouldn't crash the renderer.
Review URL: http://codereview.chromium.org/5362003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67824 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
-rw-r--r-- | media/base/pipeline_impl.cc | 122 | ||||
-rw-r--r-- | media/base/pipeline_impl.h | 2 |
2 files changed, 69 insertions, 55 deletions
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc index c53044f..049ab00 100644 --- a/media/base/pipeline_impl.cc +++ b/media/base/pipeline_impl.cc @@ -937,7 +937,7 @@ void PipelineImpl::FinishDestroyingFiltersTask() { } } -void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) { +bool PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK(IsPipelineOk()); @@ -948,7 +948,7 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) { if (!thread.get() || !thread->Start()) { NOTREACHED() << "Could not start filter thread"; SetError(PIPELINE_ERROR_INITIALIZATION_FAILED); - return; + return false; } filter->set_message_loop(thread->message_loop()); @@ -959,6 +959,7 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) { DCHECK(IsPipelineOk()); filter->set_host(this); filters_.push_back(make_scoped_refptr(filter.get())); + return true; } void PipelineImpl::InitializeDataSource() { @@ -977,7 +978,9 @@ void PipelineImpl::InitializeDataSource() { return; } - PrepareFilter(data_source); + if (!PrepareFilter(data_source)) + return; + pipeline_init_state_->data_source_ = data_source; data_source->Initialize( url_, NewCallback(this, &PipelineImpl::OnFilterInitialize)); @@ -998,7 +1001,9 @@ void PipelineImpl::InitializeDemuxer( return; } - PrepareFilter(demuxer); + if (!PrepareFilter(demuxer)) + return; + pipeline_init_state_->demuxer_ = demuxer; demuxer->Initialize(data_source, NewCallback(this, &PipelineImpl::OnFilterInitialize)); @@ -1012,23 +1017,25 @@ bool PipelineImpl::InitializeAudioDecoder( scoped_refptr<DemuxerStream> stream = FindDemuxerStream(demuxer, mime_type::kMajorTypeAudio); - if (stream) { - scoped_refptr<AudioDecoder> audio_decoder; - filter_collection_->SelectAudioDecoder(&audio_decoder); + if (!stream) + return false; - if (audio_decoder) { - PrepareFilter(audio_decoder); - pipeline_init_state_->audio_decoder_ = audio_decoder; - audio_decoder->Initialize( - stream, - NewCallback(this, &PipelineImpl::OnFilterInitialize)); - return true; - } else { - SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); - } + scoped_refptr<AudioDecoder> audio_decoder; + filter_collection_->SelectAudioDecoder(&audio_decoder); + + if (!audio_decoder) { + SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); + return false; } - return false; + if (!PrepareFilter(audio_decoder)) + return false; + + pipeline_init_state_->audio_decoder_ = audio_decoder; + audio_decoder->Initialize( + stream, + NewCallback(this, &PipelineImpl::OnFilterInitialize)); + return true; } bool PipelineImpl::InitializeVideoDecoder( @@ -1039,22 +1046,25 @@ bool PipelineImpl::InitializeVideoDecoder( scoped_refptr<DemuxerStream> stream = FindDemuxerStream(demuxer, mime_type::kMajorTypeVideo); - if (stream) { - scoped_refptr<VideoDecoder> video_decoder; - filter_collection_->SelectVideoDecoder(&video_decoder); + if (!stream) + return false; + + scoped_refptr<VideoDecoder> video_decoder; + filter_collection_->SelectVideoDecoder(&video_decoder); - if (video_decoder) { - PrepareFilter(video_decoder); - pipeline_init_state_->video_decoder_ = video_decoder; - video_decoder->Initialize( - stream, - NewCallback(this, &PipelineImpl::OnFilterInitialize)); - return true; - } else { - SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); - } + if (!video_decoder) { + SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); + return false; } - return false; + + if (!PrepareFilter(video_decoder)) + return false; + + pipeline_init_state_->video_decoder_ = video_decoder; + video_decoder->Initialize( + stream, + NewCallback(this, &PipelineImpl::OnFilterInitialize)); + return true; } bool PipelineImpl::InitializeAudioRenderer( @@ -1062,19 +1072,21 @@ bool PipelineImpl::InitializeAudioRenderer( DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK(IsPipelineOk()); - if (decoder) { - filter_collection_->SelectAudioRenderer(&audio_renderer_); + if (!decoder) + return false; - if (audio_renderer_) { - PrepareFilter(audio_renderer_); - audio_renderer_->Initialize( - decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize)); - return true; - } else { - SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); - } + filter_collection_->SelectAudioRenderer(&audio_renderer_); + if (!audio_renderer_) { + SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); + return false; } - return false; + + if (!PrepareFilter(audio_renderer_)) + return false; + + audio_renderer_->Initialize( + decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize)); + return true; } bool PipelineImpl::InitializeVideoRenderer( @@ -1082,19 +1094,21 @@ bool PipelineImpl::InitializeVideoRenderer( DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK(IsPipelineOk()); - if (decoder) { - filter_collection_->SelectVideoRenderer(&video_renderer_); + if (!decoder) + return false; - if (video_renderer_) { - PrepareFilter(video_renderer_); - video_renderer_->Initialize( - decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize)); - return true; - } else { - SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); - } + filter_collection_->SelectVideoRenderer(&video_renderer_); + if (!video_renderer_) { + SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING); + return false; } - return false; + + if (!PrepareFilter(video_renderer_)) + return false; + + video_renderer_->Initialize( + decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize)); + return true; } scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream( diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h index 86961cf..603194a 100644 --- a/media/base/pipeline_impl.h +++ b/media/base/pipeline_impl.h @@ -234,7 +234,7 @@ class PipelineImpl : public Pipeline, public FilterHost { // PrepareFilter() creates the filter's thread and injects a FilterHost and // MessageLoop. - void PrepareFilter(scoped_refptr<MediaFilter> filter); + bool PrepareFilter(scoped_refptr<MediaFilter> filter); // The following initialize methods are used to select a specific type of // MediaFilter object from MediaFilterCollection and initialize it |