From a484dd7cf655818aa849cf9dd002f76e137b3319 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 13 Apr 2010 16:22:33 +0100 Subject: tst_mediaobject: Removed compiler warnings Removed declaration of variables which are subsequently unused. Reviewed-by: Frans Englich --- tests/auto/mediaobject/tst_mediaobject.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index 5b0943e..e62bfd7 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -194,12 +194,6 @@ static qint32 castQVariantToInt32(const QVariant &variant) } static const char *const red = "\033[0;31m"; -static const char *const green = "\033[0;32m"; -static const char *const yellow = "\033[0;33m"; -static const char *const blue = "\033[0;34m"; -static const char *const purple = "\033[0;35m"; -static const char *const cyan = "\033[0;36m"; -static const char *const white = "\033[0;37m"; static const char *const normal = "\033[0m"; void tst_MediaObject::stateChanged(Phonon::State newstate, Phonon::State oldstate) -- cgit v0.12 From 57dddfe5036e256a9d2648c85fadb5abef31fb19 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 13 Apr 2010 16:25:14 +0100 Subject: tst_mediaobject: Removed compiler warnings Compiler warnings concerned unreachable code. Reviewed-by: Frans Englich --- tests/auto/mediaobject/qtesthelper.h | 1 - tests/auto/mediaobject/tst_mediaobject.cpp | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/auto/mediaobject/qtesthelper.h b/tests/auto/mediaobject/qtesthelper.h index f082296..39d5b91 100644 --- a/tests/auto/mediaobject/qtesthelper.h +++ b/tests/auto/mediaobject/qtesthelper.h @@ -215,7 +215,6 @@ namespace QTest default: return 0; } - return 0; } } // namespace QTest QT_END_NAMESPACE diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index e62bfd7..efb9132 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -209,9 +209,7 @@ void tst_MediaObject::testPlayFromResource() { #ifdef Q_OS_SYMBIAN QSKIP("Not implemented yet.", SkipAll); - return; -#endif - +#else QFile file(MEDIA_FILEPATH); MediaObject media; media.setCurrentSource(&file); @@ -223,6 +221,7 @@ void tst_MediaObject::testPlayFromResource() if (media.state() != Phonon::PlayingState) QTest::waitForSignal(&media, SIGNAL(stateChanged(Phonon::State, Phonon::State)), 10000); QCOMPARE(media.state(), Phonon::PlayingState); +#endif } void tst_MediaObject::testPlayIllegalFile() -- cgit v0.12 From 0d12963c85fc116cd7283cd16ca0524f086f9d09 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 13 Apr 2010 17:51:31 +0100 Subject: tst_mediaobject: Ensure playSDP step cleanup is run This step previously did not guarantee that the m_media object would be left in StoppedState, which in turn meant that following test steps could fail. Task-number: QTBUG-9339 Reviewed-by: Frans Englich --- tests/auto/mediaobject/tst_mediaobject.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index efb9132..7f9f57d 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -572,13 +572,18 @@ void tst_MediaObject::playSDP() if (m_media->state() != Phonon::StoppedState) QTest::waitForSignal(m_media, SIGNAL(stateChanged(Phonon::State, Phonon::State)), 10000); - // At this point we're in error state due to absent media, but it has now loaded the SDP: - QCOMPARE(m_media->errorString(), QString::fromLatin1("Buffering clip failed: Unknown error (-39)")); + // MediaObject should have loaded the SDP, but be in error state due to absent media + const bool stateMatch = (m_media->state() == Phonon::ErrorState); + const bool errorStringMatch = (m_media->errorString() == QString::fromLatin1("Buffering clip failed: Unknown error (-39)")); - // We cannot play the SDP, we can neither attempt to play it, because we - // won't get a state change from ErrorState to ErrorState, and hence block - // on a never occuring signal. + // Ensure that m_media is back in ground state prior to starting next test step m_media->setCurrentSource(oldSource); + if (m_media->state() != Phonon::StoppedState) + QTest::waitForSignal(m_media, SIGNAL(stateChanged(Phonon::State, Phonon::State))); + QCOMPARE(m_media->state(), Phonon::StoppedState); + + QVERIFY(stateMatch); + QVERIFY(errorStringMatch); #else QSKIP("Unsupported on this platform.", SkipAll); -- cgit v0.12 From 240cf56a6490bc05339cc05457c8bab3b5cab66c Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 16:15:32 +0100 Subject: tst_mediaobject: Removed non-portable escape codes from output Test step was outputting ANSI escape codes in order to highlight certain text. These codes are not natively supported by Windows consoles, resulting in raw escape codes appearing in the output. Reviewed-by: Frans Englich --- tests/auto/mediaobject/tst_mediaobject.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index 7f9f57d..5328f63 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -193,16 +193,10 @@ static qint32 castQVariantToInt32(const QVariant &variant) return *reinterpret_cast(variant.constData()); } -static const char *const red = "\033[0;31m"; -static const char *const normal = "\033[0m"; - void tst_MediaObject::stateChanged(Phonon::State newstate, Phonon::State oldstate) { - if (newstate == Phonon::ErrorState) { - QWARN(QByteArray(QByteArray(red) + ".......................................................... ") + QByteArray(QTest::toString(oldstate)) + " to " + QByteArray(QTest::toString(newstate)) + normal); - } else { - //qDebug() << ".........................................................." << cyan << QTest::toString(oldstate) << "to" << QTest::toString(newstate) << normal; - } + if (newstate == Phonon::ErrorState) + QWARN(QByteArray(QByteArray(QTest::toString(oldstate)) + " to " + QByteArray(QTest::toString(newstate)))); } void tst_MediaObject::testPlayFromResource() -- cgit v0.12 From 151c2a2bd5e764555f32e6141024172e77788efd Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 16:37:21 +0100 Subject: tst_mediaobject: ensure MediaObject is in StoppedState before each step Many of the steps in the tst_mediaobject suite (a) check that the MediaObject is in StoppedState at the start of the step and (b) call stopPlayback() at the end. If, however, a QTest check fails during the test, stopPlayback may not be called. This patch adds a call to MediaObject::stop() in the suite's init() function. This is a symptom of a wider problem with this test suite, namely that it re-uses a single instance of Phonon::MediaObject for all steps. Given the highly stateful nature of MediaObject, this can lead to test steps failing due to some state which was erroneously carried forward from an earlier step. While this test suite design may more faithfully represent real-world usage of Phonon, it makes tracking down the root causes of test failures needlessly difficult. Reviewed-by: Frans Englich --- tests/auto/mediaobject/tst_mediaobject.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index 5328f63..127b775 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -251,6 +251,13 @@ void tst_MediaObject::init() } m_stateChangedSignalSpy->clear(); } + + // Ensure that m_media is in StoppedState + if (m_media->state() != Phonon::StoppedState) { + m_media->stop(); + QTest::waitForSignal(m_media, SIGNAL(stateChanged(Phonon::State, Phonon::State))); + QCOMPARE(m_media->state(), Phonon::StoppedState); + } } void tst_MediaObject::cleanup() -- cgit v0.12 From 430718023edfe83b4e028d97c1a57fd0cdbbdace Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 13 Apr 2010 16:42:41 +0100 Subject: Phonon MMF: Removed compiler warning Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/videoplayer_dsa.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp index d607f1d..fbd8397 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp @@ -296,7 +296,7 @@ bool MMF::DsaVideoPlayer::stopDirectScreenAccess() const bool dsaWasActive = m_dsaActive; if (m_dsaActive) { - TRAPD(err, m_player->StopDirectScreenAccessL()); + TRAP(err, m_player->StopDirectScreenAccessL()); if (KErrNone == err) m_dsaActive = false; else -- cgit v0.12 From 0fa644049f3a864df4c1ffa47efd16d7d0c81cf3 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 12:14:32 +0100 Subject: Phonon MMF: Emit prefinishMarkReached(), finished() signals Fixes testPrefinishMark step in tst_mediaobject. Task-number: QTBUG-9339 Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 1 + src/3rdparty/phonon/mmf/mediaobject.cpp | 16 ++++++++++++++-- src/3rdparty/phonon/mmf/mediaobject.h | 3 +++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index 6356c21..a90131d 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -386,6 +386,7 @@ void MMF::AbstractMediaPlayer::playbackComplete(int error) } else { setError(tr("Playback complete"), error); + emit finished(); } } diff --git a/src/3rdparty/phonon/mmf/mediaobject.cpp b/src/3rdparty/phonon/mmf/mediaobject.cpp index ee459e6..f881da0 100644 --- a/src/3rdparty/phonon/mmf/mediaobject.cpp +++ b/src/3rdparty/phonon/mmf/mediaobject.cpp @@ -323,11 +323,11 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) connect(m_player.data(), SIGNAL(totalTimeChanged(qint64)), SIGNAL(totalTimeChanged(qint64))); connect(m_player.data(), SIGNAL(stateChanged(Phonon::State,Phonon::State)), SIGNAL(stateChanged(Phonon::State,Phonon::State))); connect(m_player.data(), SIGNAL(finished()), SIGNAL(finished())); - connect(m_player.data(), SIGNAL(tick(qint64)), SIGNAL(tick(qint64))); connect(m_player.data(), SIGNAL(bufferStatus(int)), SIGNAL(bufferStatus(int))); connect(m_player.data(), SIGNAL(metaDataChanged(QMultiMap)), SIGNAL(metaDataChanged(QMultiMap))); connect(m_player.data(), SIGNAL(aboutToFinish()), SIGNAL(aboutToFinish())); - connect(m_player.data(), SIGNAL(prefinishMarkReached(qint32)), SIGNAL(tick(qint32))); + connect(m_player.data(), SIGNAL(prefinishMarkReached(qint32)), SIGNAL(prefinishMarkReached(qint32))); + connect(m_player.data(), SIGNAL(prefinishMarkReached(qint32)), SLOT(handlePrefinishMarkReached(qint32))); // We need to call setError() after doing the connects, otherwise the // error won't be received. @@ -414,8 +414,20 @@ void MMF::MediaObject::switchToNextSource() m_nextSourceSet = false; switchToSource(m_nextSource); play(); + } else { + emit finished(); } } +//----------------------------------------------------------------------------- +// Other private functions +//----------------------------------------------------------------------------- + +void MMF::MediaObject::handlePrefinishMarkReached(qint32 time) +{ + emit tick(time); +} + + QT_END_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/mediaobject.h b/src/3rdparty/phonon/mmf/mediaobject.h index 62e0a0e..f15eb21 100644 --- a/src/3rdparty/phonon/mmf/mediaobject.h +++ b/src/3rdparty/phonon/mmf/mediaobject.h @@ -107,6 +107,9 @@ Q_SIGNALS: void finished(); void tick(qint64 time); +private Q_SLOTS: + void handlePrefinishMarkReached(qint32); + private: void switchToSource(const MediaSource &source); void createPlayer(const MediaSource &source); -- cgit v0.12 From dc2d0315fc18fdbad7686adaf5f4886252fe0e5a Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 10:54:44 +0100 Subject: Phonon MMF: Emit tick() signal Fixes testTickSignal step in tst_mediaobject. Task-number: QTBUG-9339 Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/mediaobject.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/3rdparty/phonon/mmf/mediaobject.cpp b/src/3rdparty/phonon/mmf/mediaobject.cpp index f881da0..e1b921b 100644 --- a/src/3rdparty/phonon/mmf/mediaobject.cpp +++ b/src/3rdparty/phonon/mmf/mediaobject.cpp @@ -328,6 +328,7 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) connect(m_player.data(), SIGNAL(aboutToFinish()), SIGNAL(aboutToFinish())); connect(m_player.data(), SIGNAL(prefinishMarkReached(qint32)), SIGNAL(prefinishMarkReached(qint32))); connect(m_player.data(), SIGNAL(prefinishMarkReached(qint32)), SLOT(handlePrefinishMarkReached(qint32))); + connect(m_player.data(), SIGNAL(tick(qint64)), SIGNAL(tick(qint64))); // We need to call setError() after doing the connects, otherwise the // error won't be received. -- cgit v0.12 From 468a0b27f8706e7d40f7b1c07f0cc0b50a48971b Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 14:03:35 +0100 Subject: Phonon MMF: Suppress intermediate stateChanged() signal If MediaObject::play() is called while the object is in LoadingState, playback will start once loading is completed. Previously, the Symbian backend at this point emitted two stateChanged signals - first to StoppedState and then to PlayingState. The testPlayOnFinish step in tst_mediaobject requires that only one state change occurs: directly from LoadingState to PlayingState. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 20 +++++++++++++------- src/3rdparty/phonon/mmf/abstractmediaplayer.h | 4 +++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index a90131d..221443e 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -79,9 +79,7 @@ void MMF::AbstractMediaPlayer::play() case StoppedState: case PausedState: - doPlay(); - startPositionTimer(); - changeState(PlayingState); + startPlayback(); break; case PlayingState: @@ -441,6 +439,13 @@ void MMF::AbstractMediaPlayer::resetMarksIfRewound() m_aboutToFinishSent = false; } +void MMF::AbstractMediaPlayer::startPlayback() +{ + doPlay(); + startPositionTimer(); + changeState(PlayingState); +} + void MMF::AbstractMediaPlayer::bufferStatusTick() { emit MMF::AbstractPlayer::bufferStatus(bufferStatus()); @@ -453,9 +458,6 @@ void MMF::AbstractMediaPlayer::changeState(PrivateState newState) const Phonon::State oldPhononState = phononState(privateState()); const Phonon::State newPhononState = phononState(newState); - // TODO: add some invariants to check that the transition is valid - AbstractPlayer::changeState(newState); - if (LoadingState == oldPhononState && StoppedState == newPhononState) { // Ensure initial volume is set on MMF API before starting playback doVolumeChanged(); @@ -465,8 +467,12 @@ void MMF::AbstractMediaPlayer::changeState(PrivateState newState) if (m_playPending) { TRACE_0("play was called while loading; starting playback now"); m_playPending = false; - play(); + startPlayback(); + } else { + AbstractPlayer::changeState(newState); } + } else { + AbstractPlayer::changeState(newState); } } diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.h b/src/3rdparty/phonon/mmf/abstractmediaplayer.h index 308b5af..f8f86af 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.h +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.h @@ -70,7 +70,8 @@ protected: virtual int openUrl(const QString& url) = 0; virtual int bufferStatus() const = 0; virtual void close() = 0; - virtual void changeState(PrivateState newState); + + void changeState(PrivateState newState); void updateMetaData(); virtual int numberOfMetaDataEntries() const = 0; @@ -93,6 +94,7 @@ private: void doVolumeChanged(); void emitMarksIfReached(qint64 position); void resetMarksIfRewound(); + void startPlayback(); private Q_SLOTS: void positionTick(); -- cgit v0.12 From 4f97cf29e5be9f34341dd99ad4bd8dd3d61b1441 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 14:39:39 +0100 Subject: Phonon MMF: change to PausedState, not StoppedState when finished This behaviour is required by the testPauseOnFinish step in tst_mediaobject. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index 221443e..ea2d536 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -375,7 +375,7 @@ void MMF::AbstractMediaPlayer::playbackComplete(int error) } if (KErrNone == error) { - changeState(StoppedState); + changeState(PausedState); // MediaObject::switchToNextSource deletes the current player, so we // call it via delayed slot invokation to ensure that this object does -- cgit v0.12 From 7e598398ec161e3fc5df98ea754bd637d0c1ba30 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 16:03:50 +0100 Subject: Phonon MMF: calling pause() when in StoppedState triggers stateChanged() Previously, the MMF backend simply swallowed a call to pause() when in StoppedState. However, the stopToPause step in tst_mediaobject requires the backend to emit a stateChanged signal when this happens. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 65 ++++++++++++++++++++----- src/3rdparty/phonon/mmf/abstractmediaplayer.h | 20 +++++--- src/3rdparty/phonon/mmf/abstractplayer.cpp | 2 +- src/3rdparty/phonon/mmf/abstractplayer.h | 2 +- src/3rdparty/phonon/mmf/abstractvideoplayer.cpp | 5 +- src/3rdparty/phonon/mmf/audioplayer.cpp | 6 +-- tests/auto/mediaobject/tst_mediaobject.cpp | 2 +- 7 files changed, 72 insertions(+), 30 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index ea2d536..7104ebd 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -48,7 +48,7 @@ MMF::AbstractMediaPlayer::AbstractMediaPlayer (MediaObject *parent, const AbstractPlayer *player) : AbstractPlayer(player) , m_parent(parent) - , m_playPending(false) + , m_pending(NothingPending) , m_positionTimer(new QTimer(this)) , m_bufferStatusTimer(new QTimer(this)) , m_mmfMaxVolume(NullMaxVolume) @@ -74,7 +74,7 @@ void MMF::AbstractMediaPlayer::play() break; case LoadingState: - m_playPending = true; + setPending(PlayPending); break; case StoppedState: @@ -101,14 +101,16 @@ void MMF::AbstractMediaPlayer::pause() TRACE_CONTEXT(AbstractMediaPlayer::pause, EAudioApi); TRACE_ENTRY("state %d", privateState()); - m_playPending = false; stopTimers(); switch (privateState()) { case GroundState: case LoadingState: - case PausedState: case StoppedState: + setPending(PausePending); + break; + + case PausedState: // Do nothing break; @@ -133,7 +135,7 @@ void MMF::AbstractMediaPlayer::stop() TRACE_CONTEXT(AbstractMediaPlayer::stop, EAudioApi); TRACE_ENTRY("state %d", privateState()); - m_playPending = false; + setPending(NothingPending); stopTimers(); switch (privateState()) { @@ -363,6 +365,18 @@ void MMF::AbstractMediaPlayer::maxVolumeChanged(int mmfMaxVolume) doVolumeChanged(); } +void MMF::AbstractMediaPlayer::loadingComplete(int error) +{ + Q_ASSERT(Phonon::LoadingState == state()); + + if (KErrNone == error) { + updateMetaData(); + changeState(StoppedState); + } else { + setError(tr("Loading clip failed"), error); + } +} + void MMF::AbstractMediaPlayer::playbackComplete(int error) { stopTimers(); @@ -439,6 +453,15 @@ void MMF::AbstractMediaPlayer::resetMarksIfRewound() m_aboutToFinishSent = false; } +void MMF::AbstractMediaPlayer::setPending(Pending pending) +{ + const Phonon::State oldState = state(); + m_pending = pending; + const Phonon::State newState = state(); + if (newState != oldState) + emit stateChanged(newState, oldState); +} + void MMF::AbstractMediaPlayer::startPlayback() { doPlay(); @@ -451,6 +474,18 @@ void MMF::AbstractMediaPlayer::bufferStatusTick() emit MMF::AbstractPlayer::bufferStatus(bufferStatus()); } +Phonon::State MMF::AbstractMediaPlayer::phononState(PrivateState state) const +{ + Phonon::State result = AbstractPlayer::phononState(state); + + if (PausePending == m_pending) { + Q_ASSERT(Phonon::StoppedState == result || Phonon::LoadingState == result); + result = Phonon::PausedState; + } + + return result; +} + void MMF::AbstractMediaPlayer::changeState(PrivateState newState) { TRACE_CONTEXT(AbstractMediaPlayer::changeState, EAudioInternal); @@ -462,15 +497,21 @@ void MMF::AbstractMediaPlayer::changeState(PrivateState newState) // Ensure initial volume is set on MMF API before starting playback doVolumeChanged(); - // Check whether play() was called while clip was being loaded. If so, - // playback should be started now - if (m_playPending) { - TRACE_0("play was called while loading; starting playback now"); - m_playPending = false; - startPlayback(); - } else { + switch (m_pending) { + case NothingPending: AbstractPlayer::changeState(newState); + break; + + case PlayPending: + startPlayback(); + break; + + case PausePending: + AbstractPlayer::changeState(PausedState); + break; } + + setPending(NothingPending); } else { AbstractPlayer::changeState(newState); } diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.h b/src/3rdparty/phonon/mmf/abstractmediaplayer.h index f8f86af..23a8233 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.h +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.h @@ -60,6 +60,8 @@ public: protected: // AbstractPlayer virtual void doSetTickInterval(qint32 interval); + virtual Phonon::State phononState(PrivateState state) const; + virtual void changeState(PrivateState newState); virtual void doPlay() = 0; virtual void doPause() = 0; @@ -71,8 +73,6 @@ protected: virtual int bufferStatus() const = 0; virtual void close() = 0; - void changeState(PrivateState newState); - void updateMetaData(); virtual int numberOfMetaDataEntries() const = 0; virtual QPair metaDataEntry(int index) const = 0; @@ -81,6 +81,7 @@ protected: void bufferingStarted(); void bufferingComplete(); void maxVolumeChanged(int maxVolume); + void loadingComplete(int error); void playbackComplete(int error); static qint64 toMilliSeconds(const TTimeIntervalMicroSeconds &); @@ -96,6 +97,14 @@ private: void resetMarksIfRewound(); void startPlayback(); + enum Pending { + NothingPending, + PausePending, + PlayPending + }; + + void setPending(Pending pending); + private Q_SLOTS: void positionTick(); void bufferStatusTick(); @@ -103,12 +112,7 @@ private Q_SLOTS: private: MediaObject *const m_parent; - /** - * This flag is set to true if play is called when the object is - * in a Loading state. Once loading is complete, playback will - * be started. - */ - bool m_playPending; + Pending m_pending; QScopedPointer m_positionTimer; diff --git a/src/3rdparty/phonon/mmf/abstractplayer.cpp b/src/3rdparty/phonon/mmf/abstractplayer.cpp index 421155b..8c3f5cb 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractplayer.cpp @@ -141,7 +141,7 @@ Phonon::State MMF::AbstractPlayer::phononState() const return phononState(m_state); } -Phonon::State MMF::AbstractPlayer::phononState(PrivateState state) +Phonon::State MMF::AbstractPlayer::phononState(PrivateState state) const { const Phonon::State phononState = GroundState == state diff --git a/src/3rdparty/phonon/mmf/abstractplayer.h b/src/3rdparty/phonon/mmf/abstractplayer.h index 92bd87e..ab892f5 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.h +++ b/src/3rdparty/phonon/mmf/abstractplayer.h @@ -133,7 +133,7 @@ protected: /** * Converts PrivateState into the corresponding Phonon::State */ - static Phonon::State phononState(PrivateState state); + virtual Phonon::State phononState(PrivateState state) const; virtual void videoOutputChanged(); diff --git a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp index c2bcce0..2e0ab1c 100644 --- a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp @@ -268,11 +268,10 @@ void MMF::AbstractVideoPlayer::MvpuoPrepareComplete(TInt aError) handlePendingParametersChanged(); emit totalTimeChanged(totalTime()); - changeState(StoppedState); - } else { - setError(tr("Buffering clip failed"), err); } + loadingComplete(aError); + TRACE_EXIT_0(); } diff --git a/src/3rdparty/phonon/mmf/audioplayer.cpp b/src/3rdparty/phonon/mmf/audioplayer.cpp index ee07229..77a0964 100644 --- a/src/3rdparty/phonon/mmf/audioplayer.cpp +++ b/src/3rdparty/phonon/mmf/audioplayer.cpp @@ -203,12 +203,10 @@ void MMF::AudioPlayer::MapcInitComplete(TInt aError, maxVolumeChanged(m_player->MaxVolume()); m_totalTime = toMilliSeconds(m_player->Duration()); emit totalTimeChanged(m_totalTime); - updateMetaData(); - changeState(StoppedState); - } else { - setError(tr("Opening clip failed"), aError); } + loadingComplete(aError); + TRACE_EXIT_0(); } diff --git a/tests/auto/mediaobject/tst_mediaobject.cpp b/tests/auto/mediaobject/tst_mediaobject.cpp index 127b775..994057b 100644 --- a/tests/auto/mediaobject/tst_mediaobject.cpp +++ b/tests/auto/mediaobject/tst_mediaobject.cpp @@ -575,7 +575,7 @@ void tst_MediaObject::playSDP() // MediaObject should have loaded the SDP, but be in error state due to absent media const bool stateMatch = (m_media->state() == Phonon::ErrorState); - const bool errorStringMatch = (m_media->errorString() == QString::fromLatin1("Buffering clip failed: Unknown error (-39)")); + const bool errorStringMatch = (m_media->errorString() == QString::fromLatin1("Loading clip failed: Unknown error (-39)")); // Ensure that m_media is back in ground state prior to starting next test step m_media->setCurrentSource(oldSource); -- cgit v0.12 From 0c2f5376e921fa9badd87a3eb37d94868451248c Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 19:31:22 +0100 Subject: Phonon MMF: fix state changes emitted during playlist handling This change is required by the testPlayBeforeFinish step in tst_mediaobject. This test starts playback of one track, then calls MediaObject::setCurrentSource() followed by MediaObject::play(). The step checks that the following stateChanged() signals are emitted by the MediaObject: 1. StoppedState (optionally) 2. LoadingState 3. BufferingState or PlayingState The state changes emitted by the Phonon MMF backend were: 1. PlayingState -> StoppedState 2. LoadingState -> PlayingState This patch fixes the discontinuity in state changes which occurred while processing a playlist. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractplayer.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/3rdparty/phonon/mmf/abstractplayer.cpp b/src/3rdparty/phonon/mmf/abstractplayer.cpp index 8c3f5cb..77d7ae0 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractplayer.cpp @@ -48,6 +48,11 @@ MMF::AbstractPlayer::AbstractPlayer(const AbstractPlayer *player) m_tickInterval = player->m_tickInterval; m_transitionTime = player->m_transitionTime; m_prefinishMark = player->m_prefinishMark; + + // This is to prevent unwanted state transitions occurring as a result + // of MediaObject::switchToNextSource() during playlist playback. + if (StoppedState == player->m_state) + m_state = player->m_state; } } -- cgit v0.12 From 92ed8a1a049f60733ce15c4a5b79ed0fc389cfcd Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 20:04:39 +0100 Subject: Phonon MMF: ensure initial volume is applied A recent change meant that, if the user set a volume level before loading a clip into the MediaObject, that initial volume level was not applied to the audio output. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/abstractmediaplayer.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp index 7104ebd..be2a568 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.cpp @@ -494,15 +494,14 @@ void MMF::AbstractMediaPlayer::changeState(PrivateState newState) const Phonon::State newPhononState = phononState(newState); if (LoadingState == oldPhononState && StoppedState == newPhononState) { - // Ensure initial volume is set on MMF API before starting playback - doVolumeChanged(); - switch (m_pending) { case NothingPending: AbstractPlayer::changeState(newState); break; case PlayPending: + changeState(PlayingState); // necessary in order to apply initial volume + doVolumeChanged(); startPlayback(); break; -- cgit v0.12 From 04c24781613a937c472c556391d738b6d563201d Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 14 Apr 2010 20:45:58 +0100 Subject: Phonon MMF: fixed crash during opening of video clip A crash was observed during opening a video clip, and was traced to dereferencing a null m_player pointer in DsaVideoPlayer::handleParametersChanged(), which is called during construction, but before createPlayer() has been called. This was reproducible using the following sequence: 1. Launch qmediaplayer 2. Play an audio clip 3. Open a video clip However, the following sequence worked as expected: 1. Launch qmediaplayer 2. Play a video clip 3. Play an audio clip 4. Play a video clip ... It is not clear which commit introduced this defect. Reviewed-by: Frans Englich --- src/3rdparty/phonon/mmf/videoplayer_dsa.cpp | 60 +++++++++++++------------ src/3rdparty/phonon/mmf/videoplayer_surface.cpp | 55 +++++++++++------------ 2 files changed, 58 insertions(+), 57 deletions(-) diff --git a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp index fbd8397..1925471 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp @@ -226,38 +226,40 @@ void MMF::DsaVideoPlayer::handleParametersChanged(VideoParameters parameters) getDsaRegion(m_wsSession, *m_window); #endif - static const TBool antialias = ETrue; - int err = KErrNone; - - if (parameters & ScaleFactors) { - TRAP(err, m_player->SetScaleFactorL(m_scaleWidth, m_scaleHeight, - antialias)); - if(KErrNone != err) { - TRACE("SetScaleFactorL (1) err %d", err); - setError(tr("Video display error"), err); + if (m_player) { + static const TBool antialias = ETrue; + int err = KErrNone; + + if (parameters & ScaleFactors) { + TRAP(err, m_player->SetScaleFactorL(m_scaleWidth, m_scaleHeight, + antialias)); + if(KErrNone != err) { + TRACE("SetScaleFactorL (1) err %d", err); + setError(tr("Video display error"), err); + } } - } - if (KErrNone == err) { - if (parameters & WindowHandle || parameters & WindowScreenRect) { - TRAP(err, - m_player->SetDisplayWindowL(m_wsSession, m_screenDevice, - *m_window, - m_videoScreenRect, - m_videoScreenRect)); - } + if (KErrNone == err) { + if (parameters & WindowHandle || parameters & WindowScreenRect) { + TRAP(err, + m_player->SetDisplayWindowL(m_wsSession, m_screenDevice, + *m_window, + m_videoScreenRect, + m_videoScreenRect)); + } - if (KErrNone != err) { - TRACE("SetDisplayWindowL err %d", err); - setError(tr("Video display error"), err); - } else { - m_dsaActive = true; - if (parameters & ScaleFactors) { - TRAP(err, m_player->SetScaleFactorL(m_scaleWidth, m_scaleHeight, - antialias)); - if (KErrNone != err) { - TRACE("SetScaleFactorL (2) err %d", err); - setError(tr("Video display error"), err); + if (KErrNone != err) { + TRACE("SetDisplayWindowL err %d", err); + setError(tr("Video display error"), err); + } else { + m_dsaActive = true; + if (parameters & ScaleFactors) { + TRAP(err, m_player->SetScaleFactorL(m_scaleWidth, m_scaleHeight, + antialias)); + if (KErrNone != err) { + TRACE("SetScaleFactorL (2) err %d", err); + setError(tr("Video display error"), err); + } } } } diff --git a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp index 5f234e5..fda7342 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp @@ -104,44 +104,43 @@ void MMF::SurfaceVideoPlayer::handleVideoWindowChanged() void MMF::SurfaceVideoPlayer::handleParametersChanged(VideoParameters parameters) { - CVideoPlayerUtility2 *player = static_cast(m_player.data()); - - int err = KErrNone; - TRect rect; - if (m_videoOutput) { m_videoOutput->dump(); const QSize size = m_videoOutput->videoWindowSize(); rect.SetSize(TSize(size.width(), size.height())); } - if (parameters & WindowHandle) { - if (m_displayWindow) - player->RemoveDisplayWindow(*m_displayWindow); - - RWindow *window = static_cast(m_window); - if (window) { - window->SetBackgroundColor(TRgb(0, 0, 0, 255)); - TRAP(err, player->AddDisplayWindowL(m_wsSession, m_screenDevice, *window, rect, rect)); - if (KErrNone != err) { - setError(tr("Video display error"), err); - window = 0; + CVideoPlayerUtility2 *player = static_cast(m_player.data()); + if (player) { + int err = KErrNone; + if (parameters & WindowHandle) { + if (m_displayWindow) + player->RemoveDisplayWindow(*m_displayWindow); + + RWindow *window = static_cast(m_window); + if (window) { + window->SetBackgroundColor(TRgb(0, 0, 0, 255)); + TRAP(err, player->AddDisplayWindowL(m_wsSession, m_screenDevice, *window, rect, rect)); + if (KErrNone != err) { + setError(tr("Video display error"), err); + window = 0; + } } + m_displayWindow = window; } - m_displayWindow = window; - } - if (KErrNone == err) { - if (parameters & ScaleFactors) { - Q_ASSERT(m_displayWindow); - TRAP(err, player->SetVideoExtentL(*m_displayWindow, rect)); - if (KErrNone == err) - TRAP(err, player->SetWindowClipRectL(*m_displayWindow, rect)); - if (KErrNone == err) - TRAP(err, player->SetScaleFactorL(*m_displayWindow, m_scaleWidth, m_scaleHeight)); - if (KErrNone != err) - setError(tr("Video display error"), err); + if (KErrNone == err) { + if (parameters & ScaleFactors) { + Q_ASSERT(m_displayWindow); + TRAP(err, player->SetVideoExtentL(*m_displayWindow, rect)); + if (KErrNone == err) + TRAP(err, player->SetWindowClipRectL(*m_displayWindow, rect)); + if (KErrNone == err) + TRAP(err, player->SetScaleFactorL(*m_displayWindow, m_scaleWidth, m_scaleHeight)); + if (KErrNone != err) + setError(tr("Video display error"), err); + } } } } -- cgit v0.12 From 0ebc9a600af8188e7a9d3c2904d012c6289a515d Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Thu, 15 Apr 2010 08:09:40 +0100 Subject: Fixed compiler warning Reviewed-by: trustme --- src/multimedia/audio/qaudio_symbian_p.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/multimedia/audio/qaudio_symbian_p.cpp b/src/multimedia/audio/qaudio_symbian_p.cpp index 58e3745..afe98f5 100644 --- a/src/multimedia/audio/qaudio_symbian_p.cpp +++ b/src/multimedia/audio/qaudio_symbian_p.cpp @@ -315,7 +315,7 @@ bool isFormatSupported(const QAudioFormat &formatQt, TUint32 fourCC; bool result = false; - if (formatQt.codec() == "audio/pcm" && + if (formatQt.codec() == QString::fromAscii("audio/pcm") && formatQtToNative(formatQt, fourCC, formatNative)) { result = (formatNative.iRate & caps.caps().iRate) @@ -337,7 +337,7 @@ bool formatQtToNative(const QAudioFormat &inputFormat, TMMFMonoStereo outputChannels; TMMFSoundEncoding outputEncoding; - if (inputFormat.codec() == "audio/pcm") { + if (inputFormat.codec() == QString::fromAscii("audio/pcm")) { result = sampleRateQtToNative(inputFormat.frequency(), outputSampleRate) && channelsQtToNative(inputFormat.channels(), outputChannels) -- cgit v0.12