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

Add validation to control objects #9

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 23 additions & 5 deletions mixxx/src/control/control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <QMutexLocker>

#include "control/control.h"
#include "controlobject.h"
Copy link
Member

Choose a reason for hiding this comment

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

Ideally control.h/cpp shouldn't depend on controlobject.h/cpp


#include "util/stat.h"
#include "util/timer.h"
Expand All @@ -11,7 +12,9 @@ QHash<ConfigKey, ControlDoublePrivate*> ControlDoublePrivate::m_sqCOHash;
QMutex ControlDoublePrivate::m_sqCOHashMutex;

ControlDoublePrivate::ControlDoublePrivate()
: m_bIgnoreNops(true),
: m_Enabled(true),
m_pValidator(NULL),
m_bIgnoreNops(true),
m_bTrack(false) {
m_defaultValue.setValue(0);
m_value.setValue(0);
Expand All @@ -20,6 +23,8 @@ ControlDoublePrivate::ControlDoublePrivate()
ControlDoublePrivate::ControlDoublePrivate(ConfigKey key,
bool bIgnoreNops, bool bTrack)
: m_key(key),
m_Enabled(true),
m_pValidator(NULL),
m_bIgnoreNops(bIgnoreNops),
m_bTrack(bTrack),
m_trackKey("control " + m_key.group + "," + m_key.item),
Expand All @@ -45,6 +50,9 @@ ControlDoublePrivate::~ControlDoublePrivate() {
m_sqCOHashMutex.lock();
m_sqCOHash.remove(m_key);
m_sqCOHashMutex.unlock();
if (m_pValidator) {
delete m_pValidator;
}
}

// static
Expand Down Expand Up @@ -82,16 +90,26 @@ void ControlDoublePrivate::reset(QObject* pSender) {
set(defaultValue, pSender);
}

void ControlDoublePrivate::set(const double& value, QObject* pSender) {
bool ControlDoublePrivate::set(const double& value, QObject* pSender) {
if (!m_Enabled.getValue()) {
return false;
}

if (m_bIgnoreNops && get() == value) {
return;
return true;
}

double dValue = value;
// If the behavior says to ignore the set, ignore it.
ControlNumericBehavior* pBehavior = m_pBehavior;
if (pBehavior && !pBehavior->setFilter(&dValue)) {
return;
return false;
}

// Copy to a local to prevent race conditions
ControlValidator* validator = m_pValidator;
if (validator && validator->validateChange(dValue)) {
return false;
}
m_value.setValue(dValue);
emit(valueChanged(dValue, pSender));
Expand All @@ -100,6 +118,7 @@ void ControlDoublePrivate::set(const double& value, QObject* pSender) {
Stat::track(m_trackKey, static_cast<Stat::StatType>(m_trackType),
static_cast<Stat::ComputeFlags>(m_trackFlags), dValue);
}
return true;
}

ControlNumericBehavior* ControlDoublePrivate::setBehavior(ControlNumericBehavior* pBehavior) {
Expand Down Expand Up @@ -129,4 +148,3 @@ double ControlDoublePrivate::getMidiParameter() const {
ControlNumericBehavior* pBehavior = m_pBehavior;
return pBehavior ? pBehavior->valueToMidiParameter(get()) : get();
}

21 changes: 19 additions & 2 deletions mixxx/src/control/control.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <QAtomicPointer>

#include "control/controlbehavior.h"
#include "control/controlvalidator.h"
#include "control/controlvalue.h"
#include "configobject.h"

Expand All @@ -31,8 +32,19 @@ class ControlDoublePrivate : public QObject {
return getControl(key, bCreate, bIgnoreNops, bTrack);
}

// Sets the control value.
void set(const double& value, QObject* pSetter);
inline bool getEnabled() const { return m_Enabled.getValue(); }

inline void setEnabled(bool e) { m_Enabled.setValue(e); }

void setValidator(ControlValidator* validator) {
if (m_pValidator) {
delete m_pValidator;
}
m_pValidator = validator;
}

// Sets the control value. Returns true if the set was successful.1
bool set(const double& value, QObject* pSetter);
// Gets the control value.
double get() const;
// Resets the control value to its default.
Expand Down Expand Up @@ -69,6 +81,11 @@ class ControlDoublePrivate : public QObject {

private:
ConfigKey m_key;

// Enabled/Disabled state overrides any validation that may exist.
ControlValueAtomic<bool> m_Enabled;
// Optional custom validation object to decide if the set command should be allowed.
ControlValidator* m_pValidator;
// Whether to ignore sets which would have no effect.
bool m_bIgnoreNops;

Expand Down
13 changes: 13 additions & 0 deletions mixxx/src/control/controlvalidator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef CONTROLVALIDATOR_H
#define CONTROLVALIDATOR_H

class ControlValidator {
public:
virtual ~ControlValidator() {};

// Subclasses override this function to provide a way to reject invalid settings.
// Returns true if the change is valid.
virtual bool validateChange(double value) const = 0;
};

#endif // CONTROLVALIDATOR_H
5 changes: 5 additions & 0 deletions mixxx/src/control/controlvalue.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ class ControlValueAtomic
ControlValueAtomic()
: ControlValueAtomicBase<T, sizeof(T) <= sizeof(void*)>() {
}

ControlValueAtomic(T val)
: ControlValueAtomicBase<T, sizeof(T) <= sizeof(void*)>() {
this->setValue(val);
}
};

#endif /* CONTROLVALUE_H */
1 change: 1 addition & 0 deletions mixxx/src/controlobject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "controlobject.h"
#include "controlevent.h"
#include "control/control.h"
#include "control/controlvalidator.h"
#include "util/stat.h"
#include "util/timer.h"

Expand Down
21 changes: 21 additions & 0 deletions mixxx/src/controlobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "configobject.h"
#include "controllers/midi/midimessage.h"
#include "control/control.h"
#include "control/controlvalidator.h"

class ControlObject : public QObject {
Q_OBJECT
Expand All @@ -36,6 +37,12 @@ class ControlObject : public QObject {
bool bIgnoreNops=true, bool bTrack=false);
virtual ~ControlObject();

void setValidator(ControlValidator* validator) {
if (m_pControl) {
m_pControl->setValidator(validator);
}
}

/** Returns a pointer to the ControlObject matching the given ConfigKey */
static ControlObject* getControl(const ConfigKey& key);
static inline ControlObject* getControl(const QString& group, const QString& item) {
Expand Down Expand Up @@ -64,6 +71,20 @@ class ControlObject : public QObject {
return m_pControl ? m_pControl->defaultValue() : 0.0;
}

inline bool getEnabled() const {
if (m_pControl) {
return m_pControl->getEnabled();
} else {
return false;
}
}

inline void setEnabled(bool e) {
if (m_pControl) {
m_pControl->setEnabled(e);
}
}

signals:
void valueChanged(double);
void valueChangedFromEngine(double);
Expand Down
16 changes: 15 additions & 1 deletion mixxx/src/controlobjectthread.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
#include <QQueue>

#include "configobject.h"
#include "control/control.h"

class ControlDoublePrivate;
class ControlObject;

class ControlObjectThread : public QObject {
Expand All @@ -45,6 +45,20 @@ class ControlObjectThread : public QObject {
return m_key;
}

inline bool getEnabled() const {
if (m_pControl) {
return m_pControl->getEnabled();
} else {
return false;
}
}

inline void setEnabled(bool e) {
if (m_pControl) {
m_pControl->setEnabled(e);
}
}

// Returns the value of the object. Thread safe, non-blocking.
virtual double get();

Expand Down
23 changes: 3 additions & 20 deletions mixxx/src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ EngineBuffer::EngineBuffer(const char * _group, ConfigObject<ConfigValue> * _con
// Play button
m_playButton = new ControlPushButton(ConfigKey(m_group, "play"));
m_playButton->setButtonMode(ControlPushButton::TOGGLE);
connect(m_playButton, SIGNAL(valueChanged(double)),
this, SLOT(slotControlPlay(double)),
Qt::DirectConnection);

//Play from Start Button (for sampler)
m_playStartButton = new ControlPushButton(ConfigKey(m_group, "start_play"));
Expand Down Expand Up @@ -383,6 +380,7 @@ void EngineBuffer::slotTrackLoading() {
m_iTrackLoading = 1;
m_pause.unlock();

m_playButton->setEnabled(true);
m_playButton->set(0.0); //Stop playback
m_pTrackSamples->set(0); // Stop renderer
}
Expand All @@ -398,6 +396,7 @@ void EngineBuffer::slotTrackLoaded(TrackPointer pTrack,
m_file_length_old = iTrackNumSamples;
m_pTrackSamples->set(iTrackNumSamples);
m_pTrackSampleRate->set(iTrackSampleRate);
m_playButton->setEnabled(true);
m_pause.unlock();

// All EngingeControls are connected directly
Expand Down Expand Up @@ -441,6 +440,7 @@ void EngineBuffer::ejectTrack() {
m_playButton->set(0.0);
m_visualBpm->set(0.0);
slotControlSeek(0.);
m_playButton->setEnabled(false);
m_pause.unlock();

emit(trackUnloaded(pTrack));
Expand Down Expand Up @@ -475,16 +475,6 @@ void EngineBuffer::slotControlSeekAbs(double abs)
slotControlSeek(abs / m_file_length_old);
}

void EngineBuffer::slotControlPlay(double v)
{
// If no track is currently loaded, turn play off. If a track is loading
// allow the set since it might apply to a track we are loading due to the
// asynchrony.
if (v > 0.0 && !m_pCurrentTrack && m_iTrackLoading == 0) {
m_playButton->set(0.0f);
}
}

void EngineBuffer::slotControlStart(double v)
Copy link
Member

Choose a reason for hiding this comment

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

yey! :)

{
if (v > 0.0) {
Expand Down Expand Up @@ -895,13 +885,6 @@ void EngineBuffer::bindWorkers(EngineWorkerScheduler* pWorkerScheduler) {
pWorkerScheduler->bindWorker(m_pReader);
}

bool EngineBuffer::isTrackLoaded() {
if (m_pCurrentTrack) {
return true;
}
return false;
}

void EngineBuffer::slotEjectTrack(double v) {
if (v > 0) {
ejectTrack();
Expand Down
10 changes: 7 additions & 3 deletions mixxx/src/engine/enginebuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <QAtomicInt>

#include "defs.h"
#include "controlobject.h"
#include "engine/engineobject.h"
#include "trackinfoobject.h"
#include "configobject.h"
Expand All @@ -39,11 +40,11 @@ class RateControl;
class LoopingControl;
class ReadAheadManager;
class ControlObject;
class ControlPushButton;
class ControlObjectThreadMain;
class ControlBeat;
class ControlTTRotary;
class ControlPotmeter;
class ControlPushButton;
class CachingReader;
class EngineBufferScale;
class EngineBufferScaleDummy;
Expand Down Expand Up @@ -81,6 +82,8 @@ const int ENGINE_RAMP_UP = 1;

//const int kiRampLength = 3;

class EngineBuffer;

class EngineBuffer : public EngineObject
{
Q_OBJECT
Expand Down Expand Up @@ -112,14 +115,14 @@ class EngineBuffer : public EngineObject
void process(const CSAMPLE *pIn, const CSAMPLE *pOut, const int iBufferSize);

const char* getGroup();
bool isTrackLoaded();
bool isTrackLoaded() const { return m_pCurrentTrack != NULL; }

TrackPointer getLoadedTrack() const;

// For dependency injection of readers.
void setReader(CachingReader* pReader);

public slots:
void slotControlPlay(double);
void slotControlPlayFromStart(double);
void slotControlJumpToStartAndStop(double);
void slotControlStop(double);
Expand Down Expand Up @@ -275,6 +278,7 @@ class EngineBuffer : public EngineObject
//int m_iRampIter;

TrackPointer m_pCurrentTrack;

#ifdef __SCALER_DEBUG__
QFile df;
QTextStream writer;
Expand Down