MIDI Processor Inconsistent/Processing Speed Issue?

aaronventure

@Christoph-Hart because even if I delay the entire noteOff by humanize.get("max") * 2, so around 100ms (which introduces noticeable floatiness to performance), I can still reproduce the hanging notes by playing a very short note on a keyboard (I don't have to try and make artificially small nanosecond long MIDI events in the piano roll).

I use the humanize control for all multitracks beyond the first, so that even as you play, the sound starts coming out (the first voice plays) on your time with the others having a small random delay, so it "feels" tight.

In Kontakt, the solution is to simply call wait() and it delays the execution of the callback at that line. I My understanding is that HISE works differently so that's not exactly an option here.

aaronventure

@Christoph-Hart I made some progress by childing the sampler 2 levels. The event handler is now at the top, the humanization happens second, and at sampler level I check if the key is down.

But there's another problem here:

The multitracking I do is based on neighbouring zones. So the actual NoteNumber of these events differs from the key that is pressed.

I already have the tuning numbers for each event so I can use these correct the check to make sure I'm checking for the correct key.

The issue comes with playing multiple notes at the same time (on the same string). Due to execution order (same timestamp event, container will execute twice before the child gets to play), the second note's IDs will overwrite the first in the ID array. This is not normally an issue because for the notes on the same string I execute on note-off before playing them (and overwriting their IDs).

However, due to humanization the note are not yet playing and by the time the check for ignoring the event in AntiHang comes, the tuning numbers are already different (overwritten) so it runs the same 5 tuning numbers for both notes (10 events in total).

The execution order of script processors prevents any solid event-related information hand-down down the hierarchy. I can create a massive array and store information into indexes based on event IDs, but I'm not sure how ideal that is memory wise. The solution then gets highly specific with each individual project and refactoring, as well as keeping track of the whole setup between projects, becomes a mess.

Is there a chance the message object could get an empty property (or 16) where we could store data? Something like Message.getCustom(index) and Message.setCustom(index, value). This would be highly beneficial to any complex project. Kontakt has EVENT_PAR_CUSTOM with 16 value slots and it works really well for passing down event-related data.

~~HISEScript isn't typed so maybe just a single property, and we can then put an array there or whatever (would that be accesed by Message.getCustom()[6], after being set as Message.setCustom(array)?)~~ Actually this is dynamic and realtime+dynamic=nono, so maybe the last idea of 16 slots is better.

dxmachina

@aaronventure said in MIDI Processor Inconsistent/Processing Speed Issue?:

Is there a chance the message object could get an empty property (or 16) where we could store data? Something like Message.getCustom(index) and Message.setCustom(index, value). This would be highly beneficial to any complex project. Kontakt has EVENT_PAR_CUSTOM with 16 value slots and it works really well for passing down event-related data.

Yeah, I agree. I've been on the verge of putting in a feature request for this many times. Usually in these situations I have found it's just been my HISE ignorance, but custom event params are not something I've totally found a replacement for (and would often seem to solve problems elegantly).

Christoph Hart

I can't change the Message object - it can't exceed 16 bytes because this assumption is being used all across HISE from fast copy operations to passing the data to JIT compiled SNEX code.

What I could do is to implement a storage system that is basically an array with the event IDs, but that's nothing that couldn't be implemented on a scripting level too:

namespace CustomMessage
{
const var NUM_SLOTS = 512;
const var NUM_PER_SLOT = 16;

if(!isDefined(GLOBAL_STORAGE))
{
    global GLOBAL_STORAGE = [];
    GLOBAL_STORAGE.reserve(512);

    for(i = 0; i < NUM_SLOTS; i++)
    {
        GLOBAL_STORAGE[i] = [];
        for(j = 0; j < NUM_PER_SLOT; j++)
            GLOBAL_STORAGE[i][j] = 0;
    }
}

inline function setCustomValue(index, value)
{
    GLOBAL_STORAGE[Message.getEventId() % NUM_SLOTS][index] = value;
}

inline function getCustomValue(index)
{
    return GLOBAL_STORAGE[Message.getEventId() % NUM_SLOTS][index];
}
}

The interesting part is probably the truncation by the modulo operator which keeps the memory usage in check and can be adapted for each use case (512 is a very generous value to start with, usually you don't have this much events active at the same time).

dxmachina

@Christoph-Hart Aha... ok, that's more or less what I've been trying (though I'm learning from your optimization here -- nice). Are you suggesting we use this as is, or would you implement into the engine?

Christoph Hart

@dxmachina There's not too much benefit from implementing it in C++ - maybe the % operator gets optimized to a bit mask for power-of-two modulos, but if it's implemented in HiseScript you can adapt it more easily to your specific use case, but from a performance perspective both are O(1) operations, so the performance is pretty similar.

aaronventure

@Christoph-Hart This works really well. I thought about a global array but couldn't think of a good way to be getting around the memory issue. Are event IDs just incremental ints for each new note on? If so, modulo is the obvious solution there, thanks. 512 is plenty indeed.

Copying this thing into an external script and including it everywhere you want to use it is the simplest implementation.

This is so incredibly useful you should pin it somewhere.

I added two more functions:

    inline function setCustomValueForId(id, index, value)
    {
        GLOBAL_STORAGE[id % NUM_SLOTS][index] = value;
    }

    inline function getCustomValueForId(id, index)
    {
        return GLOBAL_STORAGE[id % NUM_SLOTS][index];
    }

Christoph Hart

Are event IDs just incremental ints for each new note on

To be fully precise, they are unsigned 16 bit integers, so they go from 0 to 65536 (they are part of the message object so every bit saved is a good bit).

Now what happens if you play more than 65536 notes? Either Y2K will happen 24 years too late, or it starts at zero again :)

This is so incredibly useful you should pin it somewhere.

Might be a good spot here:

https://docs.hise.audio/tutorials/recipes/event-processing/index.html

aaronventure

@Christoph-Hart

This solves the same timestamp issue but there was a still issue with two notes being played one next to another with a time interval big enough so as to avoid having two same timestamps, but small enough to be under the humanization window. This would cause ID overwrites and checking became problematic. I think I have accounted for all cases but will continue testing.

You may be thinking that this is highly specific to my project and, that's true. Any other complex project will have its own ultimate solution to prevent note hangings depending on how it's handling note ons and its usage of Synth.play().

Note hanging is in 99% of cases a timing issue where a noteOff doesn't execute on the proper event. For systems where you can pause code execution (Kontakt) this is trivial and a simple check can prevent this, once you're aware of how it all works (and many devs still aren't because there isn't a concrete example in the manual or a PSA on "How to avoid note hanging once and for all"). In HISE, where you have this queue-like thing (code executes and the note gets stored into the queue, even though conditions may change by the time it's supposed to play), it can get interesting.

So I'll propose a method for dealing with any complex situations where note hangings might occur without having to test for and eliminate all possible edge cases as well as having to create complex hierarchical setups where refactoring and keeping track of things becomes troublesome.

Synth.playNoteWithStartOffsetIf (channel, number, velocity, offset, condition). Whenever the time for execution comes due to possible usage of note delaying, the note will only play if the condition is true. Usually that condition would be to check whether a key is still being held, but could be anything else like checking if a variable (or array) still contains its ID etc.)

Let me know if you think this makes any sense.

Christoph Hart

@aaronventure would a Synth.cancelNote(eventId) function also work? You can call this in your note off callback and it will then look in all queued upcoming events and remove the ones with matching event ID.

function onNoteOn()
{
    Message.delayEvent(44100);
}

function onNoteOff()
{
    // if you play a note < 1 second, this will cancel the future event.
    Synth.cancelNote(Message.getEventId());
}

This is easier to implement and I don't have to drag that condition function around (the condition parameter in your suggestion must be a function object so if we try storing this from a realtime thread, our friend the AudioThreadGuard says no.

aaronventure

@Christoph-Hart said in MIDI Processor Inconsistent/Processing Speed Issue?:

(the condition parameter in your suggestion must be a function object so if we try storing this from a realtime thread, our friend the AudioThreadGuard says no.

Can't it just take true/false like a bunch of other methods? I mean e.g. Message.ignoreEvent(bool) takes a boolean, this should be the same (I have no clue how it works it the backend so I'm just throwing stuff at the wall.

The thing with Synth.cancelNote(eventId) is you again have to query various conditions. What's the physical note that caused this ID to be triggered? It's not necessarily the one in Message.getNoteNumber(). So here we go storing data in our custom array and handing it down the line again, while writing logic for the edge cases. It's barely a step less than adding another layer in hierarchy and culling the events at the lowermost script processor (which is what I'm doing now).

The idea with Synth.playNoteWithStartOffsetIf() was that you write the condition for the note to start and then you just continue writing your algorithms. The conditions for the note to start after already scheduling it will in most cases only be about whether a certain key is pressed down (because the assumption is that you're already doing all the logic before calling Synth.play() in the first place).

The idea of it is to have one last safety check before the note fires off at a point where you have no control over it (the code after calling the method gets executed anyway, but the playNote gets delayed with Message.delay() ). It would be the endgame anti-hang solution so that each processor doesn't need its own child processor with project-specific culling logic.

Basically a "go to that house, knock on the door and if no one's home, come back" kinda thing, give the order and forget about it.

I can understand if this is somehow hard to implement. I don't need this immediately today or tomorrow, I can deal with complex shit because I'm masochistic like that (I'm here after all, aren't I?). But long term it would be a good feature because it enables rock-solid note management for any project that has any kind of time-specific note management (I remember Mike on VI saying he gave up on HISE because it didn't have wait() like Kontakt -- my opinion is that you can achieve everything with Message.delay() but the queue introduces complexity to follow-up logic both because of event inheritance, script processor execution order and (the lack of, for noobs like me until today) custom event information, whereas in Kontakt you're just thinking in a linear timeline, for better or worse.

Christoph Hart

The condition is supposed to be evaluated just before the note is started and not when you call this method, hence it needs a callable object that performs a check. Just passing in a variable will evaluate it and then store the value from the time you call it which renders it completely useless (think an scripting equivalent of the useless machines you can find on Youtube).

I still think that the cancelNote function is a valuable tool for the stuck notes problem and is definitely a good addition, so I‘ll add it regardless of whether it‘s the solution for this particular problem.

In C++ there is a paradigm called RAII which comes down to „whoever created something is responsible for cleaning it up“ and this principle can be applied for most scenarios regarding stuck notes. So in your case where you start 5 artificial events with a short randomized delay on each noteOn all you need to do is to store the event id of the original note that triggered those and then call the cancelNote function in the noteoff callback. I don‘t think that the implementation is more complex than with the function you suggest, there you would have to query the „activeness“ of the note that started it which would have require similar bookkeeping.

But I agree about the attractiveness of having a wait function and it has been one of the first things I tried to hack into the JS engine back in 2013/2014 when I started the HISE development. The problem is that I can‘t suspend the function execution and then pick it up with the exact state for each variable but since my brain has grown since 2014 maybe I find a smart solution when I think about it now…

Christoph Hart

ok, hear me out: forget everything I wrote above. What if I simply solve that problem at the root so you don't have to do anything?

For each note off I just need to iterate the queue of pending note ons and remove the ones that match the event ID and have a bigger timestamp (the second condition is important because it allows super fast notes where a note-on and note-off occurs within the same buffer). This removes basically every scenario where a script creates a note-on that is scheduled after its note-off.

It comes with a tiny performance overhead because it has to do a nested loop (once for the incoming notes and once for each queued event), so while it has a theoretical complexity of O(n^2), it most likely won't matter too much because the number of events can be expected to stay reasonably low. On the other hand I could just disable this functionality by default and add a API call Synth.setCancelFutureNotes(true) that enables this for each sound generator that has a script fiddling with note delays (so all you need to do is to call this once in each onInit callback of a script that is about to mess with the event timestamps).

aaronventure

@Christoph-Hart Alright, that sounds even better than what I proposed.

So it would check which noteOfById calls were made and remove these from the queue, if I got that right?

And the enabling call sounds good without adding overhead by default.

@Christoph-Hart said in MIDI Processor Inconsistent/Processing Speed Issue?:

But I agree about the attractiveness of having a wait function and it has been one of the first things I tried to hack into the JS engine back in 2013/2014 when I started the HISE development. The problem is that I can‘t suspend the function execution and then pick it up with the exact state for each variable but since my brain has grown since 2014 maybe I find a smart solution when I think about it now…

Yeah it does have some cool uses too, some of which are like a cult secret. You can dynamically create timers by having a while loop with wait() in each iteration which puts a callback on hold until conditions are met. Once you realize that playing an artificial note and immediately doing a noteOff for its ID in the very next line creates a release callback with that same ID, you effectively get the option of infinite dynamic async callbacks. You can then call stopWait() with callback/event IDs and cancel the waiting elsewhere, etc.

Other times it can get a bit messy but with great power comes great responsibility.

Christoph Hart

So it would check which noteOfById calls were made and remove these from the queue, if I got that right?

Yes. Whenever a note-off message is about to be processed, it looks in the queue if there is a pending note on with a bigger timestamp and then just removes that so it won't happen. Whether the note-off was created artificially or is an actual key event doesn't matter.

I've pushed the change (without the new API call so it's now enabled by default). Feel free to check if that solves your issues.

aaronventure

@Christoph-Hart Alright, some observations.

So this new implementation that you added requires a specific eventID storage paradigm if you're ignoring the physical events and creating your own.

Due to the possibility of two notes with the same timestamp resulting in two noteOns happening before noteOffs, this will not prevent note hanging if you're overwriting note IDs is any way other than for the same note number.

So you NEED per-note ID storage, even for monophonic instruments.

If doing MPE, it's gonna require another dimension for the channels in case of multiple cases of same note number on different channels.

Christoph Hart

@aaronventure well if you spawn off multiple events from a single note then you definitely have to keep track of them and call note-off in the respective note-off callback of the message. I don't think this is a design flaw, but just a reasonable bookkeeping task (again, think of it as RAII: the one who created it (key down) is responsible for the cleanup (key up).

I can offer to add a few more data containers that make this a bit more convenient, but there are a few of those already:

https://docs.hise.audio/scripting/scripting-api/midilist/index.html
https://docs.hise.audio/scripting/scripting-api/unorderedstack/index.html

What this new method brings to the table is that you don't have to care about the order of on / off messages anymore due to possible timestamp manipulations, as long as the event ID matches you're fine now - I agree that this is messy and should not be the burden of the script developer.

However I realized that this didn't apply to artificial events (because they are not processed by the safe check as it was). I've extended the robustness of this method (it performs this check now at the API call that adds the note-off) and now I think it should be fine.

I've added the docs including an example that should demonstrate the "proper" way of handling artificial note creation.

Now if that is still not convenient for you, I think the next stop would be an API call that automatically handles this logic. I vaguely remember that you could pass in -1 somewhere in KSP and it would "tie" the note to the note off for the current note-on message and the HiseScript equivalent would be:

// adds a note on that will be automatically stopped at the note-off message with the given source event ID
Synth.attachNote(int sourceEventId, int noteNumber, int velocity, int timestamp);

Lindon

@Christoph-Hart said in MIDI Processor Inconsistent/Processing Speed Issue?:

Now if that is still not convenient for you, I think the next stop would be an API call that automatically handles this logic. I vaguely remember that you could pass in -1 somewhere in KSP and it would "tie" the note to the note off for the current note-on message and the HiseScript equivalent would be:
// adds a note on that will be automatically stopped at the note-off message with the given source event ID
Synth.attachNote(int sourceEventId, int noteNumber, int velocity, int timestamp);

Wouldnt this be nicer (and more flexible) like this:

Synth.attachNote(int sourceEventId, int targetEventId);

Christoph Hart

@Lindon Ah you mean first you call addNoteOff() (or whatever), then "link" the returned event ID of the new note to the sourceId? Yes that's even better.

aaronventure

@Christoph-Hart said in MIDI Processor Inconsistent/Processing Speed Issue?:

@aaronventure well if you spawn off multiple events from a single note then you definitely have to keep track of them and call note-off in the respective note-off callback of the message. I don't think this is a design flaw, but just a reasonable bookkeeping task (again, think of it as RAII: the one who created it (key down) is responsible for the cleanup (key up).

I think you misunderstood me, artificial events are no longer an issue for me after you showed me how to pass down event ID-related info down the chain and keep it tight.

The issue is the one from the very first post in this thread: two notes in a chord, not perfectly on grid, but same timestamp. MIDI Processors get processed in parallel instead of serially.

It invalidates any logic and scripting that wants to extend beyond not even a single scripting processor, but a single callback (e.g. setting flags for whatever down the line). Logic that isn't organized into per-note, 128-size arrays (and even more for MPE) gets overwritten by the second parallel event. If there's a note launch there and that's all you need, fine. But if there's anything down the line that you're hoping will successfully refer to the logic here, unless the data is stored into per-note ( x per channel for MPE) arrays, it gets overwritten.

This is why being able to store data along with MIDI events is crucial. If the per-note event ID array model is not a good fit for your project for any reason and you want to play artificial notes, you're fucked without being able to pass down event-related data.

So my observation was just about the importance of storing event IDs in a per-note (x per channel for MPE) array for any project dealing with artificial notes, unless you want to deal with writing a custom note filter(like I have). The queue removal is great and eliminates the need for a custom note filter, but it doesn't work if a simple chord causes parallel note-on execution that overwrites your artificial note ID unless you have proper storage (per-note array, x per-channel for MPE).

MIDI Processor Inconsistent/Processing Speed Issue?

14

1.9k

10.7k

93.0k