InvisibleWrench/FlutterMidiCommand

MacOS app crashes when sending large SysEx messages

intonarumori opened this issue · 5 comments

intonarumori commented

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

  • Looks like we should check the returned packet value from MIDIPacketListAdd and handle errors accordingly
  • MIDIApps wraps MIDIPacketListAdd in Obj-C and makes the return type nullable
  • We should probably break up large messages to smaller chunks, even though based on the documentation up until 65535 bytes this shouldn't be an issue
  • We use 1024 bytes as the list size in MIDIPacketListAdd - I wonder if this is correct

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<HomeView> createState() => _HomeViewState();
}

class _HomeViewState extends State<HomeView> {
  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

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

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.

intonarumori commented

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

  • 👍1
mortenboye commented

Fix included in v. 0.4.16.