-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update circle to test stuck notes issue #755
base: main
Are you sure you want to change the base?
Conversation
Build for testing: |
I think we need a bit more detail here - what makes you think circle is the issue? These PRs are adding an awful lot of code between them which is a pretty major concern to me and almost certainly likely to lead to performance issues at some point - especially including networking into what is essentially a real-time, latency-critical signal processing system... To be honest, I have to suggest that if someone wants a networked Dexed, then maybe they ought to be using Zynthian or Raspberry Pi OS with Dexed itself... One of the key unique features of MiniDexed is that it is an embedded appliance. If you're just going to stick it on a Pi at the end of a network link, the maybe you don't really need an embedded system in a box like this (in my view). Kevin |
Yes, in this build I get stuck notes when playing rapid glissando on the MIDI keyboard. No networking code involved here at all. If you look at |
I mean why do you think this will solve the stuck notes issue, which has been reported as part of those other two PRs? Is it a problem with the current release? If so, they why do you think updating circle will solve it. If not, then why are we updating circle at all? Kevin |
This PR is not meant to be merged. Just to show (by the way of providing a build) that merely updating circle is what causes the "stuck notes" issue. |
@rsta2 do you have an idea on what might be going on here? We suspect that circle Step48 introduces an issue that causes notes to be stuck (playing forever) in MiniDexed (as in the test build a few posts above), whereas this issue did not exist in Step47. Thank you very much for having a look! |
So are you're saying this update actually /causes/ the stuck notes issue or fixes it? I've not seen anything reported against the main build that there is an issue with stuck notes...? If this update to circle is causing it, then we'd probably need to know what hardware you're using; what MIDI interface; does it have issues with serial or USB MIDI or both; what does the profiler say is going on with respect to process/audio performance; does the MIDI dump show out of sequence MIDI events; and things like that really... Kevin |
With this branch I get stuck notes much sooner. |
This (updating circle with no other changes) causes stuck notes. |
In that case can you both please provide details of your hardware configuration and under what conditions stuck notes are observed? Specifically:
@soyersoyer can you describe what the "hold mode" is for your controller? Kevin |
|
I'll see if I can reproduce something here too to try to work out what is going on. It would be particularly good to know if this is just a USB thing or not. That might give some clues. I wonder if there are similar issues in USB gadget mode... Kevin |
For me, RPi3 A+, USB Host mode with Arturia Keyboard, it seems NoteOn and NoteOff becomes inverted after bug triggers. Pressing a key sends a NoteOff, depressing it sends a NoteOn. This is why it feels like it become stuck. Hard to capture trigger point for the bug on MIDIDump to console. |
I hear some crackling when it happens. According to the logs, noteoff messages are not being received. |
With the Minilab3 firmware 1.2.0, there are no more stuck keys in Hold mode. Neither with the main branch nor with this one. |
Thanks @soyersoyer although I suspect that you might still get stuck notes if you
|
With Minilab3 (fw v1.2.0) there are no stuck notes. But with Minilab2 it happens pretty quickly. |
I bisected it with rpi3b+, this commit is the culprit: rsta2/circle@ae00d9d |
@soyersoyer Thanks, mentioned at circle |
Note to self, TODO: Retest once Circle Step49 is available. |
DO NOT MERGE THIS. It causes stuck notes.
No other changes.
See whether we get the stuck notes issue.
References: