Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show effect parameter units in parameter name label #11041

Merged
merged 10 commits into from
Jan 9, 2023
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2738,8 +2738,8 @@ endif()
find_package(lilv)
default_option(LILV "Lilv (LV2) support" "lilv_FOUND")
if(LILV)
if(NOT TARGET lilv::lilv)
message(FATAL_ERROR "Lilv (LV2) support requires the liblilv-0 and its development headers.")
if(NOT lilv_FOUND)
message(FATAL_ERROR "Lilv (LV2) support requires the liblilv-0 and LV2 libraries and development headers.")
endif()
target_sources(mixxx-lib PRIVATE
src/effects/backends/lv2/lv2backend.cpp
Expand Down
6 changes: 3 additions & 3 deletions cmake/modules/Findlilv.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Findlilv
--------

Finds the lilv library.
Finds the lilv library and lv2-dev package containing 'units' headers.

Imported Targets
^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -37,7 +37,7 @@ Cache Variables
The following cache variables may also be set:

``lilv_INCLUDE_DIR``
The directory containing ``lilv-0/lilb/lilv.h``.
The directory containing ``lilv-0/lilv/lilv.h``.
``lilv_LIBRARY``
The path to the lilv library.

Expand All @@ -47,7 +47,7 @@ include(IsStaticLibrary)

find_package(PkgConfig QUIET)
if(PkgConfig_FOUND)
pkg_check_modules(PC_lilv QUIET lilv-0)
pkg_check_modules(PC_lilv QUIET lilv-0 lv2)
endif()

find_path(lilv_INCLUDE_DIR
Expand Down
2 changes: 1 addition & 1 deletion src/effects/backends/builtin/balanceeffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ EffectManifestPointer BalanceEffect::getManifest() {
midLowPass->setDescription(QObject::tr(
"Frequencies below this cutoff are not adjusted in the stereo field"));
midLowPass->setValueScaler(EffectManifestParameter::ValueScaler::Logarithmic);
midLowPass->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
midLowPass->setUnitsHint(EffectManifestParameter::UnitsHint::Hertz);
midLowPass->setDefaultLinkType(EffectManifestParameter::LinkType::None);
midLowPass->setNeutralPointOnScale(1);
midLowPass->setRange(kMinCornerHz, kMinCornerHz, kMaxCornerHz);
Expand Down
4 changes: 2 additions & 2 deletions src/effects/backends/builtin/flangereffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ EffectManifestPointer FlangerEffect::getManifest() {
width->setDescription(QObject::tr(
"Delay amplitude of the LFO (low frequency oscillator)"));
width->setValueScaler(EffectManifestParameter::ValueScaler::Logarithmic);
width->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
width->setUnitsHint(EffectManifestParameter::UnitsHint::Millisecond);
width->setRange(0.0, kMaxLfoWidthMs / 2, kMaxLfoWidthMs);

EffectManifestParameterPointer manual = pManifest->addParameter();
Expand All @@ -61,7 +61,7 @@ EffectManifestPointer FlangerEffect::getManifest() {
"Delay offset of the LFO (low frequency oscillator).\n"
"With width at zero, this allows for manually sweeping over the entire delay range."));
manual->setValueScaler(EffectManifestParameter::ValueScaler::Logarithmic);
manual->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
manual->setUnitsHint(EffectManifestParameter::UnitsHint::Millisecond);
manual->setRange(kMinDelayMs, kCenterDelayMs, kMaxDelayMs);

EffectManifestParameterPointer regen = pManifest->addParameter();
Expand Down
4 changes: 2 additions & 2 deletions src/effects/backends/builtin/parametriceqeffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ EffectManifestPointer ParametricEQEffect::getManifest() {
gain1->setDescription(QObject::tr(
"Gain for Filter 1"));
gain1->setValueScaler(EffectManifestParameter::ValueScaler::Linear);
gain1->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
gain1->setUnitsHint(EffectManifestParameter::UnitsHint::Decibel);
gain1->setNeutralPointOnScale(0.5);
gain1->setRange(-18, 0, 18); // dB

Expand Down Expand Up @@ -69,7 +69,7 @@ EffectManifestPointer ParametricEQEffect::getManifest() {
gain2->setDescription(QObject::tr(
"Gain for Filter 2"));
gain2->setValueScaler(EffectManifestParameter::ValueScaler::Linear);
gain2->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
gain2->setUnitsHint(EffectManifestParameter::UnitsHint::Decibel);
gain2->setNeutralPointOnScale(0.5);
gain2->setRange(-18, 0, 18); // dB

Expand Down
111 changes: 108 additions & 3 deletions src/effects/backends/effectmanifestparameter.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,108 @@ class EffectManifestParameter {

enum class UnitsHint : int {
Unknown = 0,
Time,
/// Multiples of a Beat
Beats,
Beat,
Bar,
BPM,
Cent,
Centimetre,
Coefficient,
Decibel,
Degree,
Frame,
Hertz,
Inch,
KiloHertz,
Kilometer,
Meter,
MegaHertz,
Midinote,
Mile,
Minute,
Millmeter,
Millisecond,
Octave,
Percentage,
/// Fraction of the Sample Rate
SampleRate,
/// Multiples of a Beat
Beats,
Seconds,
Semitone12tet,
Time, // units?
};

const QHash<QString, UnitsHint> lv2UnitToUnitsHintHash{
// Add custom LV2 units here (with correct case) and also
// in unitsHintStringHash()
{QString("bar"), UnitsHint::Bar},
{QString("beat"), UnitsHint::Beat},
{QString("bpm"), UnitsHint::BPM},
{QString("cent"), UnitsHint::Cent},
{QString("cm"), UnitsHint::Centimetre},
{QString("coef"), UnitsHint::Coefficient},
{QString("db"), UnitsHint::Decibel},
{QString("degree"), UnitsHint::Degree},
{QString("frame"), UnitsHint::Frame},
{QString("hz"), UnitsHint::Hertz},
{QString("inch"), UnitsHint::Inch},
{QString("khz"), UnitsHint::KiloHertz},
{QString("km"), UnitsHint::Kilometer},
{QString("m"), UnitsHint::Meter},
{QString("mhz"), UnitsHint::MegaHertz},
{QString("midiNote"), UnitsHint::Midinote},
{QString("mile"), UnitsHint::Mile},
{QString("min"), UnitsHint::Minute},
{QString("mm"), UnitsHint::Millmeter},
{QString("ms"), UnitsHint::Millisecond},
{QString("oct"), UnitsHint::Octave},
{QString("pc"), UnitsHint::Percentage},
{QString("s"), UnitsHint::Seconds},
{QString("semitone12TET"), UnitsHint::Semitone12tet}};

UnitsHint lv2UnitToUnitsHint(const QString& lv2) {
QHash<QString, UnitsHint>::const_iterator uHintIt =
lv2UnitToUnitsHintHash.find(lv2);
if (uHintIt == lv2UnitToUnitsHintHash.constEnd()) {
return UnitsHint::Unknown;
}
return uHintIt.value();
}

const QHash<UnitsHint, QString> unitsHintStringHash{
{UnitsHint::BPM, QString("BPM")},
{UnitsHint::Cent, QString("ct")},
{UnitsHint::Centimetre, QString("cm")},
{UnitsHint::Decibel, QString("dB")},
{UnitsHint::Degree, QString("°")},
{UnitsHint::Frame, QString("f")},
{UnitsHint::Hertz, QString("Hz")},
{UnitsHint::Inch, QString("in")},
{UnitsHint::KiloHertz, QString("kHz")},
{UnitsHint::Kilometer, QString("km")},
{UnitsHint::Meter, QString("m")},
{UnitsHint::MegaHertz, QString("MHz")},
{UnitsHint::Mile, QString("mi")},
{UnitsHint::Minute, QString("min")},
{UnitsHint::Millmeter, QString("mm")},
{UnitsHint::Millisecond, QString("ms")},
{UnitsHint::Octave, QString("oct")},
{UnitsHint::Percentage, QString("%")},
{UnitsHint::Seconds, QString("s")},
{UnitsHint::Semitone12tet, QString("semi")},
};

// Custom units we do not want to use in effect widgets
QSet<QString> customUnitsBlacklist = {
"(coef)",
"G",
"samp",
"st"};

bool ignoreCustomUnit(const QString& unit) {
return customUnitsBlacklist.contains(unit);
}

enum class LinkType : int {
/// Not controlled by the meta knob
None = 0,
Expand Down Expand Up @@ -170,6 +264,10 @@ class EffectManifestParameter {
}
void setUnitsHint(UnitsHint unitsHint) {
m_unitsHint = unitsHint;
m_unitString = EffectManifestParameter::unitsHintStringHash.value(unitsHint);
}
const QString unitString() const {
return m_unitString;
}

LinkType defaultLinkType() const {
Expand Down Expand Up @@ -258,6 +356,7 @@ class EffectManifestParameter {
ParameterType m_parameterType;
ValueScaler m_valueScaler;
UnitsHint m_unitsHint;
QString m_unitString;
LinkType m_defaultLinkType;
LinkInversion m_defaultLinkInversion;
double m_neutralPointOnScale;
Expand All @@ -281,3 +380,9 @@ typedef EffectManifestParameter::ParameterType EffectParameterType;
inline qhash_seed_t qHash(const EffectParameterType& parameterType) {
return static_cast<qhash_seed_t>(parameterType);
}

typedef EffectManifestParameter::UnitsHint UnitsHint;

inline qhash_seed_t qHash(const UnitsHint& uhint) {
return static_cast<qhash_seed_t>(uhint);
}
7 changes: 6 additions & 1 deletion src/effects/backends/lv2/lv2backend.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "effects/backends/lv2/lv2backend.h"

#include <lv2/units/units.h>

#include "effects/backends/lv2/lv2effectprocessor.h"
#include "effects/backends/lv2/lv2manifest.h"

Expand All @@ -25,7 +27,7 @@ void LV2Backend::enumeratePlugins() {
if (lilv_plugin_is_replaced(plug)) {
continue;
}
auto lv2Manifest = LV2EffectManifestPointer::create(plug, m_properties);
auto lv2Manifest = LV2EffectManifestPointer::create(m_pWorld, plug, m_properties);
lv2Manifest->setBackendType(getType());
m_registeredEffects.insert(lv2Manifest->id(), lv2Manifest);
}
Expand All @@ -39,6 +41,9 @@ void LV2Backend::initializeProperties() {
m_properties["button_port"] = lilv_new_uri(m_pWorld, LV2_CORE__toggled);
m_properties["integer_port"] = lilv_new_uri(m_pWorld, LV2_CORE__integer);
m_properties["enumeration_port"] = lilv_new_uri(m_pWorld, LV2_CORE__enumeration);
m_properties["unit"] = lilv_new_uri(m_pWorld, LV2_UNITS__unit);
m_properties["unit_prefix"] = lilv_new_uri(m_pWorld, LV2_UNITS_PREFIX);
m_properties["unit_symbol"] = lilv_new_uri(m_pWorld, LV2_UNITS__symbol);
}

const QList<QString> LV2Backend::getEffectIds() const {
Expand Down
71 changes: 52 additions & 19 deletions src/effects/backends/lv2/lv2manifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
#include "effects/backends/effectmanifestparameter.h"
#include "util/fpclassify.h"

LV2Manifest::LV2Manifest(const LilvPlugin* plug,
namespace {
constexpr bool lv2ParamDebug = true;
} // namespace

LV2Manifest::LV2Manifest(LilvWorld* world,
const LilvPlugin* plug,
QHash<QString, LilvNode*>& properties)
: EffectManifest(),
m_minimum(lilv_plugin_get_num_ports(plug)),
Expand Down Expand Up @@ -47,6 +52,7 @@ LV2Manifest::LV2Manifest(const LilvPlugin* plug,
}
}

// Does this detect a knob/slider parameter?
if (lilv_port_is_a(m_pLV2plugin, port, properties["control_port"]) &&
!lilv_port_has_property(
m_pLV2plugin, port, properties["enumeration_port"]) &&
Expand All @@ -60,29 +66,59 @@ LV2Manifest::LV2Manifest(const LilvPlugin* plug,

// Get and set the parameter name
info = lilv_port_get_name(m_pLV2plugin, port);
QString paramName = lilv_node_as_string(info);
param->setName(paramName);
param->setName(lilv_node_as_string(info));
lilv_node_free(info);

const LilvNode* node = lilv_port_get_symbol(m_pLV2plugin, port);
QString symbol = lilv_node_as_string(node);
param->setId(symbol);
param->setId(lilv_node_as_string(node));
// node must not be freed here, it is owned by port

param->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
EffectManifestParameter::UnitsHint unitsHint =
EffectManifestParameter::UnitsHint::Unknown;
LilvNodes* units = lilv_port_get_value(m_pLV2plugin, port, properties["unit"]);
if (lilv_nodes_size(units) > 0) {
LilvNode* unit = lilv_nodes_get_first(units);
// If this is a unit symbol URI, e.g. http://lv2plug.in/ns/extensions/units#ms
if (lilv_node_is_uri(unit)) {
QString unitStr = lilv_node_as_uri(unit);
// that starts with the 'units' prefix, isolate the identifier
if (unitStr.startsWith(lilv_node_as_string(properties["unit_prefix"]))) {
unitsHint = param->lv2UnitToUnitsHint(
unitStr.remove(lilv_node_as_string(properties["unit_prefix"])));
}
} else { // Try to extract the custom unit symbol string
LilvNode* customUnit =
lilv_world_get(world, unit, properties["unit_symbol"], NULL);
if (customUnit) {
ronso0 marked this conversation as resolved.
Show resolved Hide resolved
// Accepted custom units needs to be 'whitelisted' in
// EffectManifestParameter::lv2UnitToUnitsHint and added to
// EffectManifestParameter::unitsHintStringHash
unitsHint = param->lv2UnitToUnitsHint(
lilv_node_as_string(customUnit));
if (lv2ParamDebug &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should be always enabled. If it happens on a user machine it will go to the mixxx.log and we can request this during bug hunting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, yes, but in my case with the LSP suite it prints hundreds of lines with G

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can leave it enabled until the release so we can easily look for missing units?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we know of the G case and have decided to use no unit for it. How about skip the message in this case. This also gives a chance to drop a comment about to our decision.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, for G it's a simple check. if the blacklist grows we need to rework this though.

unitsHint == EffectManifestParameter::UnitsHint::Unknown &&
!param->ignoreCustomUnit(lilv_node_as_string(customUnit))) {
qDebug().nospace()
<< "LV2Manifest: plugin \""
<< lilv_node_as_string(lilv_plugin_get_name(
m_pLV2plugin))
<< "\" has custom unit \""
<< lilv_node_as_string(customUnit)
<< "\" for parameter " << param->name();
}
}
}
}
param->setUnitsHint(unitsHint);
lilv_nodes_free(units);

if (util_isnan(m_default[i]) || m_default[i] < m_minimum[i] ||
m_default[i] > m_maximum[i]) {
m_default[i] = m_minimum[i];
}
param->setRange(m_minimum[i], m_default[i], m_maximum[i]);

// Set the appropriate Hints
if (lilv_port_has_property(m_pLV2plugin, port, properties["button_port"])) {
param->setValueScaler(EffectManifestParameter::ValueScaler::Toggle);
} else if (lilv_port_has_property(m_pLV2plugin, port, properties["enumeration_port"])) {
buildEnumerationOptions(port, param);
param->setValueScaler(EffectManifestParameter::ValueScaler::Toggle);
} else if (lilv_port_has_property(m_pLV2plugin, port, properties["integer_port"])) {
if (lilv_port_has_property(m_pLV2plugin, port, properties["integer_port"])) {
param->setValueScaler(EffectManifestParameter::ValueScaler::Integral);
} else {
param->setValueScaler(EffectManifestParameter::ValueScaler::Linear);
Expand All @@ -102,13 +138,11 @@ LV2Manifest::LV2Manifest(const LilvPlugin* plug,

// Get and set the parameter name
info = lilv_port_get_name(m_pLV2plugin, port);
QString paramName = lilv_node_as_string(info);
param->setName(paramName);
param->setName(lilv_node_as_string(info));
lilv_node_free(info);

const LilvNode* node = lilv_port_get_symbol(m_pLV2plugin, port);
QString symbol = lilv_node_as_string(node);
param->setId(symbol);
param->setId(lilv_node_as_string(node));
// info must not be freed here, it is owned by port

param->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
Expand Down Expand Up @@ -181,9 +215,8 @@ void LV2Manifest::buildEnumerationOptions(const LilvPort* port,
const LilvScalePoint* option = lilv_scale_points_get(options, iterator);
const LilvNode* description = lilv_scale_point_get_label(option);
const LilvNode* value = lilv_scale_point_get_value(option);
QString strDescription(lilv_node_as_string(description));

param->appendStep(qMakePair(strDescription,
param->appendStep(qMakePair(QString(lilv_node_as_string(description)),
static_cast<double>(lilv_node_as_float(value))));
}

Expand Down
2 changes: 1 addition & 1 deletion src/effects/backends/lv2/lv2manifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class LV2Manifest : public EffectManifest {
HAS_REQUIRED_FEATURES
};

LV2Manifest(const LilvPlugin* plug, QHash<QString, LilvNode*>& properties);
LV2Manifest(LilvWorld* world, const LilvPlugin* plug, QHash<QString, LilvNode*>& properties);

QList<int> getAudioPortIndices();
QList<int> getControlPortIndices();
Expand Down
Loading