Modified effect parameter handling to improve user experience

This change is to work around a limitation in the Phonon::EffectWidget
class.  This widget only displays sliders for parameters with numeric
values if the variant type of the parameter is QReal and the range is
exactly -1.0 to +1.0; otherwise, a spinbox is displayed. This is
rather inconvenient for many effects, such as the audio equalizer, for
which a slider is a much more natural UI control.

The MMF backend therefore reports the type of numeric parameters to
be QReal, and the range to be -1.0 to +1.0.  Internally, the integer
range for the parameter is stored.  Changes to the parameter value are
converted from the client-side, floating point representation to the
internal, integer representation.

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

9 files changed, 175 insertions, 23 deletions

diff --git a/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp b/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
index c75c08e..1939e04 100644
--- a/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
+++ b/src/3rdparty/phonon/mmf/abstractaudioeffect.cpp
@@ -38,18 +38,23 @@ using namespace Phonon::MMF;
 AbstractAudioEffect::AbstractAudioEffect(QObject *parent,
                                          const QList<EffectParameter> &params)
     :   MediaNode(parent)
-    ,   m_player(0)
     ,   m_params(params)
+    ,   m_player(0)
 {
 
 }
 
-QList<EffectParameter> AbstractAudioEffect::parameters() const
+QList<Phonon::EffectParameter> AbstractAudioEffect::parameters() const
 {
-    return m_params;
+    // Convert from QList<MMF::EffectParameter> to QList<Phonon::EffectParameter>
+    QList<Phonon::EffectParameter> result;
+    EffectParameter param;
+    foreach (param, m_params)
+        result += param;
+    return result;
 }
 
-QVariant AbstractAudioEffect::parameterValue(const EffectParameter &queriedParam) const
+QVariant AbstractAudioEffect::parameterValue(const Phonon::EffectParameter &queriedParam) const
 {
     const QVariant &val = m_values.value(queriedParam.id());
 
@@ -59,7 +64,7 @@ QVariant AbstractAudioEffect::parameterValue(const EffectParameter &queriedParam
         return val;
 }
 
-void AbstractAudioEffect::setParameterValue(const EffectParameter &param,
+void AbstractAudioEffect::setParameterValue(const Phonon::EffectParameter &param,
                                             const QVariant &newValue)
 {
     m_values.insert(param.id(), newValue);
diff --git a/src/3rdparty/phonon/mmf/abstractaudioeffect.h b/src/3rdparty/phonon/mmf/abstractaudioeffect.h
index df1d08a..4772eb8 100644
--- a/src/3rdparty/phonon/mmf/abstractaudioeffect.h
+++ b/src/3rdparty/phonon/mmf/abstractaudioeffect.h
@@ -24,9 +24,9 @@ along with this library.  If not, see <http://www.gnu.org/licenses/>.
 #include <AudioEffectBase.h>
 
 #include <Phonon/EffectInterface>
-#include <Phonon/EffectParameter>
 
 #include "audioplayer.h"
+#include "effectparameter.h"
 #include "mmf_medianode.h"
 #include "mmf_videoplayer.h"
 
@@ -63,9 +63,10 @@ public:
     AbstractAudioEffect(QObject *parent,
                         const QList<EffectParameter> &params);
 
-    virtual QList<EffectParameter> parameters() const;
-    virtual QVariant parameterValue(const EffectParameter &param) const;
-    virtual void setParameterValue(const EffectParameter &,
+    // Phonon::EffectInterface
+    virtual QList<Phonon::EffectParameter> parameters() const;
+    virtual QVariant parameterValue(const Phonon::EffectParameter &param) const;
+    virtual void setParameterValue(const Phonon::EffectParameter &,
                                    const QVariant &newValue);
 
 protected:
@@ -81,10 +82,10 @@ protected:
 
 protected:
     QScopedPointer<CAudioEffect>    m_effect;
+    const QList<EffectParameter>    m_params;
 
 private:
     AbstractMediaPlayer *           m_player;
-    const QList<EffectParameter>    m_params;
     QHash<int, QVariant>            m_values;
 };
 
diff --git a/src/3rdparty/phonon/mmf/audioequalizer.cpp b/src/3rdparty/phonon/mmf/audioequalizer.cpp
index 5a1ce19..adbe6c8 100644
--- a/src/3rdparty/phonon/mmf/audioequalizer.cpp
+++ b/src/3rdparty/phonon/mmf/audioequalizer.cpp
@@ -39,7 +39,7 @@ void AudioEqualizer::parameterChanged(const int pid,
 {
     if (m_effect.data()) {
         const int band = pid;
-        const int level = value.toInt();
+        const qreal level = value.toReal();
         setBandLevel(band, level);
     }
 }
@@ -54,7 +54,7 @@ void AudioEqualizer::connectAudioPlayer(AudioPlayer::NativePlayer *player)
 void AudioEqualizer::applyParameters()
 {
     if (m_effect.data()) {
-        EffectParameter param;
+	Phonon::EffectParameter param;
         foreach (param, parameters()) {
             const int band = param.id();
             const int level = parameterValue(param).toInt();
@@ -63,11 +63,14 @@ void AudioEqualizer::applyParameters()
     }
 }
 
-void AudioEqualizer::setBandLevel(int band, int level)
+void AudioEqualizer::setBandLevel(int band, qreal externalLevel)
 {
+    const EffectParameter &param = m_params[band];
+    const int internalLevel = param.toInternalValue(externalLevel);
+
     CAudioEqualizer *const effect = static_cast<CAudioEqualizer *>(m_effect.data());
     // TODO: handle audio effect errors
-    TRAP_IGNORE(effect->SetBandLevelL(band, level));
+    TRAP_IGNORE(effect->SetBandLevelL(band, internalLevel));
 }
 
 //-----------------------------------------------------------------------------
@@ -93,17 +96,22 @@ void AudioEqualizer::getParameters(NativeEffect *effect,
     for (int i = 1; i <= bandCount; ++i) {
         const qint32 hz = effect->CenterFrequency(i);
 
-        const qint32 defVol = effect->BandLevel(i);
-
-        parameters.append(EffectParameter(
+        // We pass a floating-point parameter range of -1.0 to +1.0 for
+        // each band in order to work around a limitation in
+        // Phonon::EffectWidget.  See documentation of EffectParameter
+        // for more details.
+        EffectParameter param(
              /* parameterId */        i,
              /* name */               tr("%1 Hz").arg(hz),
              /* hints */              EffectParameter::LogarithmicHint,
-             /* defaultValue */       QVariant(qint32(defVol)),
-             /* minimumValue */       QVariant(qint32(dbMin)),
-             /* maximumValue */       QVariant(qint32(dbMax)),
+             /* defaultValue */       QVariant(qreal(0.0)),
+             /* minimumValue */       QVariant(qreal(-1.0)),
+             /* maximumValue */       QVariant(qreal(+1.0)),
              /* values */             QVariantList(),
-             /* description */        QString()));
+             /* description */        QString());
+
+        param.setInternalRange(dbMin, dbMax);
+        parameters.append(param);
     }
 }
 
diff --git a/src/3rdparty/phonon/mmf/audioequalizer.h b/src/3rdparty/phonon/mmf/audioequalizer.h
index ee3d297..d10cbf3 100644
--- a/src/3rdparty/phonon/mmf/audioequalizer.h
+++ b/src/3rdparty/phonon/mmf/audioequalizer.h
@@ -56,7 +56,7 @@ protected:
     virtual void parameterChanged(const int id, const QVariant &value);
 
 private:
-    void setBandLevel(int band, int level);
+    void setBandLevel(int band, qreal externalLevel);
 
 };
 }
diff --git a/src/3rdparty/phonon/mmf/effectfactory.h b/src/3rdparty/phonon/mmf/effectfactory.h
index 45bd0f0..dd4b58d 100644
--- a/src/3rdparty/phonon/mmf/effectfactory.h
+++ b/src/3rdparty/phonon/mmf/effectfactory.h
@@ -20,6 +20,7 @@ along with this library.  If not, see <http://www.gnu.org/licenses/>.
 #define PHONON_MMF_EFFECTFACTORY_H
 
 #include "abstractaudioeffect.h"
+#include "effectparameter.h"
 
 QT_BEGIN_NAMESPACE
 
diff --git a/src/3rdparty/phonon/mmf/effectparameter.cpp b/src/3rdparty/phonon/mmf/effectparameter.cpp
new file mode 100644
index 0000000..f4287b8
--- /dev/null
+++ b/src/3rdparty/phonon/mmf/effectparameter.cpp
@@ -0,0 +1,63 @@
+/*  This file is part of the KDE project.
+
+Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+
+This library is free software: you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation, either version 2.1 or 3 of the License.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License
+along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#include "effectparameter.h"
+
+QT_BEGIN_NAMESPACE
+
+using namespace Phonon;
+using namespace Phonon::MMF;
+
+/*! \class MMF::EffectParameter
+  \internal
+*/
+
+MMF::EffectParameter::EffectParameter()
+    :   m_hasInternalRange(false)
+{
+
+}
+
+MMF::EffectParameter::EffectParameter(
+            int parameterId, const QString &name, Hints hints,
+            const QVariant &defaultValue, const QVariant &min,
+            const QVariant &max, const QVariantList &values,
+            const QString &description)
+    :   Phonon::EffectParameter(parameterId, name, hints, defaultValue,
+            min, max, values, description)
+    ,   m_hasInternalRange(false)
+{
+
+}
+
+void MMF::EffectParameter::setInternalRange(qint32 min, qint32 max)
+{
+    Q_ASSERT_X(max >= min, Q_FUNC_INFO, "Invalid range");
+    m_internalRange = QPair<qint32, qint32>(min, max);
+    m_hasInternalRange = true;
+}
+
+qint32 MMF::EffectParameter::toInternalValue(qreal external) const
+{
+    Q_ASSERT_X(m_hasInternalRange, Q_FUNC_INFO, "Does not have internal range");
+    const qint32 range = m_internalRange.second - m_internalRange.first;
+    return m_internalRange.first + ((1.0 + external) / 2) * range;
+}
+
+QT_END_NAMESPACE
+
diff --git a/src/3rdparty/phonon/mmf/effectparameter.h b/src/3rdparty/phonon/mmf/effectparameter.h
new file mode 100644
index 0000000..3008159
--- /dev/null
+++ b/src/3rdparty/phonon/mmf/effectparameter.h
@@ -0,0 +1,72 @@
+/*  This file is part of the KDE project.
+
+Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+
+This library is free software: you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation, either version 2.1 or 3 of the License.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License
+along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#ifndef PHONON_MMF_EFFECTPARAMETER_H
+#define PHONON_MMF_EFFECTPARAMETER_H
+
+#include <Phonon/EffectParameter>
+
+QT_BEGIN_NAMESPACE
+
+namespace Phonon
+{
+namespace MMF
+{
+
+/**
+ * @short Parameter value for an audio effect
+ *
+ * The base class is extended in order to work around a shortcoming
+ * in Phonon::EffectWidget.  This widget only displays sliders for
+ * parameters with numeric values if the variant type of the parameter
+ * is QReal and the range is exactly -1.0 to +1.0; otherwise, a
+ * spinbox is used to set numeric parameters.  This is rather
+ * inconvenient for many effects, such as the audio equalizer, for
+ * which a slider is a much more natural UI control.
+ *
+ * For many such parameters, we therefore report the type to be QReal
+ * and the range to be -1.0 to +1.0.  This class stores the actual
+ * integer range for the parameter, and provides the toInternalValue
+ * function for converting between the client-side floating point
+ * value and the internal integer value.
+ */
+class EffectParameter : public Phonon::EffectParameter
+{
+public:
+    EffectParameter();
+    EffectParameter(int parameterId, const QString &name, Hints hints,
+                    const QVariant &defaultValue, const QVariant &min = QVariant(),
+                    const QVariant &max = QVariant(), const QVariantList &values = QVariantList(),
+                    const QString &description = QString());
+
+    void setInternalRange(qint32 min, qint32 max);
+    qint32 toInternalValue(qreal external) const;
+
+private:
+    bool                    m_hasInternalRange;
+    QPair<qint32, qint32>   m_internalRange;
+
+};
+
+}
+}
+
+QT_END_NAMESPACE
+
+#endif
+
diff --git a/src/3rdparty/phonon/phonon/effectwidget.cpp b/src/3rdparty/phonon/phonon/effectwidget.cpp
index b39299a..a2fe50f 100644
--- a/src/3rdparty/phonon/phonon/effectwidget.cpp
+++ b/src/3rdparty/phonon/phonon/effectwidget.cpp
@@ -15,7 +15,7 @@
     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
     Lesser General Public License for more details.
 
-    You should have received a copy of the GNU Lesser General Public 
+    You should have received a copy of the GNU Lesser General Public
     License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 */
diff --git a/src/plugins/phonon/mmf/mmf.pro b/src/plugins/phonon/mmf/mmf.pro
index 65a7a29..f72a657 100644
--- a/src/plugins/phonon/mmf/mmf.pro
+++ b/src/plugins/phonon/mmf/mmf.pro
@@ -35,6 +35,7 @@ HEADERS +=                                           \
            $$PHONON_MMF_DIR/defs.h                   \
            $$PHONON_MMF_DIR/dummyplayer.h            \
            $$PHONON_MMF_DIR/effectfactory.h          \
+           $$PHONON_MMF_DIR/effectparameter.h        \
            $$PHONON_MMF_DIR/mediaobject.h            \
            $$PHONON_MMF_DIR/mmf_medianode.h          \
            $$PHONON_MMF_DIR/mmf_videoplayer.h        \
@@ -57,6 +58,7 @@ SOURCES +=                                           \
            $$PHONON_MMF_DIR/bassboost.cpp            \
            $$PHONON_MMF_DIR/dummyplayer.cpp          \
            $$PHONON_MMF_DIR/effectfactory.cpp        \
+           $$PHONON_MMF_DIR/effectparameter.cpp      \
            $$PHONON_MMF_DIR/mediaobject.cpp          \
            $$PHONON_MMF_DIR/mmf_medianode.cpp        \
            $$PHONON_MMF_DIR/mmf_videoplayer.cpp      \