From 58efa8aa5e845af2e3db840a8a654bd55fb98fb0 Mon Sep 17 00:00:00 2001 From: Frans Englich Date: Fri, 23 Oct 2009 11:52:05 +0200 Subject: Improve error handling. Errors reported via: * the DummyPlayer didn't work due to it not doing the usual state transitions/emission * MediaObject::setSource() due to errors being emitted before connections being set up. * A general state bug. Task-number: QTBUG-4752 Reviewed-by: Gareth Stockwell --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 36 +++++++++---------------- src/3rdparty/phonon/mmf/abstractmediaplayer.h | 4 --- src/3rdparty/phonon/mmf/abstractplayer.cpp | 32 +++++++++++++++++++--- src/3rdparty/phonon/mmf/abstractplayer.h | 9 +++++-- src/3rdparty/phonon/mmf/dummyplayer.cpp | 9 ------- src/3rdparty/phonon/mmf/dummyplayer.h | 4 --- src/3rdparty/phonon/mmf/mediaobject.cpp | 14 ++++++---- 7 files changed, 57 insertions(+), 51 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index af2c31e..998e861 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -359,26 +359,27 @@ qint64 MMF::AbstractMediaPlayer::toMilliSeconds(const TTimeIntervalMicroSeconds return in.Int64() / 1000; } +//----------------------------------------------------------------------------- +// Slots +//----------------------------------------------------------------------------- + +void MMF::AbstractMediaPlayer::tick() +{ + // For the MWC compiler, we need to qualify the base class. + emit MMF::AbstractPlayer::tick(currentTime()); +} + void MMF::AbstractMediaPlayer::changeState(PrivateState newState) { - TRACE_CONTEXT(AbstractPlayer::changeState, EAudioInternal); - TRACE_ENTRY("state %d newState %d", privateState(), newState); + TRACE_CONTEXT(AbstractMediaPlayer::changeState, EAudioInternal); // TODO: add some invariants to check that the transition is valid + AbstractPlayer::changeState(newState); const Phonon::State oldPhononState = phononState(privateState()); const Phonon::State newPhononState = phononState(newState); - if (oldPhononState != newPhononState) { - TRACE("emit stateChanged(%d, %d)", newPhononState, oldPhononState); - emit stateChanged(newPhononState, oldPhononState); - } - - setState(newState); - if ( - LoadingState == oldPhononState - and StoppedState == newPhononState - ) { + if (LoadingState == oldPhononState && StoppedState == newPhononState) { // Ensure initial volume is set on MMF API before starting playback doVolumeChanged(); @@ -391,17 +392,6 @@ void MMF::AbstractMediaPlayer::changeState(PrivateState newState) } } - TRACE_EXIT_0(); -} - -//----------------------------------------------------------------------------- -// Slots -//----------------------------------------------------------------------------- - -void MMF::AbstractMediaPlayer::tick() -{ - // For the MWC compiler, we need to qualify the base class. - emit MMF::AbstractPlayer::tick(currentTime()); } QT_END_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.h b/src/3rdparty/phonon/mmf/abstractmediaplayer.h index 698b899..1ea236b 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.h +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.h @@ -71,10 +71,6 @@ protected: virtual int setDeviceVolume(int mmfVolume) = 0; virtual int openFile(RFile& file) = 0; virtual void close() = 0; - - /** - * Changes state and emits stateChanged() - */ virtual void changeState(PrivateState newState); protected: diff --git a/src/3rdparty/phonon/mmf/abstractplayer.cpp b/src/3rdparty/phonon/mmf/abstractplayer.cpp index e3c0ecb..de2722d 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractplayer.cpp @@ -118,12 +118,14 @@ void MMF::AbstractPlayer::videoOutputChanged() // Default behaviour is empty - overridden by VideoPlayer } -void MMF::AbstractPlayer::setError(Phonon::ErrorType error) +void MMF::AbstractPlayer::setError(Phonon::ErrorType error, + const QString &errorMessage) { TRACE_CONTEXT(AbstractPlayer::setError, EAudioInternal); TRACE_ENTRY("state %d error %d", m_state, error); m_error = error; + m_errorString = errorMessage; changeState(ErrorState); TRACE_EXIT_0(); @@ -138,9 +140,7 @@ Phonon::ErrorType MMF::AbstractPlayer::errorType() const QString MMF::AbstractPlayer::errorString() const { - // TODO: put in proper error strings - QString result; - return result; + return m_errorString; } Phonon::State MMF::AbstractPlayer::phononState() const @@ -173,5 +173,29 @@ void MMF::AbstractPlayer::setState(PrivateState newState) m_state = newState; } +void MMF::AbstractPlayer::changeState(PrivateState newState) +{ + TRACE_CONTEXT(AbstractPlayer::changeState, EAudioInternal); + TRACE_ENTRY("state %d newState %d", privateState(), newState); + + // TODO: add some invariants to check that the transition is valid + + const Phonon::State oldPhononState = phononState(privateState()); + + // We need to change the state before we emit stateChanged(), because + // some user code, for instance the mediaplayer, switch on MediaObject's + // state. + setState(newState); + + const Phonon::State newPhononState = phononState(newState); + + if (oldPhononState != newPhononState) { + TRACE("emit stateChanged(%d, %d)", newPhononState, oldPhononState); + emit stateChanged(newPhononState, oldPhononState); + } + + TRACE_EXIT_0(); +} + QT_END_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/abstractplayer.h b/src/3rdparty/phonon/mmf/abstractplayer.h index 08558cf..1c4ea02 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.h +++ b/src/3rdparty/phonon/mmf/abstractplayer.h @@ -93,7 +93,8 @@ public: /** * Records error and changes state to ErrorState */ - void setError(Phonon::ErrorType error); + void setError(Phonon::ErrorType error, + const QString &errorMessage = QString()); Phonon::State state() const; Q_SIGNALS: @@ -132,7 +133,10 @@ protected: PrivateState privateState() const; - virtual void changeState(PrivateState newState) = 0; + /** + * Changes state and emits stateChanged() + */ + virtual void changeState(PrivateState newState); /** * Modifies m_state directly. Typically you want to call changeState(), @@ -152,6 +156,7 @@ protected: private: PrivateState m_state; Phonon::ErrorType m_error; + QString m_errorString; qint32 m_tickInterval; qint32 m_transitionTime; qint32 m_prefinishMark; diff --git a/src/3rdparty/phonon/mmf/dummyplayer.cpp b/src/3rdparty/phonon/mmf/dummyplayer.cpp index bd21d20..e6f3855 100644 --- a/src/3rdparty/phonon/mmf/dummyplayer.cpp +++ b/src/3rdparty/phonon/mmf/dummyplayer.cpp @@ -87,11 +87,6 @@ qint64 MMF::DummyPlayer::currentTime() const return 0; } -QString MMF::DummyPlayer::errorString() const -{ - return QString(); -} - Phonon::ErrorType MMF::DummyPlayer::errorType() const { return Phonon::NoError; @@ -127,9 +122,5 @@ void MMF::DummyPlayer::doSetTickInterval(qint32) } -void MMF::DummyPlayer::changeState(PrivateState) -{ -} - QT_END_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/dummyplayer.h b/src/3rdparty/phonon/mmf/dummyplayer.h index 9ff9f78..c6270c9 100644 --- a/src/3rdparty/phonon/mmf/dummyplayer.h +++ b/src/3rdparty/phonon/mmf/dummyplayer.h @@ -54,7 +54,6 @@ public: virtual bool isSeekable() const; virtual qint64 currentTime() const; virtual Phonon::State state() const; - virtual QString errorString() const; virtual Phonon::ErrorType errorType() const; virtual qint64 totalTime() const; virtual MediaSource source() const; @@ -64,9 +63,6 @@ public: // AbstractPlayer virtual void doSetTickInterval(qint32 interval); - -protected: - virtual void changeState(PrivateState newState); }; } } diff --git a/src/3rdparty/phonon/mmf/mediaobject.cpp b/src/3rdparty/phonon/mmf/mediaobject.cpp index 29ac2df..74aaa58 100644 --- a/src/3rdparty/phonon/mmf/mediaobject.cpp +++ b/src/3rdparty/phonon/mmf/mediaobject.cpp @@ -239,6 +239,7 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) const bool oldPlayerSeekable = oldPlayer->isSeekable(); Phonon::ErrorType error = NoError; + QString errorMessage; // Determine media type switch (source.type()) { @@ -253,7 +254,7 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) mediaType = fileMediaType(url.toLocalFile()); } else { - TRACE_0("Network streaming not supported yet"); + errorMessage = QLatin1String("Network streaming not supported yet"); error = NormalError; } } @@ -286,7 +287,8 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) newPlayer = new DummyPlayer(); } - newPlayer->setError(NormalError); + error = NormalError; + errorMessage = tr("Media type could not be determined"); break; case MediaTypeAudio: @@ -321,9 +323,11 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) connect(m_player.data(), SIGNAL(finished()), SIGNAL(finished())); connect(m_player.data(), SIGNAL(tick(qint64)), SIGNAL(tick(qint64))); - if (error != NoError ) { - newPlayer = new DummyPlayer(); - newPlayer->setError(error); + // We need to call setError() after doing the connects, otherwise the + // error won't be received. + if (error != NoError) { + Q_ASSERT(m_player); + m_player->setError(error, errorMessage); } TRACE_EXIT_0(); -- cgit v0.12