Skip to content

Commit

Permalink
use QVector<uint8_t> instead of QByteArray for controller input
Browse files Browse the repository at this point in the history
Converting QByteArray to a QJSValue turns it into an ArrayBuffer in JS,
which then needs to be converted to a Uint8Array to be useful.
QVector<uint8_t> is converted directly to a JS Array.

This temporarily disables debugging for MIDI System Exclusive messages.
Fixing this will require refactoring sending controller output to also
use QVector<uint8_t> instead of QByteArray, which would make this commit
huge.
  • Loading branch information
Be-ing committed Jul 13, 2020
1 parent 9363b49 commit f6adbc9
Show file tree
Hide file tree
Showing 19 changed files with 71 additions and 70 deletions.
8 changes: 5 additions & 3 deletions src/controllers/bulk/bulkcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ void BulkReader::run() {
Trace timeout("BulkReader timeout");
if (result >= 0) {
Trace process("BulkReader process packet");
//qDebug() << "Read" << result << "bytes, pointer:" << data;
QByteArray outData((char*)data, transferred);
emit incomingData(outData, mixxx::Time::elapsed());
QVector<uint8_t> vector(transferred);
for (int i = 0; i < transferred; i++) {
vector.append(data[i]);
}
emit incomingData(vector, mixxx::Time::elapsed());
}
}
qDebug() << "Stopped Reader";
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/bulk/bulkcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class BulkReader : public QThread {
void stop();

signals:
void incomingData(QByteArray data, mixxx::Duration timestamp);
void incomingData(const QVector<uint8_t>& data, mixxx::Duration timestamp);

protected:
void run();
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ void Controller::triggerActivity()
m_userActivityInhibitTimer.start();
}
}
void Controller::receive(const QByteArray data, mixxx::Duration timestamp) {

void Controller::receive(const QVector<uint8_t>& data, mixxx::Duration timestamp) {
if (m_pScriptEngineLegacy == nullptr) {
//qWarning() << "Controller::receive called with no active engine!";
// Don't complain, since this will always show after closing a device as
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class Controller : public QObject, ConstControllerPresetVisitor {
// Handles packets of raw bytes and passes them to an ".incomingData" script
// function that is assumed to exist. (Sub-classes may want to reimplement
// this if they have an alternate way of handling such data.)
virtual void receive(const QByteArray data, mixxx::Duration timestamp);
virtual void receive(const QVector<uint8_t>& data, mixxx::Duration timestamp);

/// Apply the preset to the controller.
/// @brief Initializes both controller engine and static output mappings.
Expand Down
10 changes: 7 additions & 3 deletions src/controllers/hid/hidcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,17 @@ int HidController::close() {
bool HidController::poll() {
Trace hidRead("HidController poll");

int result = hid_read(m_pHidDevice, m_pPollData, sizeof(m_pPollData) / sizeof(m_pPollData[0]));
int packetLength = sizeof(m_pPollData) / sizeof(m_pPollData[0]);
int result = hid_read(m_pHidDevice, m_pPollData, packetLength);
if (result == -1) {
return false;
} else if (result > 0) {
Trace process("HidController process packet");
QByteArray outData(reinterpret_cast<char*>(m_pPollData), result);
receive(outData, mixxx::Time::elapsed());
QVector<uint8_t> data(packetLength);
for (int i = 0; i < packetLength; i++) {
data.append(m_pPollData[i]);
}
receive(data, mixxx::Time::elapsed());
}

return true;
Expand Down
32 changes: 21 additions & 11 deletions src/controllers/midi/hss1394controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,19 @@ void DeviceChannelListener::Process(const hss1394::uint8 *pBuffer, hss1394::uint
if (i + 2 < uBufferSize) {
note = pBuffer[i+1];
velocity = pBuffer[i+2];
emit incomingData(status, note, velocity, timestamp);
emit receiveShortMessage(status, note, velocity, timestamp);
} else {
qWarning() << "Buffer underflow in DeviceChannelListener::Process()";
}
i += 3;
break;
default:
// Handle platter messages and any others that are not 3 bytes
QByteArray outArray((char*)pBuffer,uBufferSize);
emit incomingData(outArray, timestamp);
QVector<uint8_t> vector(uBufferSize);
for (uint i = 0; i < uBufferSize; i++) {
vector.append(pBuffer[i]);
}
emit receiveSysex(vector, timestamp);
i = uBufferSize;
break;
}
Expand Down Expand Up @@ -110,10 +113,12 @@ int Hss1394Controller::open() {
}

m_pChannelListener = new DeviceChannelListener(this, getName());
connect(m_pChannelListener, SIGNAL(incomingData(QByteArray, mixxx::Duration)),
this, SLOT(receive(QByteArray, mixxx::Duration)));
connect(m_pChannelListener,
&DeviceChannelListener::incomingData,
&DeviceChannelListener::receiveSysex,
this,
&Hss1394Controller::receive);
connect(m_pChannelListener,
&DeviceChannelListener::receiveShortMessage,
this,
&Hss1394Controller::receiveShortMessage);

Expand Down Expand Up @@ -150,10 +155,14 @@ int Hss1394Controller::close() {
return -1;
}

disconnect(m_pChannelListener, SIGNAL(incomingData(QByteArray, mixxx::Duration)),
this, SLOT(receive(QByteArray, mixxx::Duration)));
disconnect(m_pChannelListener, SIGNAL(incomingData(unsigned char, unsigned char, unsigned char, mixxx::Duration)),
this, SLOT(receive(unsigned char, unsigned char, unsigned char, mixxx::Duration)));
disconnect(m_pChannelListener,
&DeviceChannelListener::receiveSysex,
this,
&Hss1394Controller::receive);
disconnect(m_pChannelListener,
&DeviceChannelListener::receiveShortMessage,
this,
&Hss1394Controller::receiveShortMessage);

stopEngine();
MidiController::close();
Expand Down Expand Up @@ -193,7 +202,8 @@ void Hss1394Controller::send(QByteArray data) {
int bytesSent = m_pChannel->SendChannelBytes(
(unsigned char*)data.constData(), data.size());

controllerDebug(MidiUtils::formatSysexMessage(getName(), data));
// TODO: re-enable debug output when switching to QVector<uint8_t> instead of QByteArray
//controllerDebug(MidiUtils::formatSysexMessage(getName(), data));
//if (bytesSent != length) {
// qDebug()<<"ERROR: Sent" << bytesSent << "of" << length << "bytes (SysEx)";
// //m_pChannel->Flush();
Expand Down
9 changes: 6 additions & 3 deletions src/controllers/midi/hss1394controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ class DeviceChannelListener : public QObject, public hss1394::ChannelListener {
void Disconnected();
void Reconnected();
signals:
void incomingData(unsigned char status, unsigned char control, unsigned char value,
mixxx::Duration timestamp);
void incomingData(QByteArray data, mixxx::Duration timestamp);
void receiveShortMessage(unsigned char status,
unsigned char control,
unsigned char value,
mixxx::Duration timestamp);
void receiveSysex(QVector<uint8_t> data, mixxx::Duration timestamp);

private:
QString m_sName;
};
Expand Down
6 changes: 3 additions & 3 deletions src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ double MidiController::computeValue(
return newmidivalue;
}

void MidiController::receive(QByteArray data, mixxx::Duration timestamp) {
void MidiController::receive(const QVector<uint8_t>& data, mixxx::Duration timestamp) {
controllerDebug(MidiUtils::formatSysexMessage(getName(), data, timestamp));

MidiKey mappingKey(data.at(0), 0xFF);
Expand Down Expand Up @@ -498,8 +498,8 @@ void MidiController::receive(QByteArray data, mixxx::Duration timestamp) {
}

void MidiController::processInputMapping(const MidiInputMapping& mapping,
const QByteArray& data,
mixxx::Duration timestamp) {
const QVector<uint8_t>& data,
mixxx::Duration timestamp) {
// Custom script handler
if (mapping.options.script) {
ControllerScriptEngineLegacy* pEngine = getScriptEngine();
Expand Down
6 changes: 3 additions & 3 deletions src/controllers/midi/midicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class MidiController : public Controller {
unsigned char value,
mixxx::Duration timestamp);
// For receiving System Exclusive messages
void receive(const QByteArray data, mixxx::Duration timestamp) override;
void receive(const QVector<uint8_t>& data, mixxx::Duration timestamp) override;
int close() override;

private slots:
Expand All @@ -97,8 +97,8 @@ class MidiController : public Controller {
unsigned char value,
mixxx::Duration timestamp);
void processInputMapping(const MidiInputMapping& mapping,
const QByteArray& data,
mixxx::Duration timestamp);
const QVector<uint8_t>& data,
mixxx::Duration timestamp);

double computeValue(MidiOptions options, double _prevmidivalue, double _newmidivalue);
void createOutputHandlers();
Expand Down
5 changes: 3 additions & 2 deletions src/controllers/midi/midiutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ QString MidiUtils::formatMidiMessage(const QString& controllerName,
}

// static
QString MidiUtils::formatSysexMessage(const QString& controllerName, const QByteArray& data,
mixxx::Duration timestamp) {
QString MidiUtils::formatSysexMessage(const QString& controllerName,
const QVector<uint8_t>& data,
mixxx::Duration timestamp) {
QString msg2;
if (timestamp == mixxx::Duration::fromMillis(0)) {
msg2 = "outgoing:";
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/midi/midiutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class MidiUtils {
unsigned char opCode,
mixxx::Duration timestamp = mixxx::Duration::fromMillis(0));
static QString formatSysexMessage(const QString& controllerName,
const QByteArray& data,
mixxx::Duration timestamp = mixxx::Duration::fromMillis(0));
const QVector<uint8_t>& data,
mixxx::Duration timestamp = mixxx::Duration::fromMillis(0));
};


Expand Down
18 changes: 10 additions & 8 deletions src/controllers/midi/portmidicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ PortMidiController::PortMidiController(const PmDeviceInfo* inputDeviceInfo,
const PmDeviceInfo* outputDeviceInfo,
int inputDeviceIndex,
int outputDeviceIndex)
: MidiController(), m_cReceiveMsg_index(0), m_bInSysex(false) {
: MidiController(),
m_cReceiveMsg(MIXXX_SYSEX_BUFFER_LEN),
m_cReceiveMsg_index(0),
m_bInSysex(false) {
for (unsigned int k = 0; k < MIXXX_PORTMIDI_BUFFER_LEN; ++k) {
// Can be shortened to `m_midiBuffer[k] = {}` with C++11.
m_midiBuffer[k].message = 0;
Expand Down Expand Up @@ -178,7 +181,7 @@ bool PortMidiController::poll() {
}

// Collect bytes from PmMessage
unsigned char data = 0;
uint8_t data = 0;
for (int shift = 0; shift < 32 && (data != MIDI_EOX); shift += 8) {
// TODO(rryan): This prevents buffer overflow if the sysex is
// larger than 1024 bytes. I don't want to radically change
Expand All @@ -192,9 +195,7 @@ bool PortMidiController::poll() {
// End System Exclusive message if the EOX byte was received
if (data == MIDI_EOX) {
m_bInSysex = false;
const char* buffer = reinterpret_cast<const char*>(m_cReceiveMsg);
receive(QByteArray::fromRawData(buffer, m_cReceiveMsg_index),
timestamp);
receive(m_cReceiveMsg, timestamp);
m_cReceiveMsg_index = 0;
}
}
Expand Down Expand Up @@ -243,12 +244,13 @@ void PortMidiController::send(QByteArray data) {
}

PmError err = m_pOutputDevice->writeSysEx((unsigned char*)data.constData());
// TODO: re-enable debugging messages when refactoring to use QVector<uint8_t> instead of QByteArray
if (err == pmNoError) {
controllerDebug(MidiUtils::formatSysexMessage(getName(), data));
//controllerDebug(MidiUtils::formatSysexMessage(getName(), data));
} else {
// Use two qWarnings() to ensure line break works on all operating systems
qWarning() << "Error sending SysEx message:"
<< MidiUtils::formatSysexMessage(getName(), data);
qWarning() << "Error sending SysEx message:";
//<< MidiUtils::formatSysexMessage(getName(), data);
qWarning() << "PortMidi error:" << Pm_GetErrorText(err);
}
}
2 changes: 1 addition & 1 deletion src/controllers/midi/portmidicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class PortMidiController : public MidiController {
PmEvent m_midiBuffer[MIXXX_PORTMIDI_BUFFER_LEN];

// Storage for SysEx messages
unsigned char m_cReceiveMsg[MIXXX_SYSEX_BUFFER_LEN];
QVector<uint8_t> m_cReceiveMsg;
int m_cReceiveMsg_index;
bool m_bInSysex;

Expand Down
17 changes: 0 additions & 17 deletions src/controllers/scripting/controllerscriptenginebase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ bool ControllerScriptEngineBase::initialize() {
"midi", m_pJSEngine->newQObject(controllerProxy));
}

m_byteArrayToScriptValueJSFunction = m_pJSEngine->evaluate(
"(function(arg1) { return new Uint8Array(arg1) })");

return true;
}

Expand Down Expand Up @@ -234,17 +231,3 @@ void ControllerScriptEngineBase::throwJSError(const QString& message) {
m_pJSEngine->throwError(message);
#endif
}

QJSValue ControllerScriptEngineBase::byteArrayToScriptValue(
const QByteArray& byteArray) {
// The QJSEngine converts the QByteArray to an ArrayBuffer object.
QJSValue arrayBuffer = m_pJSEngine->toScriptValue(byteArray);
// Convert the ArrayBuffer to a Uint8 typed array so scripts can access its bytes
// with the [] operator.
QJSValue result =
m_byteArrayToScriptValueJSFunction.call(QJSValueList{arrayBuffer});
if (result.isError()) {
showScriptExceptionDialog(result);
}
return result;
}
5 changes: 0 additions & 5 deletions src/controllers/scripting/controllerscriptenginebase.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ class ControllerScriptEngineBase : public QObject {

void scriptErrorDialog(const QString& detailedError, const QString& key, bool bFatal = false);

QJSValue byteArrayToScriptValue(const QByteArray& byteArray);

bool m_bDisplayingExceptionDialog;
QJSEngine* m_pJSEngine;

Expand All @@ -57,9 +55,6 @@ class ControllerScriptEngineBase : public QObject {
protected slots:
void reload();

private:
QJSValue m_byteArrayToScriptValueJSFunction;

private slots:
void errorDialogButton(const QString& key, QMessageBox::StandardButton button);

Expand Down
4 changes: 2 additions & 2 deletions src/controllers/scripting/controllerscriptmoduleengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ void ControllerScriptModuleEngine::shutdown() {
}

void ControllerScriptModuleEngine::handleInput(
QByteArray data, mixxx::Duration timestamp) {
const QVector<uint8_t>& data, mixxx::Duration timestamp) {
QJSValueList args;
args << byteArrayToScriptValue(data);
args << m_pJSEngine->toScriptValue(data);
args << timestamp.toDoubleMillis();
executeFunction(m_handleInputFunction, args);
}
2 changes: 1 addition & 1 deletion src/controllers/scripting/controllerscriptmoduleengine.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ControllerScriptModuleEngine : public ControllerScriptEngineBase {
m_moduleFileInfo = moduleFileInfo;
}

void handleInput(QByteArray data, mixxx::Duration timestamp);
void handleInput(const QVector<uint8_t>& data, mixxx::Duration timestamp);

private:
void shutdown() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@ void ControllerScriptEngineLegacy::shutdown() {
ControllerScriptEngineBase::shutdown();
}

bool ControllerScriptEngineLegacy::handleIncomingData(const QByteArray& data) {
bool ControllerScriptEngineLegacy::handleIncomingData(const QVector<uint8_t>& data) {
// This function is called from outside the controller engine, so we can't
// use VERIFY_OR_DEBUG_ASSERT here
if (!m_pJSEngine) {
return false;
}

QJSValueList args;
args << byteArrayToScriptValue(data);
args << m_pJSEngine->toScriptValue(data);
args << QJSValue(data.size());

for (const QJSValue& function : m_incomingDataFunctions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase {

bool initialize() override;

bool handleIncomingData(const QByteArray& data);
bool handleIncomingData(const QVector<uint8_t>& data);

/// Wrap a string of JS code in an anonymous function. This allows any JS
/// string that evaluates to a function to be used in MIDI mapping XML files
Expand Down

0 comments on commit f6adbc9

Please sign in to comment.