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

PitchShiftEffect: extend effect options #4901

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6513250
PitchShiftEffect: add wide range option
davidchocholaty Aug 19, 2022
ab124c3
PitchShiftEffect: add semitones mode option
davidchocholaty Aug 19, 2022
b913b7e
math: add sgn function
davidchocholaty Aug 19, 2022
cd431e6
PitchShiftEffect: add range mode option
davidchocholaty Aug 19, 2022
bc09b72
PitchShiftEffect: add a formant preserving option
davidchocholaty Aug 21, 2022
2c57997
PitchShiftEffect: remove insignificant TODO
davidchocholaty Aug 21, 2022
150f647
math: clean up sgn function
davidchocholaty Aug 21, 2022
9ea8322
PitchShiftEffect: semitones mode description
davidchocholaty Aug 21, 2022
c49712a
PitchShiftEffect: replace formant description
davidchocholaty Aug 21, 2022
ba5e5d4
PitchShiftEffect: remove range mode and wide range
davidchocholaty Aug 22, 2022
d44fc12
Pitch shift effect: add Range option
davidchocholaty Aug 22, 2022
c60407c
PitchShiftEffect: update semitones description
davidchocholaty Aug 22, 2022
0d9bc0c
PitchShiftEffect: extend formant description
davidchocholaty Aug 23, 2022
5901cfd
PitchShiftEffect: formant into formant preserving
davidchocholaty Aug 23, 2022
9c42ae2
Merge branch 'main' into extend_pitch_shift_effect_options
Swiftb0y Aug 24, 2022
7eef608
PitchShiftEffect: rename kSemitones constant
davidchocholaty Aug 25, 2022
cd26497
PitchShiftEffect: replace flip-flopping
davidchocholaty Aug 25, 2022
7d40f6d
PitchShiftEffect: fix segmentation fault
davidchocholaty Aug 26, 2022
5d98c7f
PitchShiftEffect: update version
davidchocholaty Aug 26, 2022
fe7fe7d
PitchShiftEffect: add id literals into a namespace
davidchocholaty Aug 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 101 additions & 6 deletions src/effects/backends/builtin/pitchshifteffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

#include "util/sample.h"

namespace {
static constexpr SINT kDefaultNorm = 2;
static constexpr SINT kSemitones = 12;
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
static constexpr SINT kWideRangeOctaves = 2;
} // anonymous namespace

PitchShiftEffect::PitchShiftEffect()
: m_currentFormant(false) {
}

PitchShiftGroupState::PitchShiftGroupState(
const mixxx::EngineParameters& engineParameters)
: EffectState(engineParameters) {
Expand Down Expand Up @@ -61,12 +71,69 @@ EffectManifestPointer PitchShiftEffect::getManifest() {
pitch->setNeutralPointOnScale(0.0);
pitch->setRange(-1.0, 0.0, 1.0);

EffectManifestParameterPointer rangeMode = pManifest->addParameter();
rangeMode->setId("rangeMode");
rangeMode->setName(QObject::tr("Mode"));
rangeMode->setShortName(QObject::tr("Mode"));
rangeMode->setDescription(QObject::tr(
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
"Set the range mode. There are three possible options "
"based on the knob position:\n"
"neutral:\n"
"\t - knob: default\n"
"\t - range: from the lowest pitch to the highest pitch\n"
"positive:\n"
"\t - knob: maximum\n"
"\t - range: from the default pitch to the highest pitch\n"
"negative:\n"
"\t - knob: minimum\n"
"\t - range: from the default pitch to the lowest pitch"));
rangeMode->setValueScaler(EffectManifestParameter::ValueScaler::Linear);
rangeMode->setDefaultLinkType(EffectManifestParameter::LinkType::Linked);
rangeMode->setNeutralPointOnScale(0.0);
rangeMode->setRange(-1.0, 0.0, 1.0);

EffectManifestParameterPointer semitonesMode = pManifest->addParameter();
semitonesMode->setId("semitonesMode");
semitonesMode->setName(QObject::tr("Semitones"));
semitonesMode->setShortName(QObject::tr("Semitones"));
semitonesMode->setDescription(QObject::tr(
"Set the pitch scale to the semitones mode "
"instead of the contiguous mode (set as default)."));
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
semitonesMode->setValueScaler(EffectManifestParameter::ValueScaler::Toggle);
semitonesMode->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
semitonesMode->setRange(0, 1, 1);

EffectManifestParameterPointer wideRange = pManifest->addParameter();
wideRange->setId("wideRange");
wideRange->setName(QObject::tr("Wide Range"));
wideRange->setShortName(QObject::tr("Wide Range"));
wideRange->setDescription(QObject::tr(
"Double the pitch scale range."));
wideRange->setValueScaler(EffectManifestParameter::ValueScaler::Toggle);
wideRange->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
wideRange->setRange(0, 0, 1);

EffectManifestParameterPointer formant = pManifest->addParameter();
formant->setId("formant");
formant->setName(QObject::tr("Formant"));
Copy link
Member

Choose a reason for hiding this comment

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

It looks like RB "engine fine" actually supports to shift the formants. If we consider this, the name "Formant" is better used for the formant rotary knob. This one is "Formant Preserving" in case we use the LV2 variant. Is the length still OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, in the R3 version the formant can be shifted using a knob. With introducing the R3 version in the Pitch shift effect, this option of course could be added and if it would be, the identical toggle name will be the problem too. On the other hand, the "Formant Preserving" name is too wide, see the print screen:

image

It looks like the maximum number of characters is about 10.

Copy link
Member

Choose a reason for hiding this comment

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

Then just name it formant and put the detailed explanation in the tooltip imo.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but at least the Id should not be "formant". Since the name can be changed at any time, but not the ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, that looks good to me.

formant->setShortName(QObject::tr("Formant"));
formant->setDescription(QObject::tr(
"Applies the Formant Preserving processing "
"and activates the handling of the formant shape."));
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
formant->setValueScaler(EffectManifestParameter::ValueScaler::Toggle);
formant->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown);
formant->setRange(0, 0, 1);

return pManifest;
}

void PitchShiftEffect::loadEngineEffectParameters(
const QMap<QString, EngineEffectParameterPointer>& parameters) {
m_pPitchParameter = parameters.value("pitch");
m_pRangeModeParameter = parameters.value("rangeMode");
m_pSemitonesModeParameter = parameters.value("semitonesMode");
m_pWideRangeParameter = parameters.value("wideRange");
m_pFormantParameter = parameters.value("formant");
}

void PitchShiftEffect::processChannel(
Expand All @@ -79,16 +146,44 @@ void PitchShiftEffect::processChannel(
Q_UNUSED(groupFeatures);
Q_UNUSED(enableState);

const double pitchParameter = m_pPitchParameter->value();

const double pitch = 1.0 + [=] {
if (pitchParameter < 0.0) {
return pitchParameter / 2.0;
if (m_currentFormant != m_pFormantParameter->toBool()) {
m_currentFormant = !m_currentFormant;

pState->m_pRubberBand->setFormantOption(m_currentFormant
? RubberBand::RubberBandStretcher::
OptionFormantPreserved
: RubberBand::RubberBandStretcher::
OptionFormantShifted);
}

const double rangeModeParameter = m_pRangeModeParameter->value();

double pitchParameter = [=, this] {
if (std::abs(rangeModeParameter) < 0.5) {
if (m_pWideRangeParameter->toBool()) {
return m_pPitchParameter->value() * kWideRangeOctaves;
} else {
return m_pPitchParameter->value();
}
} else {
return pitchParameter;
if (m_pWideRangeParameter->toBool()) {
return sgn(rangeModeParameter) * kWideRangeOctaves *
(m_pPitchParameter->value() + 1) /
kDefaultNorm;
} else {
return sgn(rangeModeParameter) *
(m_pPitchParameter->value() + 1) /
kDefaultNorm;
}
}
}();

if (m_pSemitonesModeParameter->toBool()) {
pitchParameter = roundToFraction(pitchParameter, kSemitones);
}

const double pitch = std::pow(2.0, pitchParameter);

pState->m_pRubberBand->setPitchScale(pitch);

SampleUtil::deinterleaveBuffer(
Expand Down
8 changes: 7 additions & 1 deletion src/effects/backends/builtin/pitchshifteffect.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "engine/effects/engineeffectparameter.h"
#include "util/class.h"
#include "util/defs.h"
#include "util/math.h"
#include "util/sample.h"
#include "util/types.h"

Expand All @@ -30,7 +31,7 @@ class PitchShiftGroupState : public EffectState {

class PitchShiftEffect final : public EffectProcessorImpl<PitchShiftGroupState> {
public:
PitchShiftEffect() = default;
PitchShiftEffect();

static QString getId();
static EffectManifestPointer getManifest();
Expand All @@ -51,7 +52,12 @@ class PitchShiftEffect final : public EffectProcessorImpl<PitchShiftGroupState>
return getId();
}

bool m_currentFormant;
EngineEffectParameterPointer m_pPitchParameter;
EngineEffectParameterPointer m_pRangeModeParameter;
EngineEffectParameterPointer m_pSemitonesModeParameter;
EngineEffectParameterPointer m_pWideRangeParameter;
EngineEffectParameterPointer m_pFormantParameter;

DISALLOW_COPY_AND_ASSIGN(PitchShiftEffect);
};
6 changes: 6 additions & 0 deletions src/util/math.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,9 @@ inline const T db2ratio(const T a) {
static_assert(std::is_floating_point_v<T>, "db2ratio works only for floating point type");
return static_cast<T>(pow(10, a / 20));
}

template<typename T>
inline const T sgn(const T a) {
static_assert(std::is_arithmetic_v<T>, "the value has to be an arithmetic type");
return (a > T(0)) - (a < T(0));
}
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved