Skip to content

Commit

Permalink
refactor: modernize Softakeover
Browse files Browse the repository at this point in the history
* eliminate unnecessary allocation in `EffectKnobParameterSlot`
* remove unnecessary allocation in `SoftTakeoverTest*`
* eliminate checks in `SoftTakeoverCtrl::enable` by encoding constraints
  in the type system
* deduplicate common code in `SoftTakeoverTest`. Now bundled as
  `SoftTakeoverTestWithValue`
* remove unnecessary allocation in `SoftTakeoverCtrl`
* many more miscellaneous improvements to `SoftTakeover(-Ctrl)`.
  • Loading branch information
Swiftb0y committed Aug 10, 2024
1 parent 72610b1 commit 38dfdc4
Show file tree
Hide file tree
Showing 16 changed files with 356 additions and 586 deletions.
7 changes: 6 additions & 1 deletion src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <algorithm>

#include "control/controlobject.h"
#include "control/controlpotmeter.h"
#include "controllers/defs_controllers.h"
#include "controllers/midi/midioutputhandler.h"
#include "controllers/midi/midiutils.h"
Expand Down Expand Up @@ -451,7 +452,11 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping,

if (mapping.options.testFlag(MidiOption::SoftTakeover)) {
// This is the only place to enable it if it isn't already.
m_st.enable(pCO);
auto* pControlPotmeter = qobject_cast<ControlPotmeter*>(pCO);
if (!pControlPotmeter) {
return;
}
m_st.enable(gsl::not_null(pControlPotmeter));
if (m_st.ignore(pCO, pCO->getParameterForMidi(newValue))) {
return;
}
Expand Down
3 changes: 1 addition & 2 deletions src/controllers/midi/midicontroller.h
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
#pragma once

#include <QJSValue>
#include <utility>

#include "controllers/controller.h"
#include "controllers/midi/legacymidicontrollermappingfilehandler.h"
#include "controllers/midi/legacymidicontrollermapping.h"
#include "controllers/midi/midimessage.h"
#include "controllers/softtakeover.h"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#include "controllerscriptinterfacelegacy.h"

#include <gsl/pointers>

#include "control/controlobject.h"
#include "control/controlobjectscript.h"
#include "control/controlpotmeter.h"
#include "controllers/scripting/legacy/controllerscriptenginelegacy.h"
#include "controllers/scripting/legacy/scriptconnectionjsproxy.h"
#include "mixer/playermanager.h"
Expand Down Expand Up @@ -584,11 +587,12 @@ void ControllerScriptInterfaceLegacy::softTakeover(
const QString& group, const QString& name, bool set) {
ControlObject* pControl = ControlObject::getControl(
ConfigKey(group, name), ControlFlag::AllowMissingOrInvalid);
if (!pControl) {
return;
}
if (set) {
m_st.enable(pControl);
auto* pControlPotmeter = qobject_cast<ControlPotmeter*>(pControl);
if (!pControlPotmeter) {
return;
}
m_st.enable(gsl::not_null(pControlPotmeter));
} else {
m_st.disable(pControl);
}
Expand Down
103 changes: 13 additions & 90 deletions src/controllers/softtakeover.cpp
Original file line number Diff line number Diff line change
@@ -1,86 +1,16 @@
#include "controllers/softtakeover.h"
#include "control/controlpotmeter.h"
#include "util/math.h"
#include "util/time.h"

// 3/128 units away from the current is enough to catch fast non-sequential moves
// but not cause an audibly noticeable jump, determined experimentally with
// slow-refresh controllers.
const double SoftTakeover::kDefaultTakeoverThreshold = 3.0 / 128;

const mixxx::Duration SoftTakeover::kSubsequentValueOverrideTime =
mixxx::Duration::fromMillis(50);

SoftTakeoverCtrl::SoftTakeoverCtrl() {

}

SoftTakeoverCtrl::~SoftTakeoverCtrl() {
QHashIterator<ControlObject*, SoftTakeover*> i(m_softTakeoverHash);
while (i.hasNext()) {
i.next();
delete i.value();
}
}

void SoftTakeoverCtrl::enable(ControlObject* control) {
ControlPotmeter* cpo = qobject_cast<ControlPotmeter*>(control);
if (cpo == nullptr) {
// softtakecover works only for continuous ControlPotmeter based COs
return;
}

// Initialize times
if (!m_softTakeoverHash.contains(control)) {
m_softTakeoverHash.insert(control, new SoftTakeover());
}
}

void SoftTakeoverCtrl::disable(ControlObject* control) {
if (control == nullptr) {
return;
}
SoftTakeover* pSt = m_softTakeoverHash.take(control);
if (pSt) {
delete pSt;
}
}

bool SoftTakeoverCtrl::ignore(ControlObject* control, double newParameter) {
if (control == nullptr) {
return false;
}
bool ignore = false;
SoftTakeover* pSt = m_softTakeoverHash.value(control);
if (pSt) {
ignore = pSt->ignore(control, newParameter);
}
return ignore;
}

void SoftTakeoverCtrl::ignoreNext(ControlObject* control) {
if (control == nullptr) {
return;
}

SoftTakeover* pSt = m_softTakeoverHash.value(control);
if (pSt == nullptr) {
return;
}

pSt->ignoreNext();
}
#include "control/controlobject.h"
#include "control/controlpotmeter.h"

SoftTakeover::SoftTakeover()
: m_prevParameter(0),
m_dThreshold(kDefaultTakeoverThreshold) {
}
using namespace std::chrono_literals;

void SoftTakeover::setThreshold(double threshold) {
m_dThreshold = threshold;
void SoftTakeoverCtrl::enable(gsl::not_null<ControlPotmeter*> control) {
// explicitly not in the header to avoid adding dependency on ControlPotmeter
m_softTakeoverHash.try_emplace(static_cast<ControlObject*>(control.get()));
}

bool SoftTakeover::ignore(ControlObject* control, double newParameter) {
bool SoftTakeover::ignore(const ControlObject& control, double newParameter) {
bool ignore = false;
/*
* We only want to ignore the controller when:
Expand Down Expand Up @@ -108,22 +38,19 @@ bool SoftTakeover::ignore(ControlObject* control, double newParameter) {
* Don't ignore in every other case.
*/

mixxx::Duration currentTime = mixxx::Time::elapsed();
auto currentTime = ClockT::now();
// We will get a sudden jump if we don't ignore the first value.
if (m_time == mixxx::Duration::fromMillis(0)) {
if (m_time == kFirstValueTime) {
ignore = true;
// Change the stored time (but keep it far away from the current time)
// so this block doesn't run again.
m_time = mixxx::Duration::fromMillis(1);
// qDebug() << "SoftTakeover::ignore: ignoring the first value"
// << newParameter;
} else if (currentTime - m_time > kSubsequentValueOverrideTime) {
m_time = ClockT::time_point(kFirstValueTime + 1ms);
} else if (currentTime >= m_time + kSubsequentValueOverrideTime) {
// don't ignore value if a previous one was not ignored in time
const double currentParameter = control->getParameter();
const double currentParameter = control.getParameter();
const double difference = currentParameter - newParameter;
const double prevDiff = currentParameter - m_prevParameter;
if ((prevDiff < 0 && difference < 0) ||
(prevDiff > 0 && difference > 0)) {
if (std::signbit(prevDiff) == std::signbit(difference)) {
// On same side of the current parameter value
if (fabs(difference) > m_dThreshold && fabs(prevDiff) > m_dThreshold) {
// differences are above threshold
Expand All @@ -143,7 +70,3 @@ bool SoftTakeover::ignore(ControlObject* control, double newParameter) {

return ignore;
}

void SoftTakeover::ignoreNext() {
m_time = mixxx::Duration::fromMillis(0);
}
93 changes: 67 additions & 26 deletions src/controllers/softtakeover.h
Original file line number Diff line number Diff line change
@@ -1,56 +1,97 @@
#pragma once

#include <QHash>
#include <chrono>
#include <gsl/pointers>
#include <unordered_map>

#include "util/duration.h"
#include "util/assert.h"
#include "util/time.h"

class ControlObject;
class ControlPotmeter;

class SoftTakeover {
public:
// I would initialize it here but that's C++11 coolness. (Because it's a double.)
static const double kDefaultTakeoverThreshold;
// 3/128 units away from the current is enough to catch fast non-sequential moves
// but not cause an audibly noticeable jump, determined experimentally with
// slow-refresh controllers.
// TODO (XXX): Expose this to the controller mapping environment?
static constexpr double kDefaultTakeoverThreshold = 3.0 / 128;

SoftTakeover();
bool ignore(ControlObject* control, double newParameter);
void ignoreNext();
void setThreshold(double threshold);
SoftTakeover() = default;
bool ignore(const ControlObject& control, double newParameter);
void ignoreNext() {
m_time = kFirstValueTime;
}
void setThreshold(double threshold) {
m_dThreshold = threshold;
}

struct TestAccess;
// TODO (XXX): find a better testing solution than this TestAccess
// front-door coupled to `mixxx::Time`.
struct TestAccess {
static constexpr auto getTimeThreshold() {
return kSubsequentValueOverrideTime;
}
template<class Rep = mixxx::Time::rep,
class Period = mixxx::Time::period>
static void advanceTimePastThreshold(
std::chrono::duration<Rep, Period> offset =
std::chrono::nanoseconds(0)) {
mixxx::Time::addTestTime(getTimeThreshold() + offset);
}
};

private:
using ClockT = mixxx::Time;
// If a new value is received within this amount of time, jump to it
// regardless. This allows quickly whipping controls to work while retaining
// the benefits of soft-takeover for slower movements. Setting this too
// high will defeat the purpose of soft-takeover.
static const mixxx::Duration kSubsequentValueOverrideTime;
static constexpr ClockT::duration kSubsequentValueOverrideTime = std::chrono::milliseconds(50);
static constexpr ClockT::time_point kFirstValueTime = ClockT::time_point::min();

mixxx::Duration m_time;
double m_prevParameter;
double m_dThreshold;
};

struct SoftTakeover::TestAccess {
static mixxx::Duration getTimeThreshold() {
return kSubsequentValueOverrideTime;
}
ClockT::time_point m_time{kFirstValueTime};
double m_prevParameter{0};
double m_dThreshold{kDefaultTakeoverThreshold};
};

class SoftTakeoverCtrl {
public:
SoftTakeoverCtrl();
~SoftTakeoverCtrl();
SoftTakeoverCtrl() = default;

// Enable soft-takeover for the given Control.
// This does nothing on a control that already has soft-takeover enabled.
void enable(ControlObject* control);
void enable(gsl::not_null<ControlPotmeter*> control);
// Disable soft-takeover for the given Control
void disable(ControlObject* control);
void disable(ControlObject* control) {
m_softTakeoverHash.erase(control);
}
// Check to see if the new value for the Control should be ignored
bool ignore(ControlObject* control, double newMidiParameter);
bool ignore(ControlObject* control, double newParameter) {
auto coIt = m_softTakeoverHash.find(control);
if (coIt == m_softTakeoverHash.end()) {
return false;
}
VERIFY_OR_DEBUG_ASSERT(control) {
return false;
}
return coIt->second.ignore(*control, newParameter);
}
// Ignore the next supplied parameter
void ignoreNext(ControlObject* control);
void ignoreNext(ControlObject* control) {
auto coIt = m_softTakeoverHash.find(control);
if (coIt == m_softTakeoverHash.end()) {
return;
}

coIt->second.ignoreNext();
}

private:
QHash<ControlObject*, SoftTakeover*> m_softTakeoverHash;
// ControlObjects are borrowed. They must outlive this object.
// Note that even though we can only enable softTakeover on
// `ControlPotmeter`s, we store the base ControlObject to not force the user
// to downcast for `disable()` and `ignore()`/`ignoreNext()`.
std::unordered_map<ControlObject*, SoftTakeover> m_softTakeoverHash;
};
Loading

0 comments on commit 38dfdc4

Please sign in to comment.