From 40f27657de9dbe8de584a9bbad10d9203256b749 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 17:28:46 +0100
Subject: [PATCH 1/9] REFAC(client,audio): Replace explicit locking with lock
 guards

---
 src/mumble/AudioOutput.cpp | 642 +++++++++++++++++++------------------
 1 file changed, 332 insertions(+), 310 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index cd0f9b9aec5..5415247dc04 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -181,17 +181,28 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 		return;
 	}
 
-	qrwlOutputs.lockForRead();
-	// qmOutputs is a map of users and their AudioOutputSpeech objects, which will be created when audio from that user
-	// is received. It also contains AudioOutputSample objects with various other non-speech sounds.
-	// This map will be iterated in mix(). After the speech or sample audio is finished, the AudioOutputBuffer object
-	// will be removed from this map and deleted.
-	AudioOutputSpeech *speech = qobject_cast< AudioOutputSpeech * >(qmOutputs.value(sender));
+	AudioOutputSpeech *speech = nullptr;
+	bool createNew            = false;
+	{
+		QReadLocker lock(&qrwlOutputs);
+
+		// qmOutputs is a map of users and their AudioOutputSpeech objects, which will be created when audio from that
+		// user is received. It also contains AudioOutputSample objects with various other non-speech sounds. This map
+		// will be iterated in mix(). After the speech or sample audio is finished, the AudioOutputBuffer object will be
+		// removed from this map and deleted.
+		speech = qobject_cast< AudioOutputSpeech * >(qmOutputs.value(sender));
+
+		createNew = !speech || (speech->m_codec != audioData.usedCodec);
 
-	if (!speech || (speech->m_codec != audioData.usedCodec)) {
-		qrwlOutputs.unlock();
+		if (!createNew) {
+			speech->addFrameToBuffer(audioData);
+		}
+	}
 
+	if (createNew) {
 		if (speech) {
+			// removeBuffer doesn't dereference speech, so passing the pointer around without
+			// holding the lock is fine.
 			removeBuffer(static_cast< AudioOutputBuffer * >(speech));
 		}
 
@@ -203,15 +214,13 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 			return;
 		}
 
-		qrwlOutputs.lockForWrite();
+		QWriteLocker lock(&qrwlOutputs);
 
 		speech = new AudioOutputSpeech(sender, iMixerFreq, audioData.usedCodec, iBufferSize);
 		qmOutputs.replace(sender, speech);
-	}
 
-	speech->addFrameToBuffer(audioData);
-
-	qrwlOutputs.unlock();
+		speech->addFrameToBuffer(audioData);
+	}
 }
 
 void AudioOutput::handleInvalidatedBuffer(AudioOutputBuffer *buffer) {
@@ -426,366 +435,379 @@ bool AudioOutput::mix(void *outbuff, unsigned int frameCount) {
 	positions.clear();
 #endif
 
-	// A list of buffers that have audio to contribute
-	QList< AudioOutputBuffer * > qlMix;
-	// A list of buffers that no longer have any audio to play and can thus be deleted
-	QList< AudioOutputBuffer * > qlDel;
-
 	if (Global::get().s.fVolume < 0.01f) {
 		return false;
 	}
 
-	const float adjustFactor = std::pow(10.f, -18.f / 20);
-	const float mul          = Global::get().s.fVolume;
-	const unsigned int nchan = iChannels;
-	ServerHandlerPtr sh      = Global::get().sh;
-	VoiceRecorderPtr recorder;
-	if (sh) {
-		recorder = Global::get().sh->recorder;
-	}
+	// A list of buffers that no longer have any audio to play and can thus be deleted
+	QList< AudioOutputBuffer * > qlDel;
 
-	qrwlOutputs.lockForRead();
+	bool haveAudio = false;
 
-	bool prioritySpeakerActive = false;
+	{
+		QReadLocker lock(&qrwlOutputs);
+
+		// A list of buffers that have audio to contribute
+		QList< AudioOutputBuffer * > qlMix;
+
+		const float adjustFactor = std::pow(10.f, -18.f / 20);
+		const float mul          = Global::get().s.fVolume;
+		const unsigned int nchan = iChannels;
+		ServerHandlerPtr sh      = Global::get().sh;
+		VoiceRecorderPtr recorder;
+		if (sh) {
+			recorder = Global::get().sh->recorder;
+		}
 
-	// Get the users that are currently talking (and are thus serving as an audio source)
-	QMultiHash< const ClientUser *, AudioOutputBuffer * >::const_iterator it = qmOutputs.constBegin();
-	while (it != qmOutputs.constEnd()) {
-		AudioOutputBuffer *buffer = it.value();
-		if (!buffer->prepareSampleBuffer(frameCount)) {
-			qlDel.append(buffer);
-		} else {
-			qlMix.append(buffer);
+		bool prioritySpeakerActive = false;
 
-			const ClientUser *user = it.key();
-			if (user && user->bPrioritySpeaker) {
-				prioritySpeakerActive = true;
+		// Get the users that are currently talking (and are thus serving as an audio source)
+		QMultiHash< const ClientUser *, AudioOutputBuffer * >::const_iterator it = qmOutputs.constBegin();
+		while (it != qmOutputs.constEnd()) {
+			AudioOutputBuffer *buffer = it.value();
+			if (!buffer->prepareSampleBuffer(frameCount)) {
+				qlDel.append(buffer);
+			} else {
+				qlMix.append(buffer);
+
+				const ClientUser *user = it.key();
+				if (user && user->bPrioritySpeaker) {
+					prioritySpeakerActive = true;
+				}
 			}
+			++it;
 		}
-		++it;
-	}
 
-	if (Global::get().prioritySpeakerActiveOverride) {
-		prioritySpeakerActive = true;
-	}
+		haveAudio = !qlMix.empty();
 
-	// If the audio backend uses a float-array we can sample and mix the audio sources directly into the output.
-	// Otherwise we'll have to use an intermediate buffer which we will convert to an array of shorts later
-	static std::vector< float > fOutput;
-	fOutput.resize(iChannels * frameCount);
-	float *output = (eSampleFormat == SampleFloat) ? reinterpret_cast< float * >(outbuff) : fOutput.data();
-	memset(output, 0, sizeof(float) * frameCount * iChannels);
-
-	if (!qlMix.isEmpty()) {
-		// There are audio sources available -> mix those sources together and feed them into the audio backend
-		static std::vector< float > speaker;
-		speaker.resize(iChannels * 3);
-		static std::vector< float > svol;
-		svol.resize(iChannels);
-
-		bool validListener = false;
-
-		// Initialize recorder if recording is enabled
-		boost::shared_array< float > recbuff;
-		if (recorder) {
-			recbuff = boost::shared_array< float >(new float[frameCount]);
-			memset(recbuff.get(), 0, sizeof(float) * frameCount);
-			recorder->prepareBufferAdds();
+		if (Global::get().prioritySpeakerActiveOverride) {
+			prioritySpeakerActive = true;
 		}
 
-		for (unsigned int i = 0; i < iChannels; ++i)
-			svol[i] = mul * fSpeakerVolume[i];
+		// If the audio backend uses a float-array we can sample and mix the audio sources directly into the output.
+		// Otherwise we'll have to use an intermediate buffer which we will convert to an array of shorts later
+		static std::vector< float > fOutput;
+		fOutput.resize(iChannels * frameCount);
+		float *output = (eSampleFormat == SampleFloat) ? reinterpret_cast< float * >(outbuff) : fOutput.data();
+		memset(output, 0, sizeof(float) * frameCount * iChannels);
 
-		if (Global::get().s.bPositionalAudio && (iChannels > 1) && Global::get().pluginManager->fetchPositionalData()) {
-			// Calculate the positional audio effects if it is enabled
+		if (haveAudio) {
+			// There are audio sources available -> mix those sources together and feed them into the audio backend
+			static std::vector< float > speaker;
+			speaker.resize(iChannels * 3);
+			static std::vector< float > svol;
+			svol.resize(iChannels);
 
-			Vector3D cameraDir = Global::get().pluginManager->getPositionalData().getCameraDir();
+			bool validListener = false;
 
-			Vector3D cameraAxis = Global::get().pluginManager->getPositionalData().getCameraAxis();
+			// Initialize recorder if recording is enabled
+			boost::shared_array< float > recbuff;
+			if (recorder) {
+				recbuff = boost::shared_array< float >(new float[frameCount]);
+				memset(recbuff.get(), 0, sizeof(float) * frameCount);
+				recorder->prepareBufferAdds();
+			}
 
-			// Direction vector is dominant; if it's zero we presume all is zero.
+			for (unsigned int i = 0; i < iChannels; ++i)
+				svol[i] = mul * fSpeakerVolume[i];
 
-			if (!cameraDir.isZero()) {
-				cameraDir.normalize();
+			if (Global::get().s.bPositionalAudio && (iChannels > 1)
+				&& Global::get().pluginManager->fetchPositionalData()) {
+				// Calculate the positional audio effects if it is enabled
 
-				if (!cameraAxis.isZero()) {
-					cameraAxis.normalize();
-				} else {
-					cameraAxis = { 0.0f, 1.0f, 0.0f };
-				}
+				Vector3D cameraDir = Global::get().pluginManager->getPositionalData().getCameraDir();
+
+				Vector3D cameraAxis = Global::get().pluginManager->getPositionalData().getCameraAxis();
 
-				const float dotproduct = cameraDir.dotProduct(cameraAxis);
-				const float error      = std::abs(dotproduct);
-				if (error > 0.5f) {
-					// Not perpendicular by a large margin. Assume Y up and rotate 90 degrees.
+				// Direction vector is dominant; if it's zero we presume all is zero.
 
-					float azimuth = 0.0f;
-					if (cameraDir.x != 0.0f || cameraDir.z != 0.0f) {
-						azimuth = atan2f(cameraDir.z, cameraDir.x);
+				if (!cameraDir.isZero()) {
+					cameraDir.normalize();
+
+					if (!cameraAxis.isZero()) {
+						cameraAxis.normalize();
+					} else {
+						cameraAxis = { 0.0f, 1.0f, 0.0f };
 					}
 
-					float inclination = acosf(cameraDir.y) - static_cast< float >(M_PI) / 2.0f;
+					const float dotproduct = cameraDir.dotProduct(cameraAxis);
+					const float error      = std::abs(dotproduct);
+					if (error > 0.5f) {
+						// Not perpendicular by a large margin. Assume Y up and rotate 90 degrees.
 
-					cameraAxis.x = sinf(inclination) * cosf(azimuth);
-					cameraAxis.y = cosf(inclination);
-					cameraAxis.z = sinf(inclination) * sinf(azimuth);
-				} else if (error > 0.01f) {
-					// Not perpendicular by a small margin. Find the nearest perpendicular vector.
-					cameraAxis = cameraAxis - cameraDir * dotproduct;
+						float azimuth = 0.0f;
+						if (cameraDir.x != 0.0f || cameraDir.z != 0.0f) {
+							azimuth = atan2f(cameraDir.z, cameraDir.x);
+						}
 
-					// normalize axis again (the orthogonalized vector us guaranteed to be non-zero
-					// as the error (dotproduct) was only 0.5 (and not 1 in which case above operation
-					// would create the zero-vector).
-					cameraAxis.normalize();
-				}
-			} else {
-				cameraDir = { 0.0f, 0.0f, 1.0f };
+						float inclination = acosf(cameraDir.y) - static_cast< float >(M_PI) / 2.0f;
 
-				cameraAxis = { 0.0f, 1.0f, 0.0f };
-			}
+						cameraAxis.x = sinf(inclination) * cosf(azimuth);
+						cameraAxis.y = cosf(inclination);
+						cameraAxis.z = sinf(inclination) * sinf(azimuth);
+					} else if (error > 0.01f) {
+						// Not perpendicular by a small margin. Find the nearest perpendicular vector.
+						cameraAxis = cameraAxis - cameraDir * dotproduct;
 
-			// Calculate right vector as front X top
-			Vector3D right = cameraAxis.crossProduct(cameraDir);
-
-			/*
-						qWarning("Front: %f %f %f", front[0], front[1], front[2]);
-						qWarning("Top: %f %f %f", top[0], top[1], top[2]);
-						qWarning("Right: %f %f %f", right[0], right[1], right[2]);
-			*/
-			// Rotate speakers to match orientation
-			for (unsigned int i = 0; i < iChannels; ++i) {
-				speaker[3 * i + 0] = fSpeakers[3 * i + 0] * right.x + fSpeakers[3 * i + 1] * cameraAxis.x
-									 + fSpeakers[3 * i + 2] * cameraDir.x;
-				speaker[3 * i + 1] = fSpeakers[3 * i + 0] * right.y + fSpeakers[3 * i + 1] * cameraAxis.y
-									 + fSpeakers[3 * i + 2] * cameraDir.y;
-				speaker[3 * i + 2] = fSpeakers[3 * i + 0] * right.z + fSpeakers[3 * i + 1] * cameraAxis.z
-									 + fSpeakers[3 * i + 2] * cameraDir.z;
-			}
-			validListener = true;
-		}
+						// normalize axis again (the orthogonalized vector us guaranteed to be non-zero
+						// as the error (dotproduct) was only 0.5 (and not 1 in which case above operation
+						// would create the zero-vector).
+						cameraAxis.normalize();
+					}
+				} else {
+					cameraDir = { 0.0f, 0.0f, 1.0f };
 
-		for (AudioOutputBuffer *buffer : qlMix) {
-			// Iterate through all audio sources and mix them together into the output (or the intermediate array)
-			float *RESTRICT pfBuffer = buffer->pfBuffer;
-			float volumeAdjustment   = 1;
-
-			// Check if the audio source is a user speaking or a sample playback and apply potential volume
-			// adjustments
-			AudioOutputSpeech *speech = qobject_cast< AudioOutputSpeech * >(buffer);
-			AudioOutputSample *sample = qobject_cast< AudioOutputSample * >(buffer);
-			const ClientUser *user    = nullptr;
-			if (speech) {
-				user = speech->p;
-
-				volumeAdjustment *= user->getLocalVolumeAdjustments();
-
-				if (sh && sh->m_version >= Mumble::Protocol::PROTOBUF_INTRODUCTION_VERSION) {
-					// The new protocol supports sending volume adjustments which is used to figure out the correct
-					// volume adjustment for listeners on the server. Thus, we only have to apply that here.
-					volumeAdjustment *= speech->m_suggestedVolumeAdjustment;
-				} else if (user->cChannel
-						   && Global::get().channelListenerManager->isListening(Global::get().uiSession,
-																				user->cChannel->iId)
-						   && (speech->m_audioContext == Mumble::Protocol::AudioContext::LISTEN)) {
-					// We are receiving this audio packet only because we are listening to the channel
-					// the speaking user is in. Thus we receive the audio via our "listener proxy".
-					// Thus we'll apply the volume adjustment for our listener proxy as well
-					volumeAdjustment *= Global::get()
-											.channelListenerManager
-											->getListenerVolumeAdjustment(Global::get().uiSession, user->cChannel->iId)
-											.factor;
+					cameraAxis = { 0.0f, 1.0f, 0.0f };
 				}
 
-				if (prioritySpeakerActive) {
-					if (user->tsState != Settings::Whispering && !user->bPrioritySpeaker) {
-						volumeAdjustment *= adjustFactor;
-					}
+				// Calculate right vector as front X top
+				Vector3D right = cameraAxis.crossProduct(cameraDir);
+
+				/*
+							qWarning("Front: %f %f %f", front[0], front[1], front[2]);
+							qWarning("Top: %f %f %f", top[0], top[1], top[2]);
+							qWarning("Right: %f %f %f", right[0], right[1], right[2]);
+				*/
+				// Rotate speakers to match orientation
+				for (unsigned int i = 0; i < iChannels; ++i) {
+					speaker[3 * i + 0] = fSpeakers[3 * i + 0] * right.x + fSpeakers[3 * i + 1] * cameraAxis.x
+										 + fSpeakers[3 * i + 2] * cameraDir.x;
+					speaker[3 * i + 1] = fSpeakers[3 * i + 0] * right.y + fSpeakers[3 * i + 1] * cameraAxis.y
+										 + fSpeakers[3 * i + 2] * cameraDir.y;
+					speaker[3 * i + 2] = fSpeakers[3 * i + 0] * right.z + fSpeakers[3 * i + 1] * cameraAxis.z
+										 + fSpeakers[3 * i + 2] * cameraDir.z;
 				}
-			} else if (sample) {
-				volumeAdjustment *= sample->getVolume();
+				validListener = true;
 			}
 
-			// As the events may cause the output PCM to change, the connection has to be direct in any case
-			const int channels = (speech && speech->bStereo) ? 2 : 1;
-			// If user != nullptr, then the current audio is considered speech
-			assert(channels >= 0);
-			emit audioSourceFetched(pfBuffer, frameCount, static_cast< unsigned int >(channels), SAMPLE_RATE,
-									static_cast< bool >(user), user);
+			for (AudioOutputBuffer *buffer : qlMix) {
+				// Iterate through all audio sources and mix them together into the output (or the intermediate array)
+				float *RESTRICT pfBuffer = buffer->pfBuffer;
+				float volumeAdjustment   = 1;
 
-			// If recording is enabled add the current audio source to the recording buffer
-			if (recorder) {
+				// Check if the audio source is a user speaking or a sample playback and apply potential volume
+				// adjustments
+				AudioOutputSpeech *speech = qobject_cast< AudioOutputSpeech * >(buffer);
+				AudioOutputSample *sample = qobject_cast< AudioOutputSample * >(buffer);
+				const ClientUser *user    = nullptr;
 				if (speech) {
-					if (speech->bStereo) {
-						// Mix down stereo to mono. TODO: stereo record support
-						// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
-						for (std::size_t i = 0; i < frameCount; ++i) {
-							recbuff.get()[i] +=
-								(pfBuffer[2 * i] / 2.0f + pfBuffer[2 * i + 1] / 2.0f) * volumeAdjustment;
-						}
-					} else {
-						for (std::size_t i = 0; i < frameCount; ++i) {
-							recbuff.get()[i] += pfBuffer[i] * volumeAdjustment;
-						}
+					user = speech->p;
+
+					volumeAdjustment *= user->getLocalVolumeAdjustments();
+
+					if (sh && sh->m_version >= Mumble::Protocol::PROTOBUF_INTRODUCTION_VERSION) {
+						// The new protocol supports sending volume adjustments which is used to figure out the correct
+						// volume adjustment for listeners on the server. Thus, we only have to apply that here.
+						volumeAdjustment *= speech->m_suggestedVolumeAdjustment;
+					} else if (user->cChannel
+							   && Global::get().channelListenerManager->isListening(Global::get().uiSession,
+																					user->cChannel->iId)
+							   && (speech->m_audioContext == Mumble::Protocol::AudioContext::LISTEN)) {
+						// We are receiving this audio packet only because we are listening to the channel
+						// the speaking user is in. Thus we receive the audio via our "listener proxy".
+						// Thus we'll apply the volume adjustment for our listener proxy as well
+						volumeAdjustment *=
+							Global::get()
+								.channelListenerManager
+								->getListenerVolumeAdjustment(Global::get().uiSession, user->cChannel->iId)
+								.factor;
 					}
 
-					if (!recorder->isInMixDownMode()) {
-						recorder->addBuffer(speech->p, recbuff, static_cast< int >(frameCount));
-						recbuff = boost::shared_array< float >(new float[frameCount]);
-						memset(recbuff.get(), 0, sizeof(float) * frameCount);
+					if (prioritySpeakerActive) {
+						if (user->tsState != Settings::Whispering && !user->bPrioritySpeaker) {
+							volumeAdjustment *= adjustFactor;
+						}
 					}
+				} else if (sample) {
+					volumeAdjustment *= sample->getVolume();
+				}
+
+				// As the events may cause the output PCM to change, the connection has to be direct in any case
+				const int channels = (speech && speech->bStereo) ? 2 : 1;
+				// If user != nullptr, then the current audio is considered speech
+				assert(channels >= 0);
+				emit audioSourceFetched(pfBuffer, frameCount, static_cast< unsigned int >(channels), SAMPLE_RATE,
+										static_cast< bool >(user), user);
+
+				// If recording is enabled add the current audio source to the recording buffer
+				if (recorder) {
+					if (speech) {
+						if (speech->bStereo) {
+							// Mix down stereo to mono. TODO: stereo record support
+							// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
+							for (std::size_t i = 0; i < frameCount; ++i) {
+								recbuff.get()[i] +=
+									(pfBuffer[2 * i] / 2.0f + pfBuffer[2 * i + 1] / 2.0f) * volumeAdjustment;
+							}
+						} else {
+							for (std::size_t i = 0; i < frameCount; ++i) {
+								recbuff.get()[i] += pfBuffer[i] * volumeAdjustment;
+							}
+						}
+
+						if (!recorder->isInMixDownMode()) {
+							recorder->addBuffer(speech->p, recbuff, static_cast< int >(frameCount));
+							recbuff = boost::shared_array< float >(new float[frameCount]);
+							memset(recbuff.get(), 0, sizeof(float) * frameCount);
+						}
 
-					// Don't add the local audio to the real output
-					if (qobject_cast< RecordUser * >(speech->p)) {
-						continue;
+						// Don't add the local audio to the real output
+						if (qobject_cast< RecordUser * >(speech->p)) {
+							continue;
+						}
 					}
 				}
-			}
 
-			if (validListener
-				&& ((buffer->fPos[0] != 0.0f) || (buffer->fPos[1] != 0.0f) || (buffer->fPos[2] != 0.0f))) {
-				// Add position to position map
+				if (validListener
+					&& ((buffer->fPos[0] != 0.0f) || (buffer->fPos[1] != 0.0f) || (buffer->fPos[2] != 0.0f))) {
+					// Add position to position map
 #ifdef USE_MANUAL_PLUGIN
-				if (user) {
-					// The coordinates in the plane are actually given by x and z instead of x and y (y is up)
-					positions.insert(user->uiSession, { buffer->fPos[0], buffer->fPos[2] });
-				}
+					if (user) {
+						// The coordinates in the plane are actually given by x and z instead of x and y (y is up)
+						positions.insert(user->uiSession, { buffer->fPos[0], buffer->fPos[2] });
+					}
 #endif
 
-				// If positional audio is enabled, calculate the respective audio effect here
-				Position3D outputPos = { buffer->fPos[0], buffer->fPos[1], buffer->fPos[2] };
-				Position3D ownPos    = Global::get().pluginManager->getPositionalData().getCameraPos();
+					// If positional audio is enabled, calculate the respective audio effect here
+					Position3D outputPos = { buffer->fPos[0], buffer->fPos[1], buffer->fPos[2] };
+					Position3D ownPos    = Global::get().pluginManager->getPositionalData().getCameraPos();
 
-				Vector3D connectionVec = outputPos - ownPos;
-				float len              = connectionVec.norm();
+					Vector3D connectionVec = outputPos - ownPos;
+					float len              = connectionVec.norm();
 
-				if (len > 0.0f) {
-					// Don't use normalize-func in order to save the re-computation of the vector's length
-					connectionVec.x /= len;
-					connectionVec.y /= len;
-					connectionVec.z /= len;
-				}
-				/*
-								qWarning("Voice pos: %f %f %f", aop->fPos[0], aop->fPos[1], aop->fPos[2]);
-								qWarning("Voice dir: %f %f %f", connectionVec.x, connectionVec.y, connectionVec.z);
-				*/
-				if (!buffer->pfVolume) {
-					buffer->pfVolume = new float[nchan];
-					for (unsigned int s = 0; s < nchan; ++s)
-						buffer->pfVolume[s] = -1.0;
-				}
-
-				if (!buffer->piOffset) {
-					buffer->piOffset = std::make_unique< unsigned int[] >(nchan);
-					for (unsigned int s = 0; s < nchan; ++s) {
-						buffer->piOffset[s] = 0;
+					if (len > 0.0f) {
+						// Don't use normalize-func in order to save the re-computation of the vector's length
+						connectionVec.x /= len;
+						connectionVec.y /= len;
+						connectionVec.z /= len;
+					}
+					/*
+									qWarning("Voice pos: %f %f %f", aop->fPos[0], aop->fPos[1], aop->fPos[2]);
+									qWarning("Voice dir: %f %f %f", connectionVec.x, connectionVec.y, connectionVec.z);
+					*/
+					if (!buffer->pfVolume) {
+						buffer->pfVolume = new float[nchan];
+						for (unsigned int s = 0; s < nchan; ++s)
+							buffer->pfVolume[s] = -1.0;
 					}
-				}
 
-				const bool isAudible =
-					(Global::get().s.fAudioMaxDistVolume > 0) || (len < Global::get().s.fAudioMaxDistance);
-
-				for (unsigned int s = 0; s < nchan; ++s) {
-					const float dot = bSpeakerPositional[s]
-										  ? connectionVec.x * speaker[s * 3 + 0] + connectionVec.y * speaker[s * 3 + 1]
-												+ connectionVec.z * speaker[s * 3 + 2]
-										  : 1.0f;
-					float channelVol;
-					if (isAudible) {
-						// In the current contex, we know that sound reaches at least one ear.
-						channelVol = svol[s] * calcGain(dot, len) * volumeAdjustment;
-					} else {
-						// The user has set the minimum positional volume to 0 and this sound source
-						// is exceeding the positional volume range. This means that the sound is completely
-						// inaudible at the current position. We therefore set the volume the to 0,
-						// making sure the user really can not hear any audio from that source.
-						channelVol = 0;
+					if (!buffer->piOffset) {
+						buffer->piOffset = std::make_unique< unsigned int[] >(nchan);
+						for (unsigned int s = 0; s < nchan; ++s) {
+							buffer->piOffset[s] = 0;
+						}
 					}
 
-					float *RESTRICT o   = output + s;
-					const float old     = (buffer->pfVolume[s] >= 0.0f) ? buffer->pfVolume[s] : channelVol;
-					const float inc     = (channelVol - old) / static_cast< float >(frameCount);
-					buffer->pfVolume[s] = channelVol;
-
-					// Calculates the ITD offset of the audio data this frame.
-					// Interaural Time Delay (ITD) is a small time delay between your ears
-					// depending on the sound source position on the horizontal plane and the
-					// distance between your ears.
-					//
-					// Offset for ITD is not applied directly, but rather the offset is interpolated
-					// linearly across the entire chunk, between the offset of the last chunk and the
-					// newly calculated offset for this chunk. This prevents clicking / buzzing when the
-					// audio source or camera is moving, because abruptly changing offsets (and thus
-					// abruptly changing the playback position) will create a clicking noise.
-					// Normalize dot to range [0,1] instead [-1,1]
-					const int offset      = static_cast< int >(INTERAURAL_DELAY * (1.0f + dot) / 2.0f);
-					const int oldOffset   = static_cast< int >(buffer->piOffset[s]);
-					const float incOffset = static_cast< float >(offset - oldOffset) / static_cast< float >(frameCount);
-					buffer->piOffset[s]   = static_cast< unsigned int >(offset);
-					/*
-										qWarning("%d: Pos %f %f %f : Dot %f Len %f ChannelVol %f", s, speaker[s*3+0],
-					   speaker[s*3+1], speaker[s*3+2], dot, len, channelVol);
-					*/
-					if ((old >= 0.00000001f) || (channelVol >= 0.00000001f)) {
-						for (unsigned int i = 0; i < frameCount; ++i) {
-							unsigned int currentOffset = static_cast< unsigned int >(
-								static_cast< float >(oldOffset) + incOffset * static_cast< float >(i));
-							if (speech && speech->bStereo) {
-								// Mix stereo user's stream into mono
-								// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
-								o[i * nchan] += (pfBuffer[2 * i + currentOffset] / 2.0f
-												 + pfBuffer[2 * i + currentOffset + 1] / 2.0f)
-												* (old + inc * static_cast< float >(i));
-							} else {
-								o[i * nchan] += pfBuffer[i + currentOffset] * (old + inc * static_cast< float >(i));
+					const bool isAudible =
+						(Global::get().s.fAudioMaxDistVolume > 0) || (len < Global::get().s.fAudioMaxDistance);
+
+					for (unsigned int s = 0; s < nchan; ++s) {
+						const float dot = bSpeakerPositional[s] ? connectionVec.x * speaker[s * 3 + 0]
+																	  + connectionVec.y * speaker[s * 3 + 1]
+																	  + connectionVec.z * speaker[s * 3 + 2]
+																: 1.0f;
+						float channelVol;
+						if (isAudible) {
+							// In the current contex, we know that sound reaches at least one ear.
+							channelVol = svol[s] * calcGain(dot, len) * volumeAdjustment;
+						} else {
+							// The user has set the minimum positional volume to 0 and this sound source
+							// is exceeding the positional volume range. This means that the sound is completely
+							// inaudible at the current position. We therefore set the volume the to 0,
+							// making sure the user really can not hear any audio from that source.
+							channelVol = 0;
+						}
+
+						float *RESTRICT o   = output + s;
+						const float old     = (buffer->pfVolume[s] >= 0.0f) ? buffer->pfVolume[s] : channelVol;
+						const float inc     = (channelVol - old) / static_cast< float >(frameCount);
+						buffer->pfVolume[s] = channelVol;
+
+						// Calculates the ITD offset of the audio data this frame.
+						// Interaural Time Delay (ITD) is a small time delay between your ears
+						// depending on the sound source position on the horizontal plane and the
+						// distance between your ears.
+						//
+						// Offset for ITD is not applied directly, but rather the offset is interpolated
+						// linearly across the entire chunk, between the offset of the last chunk and the
+						// newly calculated offset for this chunk. This prevents clicking / buzzing when the
+						// audio source or camera is moving, because abruptly changing offsets (and thus
+						// abruptly changing the playback position) will create a clicking noise.
+						// Normalize dot to range [0,1] instead [-1,1]
+						const int offset    = static_cast< int >(INTERAURAL_DELAY * (1.0f + dot) / 2.0f);
+						const int oldOffset = static_cast< int >(buffer->piOffset[s]);
+						const float incOffset =
+							static_cast< float >(offset - oldOffset) / static_cast< float >(frameCount);
+						buffer->piOffset[s] = static_cast< unsigned int >(offset);
+						/*
+											qWarning("%d: Pos %f %f %f : Dot %f Len %f ChannelVol %f", s,
+						   speaker[s*3+0], speaker[s*3+1], speaker[s*3+2], dot, len, channelVol);
+						*/
+						if ((old >= 0.00000001f) || (channelVol >= 0.00000001f)) {
+							for (unsigned int i = 0; i < frameCount; ++i) {
+								unsigned int currentOffset = static_cast< unsigned int >(
+									static_cast< float >(oldOffset) + incOffset * static_cast< float >(i));
+								if (speech && speech->bStereo) {
+									// Mix stereo user's stream into mono
+									// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
+									o[i * nchan] += (pfBuffer[2 * i + currentOffset] / 2.0f
+													 + pfBuffer[2 * i + currentOffset + 1] / 2.0f)
+													* (old + inc * static_cast< float >(i));
+								} else {
+									o[i * nchan] += pfBuffer[i + currentOffset] * (old + inc * static_cast< float >(i));
+								}
 							}
 						}
 					}
-				}
-			} else {
-				// Mix the current audio source into the output by adding it to the elements of the output buffer after
-				// having applied a volume adjustment
-				for (unsigned int s = 0; s < nchan; ++s) {
-					const float channelVol = svol[s] * volumeAdjustment;
-					float *RESTRICT o      = output + s;
-					if (buffer->bStereo) {
-						// Linear-panning stereo stream according to the projection of fSpeaker vector on left-right
-						// direction.
-						// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
-						for (unsigned int i = 0; i < frameCount; ++i)
-							o[i * nchan] += (pfBuffer[2 * i] * fStereoPanningFactor[2 * s + 0]
-											 + pfBuffer[2 * i + 1] * fStereoPanningFactor[2 * s + 1])
-											* channelVol;
-					} else {
-						for (unsigned int i = 0; i < frameCount; ++i)
-							o[i * nchan] += pfBuffer[i] * channelVol;
+				} else {
+					// Mix the current audio source into the output by adding it to the elements of the output buffer
+					// after having applied a volume adjustment
+					for (unsigned int s = 0; s < nchan; ++s) {
+						const float channelVol = svol[s] * volumeAdjustment;
+						float *RESTRICT o      = output + s;
+						if (buffer->bStereo) {
+							// Linear-panning stereo stream according to the projection of fSpeaker vector on left-right
+							// direction.
+							// frame: for a stereo stream, the [LR] pair inside ...[LR]LRLRLR.... is a frame
+							for (unsigned int i = 0; i < frameCount; ++i)
+								o[i * nchan] += (pfBuffer[2 * i] * fStereoPanningFactor[2 * s + 0]
+												 + pfBuffer[2 * i + 1] * fStereoPanningFactor[2 * s + 1])
+												* channelVol;
+						} else {
+							for (unsigned int i = 0; i < frameCount; ++i)
+								o[i * nchan] += pfBuffer[i] * channelVol;
+						}
 					}
 				}
 			}
-		}
 
-		if (recorder && recorder->isInMixDownMode()) {
-			recorder->addBuffer(nullptr, recbuff, static_cast< int >(frameCount));
+			if (recorder && recorder->isInMixDownMode()) {
+				recorder->addBuffer(nullptr, recbuff, static_cast< int >(frameCount));
+			}
 		}
-	}
 
-	bool pluginModifiedAudio = false;
-	emit audioOutputAboutToPlay(output, frameCount, nchan, SAMPLE_RATE, &pluginModifiedAudio);
-
-	if (pluginModifiedAudio || (!qlMix.isEmpty())) {
-		// Clip the output audio
-		if (eSampleFormat == SampleFloat)
-			for (unsigned int i = 0; i < frameCount * iChannels; i++)
-				output[i] = qBound(-1.0f, output[i], 1.0f);
-		else
-			// Also convert the intermediate float array into an array of shorts before writing it to the outbuff
-			for (unsigned int i = 0; i < frameCount * iChannels; i++)
-				reinterpret_cast< short * >(outbuff)[i] =
-					static_cast< short >(qBound(-32768.f, (output[i] * 32768.f), 32767.f));
+		bool pluginModifiedAudio = false;
+		emit audioOutputAboutToPlay(output, frameCount, nchan, SAMPLE_RATE, &pluginModifiedAudio);
+
+		haveAudio |= pluginModifiedAudio;
+
+		if (haveAudio) {
+			// Clip the output audio
+			if (eSampleFormat == SampleFloat)
+				for (unsigned int i = 0; i < frameCount * iChannels; i++)
+					output[i] = qBound(-1.0f, output[i], 1.0f);
+			else
+				// Also convert the intermediate float array into an array of shorts before writing it to the outbuff
+				for (unsigned int i = 0; i < frameCount * iChannels; i++)
+					reinterpret_cast< short * >(outbuff)[i] =
+						static_cast< short >(qBound(-32768.f, (output[i] * 32768.f), 32767.f));
+		}
 	}
 
-	qrwlOutputs.unlock();
-
 	// Delete all AudioOutputBuffer that no longer provide any new audio
+	// Note: removeBuffer does not dereference the passed pointers, which is
+	// why it is safe to pass these pointers around while no longer holding
+	// the qrwlOutputs lock.
 	for (AudioOutputBuffer *buffer : qlDel) {
 		removeBuffer(buffer);
 	}
@@ -795,7 +817,7 @@ bool AudioOutput::mix(void *outbuff, unsigned int frameCount) {
 #endif
 
 	// Return whether data has been written to the outbuff
-	return (pluginModifiedAudio || (!qlMix.isEmpty()));
+	return haveAudio;
 }
 
 bool AudioOutput::isAlive() const {

From d10d1dd0b352ffd159742e5766b827fb6b31c01f Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 17:30:02 +0100
Subject: [PATCH 2/9] FIX(client): Potential concurrent map access

If QMultiHash::value() was called at the exact same time that other
parts of the code mutate the map, this could lead to a data race.

By protecting the map access via the associated lock, concurrent
modification is prevented (assuming all mutating code obtain a write
lock as they should).
---
 src/mumble/AudioOutput.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 5415247dc04..421b634c8a7 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -263,7 +263,15 @@ void AudioOutput::removeBuffer(AudioOutputBuffer *buffer) {
 }
 
 void AudioOutput::removeUser(const ClientUser *user) {
-	removeBuffer(qmOutputs.value(user));
+	AudioOutputBuffer *buffer = nullptr;
+	{
+		QReadLocker lock(&qrwlOutputs);
+		buffer = qmOutputs.value(user);
+	}
+
+	// We rely on removeBuffer not actually dereferencing the passed pointer.
+	// It it did, releasing the lock before calling the function cries for trouble.
+	removeBuffer(buffer);
 }
 
 void AudioOutput::removeToken(AudioOutputToken &token) {

From ca87877a56067bf62a3adaa58736e76eb270ece8 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 17:49:21 +0100
Subject: [PATCH 3/9] REFAC(client,audio): Clarify that passed pointer is not
 dereferenced

This is now encoded into the type system as it is illegal to dereference
a void *. Previously, calling functions just needed to know that the
passed pointer was never going to be dereferenced (in order to
determined whether or not the function call may happen while not holding
the qrwlOutputs lock.
---
 src/mumble/AudioOutput.cpp | 15 +++++++++------
 src/mumble/AudioOutput.h   | 10 +++++-----
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 421b634c8a7..d63c8da55f6 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -203,7 +203,7 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 		if (speech) {
 			// removeBuffer doesn't dereference speech, so passing the pointer around without
 			// holding the lock is fine.
-			removeBuffer(static_cast< AudioOutputBuffer * >(speech));
+			removeBuffer(speech);
 		}
 
 		while ((iMixerFreq == 0) && isAlive()) {
@@ -223,21 +223,24 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 	}
 }
 
-void AudioOutput::handleInvalidatedBuffer(AudioOutputBuffer *buffer) {
+void AudioOutput::handleInvalidatedBuffer(const void *buffer) {
 	QWriteLocker locker(&qrwlOutputs);
 	for (auto iter = qmOutputs.begin(); iter != qmOutputs.end(); ++iter) {
 		if (iter.value() == buffer) {
+			delete iter.value();
 			qmOutputs.erase(iter);
-			delete buffer;
+
 			break;
 		}
 	}
 }
 
-void AudioOutput::handlePositionedBuffer(AudioOutputBuffer *buffer, float x, float y, float z) {
+void AudioOutput::handlePositionedBuffer(const void *bufferPtr, float x, float y, float z) {
 	QWriteLocker locker(&qrwlOutputs);
 	for (auto iter = qmOutputs.begin(); iter != qmOutputs.end(); ++iter) {
-		if (iter.value() == buffer) {
+		if (iter.value() == bufferPtr) {
+			AudioOutputBuffer *buffer = iter.value();
+
 			buffer->fPos[0] = x;
 			buffer->fPos[1] = y;
 			buffer->fPos[2] = z;
@@ -254,7 +257,7 @@ void AudioOutput::setBufferPosition(const AudioOutputToken &token, float x, floa
 	emit bufferPositionChanged(token.m_buffer, x, y, z);
 }
 
-void AudioOutput::removeBuffer(AudioOutputBuffer *buffer) {
+void AudioOutput::removeBuffer(const void *buffer) {
 	if (!buffer) {
 		return;
 	}
diff --git a/src/mumble/AudioOutput.h b/src/mumble/AudioOutput.h
index 675e8f501ef..a19c3c6a4e7 100644
--- a/src/mumble/AudioOutput.h
+++ b/src/mumble/AudioOutput.h
@@ -79,11 +79,11 @@ class AudioOutput : public QThread {
 	bool *bSpeakerPositional = nullptr;
 	/// Used when panning stereo stream w.r.t. each speaker.
 	float *fStereoPanningFactor = nullptr;
-	void removeBuffer(AudioOutputBuffer *);
+	void removeBuffer(const void *);
 
 private slots:
-	void handleInvalidatedBuffer(AudioOutputBuffer *);
-	void handlePositionedBuffer(AudioOutputBuffer *, float x, float y, float z);
+	void handleInvalidatedBuffer(const void *);
+	void handlePositionedBuffer(const void *, float x, float y, float z);
 
 protected:
 	enum { SampleShort, SampleFloat } eSampleFormat = SampleFloat;
@@ -151,8 +151,8 @@ private slots:
 	void audioOutputAboutToPlay(float *outputPCM, unsigned int sampleCount, unsigned int channelCount,
 								unsigned int sampleRate, bool *modifiedAudio);
 
-	void bufferInvalidated(AudioOutputBuffer *);
-	void bufferPositionChanged(AudioOutputBuffer *, float x, float y, float z);
+	void bufferInvalidated(const void *);
+	void bufferPositionChanged(const void *, float x, float y, float z);
 };
 
 #endif

From 457f19770deaa8ae08cd002205cfbe727b044c6a Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 17:55:05 +0100
Subject: [PATCH 4/9] REFAC(client,audio): Rename removeBuffer ->
 invalidateBuffer

---
 src/mumble/AudioOutput.cpp | 20 ++++++++++----------
 src/mumble/AudioOutput.h   |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index d63c8da55f6..52d5eb25773 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -150,7 +150,7 @@ float AudioOutput::calcGain(float dotproduct, float distance) {
 
 void AudioOutput::wipe() {
 	// We need to remove all buffers from the qmOutputs map.
-	// However, the removeBuffer calls a signal-slot mechanism
+	// However, the invalidateBuffer calls a signal-slot mechanism
 	// asynchronously. Doing that while iterating over the map
 	// will cause a concurrent modification
 
@@ -164,7 +164,7 @@ void AudioOutput::wipe() {
 	}
 
 	for (AudioOutputBuffer *buffer : list) {
-		removeBuffer(buffer);
+		invalidateBuffer(buffer);
 	}
 }
 
@@ -201,9 +201,9 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 
 	if (createNew) {
 		if (speech) {
-			// removeBuffer doesn't dereference speech, so passing the pointer around without
+			// invalidateBuffer doesn't dereference speech, so passing the pointer around without
 			// holding the lock is fine.
-			removeBuffer(speech);
+			invalidateBuffer(speech);
 		}
 
 		while ((iMixerFreq == 0) && isAlive()) {
@@ -257,7 +257,7 @@ void AudioOutput::setBufferPosition(const AudioOutputToken &token, float x, floa
 	emit bufferPositionChanged(token.m_buffer, x, y, z);
 }
 
-void AudioOutput::removeBuffer(const void *buffer) {
+void AudioOutput::invalidateBuffer(const void *buffer) {
 	if (!buffer) {
 		return;
 	}
@@ -272,13 +272,13 @@ void AudioOutput::removeUser(const ClientUser *user) {
 		buffer = qmOutputs.value(user);
 	}
 
-	// We rely on removeBuffer not actually dereferencing the passed pointer.
+	// We rely on invalidateBuffer not actually dereferencing the passed pointer.
 	// It it did, releasing the lock before calling the function cries for trouble.
-	removeBuffer(buffer);
+	invalidateBuffer(buffer);
 }
 
 void AudioOutput::removeToken(AudioOutputToken &token) {
-	removeBuffer(token.m_buffer);
+	invalidateBuffer(token.m_buffer);
 	token = {};
 }
 
@@ -816,11 +816,11 @@ bool AudioOutput::mix(void *outbuff, unsigned int frameCount) {
 	}
 
 	// Delete all AudioOutputBuffer that no longer provide any new audio
-	// Note: removeBuffer does not dereference the passed pointers, which is
+	// Note: invalidateBuffer does not dereference the passed pointers, which is
 	// why it is safe to pass these pointers around while no longer holding
 	// the qrwlOutputs lock.
 	for (AudioOutputBuffer *buffer : qlDel) {
-		removeBuffer(buffer);
+		invalidateBuffer(buffer);
 	}
 
 #ifdef USE_MANUAL_PLUGIN
diff --git a/src/mumble/AudioOutput.h b/src/mumble/AudioOutput.h
index a19c3c6a4e7..19334a8c7f2 100644
--- a/src/mumble/AudioOutput.h
+++ b/src/mumble/AudioOutput.h
@@ -79,7 +79,7 @@ class AudioOutput : public QThread {
 	bool *bSpeakerPositional = nullptr;
 	/// Used when panning stereo stream w.r.t. each speaker.
 	float *fStereoPanningFactor = nullptr;
-	void removeBuffer(const void *);
+	void invalidateBuffer(const void *);
 
 private slots:
 	void handleInvalidatedBuffer(const void *);

From 4caed1cecb3f05a8eac141efeb94b809f6693ba5 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 17:57:31 +0100
Subject: [PATCH 5/9] REFAC(client,audio): Rename handleInvalidatedBuffer ->
 removeBuffer

---
 src/mumble/AudioOutput.cpp | 4 ++--
 src/mumble/AudioOutput.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 52d5eb25773..1a4e56e9516 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -81,7 +81,7 @@ bool AudioOutputRegistrar::canExclusive() const {
 }
 
 AudioOutput::AudioOutput() {
-	QObject::connect(this, &AudioOutput::bufferInvalidated, this, &AudioOutput::handleInvalidatedBuffer);
+	QObject::connect(this, &AudioOutput::bufferInvalidated, this, &AudioOutput::removeBuffer);
 	QObject::connect(this, &AudioOutput::bufferPositionChanged, this, &AudioOutput::handlePositionedBuffer);
 }
 
@@ -223,7 +223,7 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 	}
 }
 
-void AudioOutput::handleInvalidatedBuffer(const void *buffer) {
+void AudioOutput::removeBuffer(const void *buffer) {
 	QWriteLocker locker(&qrwlOutputs);
 	for (auto iter = qmOutputs.begin(); iter != qmOutputs.end(); ++iter) {
 		if (iter.value() == buffer) {
diff --git a/src/mumble/AudioOutput.h b/src/mumble/AudioOutput.h
index 19334a8c7f2..64a9a2238a0 100644
--- a/src/mumble/AudioOutput.h
+++ b/src/mumble/AudioOutput.h
@@ -82,7 +82,7 @@ class AudioOutput : public QThread {
 	void invalidateBuffer(const void *);
 
 private slots:
-	void handleInvalidatedBuffer(const void *);
+	void removeBuffer(const void *);
 	void handlePositionedBuffer(const void *, float x, float y, float z);
 
 protected:

From 0cd73f41fc4260896e10add4e625fe80abc38300 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 18:17:57 +0100
Subject: [PATCH 6/9] REFAC(client): Rename removeToken -> invalidateToken

---
 src/mumble/AudioInput.cpp  | 3 ++-
 src/mumble/AudioOutput.cpp | 3 +--
 src/mumble/AudioOutput.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/mumble/AudioInput.cpp b/src/mumble/AudioInput.cpp
index ffd2c4254fd..5a35057c63f 100644
--- a/src/mumble/AudioInput.cpp
+++ b/src/mumble/AudioInput.cpp
@@ -1031,7 +1031,8 @@ void AudioInput::encodeAudioFrame(AudioChunk chunk) {
 
 			if (stopActiveCue) {
 				// Cancel active cue first, if there is any
-				ao->removeToken(m_activeAudioCue);
+				ao->invalidateToken(m_activeAudioCue);
+				m_activeAudioCue = {};
 			}
 
 			if (playAudioOnCue) {
diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 1a4e56e9516..ab49761e157 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -277,9 +277,8 @@ void AudioOutput::removeUser(const ClientUser *user) {
 	invalidateBuffer(buffer);
 }
 
-void AudioOutput::removeToken(AudioOutputToken &token) {
+void AudioOutput::invalidateToken(const AudioOutputToken &token) {
 	invalidateBuffer(token.m_buffer);
-	token = {};
 }
 
 AudioOutputToken AudioOutput::playSample(const QString &filename, float volume, bool loop) {
diff --git a/src/mumble/AudioOutput.h b/src/mumble/AudioOutput.h
index 64a9a2238a0..2c594ded965 100644
--- a/src/mumble/AudioOutput.h
+++ b/src/mumble/AudioOutput.h
@@ -127,7 +127,7 @@ private slots:
 	unsigned int getMixerFreq() const;
 	void setBufferSize(unsigned int bufferSize);
 	void setBufferPosition(const AudioOutputToken &, float x, float y, float z);
-	void removeToken(AudioOutputToken &);
+	void invalidateToken(const AudioOutputToken &);
 	void removeUser(const ClientUser *);
 
 signals:

From fe11451b15207368bbe74cb026624bc8d34e8b03 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 18:32:41 +0100
Subject: [PATCH 7/9] REFAC(client, audio): Ensure removeUser removes
 immediately

The AudioOutput::removeUser function used to initiate a signal being
emitted that leads to the deletion of the audio buffer related to the
given user and also to the removal of that user (and buffer) from the
qmOutputs map.

Since removeUser has (thus far) always been called from the main thread
(in which the AudioOutput QThread object lives), the connection has been
direct, meaning that once removeUser returns to the caller, the user has
indeed been removed from AudioOutput.
However, if removeUser was called from a different thread, the
connection would be queued, meaning that AudioOutput still holds a
reference (or rather pointer) to that user when removeUser returns. This
can easily lead to a memory corruption issue.

Therefore, this commit ensures that removeUser always makes a direct
function call to remove all references to that user so that the caller
can be sure that AudioOutput will have forgotten about that particular
user once that function returns.
---
 src/mumble/AudioOutput.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index ab49761e157..1a09cb448b5 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -272,9 +272,9 @@ void AudioOutput::removeUser(const ClientUser *user) {
 		buffer = qmOutputs.value(user);
 	}
 
-	// We rely on invalidateBuffer not actually dereferencing the passed pointer.
-	// It it did, releasing the lock before calling the function cries for trouble.
-	invalidateBuffer(buffer);
+	// We rely on removeBuffer not actually dereferencing the passed pointer.
+	// If it did, releasing the lock before calling the function cries for trouble.
+	removeBuffer(buffer);
 }
 
 void AudioOutput::invalidateToken(const AudioOutputToken &token) {

From 05f31ca97cdaed81a997920e5b20bd8c67f1de7e Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 18:37:15 +0100
Subject: [PATCH 8/9] REFAC(client, audio): Make wipe() do the wiping directly

Before, the actual wiping of elements was delegated to the removeBuffer
function by means of Qt's signal/slot mechanism. This required a rather
inefficient implementation inside wipe() (using an auxiliary list), lead
to constant locking and re-locking (since a write lock is acquired and
released for every buffer individually) and made it unclear whether
after AudioOutput::wipe() returns, the buffer has actually been wiped
(due to the way the Auto connection type for Qt signal/slots work).

This commit ensures that wipe no longer delegates the work, thereby
fixing all the abovementioned issues.
---
 src/mumble/AudioOutput.cpp | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 1a09cb448b5..332a00e2a14 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -149,23 +149,13 @@ float AudioOutput::calcGain(float dotproduct, float distance) {
 }
 
 void AudioOutput::wipe() {
-	// We need to remove all buffers from the qmOutputs map.
-	// However, the invalidateBuffer calls a signal-slot mechanism
-	// asynchronously. Doing that while iterating over the map
-	// will cause a concurrent modification
-
-	QList< AudioOutputBuffer * > list;
+	QWriteLocker locker(&qrwlOutputs);
 
-	{
-		QReadLocker locker(&qrwlOutputs);
-		for (AudioOutputBuffer *buffer : qmOutputs) {
-			list.append(buffer);
-		}
+	for (AudioOutputBuffer *buffer : qmOutputs) {
+		delete buffer;
 	}
 
-	for (AudioOutputBuffer *buffer : list) {
-		invalidateBuffer(buffer);
-	}
+	qmOutputs.clear();
 }
 
 const float *AudioOutput::getSpeakerPos(unsigned int &speakers) {

From 8fc7a9316b299e35850ab6688572a402f8ef9eb5 Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sat, 18 Jan 2025 18:40:27 +0100
Subject: [PATCH 9/9] REFAC(client,audio): Skip Qt signal/slot mechanism for
 buffer deletion

AudioOutput::addFrameToBuffer used to emit a signal for deleting the old
speech buffer. However, it implicitly relied on that connection being
direct (i.e. it relied on addFrameToBuffer to be called from the main
thread) as otherwise, there would be the potential for a memory leak
where the speech buffer is not processed by removeBuffer before the
addFrameToBuffer call has replaced the associated entry in qmOutputs. At
that point speech will be the last reference pointing to that object,
and as soon as speech goes out of scope, the memory for that object
would have leaked.

In order to make the control flow more clear, we now skip the signal
emission and instead call removeBuffer directly. This has been done in a
way that also prevents the need for an additional acquisition of a write
lock.
---
 src/mumble/AudioOutput.cpp | 25 ++++++++++++++++---------
 src/mumble/AudioOutput.h   |  2 +-
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/mumble/AudioOutput.cpp b/src/mumble/AudioOutput.cpp
index 332a00e2a14..f24137ae850 100644
--- a/src/mumble/AudioOutput.cpp
+++ b/src/mumble/AudioOutput.cpp
@@ -81,7 +81,7 @@ bool AudioOutputRegistrar::canExclusive() const {
 }
 
 AudioOutput::AudioOutput() {
-	QObject::connect(this, &AudioOutput::bufferInvalidated, this, &AudioOutput::removeBuffer);
+	QObject::connect(this, &AudioOutput::bufferInvalidated, this, [this](const void *buffer) { removeBuffer(buffer); });
 	QObject::connect(this, &AudioOutput::bufferPositionChanged, this, &AudioOutput::handlePositionedBuffer);
 }
 
@@ -190,12 +190,6 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 	}
 
 	if (createNew) {
-		if (speech) {
-			// invalidateBuffer doesn't dereference speech, so passing the pointer around without
-			// holding the lock is fine.
-			invalidateBuffer(speech);
-		}
-
 		while ((iMixerFreq == 0) && isAlive()) {
 			QThread::yieldCurrentThread();
 		}
@@ -205,6 +199,9 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 		}
 
 		QWriteLocker lock(&qrwlOutputs);
+		if (speech) {
+			removeBuffer(speech, false);
+		}
 
 		speech = new AudioOutputSpeech(sender, iMixerFreq, audioData.usedCodec, iBufferSize);
 		qmOutputs.replace(sender, speech);
@@ -213,8 +210,18 @@ void AudioOutput::addFrameToBuffer(ClientUser *sender, const Mumble::Protocol::A
 	}
 }
 
-void AudioOutput::removeBuffer(const void *buffer) {
-	QWriteLocker locker(&qrwlOutputs);
+void AudioOutput::removeBuffer(const void *buffer, bool acquireWriteLock) {
+	if (!buffer) {
+		return;
+	}
+
+	// Either this function is asked to obtain a write lock, or
+	// the calling scope already holds a **write** lock (which means
+	// that attempting to lock for read access will fail, whereas it
+	// would work if the calling scope only held a read lock).
+	assert(acquireWriteLock || !qrwlOutputs.tryLockForRead(0));
+	QWriteLocker locker(acquireWriteLock ? &qrwlOutputs : nullptr);
+
 	for (auto iter = qmOutputs.begin(); iter != qmOutputs.end(); ++iter) {
 		if (iter.value() == buffer) {
 			delete iter.value();
diff --git a/src/mumble/AudioOutput.h b/src/mumble/AudioOutput.h
index 2c594ded965..e71cda05a16 100644
--- a/src/mumble/AudioOutput.h
+++ b/src/mumble/AudioOutput.h
@@ -82,7 +82,7 @@ class AudioOutput : public QThread {
 	void invalidateBuffer(const void *);
 
 private slots:
-	void removeBuffer(const void *);
+	void removeBuffer(const void *, bool acquireWriteLock = true);
 	void handlePositionedBuffer(const void *, float x, float y, float z);
 
 protected: