-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ControllerManager: poll every 1ms on Linux #2966
Conversation
Polling at 5ms causes a noticable lag with the GUI. If the audio buffer is set low, it also produces a lag with the audio. The bug report ( https://bugs.launchpad.net/mixxx/+bug/990992 ) referenced for why this was increased to 5 ms on Linux is 8 years old. The root cause of that bug was not identified, but increasing the controller polling latency to 5ms worked around it. After setting the polling latency back to 1ms, I can not reproduce the bug with high CPU usage on modern Linux (Fedora 32 running kernel 5.4.39-rt23.fc32.x86_64 with realtime patch set).
Also kScratchTimerMs in ControllerEngine assumes 1ms polling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have never understand why we need a higher polling frequency than the audio thread.
This is IMHO only useless CPU burning,
Because the values are only evaluated during the next audio thread.
We should work to fix the root cause and not rush into a already fixed bug.
We need a higher polling frequency than the audio thread frequency so the GUI does not lag. |
Please describe exactly the GUI lag and audio lag you experience. |
At least this is nothing to be changed in a rush just before the 2.3 release. |
The GUI is updated with 60 Hz. |
If I move the volume fader quickly with a 5 ms polling timer, I can see a lag before the fader on screen responds. IMO timing the controller polling with the audio thread is not a good solution because the GUI would become very unresponsive with high audio buffers. |
How long is the lag you see? |
Interestingly I notice no lag using my A&H Xone K2 MIDI controller with a 5 ms polling timer. But using my friend's NI Kontrol S2 Mk2 HID controller, the lag is very noticable. |
I tested with a proof-of-concept bypassing the whole common-hid-packet-parser.js library and only mapping a single fader using a DataView on the ArrayBuffer to verify the problem was in C++, not the controller script. I also tested cherry-picking e8e2487 with that minimal proof-of-concept HID script. Only changing the polling timer fixed the lag. |
I cannot quantify it exactly, but it is very noticeable. It was the first thing my friend noticed about Mixxx when trying to use Mixxx with her controller. |
As far as I can tell whatever caused that bug with 100% CPU usage 8 years ago is no longer relevant. It may have been a kernel bug that has since been fixed or a kernel configuration in distributions which is no longer used. With a 1 ms polling timer, Mixxx is using less CPU (~5-6%) than Firefox running in the background (~6-8%). Please test on other Linux distros to verify this. Can we get people on different distributions to test this and report Mixxx's CPU usage? @Holzhaus can you test on Arch? |
If we consider a 33 ms waveform as smooth, why can a 5 ms -> to 1 ms change make a difference. |
I cannot reproduce the lag with Mixxx 2.2 using the Kontrol S2 Mk2 on Linux. I suspect #2179 introduced this bug, so IMO this is a high priority bug to fix before releasing 2.3.
Quite possibly. But this simple change works around it and I do not notice any downside. If we get bug reports from beta users about high CPU usage we can revert it and try other approaches. |
@ywwg are you able to reproduce the lag with the Kontrol S3 on Linux using Mixxx 2.3 and confirm that there is not lag with 2.2? |
It is also possible that changes in Qt in the last 8 years have made the CPU usage issue obsolete. |
I tested both 2.3 and master to confirm the bug was not introduced by the switch to QJSEngine (#2682). |
Interesting, I suppose the synchronous HID polling could've changed this behavior. I believe #2179 has several different solutions to the problem, it might be worth testing if using those instead of the one that's being used right now fixes the problem. (Particularly the "loop until no more messages" thing that ended up not being super useful at the time.) |
Interesting, I tested a proof of concept with a 5 ms poll timer that loops 5 times per call to HidController::poll and the GUI is as responsive as the 1 ms timer. |
Otherwise, it seems messages build up in a queue which produces a dramatic lag on Linux when moving faders quickly. This regression was introduced between Mixxx 2.2 and 2.3 beta, likely by switching HidController to nonblocking polling in PR mixxxdj#2966.
When I set the poll timer very high (100 ms) with #2970, I do not see the dramatic GUI lag as the current 2.3 branch. Instead, when I move the fader quickly, I see choppy discrete movements, but I do not perceive a dramatic delay between moving the fader and seeing a response. So I think maybe we should go with @daschuer's idea to couple the controller polling timer to the audio buffer size. Polling controllers at 5 ms doesn't make sense if the audio buffer is smaller than that. |
Polling at 5ms causes a noticable lag with the GUI. If the audio
buffer is set low, it also produces a lag with the audio. The
bug report ( https://bugs.launchpad.net/mixxx/+bug/990992 )
referenced for why this was increased to 5 ms on Linux is 8 years
old. The root cause of that bug was not identified, but increasing
the controller polling latency to 5ms worked around it. After
setting the polling latency back to 1ms, I can not reproduce the
bug with high CPU usage on modern Linux (Fedora 32 running kernel
5.4.39-rt23.fc32.x86_64 with realtime patch set).