From 174827d32c2246d47f465da72572cb063e4219c4 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 21:50:14 +0000 Subject: [PATCH 01/17] buffer.cpp: state the actual bound the block-count divide needs The input is iNumBlocks * ( iBlockSize + iNumBytesSeqNum ) bytes, so the divide by iBlockSize is exact only while iNumBlocks * iNumBytesSeqNum stays below iBlockSize. The sequence number being "much smaller" than the coded audio is not the condition: at iNumBlocks == iBlockSize the count comes out one too high whatever the ratio is. --- src/buffer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/buffer.cpp b/src/buffer.cpp index 441b4121e4..c191c11ba6 100644 --- a/src/buffer.cpp +++ b/src/buffer.cpp @@ -164,8 +164,8 @@ bool CNetBuf::Put ( const CVector& vecbyData, int iInSize ) return false; } - // to get the number of input blocks we assume that the number of bytes for - // the sequence number is much smaller than the number of coded audio bytes + // to get the number of input blocks we assume that the total sequence number + // overhead, iNumBlocks * iNumBytesSeqNum, is smaller than iBlockSize const int iNumBlocks = /* floor */ ( iInSize / iBlockSize ); // copy new data in internal buffer From b08eca65ad5f3439159ad016e51f669075d564f2 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 21:50:22 +0000 Subject: [PATCH 02/17] buffer.cpp: correct the wrap-detection horizon to 128 counts The fold five lines above maps the difference into -128...127, so detection fails past half the wrap distance, not the full 256. 128 counts is 170 ms at the fastest frame rate (64 samples at 48 kHz, 750 counts/s) and 341 ms at 128-sample frames, so the "more than 100 ms" figure was low as well. --- src/buffer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/buffer.cpp b/src/buffer.cpp index c191c11ba6..a219204332 100644 --- a/src/buffer.cpp +++ b/src/buffer.cpp @@ -191,8 +191,8 @@ bool CNetBuf::Put ( const CVector& vecbyData, int iInSize ) } // The 1-byte sequence number wraps around at a count of 256. So, if a packet is delayed - // further than this we cannot detect it. But it does not matter since such a packet is - // more than 100 ms delayed so we have a bad network situation anyway. Therefore we + // further than half of this we cannot detect it. But it does not matter since such a packet is + // more than 170 ms delayed so we have a bad network situation anyway. Therefore we // assume that the sequence number difference between the received and local counter is // correct. The idea of the following code is that we always move our "buffer window" so // that the received packet fits into the buffer. By doing this we are robust against From 3351d54a15c08586645749f07576826ab3506ca7 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 21:50:33 +0000 Subject: [PATCH 03/17] client.cpp: a restart is not the only way iActiveChannels is non-zero CChannel drops a channel whose receive timeout expires (CON_TIME_OUT_SEC_MAX, 30 s in channel.h) and treats the next packet from the same peer as a new connection, so a traffic gap alone produces this state from a server that never restarted. The action taken is right either way. --- src/client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.cpp b/src/client.cpp index e1532c96b2..7d0678682a 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1000,7 +1000,7 @@ void CClient::OnControllerInMuteMyself ( bool bMute ) void CClient::OnClientIDReceived ( int iServerChanID ) { // if we have just connected to a running server, iActiveChannels will be 0 - // if iActiveChannels is not 0, the server must have been restarted on the fly + // if iActiveChannels is not 0, the server was restarted or our channel timed out // in that case, channels might have changed, so clear our list to get it afresh. if ( iActiveChannels != 0 ) { From 26e42be9135ad1739fa682fe43bbde9d0bcce828 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 21:50:41 +0000 Subject: [PATCH 04/17] client.cpp: FindClientChannel returns 0 only for an in-range channel ID "should always return channel 0" contradicts the declaration in client.h, which documents the return as "a client channel ID or INVALID_INDEX": FindClientChannel returns INVALID_INDEX for iServerChannelID >= MAX_NUM_CHANNELS. --- src/client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.cpp b/src/client.cpp index 7d0678682a..d30413d04a 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1009,7 +1009,7 @@ void CClient::OnClientIDReceived ( int iServerChanID ) } // allocate and map client-side channel 0 - int iChanID = FindClientChannel ( iServerChanID, true ); // should always return channel 0 + int iChanID = FindClientChannel ( iServerChanID, true ); // returns channel 0 for an in-range iServerChanID // for headless mode we support to mute our own signal in the personal mix // (note that the check for headless is done in the main.cpp and must not From 89c98b390e1e0a3c3af5b620b86fb7b68e26e980 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 21:50:52 +0000 Subject: [PATCH 05/17] server.cpp: drop the stale QtConcurrent::run rationale for the flag The decode workers are dispatched through CThreadPool::enqueue (threadpool.h), which is variadic; QtConcurrent does not appear in server.cpp at all. What does require a member is that DecodeReceiveData writes the flag and OnTimer reads it after the futures join. --- src/server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index a49eab776c..ff8444089d 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -662,7 +662,7 @@ void CServer::OnTimer() bool bUseMT = false; int iNumBlocks = 0; // init number of blocks for multithreading int iMTBlockSize = 0; // init block size for multithreading - bChannelIsNowDisconnected = false; // note that the flag must be a member function since QtConcurrent::run can only take 5 params + bChannelIsNowDisconnected = false; // note that the flag is a member since DecodeReceiveData sets it and the check below reads it { // Make put and get calls thread safe. From 47abc320bab3ad67f0c137d2f04c847b366c0666 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:34 +0000 Subject: [PATCH 06/17] global.h: the 128 assumption belongs to the filter constants, not UpdateAutoSetting SYSTEM_FRAME_SIZE_SAMPLES is 64 and UpdateAutoSetting() assumes nothing about it. The frame-size assumption lives in the IIR_WEIGTH_* constants in buffer.h:80-91, which exist in a 128 set (*_DOUBLE_FRAME_SIZE) and a 64 set, selected by frame size at buffer.cpp:486 and :496. The 64 set is derived from the 128 set by exp(64/128*log(x)) (buffer.h:86); that conversion checks out for all four constants. --- src/global.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/global.h b/src/global.h index 5414a58c41..3b075d7161 100644 --- a/src/global.h +++ b/src/global.h @@ -102,7 +102,7 @@ LED bar: lbr // System block size, this is the block size on which the audio coder works. // All other block sizes must be a multiple of this size. -// Note that the UpdateAutoSetting() function assumes a value of 128. +// Note that the IIR_WEIGTH_* filter constants in buffer.h assume values of 64 and 128. #define SYSTEM_FRAME_SIZE_SAMPLES 64 #define DOUBLE_SYSTEM_FRAME_SIZE_SAMPLES ( 2 * SYSTEM_FRAME_SIZE_SAMPLES ) From d369482059f43d3d7ffd5ade6b39a2922b944469 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 07/17] client.h: drop the declaration-order claim for pSignalHandler No init-order dependency exists. pSignalHandler ( CSignalHandler::getSingletonP() ) and pSettings ( nullptr ) are adjacent initialisers at client.cpp:60-61, neither reads the other, and nothing constructed between them reads either; pSignalHandler is first used at client.cpp:199 and pSettings only via SetSettings() at :219. Swapping the two declarations and rebuilding produced byte-identical startup output. CServer supplies the control: it declares pSignalHandler last (server.h:325) with no ordering comment. --- src/client.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client.h b/src/client.h index b56e5c42d1..ce524da4bd 100644 --- a/src/client.h +++ b/src/client.h @@ -335,7 +335,6 @@ class CClient : public QObject void SetSettings ( CClientSettings* settings ); protected: - // Signal handler must be declared before pSettings for correct init order CSignalHandler* pSignalHandler; // Pointer to settings for MIDI and other config CClientSettings* pSettings; From c150d5cfd9d6b2317c91a21cc56f9e9bdf37cddf Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 08/17] main.cpp: the GUI server does not default to registering somewhere A server GUI started with a fresh ini, no directory options and a clean quit writes -1. -1 is AT_NONE, defined at util.h:612 as "means not registered, invalid value"; settings.cpp:1162 initialises directoryType = AT_NONE and says so. The same run with the ini pre-set to a genre directory logged "Server Registration Status update: Registered"; the -1 arm logged nothing. The justification for not validating here survives without the false premise. --- src/main.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 10f962d4be..cc5ae25eef 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -706,8 +706,8 @@ int main ( int argc, char** argv ) #ifndef HEADLESS if ( bUseGUI ) { - // by definition, when running with the GUI we always default to registering somewhere but - // until the settings are loaded we do not know where, so we cannot be prescriptive here + // when running with the GUI, until the settings are loaded we do not know whether or + // where this server will register, so we cannot be prescriptive here if ( !strServerListFileName.isEmpty() ) { From a1d5b5e269c9a37dad77be64b5d0f03f752e3454 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 09/17] connectdlg.cpp: drop the last-column-width claim The width of all five visible columns is set a few lines below, including the last one (LVC_VERSION), and setStretchLastSection ( false ) is the Qt call that stops the last section absorbing remaining space. Measured on a CConnectDlg resized to 900x600: the five columns total 680 px in an 876 px viewport, so nothing absorbs the rest. The only columns without a set width are the two hidden sorting columns. --- src/connectdlg.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/connectdlg.cpp b/src/connectdlg.cpp index 2af67f43f8..2c942854d5 100644 --- a/src/connectdlg.cpp +++ b/src/connectdlg.cpp @@ -164,8 +164,7 @@ CConnectDlg::CConnectDlg ( CClient* pNCliP, CClientSettings* pNSetP, const bool cbxServerAddr->installEventFilter ( this ); lvwServers->installEventFilter ( this ); - // set up list view for connected clients (note that the last column size - // must not be specified since this column takes all the remaining space) + // set up list view for connected clients #ifdef ANDROID // for Android we need larger numbers because of the default font size lvwServers->setColumnWidth ( LVC_NAME, 200 ); From 2660f9a96e8836d9e6db5a4bd5cdfc990d6326e9 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 10/17] audiomixerboard.cpp: state what "int" buys for the in-use array A size_t arm of this array produces identical output on every probe: INVALID_INDEX is -1 and both sides of iFaderNumber[i] == INVALID_INDEX convert to SIZE_MAX, so the sentinel round-trips. What the signed type does buy is measured: the size_t arm draws -Wsign-compare at the comparison below and the shipped arm compiles that file without warnings. --- src/audiomixerboard.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/audiomixerboard.cpp b/src/audiomixerboard.cpp index bedd53526f..454aa9f0bf 100644 --- a/src/audiomixerboard.cpp +++ b/src/audiomixerboard.cpp @@ -1339,7 +1339,8 @@ void CAudioMixerBoard::ApplyNewConClientList ( CVector& vecChanInf // get all channels which are in use/not in use. // We use the array index of vecChanInfo if the fader is in use, // else INVALID_INDEX to specify it is not in use - // so must use "int" for the array type. + // so the array type is "int", which also keeps the comparisons below + // free of signedness warnings. int iFaderNumber[MAX_NUM_CHANNELS]; for ( size_t iChanID = 0; iChanID < MAX_NUM_CHANNELS; iChanID++ ) From d8bddaea9951f3fd9798a72a523f7b27e5b341c2 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 11/17] clientdlg.cpp: it is the delivery that must be queued, not this connection flag With and without the explicit Qt::QueuedConnection the behaviour is identical: emits land on QtConcurrent worker threads and every slot invocation runs on the main thread after the emit returns. CConnectDlg::EmitCLServerListPingMes is only ever reached through QtConcurrent::run (connectdlg.cpp:863/866), so Qt::AutoConnection queues the call regardless. The requirement the comment states is real; the flag is not what satisfies it. --- src/clientdlg.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index d939b54aa2..6eb0750511 100644 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -575,8 +575,8 @@ CClientDlg::CClientDlg ( CClient* pNCliP, QObject::connect ( &ConnectDlg, &CConnectDlg::ReqServerListQuery, this, &CClientDlg::OnReqServerListQuery ); - // note that this connection must be a queued connection, otherwise the server list ping - // times are not accurate and the client list may not be retrieved for all servers listed + // note that this delivery must be queued, otherwise the server list ping times are not + // accurate and the client list may not be retrieved for all servers listed // (it seems the sendto() function needs to be called from different threads to fire the // packet immediately and do not collect packets before transmitting) QObject::connect ( &ConnectDlg, &CConnectDlg::CreateCLServerListPingMes, this, &CClientDlg::OnCreateCLServerListPingMes, Qt::QueuedConnection ); From 5f05e77d92f097eb7ff26fe71e69a6208bfad8de Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 12/17] socket.h: the socket thread's priority is requested, not guaranteed QThread::TimeCriticalPriority is a request the platform can refuse. Measured from inside the thread with pthread_getschedparam plus getpriority, on an ordinary unprivileged process (RLIMIT_RTPRIO soft limit 0, the desktop-Linux default): the shipped thread runs SCHED_OTHER, rt_priority 0, nice 0 - identical to NormalPriority. The IdlePriority control shows Qt does apply the argument on this platform, downward, to SCHED_IDLE. On macOS 12.7.6 the same harness reads sched_priority 47 against a main thread at 31, so the request is granted there. QThread::start returns void, so a refusal is silent. Also fixes effect -> affect in the same block. --- src/socket.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/socket.h b/src/socket.h index 57f005b2bf..a7df73abd3 100644 --- a/src/socket.h +++ b/src/socket.h @@ -151,9 +151,9 @@ class CSocket : public QObject void ProtocolCLMessageReceived ( int iRecID, CVector vecbyMesBodyData, CHostAddress HostAdr ); }; -/* Socket which runs in a separate high priority thread --------------------- */ +/* Socket which runs in a separate thread requesting high priority ---------- */ // The receive socket should be put in a high priority thread to ensure the GUI -// does not effect the stability of the audio stream (e.g. if the GUI is on +// does not affect the stability of the audio stream (e.g. if the GUI is on // high load because of a table update, the incoming network packets must still // be put in the jitter buffer with highest priority). class CHighPrioSocket : public QObject @@ -240,7 +240,7 @@ class CHighPrioSocket : public QObject void Init() { - // Creation of the new socket thread which has to have the highest + // Creation of the new socket thread which requests the highest // possible thread priority to make sure the jitter buffer is reliably // filled with the network audio packets and does not get interrupted // by other GUI threads. The following code is based on: From de662faf6b73293f69d79ee9008c19178cb8a918 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 13/17] socket.cpp: name a reachable cause of a failed IPv4 socket socket ( AF_INET, SOCK_DGRAM, 0 ) returns -1 under file descriptor exhaustion (measured: RLIMIT_NOFILE=16, 13 sockets, then EMFILE), so the check is live and "should never happen" was wrong. Inside an empty network namespace - loopback only, no interfaces - the same call still succeeds, so "IPv4 not available" is not a state any tested configuration reaches. bDisableIPv4 is a hardcoded const false at socket.cpp:152, so the branch is unconditional. --- src/socket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/socket.cpp b/src/socket.cpp index e95ab4a41f..d400a1bee8 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -192,7 +192,7 @@ void CSocket::Init ( const quint16 iNewPortNumber, UdpSocket4 = socket ( AF_INET, SOCK_DGRAM, 0 ); if ( UdpSocket4 == INVALID_SOCKET ) { - // IPv4 requested but not available, throw error (should never happen, but check anyway) + // socket creation can fail (e.g. under file descriptor exhaustion), throw error throw CGenErr ( "IPv4 requested but not available on this system.", "Network Error" ); } From 264469db4c522836509318935fc5407d9e3c8f2a Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 14/17] client.cpp: only a GUI client measures the ping time this timer period uses iCurPingTime is written in exactly one place, OnCLPingReceived (client.cpp:440), reachable only through CClient::CreateCLPingMes(), whose only caller in the tree is CClientDlg::OnTimerPing (clientdlg.cpp:1146). Measured on the wire with a recording UDP proxy: a headless client emits zero CLM_PING frames at RTT 0 and at RTT 122 ms and keeps the 50 ms default period, while a GUI client built from identical sources spaces gain messages at 49, 122 and 244 ms - twice the ping. The headless consequence is issue #3874. --- src/client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.cpp b/src/client.cpp index d30413d04a..5f5f8723d7 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -482,7 +482,7 @@ void CClient::SetDoAutoSockBufSize ( const bool bValue ) // // When the first gain or pan change message is requested after an idle period (i.e. the timer is not // running), it will be sent immediately, and a timer started. The timer period is dependent on -// the current ping time to the remote server. +// the current ping time to the remote server, which only a GUI client measures (see #3874). // // If a gain or pan change message is requested while the timer is still running, the new value is not sent, // but just stored in newGain or newPan within clientChannels[iId], and the minGainOrPanId and maxGainOrPanId From 1d393b7eb8129dc1ca15bfa146b201ce130ea48c Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 15/17] channel.cpp: name the access model behind the 77-byte overhead The three terms - 28 (UDP+IP), 26 (PPP+PPPoE+MAC), 23 (RFC1483B+AAL+ATM) - are the PPPoE-over-ATM DSL access path of the cited paper, not a property of "the UDP packet which is transported via IP". A packet capture on a real internet path (~90k packets) measures 46 bytes over IPv4/Ethernet and 66 over IPv6, with no PPPoE, ATM or VLAN present. What the constant should be is a design question about which layer to bill; the comment now names the model it quotes and the code is unchanged. --- src/channel.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/channel.cpp b/src/channel.cpp index 7755b7ec92..49f7b80cce 100644 --- a/src/channel.cpp +++ b/src/channel.cpp @@ -716,9 +716,9 @@ int CChannel::GetUploadRateKbps() { const int iAudioSizeOut = iNetwFrameSizeFact * iAudioFrameSizeSamples; - // we assume that the UDP packet which is transported via IP has an - // additional header size of ("Network Music Performance (NMP) in narrow - // band networks; Carot, Kraemer, Schuller; 2006") + // we assume the PPPoE-over-ATM DSL access path described in ("Network Music + // Performance (NMP) in narrow band networks; Carot, Kraemer, Schuller; 2006"), + // whose additional header size is // 8 (UDP) + 20 (IP without optional fields) = 28 bytes // 2 (PPP) + 6 (PPPoE) + 18 (MAC) = 26 bytes // 5 (RFC1483B) + 8 (AAL) + 10 (ATM) = 23 bytes From cd30e5136eee2ed5ca9c7dc80e96c5501a95c12c Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 16/17] server.cpp: spelling, clitches -> glitches --- src/server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index ff8444089d..fd42f45803 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -165,7 +165,7 @@ CServer::CServer ( const int iNewMaxNumChan, iServerFrameSizeSamples = SYSTEM_FRAME_SIZE_SAMPLES; } - // To avoid audio clitches, in the entire realtime timer audio processing + // To avoid audio glitches, in the entire realtime timer audio processing // routine including the ProcessData no memory must be allocated. Since we // do not know the required sizes for the vectors, we allocate memory for // the worst case here: From 08f5f753584b813ce707e13f48611d72ccb62b46 Mon Sep 17 00:00:00 2001 From: jrd Date: Sat, 8 Aug 2026 22:10:35 +0000 Subject: [PATCH 17/17] protocol.h: spelling, sequred -> secured --- src/protocol.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocol.h b/src/protocol.h index 8d4125a9ab..e3959f4786 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -316,7 +316,7 @@ class CProtocol : public QObject int iOldRecID; int iOldRecCnt; - // these two objects must be sequred by a mutex + // these two objects must be secured by a mutex uint8_t iCounter; std::list SendMessQueue;