Truncate MIDI SysEx messages after termination byte

[Johannes Kroll]

On Mon, 14 Jan 2013 11:42:46 +0100
<Joerg-Cyril.Hoehle at t-systems.com> wrote:

> Hi,
> 
> Johannes Kroll wrote:
> >+    for(i = 0; i < lpMidiOutHdr->dwBufferLength; i++)
> >+        if((unsigned char)lpMidiOutHdr->lpData[i] == 0xF7 && i < lpMidiOutHdr->dwBufferLength-1)
> 
> What about
>      for(i = 0; i+1 < lpMidiOutHdr->dwBufferLength, i++)
>   without the redundant second if(&&) half?

Sure. That patch was quickly written, I was just happy to have
something which 'made it work'.


> I'm always happy when somebody with real MIDI HW speaks up, because
> I'm not convinced that the Wine MIDI code does TRT about SysEx --
> based on my readings, not experience, as I own no MIDI device...
> In particular, do you have any experience with multi-part SysEx
> messages (e.g. large data, such as uploads)?

I don't think so, no. I have the nanokontrol/nanokey/nanopad combo,
but they don't take large data blocks such as samples if that's what
you mean. They're also USB-MIDI devices, which might explain some
specific quirks. I.e. at the place I tried to fix with my patch, a
traditional MIDI device might just stop at the 0xF7 and get the SysEx
message OK (but it would still be confused by the rest of the bytes in
the buffer, there is still something wrong).

I have a really old keyboard/synth here with traditional MIDI but I
think it also doesn't use multipart SysEx messages.


> Still, I'm not persuaded that your patch is at the right place.
> I believe the midi* functions should be tiny wrappers around MODM_*
> messages, same for the wave* functions.  Every time I see somebody
> attempt to violate this 1:1 mapping, I'm suspicious.  Perhaps the
> logic that you're adding belongs to the individual wine*.drv/midi.c?

I'm not familiar with Wine code so if you think the fix should go to
some other place you're probably right.

I will investigate some more later when I have some time. Thanks.



> Do you get to see this warning from winealsa.drv where it
> specifically checks for a terminating F7 (but not intermediate F7)?
> http://source.winehq.org/source/dlls/winealsa.drv/midi.c#L969
> 964     /* FIXME: MS doc is not 100% clear. Will lpData only contain system exclusive
> 965      * data, or can it also contain raw MIDI data, to be split up and sent to
> 966      * modShortData() ?
> 967      * If the latest is true, then the following WARNing will fire up
> 968      */
> 969     if (lpData[0] != 0xF0 || lpData[lpMidiHdr->dwBufferLength - 1] != 0xF7) {
> 970         WARN("Alleged system exclusive buffer is not correct\n\tPlease report with MIDI file\n");
> 971         lpNewData = HeapAlloc(GetProcessHeap(), 0, lpMidiHdr->dwBufferLength + 2);
> 972     }
> 
> Therefore, I'd be happy if you could invest some more time and check,
> based on your real MIDI HW, and perhaps native w* machines,
> whether MODM_LONGDATA and midiOutLongMsg are equivalent
> or whether midiOut* does some additional processing.
> 
> Regards,
> 	Jörg Höhle