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

MIDI Clock Input #702

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
5d826c9
MidiClock: a class that listens to incoming midi clock messages
ywwg Jun 14, 2015
e46cceb
Enable -std=c++11 support.
rryan Jun 4, 2015
5518af1
MidiClock: Actually commit the source files
ywwg Jun 14, 2015
098c92e
Midi Clock master works
ywwg Jun 15, 2015
02cd418
MidiClock: remove atomic / threading stuff since it's not needed..
ywwg Jun 15, 2015
a9df9a9
MidiClock: Add indication that midi is running or not.
ywwg Jun 15, 2015
b1f93bd
MidiClock: Add a sync tweak button to adjust for midi offset
ywwg Jun 16, 2015
26e4ecd
Merge branch '1.12' into midi-master
ywwg Jun 16, 2015
afdd57b
MidiClock: Fix midi not updating follower rate sliders
ywwg Jun 18, 2015
96936f4
LateNight: Fix effect unit getting too big when fourth unit is populated
ywwg Jun 21, 2015
f9ff1ca
move midi sync to below right decks
ywwg Jun 21, 2015
8286af1
MidiClock: Always update bpm on every callback
ywwg Jun 21, 2015
421edb7
Merge branch 'master' into midi-master
ywwg Sep 1, 2015
4857f4a
MidiMaster: Cleanup, clamp bpm calculation to a range
ywwg Sep 2, 2015
e1ebbb5
MidiMaster: Rename MidiClock to MidiSourceClock
ywwg Sep 2, 2015
eb405df
Merge branch 'master' into midi-master
ywwg Sep 2, 2015
5cf310b
Merge branch 'master' into midi-master
ywwg Nov 22, 2015
d038c61
Merge branch 'master' into midi-master
ywwg Nov 22, 2015
c4edd54
MidiMaster: Address notes
ywwg Nov 22, 2015
8ea12bf
MidiMaster: Propagate ConfigObject down to midicontroller where it's …
ywwg Nov 22, 2015
8530c02
Merge branch 'master' into midi-master
ywwg Jan 30, 2016
2a0e34a
Merge branch 'master' into midi-master
ywwg Feb 22, 2016
e6a281a
Midi Master: Update with latest trunk API changes
ywwg Feb 28, 2016
b20d51b
Merge branch 'master' into midi-master
ywwg Feb 28, 2016
c3aad6c
Midi Master: Use external timestamps
ywwg Mar 4, 2016
66b6c54
Midi Master: Fix tests, which were so tolerant they couldn't fail
ywwg Mar 4, 2016
1b2a11c
Midi Master: Disable setting of midi master until UI is done
ywwg Mar 5, 2016
c9be401
Merge branch 'master' into midi-master
ywwg May 4, 2016
9bffb9f
Merge branch 'master' into midi-master
ywwg Nov 19, 2016
98f313d
Merge branch 'midi-master' of https://github.com/ywwg/mixxx into midi…
Holzhaus Feb 6, 2021
e3468de
Merge pull request #11 from Holzhaus/midi-clock-input
ywwg Feb 7, 2021
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
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/controllers/midi/midienumerator.cpp
src/controllers/midi/midimessage.cpp
src/controllers/midi/midioutputhandler.cpp
src/controllers/midi/midisourceclock.cpp
src/controllers/midi/midiutils.cpp
src/controllers/midi/portmidicontroller.cpp
src/controllers/midi/portmidienumerator.cpp
Expand Down Expand Up @@ -544,6 +545,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/engine/sync/enginesync.cpp
src/engine/sync/internalclock.cpp
src/engine/sync/synccontrol.cpp
src/engine/sync/midimaster.cpp
src/errordialoghandler.cpp
src/library/analysisfeature.cpp
src/library/analysislibrarytablemodel.cpp
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/controllermanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void ControllerManager::slotInitialize() {

// Instantiate all enumerators. Enumerators can take a long time to
// construct since they interact with host MIDI APIs.
m_enumerators.append(new PortMidiEnumerator());
m_enumerators.append(new PortMidiEnumerator(m_pConfig));
#ifdef __HSS1394__
m_enumerators.append(new Hss1394Enumerator(m_pConfig));
#endif
Expand Down
40 changes: 28 additions & 12 deletions src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "controllers/midi/midicontroller.h"

#include "control/controlobject.h"
#include "control/controlproxy.h"
#include "controllers/controllerdebug.h"
#include "controllers/defs_controllers.h"
#include "controllers/midi/midiutils.h"
Expand All @@ -11,15 +12,23 @@
#include "util/math.h"
#include "util/screensaver.h"

MidiController::MidiController()
: Controller() {
MidiController::MidiController(UserSettingsPointer config)
: Controller(), m_pConfig(config) {
m_pClockBpm = new ControlProxy("[MidiSourceClock]", "bpm", this);
m_pClockLastBeat = new ControlProxy("[MidiSourceClock]",
"last_beat_time",
this);
m_pClockRunning = new ControlProxy("[MidiSourceClock]", "run", this);
setDeviceCategory(tr("MIDI Controller"));
}

MidiController::~MidiController() {
destroyOutputHandlers();
// Don't close the device here. Sub-classes should close the device in their
// destructors.
delete m_pClockRunning;
delete m_pClockLastBeat;
delete m_pClockBpm;
}

ControllerJSProxy* MidiController::jsProxy() {
Expand Down Expand Up @@ -206,18 +215,25 @@ void MidiController::receivedShortMessage(unsigned char status,
unsigned char channel = MidiUtils::channelFromStatus(status);
unsigned char opCode = MidiUtils::opCodeFromStatus(status);

// Ignore MIDI beat clock messages (0xF8) until we have proper MIDI sync in
// Mixxx. These messages are not suitable to use in JS anyway, as they are
// sent at 24 ppqn (i.e. one message every 20.83 ms for a 120 BPM track)
// and require real-time code. Currently, they are only spam on the
// console, inhibit the screen saver unintentionally, could potentially
// slow down Mixxx or interfere with the learning wizard.
if (status == 0xF8) {
controllerDebug(MidiUtils::formatMidiMessage(
getName(), status, control, value, channel, opCode, timestamp));

// If MidiSourceClock handles the message, record the updated values and
// no further action is needed. Note that the clock code is active even if
// this device is not master, so if the user changes masters all of the data
// is ready to be used instantly.
if (m_midiSourceClock.handleMessage(status, timestamp)) {
// TODO(owen): Use preferences to determine if we are clock master and only
// set values if so.
// TODO(owen): Enable midi clock handling when UI is finished.
if (false) {
m_pClockBpm->set(m_midiSourceClock.bpm());
m_pClockLastBeat->set(m_midiSourceClock.smoothedBeatTime().toDoubleNanos());
m_pClockRunning->set(static_cast<double>(m_midiSourceClock.running()));
}
return;
}

controllerDebug(MidiUtils::formatMidiMessage(getName(), status, control, value,
channel, opCode, timestamp));
MidiKey mappingKey(status, control);

triggerActivity();
Expand Down Expand Up @@ -413,7 +429,7 @@ double MidiController::computeValue(
newmidivalue = newmidivalue - 128.;
}
//Apply sensitivity to signed value. FIXME
// if(sensitivity > 0)
// if(sensitivity > 0)
// _newmidivalue = _newmidivalue * ((double)sensitivity / 50.);
//Apply new value to current value.
newmidivalue = prevmidivalue + newmidivalue;
Expand Down
12 changes: 11 additions & 1 deletion src/controllers/midi/midicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
#include "controllers/midi/legacymidicontrollermappingfilehandler.h"
#include "controllers/midi/midimessage.h"
#include "controllers/midi/midioutputhandler.h"
#include "controllers/midi/midisourceclock.h"
#include "controllers/softtakeover.h"

class ControlProxy;
class DlgControllerLearning;

/// MIDI Controller base class
Expand All @@ -15,10 +17,11 @@ class DlgControllerLearning;
/// It must be inherited by a class that implements it on some API.
///
/// Note that the subclass' destructor should call close() at a minimum.

class MidiController : public Controller {
Q_OBJECT
public:
explicit MidiController();
explicit MidiController(UserSettingsPointer config);
~MidiController() override;

ControllerJSProxy* jsProxy() override;
Expand Down Expand Up @@ -107,6 +110,13 @@ class MidiController : public Controller {
LegacyMidiControllerMapping m_mapping;
SoftTakeoverCtrl m_st;
QList<QPair<MidiInputMapping, unsigned char> > m_fourteen_bit_queued_mappings;
UserSettingsPointer m_pConfig;
MidiSourceClock m_midiSourceClock;

// Slaves are cleaned up by QT.
ControlProxy* m_pClockBpm;
ControlProxy* m_pClockLastBeat;
ControlProxy* m_pClockRunning;

// So it can access sendShortMsg()
friend class MidiOutputHandler;
Expand Down
140 changes: 140 additions & 0 deletions src/controllers/midi/midisourceclock.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
#include "controllers/midi/midisourceclock.h"

#include "controllers/midi/midimessage.h"
#include "util/math.h"

bool MidiSourceClock::handleMessage(unsigned char status,
const mixxx::Duration& timestamp) {
// TODO(owen): We need to support MIDI_CONTINUE.
switch (status) {
case MIDI_START:
Copy link
Member

Choose a reason for hiding this comment

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

to be complete, we need to support MIDI_CONTINUE

Copy link
Member Author

Choose a reason for hiding this comment

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

supporting midi_continue is going to be tricky because right now the midisourceclock handles bpm calculation, but beat fraction information is handled by the calling code. It will take some work to figure out how to freeze the data, then thaw it, because all the timing is based on an absolute clock. For now this will be a todo.

Since this code won't be exposed in Mixxx, I think it's safe to have it be somewhat feature incomplete.

start();
return true;
case MIDI_STOP:
stop();
return true;
case MIDI_TIMING_CLK:
pulse(timestamp);
return true;
default:
return false;
}
}

void MidiSourceClock::start() {
// Treating MIDI_START as the first downbeat is standard practice:
// http://www.blitter.com/%7Erusstopia/MIDI/%7Ejglatt/tech/midispec/seq.htm
m_bRunning = true;
m_iFilled = 0;
m_iRingBufferPos = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Can we consider the first clock tick after start a bar? I think yes. We should drop a note about it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes! Here it is:
http://www.blitter.com/~russtopia/MIDI/~jglatt/tech/midispec/seq.htm

In other words, the MIDI Start puts the slave in "play mode", and the receipt of that first MIDI Clock marks the initial downbeat of the song (ie, MIDI Beat 0).

Copy link
Member

Choose a reason for hiding this comment

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

Please drop a comment about that.

}

void MidiSourceClock::stop() {
m_bRunning = false;
}

void MidiSourceClock::pulse(const mixxx::Duration& timestamp) {
// Update the ring buffer and calculate new bpm. Update the last beat time
// if we are on a beat.

if (!m_bRunning) {
qDebug()
<< "MidiSourceClock: Got clock pulse but not started, starting now.";
start();
}

// Ringbuffer filling.
// TODO(owen): We should have a ringbuffer convenience class.
m_pulseRingBuffer[m_iRingBufferPos] = timestamp;
m_iRingBufferPos = (m_iRingBufferPos + 1) % kRingBufferSize;
if (m_iFilled < kRingBufferSize) {
++m_iFilled;
}

// If this pulse is a beat mark, record it, even if we have very few samples.
if (m_iRingBufferPos % kPulsesPerQuarter == 0) {
QMutexLocker lock(&m_mutex);
if (m_dBpm != 0.0 && m_lastBeatTime.toIntegerNanos() != 0) {
// Calculate the smoothed last beat time from the current bpm
// and the actual last beat time. By not using the last smoothed
// time we prevent drift.
const double beat_length = 60.0 * 1e9 / m_dBpm;
const auto beat_duration = mixxx::Duration::fromNanos(static_cast<qint64>(beat_length));
m_smoothedBeatTime = m_lastBeatTime + beat_duration;
} else {
m_smoothedBeatTime = timestamp;
}
m_lastBeatTime = timestamp;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this thick a bar? Is there any rule inside the Midi standard to know which clock signal is really the bar?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is this thick a bar?

what?

The midi standard does not communicate measures. Just 24 ticks per quarter. Note that the standard says "quarter", and in music, a "quarter note" does not imply 4/4 time. I'm ok changing the word to "beat" but "PPQ" -- Pulses Per Quarter, is the industry standard term and I think we should stay with the standard: https://www.google.com/webhp?sourceid=chrome-instant&ion=1&espv=2&ie=UTF-8#q=midi+ppq

Copy link
Member

Choose a reason for hiding this comment

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

The midi standard does not communicate measures.

Yes, I just wonder if there is a quasi standard for measures since all connected devices have to map their measures to the midi clock ticks.

According to https://en.wikipedia.org/wiki/Tempo Quaters per minute is the same as beats per minute. Quarter would be a new term in the Mixxx syncing code so I prefer to stick at "beat".

Copy link
Member

Choose a reason for hiding this comment

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

This makes the wording issue worse:

MIDI Beat is a 16th note

But since we do not use the Midi beats, we can Ignore it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is midi code so I will to stick to midi teminology


// Figure out the bpm if we have enough samples.
if (m_iFilled > 2) {
mixxx::Duration earlyPulseTime;
if (m_iFilled < kRingBufferSize) {
earlyPulseTime = m_pulseRingBuffer[0];
} else {
// In a filled ring buffer, the earliest pulse is the next one that
// will get filled.
earlyPulseTime = m_pulseRingBuffer[m_iRingBufferPos];
}
QMutexLocker lock(&m_mutex);
m_dBpm = calcBpm(earlyPulseTime, timestamp, m_iFilled);
}
Copy link
Member

Choose a reason for hiding this comment

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

This mean value calculation kills all abrupt tempo changes.
Can we include a plausibility check to detect such changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. Midi is not precise enough to handle truly abrupt changes, and midi gear often gets out of sync when the tempo changes quickly, so that behavior is expected. Just google "midi sync problems" sometime. It's an old, imprecise standard.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for you explanations. Can you add that as comment?

}

// static
double MidiSourceClock::calcBpm(const mixxx::Duration& early_pulse,
const mixxx::Duration& late_pulse,
int pulse_count) {
// Get the elapsed time between the latest pulse and the earliest pulse
// and divide by the number of pulses in the buffer to get bpm. Midi
// clock information is by nature imprecise, and issues such as drift and
// inability to adapt to abrupt tempo changes are well known. We can not
// expect to wring more precision out of an imprecise standard.

// If we have too few samples, we can't calculate a bpm, so return 0.0.
VERIFY_OR_DEBUG_ASSERT(pulse_count >= 2) {
qWarning() << "MidiSourceClock::calcBpm called with too few pulses";
return 0.0;
}

VERIFY_OR_DEBUG_ASSERT(late_pulse >= early_pulse) {
qWarning() << "MidiSourceClock asked to calculate beat fraction but "
<< "late_pulse < early_pulse:" << late_pulse << early_pulse;
return 0.0;
}

const mixxx::Duration elapsed = late_pulse - early_pulse;
const double elapsed_mins = elapsed.toDoubleSeconds() / 60.0;
Copy link
Member

Choose a reason for hiding this comment

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

We should introduce toDoubleMins()


// We subtract one since two time values denote a single span of time --
// so a filled value of 3 indicates 2 pulse periods, etc.
const double bpm = static_cast<double>(pulse_count - 1) / kPulsesPerQuarter / elapsed_mins;

if (bpm < kMinMidiBpm || bpm > kMaxMidiBpm) {
qWarning() << "MidiSourceClock bpm out of range, returning 0:" << bpm;
return 0;
}
return bpm;
}

// static
double MidiSourceClock::beatFraction(const mixxx::Duration& last_beat,
const mixxx::Duration& now,
const double bpm) {
VERIFY_OR_DEBUG_ASSERT(now >= last_beat) {
qWarning() << "MidiSourceClock asked to calculate beat fraction but "
<< "now < last_beat:" << now << last_beat;
return 0.0;
Copy link
Member

Choose a reason for hiding this comment

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

We can return a valid value.

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 make a guess, but by the API I've written this situation is invalid and indicates a bad call upstream.

}
if (bpm == 0.0) {
return 0.0;
}
// Get seconds per beat.
const double beat_length = 60.0 / bpm;
Copy link
Member

Choose a reason for hiding this comment

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

bpm can be 0

// seconds / secondsperbeat = fraction of beat.
const mixxx::Duration beat_duration = now - last_beat;
const double beat_percent = beat_duration.toDoubleSeconds() / beat_length;
// Ensure values are < 1.0.
return beat_percent - floor(beat_percent);
}
112 changes: 112 additions & 0 deletions src/controllers/midi/midisourceclock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
#ifndef MIDISOURCECLOCK_H
#define MIDISOURCECLOCK_H

#include <QList>
#include <QMutex>

#include "util/duration.h"

// MidiSourceClock is not thread-safe, but is thread compatible using ControlObjects.
// The MIDI thread will make calls into MidiSourceClock and then update two Control
// Objects with the current reported BPM and last beat time. The engine thread
// can use those values to call a static function to calculate beat fraction
// at any time in the future. Time values are in nanoseconds for compatibility
// with Time::elapsed().

// TODO(owen): MidiSourceClock needs to support MIDI_CONTINUE. This is tricky
// because all of our times are absolute and beatFraction information is not
// stored in this class. Probably the solution is to move beatFraction into
// this class and move away from storing absolute timestamps in the ringbuffer.
class MidiSourceClock {
public:
// The number of midi pulses per quarter note (1 beat in 4/4 time).
static constexpr int kPulsesPerQuarter = 24;
// Minimum allowable calculated bpm. The bpm can still be reported as 0.0
// if there is no incoming data or if there is a problem with the
// calculation.
static constexpr double kMinMidiBpm = 10.0;
static constexpr double kMaxMidiBpm = 300.0;

private:
// Some of the tests use the ring buffer size, so keep those test in sync
// with this constant.
static const int kRingBufferSize = kPulsesPerQuarter * 4;
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 can be constexpr as well.

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 are not using constexpr thanks to windows


public:
MidiSourceClock() {}

// Handle an incoming midi status. Return true if handled.
bool handleMessage(unsigned char status,
const mixxx::Duration& timestamp);

// Signals MIDI Start Sequence. The MidiSourceClock will reset its beat
// fraction to 0, but the bpm value will be seeded with the last recorded
// value.
void start();

// Signals MIDI Stop Sequence. The MidiSourceClock will stop updating its beat
// precentage. Subsequent calls to beatFraction will return valid results
// based on the last recorded beat time and last reported bpm.
void stop();

// Signals MIDI Timing Clock. The timing between pulses will be used to
// determine bpm. kPulsesPerQuarter pulses = 1 beat.
void pulse(const mixxx::Duration& timestamp);

// Return the current BPM. Values are significant to 5 decimal places.
double bpm() const {
QMutexLocker lock(&m_mutex);
return m_dBpm;
Copy link
Member

Choose a reason for hiding this comment

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

This requires locking or atomic access.

}

// Return the exact recorded time of the last beat pulse.
mixxx::Duration lastBeatTime() const {
QMutexLocker lock(&m_mutex);
return m_lastBeatTime;
Copy link
Member

Choose a reason for hiding this comment

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

This requires locking or atomic access.

}

// Return a smoothed beat time interpolated from received data.
mixxx::Duration smoothedBeatTime() const {
QMutexLocker lock(&m_mutex);
return m_smoothedBeatTime;
}

// Calculate instantaneous beat fraction based on provided values. If
// the beat fraction is >= 1.0, the integer value will be sliced off until
// the result is between 0 <= x < 1.0. Can be called from any thread
// since it's static.
static double beatFraction(const mixxx::Duration& last_beat,
const mixxx::Duration& now,
const double bpm);

// Returns true if the clock is running. A master sync listener should
// always call this to make sure that the beatfraction and bpm are
// valid.
bool running() const {
return m_bRunning;
}

private:
// Calculate the bpm based on the pulse times and counts. Returns values
// between the min and max allowable bpm, or 0.0 for error conditions.
static double calcBpm(const mixxx::Duration& early_pulse,
const mixxx::Duration& late_pulse,
int pulse_count);

bool m_bRunning = false;
// It's a hack to say 124 all over the source, but it provides a sane
// baseline in case the midi device is already running when Mixxx starts up.
double m_dBpm = 124.0;
// Reported time of the last beat
mixxx::Duration m_lastBeatTime;
// De-jittered time of the last beat
mixxx::Duration m_smoothedBeatTime;
// Mutex for accessing bpm and last beat time for thread safety.
mutable QMutex m_mutex;

mixxx::Duration m_pulseRingBuffer[kRingBufferSize];
int m_iRingBufferPos = 0;
int m_iFilled = 0;
};

#endif // MIDISOURCECLOCK_H
Loading