diff options
author | acolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-13 16:32:19 +0000 |
---|---|---|
committer | acolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-13 16:32:19 +0000 |
commit | c802bc33b52d0332f1897461348781795d3dcb13 (patch) | |
tree | 65394499ef8c0631f54b2e8c6bd49339ecd659ed /media | |
parent | 98f24e9d671ea11419bae4ab13f5a18e3586ac49 (diff) | |
download | chromium_src-c802bc33b52d0332f1897461348781795d3dcb13.zip chromium_src-c802bc33b52d0332f1897461348781795d3dcb13.tar.gz chromium_src-c802bc33b52d0332f1897461348781795d3dcb13.tar.bz2 |
Convert Filter::Seek() to use new callback system.
BUG=82167
TEST=none. Existing tests still pass.
Review URL: http://codereview.chromium.org/6969026
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85279 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
33 files changed, 422 insertions, 221 deletions
diff --git a/media/base/composite_filter.cc b/media/base/composite_filter.cc index 537d262..c765978 100644 --- a/media/base/composite_filter.cc +++ b/media/base/composite_filter.cc @@ -4,6 +4,7 @@ #include "media/base/composite_filter.h" +#include "base/bind.h" #include "base/message_loop.h" #include "base/stl_util-inl.h" #include "media/base/callback.h" @@ -83,7 +84,7 @@ FilterHost* CompositeFilter::host() { void CompositeFilter::Play(FilterCallback* play_callback) { DCHECK_EQ(message_loop_, MessageLoop::current()); scoped_ptr<FilterCallback> callback(play_callback); - if (callback_.get()) { + if (IsOperationPending()) { SendErrorToHost(PIPELINE_ERROR_OPERATION_PENDING); callback->Run(); return; @@ -104,7 +105,7 @@ void CompositeFilter::Play(FilterCallback* play_callback) { void CompositeFilter::Pause(FilterCallback* pause_callback) { DCHECK_EQ(message_loop_, MessageLoop::current()); scoped_ptr<FilterCallback> callback(pause_callback); - if (callback_.get()) { + if (IsOperationPending()) { SendErrorToHost(PIPELINE_ERROR_OPERATION_PENDING); callback->Run(); return; @@ -125,7 +126,7 @@ void CompositeFilter::Pause(FilterCallback* pause_callback) { void CompositeFilter::Flush(FilterCallback* flush_callback) { DCHECK_EQ(message_loop_, MessageLoop::current()); scoped_ptr<FilterCallback> callback(flush_callback); - if (callback_.get()) { + if (IsOperationPending()) { SendErrorToHost(PIPELINE_ERROR_OPERATION_PENDING); callback->Run(); return; @@ -193,21 +194,19 @@ void CompositeFilter::SetPlaybackRate(float playback_rate) { } void CompositeFilter::Seek(base::TimeDelta time, - FilterCallback* seek_callback) { + const FilterStatusCB& seek_cb) { DCHECK_EQ(message_loop_, MessageLoop::current()); - scoped_ptr<FilterCallback> callback(seek_callback); - if (callback_.get()) { - SendErrorToHost(PIPELINE_ERROR_OPERATION_PENDING); - callback->Run(); + + if (IsOperationPending()) { + seek_cb.Run(PIPELINE_ERROR_OPERATION_PENDING); return; } else if (!host() || (state_ != kPaused && state_ != kCreated)) { - SendErrorToHost(PIPELINE_ERROR_INVALID_STATE); - callback->Run(); + seek_cb.Run(PIPELINE_ERROR_INVALID_STATE); return; } ChangeState(kSeekPending); - callback_.reset(callback.release()); + status_cb_ = seek_cb; pending_seek_time_ = time; StartSerialCallSequence(); } @@ -229,13 +228,12 @@ void CompositeFilter::ChangeState(State new_state) { void CompositeFilter::StartSerialCallSequence() { DCHECK_EQ(message_loop_, MessageLoop::current()); status_ = PIPELINE_OK; + sequence_index_ = 0; if (!filters_.empty()) { - sequence_index_ = 0; CallFilter(filters_[sequence_index_], NewThreadSafeCallback(&CompositeFilter::SerialCallback)); } else { - sequence_index_ = 0; SerialCallback(); } } @@ -243,15 +241,14 @@ void CompositeFilter::StartSerialCallSequence() { void CompositeFilter::StartParallelCallSequence() { DCHECK_EQ(message_loop_, MessageLoop::current()); status_ = PIPELINE_OK; + sequence_index_ = 0; if (!filters_.empty()) { - sequence_index_ = 0; for (size_t i = 0; i < filters_.size(); i++) { CallFilter(filters_[i], NewThreadSafeCallback(&CompositeFilter::ParallelCallback)); } } else { - sequence_index_ = 0; ParallelCallback(); } } @@ -272,17 +269,29 @@ void CompositeFilter::CallFilter(scoped_refptr<Filter>& filter, filter->Stop(callback); break; case kSeekPending: - filter->Seek(pending_seek_time_, callback); + filter->Seek(pending_seek_time_, + base::Bind(&CompositeFilter::OnStatusCB, this, callback)); break; default: delete callback; ChangeState(kError); - HandleError(PIPELINE_ERROR_INVALID_STATE); + DispatchPendingCallback(PIPELINE_ERROR_INVALID_STATE); } } -void CompositeFilter::DispatchPendingCallback() { +void CompositeFilter::DispatchPendingCallback(PipelineStatus status) { + DCHECK((status_cb_.is_null() && callback_.get()) || + (!status_cb_.is_null() && !callback_.get())); + + if (!status_cb_.is_null()) { + ResetAndRunCB(&status_cb_, status); + return; + } + if (callback_.get()) { + if (status != PIPELINE_OK) + SendErrorToHost(status); + scoped_ptr<FilterCallback> callback(callback_.release()); callback->Run(); } @@ -331,7 +340,7 @@ void CompositeFilter::SerialCallback() { if (status_ != PIPELINE_OK) { // We encountered an error. Terminate the sequence now. ChangeState(kError); - HandleError(status_); + DispatchPendingCallback(status_); return; } @@ -362,7 +371,7 @@ void CompositeFilter::ParallelCallback() { if (status_ != PIPELINE_OK) { // We encountered an error. ChangeState(kError); - HandleError(status_); + DispatchPendingCallback(status_); return; } @@ -376,7 +385,8 @@ void CompositeFilter::OnCallSequenceDone() { if (next_state == kInvalid) { // We somehow got into an unexpected state. ChangeState(kError); - HandleError(PIPELINE_ERROR_INVALID_STATE); + DispatchPendingCallback(PIPELINE_ERROR_INVALID_STATE); + return; } ChangeState(next_state); @@ -386,7 +396,7 @@ void CompositeFilter::OnCallSequenceDone() { StartSerialCallSequence(); } else { // Call the callback to indicate that the operation has completed. - DispatchPendingCallback(); + DispatchPendingCallback(PIPELINE_OK); } } @@ -395,12 +405,6 @@ void CompositeFilter::SendErrorToHost(PipelineStatus error) { host_impl_.get()->host()->SetError(error); } -void CompositeFilter::HandleError(PipelineStatus error) { - DCHECK_NE(error, PIPELINE_OK); - SendErrorToHost(error); - DispatchPendingCallback(); -} - FilterCallback* CompositeFilter::NewThreadSafeCallback( void (CompositeFilter::*method)()) { return TaskToCallbackAdapter::NewCallback( @@ -432,6 +436,21 @@ bool CompositeFilter::CanForwardError() { return (state_ == kCreated) || (state_ == kPlaying) || (state_ == kPaused); } +bool CompositeFilter::IsOperationPending() const { + DCHECK(!(callback_.get() && !status_cb_.is_null())); + + return callback_.get() || !status_cb_.is_null(); +} + +void CompositeFilter::OnStatusCB(FilterCallback* callback, + PipelineStatus status) { + if (status != PIPELINE_OK) + SetError(status); + + callback->Run(); + delete callback; +} + void CompositeFilter::SetError(PipelineStatus error) { // TODO(acolwell): Temporary hack to handle errors that occur // during filter initialization. In this case we just forward diff --git a/media/base/composite_filter.h b/media/base/composite_filter.h index a2c8c09..82a0791 100644 --- a/media/base/composite_filter.h +++ b/media/base/composite_filter.h @@ -32,7 +32,7 @@ class CompositeFilter : public Filter { virtual void Flush(FilterCallback* flush_callback); virtual void Stop(FilterCallback* stop_callback); virtual void SetPlaybackRate(float playback_rate); - virtual void Seek(base::TimeDelta time, FilterCallback* seek_callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& seek_cb); virtual void OnAudioRendererDisabled(); protected: @@ -74,7 +74,7 @@ class CompositeFilter : public Filter { void CallFilter(scoped_refptr<Filter>& filter, FilterCallback* callback); // Calls |callback_| and then clears the reference. - void DispatchPendingCallback(); + void DispatchPendingCallback(PipelineStatus status); // Gets the state to transition to given |state|. State GetNextState(State state) const; @@ -91,9 +91,6 @@ class CompositeFilter : public Filter { // Helper function for sending an error to the FilterHost. void SendErrorToHost(PipelineStatus error); - // Helper function for handling errors during call sequences. - void HandleError(PipelineStatus error); - // Creates a callback that can be called from any thread, but is guaranteed // to call the specified method on the thread associated with this filter. FilterCallback* NewThreadSafeCallback(void (CompositeFilter::*method)()); @@ -107,12 +104,23 @@ class CompositeFilter : public Filter { // to the host of this filter. bool CanForwardError(); + // Checks to see if a Play(), Pause(), Flush(), Stop(), or Seek() operation is + // pending. + bool IsOperationPending() const; + + // Called by operations that take a FilterStatusCB instead of a + // FilterCallback. + // TODO: Remove when FilterCallback goes away. + void OnStatusCB(FilterCallback* callback, PipelineStatus status); + // Vector of the filters added to the composite. typedef std::vector<scoped_refptr<Filter> > FilterVector; FilterVector filters_; // Callback for the pending request. + // TODO: Remove callback_ when FilterCallback is removed. scoped_ptr<FilterCallback> callback_; + FilterStatusCB status_cb_; // Time parameter for the pending Seek() request. base::TimeDelta pending_seek_time_; diff --git a/media/base/composite_filter_unittest.cc b/media/base/composite_filter_unittest.cc index 0be2185..bb43af0 100644 --- a/media/base/composite_filter_unittest.cc +++ b/media/base/composite_filter_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/bind.h" #include "base/message_loop.h" #include "media/base/composite_filter.h" #include "media/base/mock_callback.h" @@ -53,9 +54,15 @@ class CompositeFilterTest : public testing::Test { // |seek_time| - The time to pass to the Seek() call if |method_to_call| // equals SEEK. // |callback| - The callback object to pass to the method. + // |expected_status| - Some filter methods use a FilterStatusCB instead of + // a FilterCallback. For these methods this function + // creates a FilterStatusCB that makes sure the status + // passed to the callback matches |expected_status| and + // then calls |callback|. void DoFilterCall(MethodToCall method_to_call, Filter* filter, base::TimeDelta seek_time, - FilterCallback* callback); + FilterCallback* callback, + PipelineStatus expected_status); // Creates an expectation sequence based on the value of method_to_call. // @@ -82,10 +89,16 @@ class CompositeFilterTest : public testing::Test { void ExpectInvalidStateFail(MethodToCall method_to_call, base::TimeDelta seek_time = base::TimeDelta()); - // Run the callback stored in |filter_1_callback_|. + // Returns whether |filter_1_callback_| or |filter_1_status_cb_| is set. + bool HasFilter1Callback() const; + + // Run the callback stored in |filter_1_callback_| or |filter_2_status_cb_|. void RunFilter1Callback(); - // Run the callback stored in |filter_2_callback_|. + // Returns whether |filter_2_callback_| or |filter_2_status_cb_| is set. + bool HasFilter2Callback() const; + + // Run the callback stored in |filter_2_callback_| or |filter_2_status_cb_|. void RunFilter2Callback(); protected: @@ -98,9 +111,15 @@ class CompositeFilterTest : public testing::Test { scoped_refptr<StrictMock<MockFilter> > filter_1_; // Callback passed to |filter_1_| during last Play(), Pause(), Flush(), - // Stop(), or Seek() call. + // or Stop() call. FilterCallback* filter_1_callback_; + // Status to pass to |filter_1_status_cb_|. + PipelineStatus filter_1_status_; + + // Callback passed to |filter_1_| during last Seek() call. + FilterStatusCB filter_1_status_cb_; + // Second filter added to the composite. scoped_refptr<StrictMock<MockFilter> > filter_2_; @@ -108,6 +127,12 @@ class CompositeFilterTest : public testing::Test { // Stop(), or Seek() call. FilterCallback* filter_2_callback_; + // Status to pass to |filter_2_status_cb_|. + PipelineStatus filter_2_status_; + + // Callback passed to |filter_2_| during last Seek() call. + FilterStatusCB filter_2_status_cb_; + // FilterHost implementation passed to |composite_| via set_host(). scoped_ptr<StrictMock<MockFilterHost> > mock_filter_host_; @@ -117,7 +142,9 @@ class CompositeFilterTest : public testing::Test { CompositeFilterTest::CompositeFilterTest() : composite_(new CompositeFilter(&message_loop_)), filter_1_callback_(NULL), + filter_1_status_(PIPELINE_OK), filter_2_callback_(NULL), + filter_2_status_(PIPELINE_OK), mock_filter_host_(new StrictMock<MockFilterHost>()) { } @@ -132,6 +159,8 @@ void CompositeFilterTest::SetupAndAdd2Filters() { // |filter_1_callback_| when they are called. filter_1_ = new StrictMock<MockFilter>(); filter_1_callback_ = NULL; + filter_1_status_ = PIPELINE_OK; + filter_1_status_cb_.Reset(); ON_CALL(*filter_1_, Play(_)) .WillByDefault(SaveArg<0>(&filter_1_callback_)); ON_CALL(*filter_1_, Pause(_)) @@ -141,12 +170,14 @@ void CompositeFilterTest::SetupAndAdd2Filters() { ON_CALL(*filter_1_, Stop(_)) .WillByDefault(SaveArg<0>(&filter_1_callback_)); ON_CALL(*filter_1_, Seek(_,_)) - .WillByDefault(SaveArg<1>(&filter_1_callback_)); + .WillByDefault(SaveArg<1>(&filter_1_status_cb_)); // Setup |filter_2_| and arrange for methods to set // |filter_2_callback_| when they are called. filter_2_ = new StrictMock<MockFilter>(); filter_2_callback_ = NULL; + filter_2_status_ = PIPELINE_OK; + filter_2_status_cb_.Reset(); ON_CALL(*filter_2_, Play(_)) .WillByDefault(SaveArg<0>(&filter_2_callback_)); ON_CALL(*filter_2_, Pause(_)) @@ -156,7 +187,7 @@ void CompositeFilterTest::SetupAndAdd2Filters() { ON_CALL(*filter_2_, Stop(_)) .WillByDefault(SaveArg<0>(&filter_2_callback_)); ON_CALL(*filter_2_, Seek(_,_)) - .WillByDefault(SaveArg<1>(&filter_2_callback_)); + .WillByDefault(SaveArg<1>(&filter_2_status_cb_)); composite_->AddFilter(filter_1_); composite_->AddFilter(filter_2_); @@ -184,10 +215,22 @@ void CompositeFilterTest::ExpectFilterCall(MethodToCall method_to_call, }; } +void OnStatusCB(PipelineStatus expected_status, FilterCallback* callback, + PipelineStatus status) { + EXPECT_EQ(status, expected_status); + + callback->Run(); + delete callback; +} + void CompositeFilterTest::DoFilterCall(MethodToCall method_to_call, Filter* filter, base::TimeDelta seek_time, - FilterCallback* callback) { + FilterCallback* callback, + PipelineStatus expected_status) { + filter_1_status_ = expected_status; + filter_2_status_ = expected_status; + switch(method_to_call) { case PLAY: filter->Play(callback); @@ -202,7 +245,8 @@ void CompositeFilterTest::DoFilterCall(MethodToCall method_to_call, filter->Stop(callback); break; case SEEK: - filter->Seek(seek_time, callback); + filter->Seek(seek_time, base::Bind(&OnStatusCB, expected_status, + callback)); break; }; } @@ -221,25 +265,26 @@ void CompositeFilterTest::ExpectSuccess(MethodToCall method_to_call, // Make method call on the composite. StrictMock<MockCallback>* callback = new StrictMock<MockCallback>(); - DoFilterCall(method_to_call, composite_.get(), seek_time, callback); + DoFilterCall(method_to_call, composite_.get(), seek_time, callback, + PIPELINE_OK); if (is_parallel_call) { // Make sure both filters have their callbacks set. - EXPECT_TRUE(filter_1_callback_ != NULL); - EXPECT_TRUE(filter_2_callback_ != NULL); + EXPECT_TRUE(HasFilter1Callback()); + EXPECT_TRUE(HasFilter2Callback()); RunFilter1Callback(); } else { // Make sure that only |filter_1_| has its callback set. - EXPECT_TRUE(filter_1_callback_ != NULL); - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_TRUE(HasFilter1Callback()); + EXPECT_FALSE(HasFilter2Callback()); ExpectFilterCall(method_to_call, filter_2_.get(), seek_time); RunFilter1Callback(); // Verify that |filter_2_| was called by checking the callback pointer. - EXPECT_TRUE(filter_2_callback_ != NULL); + EXPECT_TRUE(HasFilter2Callback()); } callback->ExpectRunAndDelete(); @@ -271,18 +316,34 @@ void CompositeFilterTest::ExpectInvalidStateFail(MethodToCall method_to_call, base::TimeDelta seek_time) { InSequence seq; - EXPECT_CALL(*mock_filter_host_, SetError(PIPELINE_ERROR_INVALID_STATE)) - .WillOnce(Return()); + if (method_to_call != SEEK) { + EXPECT_CALL(*mock_filter_host_, SetError(PIPELINE_ERROR_INVALID_STATE)) + .WillOnce(Return()); + } - DoFilterCall(method_to_call, composite_, seek_time, NewExpectedCallback()); + DoFilterCall(method_to_call, composite_, seek_time, NewExpectedCallback(), + PIPELINE_ERROR_INVALID_STATE); // Make sure that neither of the filters were called by // verifying that the callback pointers weren't set. - EXPECT_EQ((FilterCallback*)NULL, filter_1_callback_); - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_FALSE(HasFilter1Callback()); + EXPECT_FALSE(HasFilter2Callback()); +} + +bool CompositeFilterTest::HasFilter1Callback() const { + CHECK(!(filter_1_callback_ && !filter_1_status_cb_.is_null())); + return filter_1_callback_ != NULL || !filter_1_status_cb_.is_null(); } void CompositeFilterTest::RunFilter1Callback() { + EXPECT_TRUE(HasFilter1Callback()); + + if (!filter_1_status_cb_.is_null()) { + ResetAndRunCB(&filter_1_status_cb_, filter_1_status_); + filter_1_status_ = PIPELINE_OK; + return; + } + EXPECT_TRUE(filter_1_callback_ != NULL); FilterCallback* callback = filter_1_callback_; filter_1_callback_ = NULL; @@ -290,7 +351,20 @@ void CompositeFilterTest::RunFilter1Callback() { delete callback; } +bool CompositeFilterTest::HasFilter2Callback() const { + CHECK(!(filter_2_callback_ && !filter_2_status_cb_.is_null())); + return filter_2_callback_ != NULL || !filter_2_status_cb_.is_null(); +} + void CompositeFilterTest::RunFilter2Callback() { + EXPECT_TRUE(HasFilter2Callback()); + + if (!filter_2_status_cb_.is_null()) { + ResetAndRunCB(&filter_2_status_cb_, filter_2_status_); + filter_2_status_ = PIPELINE_OK; + return; + } + EXPECT_TRUE(filter_2_callback_ != NULL); FilterCallback* callback = filter_2_callback_; filter_2_callback_ = NULL; @@ -339,8 +413,8 @@ TEST_F(CompositeFilterTest, TestPlay) { composite_->Play(NewExpectedCallback()); // Verify that neither of the filter callbacks were set. - EXPECT_EQ((FilterCallback*)NULL, filter_1_callback_); - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_FALSE(HasFilter1Callback()); + EXPECT_FALSE(HasFilter2Callback()); // Stop playback. DoStop(); @@ -351,8 +425,8 @@ TEST_F(CompositeFilterTest, TestPlay) { composite_->Stop(NewExpectedCallback()); // Verify that neither of the filter callbacks were set. - EXPECT_EQ((FilterCallback*)NULL, filter_1_callback_); - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_FALSE(HasFilter1Callback()); + EXPECT_FALSE(HasFilter2Callback()); // Try calling Play() again to make sure we get an error. ExpectInvalidStateFail(PLAY); @@ -418,8 +492,8 @@ TEST_F(CompositeFilterTest, TestPause) { composite_->Pause(NewExpectedCallback()); // Verify that neither of the filter callbacks were set. - EXPECT_EQ((FilterCallback*)NULL, filter_1_callback_); - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_FALSE(HasFilter1Callback()); + EXPECT_FALSE(HasFilter2Callback()); // Verify that we can transition pack to the play state. DoPlay(); @@ -460,7 +534,7 @@ TEST_F(CompositeFilterTest, TestPauseErrors) { RunFilter1Callback(); // Make sure |filter_2_callback_| was not set. - EXPECT_EQ((FilterCallback*)NULL, filter_2_callback_); + EXPECT_FALSE(HasFilter2Callback()); // Verify that Play/Pause/Flush/Seek fail now that an error occured. ExpectInvalidStateFail(PLAY); @@ -714,7 +788,7 @@ TEST_F(CompositeFilterTest, TestEmptyComposite) { // Issue a Seek() and expect no errors. composite_->Seek(base::TimeDelta::FromSeconds(5), - NewExpectedCallback()); + NewExpectedStatusCB(PIPELINE_OK)); // Issue a Play() and expect no errors. composite_->Play(NewExpectedCallback()); diff --git a/media/base/filters.cc b/media/base/filters.cc index 8b2e24a..c8c0c76 100644 --- a/media/base/filters.cc +++ b/media/base/filters.cc @@ -8,6 +8,13 @@ namespace media { +void ResetAndRunCB(FilterStatusCB* cb, PipelineStatus status) { + DCHECK(cb); + FilterStatusCB tmp_cb(*cb); + cb->Reset(); + tmp_cb.Run(status); +} + Filter::Filter() : host_(NULL) {} Filter::~Filter() {} @@ -24,43 +31,33 @@ FilterHost* Filter::host() { void Filter::Play(FilterCallback* callback) { DCHECK(callback); - if (callback) { - callback->Run(); - delete callback; - } + callback->Run(); + delete callback; } void Filter::Pause(FilterCallback* callback) { DCHECK(callback); - if (callback) { - callback->Run(); - delete callback; - } + callback->Run(); + delete callback; } void Filter::Flush(FilterCallback* callback) { DCHECK(callback); - if (callback) { - callback->Run(); - delete callback; - } + callback->Run(); + delete callback; } void Filter::Stop(FilterCallback* callback) { DCHECK(callback); - if (callback) { - callback->Run(); - delete callback; - } + callback->Run(); + delete callback; } void Filter::SetPlaybackRate(float playback_rate) {} -void Filter::Seek(base::TimeDelta time, FilterCallback* callback) { - scoped_ptr<FilterCallback> seek_callback(callback); - if (seek_callback.get()) { - seek_callback->Run(); - } +void Filter::Seek(base::TimeDelta time, const FilterStatusCB& callback) { + DCHECK(!callback.is_null()); + callback.Run(PIPELINE_OK); } void Filter::OnAudioRendererDisabled() { diff --git a/media/base/filters.h b/media/base/filters.h index 901a91b..cceed67 100644 --- a/media/base/filters.h +++ b/media/base/filters.h @@ -33,6 +33,7 @@ #include "base/time.h" #include "media/base/audio_decoder_config.h" #include "media/base/media_format.h" +#include "media/base/pipeline_status.h" #include "media/base/video_frame.h" struct AVStream; @@ -62,6 +63,12 @@ enum Preload { // Used for completing asynchronous methods. typedef Callback0::Type FilterCallback; +typedef base::Callback<void(PipelineStatus)> FilterStatusCB; + +// This function copies |cb|, calls Reset() on |cb|, and then calls Run() +// on the copy. This is used in the common case where you need to clear +// a callback member variable before running the callback. +void ResetAndRunCB(FilterStatusCB* cb, PipelineStatus status); // Used for updating pipeline statistics. typedef Callback1<const PipelineStatistics&>::Type StatisticsCallback; @@ -104,7 +111,7 @@ class Filter : public base::RefCountedThreadSafe<Filter> { // Carry out any actions required to seek to the given time, executing the // callback upon completion. - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& callback); // This method is called from the pipeline when the audio renderer // is disabled. Filters can ignore the notification if they do not diff --git a/media/base/mock_callback.cc b/media/base/mock_callback.cc index eeedd15..cc4f141 100644 --- a/media/base/mock_callback.cc +++ b/media/base/mock_callback.cc @@ -4,6 +4,8 @@ #include "media/base/mock_callback.h" +#include "base/bind.h" + using ::testing::_; using ::testing::StrictMock; @@ -51,4 +53,22 @@ MockStatusCallback* NewExpectedStatusCallback(PipelineStatus status) { return callback; } +class MockStatusCB : public base::RefCountedThreadSafe<MockStatusCB> { + public: + MockStatusCB() {} + virtual ~MockStatusCB() {} + + MOCK_METHOD1(Run, void(PipelineStatus)); + + private: + DISALLOW_COPY_AND_ASSIGN(MockStatusCB); +}; + +base::Callback<void(PipelineStatus)> NewExpectedStatusCB( + PipelineStatus status) { + StrictMock<MockStatusCB>* callback = new StrictMock<MockStatusCB>(); + EXPECT_CALL(*callback, Run(status)); + return base::Bind(&MockStatusCB::Run, callback); +} + } // namespace media diff --git a/media/base/mock_callback.h b/media/base/mock_callback.h index d142a9b..3b26df9 100644 --- a/media/base/mock_callback.h +++ b/media/base/mock_callback.h @@ -73,6 +73,7 @@ class MockStatusCallback : public CallbackRunner<Tuple1<PipelineStatus> > { // the callback to run. MockCallback* NewExpectedCallback(); MockStatusCallback* NewExpectedStatusCallback(PipelineStatus status); +base::Callback<void(PipelineStatus)> NewExpectedStatusCB(PipelineStatus status); } // namespace media diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc index da601c4..6fe84c3 100644 --- a/media/base/mock_filters.cc +++ b/media/base/mock_filters.cc @@ -169,6 +169,10 @@ void RunFilterCallback(::testing::Unused, FilterCallback* callback) { delete callback; } +void RunFilterStatusCB(::testing::Unused, const FilterStatusCB& cb) { + cb.Run(PIPELINE_OK); +} + void RunPipelineStatusCallback( PipelineStatus status, PipelineStatusCallback* callback) { callback->Run(status); diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index 1564437..7df9280 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -54,7 +54,7 @@ class MockFilter : public Filter { MOCK_METHOD1(Flush, void(FilterCallback* callback)); MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); protected: @@ -73,7 +73,7 @@ class MockDataSource : public DataSource { MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // DataSource implementation. @@ -106,7 +106,7 @@ class MockDemuxer : public Demuxer { MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); MOCK_METHOD1(SetPreload, void(Preload preload)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // Demuxer implementation. @@ -177,7 +177,7 @@ class MockVideoDecoder : public VideoDecoder { // Filter implementation. MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // VideoDecoder implementation. @@ -206,7 +206,7 @@ class MockAudioDecoder : public AudioDecoder { // Filter implementation. MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // AudioDecoder implementation. @@ -234,7 +234,7 @@ class MockVideoRenderer : public VideoRenderer { // Filter implementation. MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // VideoRenderer implementation. @@ -261,7 +261,7 @@ class MockAudioRenderer : public AudioRenderer { // Filter implementation. MOCK_METHOD1(Stop, void(FilterCallback* callback)); MOCK_METHOD1(SetPlaybackRate, void(float playback_rate)); - MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback)); + MOCK_METHOD2(Seek, void(base::TimeDelta time, const FilterStatusCB& cb)); MOCK_METHOD0(OnAudioRendererDisabled, void()); // AudioRenderer implementation. @@ -314,6 +314,7 @@ class MockFilterCollection { // FilterCallback on behalf of the provided filter. Can be used when mocking // the Initialize() and Seek() methods. void RunFilterCallback(::testing::Unused, FilterCallback* callback); +void RunFilterStatusCB(::testing::Unused, const FilterStatusCB& cb); void RunPipelineStatusCallback(PipelineStatus status, PipelineStatusCallback* callback); void RunFilterCallback3(::testing::Unused, FilterCallback* callback, diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc index d7ad574..2bd4f31 100644 --- a/media/base/pipeline_impl.cc +++ b/media/base/pipeline_impl.cc @@ -9,6 +9,7 @@ #include <algorithm> +#include "base/bind.h" #include "base/callback.h" #include "base/compiler_specific.h" #include "base/stl_util-inl.h" @@ -575,6 +576,19 @@ void PipelineImpl::OnFilterStateTransition() { NewRunnableMethod(this, &PipelineImpl::FilterStateTransitionTask)); } +// Called from any thread. +// This method makes the FilterStatusCB behave like a FilterCallback. It +// makes it look like a host()->SetError() call followed by a call to +// OnFilterStateTransition() when errors occur. +// +// TODO: Revisit this code when SetError() is removed from FilterHost and +// all the FilterCallbacks are converted to FilterStatusCB. +void PipelineImpl::OnFilterStateTransitionWithStatus(PipelineStatus status) { + if (status != PIPELINE_OK) + SetError(status); + OnFilterStateTransition(); +} + void PipelineImpl::OnTeardownStateTransition() { message_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, &PipelineImpl::TeardownStateTransitionTask)); @@ -710,8 +724,9 @@ void PipelineImpl::InitializeTask() { seek_pending_ = true; set_state(kSeeking); seek_timestamp_ = base::TimeDelta(); - pipeline_filter_->Seek(seek_timestamp_, - NewCallback(this, &PipelineImpl::OnFilterStateTransition)); + pipeline_filter_->Seek( + seek_timestamp_, + base::Bind(&PipelineImpl::OnFilterStateTransitionWithStatus, this)); } } @@ -952,7 +967,7 @@ void PipelineImpl::FilterStateTransitionTask() { NewCallback(this, &PipelineImpl::OnFilterStateTransition)); } else if (state_ == kSeeking) { pipeline_filter_->Seek(seek_timestamp_, - NewCallback(this, &PipelineImpl::OnFilterStateTransition)); + base::Bind(&PipelineImpl::OnFilterStateTransitionWithStatus, this)); } else if (state_ == kStarting) { pipeline_filter_->Play( NewCallback(this, &PipelineImpl::OnFilterStateTransition)); diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h index 7b5fe4a..caf4e19f 100644 --- a/media/base/pipeline_impl.h +++ b/media/base/pipeline_impl.h @@ -205,10 +205,12 @@ class PipelineImpl : public Pipeline, public FilterHost { // Callback executed by filters upon completing initialization. void OnFilterInitialize(); - // Callback executed by filters upon completing Play(), Pause(), Seek(), - // or Stop(). + // Callback executed by filters upon completing Play(), Pause(), or Stop(). void OnFilterStateTransition(); + // Callback executed by filters upon completing Seek(). + void OnFilterStateTransitionWithStatus(PipelineStatus status); + // Callback executed by filters when completing teardown operations. void OnTeardownStateTransition(); diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc index 2028e7a..af68cd6 100644 --- a/media/base/pipeline_impl_unittest.cc +++ b/media/base/pipeline_impl_unittest.cc @@ -101,8 +101,8 @@ class PipelineImplTest : public ::testing::Test { kTotalBytes, kBufferedBytes, duration); EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f)); EXPECT_CALL(*mocks_->demuxer(), SetPreload(AUTO)); - EXPECT_CALL(*mocks_->demuxer(), Seek(base::TimeDelta(), NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->demuxer(), Seek(base::TimeDelta(), _)) + .WillOnce(Invoke(&RunFilterStatusCB)); EXPECT_CALL(*mocks_->demuxer(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); @@ -128,8 +128,8 @@ class PipelineImplTest : public ::testing::Test { Initialize(stream, NotNull(), NotNull())) .WillOnce(DoAll(Invoke(&RunFilterCallback3), DeleteArg<2>())); EXPECT_CALL(*mocks_->video_decoder(), SetPlaybackRate(0.0f)); - EXPECT_CALL(*mocks_->video_decoder(), Seek(base::TimeDelta(), NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->video_decoder(), Seek(base::TimeDelta(), _)) + .WillOnce(Invoke(&RunFilterStatusCB)); EXPECT_CALL(*mocks_->video_decoder(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); } @@ -140,8 +140,8 @@ class PipelineImplTest : public ::testing::Test { Initialize(stream, NotNull(), NotNull())) .WillOnce(DoAll(Invoke(&RunFilterCallback3), DeleteArg<2>())); EXPECT_CALL(*mocks_->audio_decoder(), SetPlaybackRate(0.0f)); - EXPECT_CALL(*mocks_->audio_decoder(), Seek(base::TimeDelta(), NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->audio_decoder(), Seek(base::TimeDelta(), _)) + .WillOnce(Invoke(&RunFilterStatusCB)); EXPECT_CALL(*mocks_->audio_decoder(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); } @@ -152,8 +152,8 @@ class PipelineImplTest : public ::testing::Test { Initialize(mocks_->video_decoder(), NotNull(), NotNull())) .WillOnce(DoAll(Invoke(&RunFilterCallback3), DeleteArg<2>())); EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f)); - EXPECT_CALL(*mocks_->video_renderer(), Seek(base::TimeDelta(), NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->video_renderer(), Seek(base::TimeDelta(), _)) + .WillOnce(Invoke(&RunFilterStatusCB)); EXPECT_CALL(*mocks_->video_renderer(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); } @@ -172,8 +172,8 @@ class PipelineImplTest : public ::testing::Test { } EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f)); EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f)); - EXPECT_CALL(*mocks_->audio_renderer(), Seek(base::TimeDelta(), NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->audio_renderer(), Seek(base::TimeDelta(), _)) + .WillOnce(Invoke(&RunFilterStatusCB)); EXPECT_CALL(*mocks_->audio_renderer(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); } @@ -233,21 +233,21 @@ class PipelineImplTest : public ::testing::Test { void ExpectSeek(const base::TimeDelta& seek_time) { // Every filter should receive a call to Seek(). - EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _)) + .WillOnce(Invoke(&RunFilterStatusCB)); if (audio_stream_) { - EXPECT_CALL(*mocks_->audio_decoder(), Seek(seek_time, NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); - EXPECT_CALL(*mocks_->audio_renderer(), Seek(seek_time, NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->audio_decoder(), Seek(seek_time, _)) + .WillOnce(Invoke(&RunFilterStatusCB)); + EXPECT_CALL(*mocks_->audio_renderer(), Seek(seek_time, _)) + .WillOnce(Invoke(&RunFilterStatusCB)); } if (video_stream_) { - EXPECT_CALL(*mocks_->video_decoder(), Seek(seek_time, NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); - EXPECT_CALL(*mocks_->video_renderer(), Seek(seek_time, NotNull())) - .WillOnce(Invoke(&RunFilterCallback)); + EXPECT_CALL(*mocks_->video_decoder(), Seek(seek_time, _)) + .WillOnce(Invoke(&RunFilterStatusCB)); + EXPECT_CALL(*mocks_->video_renderer(), Seek(seek_time, _)) + .WillOnce(Invoke(&RunFilterStatusCB)); } // We expect a successful seek callback. @@ -766,6 +766,10 @@ TEST_F(PipelineImplTest, AudioStreamShorterThanVideo) { host->NotifyEnded(); } +void SendReadErrorToCB(::testing::Unused, const FilterStatusCB& cb) { + cb.Run(PIPELINE_ERROR_READ); +} + TEST_F(PipelineImplTest, ErrorDuringSeek) { CreateAudioStream(); MockDemuxerStreamVector streams; @@ -787,10 +791,8 @@ TEST_F(PipelineImplTest, ErrorDuringSeek) { base::TimeDelta seek_time = base::TimeDelta::FromSeconds(5); - EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, NotNull())) - .WillOnce(DoAll(SetError(mocks_->demuxer(), - PIPELINE_ERROR_READ), - Invoke(&RunFilterCallback))); + EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _)) + .WillOnce(Invoke(&SendReadErrorToCB)); pipeline_->Seek(seek_time, NewCallback( reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnSeek)); diff --git a/media/filters/adaptive_demuxer.cc b/media/filters/adaptive_demuxer.cc index 9a94d11..5599b15 100644 --- a/media/filters/adaptive_demuxer.cc +++ b/media/filters/adaptive_demuxer.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/bind.h" #include "base/logging.h" #include "base/string_number_conversions.h" #include "base/string_split.h" @@ -160,6 +161,8 @@ Demuxer* AdaptiveDemuxer::current_demuxer(DemuxerStream::Type type) { // Helper class that wraps a FilterCallback and expects to get called a set // number of times, after which the wrapped callback is fired (and deleted). +// +// TODO: Remove this class once Stop() is converted to FilterStatusCB. class CountingCallback { public: CountingCallback(int count, FilterCallback* orig_cb) @@ -191,21 +194,69 @@ class CountingCallback { scoped_ptr<FilterCallback> orig_cb_; }; +// Helper class that wraps FilterStatusCB and expects to get called a set +// number of times, after which the wrapped callback is fired. If an error +// is reported in any of the callbacks, only the first error code is passed +// to the wrapped callback. +class CountingStatusCB : public base::RefCountedThreadSafe<CountingStatusCB> { + public: + CountingStatusCB(int count, const FilterStatusCB& orig_cb) + : remaining_count_(count), orig_cb_(orig_cb), + overall_status_(PIPELINE_OK) { + DCHECK_GT(remaining_count_, 0); + DCHECK(!orig_cb.is_null()); + } + + FilterStatusCB GetACallback() { + return base::Bind(&CountingStatusCB::OnChildCallbackDone, this); + } + + private: + void OnChildCallbackDone(PipelineStatus status) { + bool fire_orig_cb = false; + PipelineStatus overall_status = PIPELINE_OK; + + { + base::AutoLock auto_lock(lock_); + + if (overall_status_ == PIPELINE_OK && status != PIPELINE_OK) + overall_status_ = status; + + if (--remaining_count_ == 0) { + fire_orig_cb = true; + overall_status = overall_status_; + } + } + + if (fire_orig_cb) + orig_cb_.Run(overall_status); + } + + base::Lock lock_; + int remaining_count_; + FilterStatusCB orig_cb_; + PipelineStatus overall_status_; + + DISALLOW_COPY_AND_ASSIGN(CountingStatusCB); +}; + void AdaptiveDemuxer::Stop(FilterCallback* callback) { // Stop() must be called on all of the demuxers even though only one demuxer // is actively delivering audio and another one is delivering video. This // just satisfies the contract that all demuxers must have Stop() called on // them before they are destroyed. + // + // TODO: Remove CountingCallback once Stop() is converted to FilterStatusCB. CountingCallback* wrapper = new CountingCallback(demuxers_.size(), callback); for (size_t i = 0; i < demuxers_.size(); ++i) demuxers_[i]->Stop(wrapper->GetACallback()); } -void AdaptiveDemuxer::Seek(base::TimeDelta time, FilterCallback* callback) { +void AdaptiveDemuxer::Seek(base::TimeDelta time, const FilterStatusCB& cb) { Demuxer* audio = current_demuxer(DemuxerStream::AUDIO); Demuxer* video = current_demuxer(DemuxerStream::VIDEO); int count = (audio ? 1 : 0) + (video && audio != video ? 1 : 0); - CountingCallback* wrapper = new CountingCallback(count, callback); + CountingStatusCB* wrapper = new CountingStatusCB(count, cb); if (audio) audio->Seek(time, wrapper->GetACallback()); if (video && audio != video) diff --git a/media/filters/adaptive_demuxer.h b/media/filters/adaptive_demuxer.h index b4836bb..6402b03 100644 --- a/media/filters/adaptive_demuxer.h +++ b/media/filters/adaptive_demuxer.h @@ -79,7 +79,7 @@ class AdaptiveDemuxer : public Demuxer { // Filter implementation. virtual void Stop(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); virtual void OnAudioRendererDisabled(); virtual void set_host(FilterHost* filter_host); virtual void SetPlaybackRate(float playback_rate); diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc index 45dd699..a98b8a2 100644 --- a/media/filters/audio_renderer_base.cc +++ b/media/filters/audio_renderer_base.cc @@ -64,12 +64,13 @@ void AudioRendererBase::Stop(FilterCallback* callback) { } } -void AudioRendererBase::Seek(base::TimeDelta time, FilterCallback* callback) { +void AudioRendererBase::Seek(base::TimeDelta time, const FilterStatusCB& cb) { base::AutoLock auto_lock(lock_); DCHECK_EQ(kPaused, state_); DCHECK_EQ(0u, pending_reads_) << "Pending reads should have completed"; + DCHECK(seek_cb_.is_null()); state_ = kSeeking; - seek_callback_.reset(callback); + seek_cb_ = cb; seek_timestamp_ = time; // Throw away everything and schedule our reads. @@ -159,13 +160,12 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) { // Check for our preroll complete condition. if (state_ == kSeeking) { - DCHECK(seek_callback_.get()); + DCHECK(!seek_cb_.is_null()); if (algorithm_->IsQueueFull() || recieved_end_of_stream_) { // Transition into paused whether we have data in |algorithm_| or not. // FillBuffer() will play silence if there's nothing to fill. state_ = kPaused; - seek_callback_->Run(); - seek_callback_.reset(); + ResetAndRunCB(&seek_cb_, PIPELINE_OK); } } else if (state_ == kPaused && pending_reads_ == 0) { // No more pending reads! We're now officially "paused". diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h index 0b13e02a..1882743 100644 --- a/media/filters/audio_renderer_base.h +++ b/media/filters/audio_renderer_base.h @@ -37,7 +37,7 @@ class AudioRendererBase : public AudioRenderer { virtual void Pause(FilterCallback* callback); virtual void Stop(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); // AudioRenderer implementation. virtual void Initialize(AudioDecoder* decoder, FilterCallback* callback); @@ -125,7 +125,7 @@ class AudioRendererBase : public AudioRenderer { // Filter callbacks. scoped_ptr<FilterCallback> pause_callback_; - scoped_ptr<FilterCallback> seek_callback_; + FilterStatusCB seek_cb_; base::TimeDelta seek_timestamp_; diff --git a/media/filters/audio_renderer_base_unittest.cc b/media/filters/audio_renderer_base_unittest.cc index 9385f53f..da616ae 100644 --- a/media/filters/audio_renderer_base_unittest.cc +++ b/media/filters/audio_renderer_base_unittest.cc @@ -117,7 +117,7 @@ TEST_F(AudioRendererBaseTest, Initialize_Successful) { // Now seek to trigger prerolling, verifying the callback hasn't been // executed yet. EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedCallback()); + renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); EXPECT_EQ(kMaxQueueSize, pending_reads_); renderer_->CheckPoint(0); @@ -145,7 +145,7 @@ TEST_F(AudioRendererBaseTest, OneCompleteReadCycle) { // Now seek to trigger prerolling, verifying the callback hasn't been // executed yet. EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedCallback()); + renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); EXPECT_EQ(kMaxQueueSize, pending_reads_); renderer_->CheckPoint(0); diff --git a/media/filters/decoder_base.h b/media/filters/decoder_base.h index 6cf4937..a889e19fb 100644 --- a/media/filters/decoder_base.h +++ b/media/filters/decoder_base.h @@ -34,11 +34,10 @@ class DecoderBase : public Decoder { NewRunnableMethod(this, &DecoderBase::StopTask, callback)); } - virtual void Seek(base::TimeDelta time, - FilterCallback* callback) { + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb) { message_loop_->PostTask( FROM_HERE, - NewRunnableMethod(this, &DecoderBase::SeekTask, time, callback)); + NewRunnableMethod(this, &DecoderBase::SeekTask, time, cb)); } // Decoder implementation. @@ -170,25 +169,22 @@ class DecoderBase : public Decoder { } } - void SeekTask(base::TimeDelta time, FilterCallback* callback) { + void SeekTask(base::TimeDelta time, const FilterStatusCB& cb) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK_EQ(0u, pending_reads_) << "Pending reads should have completed"; DCHECK_EQ(0u, pending_requests_) << "Pending requests should be empty"; // Delegate to the subclass first. - DoSeek(time, - NewRunnableMethod(this, &DecoderBase::OnSeekComplete, callback)); + DoSeek(time, NewRunnableMethod(this, &DecoderBase::OnSeekComplete, cb)); } - void OnSeekComplete(FilterCallback* callback) { + void OnSeekComplete(const FilterStatusCB& cb) { // Flush our decoded results. result_queue_.clear(); // Signal that we're done seeking. - if (callback) { - callback->Run(); - delete callback; - } + if (!cb.is_null()) + cb.Run(PIPELINE_OK); } void InitializeTask(DemuxerStream* demuxer_stream, diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc index 98ec882..8347019 100644 --- a/media/filters/ffmpeg_demuxer.cc +++ b/media/filters/ffmpeg_demuxer.cc @@ -313,13 +313,13 @@ void FFmpegDemuxer::Stop(FilterCallback* callback) { SignalReadCompleted(DataSource::kReadError); } -void FFmpegDemuxer::Seek(base::TimeDelta time, FilterCallback* callback) { +void FFmpegDemuxer::Seek(base::TimeDelta time, const FilterStatusCB& cb) { // TODO(hclam): by returning from this method, it is assumed that the seek // operation is completed and filters behind the demuxer is good to issue // more reads, but we are posting a task here, which makes the seek operation // asynchronous, should change how seek works to make it fully asynchronous. message_loop_->PostTask(FROM_HERE, - NewRunnableMethod(this, &FFmpegDemuxer::SeekTask, time, callback)); + NewRunnableMethod(this, &FFmpegDemuxer::SeekTask, time, cb)); } void FFmpegDemuxer::SetPlaybackRate(float playback_rate) { @@ -528,9 +528,8 @@ void FFmpegDemuxer::InitializeTask(DataSource* data_source, callback->Run(PIPELINE_OK); } -void FFmpegDemuxer::SeekTask(base::TimeDelta time, FilterCallback* callback) { +void FFmpegDemuxer::SeekTask(base::TimeDelta time, const FilterStatusCB& cb) { DCHECK_EQ(MessageLoop::current(), message_loop_); - scoped_ptr<FilterCallback> c(callback); // Tell streams to flush buffers due to seeking. StreamVector::iterator iter; @@ -553,7 +552,7 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time, FilterCallback* callback) { } // Notify we're finished seeking. - callback->Run(); + cb.Run(PIPELINE_OK); } void FFmpegDemuxer::DemuxTask() { diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h index 3fe2cf1..d96622c 100644 --- a/media/filters/ffmpeg_demuxer.h +++ b/media/filters/ffmpeg_demuxer.h @@ -142,7 +142,7 @@ class FFmpegDemuxer : public Demuxer, // Filter implementation. virtual void Stop(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); virtual void OnAudioRendererDisabled(); virtual void set_host(FilterHost* filter_host); virtual void SetPlaybackRate(float playback_rate); @@ -171,7 +171,7 @@ class FFmpegDemuxer : public Demuxer, DataSource* data_source, PipelineStatusCallback* callback); // Carries out a seek on the demuxer thread. - void SeekTask(base::TimeDelta time, FilterCallback* callback); + void SeekTask(base::TimeDelta time, const FilterStatusCB& cb); // Carries out demuxing and satisfying stream reads on the demuxer thread. void DemuxTask(); diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc index e13fbb1..74ce0ed 100644 --- a/media/filters/ffmpeg_demuxer_unittest.cc +++ b/media/filters/ffmpeg_demuxer_unittest.cc @@ -425,7 +425,7 @@ TEST_F(FFmpegDemuxerTest, Seek) { .WillOnce(Return(0)); // ...then our callback will be executed... - FilterCallback* seek_callback = NewExpectedCallback(); + FilterStatusCB seek_cb = NewExpectedStatusCB(PIPELINE_OK); EXPECT_CALL(mock_ffmpeg_, CheckPoint(2)); // ...followed by two audio packet reads we'll trigger... @@ -468,7 +468,7 @@ TEST_F(FFmpegDemuxerTest, Seek) { // Issue a simple forward seek, which should discard queued packets. demuxer_->Seek(base::TimeDelta::FromMicroseconds(kExpectedTimestamp), - seek_callback); + seek_cb); message_loop_.RunAllPending(); mock_ffmpeg_.CheckPoint(2); diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc index b281fb4..5487398 100644 --- a/media/filters/ffmpeg_video_decoder.cc +++ b/media/filters/ffmpeg_video_decoder.cc @@ -183,30 +183,27 @@ void FFmpegVideoDecoder::OnFlushComplete() { state_ = kNormal; } -void FFmpegVideoDecoder::Seek(base::TimeDelta time, - FilterCallback* callback) { +void FFmpegVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) { if (MessageLoop::current() != message_loop_) { message_loop_->PostTask(FROM_HERE, - NewRunnableMethod(this, - &FFmpegVideoDecoder::Seek, - time, - callback)); + NewRunnableMethod(this, &FFmpegVideoDecoder::Seek, + time, cb)); return; } DCHECK_EQ(MessageLoop::current(), message_loop_); - DCHECK(!seek_callback_.get()); + DCHECK(seek_cb_.is_null()); pts_stream_.Seek(time); - seek_callback_.reset(callback); + seek_cb_ = cb; decode_engine_->Seek(); } void FFmpegVideoDecoder::OnSeekComplete() { DCHECK_EQ(MessageLoop::current(), message_loop_); - DCHECK(seek_callback_.get()); + DCHECK(!seek_cb_.is_null()); - AutoCallbackRunner done_runner(seek_callback_.release()); + ResetAndRunCB(&seek_cb_, PIPELINE_OK); } void FFmpegVideoDecoder::OnError() { diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h index ab27aad..3281193 100644 --- a/media/filters/ffmpeg_video_decoder.h +++ b/media/filters/ffmpeg_video_decoder.h @@ -28,7 +28,7 @@ class FFmpegVideoDecoder : public VideoDecoder, // Filter implementation. virtual void Stop(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); virtual void Pause(FilterCallback* callback); virtual void Flush(FilterCallback* callback); @@ -100,7 +100,7 @@ class FFmpegVideoDecoder : public VideoDecoder, scoped_ptr<FilterCallback> initialize_callback_; scoped_ptr<FilterCallback> uninitialize_callback_; scoped_ptr<FilterCallback> flush_callback_; - scoped_ptr<FilterCallback> seek_callback_; + FilterStatusCB seek_cb_; scoped_ptr<StatisticsCallback> statistics_callback_; // Hold video frames when flush happens. diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc index 45869f4..38411d3 100644 --- a/media/filters/ffmpeg_video_decoder_unittest.cc +++ b/media/filters/ffmpeg_video_decoder_unittest.cc @@ -429,7 +429,7 @@ TEST_F(FFmpegVideoDecoderTest, DoSeek) { // Expect Seek and verify the results. EXPECT_CALL(*engine_, Seek()) .WillOnce(EngineSeek(engine_)); - decoder_->Seek(kZero, NewExpectedCallback()); + decoder_->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK)); EXPECT_TRUE(kZero == decoder_->pts_stream_.current_duration()); EXPECT_EQ(FFmpegVideoDecoder::kNormal, decoder_->state_); diff --git a/media/filters/file_data_source_unittest.cc b/media/filters/file_data_source_unittest.cc index 1ae111e..5cf012b 100644 --- a/media/filters/file_data_source_unittest.cc +++ b/media/filters/file_data_source_unittest.cc @@ -101,7 +101,7 @@ TEST(FileDataSourceTest, Seek) { const base::TimeDelta kZero; scoped_refptr<FileDataSource> filter(new FileDataSource()); - filter->Seek(kZero, NewExpectedCallback()); + filter->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK)); filter->Stop(NewExpectedCallback()); } diff --git a/media/filters/omx_video_decoder.cc b/media/filters/omx_video_decoder.cc index 43f8100..b185965 100644 --- a/media/filters/omx_video_decoder.cc +++ b/media/filters/omx_video_decoder.cc @@ -156,30 +156,29 @@ void OmxVideoDecoder::OnFlushComplete() { pts_stream_.Flush(); } -void OmxVideoDecoder::Seek(base::TimeDelta time, - FilterCallback* callback) { +void OmxVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) { if (MessageLoop::current() != message_loop_) { message_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, &OmxVideoDecoder::Seek, time, - callback)); + cb)); return; } DCHECK_EQ(MessageLoop::current(), message_loop_); - DCHECK(!seek_callback_.get()); + DCHECK(seek_cb_.is_null()); pts_stream_.Seek(time); - seek_callback_.reset(callback); + seek_cb_ = cb; decode_engine_->Seek(); } void OmxVideoDecoder::OnSeekComplete() { DCHECK_EQ(MessageLoop::current(), message_loop_); - DCHECK(seek_callback_.get()); + DCHECK(!seek_cb_.is_null()); - AutoCallbackRunner done_runner(seek_callback_.release()); + ResetAndRunCB(&seek_cb_, PIPELINE_OK); } void OmxVideoDecoder::OnError() { diff --git a/media/filters/omx_video_decoder.h b/media/filters/omx_video_decoder.h index c86b2fe..40bf721 100644 --- a/media/filters/omx_video_decoder.h +++ b/media/filters/omx_video_decoder.h @@ -34,7 +34,7 @@ class OmxVideoDecoder : public VideoDecoder, StatisticsCallback* stats_callback); virtual void Stop(FilterCallback* callback); virtual void Flush(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); virtual void ProduceVideoFrame(scoped_refptr<VideoFrame> frame); virtual bool ProvidesBuffer(); virtual const MediaFormat& media_format(); @@ -67,7 +67,7 @@ class OmxVideoDecoder : public VideoDecoder, scoped_ptr<FilterCallback> initialize_callback_; scoped_ptr<FilterCallback> uninitialize_callback_; scoped_ptr<FilterCallback> flush_callback_; - scoped_ptr<FilterCallback> seek_callback_; + FilterStatusCB seek_cb_; scoped_ptr<StatisticsCallback> statistics_callback_; VideoCodecInfo info_; diff --git a/media/filters/rtc_video_decoder.cc b/media/filters/rtc_video_decoder.cc index 0ddf630..6744569 100644 --- a/media/filters/rtc_video_decoder.cc +++ b/media/filters/rtc_video_decoder.cc @@ -113,14 +113,11 @@ void RTCVideoDecoder::Stop(FilterCallback* callback) { // TODO(ronghuawu): Stop rtc } -void RTCVideoDecoder::Seek(base::TimeDelta time, - FilterCallback* callback) { +void RTCVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) { if (MessageLoop::current() != message_loop_) { message_loop_->PostTask(FROM_HERE, - NewRunnableMethod(this, - &RTCVideoDecoder::Seek, - time, - callback)); + NewRunnableMethod(this, &RTCVideoDecoder::Seek, + time, cb)); return; } @@ -165,8 +162,7 @@ void RTCVideoDecoder::Seek(base::TimeDelta time, state_ = kNormal; - callback->Run(); - delete callback; + cb.Run(PIPELINE_OK); // TODO(ronghuawu): Start rtc } diff --git a/media/filters/rtc_video_decoder.h b/media/filters/rtc_video_decoder.h index 02bd96f..6b5b914 100644 --- a/media/filters/rtc_video_decoder.h +++ b/media/filters/rtc_video_decoder.h @@ -36,7 +36,7 @@ class RTCVideoDecoder : public VideoDecoder, // Filter implementation. virtual void Play(FilterCallback* callback); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); virtual void Pause(FilterCallback* callback); virtual void Stop(FilterCallback* callback); @@ -91,4 +91,3 @@ class RTCVideoDecoder : public VideoDecoder, } // namespace media #endif // MEDIA_FILTERS_RTC_VIDEO_DECODER_H_ - diff --git a/media/filters/rtc_video_decoder_unittest.cc b/media/filters/rtc_video_decoder_unittest.cc index f0970ad..71287d0 100644 --- a/media/filters/rtc_video_decoder_unittest.cc +++ b/media/filters/rtc_video_decoder_unittest.cc @@ -111,7 +111,7 @@ TEST_F(RTCVideoDecoderTest, DoSeek) { // Expect Seek and verify the results. EXPECT_CALL(*renderer_.get(), ConsumeVideoFrame(_)) .Times(Limits::kMaxVideoFrames); - decoder_->Seek(kZero, NewExpectedCallback()); + decoder_->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK)); message_loop_.RunAllPending(); EXPECT_EQ(RTCVideoDecoder::kNormal, decoder_->state_); @@ -127,7 +127,7 @@ TEST_F(RTCVideoDecoderTest, DoDeliverFrame) { decoder_->set_consume_video_frame_callback( base::Bind(&RTCVideoDecoder::ProduceVideoFrame, base::Unretained(decoder_.get()))); - decoder_->Seek(kZero, NewExpectedCallback()); + decoder_->Seek(kZero, NewExpectedStatusCB(PIPELINE_OK)); decoder_->set_consume_video_frame_callback( base::Bind(&MockVideoRenderer::ConsumeVideoFrame, diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc index 92bedba..7705946 100644 --- a/media/filters/video_renderer_base.cc +++ b/media/filters/video_renderer_base.cc @@ -129,28 +129,37 @@ void VideoRendererBase::SetPlaybackRate(float playback_rate) { playback_rate_ = playback_rate; } -void VideoRendererBase::Seek(base::TimeDelta time, FilterCallback* callback) { - base::AutoLock auto_lock(lock_); - // There is a race condition between filters to receive SeekTask(). - // It turns out we could receive buffer from decoder before seek() - // is called on us. so we do the following: - // kFlushed => ( Receive first buffer or Seek() ) => kSeeking and - // kSeeking => ( Receive enough buffers) => kPrerolled. ) - DCHECK(kPrerolled == state_ || kFlushed == state_ || kSeeking == state_); - - if (state_ == kPrerolled) { - // Already get enough buffers from decoder. - callback->Run(); - delete callback; - } else { - // Otherwise we are either kFlushed or kSeeking, but without enough buffers; - // we should save the callback function and call it later. - state_ = kSeeking; - seek_callback_.reset(callback); +void VideoRendererBase::Seek(base::TimeDelta time, const FilterStatusCB& cb) { + bool run_callback = false; + + { + base::AutoLock auto_lock(lock_); + // There is a race condition between filters to receive SeekTask(). + // It turns out we could receive buffer from decoder before seek() + // is called on us. so we do the following: + // kFlushed => ( Receive first buffer or Seek() ) => kSeeking and + // kSeeking => ( Receive enough buffers) => kPrerolled. ) + DCHECK(kPrerolled == state_ || kFlushed == state_ || kSeeking == state_); + DCHECK(!cb.is_null()); + DCHECK(seek_cb_.is_null()); + + if (state_ == kPrerolled) { + // Already get enough buffers from decoder. + run_callback = true; + + } else { + // Otherwise we are either kFlushed or kSeeking, but without enough + // buffers we should save the callback function and call it later. + state_ = kSeeking; + seek_cb_ = cb; + } + + seek_timestamp_ = time; + ScheduleRead_Locked(); } - seek_timestamp_ = time; - ScheduleRead_Locked(); + if (run_callback) + cb.Run(PIPELINE_OK); } void VideoRendererBase::Initialize(VideoDecoder* decoder, @@ -462,9 +471,8 @@ void VideoRendererBase::ConsumeVideoFrame(scoped_refptr<VideoFrame> frame) { // If we reach prerolled state before Seek() is called by pipeline, // |seek_callback_| is not set, we will return immediately during // when Seek() is eventually called. - if ((seek_callback_.get())) { - seek_callback_->Run(); - seek_callback_.reset(); + if (!seek_cb_.is_null()) { + ResetAndRunCB(&seek_cb_, PIPELINE_OK); } } } else if (state_ == kFlushing && pending_reads_ == 0 && !pending_paint_) { @@ -578,7 +586,10 @@ void VideoRendererBase::EnterErrorState_Locked(PipelineStatus status) { lock_.AssertAcquired(); scoped_ptr<FilterCallback> callback; - switch (state_) { + State old_state = state_; + state_ = kError; + + switch (old_state) { case kUninitialized: case kPrerolled: case kPaused: @@ -593,8 +604,9 @@ void VideoRendererBase::EnterErrorState_Locked(PipelineStatus status) { break; case kSeeking: - CHECK(seek_callback_.get()); - callback.swap(seek_callback_); + CHECK(!seek_cb_.is_null()); + ResetAndRunCB(&seek_cb_, status); + return; break; case kStopped: @@ -604,7 +616,7 @@ void VideoRendererBase::EnterErrorState_Locked(PipelineStatus status) { case kError: return; } - state_ = kError; + host()->SetError(status); if (callback.get()) diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h index 66eba6e..6a9a97e 100644 --- a/media/filters/video_renderer_base.h +++ b/media/filters/video_renderer_base.h @@ -51,7 +51,7 @@ class VideoRendererBase : public VideoRenderer, virtual void Flush(FilterCallback* callback); virtual void Stop(FilterCallback* callback); virtual void SetPlaybackRate(float playback_rate); - virtual void Seek(base::TimeDelta time, FilterCallback* callback); + virtual void Seek(base::TimeDelta time, const FilterStatusCB& cb); // VideoRenderer implementation. virtual void Initialize(VideoDecoder* decoder, @@ -213,7 +213,7 @@ class VideoRendererBase : public VideoRenderer, // Filter callbacks. scoped_ptr<FilterCallback> flush_callback_; - scoped_ptr<FilterCallback> seek_callback_; + FilterStatusCB seek_cb_; scoped_ptr<StatisticsCallback> statistics_callback_; base::TimeDelta seek_timestamp_; diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc index c21d6c7..8a8c348 100644 --- a/media/filters/video_renderer_base_unittest.cc +++ b/media/filters/video_renderer_base_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/bind.h" #include "base/stl_util-inl.h" #include "media/base/callback.h" #include "media/base/data_buffer.h" @@ -110,13 +111,15 @@ class VideoRendererBaseTest : public ::testing::Test { Seek(0); } - void StartSeeking(int64 timestamp) { + void StartSeeking(int64 timestamp, PipelineStatus expected_status) { EXPECT_FALSE(seeking_); // Seek to trigger prerolling. seeking_ = true; renderer_->Seek(base::TimeDelta::FromMicroseconds(timestamp), - NewCallback(this, &VideoRendererBaseTest::OnSeekComplete)); + base::Bind(&VideoRendererBaseTest::OnSeekComplete, + base::Unretained(this), + expected_status)); } void FinishSeeking() { @@ -132,7 +135,7 @@ class VideoRendererBaseTest : public ::testing::Test { } void Seek(int64 timestamp) { - StartSeeking(timestamp); + StartSeeking(timestamp, PIPELINE_OK); FinishSeeking(); } @@ -191,7 +194,8 @@ class VideoRendererBaseTest : public ::testing::Test { read_queue_.push_back(frame); } - void OnSeekComplete() { + void OnSeekComplete(PipelineStatus expected_status, PipelineStatus status) { + EXPECT_EQ(status, expected_status); EXPECT_TRUE(seeking_); seeking_ = false; } @@ -269,9 +273,7 @@ TEST_F(VideoRendererBaseTest, Error_Playing) { TEST_F(VideoRendererBaseTest, Error_Seeking) { Initialize(); Flush(); - StartSeeking(0); - - EXPECT_CALL(host_, SetError(PIPELINE_ERROR_DECODE)); + StartSeeking(0, PIPELINE_ERROR_DECODE); CreateError(); Flush(); } |