From 447cd666865e38da79a35f5d4dbcedb368e849fa Mon Sep 17 00:00:00 2001 From: Alex Montgomery Date: Mon, 9 May 2011 00:39:33 +0100 Subject: [PATCH] redesigned QJackMMC to be always listening, threadsafe --- TODO | 8 +- common.c | 40 ++++----- common.h | 3 +- main.c | 7 +- qt/src/main.cpp | 2 +- qt/src/mainWindow.cpp | 213 +++++++++++++++++++++++++-------------------- qt/src/mainWindow.h | 16 +++- qt/src/qjackmmc.ui | 97 +++++++++++---------- qt/src/sequencerThread.cpp | 21 +++-- qt/src/sequencerThread.h | 2 + qt/src/validator.cpp | 8 +- 11 files changed, 229 insertions(+), 188 deletions(-) diff --git a/TODO b/TODO index 5e64376..941481f 100644 --- a/TODO +++ b/TODO @@ -1,11 +1,7 @@ - fix XRun on quit -- thread-proof / separate the listen loop -- make listen loop "always on" -- change verbose to dropdown with output levels: normal, verbose, silent -- allow 'output level' to be changed while listening +- hunt down clock drift on subsequent play/stop gotos +- change verbose checkbox to dropdown with output levels: normal, verbose, silent - use XML for settings -- have listen on startup automatically save to defaults -- add "what's this?" as a button - change icon - update NEWS, VERSION - add README with advice on configure's parameters and how to call help diff --git a/common.c b/common.c index e7c8695..65e48b3 100644 --- a/common.c +++ b/common.c @@ -21,7 +21,6 @@ // exported globals int g_quit = 0; // a flag which will be set by our signal handler when it's time to exit -int g_isListening = 0; // globals used only here in common.c snd_seq_t* g_seq_ptr = NULL; @@ -92,22 +91,19 @@ jack_port_t* g_jackMidiIn = NULL; int jack_process_cb(jack_nframes_t numFrames, void* cbContext) { - if (g_isListening) - { - uint8_t* inBuffer = 0; - int numMidiEvents = 0; - jack_midi_event_t currEvent; - int midiIndex = 0; - MidiSettings* settings = (MidiSettings*) cbContext; + uint8_t* inBuffer = 0; + int numMidiEvents = 0; + jack_midi_event_t currEvent; + int midiIndex = 0; + MidiSettings* settings = (MidiSettings*) cbContext; - inBuffer = jack_port_get_buffer(g_jackMidiIn, numFrames); - numMidiEvents = jack_midi_get_event_count(inBuffer); + inBuffer = jack_port_get_buffer(g_jackMidiIn, numFrames); + numMidiEvents = jack_midi_get_event_count(inBuffer); - for (midiIndex = 0; midiIndex < numMidiEvents; ++midiIndex) - { - jack_midi_event_get(&currEvent, inBuffer, midiIndex); - handle_midi(currEvent.buffer, settings); - } + for (midiIndex = 0; midiIndex < numMidiEvents; ++midiIndex) + { + jack_midi_event_get(&currEvent, inBuffer, midiIndex); + handle_midi(currEvent.buffer, settings); } return 0; @@ -247,7 +243,7 @@ void handle_midi(uint8_t* midiBuff, MidiSettings* settings) uint32_t jitter = (device_time_ms > jack_time_ms ? (device_time_ms - jack_time_ms) : (jack_time_ms - device_time_ms)); if (settings->verbose) - printMMCMessage("MMC locate to hour: %d, minute: %d, second: %d, frame: %d, subframe: %d\n", + printMMCMessage("MMC goto hour: %d, min: %d, sec: %d, frame: %d, subframe: %d\n", hour, minute, second, @@ -266,7 +262,7 @@ void handle_midi(uint8_t* midiBuff, MidiSettings* settings) jack_transport_reposition(g_jack_client, &jack_pos); } else if (settings->verbose) - printMMCMessage("New position is %d ms away and within jitter range, ignoring.\n", jitter); + printMMCMessage("New position is within jitter range, ignoring. (%d ms)\n", jitter); } break; case 0x48: /* step */ @@ -309,13 +305,10 @@ void handle_midi(uint8_t* midiBuff, MidiSettings* settings) } } -void poll_midi (MidiSettings* settings) +uint8_t* get_midi_input () { snd_seq_event_t * seq_event_ptr = NULL; - if (poll(g_pollDescriptor, g_numDescriptors, 250) > 0 && snd_seq_event_input(g_seq_ptr, &seq_event_ptr) >= 0 && seq_event_ptr->type == SND_SEQ_EVENT_SYSEX) - handle_midi((uint8_t *)seq_event_ptr->data.ext.ptr, settings); - #if LASH_SUPPORT /* Process LASH events */ { @@ -335,6 +328,11 @@ void poll_midi (MidiSettings* settings) } } #endif + + if (poll(g_pollDescriptor, g_numDescriptors, 250) > 0 && snd_seq_event_input(g_seq_ptr, &seq_event_ptr) >= 0 && seq_event_ptr->type == SND_SEQ_EVENT_SYSEX) + return ((uint8_t *)seq_event_ptr->data.ext.ptr); + + return 0; } void cleanup_globals() diff --git a/common.h b/common.h index 75ed435..4f7d58d 100644 --- a/common.h +++ b/common.h @@ -34,7 +34,6 @@ typedef struct // globals extern int g_quit; -extern int g_isListening; // functions implemented differently by cli and qt frontends extern void printMMCMessage(const char* message, ...); @@ -43,7 +42,7 @@ extern void printMMCMessage(const char* message, ...); int init_alsa_sequencer(const char* appName); int init_jack(const char* appName); int activate_jack(); -void poll_midi (MidiSettings* settings); +uint8_t* get_midi_input (); void cleanup_globals(); void handle_midi(uint8_t* midiBuff, MidiSettings* settings); diff --git a/main.c b/main.c index 8741a64..1ab2a4d 100644 --- a/main.c +++ b/main.c @@ -96,12 +96,11 @@ int main(int argc, char *argv[]) activate_jack(); - // start listening for and handling MMC events, stops when g_quit is true (SIGINT) - g_isListening = true; - while (!g_quit) { - poll_midi(&settings); + uint8_t* midiData = get_midi_input(); + if (midiData) + handle_midi(midiData, &settings); } // so we shall quit, eh? ok, cleanup time. otherwise jack would probably produce an xrun on shutdown diff --git a/qt/src/main.cpp b/qt/src/main.cpp index 2a68382..2ca29e6 100644 --- a/qt/src/main.cpp +++ b/qt/src/main.cpp @@ -49,7 +49,7 @@ extern "C" { messageString.vsprintf(message, args); // send verbose messages across thread using the invoke system - QMetaObject::invokeMethod(mainWindow, "onMessageReceived", Q_ARG(QString, messageString)); + QMetaObject::invokeMethod(mainWindow, "onMessageReceived", Qt::QueuedConnection, Q_ARG(QString, messageString)); va_end(args); } diff --git a/qt/src/mainWindow.cpp b/qt/src/mainWindow.cpp index 39f7c5f..d2f1e2d 100644 --- a/qt/src/mainWindow.cpp +++ b/qt/src/mainWindow.cpp @@ -32,7 +32,7 @@ extern "C" { #include "../../common.h" } -MainWindow::MainWindow() : sequencerThread(0) +MainWindow::MainWindow() : m_sequencerThread(0), m_settingsMutex(QMutex::Recursive) { setupUi(this); // use the UI layout generated by qjackmmc.ui @@ -45,6 +45,14 @@ MainWindow::MainWindow() : sequencerThread(0) validator = new HexValidator(this); deviceEdit->setValidator(validator); + // connect all UI editable elements + connect(fpsEdit, SIGNAL(editingFinished()), this, SLOT(onValidateFps())); + connect(jitterEdit, SIGNAL(editingFinished()), this, SLOT(onValidateJitter())); + connect(deviceEdit, SIGNAL(editingFinished()), this, SLOT(onValidateDeviceID())); + connect(rtBox, SIGNAL(clicked(bool)), this, SLOT(onRealtimeChanged(bool))); + connect(verboseBox, SIGNAL(clicked(bool)), this, SLOT(onVerboseChanged(bool))); + connect(whatsThisButton, SIGNAL(clicked()), this, SLOT(on_actionWhat_triggered())); + // set the program's icon const QString iconPath(QString(ICON_DIR) + "/qjackmmc.png"); if (QFile::exists(iconPath)) @@ -57,14 +65,21 @@ bool MainWindow::init(int argc, char *argv[]) if (succeeded) { + setDefaultSettings(); + // load the default configuration file if it exists QFile loadFile(QDir::homePath() + QJACKMMC_CONFIG); if (loadFile.exists()) loadConfig(loadFile); - if (listenBox->isChecked()) - on_startButton_clicked(); + + // start the MMC listener thread + Q_ASSERT(!m_sequencerThread); + m_sequencerThread = new SequencerThread(this, &m_settings, &m_settingsMutex); + m_sequencerThread->listen(rtBox->isChecked()); + + printMMCMessage("QJackMMC is actively listening for MMC messages. If you want to see information about them as they come in, make sure \"Verbose output\" is checked."); } return succeeded; @@ -97,76 +112,6 @@ void MainWindow::on_actionWhat_triggered() QWhatsThis::enterWhatsThisMode(); } -void MainWindow::on_startButton_clicked() -{ - if (g_isListening) - { - // stop the process and wait for it to die - if (sequencerThread) - { - sequencerThread->die(); - sequencerThread->wait(); - delete sequencerThread; - sequencerThread = 0; - } - - startButton->setText("&Start Listening"); - - g_isListening = false; - - enableRelevantWidgets(g_isListening); - } - else - { - bool ok = true; - m_settings.frameRate = fpsEdit->text().toInt(&ok); - - // we go to great lengths to make sure that the input boxes only accept positive integers, but check the fields just in case - if (!ok) - { - QMessageBox* inputError = new QMessageBox(this); - inputError->setText("the frames / sec parameter needs to be a positive integer."); - inputError->exec(); - return; - } - m_settings.jitterTolerance = jitterEdit->text().toInt(&ok); - if (!ok) - { - QMessageBox* inputError = new QMessageBox(this); - inputError->setText("the jitter tolerance needs to be a positive integer."); - inputError->exec(); - return; - } - int deviceID = deviceEdit->text().toInt(&ok, 16); - if (!ok || deviceID > 255 || deviceID < 0) - { - QMessageBox* inputError = new QMessageBox(this); - inputError->setText("the deviceID needs to be a hexadecimal number between 0 and ff."); - inputError->exec(); - return; - } - else - m_settings.deviceID = (uint8_t) deviceID; - - m_settings.verbose = verboseBox->isChecked(); - - startButton->setText("&Stop Listening"); - - // start the MMC listener thread - sequencerThread = new SequencerThread(this, &m_settings, &m_settingsMutex); - - - - bool realtime = rtBox->isChecked(); - sequencerThread->listen(realtime); - - g_isListening = true; - - // disable tinkering with MMC parameters while the process is running - enableRelevantWidgets(g_isListening); - } -} - void MainWindow::on_loadButton_clicked() { QFile loadFile(QDir::homePath() + QJACKMMC_CONFIG); @@ -193,7 +138,10 @@ void MainWindow::on_saveButton_clicked() out << boolVal << endl; boolVal = (rtBox->isChecked() ? 1 : 0); out << boolVal << endl; - boolVal = (listenBox->isChecked() ? 1 : 0); + + // note: this value is ignored but written for backwards compatibility. + // It used to signify "listen on startup" but now QJackMMC is always listening. + boolVal = false; out << boolVal << endl; // write out text boxes @@ -217,6 +165,91 @@ void MainWindow::on_saveButton_clicked() } +void MainWindow::onValidateFps() +{ + // validate setting, revert to prior value if not valid + bool ok = true; + int fps = fpsEdit->text().toInt(&ok); + + // we go to great lengths to make sure that the input boxes only accept positive integers, but check the fields just in case + if (!ok || fps < 0) + { + QMessageBox* inputError = new QMessageBox(this); + inputError->setText("the frames / sec parameter needs to be a positive integer."); + inputError->exec(); + + // revert value + QMutexLocker settingsLock(&m_settingsMutex); + fpsEdit->setText(QString::number(m_settings.frameRate)); + } + else + { + QMutexLocker settingsLock(&m_settingsMutex); + m_settings.frameRate = fps; + } +} + +void MainWindow::onValidateJitter() +{ + // validate setting, revert to prior value if not valid + bool ok = true; + int jitterTolerance = jitterEdit->text().toInt(&ok); + if (!ok || jitterTolerance < 0) + { + QMessageBox* inputError = new QMessageBox(this); + inputError->setText("the jitter tolerance needs to be a positive integer."); + inputError->exec(); + + // revert value + QMutexLocker settingsLock(&m_settingsMutex); + jitterEdit->setText(QString::number(m_settings.jitterTolerance)); + } + else + { + QMutexLocker settingsLock(&m_settingsMutex); + m_settings.jitterTolerance = jitterTolerance; + } +} + +void MainWindow::onValidateDeviceID() +{ + // validate setting, revert to prior value if not valid + bool ok = true; + int deviceID = deviceEdit->text().toInt(&ok, 16); + if (!ok || deviceID > 255 || deviceID < 0) + { + QMessageBox* inputError = new QMessageBox(this); + inputError->setText("the deviceID needs to be a hexadecimal number between 0 and ff."); + inputError->exec(); + + // revert value + QMutexLocker settingsLock(&m_settingsMutex); + deviceEdit->setText(QString::number(m_settings.deviceID)); + } + else + { + QMutexLocker settingsLock(&m_settingsMutex); + m_settings.deviceID = (uint8_t) deviceID; + } +} + + +void MainWindow::onVerboseChanged(bool checked) +{ + QMutexLocker settingsLock(&m_settingsMutex); + m_settings.verbose = checked; +} + +void MainWindow::onRealtimeChanged(bool checked) +{ + if (m_sequencerThread) + m_sequencerThread->die(); // die() calls deleteLater and ensures the thread is cleaned up once the listen loop ends + + m_sequencerThread = new SequencerThread(this, &m_settings, &m_settingsMutex); + m_sequencerThread->listen(checked); + +} + void MainWindow::onMessageReceived(QString message) { messageArea->append(message); @@ -293,23 +326,19 @@ void MainWindow::loadConfig(QFile& loadFile) int boolVal; in >> boolVal; verboseBox->setChecked(boolVal == 1); + m_settings.verbose = boolVal == 1; in >> boolVal; rtBox->setChecked(boolVal == 1); + + // note: this value is ignored but read for backwards compatibility. + // It used to signify "listen on startup" but now QJackMMC is always listening. in >> boolVal; - listenBox->setChecked(boolVal == 1); - // read in edit boxes QString value; - in >> value; - if (value.length()) - fpsEdit->setText(value); - in >> value; - if (value.length()) - jitterEdit->setText(value); - in >> value; - if (value.length()) - deviceEdit->setText(value); + in >> value; fpsEdit->setText(value); onValidateFps(); + in >> value; jitterEdit->setText(value); onValidateJitter(); + in >> value; deviceEdit->setText(value); onValidateDeviceID(); if (loadFile.error() != QFile::NoError) { @@ -328,13 +357,11 @@ void MainWindow::loadConfig(QFile& loadFile) } } -void MainWindow::enableRelevantWidgets(bool isRunning) +void MainWindow::setDefaultSettings() { - // re-enable MMC parameter input boxes - fpsEdit->setEnabled(!isRunning); - jitterEdit->setEnabled(!isRunning); - deviceEdit->setEnabled(!isRunning); - rtBox->setEnabled(!isRunning); - verboseBox->setEnabled(!isRunning); - loadButton->setEnabled(!isRunning); + QMutexLocker settingsLocker(&m_settingsMutex); + m_settings.deviceID = 0x7f; + m_settings.frameRate = 30; + m_settings.jitterTolerance = 50; + m_settings.verbose = false; } diff --git a/qt/src/mainWindow.h b/qt/src/mainWindow.h index 71e1105..3d3f130 100644 --- a/qt/src/mainWindow.h +++ b/qt/src/mainWindow.h @@ -23,6 +23,7 @@ #include #include +#include extern "C" { #include "../../common.h" @@ -47,18 +48,27 @@ class MainWindow : public QMainWindow, public Ui::QjackMMC void on_actionQuit_triggered(); void on_actionWhat_triggered(); void on_actionAbout_triggered(); - void on_startButton_clicked(); void on_loadButton_clicked(); void on_saveButton_clicked(); + // input validator slots + void onValidateFps(); + void onValidateJitter(); + void onValidateDeviceID(); + + // checkbox slots + void onVerboseChanged(bool checked); + void onRealtimeChanged(bool checked); + + // other slots void onMessageReceived(QString message); protected: bool initSound(int argc, char *argv[]); void loadConfig(QFile& loadFile); - void enableRelevantWidgets(bool isRunning); + void setDefaultSettings(); - SequencerThread* sequencerThread; + QPointer m_sequencerThread; MidiSettings m_settings; //< shared between threads, Always lock m_settingsMutex when accessing! QMutex m_settingsMutex; //< guards m_settings }; diff --git a/qt/src/qjackmmc.ui b/qt/src/qjackmmc.ui index ac4d3a0..9008317 100644 --- a/qt/src/qjackmmc.ui +++ b/qt/src/qjackmmc.ui @@ -105,27 +105,7 @@ - - - - Check this to schedule QJackMMC with the highest allowable thread priority. - - - Use &high priority thread - - - - - - - Check this to output verbose messages to the command line when QJackMMC is started from a terminal. - - - &Verbose output - - - - + Load your last saved settings from ~/.qjackmmc @@ -135,7 +115,7 @@ - + Save your current settings to ~/.qjackmmc @@ -145,26 +125,6 @@ - - - - Toggles whether QjackMMC is currently listening to connected MIDI devices for MMC commands. - - - &Start listening - - - - - - - Check this so that QJackMMC will start listening for MMC commands as soon as it starts up. Don't forget to click "save as Default settings"! - - - Listen &On Startup - - - @@ -191,10 +151,59 @@ + + + + It shows you messages like this. Now try clicking this then something else you want to know about. + + + &What does that do? + + + + + + + Check this to output verbose messages to the message area below. + + + &Verbose output + + + + + + + Qt::Vertical + + + QSizePolicy::Fixed + + + + 20 + 10 + + + + + + + Check this to schedule alsa sequencer polling with the highest allowable thread priority. + + + Use &high priority thread for alsa polling + + + + + + if "Verbose output" is checked, text about any received MMC messages will appear here. + true @@ -261,12 +270,8 @@ fpsEdit jitterEdit deviceEdit - rtBox - verboseBox loadButton saveButton - listenBox - startButton diff --git a/qt/src/sequencerThread.cpp b/qt/src/sequencerThread.cpp index 52d7e5c..41ba4c1 100644 --- a/qt/src/sequencerThread.cpp +++ b/qt/src/sequencerThread.cpp @@ -20,9 +20,10 @@ #include "sequencerThread.h" #include +#include SequencerThread::SequencerThread(QObject* parent, MidiSettings* settings, QMutex* settingsMutex) - : QThread(parent), m_settings(settings), m_settingsMutex(settingsMutex) + : QThread(parent), m_settings(settings), m_settingsMutex(settingsMutex), m_shouldExit(false) { } @@ -38,15 +39,25 @@ void SequencerThread::listen(bool realTime) void SequencerThread::run () { g_quit = 0; + quint8* midiData = 0; - while(!g_quit) + QPointer stillAlive(this); + while(!g_quit && !stillAlive.isNull() && !m_shouldExit) { - QMutexLocker settingsLock(m_settingsMutex); - poll_midi(m_settings); + midiData = get_midi_input(); + if (midiData && !stillAlive.isNull()) + { + QMutexLocker settingsLocker(m_settingsMutex); + handle_midi(midiData, m_settings); + } } + + // not ideal, but we need to dispose of ourself once we're out of the loop. If we don't + // calling deleteLater from mainWindow after SequencerThread::die still causes a segfault + deleteLater(); } void SequencerThread::die() { - g_quit = 1; + m_shouldExit = true; } diff --git a/qt/src/sequencerThread.h b/qt/src/sequencerThread.h index 4d7a9ca..41b6e3f 100644 --- a/qt/src/sequencerThread.h +++ b/qt/src/sequencerThread.h @@ -40,4 +40,6 @@ class SequencerThread : public QThread MidiSettings* m_settings; QMutex* m_settingsMutex; + + bool m_shouldExit; }; diff --git a/qt/src/validator.cpp b/qt/src/validator.cpp index 19d6b47..b54ef53 100644 --- a/qt/src/validator.cpp +++ b/qt/src/validator.cpp @@ -23,9 +23,6 @@ QValidator::State Validator::validate ( QString & input, int & pos ) const { Q_UNUSED(pos); - - if (!input.length()) - return QValidator::Intermediate; for (int i = 0; i < input.length(); i++) { @@ -39,9 +36,6 @@ QValidator::State Validator::validate ( QString & input, int & pos ) const QValidator::State HexValidator::validate ( QString & input, int & pos ) const { Q_UNUSED(pos); - - if (!input.length()) - return QValidator::Intermediate; for (int i = 0; i < input.length(); i++) { @@ -50,4 +44,4 @@ QValidator::State HexValidator::validate ( QString & input, int & pos ) const } return QValidator::Acceptable; -} \ No newline at end of file +} -- 2.11.4.GIT