From 9835897343fe91b0053936962fffbd2f2412aa0d Mon Sep 17 00:00:00 2001
From: Christian Hoffmann <mail@hoffmann-christian.info>
Date: Sun, 17 Jul 2022 13:35:23 +0200
Subject: [PATCH] Client: Avoid losing gain changes

PR #2535 introduced rate limiting for gain change messages. The logic
required storing the previously used gain value per channel. This logic
had some flaws:
1. The previously used gain value defaulted to 0, despite the server-side
   view of the channel being set to 1 (as the default). Therefore,
   gain(0) changes during a series of gain changes would be lost. The
   most common scenario would be the initial connection, which always
   triggers the rate limit and therefore the faulty logic. This also
   affected New Client Level = 0.
2. The previously used gain values were not reset upon changing servers.
   This might have caused losing arbitrary gain change messages, e.g.
   stored fader values.
3. The previously used gain values were not reset upon a channel
   disconnect. This might have caused missing fader level restores.

This commit introduces a gain level memory reset to 1 (100%) on connect
as well as on channel disconnects to fix these issues.

Fixes: #2730

Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>
---
 src/client.cpp | 38 ++++++++++++++++++++++++++++++++++++++
 src/client.h   |  1 +
 2 files changed, 39 insertions(+)

diff --git a/src/client.cpp b/src/client.cpp
index 99f74caf10..ad9beb624c 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -118,6 +118,8 @@ CClient::CClient ( const quint16  iPortNumber,
 
     QObject::connect ( &Channel, &CChannel::ReqChanInfo, this, &CClient::OnReqChanInfo );
 
+    // The first ConClientListMesReceived handler performs the necessary cleanup and has to run first:
+    QObject::connect ( &Channel, &CChannel::ConClientListMesReceived, this, &CClient::OnConClientListMesReceived );
     QObject::connect ( &Channel, &CChannel::ConClientListMesReceived, this, &CClient::ConClientListMesReceived );
 
     QObject::connect ( &Channel, &CChannel::Disconnected, this, &CClient::Disconnected );
@@ -261,6 +263,15 @@ void CClient::OnJittBufSizeChanged ( int iNewJitBufSize )
 
 void CClient::OnNewConnection()
 {
+    // The oldGain and newGain arrays are used to avoid sending duplicate gain change messages.
+    // As these values depend on the channel IDs of a specific server, we have
+    // to reset those upon connect.
+    // We reset to 1 because this is what the server part sets by default.
+    for ( int iId = 0; iId < MAX_NUM_CHANNELS; iId++ )
+    {
+        oldGain[iId] = newGain[iId] = 1;
+    }
+
     // a new connection was successfully initiated, send infos and request
     // connected clients list
     Channel.SetRemoteInfo ( ChannelInfo );
@@ -280,6 +291,33 @@ void CClient::OnNewConnection()
     //### TODO: END ###//
 }
 
+void CClient::OnConClientListMesReceived ( CVector<CChannelInfo> vecChanInfo )
+{
+    // Upon receiving a new client list, we have to reset oldGain and newGain
+    // entries for unused channels. This ensures that a disconnected channel
+    // does not leave behind wrong cached gain values which would leak into
+    // any new channel which reused this channel id.
+    int iNumConnectedClients = vecChanInfo.Size();
+
+    // Save what channel IDs are in use:
+    bool bChanIdInUse[MAX_NUM_CHANNELS] = {};
+    for ( int i = 0; i < iNumConnectedClients; i++ )
+    {
+        bChanIdInUse[vecChanInfo[i].iChanID] = true;
+    }
+
+    // Reset all gains for unused channel IDs:
+    for ( int iId = 0; iId < MAX_NUM_CHANNELS; iId++ )
+    {
+        if ( !bChanIdInUse[iId] )
+        {
+            // reset oldGain and newGain as this channel id is currently unused and will
+            // start with a server-side gain at 1 (100%) again.
+            oldGain[iId] = newGain[iId] = 1;
+        }
+    }
+}
+
 void CClient::CreateServerJitterBufferMessage()
 {
     // per definition in the client: if auto jitter buffer is enabled, both,
diff --git a/src/client.h b/src/client.h
index 1a5d590c75..f7553446a5 100644
--- a/src/client.h
+++ b/src/client.h
@@ -403,6 +403,7 @@ protected slots:
     void OnControllerInFaderIsMute ( int iChannelIdx, bool bIsMute );
     void OnControllerInMuteMyself ( bool bMute );
     void OnClientIDReceived ( int iChanID );
+    void OnConClientListMesReceived ( CVector<CChannelInfo> vecChanInfo );
 
 signals:
     void ConClientListMesReceived ( CVector<CChannelInfo> vecChanInfo );