InvisibleWrench / FlutterMidiCommand

A Flutter plugin to send and receive MIDI
BSD 3-Clause "New" or "Revised" License
86 stars 45 forks source link

MacOS app crashes when sending large SysEx messages #107

Closed intonarumori closed 1 week ago

intonarumori commented 1 month ago

I've been working on a MIDI editor application and noticed crashes when sending larger SysEx messages. I traced down the issue to this line of code for iOS and macOS, see screenshot.

Screenshot 2024-04-06 at 22 03 30

I looked at some other repositories for reference and here's what I found:

Takeaways so far

I've attached an example application, pressing "Sending 512 bytes" couple of times crashes the app for me, let me know if you can reproduce.

I will be looking into the issue to see if I can fix it as I have some existing Obj-C implementation that worked fine in the past.

Flutter Example for sending large SysEx messages ``` import 'dart:typed_data'; import 'package:flutter/material.dart'; import 'package:flutter_midi_command/flutter_midi_command.dart'; void main() { runApp(const MainApp()); } class MainApp extends StatelessWidget { const MainApp({super.key}); @override Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: HomeView()), ); } } class HomeView extends StatefulWidget { const HomeView({super.key}); @override State createState() => _HomeViewState(); } class _HomeViewState extends State { final midi = MidiCommand(); @override void initState() { super.initState(); midi.onMidiSetupChanged?.listen((_) => _updateSetup()); _updateSetup(); } void _updateSetup() async { final devices = await midi.devices ?? []; for (final device in devices) { if (!device.connected) { try { await midi.connectToDevice(device); } catch (_) { debugPrint('Could not connect to device: $device'); } } } } void _sendEmptySysex(int length) { final data = Uint8List(length); data[0] = 0xF0; data[length - 1] = 0xF7; _sendData(data); } void _sendData(Uint8List data) async { final devices = await midi.devices ?? []; for (final device in devices) { midi.sendData(data, deviceId: device.id); } } @override Widget build(BuildContext context) { return Center( child: Column( mainAxisSize: MainAxisSize.min, children: [256, 512, 768, 1024] .map( (e) => Padding( padding: const EdgeInsets.all(8.0), child: ElevatedButton( onPressed: () => _sendEmptySysex(e), child: Text('Send $e bytes'), ), ), ) .toList(), ), ); } } ```
intonarumori commented 1 month ago

Quick update:

I was able to fix the crashes by using these two parts of the MIDIApps code:

I will see if I can simplify the code a little bit, it looks like we have to introduce some Obj-C helpers.

mortenboye commented 1 month ago

Thank you for diving into this issue.

Splitting large arrays of data into smaller ones should not be a big challenge.

However, I am a little confused as to why sending multiple 512 bytes SysEx chunks causes the system to crash. Mostly because every send call creates its own MIDIPacketList and puts data into that. (I can see some other potential issues here though). This must mean that there is further "queueing' deeper down the system ?

Alternatively, we should consider migrating to the never MIDIEventList type on Apple's platforms, but that is probably a bigger undertaking.

mortenboye commented 2 weeks ago

@intonarumori please try out this branch which does packet splitting: https://github.com/InvisibleWrench/FlutterMidiCommand/tree/bugfix/coremidi_dealloc

intonarumori commented 2 weeks ago

@mortenboye I ran my example app and this looks to be working great.

mortenboye commented 1 week ago

Fix included in v. 0.4.16.