Delayed creation of audio effect object until audio is initialized

When applying effects to an audio stream, we must ensure that the MMF
utility API (CMdaAudioPlayerUtility or CVideoPlayerUtility) instance has
loaded a controller before calling attempting to create a CAudioEffect-
derived object.  If the controller has not been loaded, construction of
the effect object will fail.  Even if we mitigate against this, calling
CAudioEffect::ApplyL() will only have an effect if there is a controller
object - or more precisely, a DevSound instance - on which to apply the
effect.

This patch delays construction of the effect object until the MediaObject
has transitioned out of the LoadingState, indicating that an underlying
DevSound instance will have been created.

Task-number: QTBUG-4659
Reviewed-by: Espen Riskedal

8 files changed, 63 insertions, 23 deletions

diff --git a/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp b/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
index 1939e04..cdddf02 100644
--- a/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
+++ b/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
@@ -68,11 +68,26 @@ void AbstractAudioEffect::setParameterValue(const Phonon::EffectParameter &param
                                             const QVariant &newValue)
 {
     m_values.insert(param.id(), newValue);
-    parameterChanged(param.id(), newValue);
 
-    if (m_effect.data())
+    if (m_effect.data()) {
+        parameterChanged(param.id(), newValue);
         // TODO: handle audio effect errors
         TRAP_IGNORE(m_effect->ApplyL());
+    }
+}
+
+void AbstractAudioEffect::abstractPlayerChanged(AbstractPlayer *player)
+{
+    m_player = qobject_cast<AbstractMediaPlayer *>(player);
+    m_effect.reset();
+}
+
+void AbstractAudioEffect::stateChanged(Phonon::State newState,
+                                       Phonon::State oldState)
+{
+    if (Phonon::LoadingState == oldState
+        && Phonon::LoadingState != newState)
+        createEffect();
 }
 
 void AbstractAudioEffect::connectMediaObject(MediaObject *mediaObject)
@@ -80,25 +95,37 @@ void AbstractAudioEffect::connectMediaObject(MediaObject *mediaObject)
     Q_ASSERT_X(!m_player, Q_FUNC_INFO, "Player already connected");
     Q_ASSERT_X(!m_effect.data(), Q_FUNC_INFO, "Effect already created");
 
-    AbstractMediaPlayer *const player =
-        qobject_cast<AbstractMediaPlayer *>(mediaObject->abstractPlayer());
+    abstractPlayerChanged(mediaObject->abstractPlayer());
 
-    if (player) {
-        m_player = player;
+    connect(mediaObject, SIGNAL(stateChanged(Phonon::State, Phonon::State)),
+            SLOT(stateChanged(Phonon::State, Phonon::State)));
 
-        if (AudioPlayer *audioPlayer = qobject_cast<AudioPlayer *>(player)) {
-            connectAudioPlayer(audioPlayer->nativePlayer());
-            applyParameters();
-            // TODO: handle audio effect errors
-            TRAP_IGNORE(m_effect->EnableL());
-        }
-    }
+    connect(mediaObject, SIGNAL(abstractPlayerChanged(AbstractPlayer *)),
+            SLOT(abstractPlayerChanged(AbstractPlayer *)));
+
+    if (mediaObject->state() != Phonon::LoadingState)
+        createEffect();
 }
 
-void AbstractAudioEffect::disconnectMediaObject(MediaObject * /*mediaObject*/)
+void AbstractAudioEffect::disconnectMediaObject(MediaObject *mediaObject)
 {
-    m_player = 0;
-    m_effect.reset();
+    mediaObject->disconnect(this);
+    abstractPlayerChanged(0);
+}
+
+void AbstractAudioEffect::createEffect()
+{
+    Q_ASSERT_X(m_player, Q_FUNC_INFO, "Invalid media player pointer");
+
+    if (AudioPlayer *audioPlayer = qobject_cast<AudioPlayer *>(m_player)) {
+        createEffect(audioPlayer->nativePlayer());
+    }
+
+    if (m_effect.data()) {
+        applyParameters();
+        // TODO: handle audio effect errors
+        TRAP_IGNORE(m_effect->EnableL());
+    }
 }
 
 QT_END_NAMESPACE
diff --git a/src/3rdparty/phonon/mmf/abstractaudioeffect.h b/src/3rdparty/phonon/mmf/abstractaudioeffect.h
index 4772eb8..7d44bf0 100644
--- a/src/3rdparty/phonon/mmf/abstractaudioeffect.h
+++ b/src/3rdparty/phonon/mmf/abstractaudioeffect.h
@@ -36,6 +36,7 @@ namespace Phonon
 {
 namespace MMF
 {
+class AbstractPlayer;
 class AbstractMediaPlayer;
 
 /**
@@ -69,17 +70,25 @@ public:
     virtual void setParameterValue(const Phonon::EffectParameter &,
                                    const QVariant &newValue);
 
+public Q_SLOTS:
+    void abstractPlayerChanged(AbstractPlayer *player);
+    void stateChanged(Phonon::State newState,
+                      Phonon::State oldState);
+
 protected:
     // MediaNode
     void connectMediaObject(MediaObject *mediaObject);
     void disconnectMediaObject(MediaObject *mediaObject);
 
-    virtual void connectAudioPlayer(AudioPlayer::NativePlayer *player) = 0;
+    virtual void createEffect(AudioPlayer::NativePlayer *player) = 0;
     virtual void applyParameters() = 0;
 
     virtual void parameterChanged(const int id,
                                   const QVariant &value) = 0;
 
+private:
+    void createEffect();
+
 protected:
     QScopedPointer<CAudioEffect>    m_effect;
     const QList<EffectParameter>    m_params;
diff --git a/src/3rdparty/phonon/mmf/audioequalizer.cpp b/src/3rdparty/phonon/mmf/audioequalizer.cpp
index adbe6c8..b41eda4 100644
--- a/src/3rdparty/phonon/mmf/audioequalizer.cpp
+++ b/src/3rdparty/phonon/mmf/audioequalizer.cpp
@@ -44,7 +44,7 @@ void AudioEqualizer::parameterChanged(const int pid,
     }
 }
 
-void AudioEqualizer::connectAudioPlayer(AudioPlayer::NativePlayer *player)
+void AudioEqualizer::createEffect(AudioPlayer::NativePlayer *player)
 {
     CAudioEqualizer *ptr = 0;
     QT_TRAP_THROWING(ptr = CAudioEqualizer::NewL(*player));
@@ -57,7 +57,7 @@ void AudioEqualizer::applyParameters()
 	Phonon::EffectParameter param;
         foreach (param, parameters()) {
             const int band = param.id();
-            const int level = parameterValue(param).toInt();
+            const qreal level = parameterValue(param).toReal();
             setBandLevel(band, level);
         }
     }
@@ -65,7 +65,7 @@ void AudioEqualizer::applyParameters()
 
 void AudioEqualizer::setBandLevel(int band, qreal externalLevel)
 {
-    const EffectParameter &param = m_params[band];
+    const EffectParameter &param = m_params[band-1]; // Band IDs are 1-based
     const int internalLevel = param.toInternalValue(externalLevel);
 
     CAudioEqualizer *const effect = static_cast<CAudioEqualizer *>(m_effect.data());
diff --git a/src/3rdparty/phonon/mmf/audioequalizer.h b/src/3rdparty/phonon/mmf/audioequalizer.h
index d10cbf3..22fa1e8 100644
--- a/src/3rdparty/phonon/mmf/audioequalizer.h
+++ b/src/3rdparty/phonon/mmf/audioequalizer.h
@@ -51,7 +51,7 @@ public:
 
 protected:
     // AbstractAudioEffect
-    virtual void connectAudioPlayer(AudioPlayer::NativePlayer *player);
+    virtual void createEffect(AudioPlayer::NativePlayer *player);
     virtual void applyParameters();
     virtual void parameterChanged(const int id, const QVariant &value);
 
diff --git a/src/3rdparty/phonon/mmf/bassboost.cpp b/src/3rdparty/phonon/mmf/bassboost.cpp
index 4edd102..9f62ecc 100644
--- a/src/3rdparty/phonon/mmf/bassboost.cpp
+++ b/src/3rdparty/phonon/mmf/bassboost.cpp
@@ -40,7 +40,7 @@ void BassBoost::parameterChanged(const int,
     Q_ASSERT_X(false, Q_FUNC_INFO, "BassBoost has no parameters");
 }
 
-void BassBoost::connectAudioPlayer(AudioPlayer::NativePlayer *player)
+void BassBoost::createEffect(AudioPlayer::NativePlayer *player)
 {
     CBassBoost *ptr = 0;
     QT_TRAP_THROWING(ptr = CBassBoost::NewL(*player));
diff --git a/src/3rdparty/phonon/mmf/bassboost.h b/src/3rdparty/phonon/mmf/bassboost.h
index c3c48d0..9f3d764 100644
--- a/src/3rdparty/phonon/mmf/bassboost.h
+++ b/src/3rdparty/phonon/mmf/bassboost.h
@@ -49,7 +49,7 @@ public:
 
 protected:
     // AbstractAudioEffect
-    virtual void connectAudioPlayer(AudioPlayer::NativePlayer *player);
+    virtual void createEffect(AudioPlayer::NativePlayer *player);
     virtual void applyParameters();
     virtual void parameterChanged(const int id, const QVariant &value);
 
diff --git a/src/3rdparty/phonon/mmf/mediaobject.cpp b/src/3rdparty/phonon/mmf/mediaobject.cpp
index 4653fee..9744774 100644
--- a/src/3rdparty/phonon/mmf/mediaobject.cpp
+++ b/src/3rdparty/phonon/mmf/mediaobject.cpp
@@ -297,7 +297,10 @@ void MMF::MediaObject::createPlayer(const MediaSource &source)
         break;
     }
 
+    if (oldPlayer)
+        emit abstractPlayerChanged(0);
     m_player.reset(newPlayer);
+    emit abstractPlayerChanged(newPlayer);
 
     if (oldPlayerHasVideo != hasVideo()) {
         emit hasVideoChanged(hasVideo());
diff --git a/src/3rdparty/phonon/mmf/mediaobject.h b/src/3rdparty/phonon/mmf/mediaobject.h
index 7d330e7..c87d755 100644
--- a/src/3rdparty/phonon/mmf/mediaobject.h
+++ b/src/3rdparty/phonon/mmf/mediaobject.h
@@ -92,6 +92,7 @@ public Q_SLOTS:
     void switchToNextSource();
 
 Q_SIGNALS:
+    void abstractPlayerChanged(AbstractPlayer *player);
     void totalTimeChanged(qint64 length);
     void hasVideoChanged(bool hasVideo);
     void seekableChanged(bool seekable);