diff options
author | fischman@chromium.org <fischman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-16 16:57:02 +0000 |
---|---|---|
committer | fischman@chromium.org <fischman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-16 16:57:02 +0000 |
commit | a9590c25660d4be4caad2add5e16e08003e5ed39 (patch) | |
tree | 10b2dd0b18e9122aa0eb86d80e14c8e3efafd73e | |
parent | bac2d4979f97026ed1344cd2dc9b208f58886537 (diff) | |
download | chromium_src-a9590c25660d4be4caad2add5e16e08003e5ed39.zip chromium_src-a9590c25660d4be4caad2add5e16e08003e5ed39.tar.gz chromium_src-a9590c25660d4be4caad2add5e16e08003e5ed39.tar.bz2 |
PipelineError is dead. Long live PipelineStatus!
PipelineError was a poor naming choice because most of the time variables of
that type held the value PIPELINE_OK meaning there was in fact no error.
Replaced the idiom of [0-ary callback + GetError()] with
[1-ary callback taking PipelineStatus argument] which makes the Pipeline API
cleaner and less error-prone. Before, consumers of the API had to make sure to
call GetError() at the top of each callback, or risk missing state transitions
in the pipeline. Now each callback gets an explicit parameter holding the
pipeline status at the moment the callback was invoked so failing to handle
error conditions should be more apparent in the code.
BUG=none
TEST=media_unittests + trybots: {mac,linux,win}{_layout,}, linux_rel, linux_clang (all pass or fail with unrelated errors)
Review URL: http://codereview.chromium.org/6686061
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78379 0039d316-1c4b-4281-b951-d872f2087c98
31 files changed, 410 insertions, 303 deletions
diff --git a/media/base/async_filter_factory_base.cc b/media/base/async_filter_factory_base.cc index bbc5160..ce09c60 100644 --- a/media/base/async_filter_factory_base.cc +++ b/media/base/async_filter_factory_base.cc @@ -48,7 +48,7 @@ void AsyncDataSourceFactoryBase::Build(const std::string& url, } void AsyncDataSourceFactoryBase::RunAndDestroyCallback( - PipelineError error, + PipelineStatus error, BuildCallback* callback) const { DCHECK_NE(error, PIPELINE_OK); DCHECK(callback); @@ -84,7 +84,7 @@ void AsyncDataSourceFactoryBase::BuildRequest::Start( } void AsyncDataSourceFactoryBase::BuildRequest::RequestComplete( - PipelineError error, + PipelineStatus status, DataSource* data_source) { DCHECK(callback_.get()); DCHECK(done_callback_.get()); @@ -102,7 +102,7 @@ void AsyncDataSourceFactoryBase::BuildRequest::RequestComplete( // no modifications should be made to this object after this call. done_callback->Run(this); - callback->Run(error, data_source); + callback->Run(status, data_source); } const std::string& AsyncDataSourceFactoryBase::BuildRequest::url() const { diff --git a/media/base/async_filter_factory_base.h b/media/base/async_filter_factory_base.h index 75617f4..d800284 100644 --- a/media/base/async_filter_factory_base.h +++ b/media/base/async_filter_factory_base.h @@ -74,17 +74,17 @@ class AsyncDataSourceFactoryBase : public DataSourceFactory { void Start(RequestDoneCallback* done_callback); // Derived objects call this method to indicate that the build request - // has completed. If the build was successful |error| should be set to + // has completed. If the build was successful |status| should be set to // PIPELINE_OK and |data_source| should contain the DataSource object // that was built by this request. Ownership of |data_source| is being // passed in this call. If an error occurs during the build process, this - // method should be called with |error| set to an appropriate status code + // method should be called with |status| set to an appropriate status code // and |data_source| set to NULL. // // The derived object should be in a state where it can be deleted from // within this call. This class as well AsyncDataSourceFactoryBase use this // method to cleanup state associated with this request. - void RequestComplete(media::PipelineError error, DataSource* data_source); + void RequestComplete(media::PipelineStatus status, DataSource* data_source); protected: // Implemented by the derived object to start the build. Called by Start(). @@ -111,7 +111,7 @@ class AsyncDataSourceFactoryBase : public DataSourceFactory { BuildCallback* callback) = 0; private: - void RunAndDestroyCallback(PipelineError error, + void RunAndDestroyCallback(PipelineStatus status, BuildCallback* callback) const; typedef Callback1<BuildRequest*>::Type RequestDoneCallback; diff --git a/media/base/composite_data_source_factory.cc b/media/base/composite_data_source_factory.cc index 84b5259..7b5c573 100644 --- a/media/base/composite_data_source_factory.cc +++ b/media/base/composite_data_source_factory.cc @@ -24,7 +24,7 @@ class CompositeDataSourceFactory::BuildRequest private: void CallNextFactory(); - void OnBuildDone(PipelineError error, DataSource* data_source); + void OnBuildDone(PipelineStatus status, DataSource* data_source); FactoryList factories_; }; @@ -87,22 +87,22 @@ void CompositeDataSourceFactory::BuildRequest::CallNextFactory() { } void CompositeDataSourceFactory::BuildRequest::OnBuildDone( - PipelineError error, + PipelineStatus status, DataSource* data_source) { - if (error == PIPELINE_OK) { + if (status == PIPELINE_OK) { DCHECK(data_source); - RequestComplete(error, data_source); + RequestComplete(status, data_source); return; } DCHECK(!data_source); - if ((error == DATASOURCE_ERROR_URL_NOT_SUPPORTED) && !factories_.empty()) { + if ((status == DATASOURCE_ERROR_URL_NOT_SUPPORTED) && !factories_.empty()) { CallNextFactory(); return; } - RequestComplete(error, data_source); + RequestComplete(status, data_source); } } // namespace media diff --git a/media/base/composite_filter.cc b/media/base/composite_filter.cc index 3e7b60c..d08eacb 100644 --- a/media/base/composite_filter.cc +++ b/media/base/composite_filter.cc @@ -17,7 +17,7 @@ class CompositeFilter::FilterHostImpl : public FilterHost { FilterHost* host(); // media::FilterHost methods. - virtual void SetError(PipelineError error); + virtual void SetError(PipelineStatus error); virtual base::TimeDelta GetTime() const; virtual base::TimeDelta GetDuration() const; virtual void SetTime(base::TimeDelta time); @@ -45,7 +45,7 @@ CompositeFilter::CompositeFilter(MessageLoop* message_loop) : state_(kCreated), sequence_index_(0), message_loop_(message_loop), - error_(PIPELINE_OK) { + status_(PIPELINE_OK) { DCHECK(message_loop); runnable_factory_.reset( new ScopedRunnableMethodFactory<CompositeFilter>(this)); @@ -228,7 +228,7 @@ void CompositeFilter::ChangeState(State new_state) { void CompositeFilter::StartSerialCallSequence() { DCHECK_EQ(message_loop_, MessageLoop::current()); - error_ = PIPELINE_OK; + status_ = PIPELINE_OK; if (!filters_.empty()) { sequence_index_ = 0; @@ -242,7 +242,7 @@ void CompositeFilter::StartSerialCallSequence() { void CompositeFilter::StartParallelCallSequence() { DCHECK_EQ(message_loop_, MessageLoop::current()); - error_ = PIPELINE_OK; + status_ = PIPELINE_OK; if (!filters_.empty()) { sequence_index_ = 0; @@ -329,10 +329,10 @@ CompositeFilter::State CompositeFilter::GetNextState(State state) const { void CompositeFilter::SerialCallback() { DCHECK_EQ(message_loop_, MessageLoop::current()); - if (error_ != PIPELINE_OK) { + if (status_ != PIPELINE_OK) { // We encountered an error. Terminate the sequence now. ChangeState(kError); - HandleError(error_); + HandleError(status_); return; } @@ -360,10 +360,10 @@ void CompositeFilter::ParallelCallback() { sequence_index_++; if (sequence_index_ == filters_.size()) { - if (error_ != PIPELINE_OK) { + if (status_ != PIPELINE_OK) { // We encountered an error. ChangeState(kError); - HandleError(error_); + HandleError(status_); return; } @@ -391,16 +391,14 @@ void CompositeFilter::OnCallSequenceDone() { } } -void CompositeFilter::SendErrorToHost(PipelineError error) { +void CompositeFilter::SendErrorToHost(PipelineStatus error) { if (host_impl_.get()) host_impl_.get()->host()->SetError(error); } -void CompositeFilter::HandleError(PipelineError error) { - if (error != PIPELINE_OK) { - SendErrorToHost(error); - } - +void CompositeFilter::HandleError(PipelineStatus error) { + DCHECK_NE(error, PIPELINE_OK); + SendErrorToHost(error); DispatchPendingCallback(); } @@ -435,7 +433,7 @@ bool CompositeFilter::CanForwardError() { return (state_ == kCreated) || (state_ == kPlaying) || (state_ == kPaused); } -void CompositeFilter::SetError(PipelineError error) { +void CompositeFilter::SetError(PipelineStatus error) { // TODO(acolwell): Temporary hack to handle errors that occur // during filter initialization. In this case we just forward // the error to the host even if it is on the wrong thread. We @@ -461,7 +459,7 @@ void CompositeFilter::SetError(PipelineError error) { if (state_ == kStopPending || state_ == kStopped) return; - error_ = error; + status_ = error; if (CanForwardError()) SendErrorToHost(error); } @@ -477,7 +475,7 @@ FilterHost* CompositeFilter::FilterHostImpl::host() { } // media::FilterHost methods. -void CompositeFilter::FilterHostImpl::SetError(PipelineError error) { +void CompositeFilter::FilterHostImpl::SetError(PipelineStatus error) { parent_->SetError(error); } diff --git a/media/base/composite_filter.h b/media/base/composite_filter.h index 5fa8531..a2c8c09 100644 --- a/media/base/composite_filter.h +++ b/media/base/composite_filter.h @@ -38,7 +38,7 @@ class CompositeFilter : public Filter { protected: virtual ~CompositeFilter(); - void SetError(PipelineError error); + void SetError(PipelineStatus error); private: class FilterHostImpl; @@ -89,10 +89,10 @@ class CompositeFilter : public Filter { void OnCallSequenceDone(); // Helper function for sending an error to the FilterHost. - void SendErrorToHost(PipelineError error); + void SendErrorToHost(PipelineStatus error); // Helper function for handling errors during call sequences. - void HandleError(PipelineError error); + 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. @@ -130,8 +130,8 @@ class CompositeFilter : public Filter { // object. scoped_ptr<FilterHostImpl> host_impl_; - // Error passed in the last SetError() call. - PipelineError error_; + // PIPELINE_OK, or last error passed to SetError(). + PipelineStatus status_; scoped_ptr<ScopedRunnableMethodFactory<CompositeFilter> > runnable_factory_; diff --git a/media/base/filter_factories.h b/media/base/filter_factories.h index aad391f..b716041 100644 --- a/media/base/filter_factories.h +++ b/media/base/filter_factories.h @@ -18,7 +18,7 @@ class DataSource; class DataSourceFactory { public: // Ownership of the DataSource is transferred through this callback. - typedef Callback2<PipelineError, DataSource*>::Type BuildCallback; + typedef Callback2<PipelineStatus, DataSource*>::Type BuildCallback; virtual ~DataSourceFactory(); @@ -36,7 +36,7 @@ class Demuxer; class DemuxerFactory { public: // Ownership of the Demuxer is transferred through this callback. - typedef Callback2<PipelineError, Demuxer*>::Type BuildCallback; + typedef Callback2<PipelineStatus, Demuxer*>::Type BuildCallback; virtual ~DemuxerFactory(); diff --git a/media/base/filter_host.h b/media/base/filter_host.h index 377cb97..4bdad9f 100644 --- a/media/base/filter_host.h +++ b/media/base/filter_host.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -7,7 +7,7 @@ // reference as part of initialization. // // This interface is intentionally verbose to cover the needs for the different -// types of filters (see media/base/filters.h for filter definitionss). Filters +// types of filters (see media/base/filters.h for filter definitions). Filters // typically use parts of the interface that are relevant to their function. // For example, an audio renderer filter typically calls SetTime as it feeds // data to the audio hardware. A video renderer filter typically calls GetTime @@ -26,7 +26,7 @@ class FilterHost { public: // Stops execution of the pipeline due to a fatal error. Do not call this // method with PIPELINE_OK. - virtual void SetError(PipelineError error) = 0; + virtual void SetError(PipelineStatus error) = 0; // Gets the current time in microseconds. virtual base::TimeDelta GetTime() const = 0; diff --git a/media/base/mock_callback.cc b/media/base/mock_callback.cc index 03ed896..eeedd15 100644 --- a/media/base/mock_callback.cc +++ b/media/base/mock_callback.cc @@ -28,13 +28,13 @@ MockStatusCallback::~MockStatusCallback() { // Required by GMock to allow the RunWithParams() expectation // in ExpectRunAndDelete() to compile. -bool operator==(const Tuple1<PipelineError>& lhs, - const Tuple1<PipelineError>& rhs) { +bool operator==(const Tuple1<PipelineStatus>& lhs, + const Tuple1<PipelineStatus>& rhs) { return lhs.a == rhs.a; } -void MockStatusCallback::ExpectRunAndDelete(PipelineError error) { - EXPECT_CALL(*this, RunWithParams(Tuple1<PipelineError>(error))); +void MockStatusCallback::ExpectRunAndDelete(PipelineStatus status) { + EXPECT_CALL(*this, RunWithParams(Tuple1<PipelineStatus>(status))); EXPECT_CALL(*this, Destructor()); } @@ -44,10 +44,10 @@ MockCallback* NewExpectedCallback() { return callback; } -MockStatusCallback* NewExpectedStatusCallback(PipelineError error) { +MockStatusCallback* NewExpectedStatusCallback(PipelineStatus status) { StrictMock<MockStatusCallback>* callback = new StrictMock<MockStatusCallback>(); - callback->ExpectRunAndDelete(error); + callback->ExpectRunAndDelete(status); return callback; } diff --git a/media/base/mock_callback.h b/media/base/mock_callback.h index ddf1bef..d142a9b 100644 --- a/media/base/mock_callback.h +++ b/media/base/mock_callback.h @@ -51,19 +51,19 @@ class MockCallback : public CallbackRunner<Tuple0> { // Helper class similar to MockCallback but is used where a // PipelineStatusCallback is needed. -class MockStatusCallback : public CallbackRunner<Tuple1<PipelineError> > { +class MockStatusCallback : public CallbackRunner<Tuple1<PipelineStatus> > { public: MockStatusCallback(); virtual ~MockStatusCallback(); - MOCK_METHOD1(RunWithParams, void(const Tuple1<PipelineError>& params)); + MOCK_METHOD1(RunWithParams, void(const Tuple1<PipelineStatus>& params)); // Can be used to verify the object is destroyed. MOCK_METHOD0(Destructor, void()); // Convenience function to set expectations for the callback to execute and // deleted. - void ExpectRunAndDelete(PipelineError error); + void ExpectRunAndDelete(PipelineStatus status); private: DISALLOW_COPY_AND_ASSIGN(MockStatusCallback); @@ -72,7 +72,7 @@ class MockStatusCallback : public CallbackRunner<Tuple1<PipelineError> > { // Convenience functions that automatically create and set an expectation for // the callback to run. MockCallback* NewExpectedCallback(); -MockStatusCallback* NewExpectedStatusCallback(PipelineError error); +MockStatusCallback* NewExpectedStatusCallback(PipelineStatus status); } // namespace media diff --git a/media/base/mock_filter_host.h b/media/base/mock_filter_host.h index aecc3f6..242e516 100644 --- a/media/base/mock_filter_host.h +++ b/media/base/mock_filter_host.h @@ -24,7 +24,7 @@ class MockFilterHost : public FilterHost { // FilterHost implementation. MOCK_METHOD0(InitializationComplete, void()); - MOCK_METHOD1(SetError, void(PipelineError error)); + MOCK_METHOD1(SetError, void(PipelineStatus error)); MOCK_CONST_METHOD0(GetDuration, base::TimeDelta()); MOCK_CONST_METHOD0(GetTime, base::TimeDelta()); MOCK_METHOD1(SetTime, void(base::TimeDelta time)); diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc index fbf2bb1..5fc5b55 100644 --- a/media/base/mock_filters.cc +++ b/media/base/mock_filters.cc @@ -4,6 +4,7 @@ #include "media/base/mock_filters.h" +#include "base/logging.h" #include "media/base/filter_host.h" using ::testing::_; @@ -36,13 +37,14 @@ void MockDataSource::SetTotalAndBufferedBytes(int64 total_bytes, } MockDemuxerFactory::MockDemuxerFactory(MockDemuxer* demuxer) - : demuxer_(demuxer), error_(PIPELINE_OK) { + : demuxer_(demuxer), status_(PIPELINE_OK) { } MockDemuxerFactory::~MockDemuxerFactory() {} -void MockDemuxerFactory::SetError(PipelineError error) { - error_ = error; +void MockDemuxerFactory::SetError(PipelineStatus error) { + DCHECK_NE(error, PIPELINE_OK); + status_ = error; } void MockDemuxerFactory::RunBuildCallback(const std::string& url, @@ -58,12 +60,12 @@ void MockDemuxerFactory::RunBuildCallback(const std::string& url, scoped_refptr<MockDemuxer> demuxer = demuxer_; demuxer_ = NULL; - if (error_ == PIPELINE_OK) { + if (status_ == PIPELINE_OK) { cb->Run(PIPELINE_OK, demuxer.get()); return; } - cb->Run(error_, static_cast<Demuxer*>(NULL)); + cb->Run(status_, static_cast<Demuxer*>(NULL)); } void MockDemuxerFactory::DestroyBuildCallback(const std::string& url, @@ -133,14 +135,14 @@ MockFilterCollection::~MockFilterCollection() {} FilterCollection* MockFilterCollection::filter_collection( bool include_demuxer, bool run_build_callback, - PipelineError build_error) const { + PipelineStatus build_status) const { FilterCollection* collection = new FilterCollection(); MockDemuxerFactory* demuxer_factory = new MockDemuxerFactory(include_demuxer ? demuxer_ : NULL); - if (build_error != PIPELINE_OK) - demuxer_factory->SetError(build_error); + if (build_status != PIPELINE_OK) + demuxer_factory->SetError(build_status); if (run_build_callback) { ON_CALL(*demuxer_factory, Build(_, NotNull())).WillByDefault(Invoke( @@ -165,7 +167,7 @@ void RunFilterCallback(::testing::Unused, FilterCallback* callback) { } void RunPipelineStatusCallback( - PipelineError status, PipelineStatusCallback* callback) { + PipelineStatus status, PipelineStatusCallback* callback) { callback->Run(status); delete callback; } diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index d1ae03d..8004190 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -134,7 +134,7 @@ class MockDemuxerFactory : public DemuxerFactory { explicit MockDemuxerFactory(MockDemuxer* demuxer); virtual ~MockDemuxerFactory(); - void SetError(PipelineError error); + void SetError(PipelineStatus error); void RunBuildCallback(const std::string& url, BuildCallback* callback); void DestroyBuildCallback(const std::string& url, BuildCallback* callback); @@ -144,7 +144,7 @@ class MockDemuxerFactory : public DemuxerFactory { private: scoped_refptr<MockDemuxer> demuxer_; - PipelineError error_; + PipelineStatus status_; DISALLOW_COPY_AND_ASSIGN(MockDemuxerFactory); }; @@ -295,7 +295,7 @@ class MockFilterCollection { FilterCollection* filter_collection(bool include_demuxer, bool run_build_callback, - PipelineError build_error) const; + PipelineStatus build_status) const; private: scoped_refptr<MockDemuxer> demuxer_; @@ -311,7 +311,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 RunPipelineStatusCallback(PipelineError status, +void RunPipelineStatusCallback(PipelineStatus status, PipelineStatusCallback* callback); void RunFilterCallback3(::testing::Unused, FilterCallback* callback, ::testing::Unused); diff --git a/media/base/pipeline.h b/media/base/pipeline.h index 802903d..0df1bd3 100644 --- a/media/base/pipeline.h +++ b/media/base/pipeline.h @@ -36,10 +36,6 @@ struct PipelineStatistics { class FilterCollection; -// Client-provided callbacks for various pipeline operations. Clients should -// inspect the Pipeline for errors. -typedef Callback0::Type PipelineCallback; - class Pipeline : public base::RefCountedThreadSafe<Pipeline> { public: // Initializes pipeline. Pipeline takes ownership of all callbacks passed @@ -47,9 +43,9 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> { // |ended_callback| will be executed when the media reaches the end. // |error_callback_| will be executed upon an error in the pipeline. // |network_callback_| will be executed when there's a network event. - virtual void Init(PipelineCallback* ended_callback, - PipelineCallback* error_callback, - PipelineCallback* network_callback) = 0; + virtual void Init(PipelineStatusCallback* ended_callback, + PipelineStatusCallback* error_callback, + PipelineStatusCallback* network_callback) = 0; // Build a pipeline to render the given URL using the given filter collection // to construct a filter chain. Returns true if successful, false otherwise @@ -61,11 +57,10 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> { // // This method is asynchronous and can execute a callback when completed. // If the caller provides a |start_callback|, it will be called when the - // pipeline initialization completes. Clients are expected to call GetError() - // to check whether initialization succeeded. + // pipeline initialization completes. virtual bool Start(FilterCollection* filter_collection, const std::string& url, - PipelineCallback* start_callback) = 0; + PipelineStatusCallback* start_callback) = 0; // Asynchronously stops the pipeline and resets it to an uninitialized state. // If provided, |stop_callback| will be executed when the pipeline has been @@ -77,14 +72,15 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> { // // TODO(scherkus): ideally clients would destroy the pipeline after calling // Stop() and create a new pipeline as needed. - virtual void Stop(PipelineCallback* stop_callback) = 0; + virtual void Stop(PipelineStatusCallback* stop_callback) = 0; // Attempt to seek to the position specified by time. |seek_callback| will be // executed when the all filters in the pipeline have processed the seek. // // Clients are expected to call GetCurrentTime() to check whether the seek // succeeded. - virtual void Seek(base::TimeDelta time, PipelineCallback* seek_callback) = 0; + virtual void Seek(base::TimeDelta time, + PipelineStatusCallback* seek_callback) = 0; // Returns true if the pipeline has been started via Start(). If IsRunning() // returns true, it is expected that Stop() will be called before destroying @@ -160,10 +156,6 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> { // the media and that the network is no longer needed. virtual bool IsLoaded() const = 0; - // Gets the current error status for the pipeline. If the pipeline is - // operating correctly, this will return OK. - virtual PipelineError GetError() const = 0; - // Gets the current pipeline statistics. virtual PipelineStatistics GetStatistics() const = 0; diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc index 958cd5c..7c71e66 100644 --- a/media/base/pipeline_impl.cc +++ b/media/base/pipeline_impl.cc @@ -16,6 +16,39 @@ namespace media { +PipelineStatusNotification::PipelineStatusNotification() + : cv_(&lock_), status_(PIPELINE_OK), notified_(false) { + callback_.reset(NewCallback(this, &PipelineStatusNotification::Notify)); +} + +PipelineStatusNotification::~PipelineStatusNotification() { + DCHECK(notified_); +} + +media::PipelineStatusCallback* PipelineStatusNotification::Callback() { + return callback_.release(); +} + +void PipelineStatusNotification::Notify(media::PipelineStatus status) { + base::AutoLock auto_lock(lock_); + DCHECK(!notified_); + notified_ = true; + status_ = status; + cv_.Signal(); +} + +void PipelineStatusNotification::Wait() { + base::AutoLock auto_lock(lock_); + while (!notified_) + cv_.Wait(); +} + +media::PipelineStatus PipelineStatusNotification::status() { + base::AutoLock auto_lock(lock_); + DCHECK(notified_); + return status_; +} + class PipelineImpl::PipelineInitState { public: scoped_refptr<Demuxer> demuxer_; @@ -40,9 +73,9 @@ PipelineImpl::~PipelineImpl() { DCHECK(!seek_pending_); } -void PipelineImpl::Init(PipelineCallback* ended_callback, - PipelineCallback* error_callback, - PipelineCallback* network_callback) { +void PipelineImpl::Init(PipelineStatusCallback* ended_callback, + PipelineStatusCallback* error_callback, + PipelineStatusCallback* network_callback) { DCHECK(!IsRunning()) << "Init() should be called before the pipeline has started"; ended_callback_.reset(ended_callback); @@ -53,9 +86,9 @@ void PipelineImpl::Init(PipelineCallback* ended_callback, // Creates the PipelineInternal and calls it's start method. bool PipelineImpl::Start(FilterCollection* collection, const std::string& url, - PipelineCallback* start_callback) { + PipelineStatusCallback* start_callback) { base::AutoLock auto_lock(lock_); - scoped_ptr<PipelineCallback> callback(start_callback); + scoped_ptr<PipelineStatusCallback> callback(start_callback); scoped_ptr<FilterCollection> filter_collection(collection); if (running_) { @@ -79,9 +112,9 @@ bool PipelineImpl::Start(FilterCollection* collection, return true; } -void PipelineImpl::Stop(PipelineCallback* stop_callback) { +void PipelineImpl::Stop(PipelineStatusCallback* stop_callback) { base::AutoLock auto_lock(lock_); - scoped_ptr<PipelineCallback> callback(stop_callback); + scoped_ptr<PipelineStatusCallback> callback(stop_callback); if (!running_) { VLOG(1) << "Media pipeline has already stopped"; return; @@ -93,9 +126,9 @@ void PipelineImpl::Stop(PipelineCallback* stop_callback) { } void PipelineImpl::Seek(base::TimeDelta time, - PipelineCallback* seek_callback) { + PipelineStatusCallback* seek_callback) { base::AutoLock auto_lock(lock_); - scoped_ptr<PipelineCallback> callback(seek_callback); + scoped_ptr<PipelineStatusCallback> callback(seek_callback); if (!running_) { VLOG(1) << "Media pipeline must be running"; return; @@ -271,11 +304,6 @@ bool PipelineImpl::IsLoaded() const { return loaded_; } -PipelineError PipelineImpl::GetError() const { - base::AutoLock auto_lock(lock_); - return error_; -} - PipelineStatistics PipelineImpl::GetStatistics() const { base::AutoLock auto_lock(lock_); return statistics_; @@ -322,7 +350,7 @@ void PipelineImpl::ResetState() { video_height_ = 0; volume_ = 1.0f; playback_rate_ = 0.0f; - error_ = PIPELINE_OK; + status_ = PIPELINE_OK; has_audio_ = false; has_video_ = false; waiting_for_clock_update_ = false; @@ -335,7 +363,8 @@ void PipelineImpl::set_state(State next_state) { } bool PipelineImpl::IsPipelineOk() { - return PIPELINE_OK == GetError(); + base::AutoLock auto_lock(lock_); + return status_ == PIPELINE_OK; } bool PipelineImpl::IsPipelineStopped() { @@ -368,7 +397,7 @@ void PipelineImpl::FinishInitialization() { // Execute the seek callback, if present. Note that this might be the // initial callback passed into Start(). if (seek_callback_.get()) { - seek_callback_->Run(); + seek_callback_->Run(status_); seek_callback_.reset(); } } @@ -405,7 +434,7 @@ PipelineImpl::State PipelineImpl::FindNextState(State current) { } } -void PipelineImpl::SetError(PipelineError error) { +void PipelineImpl::SetError(PipelineStatus error) { DCHECK(IsRunning()); DCHECK(error != PIPELINE_OK) << "PIPELINE_OK isn't an error!"; VLOG(1) << "Media pipeline error: " << error; @@ -541,7 +570,7 @@ void PipelineImpl::OnUpdateStatistics(const PipelineStatistics& stats) { void PipelineImpl::StartTask(FilterCollection* filter_collection, const std::string& url, - PipelineCallback* start_callback) { + PipelineStatusCallback* start_callback) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK_EQ(kCreated, state_); filter_collection_.reset(filter_collection); @@ -578,11 +607,8 @@ void PipelineImpl::InitializeTask() { DCHECK_EQ(MessageLoop::current(), message_loop_); // If we have received the stop or error signal, return immediately. - if (IsPipelineStopPending() || - IsPipelineStopped() || - PIPELINE_OK != GetError()) { + if (IsPipelineStopPending() || IsPipelineStopped() || !IsPipelineOk()) return; - } DCHECK(state_ == kInitDemuxer || state_ == kInitAudioDecoder || @@ -669,14 +695,14 @@ void PipelineImpl::InitializeTask() { // TODO(scherkus): beware! this can get posted multiple times since we post // Stop() tasks even if we've already stopped. Perhaps this should no-op for // additional calls, however most of this logic will be changing. -void PipelineImpl::StopTask(PipelineCallback* stop_callback) { +void PipelineImpl::StopTask(PipelineStatusCallback* stop_callback) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK(!IsPipelineStopPending()); DCHECK_NE(state_, kStopped); if (state_ == kStopped) { // Already stopped so just run callback. - stop_callback->Run(); + stop_callback->Run(status_); delete stop_callback; return; } @@ -687,7 +713,7 @@ void PipelineImpl::StopTask(PipelineCallback* stop_callback) { // the teardown in progress from an error teardown into one that acts // like the error never occurred. base::AutoLock auto_lock(lock_); - error_ = PIPELINE_OK; + status_ = PIPELINE_OK; error_caused_teardown_ = false; } @@ -702,7 +728,7 @@ void PipelineImpl::StopTask(PipelineCallback* stop_callback) { } } -void PipelineImpl::ErrorChangedTask(PipelineError error) { +void PipelineImpl::ErrorChangedTask(PipelineStatus error) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK_NE(PIPELINE_OK, error) << "PIPELINE_OK isn't an error!"; @@ -714,7 +740,7 @@ void PipelineImpl::ErrorChangedTask(PipelineError error) { } base::AutoLock auto_lock(lock_); - error_ = error; + status_ = error; error_caused_teardown_ = true; @@ -751,7 +777,7 @@ void PipelineImpl::VolumeChangedTask(float volume) { } void PipelineImpl::SeekTask(base::TimeDelta time, - PipelineCallback* seek_callback) { + PipelineStatusCallback* seek_callback) { DCHECK_EQ(MessageLoop::current(), message_loop_); DCHECK(!IsPipelineStopPending()); @@ -821,14 +847,14 @@ void PipelineImpl::NotifyEndedTask() { // Transition to ended, executing the callback if present. set_state(kEnded); if (ended_callback_.get()) { - ended_callback_->Run(); + ended_callback_->Run(status_); } } void PipelineImpl::NotifyNetworkEventTask() { DCHECK_EQ(MessageLoop::current(), message_loop_); if (network_callback_.get()) { - network_callback_->Run(); + network_callback_->Run(status_); } } @@ -965,18 +991,16 @@ void PipelineImpl::FinishDestroyingFiltersTask() { pipeline_filter_ = NULL; - if (error_caused_teardown_ && GetError() != PIPELINE_OK && - error_callback_.get()) { - error_callback_->Run(); - } + if (error_caused_teardown_ && !IsPipelineOk() && error_callback_.get()) + error_callback_->Run(status_); if (stop_pending_) { stop_pending_ = false; ResetState(); - scoped_ptr<PipelineCallback> stop_callback(stop_callback_.release()); + scoped_ptr<PipelineStatusCallback> stop_callback(stop_callback_.release()); // Notify the client that stopping has finished. if (stop_callback.get()) { - stop_callback->Run(); + stop_callback->Run(status_); } } @@ -1001,19 +1025,18 @@ void PipelineImpl::InitializeDemuxer() { NewCallback(this, &PipelineImpl::OnDemuxerBuilt)); } -void PipelineImpl::OnDemuxerBuilt(PipelineError error, - Demuxer* demuxer) { +void PipelineImpl::OnDemuxerBuilt(PipelineStatus status, Demuxer* demuxer) { if (MessageLoop::current() != message_loop_) { message_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, &PipelineImpl::OnDemuxerBuilt, - error, + status, make_scoped_refptr(demuxer))); return; } - if (error != PIPELINE_OK) { - SetError(error); + if (status != PIPELINE_OK) { + SetError(status); return; } diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h index 8f0e4ea..e50322b 100644 --- a/media/base/pipeline_impl.h +++ b/media/base/pipeline_impl.h @@ -2,7 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// Implementation of Pipeline. +// Implementation of Pipeline & PipelineStatusNotification (an async-to-sync +// callback adapter). #ifndef MEDIA_BASE_PIPELINE_IMPL_H_ #define MEDIA_BASE_PIPELINE_IMPL_H_ @@ -15,6 +16,8 @@ #include "base/message_loop.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" +#include "base/synchronization/condition_variable.h" +#include "base/synchronization/lock.h" #include "base/threading/thread.h" #include "base/time.h" #include "media/base/clock.h" @@ -24,6 +27,32 @@ namespace media { +// Adapter for using asynchronous Pipeline methods in code that wants to run +// synchronously. To use, construct an instance of this class and pass the +// |Callback()| to the Pipeline method requiring a callback. Then Wait() for +// the callback to get fired and call status() to see what the callback's +// argument was. This object is for one-time use; call |Callback()| exactly +// once. +class PipelineStatusNotification { + public: + PipelineStatusNotification(); + ~PipelineStatusNotification(); + + // See class-level comment for usage. + media::PipelineStatusCallback* Callback(); + void Notify(media::PipelineStatus status); + void Wait(); + media::PipelineStatus status(); + + private: + base::Lock lock_; + base::ConditionVariable cv_; + media::PipelineStatus status_; + bool notified_; + scoped_ptr<media::PipelineStatusCallback> callback_; + + DISALLOW_COPY_AND_ASSIGN(PipelineStatusNotification); +}; // PipelineImpl runs the media pipeline. Filters are created and called on the // message loop injected into this object. PipelineImpl works like a state @@ -67,14 +96,15 @@ class PipelineImpl : public Pipeline, public FilterHost { explicit PipelineImpl(MessageLoop* message_loop); // Pipeline implementation. - virtual void Init(PipelineCallback* ended_callback, - PipelineCallback* error_callback, - PipelineCallback* network_callback); + virtual void Init(PipelineStatusCallback* ended_callback, + PipelineStatusCallback* error_callback, + PipelineStatusCallback* network_callback); virtual bool Start(FilterCollection* filter_collection, const std::string& uri, - PipelineCallback* start_callback); - virtual void Stop(PipelineCallback* stop_callback); - virtual void Seek(base::TimeDelta time, PipelineCallback* seek_callback); + PipelineStatusCallback* start_callback); + virtual void Stop(PipelineStatusCallback* stop_callback); + virtual void Seek(base::TimeDelta time, + PipelineStatusCallback* seek_callback); virtual bool IsRunning() const; virtual bool IsInitialized() const; virtual bool IsNetworkActive() const; @@ -92,7 +122,6 @@ class PipelineImpl : public Pipeline, public FilterHost { virtual void GetVideoSize(size_t* width_out, size_t* height_out) const; virtual bool IsStreaming() const; virtual bool IsLoaded() const; - virtual PipelineError GetError() const; virtual PipelineStatistics GetStatistics() const; void SetClockForTesting(Clock* clock); @@ -155,7 +184,7 @@ class PipelineImpl : public Pipeline, public FilterHost { State FindNextState(State current); // FilterHost implementation. - virtual void SetError(PipelineError error); + virtual void SetError(PipelineStatus error); virtual base::TimeDelta GetTime() const; virtual base::TimeDelta GetDuration() const; virtual void SetTime(base::TimeDelta time); @@ -190,7 +219,7 @@ class PipelineImpl : public Pipeline, public FilterHost { // message loop. void StartTask(FilterCollection* filter_collection, const std::string& url, - PipelineCallback* start_callback); + PipelineStatusCallback* start_callback); // InitializeTask() performs initialization in multiple passes. It is executed // as a result of calling Start() or InitializationComplete() that advances @@ -199,11 +228,11 @@ class PipelineImpl : public Pipeline, public FilterHost { void InitializeTask(); // Stops and destroys all filters, placing the pipeline in the kStopped state. - void StopTask(PipelineCallback* stop_callback); + void StopTask(PipelineStatusCallback* stop_callback); // Carries out stopping and destroying all filters, placing the pipeline in // the kError state. - void ErrorChangedTask(PipelineError error); + void ErrorChangedTask(PipelineStatus error); // Carries out notifying filters that the playback rate has changed. void PlaybackRateChangedTask(float playback_rate); @@ -212,7 +241,7 @@ class PipelineImpl : public Pipeline, public FilterHost { void VolumeChangedTask(float volume); // Carries out notifying filters that we are seeking to a new timestamp. - void SeekTask(base::TimeDelta time, PipelineCallback* seek_callback); + void SeekTask(base::TimeDelta time, PipelineStatusCallback* seek_callback); // Carries out handling a notification from a filter that it has ended. void NotifyEndedTask(); @@ -245,7 +274,7 @@ class PipelineImpl : public Pipeline, public FilterHost { // The following initialize methods are used to select a specific type of // Filter object from FilterCollection and initialize it asynchronously. void InitializeDemuxer(); - void OnDemuxerBuilt(PipelineError error, Demuxer* demuxer); + void OnDemuxerBuilt(PipelineStatus status, Demuxer* demuxer); // Returns true if the asynchronous action of creating decoder has started. // Returns false if this method did nothing because the corresponding @@ -351,7 +380,7 @@ class PipelineImpl : public Pipeline, public FilterHost { // the pipeline is operating correctly. Any other value indicates that the // pipeline is stopped or is stopping. Clients can call the Stop() method to // reset the pipeline state, and restore this to PIPELINE_OK. - PipelineError error_; + PipelineStatus status_; // Whether the media contains rendered audio and video streams. bool has_audio_; @@ -386,11 +415,11 @@ class PipelineImpl : public Pipeline, public FilterHost { std::string url_; // Callbacks for various pipeline operations. - scoped_ptr<PipelineCallback> seek_callback_; - scoped_ptr<PipelineCallback> stop_callback_; - scoped_ptr<PipelineCallback> ended_callback_; - scoped_ptr<PipelineCallback> error_callback_; - scoped_ptr<PipelineCallback> network_callback_; + scoped_ptr<PipelineStatusCallback> seek_callback_; + scoped_ptr<PipelineStatusCallback> stop_callback_; + scoped_ptr<PipelineStatusCallback> ended_callback_; + scoped_ptr<PipelineStatusCallback> error_callback_; + scoped_ptr<PipelineStatusCallback> network_callback_; // Reference to the filter(s) that constitute the pipeline. scoped_refptr<Filter> pipeline_filter_; diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc index ce4974f..980fa8d 100644 --- a/media/base/pipeline_impl_unittest.cc +++ b/media/base/pipeline_impl_unittest.cc @@ -6,6 +6,7 @@ #include "base/callback.h" #include "base/stl_util-inl.h" +#include "base/threading/simple_thread.h" #include "media/base/pipeline_impl.h" #include "media/base/media_format.h" #include "media/base/filters.h" @@ -39,11 +40,11 @@ class CallbackHelper { CallbackHelper() {} virtual ~CallbackHelper() {} - MOCK_METHOD0(OnStart, void()); - MOCK_METHOD0(OnSeek, void()); - MOCK_METHOD0(OnStop, void()); - MOCK_METHOD0(OnEnded, void()); - MOCK_METHOD0(OnError, void()); + MOCK_METHOD1(OnStart, void(PipelineStatus)); + MOCK_METHOD1(OnSeek, void(PipelineStatus)); + MOCK_METHOD1(OnStop, void(PipelineStatus)); + MOCK_METHOD1(OnEnded, void(PipelineStatus)); + MOCK_METHOD1(OnError, void(PipelineStatus)); private: DISALLOW_COPY_AND_ASSIGN(CallbackHelper); @@ -64,7 +65,7 @@ class PipelineImplTest : public ::testing::Test { &CallbackHelper::OnEnded), NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnError), - NULL); + static_cast<PipelineStatusCallback*>(NULL)); mocks_.reset(new MockFilterCollection()); } @@ -74,7 +75,7 @@ class PipelineImplTest : public ::testing::Test { } // Expect a stop callback if we were started. - EXPECT_CALL(callbacks_, OnStop()); + EXPECT_CALL(callbacks_, OnStop(PIPELINE_OK)); pipeline_->Stop(NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnStop)); message_loop_.RunAllPending(); @@ -174,12 +175,17 @@ class PipelineImplTest : public ::testing::Test { void InitializePipeline() { InitializePipeline(PIPELINE_OK); } - - void InitializePipeline(PipelineError factory_error) { + // Most tests can expect the |filter_collection|'s |build_status| to get + // reflected in |Start()|'s argument. + void InitializePipeline(PipelineStatus start_status) { + InitializePipeline(start_status, start_status); + } + // But some tests require different statuses in build & Start. + void InitializePipeline(PipelineStatus build_status, + PipelineStatus start_status) { // Expect an initialization callback. - EXPECT_CALL(callbacks_, OnStart()); - pipeline_->Start(mocks_->filter_collection(true, true, factory_error), - "", + EXPECT_CALL(callbacks_, OnStart(start_status)); + pipeline_->Start(mocks_->filter_collection(true, true, build_status), "", NewCallback(reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnStart)); message_loop_.RunAllPending(); @@ -221,7 +227,7 @@ class PipelineImplTest : public ::testing::Test { } // We expect a successful seek callback. - EXPECT_CALL(callbacks_, OnSeek()); + EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK)); } @@ -293,8 +299,6 @@ TEST_F(PipelineImplTest, NotStarted) { pipeline_->GetVideoSize(&width, &height); EXPECT_EQ(0u, width); EXPECT_EQ(0u, height); - - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); } TEST_F(PipelineImplTest, NeverInitializes) { @@ -307,22 +311,21 @@ TEST_F(PipelineImplTest, NeverInitializes) { message_loop_.RunAllPending(); EXPECT_FALSE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); // Because our callback will get executed when the test tears down, we'll // verify that nothing has been called, then set our expectation for the call // made during tear down. Mock::VerifyAndClear(&callbacks_); - EXPECT_CALL(callbacks_, OnStart()); + EXPECT_CALL(callbacks_, OnStart(PIPELINE_OK)); } TEST_F(PipelineImplTest, RequiredFilterMissing) { - EXPECT_CALL(callbacks_, OnError()); + EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING)); // Sets up expectations on the callback and initializes the pipeline. Called // after tests have set expectations any filters they wish to use. // Expect an initialization callback. - EXPECT_CALL(callbacks_, OnStart()); + EXPECT_CALL(callbacks_, OnStart(PIPELINE_ERROR_REQUIRED_FILTER_MISSING)); // Create a filter collection with missing filter. FilterCollection* collection = @@ -334,17 +337,17 @@ TEST_F(PipelineImplTest, RequiredFilterMissing) { message_loop_.RunAllPending(); EXPECT_FALSE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_ERROR_REQUIRED_FILTER_MISSING, - pipeline_->GetError()); } TEST_F(PipelineImplTest, URLNotFound) { - - EXPECT_CALL(callbacks_, OnError()); - + // TODO(acolwell,fischman): Since OnStart() is getting called with an error + // code already, OnError() doesn't also need to get called. Fix the pipeline + // (and it's consumers!) so that OnError doesn't need to be called after + // another callback has already reported the error. Same applies to NoStreams + // below. + EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_URL_NOT_FOUND)); InitializePipeline(PIPELINE_ERROR_URL_NOT_FOUND); EXPECT_FALSE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_ERROR_URL_NOT_FOUND, pipeline_->GetError()); } TEST_F(PipelineImplTest, NoStreams) { @@ -354,11 +357,11 @@ TEST_F(PipelineImplTest, NoStreams) { .WillRepeatedly(Return(0)); EXPECT_CALL(*mocks_->demuxer(), Stop(NotNull())) .WillOnce(Invoke(&RunStopFilterCallback)); - EXPECT_CALL(callbacks_, OnError()); + // TODO(acolwell,fischman): see TODO in URLNotFound above. + EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_COULD_NOT_RENDER)); - InitializePipeline(); + InitializePipeline(PIPELINE_OK, PIPELINE_ERROR_COULD_NOT_RENDER); EXPECT_FALSE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_ERROR_COULD_NOT_RENDER, pipeline_->GetError()); } TEST_F(PipelineImplTest, AudioStream) { @@ -370,9 +373,8 @@ TEST_F(PipelineImplTest, AudioStream) { InitializeAudioDecoder(audio_stream()); InitializeAudioRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_TRUE(pipeline_->HasAudio()); EXPECT_FALSE(pipeline_->HasVideo()); } @@ -386,9 +388,8 @@ TEST_F(PipelineImplTest, VideoStream) { InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_FALSE(pipeline_->HasAudio()); EXPECT_TRUE(pipeline_->HasVideo()); } @@ -406,9 +407,8 @@ TEST_F(PipelineImplTest, AudioVideoStream) { InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_TRUE(pipeline_->HasAudio()); EXPECT_TRUE(pipeline_->HasVideo()); } @@ -431,7 +431,7 @@ TEST_F(PipelineImplTest, Seek) { ExpectSeek(expected); // Initialize then seek! - InitializePipeline(); + InitializePipeline(PIPELINE_OK); DoSeek(expected); } @@ -449,7 +449,7 @@ TEST_F(PipelineImplTest, SetVolume) { EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(expected)); // Initialize then set volume! - InitializePipeline(); + InitializePipeline(PIPELINE_OK); pipeline_->SetVolume(expected); } @@ -463,9 +463,8 @@ TEST_F(PipelineImplTest, Properties) { InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_EQ(kDuration.ToInternalValue(), pipeline_->GetMediaDuration().ToInternalValue()); EXPECT_EQ(kTotalBytes, pipeline_->GetTotalBytes()); @@ -487,9 +486,8 @@ TEST_F(PipelineImplTest, GetBufferedTime) { InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); // TODO(vrk): The following mini-test cases are order-dependent, and should // probably be separated into independent test cases. @@ -557,9 +555,8 @@ TEST_F(PipelineImplTest, DisableAudioRenderer) { InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_TRUE(pipeline_->HasAudio()); EXPECT_TRUE(pipeline_->HasVideo()); @@ -581,7 +578,7 @@ TEST_F(PipelineImplTest, DisableAudioRenderer) { // Verify that ended event is fired when video ends. EXPECT_CALL(*mocks_->video_renderer(), HasEnded()) .WillOnce(Return(true)); - EXPECT_CALL(callbacks_, OnEnded()); + EXPECT_CALL(callbacks_, OnEnded(PIPELINE_OK)); FilterHost* host = pipeline_; host->NotifyEnded(); } @@ -610,16 +607,15 @@ TEST_F(PipelineImplTest, DisableAudioRendererDuringInit) { EXPECT_CALL(*mocks_->video_renderer(), OnAudioRendererDisabled()); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); EXPECT_TRUE(pipeline_->IsInitialized()); - EXPECT_EQ(PIPELINE_OK, pipeline_->GetError()); EXPECT_FALSE(pipeline_->HasAudio()); EXPECT_TRUE(pipeline_->HasVideo()); // Verify that ended event is fired when video ends. EXPECT_CALL(*mocks_->video_renderer(), HasEnded()) .WillOnce(Return(true)); - EXPECT_CALL(callbacks_, OnEnded()); + EXPECT_CALL(callbacks_, OnEnded(PIPELINE_OK)); FilterHost* host = pipeline_; host->NotifyEnded(); } @@ -636,7 +632,7 @@ TEST_F(PipelineImplTest, EndedCallback) { InitializeAudioRenderer(); InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); // For convenience to simulate filters calling the methods. FilterHost* host = pipeline_; @@ -657,7 +653,7 @@ TEST_F(PipelineImplTest, EndedCallback) { .WillOnce(Return(true)); EXPECT_CALL(*mocks_->video_renderer(), HasEnded()) .WillOnce(Return(true)); - EXPECT_CALL(callbacks_, OnEnded()); + EXPECT_CALL(callbacks_, OnEnded(PIPELINE_OK)); host->NotifyEnded(); } @@ -681,7 +677,7 @@ TEST_F(PipelineImplTest, AudioStreamShorterThanVideo) { InitializeAudioRenderer(); InitializeVideoDecoder(video_stream()); InitializeVideoRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); // For convenience to simulate filters calling the methods. FilterHost* host = pipeline_; @@ -729,7 +725,7 @@ TEST_F(PipelineImplTest, AudioStreamShorterThanVideo) { .WillOnce(Return(true)); EXPECT_CALL(*mocks_->video_renderer(), HasEnded()) .WillOnce(Return(true)); - EXPECT_CALL(callbacks_, OnEnded()); + EXPECT_CALL(callbacks_, OnEnded(PIPELINE_OK)); host->NotifyEnded(); } @@ -741,7 +737,7 @@ TEST_F(PipelineImplTest, ErrorDuringSeek) { InitializeDemuxer(&streams, base::TimeDelta::FromSeconds(10)); InitializeAudioDecoder(audio_stream()); InitializeAudioRenderer(); - InitializePipeline(); + InitializePipeline(PIPELINE_OK); float playback_rate = 1.0f; EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(playback_rate)); @@ -759,9 +755,62 @@ TEST_F(PipelineImplTest, ErrorDuringSeek) { PIPELINE_ERROR_READ), Invoke(&RunFilterCallback))); - pipeline_->Seek(seek_time, NewExpectedCallback()); - EXPECT_CALL(callbacks_, OnError()); + pipeline_->Seek(seek_time, NewCallback( + reinterpret_cast<CallbackHelper*>(&callbacks_), &CallbackHelper::OnSeek)); + EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ)); + EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ)); message_loop_.RunAllPending(); } +class FlexibleCallbackRunner : public base::DelegateSimpleThread::Delegate { + public: + FlexibleCallbackRunner(int delayInMs, PipelineStatus status, + PipelineStatusCallback* callback) + : delayInMs_(delayInMs), status_(status), callback_(callback) { + if (delayInMs_ < 0) { + callback_->Run(status_); + return; + } + } + virtual void Run() { + if (delayInMs_ < 0) return; + base::PlatformThread::Sleep(delayInMs_); + callback_->Run(status_); + } + + private: + int delayInMs_; + PipelineStatus status_; + PipelineStatusCallback* callback_; +}; + +void TestPipelineStatusNotification(int delayInMs) { + PipelineStatusNotification note; + // Arbitrary error value we expect to fish out of the notification after the + // callback is fired. + const PipelineStatus expected_error = PIPELINE_ERROR_URL_NOT_FOUND; + FlexibleCallbackRunner runner(delayInMs, expected_error, note.Callback()); + base::DelegateSimpleThread thread(&runner, "FlexibleCallbackRunner"); + thread.Start(); + note.Wait(); + EXPECT_EQ(note.status(), expected_error); + thread.Join(); +} + +// Test that in-line callback (same thread, no yield) works correctly. +TEST(PipelineStatusNotificationTest, InlineCallback) { + TestPipelineStatusNotification(-1); +} + +// Test that different-thread, no-delay callback works correctly. +TEST(PipelineStatusNotificationTest, ImmediateCallback) { + TestPipelineStatusNotification(0); +} + +// Test that different-thread, some-delay callback (the expected common case) +// works correctly. +TEST(PipelineStatusNotificationTest, DelayedCallback) { + TestPipelineStatusNotification(20); +} + } // namespace media diff --git a/media/base/pipeline_status.h b/media/base/pipeline_status.h index 787708e..84c8d4f 100644 --- a/media/base/pipeline_status.h +++ b/media/base/pipeline_status.h @@ -9,9 +9,8 @@ namespace media { -// Error definitions for pipeline. All codes indicate an error except: -// PIPELINE_OK indicates the pipeline is running normally. -enum PipelineError { +// Status states for pipeline. All codes except PIPELINE_OK indicate errors. +enum PipelineStatus { PIPELINE_OK, PIPELINE_ERROR_URL_NOT_FOUND, PIPELINE_ERROR_NETWORK, @@ -34,7 +33,7 @@ enum PipelineError { DATASOURCE_ERROR_URL_NOT_SUPPORTED, }; -typedef Callback1<media::PipelineError>::Type PipelineStatusCallback; +typedef Callback1<media::PipelineStatus>::Type PipelineStatusCallback; } // namespace media diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h index e47be5e..3816a01 100644 --- a/media/filters/ffmpeg_demuxer.h +++ b/media/filters/ffmpeg_demuxer.h @@ -234,7 +234,7 @@ class FFmpegDemuxer : public Demuxer, // Initialization can happen before set_host() is called, in which case we // store these bits for deferred reporting to the FilterHost when we get one. base::TimeDelta max_duration_; - PipelineError deferred_status_; + PipelineStatus deferred_status_; DISALLOW_COPY_AND_ASSIGN(FFmpegDemuxer); }; diff --git a/media/filters/ffmpeg_demuxer_factory.cc b/media/filters/ffmpeg_demuxer_factory.cc index add2c54..481f6ee 100644 --- a/media/filters/ffmpeg_demuxer_factory.cc +++ b/media/filters/ffmpeg_demuxer_factory.cc @@ -31,7 +31,7 @@ class DemuxerCallbackAsPipelineStatusCallback : public PipelineStatusCallback { virtual ~DemuxerCallbackAsPipelineStatusCallback() {} - virtual void RunWithParams(const Tuple1<PipelineError>& params) { + virtual void RunWithParams(const Tuple1<PipelineStatus>& params) { cb_->Run(params.a, demuxer_); } @@ -54,8 +54,9 @@ class DemuxerCallbackAsDataSourceCallback : virtual ~DemuxerCallbackAsDataSourceCallback() {} - virtual void RunWithParams(const Tuple2<PipelineError, DataSource*>& params) { - PipelineError status = params.a; + virtual void RunWithParams( + const Tuple2<PipelineStatus, DataSource*>& params) { + PipelineStatus status = params.a; DataSource* data_source = params.b; if (status != PIPELINE_OK) { cb_->Run(status, static_cast<Demuxer*>(NULL)); diff --git a/media/filters/file_data_source.cc b/media/filters/file_data_source.cc index 8a1f055..98a434d 100644 --- a/media/filters/file_data_source.cc +++ b/media/filters/file_data_source.cc @@ -23,7 +23,7 @@ FileDataSource::~FileDataSource() { DCHECK(!file_); } -PipelineError FileDataSource::Initialize(const std::string& url) { +PipelineStatus FileDataSource::Initialize(const std::string& url) { DCHECK(!file_); #if defined(OS_WIN) FilePath file_path(UTF8ToWide(url)); diff --git a/media/filters/file_data_source.h b/media/filters/file_data_source.h index 5ae3956..0539a36 100644 --- a/media/filters/file_data_source.h +++ b/media/filters/file_data_source.h @@ -20,7 +20,7 @@ class FileDataSource : public DataSource { FileDataSource(); virtual ~FileDataSource(); - PipelineError Initialize(const std::string& url); + PipelineStatus Initialize(const std::string& url); // Implementation of Filter. virtual void set_host(FilterHost* filter_host); diff --git a/media/filters/file_data_source_factory.cc b/media/filters/file_data_source_factory.cc index 80642fb..823fa01 100644 --- a/media/filters/file_data_source_factory.cc +++ b/media/filters/file_data_source_factory.cc @@ -26,10 +26,10 @@ void FileDataSourceFactory::Build(const std::string& url, scoped_refptr<FileDataSource> file_data_source = new FileDataSource(); - PipelineError error = file_data_source->Initialize(url); + PipelineStatus status = file_data_source->Initialize(url); DataSource* data_source = - (error == PIPELINE_OK) ? file_data_source.get() : NULL; - callback->Run(error, data_source); + (status == PIPELINE_OK) ? file_data_source.get() : NULL; + callback->Run(status, data_source); delete callback; } diff --git a/media/tools/player_wtl/movie.cc b/media/tools/player_wtl/movie.cc index 042d1f4..2c705ef 100644 --- a/media/tools/player_wtl/movie.cc +++ b/media/tools/player_wtl/movie.cc @@ -84,14 +84,13 @@ bool Movie::Open(const wchar_t* url, WtlVideoRenderer* video_renderer) { collection->AddVideoRenderer(video_renderer); // Create and start our pipeline. - pipeline_->Start(collection.release(), WideToUTF8(std::wstring(url)), NULL); - while (true) { - base::PlatformThread::Sleep(100); - if (pipeline_->IsInitialized()) - break; - if (pipeline_->GetError() != media::PIPELINE_OK) - return false; - } + media::PipelineStatusNotification note; + pipeline_->Start(collection.release(), WideToUTF8(std::wstring(url)), + note.Callback()); + // Wait until the pipeline is fully initialized. + note.Wait(); + if (note.status() != PIPELINE_OK) + return false; pipeline_->SetPlaybackRate(play_rate_); return true; } diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc index 84e08e6..ace2ba6 100644 --- a/media/tools/player_x11/player_x11.cc +++ b/media/tools/player_x11/player_x11.cc @@ -52,9 +52,17 @@ Display* g_display = NULL; Window g_window = 0; bool g_running = false; -void Quit(MessageLoop* message_loop) { - message_loop->PostTask(FROM_HERE, new MessageLoop::QuitTask()); -} +class MessageLoopQuitter { + public: + explicit MessageLoopQuitter(MessageLoop* loop) : loop_(loop) {} + void Quit(media::PipelineStatus status) { + loop_->PostTask(FROM_HERE, new MessageLoop::QuitTask()); + delete this; + } + private: + MessageLoop* loop_; + DISALLOW_COPY_AND_ASSIGN(MessageLoopQuitter); +}; // Initialize X11. Returns true if successful. This method creates the X11 // window. Further initialization is done in X11VideoRenderer. @@ -126,23 +134,20 @@ bool InitPipeline(MessageLoop* message_loop, else collection->AddAudioRenderer(new media::NullAudioRenderer()); - // Create and start the pipeline. + // Create the pipeline and start it. *pipeline = new media::PipelineImpl(message_loop); - (*pipeline)->Start(collection.release(), filename, NULL); + media::PipelineStatusNotification note; + (*pipeline)->Start(collection.release(), filename, note.Callback()); // Wait until the pipeline is fully initialized. - while (true) { - base::PlatformThread::Sleep(100); - if ((*pipeline)->IsInitialized()) - break; - if ((*pipeline)->GetError() != media::PIPELINE_OK) { - std::cout << "InitPipeline: " << (*pipeline)->GetError() << std::endl; - (*pipeline)->Stop(NULL); - return false; - } + note.Wait(); + if (note.status() != media::PIPELINE_OK) { + std::cout << "InitPipeline: " << note.status() << std::endl; + (*pipeline)->Stop(NULL); + return false; } - // And starts the playback. + // And start the playback. (*pipeline)->SetPlaybackRate(1.0f); return true; } @@ -156,10 +161,10 @@ void PeriodicalUpdate( MessageLoop* message_loop, bool audio_only) { if (!g_running) { - // interrupt signal is received during lat time period. + // interrupt signal was received during last time period. // Quit message_loop only when pipeline is fully stopped. - pipeline->Stop(media::TaskToCallbackAdapter::NewCallback( - NewRunnableFunction(Quit, message_loop))); + MessageLoopQuitter* quitter = new MessageLoopQuitter(message_loop); + pipeline->Stop(NewCallback(quitter, &MessageLoopQuitter::Quit)); return; } @@ -199,8 +204,8 @@ void PeriodicalUpdate( if (key == XK_Escape) { g_running = false; // Quit message_loop only when pipeline is fully stopped. - pipeline->Stop(media::TaskToCallbackAdapter::NewCallback( - NewRunnableFunction(Quit, message_loop))); + MessageLoopQuitter* quitter = new MessageLoopQuitter(message_loop); + pipeline->Stop(NewCallback(quitter, &MessageLoopQuitter::Quit)); return; } else if (key == XK_space) { if (pipeline->GetPlaybackRate() < 0.01f) // paused diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc index 135c49e..2622255 100644 --- a/webkit/glue/media/buffered_data_source.cc +++ b/webkit/glue/media/buffered_data_source.cc @@ -412,14 +412,15 @@ void BufferedDataSource::DoneRead_Locked(int error) { read_buffer_ = 0; } -void BufferedDataSource::DoneInitialization_Locked(media::PipelineError error) { +void BufferedDataSource::DoneInitialization_Locked( + media::PipelineStatus status) { DCHECK(MessageLoop::current() == render_loop_); DCHECK(initialize_callback_.get()); lock_.AssertAcquired(); scoped_ptr<media::PipelineStatusCallback> initialize_callback( initialize_callback_.release()); - initialize_callback->Run(error); + initialize_callback->Run(status); } ///////////////////////////////////////////////////////////////////////////// diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h index 9e043b0..5ab3035 100644 --- a/webkit/glue/media/buffered_data_source.h +++ b/webkit/glue/media/buffered_data_source.h @@ -98,7 +98,7 @@ class BufferedDataSource : public WebDataSource { void DoneRead_Locked(int error); // Calls |initialize_callback_| and reset it. - void DoneInitialization_Locked(media::PipelineError error); + void DoneInitialization_Locked(media::PipelineStatus status); // Callback method for |loader_| if URL for the resource requested is using // HTTP protocol. This method is called when response for initial request is diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc index 9763678..dd5c891 100644 --- a/webkit/glue/media/buffered_data_source_unittest.cc +++ b/webkit/glue/media/buffered_data_source_unittest.cc @@ -168,7 +168,7 @@ class BufferedDataSourceTest : public testing::Test { .WillByDefault(Return(partial_response)); ON_CALL(*loader_, url()) .WillByDefault(ReturnRef(gurl_)); - media::PipelineError expected_init_error = media::PIPELINE_OK; + media::PipelineStatus expected_init_status = media::PIPELINE_OK; if (initialized_ok) { // Expected loaded or not. EXPECT_CALL(host_, SetLoaded(loaded)); @@ -184,13 +184,13 @@ class BufferedDataSourceTest : public testing::Test { EXPECT_CALL(host_, SetStreaming(true)); } } else { - expected_init_error = media::PIPELINE_ERROR_NETWORK; + expected_init_status = media::PIPELINE_ERROR_NETWORK; EXPECT_CALL(*loader_, Stop()); } // Actual initialization of the data source. data_source_->Initialize(url, - media::NewExpectedStatusCallback(expected_init_error)); + media::NewExpectedStatusCallback(expected_init_status)); message_loop_->RunAllPending(); if (initialized_ok) { diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc index ca258d4..186f6d7 100644 --- a/webkit/glue/media/simple_data_source.cc +++ b/webkit/glue/media/simple_data_source.cc @@ -321,12 +321,12 @@ void SimpleDataSource::CancelTask() { void SimpleDataSource::DoneInitialization_Locked(bool success) { lock_.AssertAcquired(); - media::PipelineError error = media::PIPELINE_ERROR_NETWORK; + media::PipelineStatus status = media::PIPELINE_ERROR_NETWORK; if (success) { state_ = INITIALIZED; UpdateHostState(); - error = media::PIPELINE_OK; + status = media::PIPELINE_OK; } else { state_ = UNINITIALIZED; url_loader_.reset(); @@ -334,7 +334,7 @@ void SimpleDataSource::DoneInitialization_Locked(bool success) { scoped_ptr<media::PipelineStatusCallback> initialize_callback( initialize_callback_.release()); - initialize_callback->Run(error); + initialize_callback->Run(status); } void SimpleDataSource::UpdateHostState() { diff --git a/webkit/glue/media/web_data_source_factory.cc b/webkit/glue/media/web_data_source_factory.cc index f9036c7..d98e2da 100644 --- a/webkit/glue/media/web_data_source_factory.cc +++ b/webkit/glue/media/web_data_source_factory.cc @@ -21,7 +21,7 @@ class WebDataSourceFactory::BuildRequest virtual void DoStart(); private: - void InitDone(media::PipelineError error); + void InitDone(media::PipelineStatus status); scoped_refptr<WebDataSource> data_source_; WebDataSourceBuildObserverHack* build_observer_; @@ -84,17 +84,18 @@ void WebDataSourceFactory::BuildRequest::DoStart() { data_source_->Initialize(url(), NewCallback(this, &BuildRequest::InitDone)); } -void WebDataSourceFactory::BuildRequest::InitDone(media::PipelineError error) { +void WebDataSourceFactory::BuildRequest::InitDone( + media::PipelineStatus status) { scoped_refptr<WebDataSource> data_source; - data_source = (error == media::PIPELINE_OK) ? data_source_ : NULL; + data_source = (status == media::PIPELINE_OK) ? data_source_ : NULL; data_source_ = NULL; if (build_observer_ && data_source.get()) { build_observer_->Run(data_source.get()); } - RequestComplete(error, data_source); + RequestComplete(status, data_source); // Don't do anything after this line. This object is deleted by // RequestComplete(). } diff --git a/webkit/glue/webmediaplayer_impl.cc b/webkit/glue/webmediaplayer_impl.cc index 098cd61..61bb454 100644 --- a/webkit/glue/webmediaplayer_impl.cc +++ b/webkit/glue/webmediaplayer_impl.cc @@ -33,6 +33,7 @@ using WebKit::WebCanvas; using WebKit::WebRect; using WebKit::WebSize; +using media::PipelineStatus; namespace { @@ -171,29 +172,31 @@ void WebMediaPlayerImpl::Proxy::Detach() { } } -void WebMediaPlayerImpl::Proxy::PipelineInitializationCallback() { - render_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &WebMediaPlayerImpl::Proxy::PipelineInitializationTask)); +void WebMediaPlayerImpl::Proxy::PipelineInitializationCallback( + PipelineStatus status) { + render_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, &WebMediaPlayerImpl::Proxy::PipelineInitializationTask, status)); } -void WebMediaPlayerImpl::Proxy::PipelineSeekCallback() { - render_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &WebMediaPlayerImpl::Proxy::PipelineSeekTask)); +void WebMediaPlayerImpl::Proxy::PipelineSeekCallback(PipelineStatus status) { + render_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, &WebMediaPlayerImpl::Proxy::PipelineSeekTask, status)); } -void WebMediaPlayerImpl::Proxy::PipelineEndedCallback() { - render_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &WebMediaPlayerImpl::Proxy::PipelineEndedTask)); +void WebMediaPlayerImpl::Proxy::PipelineEndedCallback(PipelineStatus status) { + render_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, &WebMediaPlayerImpl::Proxy::PipelineEndedTask, status)); } -void WebMediaPlayerImpl::Proxy::PipelineErrorCallback() { - render_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &WebMediaPlayerImpl::Proxy::PipelineErrorTask)); +void WebMediaPlayerImpl::Proxy::PipelineErrorCallback(PipelineStatus error) { + DCHECK_NE(error, media::PIPELINE_OK); + render_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, &WebMediaPlayerImpl::Proxy::PipelineErrorTask, error)); } -void WebMediaPlayerImpl::Proxy::NetworkEventCallback() { - render_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &WebMediaPlayerImpl::Proxy::NetworkEventTask)); +void WebMediaPlayerImpl::Proxy::NetworkEventCallback(PipelineStatus status) { + render_loop_->PostTask(FROM_HERE, NewRunnableMethod( + this, &WebMediaPlayerImpl::Proxy::NetworkEventTask, status)); } void WebMediaPlayerImpl::Proxy::AddDataSource(WebDataSource* data_source) { @@ -213,38 +216,39 @@ void WebMediaPlayerImpl::Proxy::RepaintTask() { } } -void WebMediaPlayerImpl::Proxy::PipelineInitializationTask() { +void WebMediaPlayerImpl::Proxy::PipelineInitializationTask( + PipelineStatus status) { DCHECK(MessageLoop::current() == render_loop_); if (webmediaplayer_) { - webmediaplayer_->OnPipelineInitialize(); + webmediaplayer_->OnPipelineInitialize(status); } } -void WebMediaPlayerImpl::Proxy::PipelineSeekTask() { +void WebMediaPlayerImpl::Proxy::PipelineSeekTask(PipelineStatus status) { DCHECK(MessageLoop::current() == render_loop_); if (webmediaplayer_) { - webmediaplayer_->OnPipelineSeek(); + webmediaplayer_->OnPipelineSeek(status); } } -void WebMediaPlayerImpl::Proxy::PipelineEndedTask() { +void WebMediaPlayerImpl::Proxy::PipelineEndedTask(PipelineStatus status) { DCHECK(MessageLoop::current() == render_loop_); if (webmediaplayer_) { - webmediaplayer_->OnPipelineEnded(); + webmediaplayer_->OnPipelineEnded(status); } } -void WebMediaPlayerImpl::Proxy::PipelineErrorTask() { +void WebMediaPlayerImpl::Proxy::PipelineErrorTask(PipelineStatus error) { DCHECK(MessageLoop::current() == render_loop_); if (webmediaplayer_) { - webmediaplayer_->OnPipelineError(); + webmediaplayer_->OnPipelineError(error); } } -void WebMediaPlayerImpl::Proxy::NetworkEventTask() { +void WebMediaPlayerImpl::Proxy::NetworkEventTask(PipelineStatus status) { DCHECK(MessageLoop::current() == render_loop_); if (webmediaplayer_) { - webmediaplayer_->OnNetworkEvent(); + webmediaplayer_->OnNetworkEvent(status); } } @@ -744,9 +748,9 @@ void WebMediaPlayerImpl::Repaint() { GetClient()->repaint(); } -void WebMediaPlayerImpl::OnPipelineInitialize() { +void WebMediaPlayerImpl::OnPipelineInitialize(PipelineStatus status) { DCHECK(MessageLoop::current() == main_loop_); - if (pipeline_->GetError() == media::PIPELINE_OK) { + if (status == media::PIPELINE_OK) { // Only keep one time range starting from 0. WebKit::WebTimeRanges new_buffered(static_cast<size_t>(1)); new_buffered[0].start = 0.0f; @@ -763,7 +767,7 @@ void WebMediaPlayerImpl::OnPipelineInitialize() { SetNetworkState(WebKit::WebMediaPlayer::Loaded); } } else { - // TODO(hclam): should use pipeline_->GetError() to determine the state + // TODO(hclam): should use |status| to determine the state // properly and reports error using MediaError. // WebKit uses FormatError to indicate an error for bogus URL or bad file. // Since we are at the initialization stage we can safely treat every error @@ -775,9 +779,9 @@ void WebMediaPlayerImpl::OnPipelineInitialize() { Repaint(); } -void WebMediaPlayerImpl::OnPipelineSeek() { +void WebMediaPlayerImpl::OnPipelineSeek(PipelineStatus status) { DCHECK(MessageLoop::current() == main_loop_); - if (pipeline_->GetError() == media::PIPELINE_OK) { + if (status == media::PIPELINE_OK) { // Update our paused time. if (paused_) { paused_time_ = pipeline_->GetCurrentTime(); @@ -789,17 +793,20 @@ void WebMediaPlayerImpl::OnPipelineSeek() { } } -void WebMediaPlayerImpl::OnPipelineEnded() { +void WebMediaPlayerImpl::OnPipelineEnded(PipelineStatus status) { DCHECK(MessageLoop::current() == main_loop_); - if (pipeline_->GetError() == media::PIPELINE_OK) { + if (status == media::PIPELINE_OK) { GetClient()->timeChanged(); } } -void WebMediaPlayerImpl::OnPipelineError() { +void WebMediaPlayerImpl::OnPipelineError(PipelineStatus error) { DCHECK(MessageLoop::current() == main_loop_); - switch (pipeline_->GetError()) { + switch (error) { case media::PIPELINE_OK: + LOG(DFATAL) << "PIPELINE_OK isn't an error!"; + break; + case media::PIPELINE_ERROR_INITIALIZATION_FAILED: case media::PIPELINE_ERROR_REQUIRED_FILTER_MISSING: case media::PIPELINE_ERROR_COULD_NOT_RENDER: @@ -830,9 +837,9 @@ void WebMediaPlayerImpl::OnPipelineError() { Repaint(); } -void WebMediaPlayerImpl::OnNetworkEvent() { +void WebMediaPlayerImpl::OnNetworkEvent(PipelineStatus status) { DCHECK(MessageLoop::current() == main_loop_); - if (pipeline_->GetError() == media::PIPELINE_OK) { + if (status == media::PIPELINE_OK) { if (pipeline_->IsNetworkActive()) { SetNetworkState(WebKit::WebMediaPlayer::Loading); } else { @@ -892,7 +899,7 @@ void WebMediaPlayerImpl::Destroy() { } } -void WebMediaPlayerImpl::PipelineStoppedCallback() { +void WebMediaPlayerImpl::PipelineStoppedCallback(PipelineStatus status) { pipeline_stopped_.Signal(); } diff --git a/webkit/glue/webmediaplayer_impl.h b/webkit/glue/webmediaplayer_impl.h index e7acf27..ef356b5 100644 --- a/webkit/glue/webmediaplayer_impl.h +++ b/webkit/glue/webmediaplayer_impl.h @@ -111,11 +111,11 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer, void AbortDataSources(); // Methods for PipelineImpl -> WebMediaPlayerImpl communication. - void PipelineInitializationCallback(); - void PipelineSeekCallback(); - void PipelineEndedCallback(); - void PipelineErrorCallback(); - void NetworkEventCallback(); + void PipelineInitializationCallback(media::PipelineStatus status); + void PipelineSeekCallback(media::PipelineStatus status); + void PipelineEndedCallback(media::PipelineStatus status); + void PipelineErrorCallback(media::PipelineStatus error); + void NetworkEventCallback(media::PipelineStatus status); // Returns the message loop used by the proxy. MessageLoop* message_loop() { return render_loop_; } @@ -132,19 +132,20 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer, void RepaintTask(); // Notify |webmediaplayer_| that initialization has finished. - void PipelineInitializationTask(); + void PipelineInitializationTask(media::PipelineStatus status); // Notify |webmediaplayer_| that a seek has finished. - void PipelineSeekTask(); + void PipelineSeekTask(media::PipelineStatus status); // Notify |webmediaplayer_| that the media has ended. - void PipelineEndedTask(); + void PipelineEndedTask(media::PipelineStatus status); - // Notify |webmediaplayer_| that a pipeline error has been set. - void PipelineErrorTask(); + // Notify |webmediaplayer_| that a pipeline error has occurred during + // playback. + void PipelineErrorTask(media::PipelineStatus error); // Notify |webmediaplayer_| that there's a network event. - void NetworkEventTask(); + void NetworkEventTask(media::PipelineStatus status); // The render message loop where WebKit lives. MessageLoop* render_loop_; @@ -261,15 +262,15 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer, void Repaint(); - void OnPipelineInitialize(); + void OnPipelineInitialize(media::PipelineStatus status); - void OnPipelineSeek(); + void OnPipelineSeek(media::PipelineStatus status); - void OnPipelineEnded(); + void OnPipelineEnded(media::PipelineStatus status); - void OnPipelineError(); + void OnPipelineError(media::PipelineStatus error); - void OnNetworkEvent(); + void OnNetworkEvent(media::PipelineStatus status); private: // Helpers that set the network/ready state and notifies the client if @@ -282,7 +283,7 @@ class WebMediaPlayerImpl : public WebKit::WebMediaPlayer, // Callback executed after |pipeline_| stops which signals Destroy() // to continue. - void PipelineStoppedCallback(); + void PipelineStoppedCallback(media::PipelineStatus status); // Getter method to |client_|. WebKit::WebMediaPlayerClient* GetClient(); |