diff options
author | Daniel Schürmann <daschuer@mixxx.org> | 2015-12-23 19:58:54 +0100 |
---|---|---|
committer | Daniel Schürmann <daschuer@mixxx.org> | 2015-12-23 19:58:54 +0100 |
commit | 6c73e026ac34c7d4ba3ea70e2f2cb26d4cb77e08 (patch) | |
tree | feba6ae38d856d651b87f7cad3ce46e3b8ff18fb | |
parent | ad22ce7e8cfcc6d40c84cd24503b5cd8f341edf0 (diff) | |
parent | a543a4be560f511859d5ec0e2af43b1980e072dc (diff) |
Merge pull request #790 from daschuer/controllerpoll
removed while loop in poll slot, Bug #1520619
-rw-r--r-- | src/controllers/controllermanager.cpp | 55 | ||||
-rw-r--r-- | src/controllers/controllermanager.h | 1 | ||||
-rw-r--r-- | src/controllers/midi/midicontroller.cpp | 22 | ||||
-rw-r--r-- | src/controllers/midi/midicontroller.h | 6 | ||||
-rw-r--r-- | src/controllers/midi/portmidicontroller.cpp | 11 | ||||
-rw-r--r-- | src/controllers/midi/portmidicontroller.h | 37 | ||||
-rw-r--r-- | src/test/midicontrollertest.cpp | 2 |
7 files changed, 101 insertions, 33 deletions
diff --git a/src/controllers/controllermanager.cpp b/src/controllers/controllermanager.cpp index 43056a324d..7d81f1e8fc 100644 --- a/src/controllers/controllermanager.cpp +++ b/src/controllers/controllermanager.cpp @@ -12,6 +12,7 @@ #include "controllers/defs_controllers.h" #include "controllers/controllerlearningeventfilter.h" #include "util/cmdlineargs.h" +#include "util/time.h" #include "controllers/midi/portmidienumerator.h" #ifdef __HSS1394__ @@ -26,6 +27,7 @@ #include "controllers/bulk/bulkenumerator.h" #endif +namespace { // http://developer.qt.nokia.com/wiki/Threads_Events_QObjects // Poll every 1ms (where possible) for good controller response @@ -37,6 +39,8 @@ const int kPollIntervalMillis = 5; const int kPollIntervalMillis = 1; #endif +} // anonymous namespace + QString firstAvailableFilename(QSet<QString>& filenames, const QString originalFilename) { QString filename = originalFilename; @@ -60,7 +64,8 @@ ControllerManager::ControllerManager(ConfigObject<ConfigValue>* pConfig) // ControllerManager because the CM is moved to its own thread and runs // its own event loop. m_pControllerLearningEventFilter(new ControllerLearningEventFilter()), - m_pollTimer(this) { + m_pollTimer(this), + m_skipPoll(false) { qRegisterMetaType<ControllerPresetPointer>("ControllerPresetPointer"); // Create controller mapping paths in the user's home directory. @@ -282,17 +287,45 @@ void ControllerManager::stopPolling() { } void ControllerManager::pollDevices() { - Trace tracer("ControllerManager::pollDevices"); - bool eventsProcessed(false); - // Continue to poll while any device returned data. - do { - eventsProcessed = false; - foreach (Controller* pDevice, m_controllers) { - if (pDevice->isOpen() && pDevice->isPolling()) { - eventsProcessed = pDevice->poll() || eventsProcessed; - } + // Note: this function is called from a high priority thread which + // may stall the GUI or may reduce the available CPU time for other + // High Priority threads like caching reader or broadcasting more + // then desired, if it is called endless loop like. + // + // This especially happens if a controller like the 3x Speed + // Stanton SCS.1D emits more massages than Mixxx is able to handle + // or a controller like Hercules RMX2 goes wild. In such a case the + // receive buffer is stacked up every call to insane values > 500 messages. + // + // To avoid this we pick here a strategies similar like the audio + // thread. In case pollDevice() takes longer than a call cycle + // we are cooperative a skip the next cycle to free at least some + // CPU time + // + // Some random test data form a i5-3317U CPU @ 1.70GHz Running + // Ubuntu Trusty: + // * Idle poll: ~5 µs. + // * 5 messages burst (full midi bandwidth): ~872 µs. + + if (m_skipPoll) { + // skip poll in overload situation + m_skipPoll = false; + //qDebug() << "ControllerManager::pollDevices() skip"; + return; + } + + qint64 start = Time::elapsed(); + foreach (Controller* pDevice, m_controllers) { + if (pDevice->isOpen() && pDevice->isPolling()) { + pDevice->poll(); } - } while (eventsProcessed); + } + + qint64 duration = Time::elapsed() - start; + if (duration > kPollIntervalMillis * 1000000) { + m_skipPoll = true; + } + //qDebug() << "ControllerManager::pollDevices()" << duration << start; } void ControllerManager::openController(Controller* pController) { diff --git a/src/controllers/controllermanager.h b/src/controllers/controllermanager.h index 79cb8c8fec..b4503965af 100644 --- a/src/controllers/controllermanager.h +++ b/src/controllers/controllermanager.h @@ -90,6 +90,7 @@ class ControllerManager : public QObject { QList<Controller*> m_controllers; QThread* m_pThread; PresetInfoEnumerator* m_pMainThreadPresetEnumerator; + bool m_skipPoll; }; #endif // CONTROLLERMANAGER_H diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index d2472d9296..c889437f26 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -161,40 +161,44 @@ void MidiController::destroyOutputHandlers() { } QString formatMidiMessage(unsigned char status, unsigned char control, unsigned char value, - unsigned char channel, unsigned char opCode) { + unsigned char channel, unsigned char opCode, int32_t timestamp) { + + QString prefix = QString("MIDI t:%2 ms ").arg(QString::number(timestamp)); + //QString prefix = QString("MIDI "); + switch (opCode) { case MIDI_PITCH_BEND: - return QString("MIDI status 0x%1: pitch bend ch %2, value 0x%3") + return prefix + QString("status 0x%1: pitch bend ch %2, value 0x%3") .arg(QString::number(status, 16).toUpper(), QString::number(channel+1, 10), QString::number((value << 7) | control, 16).toUpper().rightJustified(4,'0')); case MIDI_SONG_POS: - return QString("MIDI status 0x%1: song position 0x%2") + return prefix + QString("status 0x%1: song position 0x%2") .arg(QString::number(status, 16).toUpper(), QString::number((value << 7) | control, 16).toUpper().rightJustified(4,'0')); case MIDI_PROGRAM_CH: case MIDI_CH_AFTERTOUCH: - return QString("MIDI status 0x%1 (ch %2, opcode 0x%3), value 0x%4") + return prefix + QString("status 0x%1 (ch %2, opcode 0x%3), value 0x%4") .arg(QString::number(status, 16).toUpper(), QString::number(channel+1, 10), QString::number((status & 255)>>4, 16).toUpper(), QString::number(control, 16).toUpper().rightJustified(2,'0')); case MIDI_SONG: - return QString("MIDI status 0x%1: select song #%2") + return prefix + QString("status 0x%1: select song #%2") .arg(QString::number(status, 16).toUpper(), QString::number(control+1, 10)); case MIDI_NOTE_OFF: case MIDI_NOTE_ON: case MIDI_AFTERTOUCH: case MIDI_CC: - return QString("MIDI status 0x%1 (ch %2, opcode 0x%3), ctrl 0x%4, val 0x%5") + return prefix + QString("status 0x%1 (ch %2, opcode 0x%3), ctrl 0x%4, val 0x%5") .arg(QString::number(status, 16).toUpper(), QString::number(channel+1, 10), QString::number((status & 255)>>4, 16).toUpper(), QString::number(control, 16).toUpper().rightJustified(2,'0'), QString::number(value, 16).toUpper().rightJustified(2,'0')); default: - return QString("MIDI status 0x%1") + return prefix + QString("status 0x%1") .arg(QString::number(status, 16).toUpper()); } } @@ -237,11 +241,11 @@ void MidiController::commitTemporaryInputMappings() { } void MidiController::receive(unsigned char status, unsigned char control, - unsigned char value) { + unsigned char value, int32_t timestamp) { unsigned char channel = MidiUtils::channelFromStatus(status); unsigned char opCode = MidiUtils::opCodeFromStatus(status); - controllerDebug(formatMidiMessage(status, control, value, channel, opCode)); + controllerDebug(formatMidiMessage(status, control, value, channel, opCode, timestamp)); MidiKey mappingKey(status, control); diff --git a/src/controllers/midi/midicontroller.h b/src/controllers/midi/midicontroller.h index d45dc4da7d..1df27f47f8 100644 --- a/src/controllers/midi/midicontroller.h +++ b/src/controllers/midi/midicontroller.h @@ -62,9 +62,9 @@ class MidiController : public Controller { send(data, length); } - protected slots: - void receive(unsigned char status, unsigned char control = 0, - unsigned char value = 0); + void receive(unsigned char status, unsigned char control, + unsigned char value, int32_t timestamp); + // For receiving System Exclusive messages void receive(const QByteArray data); virtual int close(); diff --git a/src/controllers/midi/portmidicontroller.cpp b/src/controllers/midi/portmidicontroller.cpp index 5a486e6c4f..e82edc4091 100644 --- a/src/controllers/midi/portmidicontroller.cpp +++ b/src/controllers/midi/portmidicontroller.cpp @@ -150,11 +150,14 @@ int PortMidiController::close() { bool PortMidiController::poll() { // Poll the controller for new data if it's an input device - if (!m_pInputStream) + if (!m_pInputStream) { + qDebug() << "PortMidiController::poll() no Input Stream"; return false; + } PmError gotEvents = Pm_Poll(m_pInputStream); if (gotEvents == FALSE) { + //qDebug() << "PortMidiController::poll() no events"; return false; } if (gotEvents < 0) { @@ -164,6 +167,8 @@ bool PortMidiController::poll() { int numEvents = Pm_Read(m_pInputStream, m_midiBuffer, MIXXX_PORTMIDI_BUFFER_LEN); + //qDebug() << "PortMidiController::poll()" << numEvents; + if (numEvents < 0) { qWarning() << "PortMidi error:" << Pm_GetErrorText((PmError)numEvents); return false; @@ -174,7 +179,7 @@ bool PortMidiController::poll() { if ((status & 0xF8) == 0xF8) { // Handle real-time MIDI messages at any time - receive(status, 0, 0); + receive(status, 0, 0, m_midiBuffer[i].timestamp); continue; } @@ -188,7 +193,7 @@ bool PortMidiController::poll() { //unsigned char channel = status & 0x0F; unsigned char note = Pm_MessageData1(m_midiBuffer[i].message); unsigned char velocity = Pm_MessageData2(m_midiBuffer[i].message); - receive(status, note, velocity); + receive(status, note, velocity, m_midiBuffer[i].timestamp); } } diff --git a/src/controllers/midi/portmidicontroller.h b/src/controllers/midi/portmidicontroller.h index 3e89792833..213b1b3e5d 100644 --- a/src/controllers/midi/portmidicontroller.h +++ b/src/controllers/midi/portmidicontroller.h @@ -20,13 +20,38 @@ #include <portmidi.h> #include "controllers/midi/midicontroller.h" -// Mixxx completely stops responding to the controller if more than this number of messages queue up. -// Don't lower this (much.) The SCS.1d accumulated 500 messages in a single poll during stress-testing. -#define MIXXX_PORTMIDI_BUFFER_LEN 1024 /**Number of MIDI messages to buffer*/ -#define MIXXX_SYSEX_BUFFER_LEN 1024 /**Length of SysEx buffer*/ -#define MIXXX_PORTMIDI_NO_DEVICE_STRING "None" /**String to display for no MIDI devices present */ +// Note: +// A standard Midi device runs at 31.25 kbps, with 10 bits / byte +// 1 byte / 320 microseconds +// a usual Midi message has 3 byte which results to +// 1042.6 messages per second +// +// The MIDI over IEEE-1394: +// http://www.midi.org/techspecs/rp27v10spec%281394%29.pdf +// which is also used for USB defines 3 speeds: +// 1 byte / 320 microseconds +// 2 bytes / 320 microseconds +// 3 bytes / 320 microseconds +// which results in up to 3125 messages per second +// if we assume normal 3 Byte messages. +// +// For instants the SCS.1d, uses the 3 x speed +// +// Due to a bug Mixxx completely stops responding to the controller +// if more than this number of messages queue up. Don't lower this (much.) +// The SCS.1d a 3x Speed device +// accumulated 500 messages in a single poll during stress-testing. +// A midi message contains 1 .. 4 bytes. +// a 1024 messages buffer will buffer ~327 ms Midi-Stream +#define MIXXX_PORTMIDI_BUFFER_LEN 1024 -/** A PortMidi-based implementation of MidiController */ +// Length of SysEx buffer in byte +#define MIXXX_SYSEX_BUFFER_LEN 1024 + +// String to display for no MIDI devices present +#define MIXXX_PORTMIDI_NO_DEVICE_STRING "None" + +// A PortMidi-based implementation of MidiController class PortMidiController : public MidiController { Q_OBJECT public: diff --git a/src/test/midicontrollertest.cpp b/src/test/midicontrollertest.cpp index af5e605a91..aa8bfebe30 100644 --- a/src/test/midicontrollertest.cpp +++ b/src/test/midicontrollertest.cpp @@ -40,7 +40,7 @@ class MidiControllerTest : public MixxxTest { void receive(unsigned char status, unsigned char control, unsigned char value) { - m_pController->receive(status, control, value); + m_pController->receive(status, control, value, 0); } MidiControllerPreset m_preset; |