mixedinkey-opensource / MIKMIDI

MIDI Library for Swift and Objective-C Mac and iOS apps.
Other
492 stars 95 forks source link

Reading In Multiple MIDI Events in MIDIPacket #201

Closed jagdeep-manik closed 7 years ago

jagdeep-manik commented 7 years ago

Hi, I connected my Casio Privia PX-850 to my computer and ran the MIDI Testbed (macOS) app to log MIDI messages. When I pressed down or released any key, I only got one MIKMIDIControlChangeCommand (but I expected a "Note On" and "Note Off" command).

This is what the packet looks like in the debugger:

(MIDIPacket *) $10 = 0x0000700009b33e74
(lldb) p *inputPacket
(MIDIPacket) $11 = {
  timeStamp = 305904732655672
  length = 6
  data = {
    [0] = '\xb0'
    [1] = 'X'
    [2] = 'Z'
    [3] = '\x90'
    [4] = '<'
    [5] = '!'
    [6] = '\0'
    [7] = '\0'
    .
    .
    .
    [255] = '\0'
  }
}

In the code above, the packet begins with a ControlChangeCommand (\xb0) with control number 88 ('X') and value 90 ('Z'). After that, it has a NoteOnCommand (\x90) with note = 60 ('<') and velocity = 33 ('!'). So it turns out my piano is sending a "note on" command, but it is also sending a control change with it.

It looks like the problem comes from this block of code (in commandsWithMIDIPacket inside MIKMIDICommand.m) which basically ignores the MIDI messages after the first (if they are not the same type):

if (packetData[0] != firstCommandType && ((packetData[0] | 0x0F) != (firstCommandType | 0x0F))) {
    // Doesn't look like multiple messages because they're not all the same type
    MIKMIDICommand *command = [MIKMIDICommand commandWithMIDIPacket:inputPacket];
    return command ? @[command] : @[];
}

It looks like a MIDIPacket can contain multiple messages with different types. If I comment this block out, the code works just fine and reports both the ControlChangeCommand and the NoteOnCommand. Just wondering what the reasoning for that chunk of code is. Thanks!

EDIT: Looks like someone does have a fix for it, it just hasn't been merged yet: https://github.com/mixedinkey-opensource/MIKMIDI/pull/184

I might make a separate pull request for this.

armadsen commented 7 years ago

Another pull request would be welcome. I haven't merged that one because the changes I requested in my last comment haven't been made.

armadsen commented 7 years ago

Fixed by PR #202.