Headless client: the gain/pan rate limiter never adapts to ping time, and branches on an uninitialised value

Inizia in src/client.cpp e src/client.h, in corrispondenza di CClient::StartTimerGainOrPan, iCurPingTime e del costruttore, quindi segui la gestione del ping attraverso src/clientdlg.cpp e CClientDlg::OnTimerPing. Confronta i due approcci indicati per gli headless clients e valida il comportamento scelto con una headless session, valgrind e la spaziatura dei fader-message in presenza di un ritardo di andata e ritorno diverso da zero.

🤖 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:

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:

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 --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 fa8766ad 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 (c862872e); 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.