🤖 AI: The rate limiter added in #2535 has two halves: a 50 ms floor, and double the current ping time, whichever is greater. In a headless build only the floor is ever used, and the value the branch tests has never been written.
CClient::StartTimerGainOrPan tests iCurPingTime. That member is assigned in exactly one place, CClient::OnCLPingReceived, which is reached only after CClient::CreateCLPingMes(). That function has one caller in the tree: CClientDlg::OnTimerPing. A client started with -n has no dialog, so no ping is sent and iCurPingTime is never assigned.
A recording UDP proxy sat between a real client and a real server with a symmetric delay injector; five bursts of 150 back-to-back fader changes per trial; CLM_PING_MS (1001) frames counted and gain-message spacing read off the capture:
| client |
round trip |
CLM_PING_MS frames on wire |
gain spacing |
| headless |
0 ms |
0 |
49 ms |
| headless |
122 ms |
0 |
121 ms |
| GUI |
2 ms |
61 |
49 ms |
| GUI |
60 ms |
66 |
122 ms |
| GUI |
122 ms |
64 |
244 ms |
Spacing follows max(timer period, round trip) within 2 ms across all five trials, with the period being max(50, 2 x ping) for the GUI client and a constant 50 ms for the headless one. The headless 121 ms is the protocol's one-unacknowledged-message-at-a-time limit, not adaptation; only the 0 ms row separates the two.
Uninitialised read
iCurPingTime has no initialiser and is absent from the constructor's initialiser list, and main.cpp constructs CClient on the stack. valgrind memcheck with --track-origins=yes reports exactly one uninitialised-conditional context in a full headless session, and it is this branch:
Conditional jump or move depends on uninitialised value(s)
at CClient::StartTimerGainOrPan()
by CClient::SetRemoteChanGain(int, float, bool)
by CClient::OnControllerInFaderLevel(int, int)
...
Uninitialised value was created by a stack allocation
at main
Effect
#2492 reported MIDI fader changes lagging the audio by about 10 s, which #2535 fixed. The same symptom is reproducible in a headless client at a round trip of 122 ms, because gain messages are generated faster than the acknowledgement gate clears them. 600 s of continuous fader motion at 50 ms intervals:
|
headless |
GUI |
| fader changes driven |
11,938 |
11,940 |
| gain messages generated |
11,938 (nothing coalesced) |
2,402 |
| sent during the drive |
4,963 |
2,401 |
| still queued when the fader stopped |
6,975 |
1 |
| drain rate |
8.27/s (one per 121 ms) |
— |
| delay on a chat message sent at that moment |
843 s |
0.0 s |
The queue itself is cheap: RSS grew 228 KB for those 6,975 messages, which is less than the GUI client's ordinary growth over the same ten minutes, and the client's outbound audio cadence is unchanged between an empty queue and a 6,975-deep one (median/p95/p99 3/3/3 ms in both windows). The wire rate stays at one protocol message per round trip throughout. What grows is the delay on everything else sharing the send queue — chat, mute state, jitter buffer size, channel info all wait behind the gain messages.
The onset sits at the limiter's own floor. The same fader drive held for 60 s per round-trip step, with a chat message sent at drive end through the same queue as a wire-visible marker:
| round trip |
queued at drive end |
chat marker delay |
| 0 ms |
0 |
0 ms |
| 22 ms |
0 |
0 ms |
| 32 ms |
0 |
0 ms |
| 40 ms |
0 |
0 ms |
| 52 ms |
18 |
935 ms |
| 62 ms |
211 |
12.9 s |
| 80 ms |
453 |
36.7 s |
| 122 ms |
698 |
84.5 s |
| 122 ms (GUI) |
0 |
99 ms |
Every measured round trip above the 50 ms floor builds a backlog; every one below it does not.
Headless clients take fader input from --ctrlmidich and from jamulusclient/setFaderLevel, which is the same path #2492 came in on.
Two directions
Initialising iCurPingTime removes the undefined behaviour and leaves the headless limiter at the fixed floor. Moving the ping timer out of CClientDlg into CClient makes both builds adapt, at the cost of a headless client sending a ping every 500 ms it currently does not send. Which is preferred?
Rig: Jamulus 3.12.3dev, Raspberry Pi 4 aarch64, Qt 5.15.15, jackd -d dummy, valgrind 3.24.0. The headless binary is fa8766a and the GUI binary 24f8f95; git diff 24f8f95 fa8766ad -- src/client.cpp src/client.h src/clientdlg.cpp src/clientdlg.h src/global.h is empty, so the two arms differ in build configuration only. All line references above are to current main (c862872); between it and the measured tree, the only changed line containing any identifier cited here is an indentation-only move of TimerPing.start, so the code measured is the code on main.
🤖 This message was written by AI and reviewed by @mcfnord.
🤖 AI: The rate limiter added in #2535 has two halves: a 50 ms floor, and double the current ping time, whichever is greater. In a headless build only the floor is ever used, and the value the branch tests has never been written.
CClient::StartTimerGainOrPantestsiCurPingTime. That member is assigned in exactly one place,CClient::OnCLPingReceived, which is reached only afterCClient::CreateCLPingMes(). That function has one caller in the tree:CClientDlg::OnTimerPing. A client started with-nhas no dialog, so no ping is sent andiCurPingTimeis never assigned.A recording UDP proxy sat between a real client and a real server with a symmetric delay injector; five bursts of 150 back-to-back fader changes per trial;
CLM_PING_MS(1001) frames counted and gain-message spacing read off the capture:Spacing follows
max(timer period, round trip)within 2 ms across all five trials, with the period beingmax(50, 2 x ping)for the GUI client and a constant 50 ms for the headless one. The headless 121 ms is the protocol's one-unacknowledged-message-at-a-time limit, not adaptation; only the 0 ms row separates the two.Uninitialised read
iCurPingTimehas no initialiser and is absent from the constructor's initialiser list, and main.cpp constructsCClienton the stack. valgrind memcheck with--track-origins=yesreports exactly one uninitialised-conditional context in a full headless session, and it is this branch:Effect
#2492 reported MIDI fader changes lagging the audio by about 10 s, which #2535 fixed. The same symptom is reproducible in a headless client at a round trip of 122 ms, because gain messages are generated faster than the acknowledgement gate clears them. 600 s of continuous fader motion at 50 ms intervals:
The queue itself is cheap: RSS grew 228 KB for those 6,975 messages, which is less than the GUI client's ordinary growth over the same ten minutes, and the client's outbound audio cadence is unchanged between an empty queue and a 6,975-deep one (median/p95/p99 3/3/3 ms in both windows). The wire rate stays at one protocol message per round trip throughout. What grows is the delay on everything else sharing the send queue — chat, mute state, jitter buffer size, channel info all wait behind the gain messages.
The onset sits at the limiter's own floor. The same fader drive held for 60 s per round-trip step, with a chat message sent at drive end through the same queue as a wire-visible marker:
Every measured round trip above the 50 ms floor builds a backlog; every one below it does not.
Headless clients take fader input from
--ctrlmidichand fromjamulusclient/setFaderLevel, which is the same path #2492 came in on.Two directions
Initialising
iCurPingTimeremoves the undefined behaviour and leaves the headless limiter at the fixed floor. Moving the ping timer out ofCClientDlgintoCClientmakes both builds adapt, at the cost of a headless client sending a ping every 500 ms it currently does not send. Which is preferred?Rig: Jamulus 3.12.3dev, Raspberry Pi 4 aarch64, Qt 5.15.15,
jackd -d dummy, valgrind 3.24.0. The headless binary is fa8766a and the GUI binary 24f8f95;git diff 24f8f95 fa8766ad -- src/client.cpp src/client.h src/clientdlg.cpp src/clientdlg.h src/global.his empty, so the two arms differ in build configuration only. All line references above are to currentmain(c862872); between it and the measured tree, the only changed line containing any identifier cited here is an indentation-only move ofTimerPing.start, so the code measured is the code onmain.🤖 This message was written by AI and reviewed by @mcfnord.