FlutterMidiCommand icon indicating copy to clipboard operation
FlutterMidiCommand copied to clipboard

Fixed size of MIDIPacketList prevents sending of bigger SysEx data

Open Noir- opened this issue 3 years ago • 10 comments

The current Swift implementation sets a fixed value of 1024 bytes for the MIDIPacketList as you can see here.

This prevents sending certain SysEx data wich can be way bigger. According to the documentation, in CoreMIDI the MIDIPacketList size can be up to 65536 bytes.

I'm not familiar with CoreMIDI and Swift but researched how a dynamic sizing of the list should look like and came up with this:

let listSize = ((MemoryLayout<MIDIPacketList>.size - MemoryLayout<MIDIPacket>.size) + MemoryLayout<MIDITimeStamp>.size + MemoryLayout<UInt16>.size) + bytes.count
packet = MIDIPacketListAdd(packetList, listSize, packet, time, bytes.count, bytes)


Which is based on code I found in the MIDIKit project, more specific here (implementation) and here (definition of the constants).

The only problem: it doesn't work. When I change the class, the program crashes when MIDIPacketListAdd is called. I couldn't figure out what's going on yet. It also happens when I try fixed numbers like 4096.

Are you aware of any issue?

Noir- avatar Jul 25 '21 12:07 Noir-

For clarification: this is how it looks like when I send data in flutter.

 int chunkSize = _syxData.indexWhere((element) => element == 0xf7) + 1;
 int chunks = (_syxData.lengthInBytes % chunkSize == 0) ?
 _syxData.lengthInBytes ~/ chunkSize :
 _syxData.lengthInBytes ~/ chunkSize + 1;

 for (int chunkCount = 1, start = 0, end = chunkSize; chunkCount <= chunks; chunkCount++ ){
   if (chunkCount == chunks){
     _midiCommand.sendData(_syxData.sublist(start));
   } else {
     _midiCommand.sendData(_syxData.sublist(start, end));
   }
   start = chunkCount*chunkSize;
   end = start+chunkSize;
   sleep(Duration(milliseconds: 200));
   setState(() {
     _progress = 100/chunks*chunkCount;
   });
 }

It's only an excerpt but I can share the full program if required.

Noir- avatar Jul 25 '21 12:07 Noir-

Reading more into CoreMIDI, it looks like an absolute nightmare. It looks like AudioKit is the defacto standard for avoiding the pain of debugging CoreMIDIs C APIs. Do you think it would be an option to use this framework?

Noir- avatar Jul 25 '21 20:07 Noir-

@Noir- I think suggesting moving to completely new API because of not being able to initially modify 1 function call to work is a bit extreme.

Just saying "the program crashes" is not super helpful. Do you have a stacktrace of the crashes? Is it the same for both using fixed and dynamically sized buffers?

I would suggest trying to debug this a bit further, have you printed out all the relevant parameters for both fixed and dynamic buffer sizes to make sure they are as expected? If you have tried both a fixed and dynamic, it sounds like perhaps you are passing in too much for the fixed size or for a fixed size, the list length needs to be padded to match?

maks avatar Aug 01 '21 07:08 maks

@Noir- have you tried setting a fixed size of 65536 bytes instead? 65k would probably be negligible in the grander scheme of things. Also the list is being deallocated right after being sent. Maybe not an ideal solution, but just to see if that fixes your immediate problem.

Also, I agree with @maks that moving to a completely different 3rd. party API is not the way to fix this. Firstly I believe it to be way overkill, it will add a huge dependency that I can't control, and although you mention it being the defacto standard for wrapping CoreMIDI, I've never seen it before and it seems to be focused around something else:

AudioKit is an audio synthesis, processing, and analysis platform for iOS, macOS (including Catalyst), and tvOS.

mortenboye avatar Aug 01 '21 09:08 mortenboye

Playing with the size of MIDIPacketList didn't helped. I suppose that the problem is with the way how the memory is allocated. I kinda replaced the current implementation for sending MIDI data with the one of AudioKit and it's working fine so far. I can create a PR later.

I agree that AudioKit has a broader focus than MIDI but MIDI is a very essential part of it because the vast majority of music apps need a MIDI implementation.

Noir- avatar Aug 01 '21 12:08 Noir-

Can you give an example of the data you’re trying to send? Or is it just any data larger than 1024 bytes?

mortenboye avatar Aug 01 '21 12:08 mortenboye

I attached an archive with the file I'm sending. In the code I split the data into 4107 byte chunks because the target only accepts this page size. firmware.syx.zip

Noir- avatar Aug 01 '21 16:08 Noir-

@Noir- I think suggesting moving to completely new API because of not being able to initially modify 1 function call to work is a bit extreme.

Just saying "the program crashes" is not super helpful. Do you have a stacktrace of the crashes? Is it the same for both using fixed and dynamically sized buffers?

I would suggest trying to debug this a bit further, have you printed out all the relevant parameters for both fixed and dynamic buffer sizes to make sure they are as expected? If you have tried both a fixed and dynamic, it sounds like perhaps you are passing in too much for the fixed size or for a fixed size, the list length needs to be padded to match?

Regarding this: I don't see any stack trace or error in the console dev console, the whole thing just died. However, I use Flutter/Dart for 11 days now and never did anything before in Swift or ObjC. MacOS and Xcode as development environment is also rather new for me. I did some console print "debugging" which led me nowhere. If you have any hints for me to make this more verbose, I'm sure we can work this out :)

The funny thing is, that it's not crashing if I'm passing more than 1024 bytes. It just doesn't send the data. But when I try to increase the size of MIDIPacketList, the crash occurs. The AudioKit way for calling the CoreMIDI API is working.

Noir- avatar Aug 01 '21 16:08 Noir-

@Noir- I know this was a while ago, but I recently ran into a similar issue with sending large sysex messages using this plugin, though on Linux not on MacOS. Reading back through your previous comments, I think you may have misunderstood the way sysex message sending works. The 1kB used in the swift code is just a buffer size and not the maximum size to sysex messages that can be sent. Because sysex messages are bracketed with start and end bytes, you can just keep calling _midiCommand.sendData() as many times as needed until you've sent the final 0xF7 terminating byte, as long as you break up your message into 1kB chunks, so the code you pasted in your second comment here is exactly right and what you need to do, the only thing that's wrong with it is that you don't set the chunk size to 1024.

In fact on Linux with ALSA (and maybe on MacOS too) you need to split large sysex messages into smaller chunks in order to be able to add delays so that you can throttle the transmission speed for devices (especially older ones) that cannot receive data at high (eg. USB) speeds.

For example you can see this functionality being added to the amidi ALSA command line tool in this patch. I suspect (though can't be sure) that this is actually what AudioKit ends up doing for you behind-the-scenes,

I actually used your for-loop code almost as is on my current project, but used a chunk size of 256B with a delay of 10ms per chunk and it worked fine, whereas sending the whole ~16kB in one go (which Linux implementation does allow) in fact is too fast for the Korg E2 I'm using.

@mortenboye given this is quite a niche use case (sending large sysex messages to older devices) perhaps rather then any code changes to accomodate throttling message transmission rates just adding this as a FAQ to the Readme would help future users of the plugin?

maks avatar Feb 21 '22 03:02 maks

@maks agreed. This should be documented rather than "fixed". I will include this change in the next round of updates.

mortenboye avatar Feb 22 '22 12:02 mortenboye