search

kampfschlaefer / jackmix

Matrix-Mixer for the Jack-Audio-connection-Kit
http://www.arnoldarts.de/jackmix/
GNU General Public License v2.0
44 stars 13 forks source link

jackmix : Segmentation fault when receiving All Sound Off controller message #17

Closed Gerbmouse closed 2 years ago

Gerbmouse commented 2 years ago

Compiled the no-lash branch to be able to use scons with python3. Jackmix is connected to a midi controller in parallel to other softsynths. Main outs of jackmix learned to General Purpose #1 Controller message. That works fine. When I send an All Sound Off (controller #120) message followed by a Program Change message to change the sound of my softsynths it segfaults jackmix. Ideally jackmix should ignore all midi data that it cannot handle?!

Thank you for your time Gerhard

nickbailey commented 2 years ago

Thanks for your report @Gerbmouse and it will be 'my bad' as I'm to blame for the MIDI stuff. I think I know where to look.

I only ever use a small MIDI controller for live mixing, so I've not tested it for other use cases. I don't have the kit. So your report is valuable. I'll try synthesizing some all-off and prog-change messages with sendmidi to reproduce the problem and see if I can get it fixed.

nickbailey commented 2 years ago

This is stranger than I expected. I couldn't reproduce the segfaults exactly. For example issuing sendmidi dev Control\ Input cc 1 1 or sendmidi dev Control\ Input pc 1 1 did not cause jackmix to crash. I suspected I wasn't filtering the controller messages properly, but I looked and I was: https://github.com/kampfschlaefer/jackmix/blob/7ceab734e0d9e7b24971f99aab87f6a8ec5897c5/libcontrol/controlsender.cpp#L62 Adding the qDebug three lines further down shows normal controller events are handled properly.

Issuing sendmidi dev Control\ Input panic did cause my build to crash, and it did so when parameter 120 and above was received. Looking in the header file, I have maxMidiParam as 120 (now changed to 128). It could have been a typo, but I probably looked it up. As I mentioned, I'm no MIDI expert. This was causing a reference to an undeclared ControlReceiver via an illegal index to dtab (no, I'm NOT going to rewrite it in Rust! ;) In any case, I suspect I was under the impression that the index couldn't ever exceed 119 as part of the MIDI spec when clearly it can).

So commit 7ceab734e0d9e7b24971f99aab87f6a8ec5897c5 clears up a potential crash, but probably not the ones you mean.

Can you tell me what gets printed out when you send the commands which cause the program to crash? There should be two integers per line, if it's part of the same problem.

If you understand MIDI better than I do and think I'm handling events in the wrong way, please say so, because all help is appreciated. Happy Hogmanay!

nickbailey commented 2 years ago

I read up on MIDI a bit and it turns out that Channel Mode Messages are actually just control messages with controller code >= 120. This explains where the original value for maxMidiParam came from. CMMs are now ignored, whereas previously the code would have been used to index a (non-existant) dispatch target, causing a segfault.

If we want CMMs to do something, here's where the code will have to go: https://github.com/kampfschlaefer/jackmix/blob/a836daf85fe3c0f993b214fdbd5b1f05767815ef/libcontrol/controlsender.cpp#L132

This should close this issue.

Gerbmouse commented 2 years ago

Thanks a bunch. Just git fetched and compiled. Will test it now.