Sysex message allocated on read

MidiMessageMetadata gives you exactly that, a pointer and a size.

2 Likes

Yes. As I said: I can fix “my code” by branching everywhere for numBytes to ensure I don’t allocate when a Sysex message arrives, or decide to filter all of them at the start of the block. Problem “solved”.

The problem is, JUCE doesn’t do that branching, for example in Synthesiser::renderNextBlock, and will continue to allocate when the host (Logic) or a controller sends out those messages, even if the user doesn’t care about Sysex. Same is true for MidiKeyboardState and pretty much all Midi functions in JUCE - and most likely all MIDI functions by JUCE users that so far didn’t think to branch in that scenario.

Additionally, due to how MidiBuffer works, many times you have to filter messages by some criteria, and then copy all matching or non-matching messages in another (pre-allocated) MidiBuffer. A Sysex message will allocate when copied from a buffer to another buffer.

You had said earlier in this thread:

Because getMessage() only allows to read the message by copy, I’m not sure there’s a way to get around that?

And even in those cases, I’d be forced to allocate when reading the SysEx messages.

…so those are the bits I was responding to, by pointing out that bothering to create a MidiMessage to read a SysEx message is a bit superfluous anyways, since you’ve already got a pointer to the data in the MidiMessageMetadata.

Now this is a different problem. I see how the automatic allocating behavior is a systemic problem for the framework, with a method like Synthesiser::renderNextBlock where there’s a fair expectation that it will be called in a real-time thread.

How do you think the problem should be solved? Have some class pre-allocate that memory, to make it safe to create a MidiMessage on the heap, from the audio thread? (it could return a pointer to a temporary MidiMessage, whose lifetime would be just during the current loop of the MidiBufferIterator)

Or would you prefer more the route of excluding SysEx from realtime MidiBuffers, and have the framework pre-sort SysEx into a separate buffer for handling on a different thread?

Or…?

I’d just have Sysex be expressed as a struct of a pointer and size that doesn’t own anything, and have the user deal with the allocations if they need to create new messages, which can be handled in many different ways.

A modern way to do this is with something like std::variant, where the MIDI message is either 3 bytes, a pointer and a size, or even something like a ReferenceCountedObject (that one is more offline and not realtime messages).

With MIDI 2.0, It seems that all common messages are exactly 8 bytes, including SYSEX (which is split into multiple messages if needed).
So I think in terms of avoiding memory allocations on the real-time thread, it might turn out easier to use MIDI 2.0 everywhere you can (and convert back to MIDI 1.0 at the ‘edges’).

1 Like

I didn’t know that, that seems like a very simple solution to this issue - make the MIDI message 8 bytes on the stack with a way to cast it on demand to MIDI 1 or MIDI 2 types if needed, maybe?

1 Like

Bumping, in case someone from the JUCE team can take a look?

Bumping again. Important issue.

Bumping… having the same issue (see Microtonality - #25 by erikronstrom)

1 Like

Bumping again… any chance that now with MIDI 2, and JUCE requiring C++17, MidiMessage could become an std::variant or something safer to use in real time contexts even when Sysex messages are involved?

1 Like

I’m not sure we could do this without it being a massive breaking change, so I think this is unlikely, at least in the short term. I am keeping this use-case in mind for the UMP compatibility work.

2 Likes

Hi, isn’t it better to have a breaking change and force people to ack this issue rather than just having loads of code silently allocating on the audio thread?

1 Like

This is the type of thread that seems to go on and on forever, and after a while you forget
what the topic originally was about and it gets less and less clear what the bumpings and added contribution addresses.

The thread seems to be about to things

  1. How to avoid allocating midi messages on the audio thread and
  2. How to handle big SysEx messages for those who needs them

These two topics are unfortunately mixed up from the very start.

The first point can easily be handled like

void processBlock(AudioSampleBuffer& buffer, MidiBuffer& midiMessages)
{
	for (auto msg : midiMessages)
	{
		if (msg.numBytes > sizeof(void*))
			continue;	//o/w we'll be allocating memory

		auto midiMessage = msg.getMessage();

		//Handle midiMessage w/ o allocating
	}
}

which won’t allocate. To make things a bit clearer and more self-documenting, the juce team could add a function like

inline bool static MidiMessage::willHeapAllocate(int numBytes) 
{ return numBytes > sizeof(void*); }

to be used like 

if (MidiMessage::willHeapAllocate(msg.numBytes))
	continue;

You don’t have to wait for a “massive breaking change” to assure you don’t allocate on the audio thread and if you’re not dealing with SysEx:es (which I guess most doesn’t) cased is closed.

To handle big messages (not only SysEx) is another beast, and will probably need different solutions for different needs. But as most doesn’t use those anyway it’s hence a minor problem and perhaps could be addressed in another thread, that deals with just that.

3 Likes

Unfortunately that’s not true.

That’s because ‘your code’ might not be the one doing the MIDI loop.
Your code might do:

synth.renderNextBlock(buffer, midi);

For which synth is a juce::MPESynthesiser (or one of the many different processors JUCE provides) that does the loop internally.

That is why this bug is really annoying - it requires user who don’t even care about direct MIDI manipulation or Sysex to do manual filtering of messages they don’t even handle, just to avoid allocation.

What could be done by JUCE to go around the bug, is to supply users with a MIDI filter, for example (pseudo code):

struct MyProcessor: AudioProcessor
{
    void processBlock(AudioBuffer<float>& buffer, MidiBuffer& midi) override
    {
        //Must be called before any additional processing
        sysexFilter.process(midi);

        //Do safe processing without allocating, call synths, etc
    }


    //This needs to be stateful since it must hold a temporary MIDIBuffer object.

    juce::SysexFilter sysexFilter;
};

And then make sure to update literally all audio-related example and tutorials.

I would still rather they just fix the bug, as even this solution (filtering out sysex) is not costless in the expensive audio callback.

1 Like

And now you’re doing it again!

Please stop talking about SYSEX when you’re concern is really about unwanted midi allocation on the audio thread!

There are small Sysex messages (like Machine control commands) that doesn’t allocate and thera are big messages that’s not SysEx but will allocate never the less (textMessages e.g)

It’s like not wanting diesel engines in the city and thinking that forbidding… buses is the solution.

If you continue this SysEx obsession this thread will focus on SysEx and tend to lead to discussions of how to handle SysEx and about breaking changes and who knows what…

Of course the Juce code could as well benefit for some updates to filter out MidiMessages bigger than 8 bytes.

Sure, I accept the semantics difference.

You can replace Sysex with “Large Sysex messages”, since AFAIK the only messages that could be larger than 3 bytes in the MIDI format are Sysex.

MidiMessage MidiMessage::textMetaEvent (int type, StringRef text) 

could be really large

I’ve been working with “regular” MIDI (i.e. streaming MIDI) for a long time, and didn’t even know Meta Events were a thing. They only exist in the MIDI File format, right?

https://ccrma.stanford.edu/~craig/14q/midifile/MidiFileFormat.html#meta_event

Meta Events start with a 0xFF byte, which in streaming MIDI should be interpreted as a single-byte System Reset message (although, looking at juce::MidiMessage, it seems that is not how 0xFF is interpreted).

1 Like