From 483893142dcec15c646ef997309dcede76466766 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Mon, 11 Jan 2010 15:03:29 +0000 Subject: 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 --- src/3rdparty/phonon/mmf/abstractaudioeffect.cpp | 15 ++++-- src/3rdparty/phonon/mmf/abstractaudioeffect.h | 11 ++-- src/3rdparty/phonon/mmf/audioequalizer.cpp | 30 +++++++---- src/3rdparty/phonon/mmf/audioequalizer.h | 2 +- src/3rdparty/phonon/mmf/effectfactory.h | 1 + src/3rdparty/phonon/mmf/effectparameter.cpp | 63 ++++++++++++++++++++++ src/3rdparty/phonon/mmf/effectparameter.h | 72 +++++++++++++++++++++++++ src/3rdparty/phonon/phonon/effectwidget.cpp | 2 +- src/plugins/phonon/mmf/mmf.pro | 2 + 9 files changed, 175 insertions(+), 23 deletions(-) create mode 100644 src/3rdparty/phonon/mmf/effectparameter.cpp create mode 100644 src/3rdparty/phonon/mmf/effectparameter.h 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 ¶ms) : MediaNode(parent) - , m_player(0) , m_params(params) + , m_player(0) { } -QList AbstractAudioEffect::parameters() const +QList AbstractAudioEffect::parameters() const { - return m_params; + // Convert from QList to QList + QList 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 ¶m, +void AbstractAudioEffect::setParameterValue(const Phonon::EffectParameter ¶m, 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 . #include #include -#include #include "audioplayer.h" +#include "effectparameter.h" #include "mmf_medianode.h" #include "mmf_videoplayer.h" @@ -63,9 +63,10 @@ public: AbstractAudioEffect(QObject *parent, const QList ¶ms); - virtual QList parameters() const; - virtual QVariant parameterValue(const EffectParameter ¶m) const; - virtual void setParameterValue(const EffectParameter &, + // Phonon::EffectInterface + virtual QList parameters() const; + virtual QVariant parameterValue(const Phonon::EffectParameter ¶m) const; + virtual void setParameterValue(const Phonon::EffectParameter &, const QVariant &newValue); protected: @@ -81,10 +82,10 @@ protected: protected: QScopedPointer m_effect; + const QList m_params; private: AbstractMediaPlayer * m_player; - const QList m_params; QHash 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 ¶m = m_params[band]; + const int internalLevel = param.toInternalValue(externalLevel); + CAudioEqualizer *const effect = static_cast(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 . #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 . + +*/ + +#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(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 . + +*/ + +#ifndef PHONON_MMF_EFFECTPARAMETER_H +#define PHONON_MMF_EFFECTPARAMETER_H + +#include + +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 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 . */ 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 \ -- cgit v0.12