On Wed, Feb 25, 2015 at 09:58:41PM +0100, tinez wrote:
Hi,
this patch fixes handling sysex midi messages in winecoreaudio driver. The code was based on winealsa driver implementation. I've tested it using external midi hardware + midi monitoring tools (MidiOX). My motivation was to fix Nord Modular Editor software which is needed to control Nord Modular G1 hardware synthetizer. After applying the patch it works without problems. I've also sent out patch to electro-music.com for other people to test and it was received well (you can see the thread here electro-music.com/forum/viewtopic.php?t=62683).
Thanks, looks mostly good. A few Wine style notes in-line below.
regards, tinez
dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-)
From e18b1c9876730d9557b97f3e16249b20eae4e875 Mon Sep 17 00:00:00 2001 From: tinez tinez@tlen.pl
Wine requires full, real names for patches.
Date: Thu, 21 Aug 2014 23:13:31 +0200 Subject: [PATCH 1/1] handling sysex midi messages in winecoreaudio
dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/dlls/winecoreaudio.drv/midi.c b/dlls/winecoreaudio.drv/midi.c index 8593e9c..c7a1ded 100644 --- a/dlls/winecoreaudio.drv/midi.c +++ b/dlls/winecoreaudio.drv/midi.c @@ -437,9 +437,9 @@ static DWORD MIDIOut_LongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) return MMSYSERR_ERROR; } }
- else
- else if (destinations[wDevID].caps.wTechnology == MOD_MIDIPORT) {
FIXME("MOD_MIDIPORT\n");
MIDIOut_Send(MIDIOutPort, destinations[wDevID].dest, lpData, lpMidiHdr->dwBufferLength);
}
lpMidiHdr->dwFlags &= ~MHDR_INQUEUE;
@@ -836,26 +836,55 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD if (src->state < 1) { TRACE("input not started, thrown away\n");
goto done;
}
/* FIXME skipping SysEx */
if (msg->data[0] == 0xF0)
{
FIXME("Starting System Exclusive\n");
src->state |= 2;
return NULL; }
if (src->state & 2)
{
for (i = 0; i < msg->length; ++i)
{
if (msg->data[i] == 0xF7)
{
FIXME("Ending System Exclusive\n");
src->state &= ~2;
bool sysexStart = (msg->data[0] == 0xF0);
Wine uses C89, so we don't have 'bool'. You could change this to a BOOL instead.
if (sysexStart || src->state & 2) {
if (sysexStart) {
TRACE("Receiving sysex message\n");
src->state |= 2;
}
int pos = 0;
int len = msg->length;
C89 also doesn't let you declare new variables in the middle of a block. They must be declared at the start of a block.
EnterCriticalSection(&midiInLock);
currentTime = GetTickCount() - src->startTime;
while (len) {
LPMIDIHDR lpMidiHdr = src->lpQueueHdr;
if (lpMidiHdr != NULL) {
int copylen = min(len, lpMidiHdr->dwBufferLength - lpMidiHdr->dwBytesRecorded);
memcpy(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded, msg->data + pos, copylen);
lpMidiHdr->dwBytesRecorded += copylen;
len -= copylen;
pos += copylen;
TRACE("Copied %d bytes of sysex message\n", copylen);
if ((lpMidiHdr->dwBytesRecorded == lpMidiHdr->dwBufferLength) ||
(*(BYTE*)(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded - 1) == 0xF7)) {
TRACE("Sysex message complete (or buffer limit reached), dispatching %d bytes\n", lpMidiHdr->dwBytesRecorded);
Tabs should be spaces, for consistency with the rest of your code.
src->lpQueueHdr = lpMidiHdr->lpNext;
lpMidiHdr->dwFlags &= ~MHDR_INQUEUE;
lpMidiHdr->dwFlags |= MHDR_DONE;
MIDI_NotifyClient(msg->devID, MIM_LONGDATA, (DWORD_PTR)lpMidiHdr, currentTime);
src->state &= ~2;
}
}
else {
Here, too.
FIXME("Sysex data received but no buffer to store it!\n");
break; } }
goto done;
LeaveCriticalSection(&midiInLock);
return NULL; }
EnterCriticalSection(&midiInLock); currentTime = GetTickCount() - src->startTime;
@@ -889,7 +918,6 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD CFRunLoopStop(CFRunLoopGetCurrent()); break; } -done: return NULL; }
-- 1.9.3 (Apple Git-50)
Ok, i hope i got it right this time.
Regards, tinez
--- dlls/winecoreaudio.drv/midi.c | 68 ++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 20 deletions(-)
W dniu 25.02.2015 o 22:16, Andrew Eikum pisze:
On Wed, Feb 25, 2015 at 09:58:41PM +0100, tinez wrote:
Hi,
this patch fixes handling sysex midi messages in winecoreaudio driver. The code was based on winealsa driver implementation. I've tested it using external midi hardware + midi monitoring tools (MidiOX). My motivation was to fix Nord Modular Editor software which is needed to control Nord Modular G1 hardware synthetizer. After applying the patch it works without problems. I've also sent out patch to electro-music.com for other people to test and it was received well (you can see the thread here electro-music.com/forum/viewtopic.php?t=62683).
Thanks, looks mostly good. A few Wine style notes in-line below.
regards, tinez
dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-) From e18b1c9876730d9557b97f3e16249b20eae4e875 Mon Sep 17 00:00:00 2001 From: tinez tinez@tlen.pl
Wine requires full, real names for patches.
Date: Thu, 21 Aug 2014 23:13:31 +0200 Subject: [PATCH 1/1] handling sysex midi messages in winecoreaudio
dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/dlls/winecoreaudio.drv/midi.c b/dlls/winecoreaudio.drv/midi.c index 8593e9c..c7a1ded 100644 --- a/dlls/winecoreaudio.drv/midi.c +++ b/dlls/winecoreaudio.drv/midi.c @@ -437,9 +437,9 @@ static DWORD MIDIOut_LongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) return MMSYSERR_ERROR; } }
- else
- else if (destinations[wDevID].caps.wTechnology == MOD_MIDIPORT) {
FIXME("MOD_MIDIPORT\n");
MIDIOut_Send(MIDIOutPort, destinations[wDevID].dest, lpData, lpMidiHdr->dwBufferLength); } lpMidiHdr->dwFlags &= ~MHDR_INQUEUE;
@@ -836,26 +836,55 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD if (src->state < 1) { TRACE("input not started, thrown away\n");
goto done;
}
/* FIXME skipping SysEx */
if (msg->data[0] == 0xF0)
{
FIXME("Starting System Exclusive\n");
src->state |= 2;
return NULL; }
if (src->state & 2)
{
for (i = 0; i < msg->length; ++i)
{
if (msg->data[i] == 0xF7)
{
FIXME("Ending System Exclusive\n");
src->state &= ~2;
bool sysexStart = (msg->data[0] == 0xF0);
Wine uses C89, so we don't have 'bool'. You could change this to a BOOL instead.
if (sysexStart || src->state & 2) {
if (sysexStart) {
TRACE("Receiving sysex message\n");
src->state |= 2;
}
int pos = 0;
int len = msg->length;
C89 also doesn't let you declare new variables in the middle of a block. They must be declared at the start of a block.
EnterCriticalSection(&midiInLock);
currentTime = GetTickCount() - src->startTime;
while (len) {
LPMIDIHDR lpMidiHdr = src->lpQueueHdr;
if (lpMidiHdr != NULL) {
int copylen = min(len, lpMidiHdr->dwBufferLength - lpMidiHdr->dwBytesRecorded);
memcpy(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded, msg->data + pos, copylen);
lpMidiHdr->dwBytesRecorded += copylen;
len -= copylen;
pos += copylen;
TRACE("Copied %d bytes of sysex message\n", copylen);
if ((lpMidiHdr->dwBytesRecorded == lpMidiHdr->dwBufferLength) ||
(*(BYTE*)(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded - 1) == 0xF7)) {
TRACE("Sysex message complete (or buffer limit reached), dispatching %d bytes\n", lpMidiHdr->dwBytesRecorded);
Tabs should be spaces, for consistency with the rest of your code.
src->lpQueueHdr = lpMidiHdr->lpNext;
lpMidiHdr->dwFlags &= ~MHDR_INQUEUE;
lpMidiHdr->dwFlags |= MHDR_DONE;
MIDI_NotifyClient(msg->devID, MIM_LONGDATA, (DWORD_PTR)lpMidiHdr, currentTime);
src->state &= ~2;
}
}
else {
Here, too.
FIXME("Sysex data received but no buffer to store it!\n");
break; } }
goto done;
LeaveCriticalSection(&midiInLock);
return NULL; }
EnterCriticalSection(&midiInLock); currentTime = GetTickCount() - src->startTime;
@@ -889,7 +918,6 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD CFRunLoopStop(CFRunLoopGetCurrent()); break; } -done: return NULL; }
-- 1.9.3 (Apple Git-50)
On Wed, Feb 25, 2015 at 11:04:45PM +0100, tinez wrote:
Ok, i hope i got it right this time.
Yes, this looks OK to me. Send it off to wine-patches for inclusion, and please use your real, full name in the patch. Thanks!
Andrew