SysEx chunking, parsing & thru-ing

[franky47]

After having a discussion with DIYElectroMusic on Mastodon, it appears that the unreleased feature that allows chunking SysEx messages that are larger than the input buffer is confusing, and hard to work with.

Context

@lathoub contributed a change to allow large SysEx messages (larger than the input buffer) to still be received, but in a chunked manner, with special markers on the first and last byte to indicate what part of the message is being handled (beginning, middle, end).

I do agree that gracefully handling large messages is desirable (we can't always predict the largest SysEx length, or we may not have enough RAM to allocate such a buffer). Large messages are valid according to the spec, and so should be handled no matter what.

The issue of using the first and last data bytes as markers is that it makes parsing difficult, and may cause problems with Thru, by injecting made-up data that did not originate from the input.

The burden of parsing

Whether the SysEx message is sent complete, chunked, or even byte-streamed to the application, it remains the responsibility of application code to parse and interpret the SysEx data, a tasks which is usually repetitive, unsatisfying and prone to error.

We should probably provide ways to further parse a SysEx byte stream, allowing applications to give constraints for which messages to accept, and observe some relevant bytes in the message to ease the extraction of values.

This would essentially make the whole chunking system irrelevant, as SysEx data could be streamed to those parsers one byte at a time (when using OneByteParsing), or by chunks (when parsing as much data as available).

@lathoub, what do you think?

[diyelectromusic]

I think the tricky bit is balancing how SysEx messages can be supported by something like a stream parser whilst also retaining the existing mechanisms for other MIDI messages. The callback mechanism works really well for example, but how would that work alongside a MIDI.read() that is returning single SysEx bytes?

I guess I'm struggling to think about the threading that will be required to balance a loop() doing MIDI.read()s with callbacks whilst still allowing some piecemeal building up of a SysEx message.

We might need a diagram! Or an attempt to write some code against the proposed API to work out how it might fit together :)

I'll chew it over and think a little more!

Kevin

[diyelectromusic]

Looks good. How do you think a set of SysEx messages might be parsed?

I've just been playing with a PSS keyboard (see: https://diyelectromusic.wordpress.com/2023/08/02/arduino-pss-680-synth-editor-part-1/) which has a fixed header (F0 43 76) followed by a format byte, and then depending on the format, will expect a different sized message, which in this case may be one of 72 bytes, 410 bytes or up to a 10K+ dump.

In my case I'm only looking for a single format, so the .expectByte thing would probably work, but if I wanted to grab two different formats I'd need a parser that was ok with handling different lengths of message?

(I still wonder if attempting to do this for things like SysEx dumps on an embedded platform is perhaps stretching things anyway, so maybe we shouldn't be spending too much time thinking about it!)

Kevin

[franky47]

Either you'd get one parser definition per type of message you're interested in, and feed the byte stream to each of them, until one returns true (the others would essentially lock themselves up once the stream is considered invalid, until they get reset).

Otherwise, variable lengths could be part of the parser logic, with the responsibility of feeding it the maximum expected length for buffer sizing. Here I was using a template argument for a fixed size as it avoids any dynamic allocation.

The one issue with these approaches is that it would duplicate the amount of memory required. If parsers could work on an external, shared buffer, that would solve this problem, but we're back to square one at finding the optimal size for that buffer.

[lathoub]

I like the idea of presenting every byte of a SysEx using a callback (code above)

arduino_midi_library/src/MIDI.hpp

Line 1041 in 2d64cc3

mMessage.sysexArray[mPendingMessageIndex] = extracted;

        if ((mPendingMessage[0] == SystemExclusiveStart)
        ||  (mPendingMessage[0] == SystemExclusiveEnd))
             if (mSysExCallback) // new
                    mSysExCallback(extracted);

and pass it to a separate parser. We could provide a default vanilla parser (the SysExParser above) and provide an extension mechanisms for alternate behaviour or another parser altogether.

By just adding the callback for 1 extracted byte from a SysEx message, we would remain backward compatible. (Use could even set the SysEx buffer size to zero, to save memory)

[lathoub]

The use of the callback would eliminate the need for a buffer in the library itself, saving memory.

arduino_midi_library/src/midi_Message.h

Line 100 in 2d64cc3

DataByte sysexArray[sSysExMaxSize];

It would however break backward compatibility, to eliminate the buffer and the associated code.

[franky47]

After experimenting for the last few days, I've got a parser object that has a good DX, and uses a shared buffer that is a bit smarter than the current built-in SysEx array.

The API would look something like this:

#include <MIDI.h>

// Define a callback for each type of SysEx message you're interested in

void onFoo(const byte* data, unsigned length)
{
  // Do something with SysEx of type Foo
}

void onBar(const byte* data, unsigned length)
{
  // ...
}

// 12 here is the maximum message length we're interested in:
using SysExBuffer = midi::SysExBuffer<12>;
using SysExParser = midi::SysExParser<SysExBuffer>;

// Declare the shared buffer
SysExBuffer sysExBuffer;

// Declare one or more parsers and configure them:
SysExParser parseFoo = SysExParser(onFoo)
  .expectManufacturer(midi::SysExManufacturers::nonCommercial)
  .expectLength(8)
  .expectByte(4, 0x2a)
  .expectByte(5, 0x00);

SysExParser parseBar = SysExParser(onBar)
  .expectManufacturer(0x21, 0x1d) // Ableton
  .expectLength(8)
  .expectByte(4, 0x2b)
  .expectByte(5, 0xff);

void onSysExByteStream(byte data)
{
  // Upon receiving a single byte of SysEx data,
  // feed it to the shared buffer, then test the parsers against the buffer.
  // If there's a match, they'll forward it to their callback.
  sysExBuffer.write(data);
  parseFoo.test(sysExBuffer);
  parseBar.test(sysExBuffer);
}

void setup()
{
  MIDI.setHandleSystemExclusiveByteStream(onSysExByteStream);
  MIDI.begin();
}

void loop()
{
  MIDI.read();
}

The one issue I see with this approach is Thru. At the moment, the library handles SysEx just like any other messages and forwards them once parsed, depending on the map/filter configuration.

Going for an external stream approach for SysEx would take them out of the regular Thru logic, and would require users to explicitly forward incoming SysEx messages. The buffer is smart enough not to require a parser to do so:

void onSysExByteStream(byte data)
{
  const isComplete = buffer.write(data);
  if (isComplete)
  {
    // SysEx Thru
    MIDI.sendSysEx(buffer.getData(), buffer.getSize(), true);
  }
}

It would be technically possible to simply forward the received byte, without any form of buffering, to improve throughput. That is, as long as the SysEx parsing callbacks don't send out other messages, which otherwise would be interleaved within the forwarded SysEx and invalidate it.

So on one hand, this gives more flexibility on if/where/when to do SysEx Thru, but on the other hand we loose the coherence of the previous Thru behaviour.

A couple of notes:

The size we define for the buffer is now the maximum length were interested in, rather than the maximum size we're expecting to receive (which there was just no way of knowing). The buffer will simply stop recording SysEx that are too long, because the parsers won't trigger onto such messages anyway. This may be a problem for Thru, because long messages would be dropped. Though we have the same behaviour in current versions: messages larger than the max SysEx size defined in settings won't be Thru'd.

Callbacks aren't strictly necessary for parsers. Like MIDI.read(), parsers will return true when they've been triggered:

SysExParser parseFoo = SysExParser() // <- no callback here
  .expect(...);

void onSysExByteStream(byte data)
{
  buffer.write(data);
  if (parseFoo.test(buffer))
  {
    const byte* const data = buffer.getData();
    const unsigned size = buffer.getSize();
  }
}