diff options
author | scherkus@chromium.org <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-12 06:33:18 +0000 |
---|---|---|
committer | scherkus@chromium.org <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-12 06:33:18 +0000 |
commit | 5eb75a05d1f57551624cccf502078262f9192bfe (patch) | |
tree | cac4f0fdf4b204c55b685be6b4dcf9e56297dd7f /media | |
parent | 5042f9a4d520284b5c51a2b56f9e4b985b8acd15 (diff) | |
download | chromium_src-5eb75a05d1f57551624cccf502078262f9192bfe.zip chromium_src-5eb75a05d1f57551624cccf502078262f9192bfe.tar.gz chromium_src-5eb75a05d1f57551624cccf502078262f9192bfe.tar.bz2 |
Trust the asynchronous read protocol and stop posting tasks to the pipeline.
Review URL: http://codereview.chromium.org/118458
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18257 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
-rw-r--r-- | media/filters/audio_renderer_base.cc | 3 | ||||
-rw-r--r-- | media/filters/audio_renderer_base.h | 11 | ||||
-rw-r--r-- | media/filters/decoder_base.h | 96 | ||||
-rw-r--r-- | media/filters/ffmpeg_audio_decoder.cc | 2 | ||||
-rw-r--r-- | media/filters/ffmpeg_video_decoder.cc | 2 | ||||
-rw-r--r-- | media/filters/test_video_decoder.h | 2 | ||||
-rw-r--r-- | media/filters/video_thread.cc | 4 | ||||
-rw-r--r-- | media/filters/video_thread.h | 4 |
8 files changed, 58 insertions, 66 deletions
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc index 7114e60..22d275b 100644 --- a/media/filters/audio_renderer_base.cc +++ b/media/filters/audio_renderer_base.cc @@ -205,8 +205,7 @@ size_t AudioRendererBase::FillBuffer(uint8* dest, size_t dest_len, } void AudioRendererBase::ScheduleRead() { - host_->PostTask(NewRunnableMethod(decoder_, &AudioDecoder::Read, - NewCallback(this, &AudioRendererBase::OnReadComplete))); + decoder_->Read(NewCallback(this, &AudioRendererBase::OnReadComplete)); } // static diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h index cb2d5f8..83111aa 100644 --- a/media/filters/audio_renderer_base.h +++ b/media/filters/audio_renderer_base.h @@ -77,6 +77,11 @@ class AudioRendererBase : public AudioRenderer { int* sample_bits_out); private: + // Helper method that schedules an asynchronous read from the decoder. + // + // Safe to call from any thread. + void ScheduleRead(); + // Audio decoder. AudioDecoder* decoder_; @@ -101,12 +106,6 @@ class AudioRendererBase : public AudioRenderer { // TODO(ralphl): Update this value after seeking. base::TimeDelta last_fill_buffer_time_; - // Posts a task on the pipeline thread to read a sample from the decoder. The - // resulting buffer will be placed in the queue. - // - // Safe to call on any thread. - void ScheduleRead(); - DISALLOW_COPY_AND_ASSIGN(AudioRendererBase); }; diff --git a/media/filters/decoder_base.h b/media/filters/decoder_base.h index 63de905..4f70f7a 100644 --- a/media/filters/decoder_base.h +++ b/media/filters/decoder_base.h @@ -33,49 +33,49 @@ class DecoderBase : public Decoder { process_task_->Cancel(); process_task_ = NULL; } - DiscardQueues(); + DiscardQueues_Locked(); } - // Because decode_thread_ is a scoped_ptr this will destroy the thread, - // if there was one, which causes it to be shut down in an orderly way. - decode_thread_.reset(); + // Stop our decoding thread. + thread_.Stop(); } virtual void Seek(base::TimeDelta time) { // Delegate to the subclass first. OnSeek(time); - { - AutoLock auto_lock(lock_); - // Flush the result queue. - result_queue_.clear(); + // Flush the result queue. + AutoLock auto_lock(lock_); + result_queue_.clear(); - // Flush the input queue. This will trigger more reads from the demuxer. - input_queue_.clear(); + // Flush the input queue. This will trigger more reads from the demuxer. + input_queue_.clear(); - // Turn on the seeking flag so that we can discard buffers until a - // discontinuous buffer is received. - seeking_ = true; - } - // ScheduleProcessTask to trigger more reads and keep the process loop - // rolling. - ScheduleProcessTask(); + // Turn on the seeking flag so that we can discard buffers until a + // discontinuous buffer is received. + seeking_ = true; + + // Trigger more reads and keep the process loop rolling. + ScheduleProcessTask_Locked(); } // Decoder implementation. virtual bool Initialize(DemuxerStream* demuxer_stream) { demuxer_stream_ = demuxer_stream; - if (decode_thread_.get()) { - if (!decode_thread_->Start()) { - NOTREACHED(); - return false; - } + + // Start our internal decoding thread. + if (!thread_.Start()) { + host()->Error(PIPELINE_ERROR_DECODE); + return false; } if (!OnInitialize(demuxer_stream)) { + // Release our resources and stop our thread. + // TODO(scherkus): shouldn't stop a thread inside Initialize(), but until I + // figure out proper error signaling semantics we're going to do it anyway!! host()->Error(PIPELINE_ERROR_DECODE); demuxer_stream_ = NULL; - decode_thread_.reset(); + thread_.Stop(); return false; } @@ -91,7 +91,7 @@ class DecoderBase : public Decoder { AutoLock auto_lock(lock_); if (IsRunning()) { read_queue_.push_back(read_callback); - ScheduleProcessTask(); + ScheduleProcessTask_Locked(); } else { delete read_callback; } @@ -113,26 +113,24 @@ class DecoderBase : public Decoder { if (!seeking_) input_queue_.push_back(buffer); --pending_reads_; - ScheduleProcessTask(); + ScheduleProcessTask_Locked(); } } protected: - // If NULL is passed for the |thread_name| then all processing of decodes - // will happen on the pipeline thread. If the name is non-NULL then a new - // thread will be created for this decoder, and it will be assigned the - // name provided by |thread_name|. + // |thread_name| is mandatory and is used to identify the thread in debuggers. explicit DecoderBase(const char* thread_name) : running_(true), demuxer_stream_(NULL), - decode_thread_(thread_name ? new base::Thread(thread_name) : NULL), + thread_(thread_name), pending_reads_(0), process_task_(NULL), seeking_(false) { } virtual ~DecoderBase() { - Stop(); + DCHECK(!thread_.IsRunning()); + DCHECK(!process_task_); } // This method is called by the derived class from within the OnDecode method. @@ -178,30 +176,27 @@ class DecoderBase : public Decoder { FilterHost* host() const { return Decoder::host_; } // Schedules a task that will execute the ProcessTask method. - void ScheduleProcessTask() { + void ScheduleProcessTask_Locked() { + lock_.AssertAcquired(); DCHECK(IsRunning()); if (!process_task_) { process_task_ = NewRunnableMethod(this, &DecoderBase::ProcessTask); - if (decode_thread_.get()) { - decode_thread_->message_loop()->PostTask(FROM_HERE, process_task_); - } else { - host()->PostTask(process_task_); - } + thread_.message_loop()->PostTask(FROM_HERE, process_task_); } } // The core work loop of the decoder base. This method will run the methods - // SubmitReads(), ProcessInput(), and ProcessOutput() in a loop until they - // either produce no further work, or the filter is stopped. Once there is - // no further work to do, the method returns. A later call to the - // ScheduleProcessTask() method will start this task again. + // SubmitReads_Locked(), ProcessInput_Locked(), and ProcessOutput_Locked() in + // a loop until they either produce no further work, or the filter is stopped. + // Once there is no further work to do, the method returns. A later call to + // the ScheduleProcessTask_Locked() method will start this task again. void ProcessTask() { AutoLock auto_lock(lock_); bool did_some_work; do { - did_some_work = SubmitReads(); - did_some_work |= ProcessInput(); - did_some_work |= ProcessOutput(); + did_some_work = SubmitReads_Locked(); + did_some_work |= ProcessInput_Locked(); + did_some_work |= ProcessOutput_Locked(); } while (IsRunning() && did_some_work); DCHECK(process_task_ || !IsRunning()); process_task_ = NULL; @@ -211,7 +206,7 @@ class DecoderBase : public Decoder { // if reads have happened, else false. This method must be called with // |lock_| acquired. If the method submits any reads, then it will Release() // the |lock_| when calling the demuxer and then re-Acquire() the |lock_|. - bool SubmitReads() { + bool SubmitReads_Locked() { lock_.AssertAcquired(); bool did_read = false; if (IsRunning() && @@ -233,7 +228,7 @@ class DecoderBase : public Decoder { // If the |input_queue_| has any buffers, this method will call the derived // class's OnDecode() method. - bool ProcessInput() { + bool ProcessInput_Locked() { lock_.AssertAcquired(); bool did_decode = false; while (IsRunning() && !input_queue_.empty()) { @@ -251,7 +246,7 @@ class DecoderBase : public Decoder { // Removes any buffers from the |result_queue_| and calls the next callback // in the |read_queue_|. - bool ProcessOutput() { + bool ProcessOutput_Locked() { lock_.AssertAcquired(); bool called_renderer = false; while (IsRunning() && !read_queue_.empty() && !result_queue_.empty()) { @@ -270,7 +265,7 @@ class DecoderBase : public Decoder { } // Throw away all buffers in all queues. - void DiscardQueues() { + void DiscardQueues_Locked() { lock_.AssertAcquired(); input_queue_.clear(); result_queue_.clear(); @@ -290,9 +285,8 @@ class DecoderBase : public Decoder { // Pointer to the demuxer stream that will feed us compressed buffers. scoped_refptr<DemuxerStream> demuxer_stream_; - // If this pointer is NULL then there is no thread dedicated to this decoder - // and decodes will happen on the pipeline thread. - scoped_ptr<base::Thread> decode_thread_; + // The dedicated decoding thread for this filter. + base::Thread thread_; // Number of times we have called Read() on the demuxer that have not yet // been satisfied. diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc index c9f4082..78e764e 100644 --- a/media/filters/ffmpeg_audio_decoder.cc +++ b/media/filters/ffmpeg_audio_decoder.cc @@ -14,7 +14,7 @@ const size_t FFmpegAudioDecoder::kOutputBufferSize = AVCODEC_MAX_AUDIO_FRAME_SIZE; FFmpegAudioDecoder::FFmpegAudioDecoder() - : DecoderBase<AudioDecoder, Buffer>(NULL), + : DecoderBase<AudioDecoder, Buffer>("AudioDecoderThread"), codec_context_(NULL) { } diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc index 004e4fe..250bc70 100644 --- a/media/filters/ffmpeg_video_decoder.cc +++ b/media/filters/ffmpeg_video_decoder.cc @@ -24,7 +24,7 @@ namespace media { static const int kDecodeThreads = 2; FFmpegVideoDecoder::FFmpegVideoDecoder() - : DecoderBase<VideoDecoder, VideoFrame>(NULL), + : DecoderBase<VideoDecoder, VideoFrame>("VideoDecoderThread"), width_(0), height_(0) { } diff --git a/media/filters/test_video_decoder.h b/media/filters/test_video_decoder.h index 84d6e66..15423d6 100644 --- a/media/filters/test_video_decoder.h +++ b/media/filters/test_video_decoder.h @@ -19,7 +19,7 @@ namespace media { class TestVideoDecoder : public DecoderBase<VideoDecoder, VideoFrame> { public: TestVideoDecoder() - : DecoderBase<VideoDecoder, VideoFrame>(NULL), + : DecoderBase<VideoDecoder, VideoFrame>("VideoDecoderThread"), video_width_(0), video_height_(0) { } diff --git a/media/filters/video_thread.cc b/media/filters/video_thread.cc index 994713d..644ae73 100644 --- a/media/filters/video_thread.cc +++ b/media/filters/video_thread.cc @@ -251,9 +251,7 @@ void VideoThread::OnReadComplete(VideoFrame* frame) { } void VideoThread::ScheduleRead() { - host_->PostTask( - NewRunnableMethod(decoder_.get(), &VideoDecoder::Read, - NewCallback(this, &VideoThread::OnReadComplete))); + decoder_->Read(NewCallback(this, &VideoThread::OnReadComplete)); } bool VideoThread::WaitForInitialized() { diff --git a/media/filters/video_thread.h b/media/filters/video_thread.h index abf632c..346f296 100644 --- a/media/filters/video_thread.h +++ b/media/filters/video_thread.h @@ -71,7 +71,9 @@ class VideoThread : public VideoRenderer, // Read complete callback from video decoder. void OnReadComplete(VideoFrame* frame); - // Helper method that asynchronously schedules a read on the pipeline thread. + // Helper method that schedules an asynchronous read from the decoder. + // + // Safe to call from any thread. void ScheduleRead(); // Called by ThreadMain() to handle preroll. Returns false if the thread |