diff options
author | scherkus@chromium.org <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-05 00:56:27 +0000 |
---|---|---|
committer | scherkus@chromium.org <scherkus@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-05 00:56:27 +0000 |
commit | 30ef6b3943c6c788f701002eaf25a45b97c32e72 (patch) | |
tree | 55e0b727ad2d11c38f3e741a80425d42f2abeb64 /media | |
parent | 90ba0b7405b7226d7702b633b927d389487a73ec (diff) | |
download | chromium_src-30ef6b3943c6c788f701002eaf25a45b97c32e72.zip chromium_src-30ef6b3943c6c788f701002eaf25a45b97c32e72.tar.gz chromium_src-30ef6b3943c6c788f701002eaf25a45b97c32e72.tar.bz2 |
Remove reference counting from media::VideoRenderer and friends.
BUG=173313
Review URL: https://codereview.chromium.org/12114024
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@180591 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
-rw-r--r-- | media/base/filter_collection.cc | 20 | ||||
-rw-r--r-- | media/base/filter_collection.h | 9 | ||||
-rw-r--r-- | media/base/mock_filters.h | 4 | ||||
-rw-r--r-- | media/base/pipeline.cc | 34 | ||||
-rw-r--r-- | media/base/pipeline.h | 2 | ||||
-rw-r--r-- | media/base/pipeline_unittest.cc | 5 | ||||
-rw-r--r-- | media/base/video_renderer.h | 12 | ||||
-rw-r--r-- | media/filters/pipeline_integration_test_base.cc | 7 | ||||
-rw-r--r-- | media/filters/pipeline_integration_test_base.h | 1 | ||||
-rw-r--r-- | media/filters/video_renderer_base.cc | 32 | ||||
-rw-r--r-- | media/filters/video_renderer_base.h | 7 | ||||
-rw-r--r-- | media/filters/video_renderer_base_unittest.cc | 6 | ||||
-rw-r--r-- | media/tools/player_x11/player_x11.cc | 3 |
13 files changed, 75 insertions, 67 deletions
diff --git a/media/base/filter_collection.cc b/media/base/filter_collection.cc index f82a61b..7824b1e 100644 --- a/media/base/filter_collection.cc +++ b/media/base/filter_collection.cc @@ -29,15 +29,20 @@ void FilterCollection::AddAudioRenderer(AudioRenderer* audio_renderer) { audio_renderers_.push_back(audio_renderer); } -void FilterCollection::AddVideoRenderer(VideoRenderer* video_renderer) { - video_renderers_.push_back(video_renderer); +void FilterCollection::SetVideoRenderer( + scoped_ptr<VideoRenderer> video_renderer) { + video_renderer_ = video_renderer.Pass(); +} + +scoped_ptr<VideoRenderer> FilterCollection::GetVideoRenderer() { + return video_renderer_.Pass(); } void FilterCollection::Clear() { audio_decoders_.clear(); video_decoders_.clear(); audio_renderers_.clear(); - video_renderers_.clear(); + video_renderer_.reset(); } void FilterCollection::SelectAudioRenderer(scoped_refptr<AudioRenderer>* out) { @@ -49,15 +54,6 @@ void FilterCollection::SelectAudioRenderer(scoped_refptr<AudioRenderer>* out) { audio_renderers_.pop_front(); } -void FilterCollection::SelectVideoRenderer(scoped_refptr<VideoRenderer>* out) { - if (video_renderers_.empty()) { - *out = NULL; - return; - } - *out = video_renderers_.front(); - video_renderers_.pop_front(); -} - FilterCollection::AudioDecoderList* FilterCollection::GetAudioDecoders() { return &audio_decoders_; } diff --git a/media/base/filter_collection.h b/media/base/filter_collection.h index e9f2be5..100cd64 100644 --- a/media/base/filter_collection.h +++ b/media/base/filter_collection.h @@ -8,6 +8,7 @@ #include <list> #include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" #include "media/base/media_export.h" namespace media { @@ -36,9 +37,10 @@ class MEDIA_EXPORT FilterCollection { const scoped_refptr<Demuxer>& GetDemuxer(); // Adds a filter to the collection. - void AddAudioDecoder(AudioDecoder* audio_decoder); void AddAudioRenderer(AudioRenderer* audio_renderer); - void AddVideoRenderer(VideoRenderer* video_renderer); + + void SetVideoRenderer(scoped_ptr<VideoRenderer> video_renderer); + scoped_ptr<VideoRenderer> GetVideoRenderer(); // Remove remaining filters. void Clear(); @@ -48,7 +50,6 @@ class MEDIA_EXPORT FilterCollection { // If a filter is returned it is removed from the collection. // Filters are selected in FIFO order. void SelectAudioRenderer(scoped_refptr<AudioRenderer>* out); - void SelectVideoRenderer(scoped_refptr<VideoRenderer>* out); AudioDecoderList* GetAudioDecoders(); VideoDecoderList* GetVideoDecoders(); @@ -58,7 +59,7 @@ class MEDIA_EXPORT FilterCollection { AudioDecoderList audio_decoders_; VideoDecoderList video_decoders_; std::list<scoped_refptr<AudioRenderer> > audio_renderers_; - std::list<scoped_refptr<VideoRenderer> > video_renderers_; + scoped_ptr<VideoRenderer> video_renderer_; DISALLOW_COPY_AND_ASSIGN(FilterCollection); }; diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index 3050fb5..2050e87 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -134,6 +134,7 @@ class MockAudioDecoder : public AudioDecoder { class MockVideoRenderer : public VideoRenderer { public: MockVideoRenderer(); + virtual ~MockVideoRenderer(); // VideoRenderer implementation. MOCK_METHOD10(Initialize, void(const scoped_refptr<DemuxerStream>& stream, @@ -153,9 +154,6 @@ class MockVideoRenderer : public VideoRenderer { MOCK_METHOD1(Stop, void(const base::Closure& callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - protected: - virtual ~MockVideoRenderer(); - private: DISALLOW_COPY_AND_ASSIGN(MockVideoRenderer); }; diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc index cc6419a..cdc0aa6 100644 --- a/media/base/pipeline.cc +++ b/media/base/pipeline.cc @@ -538,7 +538,8 @@ void Pipeline::DoInitialPreroll(const PipelineStatusCB& done_cb) { if (video_renderer_) { bound_fns.Push(base::Bind( - &VideoRenderer::Preroll, video_renderer_, seek_timestamp)); + &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()), + seek_timestamp)); } pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb); @@ -554,14 +555,18 @@ void Pipeline::DoSeek( // Pause. if (audio_renderer_) bound_fns.Push(base::Bind(&AudioRenderer::Pause, audio_renderer_)); - if (video_renderer_) - bound_fns.Push(base::Bind(&VideoRenderer::Pause, video_renderer_)); + if (video_renderer_) { + bound_fns.Push(base::Bind( + &VideoRenderer::Pause, base::Unretained(video_renderer_.get()))); + } // Flush. if (audio_renderer_) bound_fns.Push(base::Bind(&AudioRenderer::Flush, audio_renderer_)); - if (video_renderer_) - bound_fns.Push(base::Bind(&VideoRenderer::Flush, video_renderer_)); + if (video_renderer_) { + bound_fns.Push(base::Bind( + &VideoRenderer::Flush, base::Unretained(video_renderer_.get()))); + } // Seek demuxer. bound_fns.Push(base::Bind( @@ -575,7 +580,8 @@ void Pipeline::DoSeek( if (video_renderer_) { bound_fns.Push(base::Bind( - &VideoRenderer::Preroll, video_renderer_, seek_timestamp)); + &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()), + seek_timestamp)); } pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb); @@ -592,8 +598,10 @@ void Pipeline::DoPlay(const PipelineStatusCB& done_cb) { if (audio_renderer_) bound_fns.Push(base::Bind(&AudioRenderer::Play, audio_renderer_)); - if (video_renderer_) - bound_fns.Push(base::Bind(&VideoRenderer::Play, video_renderer_)); + if (video_renderer_) { + bound_fns.Push(base::Bind( + &VideoRenderer::Play, base::Unretained(video_renderer_.get()))); + } pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb); } @@ -609,8 +617,10 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) { if (audio_renderer_) bound_fns.Push(base::Bind(&AudioRenderer::Stop, audio_renderer_)); - if (video_renderer_) - bound_fns.Push(base::Bind(&VideoRenderer::Stop, video_renderer_)); + if (video_renderer_) { + bound_fns.Push(base::Bind( + &VideoRenderer::Stop, base::Unretained(video_renderer_.get()))); + } pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb); } @@ -627,7 +637,7 @@ void Pipeline::OnStopCompleted(PipelineStatus status) { pending_callbacks_.reset(); filter_collection_.reset(); audio_renderer_ = NULL; - video_renderer_ = NULL; + video_renderer_.reset(); demuxer_ = NULL; // If we stop during initialization/seeking we want to run |seek_cb_| @@ -912,7 +922,7 @@ void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) { natural_size_ = stream->video_decoder_config().natural_size(); } - filter_collection_->SelectVideoRenderer(&video_renderer_); + video_renderer_ = filter_collection_->GetVideoRenderer(); video_renderer_->Initialize( stream, *filter_collection_->GetVideoDecoders(), diff --git a/media/base/pipeline.h b/media/base/pipeline.h index 25e4dcb..54d7239 100644 --- a/media/base/pipeline.h +++ b/media/base/pipeline.h @@ -452,7 +452,7 @@ class MEDIA_EXPORT Pipeline // Renderer references used for setting the volume, playback rate, and // determining when playback has finished. scoped_refptr<AudioRenderer> audio_renderer_; - scoped_refptr<VideoRenderer> video_renderer_; + scoped_ptr<VideoRenderer> video_renderer_; // Demuxer reference used for setting the preload value. scoped_refptr<Demuxer> demuxer_; diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc index 2d5b63b..cfe41b5 100644 --- a/media/base/pipeline_unittest.cc +++ b/media/base/pipeline_unittest.cc @@ -91,7 +91,8 @@ class PipelineTest : public ::testing::Test { filter_collection_->GetAudioDecoders()->push_back(audio_decoder_); video_renderer_ = new MockVideoRenderer(); - filter_collection_->AddVideoRenderer(video_renderer_); + scoped_ptr<VideoRenderer> video_renderer(video_renderer_); + filter_collection_->SetVideoRenderer(video_renderer.Pass()); audio_renderer_ = new MockAudioRenderer(); filter_collection_->AddAudioRenderer(audio_renderer_); @@ -299,7 +300,7 @@ class PipelineTest : public ::testing::Test { scoped_refptr<MockDemuxer> demuxer_; scoped_refptr<MockVideoDecoder> video_decoder_; scoped_refptr<MockAudioDecoder> audio_decoder_; - scoped_refptr<MockVideoRenderer> video_renderer_; + MockVideoRenderer* video_renderer_; scoped_refptr<MockAudioRenderer> audio_renderer_; scoped_refptr<StrictMock<MockDemuxerStream> > audio_stream_; scoped_refptr<StrictMock<MockDemuxerStream> > video_stream_; diff --git a/media/base/video_renderer.h b/media/base/video_renderer.h index c19c8f9..c609b68 100644 --- a/media/base/video_renderer.h +++ b/media/base/video_renderer.h @@ -22,8 +22,7 @@ namespace media { class DemuxerStream; class VideoDecoder; -class MEDIA_EXPORT VideoRenderer - : public base::RefCountedThreadSafe<VideoRenderer> { +class MEDIA_EXPORT VideoRenderer { public: typedef std::list<scoped_refptr<VideoDecoder> > VideoDecoderList; @@ -37,6 +36,9 @@ class MEDIA_EXPORT VideoRenderer // Used to query the current time or duration of the media. typedef base::Callback<base::TimeDelta()> TimeDeltaCB; + VideoRenderer(); + virtual ~VideoRenderer(); + // Initialize a VideoRenderer with the given DemuxerStream and // VideoDecoderList, executing |init_cb| callback upon completion. // @@ -91,12 +93,6 @@ class MEDIA_EXPORT VideoRenderer // Updates the current playback rate. virtual void SetPlaybackRate(float playback_rate) = 0; - protected: - friend class base::RefCountedThreadSafe<VideoRenderer>; - - VideoRenderer(); - virtual ~VideoRenderer(); - private: DISALLOW_COPY_AND_ASSIGN(VideoRenderer); }; diff --git a/media/filters/pipeline_integration_test_base.cc b/media/filters/pipeline_integration_test_base.cc index b065b57..54c4fdf 100644 --- a/media/filters/pipeline_integration_test_base.cc +++ b/media/filters/pipeline_integration_test_base.cc @@ -211,7 +211,7 @@ PipelineIntegrationTestBase::CreateFilterCollection( collection->GetVideoDecoders()->push_back(vpx_decoder); // Disable frame dropping if hashing is enabled. - renderer_ = new VideoRendererBase( + scoped_ptr<VideoRenderer> renderer(new VideoRendererBase( message_loop_.message_loop_proxy(), base::Bind(&PipelineIntegrationTestBase::SetDecryptor, base::Unretained(this), decryptor), @@ -219,8 +219,9 @@ PipelineIntegrationTestBase::CreateFilterCollection( base::Unretained(this)), base::Bind(&PipelineIntegrationTestBase::OnSetOpaque, base::Unretained(this)), - !hashing_enabled_); - collection->AddVideoRenderer(renderer_); + !hashing_enabled_)); + collection->SetVideoRenderer(renderer.Pass()); + audio_sink_ = new NullAudioSink(); if (hashing_enabled_) audio_sink_->StartAudioHashForTesting(); diff --git a/media/filters/pipeline_integration_test_base.h b/media/filters/pipeline_integration_test_base.h index 411e564..23c0339 100644 --- a/media/filters/pipeline_integration_test_base.h +++ b/media/filters/pipeline_integration_test_base.h @@ -74,7 +74,6 @@ class PipelineIntegrationTestBase { base::MD5Context md5_context_; bool hashing_enabled_; scoped_refptr<Pipeline> pipeline_; - scoped_refptr<VideoRendererBase> renderer_; scoped_refptr<NullAudioSink> audio_sink_; bool ended_; PipelineStatus pipeline_status_; diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc index d2c577b..0ae775a 100644 --- a/media/filters/video_renderer_base.cc +++ b/media/filters/video_renderer_base.cc @@ -29,6 +29,7 @@ VideoRendererBase::VideoRendererBase( const SetOpaqueCB& set_opaque_cb, bool drop_frames) : message_loop_(message_loop), + weak_factory_(this), set_decryptor_ready_cb_(set_decryptor_ready_cb), frame_available_(&lock_), state_(kUninitialized), @@ -42,6 +43,12 @@ VideoRendererBase::VideoRendererBase( DCHECK(!paint_cb_.is_null()); } +VideoRendererBase::~VideoRendererBase() { + base::AutoLock auto_lock(lock_); + CHECK(state_ == kUninitialized || state_ == kStopped) << state_; + CHECK_EQ(thread_, base::kNullThreadHandle); +} + void VideoRendererBase::Play(const base::Closure& callback) { DCHECK(message_loop_->BelongsToCurrentThread()); base::AutoLock auto_lock(lock_); @@ -67,17 +74,19 @@ void VideoRendererBase::Flush(const base::Closure& callback) { if (decrypting_demuxer_stream_) { decrypting_demuxer_stream_->Reset(base::Bind( - &VideoRendererBase::ResetDecoder, this)); + &VideoRendererBase::ResetDecoder, weak_this_)); return; } - decoder_->Reset(base::Bind(&VideoRendererBase::OnDecoderResetDone, this)); + decoder_->Reset(base::Bind( + &VideoRendererBase::OnDecoderResetDone, weak_this_)); } void VideoRendererBase::ResetDecoder() { DCHECK(message_loop_->BelongsToCurrentThread()); base::AutoLock auto_lock(lock_); - decoder_->Reset(base::Bind(&VideoRendererBase::OnDecoderResetDone, this)); + decoder_->Reset(base::Bind( + &VideoRendererBase::OnDecoderResetDone, weak_this_)); } void VideoRendererBase::Stop(const base::Closure& callback) { @@ -110,7 +119,7 @@ void VideoRendererBase::Stop(const base::Closure& callback) { if (decrypting_demuxer_stream_) { decrypting_demuxer_stream_->Reset(base::Bind( - &VideoRendererBase::StopDecoder, this, callback)); + &VideoRendererBase::StopDecoder, weak_this_, callback)); return; } @@ -168,6 +177,7 @@ void VideoRendererBase::Initialize(const scoped_refptr<DemuxerStream>& stream, DCHECK(!get_duration_cb.is_null()); DCHECK_EQ(kUninitialized, state_); + weak_this_ = weak_factory_.GetWeakPtr(); init_cb_ = init_cb; statistics_cb_ = statistics_cb; max_time_cb_ = max_time_cb; @@ -189,7 +199,7 @@ void VideoRendererBase::Initialize(const scoped_refptr<DemuxerStream>& stream, decoder_selector_ptr->SelectVideoDecoder( stream, statistics_cb, - base::Bind(&VideoRendererBase::OnDecoderSelected, this, + base::Bind(&VideoRendererBase::OnDecoderSelected, weak_this_, base::Passed(&decoder_selector))); } @@ -337,7 +347,7 @@ void VideoRendererBase::PaintNextReadyFrame_Locked() { paint_cb_.Run(next_frame); message_loop_->PostTask(FROM_HERE, base::Bind( - &VideoRendererBase::AttemptRead, this)); + &VideoRendererBase::AttemptRead, weak_this_)); } void VideoRendererBase::DropNextReadyFrame_Locked() { @@ -351,13 +361,7 @@ void VideoRendererBase::DropNextReadyFrame_Locked() { statistics_cb_.Run(statistics); message_loop_->PostTask(FROM_HERE, base::Bind( - &VideoRendererBase::AttemptRead, this)); -} - -VideoRendererBase::~VideoRendererBase() { - base::AutoLock auto_lock(lock_); - CHECK(state_ == kUninitialized || state_ == kStopped) << state_; - CHECK_EQ(thread_, base::kNullThreadHandle); + &VideoRendererBase::AttemptRead, weak_this_)); } void VideoRendererBase::FrameReady(VideoDecoder::Status status, @@ -504,7 +508,7 @@ void VideoRendererBase::AttemptRead_Locked() { case kPrerolling: case kPlaying: pending_read_ = true; - decoder_->Read(base::Bind(&VideoRendererBase::FrameReady, this)); + decoder_->Read(base::Bind(&VideoRendererBase::FrameReady, weak_this_)); return; case kUninitialized: diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h index ed0177b..1234ab8 100644 --- a/media/filters/video_renderer_base.h +++ b/media/filters/video_renderer_base.h @@ -8,6 +8,7 @@ #include <deque> #include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" #include "base/synchronization/condition_variable.h" #include "base/synchronization/lock.h" #include "base/threading/platform_thread.h" @@ -60,6 +61,7 @@ class MEDIA_EXPORT VideoRendererBase const PaintCB& paint_cb, const SetOpaqueCB& set_opaque_cb, bool drop_frames); + virtual ~VideoRendererBase(); // VideoRenderer implementation. virtual void Initialize(const scoped_refptr<DemuxerStream>& stream, @@ -83,9 +85,6 @@ class MEDIA_EXPORT VideoRendererBase // PlatformThread::Delegate implementation. virtual void ThreadMain() OVERRIDE; - protected: - virtual ~VideoRendererBase(); - private: // Called when |decoder_selector_| selected the |selected_decoder|. // |decrypting_demuxer_stream| was also populated if a DecryptingDemuxerStream @@ -158,6 +157,8 @@ class MEDIA_EXPORT VideoRendererBase PipelineStatus status); scoped_refptr<base::MessageLoopProxy> message_loop_; + base::WeakPtrFactory<VideoRendererBase> weak_factory_; + base::WeakPtr<VideoRendererBase> weak_this_; // Used for accessing data members. base::Lock lock_; diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc index 1049735..79b3bf9 100644 --- a/media/filters/video_renderer_base_unittest.cc +++ b/media/filters/video_renderer_base_unittest.cc @@ -44,12 +44,12 @@ class VideoRendererBaseTest : public ::testing::Test { demuxer_stream_(new MockDemuxerStream()), video_config_(kCodecVP8, VIDEO_CODEC_PROFILE_UNKNOWN, kVideoFormat, kCodedSize, kVisibleRect, kNaturalSize, NULL, 0, false) { - renderer_ = new VideoRendererBase( + renderer_.reset(new VideoRendererBase( message_loop_.message_loop_proxy(), media::SetDecryptorReadyCB(), base::Bind(&VideoRendererBaseTest::OnPaint, base::Unretained(this)), base::Bind(&VideoRendererBaseTest::OnSetOpaque, base::Unretained(this)), - true); + true)); EXPECT_CALL(*demuxer_stream_, type()) .WillRepeatedly(Return(DemuxerStream::VIDEO)); @@ -290,7 +290,7 @@ class VideoRendererBaseTest : public ::testing::Test { protected: // Fixture members. - scoped_refptr<VideoRendererBase> renderer_; + scoped_ptr<VideoRendererBase> renderer_; scoped_refptr<MockVideoDecoder> decoder_; scoped_refptr<MockDemuxerStream> demuxer_stream_; MockStatisticsCB statistics_cb_object_; diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc index aca065d..7128479 100644 --- a/media/tools/player_x11/player_x11.cc +++ b/media/tools/player_x11/player_x11.cc @@ -111,12 +111,13 @@ bool InitPipeline(const scoped_refptr<base::MessageLoopProxy>& message_loop, message_loop)); // Create our video renderer and save a reference to it for painting. - collection->AddVideoRenderer(new media::VideoRendererBase( + scoped_ptr<media::VideoRenderer> video_renderer(new media::VideoRendererBase( message_loop, media::SetDecryptorReadyCB(), base::Bind(&Paint, paint_message_loop, paint_cb), base::Bind(&SetOpaque), true)); + collection->SetVideoRenderer(video_renderer.Pass()); collection->AddAudioRenderer(new media::AudioRendererImpl( message_loop, |