TonexOneController icon indicating copy to clipboard operation
TonexOneController copied to clipboard

Intermittent Latency

Open yikelu opened this issue 8 months ago • 32 comments

From time to time, the controller will take 0.5~2 seconds before transmitting received MIDI messages. I have experienced this with both Waveshare 1.69 (1.0.8.2) and Waveshare Zero (both 1.0.8.2 and 1.0.9.2). I am using wired MIDI via the Featherwing board and powering with pedalboard power going through a DC conversion PCB. TX1 is on latest firmware, but this problem also occurred on previous firmware.

I can verify that the MIDI is received because the Featherwing will blink and other MIDI devices I have will change their state.

It is possible this is due to ToneX One being slow to respond and not a controller problem, but seems unlikely.

This is hard to reproduce because I haven't been able to identify the conditions under which this occurs. It appears random, although typically it will be some amount of time (~30 minutes) after the everything has powered on.

yikelu avatar Apr 26 '25 04:04 yikelu

That's a tough one. Could you outline what midi messages you are sending, and how often? Maybe that will give me some clues on what to investigate

Builty avatar Apr 26 '25 10:04 Builty

I primarily observe this when sending PC change messages to TX1, however I do batch them together with other MIDI messages for other devices (other channels). I may send 3-4 at a time, for example: PC 0 to TX1 channel 16, CC 101 = 0 channel 2, CC 103 = 0 channel 2. Frequency of sending this group of messages is something like once every 30 seconds (think song transition changes).

I have also sent this group of messages once every ~2-5 seconds during testing and there's no problems doing this. For this reason, it looks unlikely it is a queue drain problem caused by MIDI overload.

That reminds me: this issue happens and persists for 5 ish minutes before self resolving.

I also send bigger batches of MIDI messages directly to TX1. This is typically like 6 CC messages to configure the exact parameters of a particular effect I want activated, for instance: delay = tape, mode = normal, mix = 45%, feedback = 30%, sync = on, delay = on. However, this is pretty rare and I typically wasn't doing this when I would notice the latency problems. Nonetheless, I have observed the latency issue affecting CC messages as well.

Bluetooth MIDI should be off, but my memory is hazy so I can't confirm that I've had this issue with BT MIDI off.

yikelu avatar Apr 26 '25 13:04 yikelu

Pretty sure I found the culprit, and it is a queue backup. I forgot I am sending MIDI clock (which incidentally, have not gotten it to work in the controller).

If I increase the rate of the clock to something like ~180 BPM, the latency starts happening. If I slow it down to ~80BPM it goes away.

yikelu avatar Apr 26 '25 21:04 yikelu

Ah OK, yes it sounds like an issue with the queue as you said. Midi clock is currently ignored in the controller.

Builty avatar Apr 27 '25 10:04 Builty

Midi clock is currently ignored in the controller.

If this is the case, what is the "Tempo Src" feature in the global section on the web config?

As a workaround I have disabled MIDI clock on my sending devices, however, is there a more permanent solution that could be coded? Somehow have the controller discard MIDI clock further up the chain so processing is faster?

yikelu avatar Apr 27 '25 17:04 yikelu

The tempo src option controls whether the tempo setting applies just to the current preset, or to all presets. It's part of the Tonex, not related to Midi.

I'll investigate the Midi clock and see if it can be improved.

Builty avatar Apr 27 '25 21:04 Builty

LMK if I can help. I can code generally, not familiar with the specifics of embedded or USB protocol, but happy to learn. I would suggest "tempo src" be renamed to "global tempo" for clarity though.

yikelu avatar Apr 28 '25 02:04 yikelu

I used the term "Tempo Source" as that is what IK Multimedia calls it.

Builty avatar Apr 28 '25 07:04 Builty

I took a look at this and I don't know if there's really a clever solution. The MIDI Featherwing looks to be primarily an analog device, there's no processor or anything, so nothing to do to block clock from that angle.

Then as far as the code, you already handle clocks here: https://github.com/Builty/TonexOneController/blob/89995e241fd0a287e9455e63bd86553873fa2609/source/main/midi_serial.c#L114C1-L118C1

I can't really think of a faster way of skipping clocks than what you already have.

At the same time, I looked at the clock spec and it makes sense why it jams up the queue: MIDI clock sends either 24 clock ticks per quarter note. At 180BPM, that's a clock spacing of about ~13 ms. So maybe if you do pdMS_TO_TICKS with 1ms it would help, but I'm guessing that increases power consumption / possible heat issues.

You can move the check for MIDI channel outside the PC/CC message switch cases, but that's a fairly small improvement for this; it's clearly the clock spam, not off-channel spam.

yikelu avatar May 01 '25 20:05 yikelu

Not sure if this needs another thread, but since its midi latency related I'll post it here. What I'm seeing is severe latency when changing parameter values (e.g. Delay Time) via midi. I tried both as wired and BT with the same results. The scenario is to use a midi controller with a pot (outputting midi CC values 0-127) to alter the parm value. It eventually might get there after a few seconds of jumping around, or not at all. FWIW, doing program changes via BT and my old MidiBuddy program change pedal works fantastic; all 20 preset scenes available at the press of a button is really nice to have. No latency in sound on changes. I'm only seeing issues doing parm changes. Waveshare Zero.

sekim6 avatar May 01 '25 23:05 sekim6

I'm guessing that turning the pot spams MIDI values and the queue gets gummed up. If the changes get lost, it's probably due to the queue overflowing.

yikelu avatar May 02 '25 01:05 yikelu

Based on what I read here I figured that might be the case. Its a "nice to have" feature so I can live without it, and its the program change feature that is the "deal maker" for me. Which the way I'm using it, works great since the sole midi "stream" is nothing but the program change signals. That said, my MidiBuddy pedal is modded to also support expression and glissando via a second midi transmitter placed inside the pedal that simply parallels into the midi out jack. Logically, I will never use program change and expression/glissando at the same time, so no chance of midi packet collision. Those are on a separate midi channel than defined in the TO controller, so I assume there should be no issue even though BT is sending them? (haven't had a chance to test that scenario yet)

sekim6 avatar May 02 '25 02:05 sekim6

Those are on a separate midi channel than defined in the TO controller, so I assume there should be no issue even though BT is sending them? (haven't had a chance to test that scenario yet)

I'm not 100% sure, but in looking at the code, there is one primary loop that processes all MIDI messages, and it has to look at everything even the ones that are on different channels.

If you don't use them at the same time you're most likely good, but I wouldn't assume the "not on the same channel" part saves you.

It's an open question for @Builty whether we should increase the core MIDI loop polling rate. Currently it takes at least 5 ms per group of MIDI messages (and I don't know to what degree they arrive together, so it may literally be 5 ms per single message to process).

Edit: possibly we could make that an optional parameter settable in the Web GUI?

yikelu avatar May 02 '25 03:05 yikelu

Yikelu is correct. Normally a Midi input to a device would be a direct connection to the main processor. That allows fast processing. As the Tonex One doesn't have native Midi, its received by the controller and then passed on to the Tonex via USB. This bridging step is slower than a native connection, and also does you a mesasage queue between multiple processes.

As such, its not well suited to continuous/analog style adjustment that generates a new mesage for every single tiny change. In the web and 4.3B ui, the change happens only when the slider is released.

The 5 msec delay could be removed but the Tonex USB transfer side of things is more the bottleneck

Builty avatar May 02 '25 03:05 Builty

Actually, what could be done is for the receiver to check if it has multiple messages in queue, which are modifying the same value. And skip the intermediate ones and just send the most recent one.

Builty avatar May 02 '25 03:05 Builty

I think a lot of midi implementations do something similar, based on what I've seen monitoring them.

sekim6 avatar May 02 '25 03:05 sekim6

This should take care of it. I will test later today.

Image

Builty avatar May 02 '25 03:05 Builty

I'll give it a whirl when you post a beta.

sekim6 avatar May 02 '25 04:05 sekim6

I've been looking at the blocking logic and I'm wondering ... are the vTaskDelay calls in the control_task and midi_serial_task necessary? They all have a full 20 ms block for their input queues. Having the vTaskDelay essentially imposes a rate limit on how fast they can each drain their respective queues. Removing them would only hog the CPU if there is spam on the input queues.

Arguably, you could even extend the queue read blocks to something like 500 ms with no consequences, as they are essentially a read timeout of sorts. If input appears, the task should essentially unblock immediately.

yikelu avatar May 02 '25 06:05 yikelu

The vTaskDelays are in there specifically so that if there is a lot on queue data, the other lower priority tasks are not starved. I can't necessary control how much queue data arrives, and I need to make sure there is no chance of task starvation.

Builty avatar May 02 '25 07:05 Builty

I've tested that fix, it does help. Can see cases where it was able to save quite a few USB transactions from occurring. Also I tweaked the task timings a little. Its still not that difficult to overwhelm it if you drag a slider/knob over a wide range quickly, but has improved. I think this one might need some more thoughts on what else could be done to improve this area.

Builty avatar May 02 '25 09:05 Builty

The vTaskDelays are in there specifically so that if there is a lot on queue data, the other lower priority tasks are not starved. I can't necessary control how much queue data arrives, and I need to make sure there is no chance of task starvation.

OK, I see how you're right. For instance midi_serial could pile up and completely starve the control task. Still, the cases this would happen are pathological abuse by the end user, and I wonder are the existing values too conservative? Maybe 1ms or 1/2 ms, either of which is still an order of magnitude larger than the context switch latency of ~15us.

Also, there are some small optimizations I can see in the midi_serial loop.

  1. Test for MIDI channel outside of the CC/PC checks.
  2. Do a longer vTaskDelay only if something has actually been executed (ie, there was a CC or PC message of the correct channel). Otherwise do a short one.
  3. Increase the time for uart_read_bytes. If there are not any MIDI messages, midi_serial can stay blocked for much longer.

If these make sense, I can code them.

Edit: I do have experience with low latency systems, but no background in embedded systems, so I defer to you on these judgment calls.

yikelu avatar May 02 '25 17:05 yikelu

You are 100% correct on the uart_read_bytes. As the amount of bytes to read is big, it would incur the timeout value every time. I've lowered that to 5 msec now. The test for Midi channel would be a matter of microseconds.

I did a bigger change though. Every time a value is changed, I refresh the UI (4.3B + Wweb UI.) In the case were Midi data is streaming, the UI updates get overwritten each time. I've changed it now to skip the UI update if another message is queued that would cause that update to be overwritten. That has made a nice improvement to latency.

These changes are committed now. Maybe its enough already.

Builty avatar May 02 '25 23:05 Builty

I'd be happy to help test. Is there a place I can download it?

sekim6 avatar May 02 '25 23:05 sekim6

I haven't done any builds yet, but I can. what PCB are you using?

Builty avatar May 03 '25 01:05 Builty

I'm using a Zero.

sekim6 avatar May 03 '25 01:05 sekim6

Wow! This works great! Changing delay time is as good as using TE, if not better!

Spoke a little too soon, it can still be overwhelmed with a fast slider, but now its usable with a gentle hand. Big improvement for sure.

sekim6 avatar May 03 '25 01:05 sekim6

Well, I spoke a little too soon....it can still be overwhelmed with a fast slider, but its usable now if you take it easy on it. Mass improvement for sure.

sekim6 avatar May 03 '25 01:05 sekim6

You are 100% correct on the uart_read_bytes. As the amount of bytes to read is big, it would incur the timeout value every time. I've lowered that to 5 msec now. The test for Midi channel would be a matter of microseconds.

Am I understanding incorrectly? uart_read_bytes called with a longer timeout should be safer and cause less latency? It blocks (therefore yields) priority until there is something to read, at which point it then executes everything in the body of the loop.

From my perspective, the true dead zone where no information can be processed is vTaskDelay, whereas uart_read_bytes timer is a "green zone" of sorts. It will come back as soon as there are bytes available and likely almost always execute the loop only on a single message.

I was thinking it should be uart_read_bytes(..., pdMS_TO_TICKS(200)) and vTaskDelay(pdMS_TO_TICKS(1)) or similar. Haven't had a chance to test yet.

yikelu avatar May 03 '25 02:05 yikelu