New subject: [PATCH 1/1] handling sysex midi messages in winecoreaudio

25 Feb 2015

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(a)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)

    

Re: [PATCH 1/1] handling sysex midi messages in winecoreaudio

Andrew Eikum

tinez

Andrew Eikum

tags

participants (2)