Fix playing notes notes retriggering when changing clip during first loop in Clip Launcher

[dikadk]

Step to reproduce:

1. Add StepClip to ClipLauncher ClipSlot 0
2. Update notes using GUI

Issue only exists in the first loop if clip is played by ClipLauncher, so I had to modify StepSequencerDemo to include ClipLauncher to debug.

https://github.com/user-attachments/assets/d8d77c67-a263-4452-9baf-5ecb70258e66

https://github.com/user-attachments/assets/5f64b2ce-d13e-4a5c-ac8d-68421edea190

This is updated StepSequencerDemo which uses ClipLuancher StepSequencerDemo.h.zip

[drowaudio]

Thanks for working on this. There's quite a few changes involved here, can you briefly go over the problem and rationale behind the fix please so I've got some background for the code review?

[dikadk]

Thanks for working on this. There's quite a few changes involved here, can you briefly go over the problem and rationale behind the fix please so I've got some background for the code review?

Yep, sure. Already playing notes were getting "stopped" and "started" when Clip's midi sequence was regenerated. It was happening because in LoopingMidiNode when sequencer was changing, it was clearing all playing notes and starting them in the same block without accounting if any of them where playing. Video of the issue is on the first video. Then I came across this comment: https://github.com/Tracktion/tracktion_engine/blob/cd002bb9240d387c99168ca36229cc599a2838b0/modules/tracktion_engine/playback/graph/tracktion_LoopingMidiNode.cpp#L726

I tracked it down to only happening on the first loop of LoopingMidiNode and added logic to account for any notes playing.

[dikadk]

@drowaudio Thanks for reviewing PR.

I'm trying to understand all the ins and outs here but there are lots of small changed that could have a big effect.

I guess what I'm struggling with is what bit is actually broken with the current implementation? The activeNoteList is supposed to track the active notes and then be checked when properties of the clip or sequence change. IIRC the idea is that the current sequence is read and if a note-on is in the sequence but an "active note" is already found, it should be skipped. If you're saying this isn't happening, then the fix should be much smaller than all these changes right?

Yeah, you're right, fix is smaller.

I was able to scrape unnecessary changes I thought were needed to fix re-trigger bug. Especially bringing back handling of sustain pedal to prevent stuck notes.

I'm just very worried that in making this changes I break something else subtle.

Ideally, I'd need some unit tests but the MIDI is one of the areas I've yet to fully add tests for. Since I wrote this, I've written a testing audio device which can be used to run the audio engine in a test harness very precisely to check behaviour. I just need time to construct test cases.

Nice, I would nice test this in the isolated environment.

[dikadk]

@drowaudio, I've revised PR and confirmed this bug is fixed even with these minimal changes. Sorry for making you go through some extra code. Your questions helped me narrow down the code changes needed.

[drowaudio]

Ok, sorry for the long delay on this, it just took me a while to get some head space and a release out so I could think about it. I've basically implemented what you've suggested here now: https://github.com/Tracktion/tracktion_engine/commit/2daa8e847c4e71bfec01994e7c478ee4538e1121

Grab it and let me know if it does indeed fix your original issue. Thanks for your patience 🙌