Merge #5514

5514: Update menu music state when the config variables change via console, various refactoring r=heinrich5991 a=Robyt3

Also update the background music when `snd_enable` or `snd_enable_music` change via console or bind. Closes #2911.

For this purpose, add `IsPlaying` method to engine sound and client sound component to check whether a specific sound sample is already playing.

Various refactoring in engine sound.

## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>

src/engine/client/sound.cpp

@@ -89,16 +89,6 @@ static int s_WVBufferSize = 0;
 const int DefaultDistance = 1500;
 int m_LastBreak = 0;
 
-// TODO: there should be a faster way todo this
-static short Int2Short(int i)
-{
-	if(i > 0x7fff)
-		return 0x7fff;
-	else if(i < -0x7fff)
-		return -0x7fff;
-	return i;
-}
-
 static int IntAbs(int i)
 {
 	if(i < 0)
@@ -108,14 +98,13 @@ static int IntAbs(int i)
 
 static void Mix(short *pFinalOut, unsigned Frames)
 {
-	int MasterVol;
 	Frames = minimum(Frames, m_MaxFrames);
 	mem_zero(m_pMixBuffer, Frames * 2 * sizeof(int));
 
 	// acquire lock while we are mixing
 	m_SoundLock.lock();
 
-	MasterVol = m_SoundVolume;
+	int MasterVol = m_SoundVolume;
 
 	for(auto &Voice : m_aVoices)
 	{
@@ -260,21 +249,9 @@ static void Mix(short *pFinalOut, unsigned Frames)
 	// release the lock
 	m_SoundLock.unlock();
 
-	{
+	// clamp accumulated values
-		// clamp accumulated values
+	for(unsigned i = 0; i < Frames * 2; i++)
-		// TODO: this seams slow
+		pFinalOut[i] = clamp<int>(((m_pMixBuffer[i] * MasterVol) / 101) >> 8, std::numeric_limits<short>::min(), std::numeric_limits<short>::max());
-		for(unsigned i = 0; i < Frames; i++)
-		{
-			int j = i << 1;
-			int vl = ((m_pMixBuffer[j] * MasterVol) / 101) >> 8;
-			int vr = ((m_pMixBuffer[j + 1] * MasterVol) / 101) >> 8;
-
-			pFinalOut[j] = Int2Short(vl);
-			pFinalOut[j + 1] = Int2Short(vr);
-
-			// dbg_msg("sound", "the real shit: %d %d", pFinalOut[j], pFinalOut[j+1]);
-		}
-	}
 
 #if defined(CONF_ARCH_ENDIAN_BIG)
 	swap_endian(pFinalOut, sizeof(short), Frames * 2);
@@ -287,20 +264,20 @@ static void SdlCallback(void *pUnused, Uint8 *pStream, int Len)
 #if defined(CONF_VIDEORECORDER)
 	if(!(IVideo::Current() && g_Config.m_ClVideoSndEnable))
 	{
-		Mix((short *)pStream, Len / sizeof(int16_t) / 2);
+		Mix((short *)pStream, Len / sizeof(short) / 2);
 	}
 	else
 	{
 		mem_zero(pStream, Len);
 	}
 #else
-	Mix((short *)pStream, Len / 2 / 2);
+	Mix((short *)pStream, Len / sizeof(short) / 2);
 #endif
 }
 
 int CSound::Init()
 {
-	m_SoundEnabled = 0;
+	m_SoundEnabled = false;
 	m_pGraphics = Kernel()->RequestInterface<IEngineGraphics>();
 	m_pStorage = Kernel()->RequestInterface<IStorage>();
 
@@ -311,7 +288,7 @@ int CSound::Init()
 
 	if(SDL_InitSubSystem(SDL_INIT_AUDIO) < 0)
 	{
-		dbg_msg("gfx", "unable to init SDL audio: %s", SDL_GetError());
+		dbg_msg("client/sound", "unable to init SDL audio: %s", SDL_GetError());
 		return -1;
 	}
 
@@ -344,7 +321,7 @@ int CSound::Init()
 
 	SDL_PauseAudioDevice(m_Device, 0);
 
-	m_SoundEnabled = 1;
+	m_SoundEnabled = true;
 	Update(); // update the volume
 	return 0;
 }
@@ -394,16 +371,14 @@ int CSound::AllocID()
 void CSound::RateConvert(int SampleID)
 {
 	CSample *pSample = &m_aSamples[SampleID];
-	int NumFrames = 0;
-	short *pNewData = 0;
 
 	// make sure that we need to convert this sound
 	if(!pSample->m_pData || pSample->m_Rate == m_MixingRate)
 		return;
 
 	// allocate new data
-	NumFrames = (int)((pSample->m_NumFrames / (float)pSample->m_Rate) * m_MixingRate);
+	int NumFrames = (int)((pSample->m_NumFrames / (float)pSample->m_Rate) * m_MixingRate);
-	pNewData = (short *)calloc((size_t)NumFrames * pSample->m_Channels, sizeof(short));
+	short *pNewData = (short *)calloc((size_t)NumFrames * pSample->m_Channels, sizeof(short));
 
 	for(int i = 0; i < NumFrames; i++)
 	{
@@ -437,11 +412,11 @@ int CSound::DecodeOpus(int SampleID, const void *pData, unsigned DataSize)
 
 	CSample *pSample = &m_aSamples[SampleID];
 
-	OggOpusFile *OpusFile = op_open_memory((const unsigned char *)pData, DataSize, NULL);
+	OggOpusFile *pOpusFile = op_open_memory((const unsigned char *)pData, DataSize, NULL);
-	if(OpusFile)
+	if(pOpusFile)
 	{
-		int NumChannels = op_channel_count(OpusFile, -1);
+		int NumChannels = op_channel_count(pOpusFile, -1);
-		int NumSamples = op_pcm_total(OpusFile, -1); // per channel!
+		int NumSamples = op_pcm_total(pOpusFile, -1); // per channel!
 
 		pSample->m_Channels = NumChannels;
 
@@ -453,15 +428,22 @@ int CSound::DecodeOpus(int SampleID, const void *pData, unsigned DataSize)
 
 		pSample->m_pData = (short *)calloc((size_t)NumSamples * NumChannels, sizeof(short));
 
-		int Read;
 		int Pos = 0;
 		while(Pos < NumSamples)
 		{
-			Read = op_read(OpusFile, pSample->m_pData + Pos * NumChannels, NumSamples * NumChannels, NULL);
+			const int Read = op_read(pOpusFile, pSample->m_pData + Pos * NumChannels, NumSamples * NumChannels, NULL);
+			if(Read < 0)
+			{
+				free(pSample->m_pData);
+				dbg_msg("sound/opus", "op_read error %d at %d", Read, Pos);
+				return -1;
+			}
+			else if(Read == 0) // EOF
+				break;
 			Pos += Read;
 		}
 
-		pSample->m_NumFrames = NumSamples; // ?
+		pSample->m_NumFrames = Pos;
 		pSample->m_Rate = 48000;
 		pSample->m_LoopStart = -1;
 		pSample->m_LoopEnd = -1;
@@ -523,7 +505,6 @@ int CSound::DecodeWV(int SampleID, const void *pData, unsigned DataSize)
 
 	CSample *pSample = &m_aSamples[SampleID];
 	char aError[100];
-	WavpackContext *pContext;
 
 	s_pWVBuffer = pData;
 	s_WVBufferSize = DataSize;
@@ -536,9 +517,9 @@ int CSound::DecodeWV(int SampleID, const void *pData, unsigned DataSize)
 	Callback.get_pos = GetPos;
 	Callback.push_back_byte = PushBackByte;
 	Callback.read_bytes = ReadData;
-	pContext = WavpackOpenFileInputEx(&Callback, (void *)1, 0, aError, 0, 0);
+	WavpackContext *pContext = WavpackOpenFileInputEx(&Callback, (void *)1, 0, aError, 0, 0);
 #else
-	pContext = WavpackOpenFileInput(ReadDataOld, aError);
+	WavpackContext *pContext = WavpackOpenFileInput(ReadDataOld, aError);
 #endif
 	if(pContext)
 	{
@@ -546,9 +527,6 @@ int CSound::DecodeWV(int SampleID, const void *pData, unsigned DataSize)
 		int BitsPerSample = WavpackGetBitsPerSample(pContext);
 		unsigned int SampleRate = WavpackGetSampleRate(pContext);
 		int NumChannels = WavpackGetNumChannels(pContext);
-		int *pSrc;
-		short *pDst;
-		int i;
 
 		pSample->m_Channels = NumChannels;
 		pSample->m_Rate = SampleRate;
@@ -566,13 +544,18 @@ int CSound::DecodeWV(int SampleID, const void *pData, unsigned DataSize)
 		}
 
 		int *pBuffer = (int *)calloc((size_t)NumSamples * NumChannels, sizeof(int));
-		WavpackUnpackSamples(pContext, pBuffer, NumSamples); // TODO: check return value
+		if(!WavpackUnpackSamples(pContext, pBuffer, NumSamples))
-		pSrc = pBuffer;
+		{
+			free(pBuffer);
+			dbg_msg("sound/wv", "WavpackUnpackSamples failed. NumSamples=%d, NumChannels=%d", NumSamples, NumChannels);
+			return -1;
+		}
+		int *pSrc = pBuffer;
 
 		pSample->m_pData = (short *)calloc((size_t)NumSamples * NumChannels, sizeof(short));
-		pDst = pSample->m_pData;
+		short *pDst = pSample->m_pData;
 
-		for(i = 0; i < NumSamples * NumChannels; i++)
+		for(int i = 0; i < NumSamples * NumChannels; i++)
 			*pDst++ = (short)*pSrc++;
 
 		free(pBuffer);
@@ -869,25 +852,23 @@ void CSound::SetChannel(int ChannelID, float Vol, float Pan)
 
 ISound::CVoiceHandle CSound::Play(int ChannelID, int SampleID, int Flags, float x, float y)
 {
-	int VoiceID = -1;
-	int Age = -1;
-	int i;
-
 	m_SoundLock.lock();
 
 	// search for voice
-	for(i = 0; i < NUM_VOICES; i++)
+	int VoiceID = -1;
+	for(int i = 0; i < NUM_VOICES; i++)
 	{
-		int id = (m_NextVoice + i) % NUM_VOICES;
+		int NextID = (m_NextVoice + i) % NUM_VOICES;
-		if(!m_aVoices[id].m_pSample)
+		if(!m_aVoices[NextID].m_pSample)
 		{
-			VoiceID = id;
+			VoiceID = NextID;
-			m_NextVoice = id + 1;
+			m_NextVoice = NextID + 1;
 			break;
 		}
 	}
 
 	// voice found, use it
+	int Age = -1;
 	if(VoiceID != -1)
 	{
 		m_aVoices[VoiceID].m_pSample = &m_aSamples[SampleID];
@@ -966,10 +947,15 @@ void CSound::StopVoice(CVoiceHandle Voice)
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
-	{
+	m_aVoices[VoiceID].m_pSample = 0;
-		m_aVoices[VoiceID].m_pSample = 0;
+	m_aVoices[VoiceID].m_Age++;
-		m_aVoices[VoiceID].m_Age++;
+}
-	}
+
+bool CSound::IsPlaying(int SampleID)
+{
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
+	const CSample *pSample = &m_aSamples[SampleID];
+	return std::any_of(std::begin(m_aVoices), std::end(m_aVoices), [pSample](const auto &Voice) { return Voice.m_pSample == pSample; });
 }
 
 ISoundMixFunc CSound::GetSoundMixFunc()

src/engine/client/sound.h

@@ -13,17 +13,12 @@ class IStorage;
 
 class CSound : public IEngineSound
 {
-	int m_SoundEnabled;
+	bool m_SoundEnabled;
 	SDL_AudioDeviceID m_Device;
 
-public:
 	IEngineGraphics *m_pGraphics;
 	IStorage *m_pStorage;
 
-	int Init() override;
-
-	int Update() override;
-	int Shutdown() override;
 	int AllocID();
 
 	static void RateConvert(int SampleID);
@@ -32,7 +27,12 @@ public:
 	static int DecodeWV(int SampleID, const void *pData, unsigned DataSize);
 	static int DecodeOpus(int SampleID, const void *pData, unsigned DataSize);
 
-	bool IsSoundEnabled() override { return m_SoundEnabled != 0; }
+public:
+	int Init() override;
+	int Update() override;
+	int Shutdown() override;
+
+	bool IsSoundEnabled() override { return m_SoundEnabled; }
 
 	int LoadWV(const char *pFilename) override;
 	int LoadWVFromMem(const void *pData, unsigned DataSize, bool FromEditor) override;
@@ -59,6 +59,7 @@ public:
 	void Stop(int SampleID) override;
 	void StopAll() override;
 	void StopVoice(CVoiceHandle Voice) override;
+	bool IsPlaying(int SampleID) override;
 
 	ISoundMixFunc GetSoundMixFunc() override;
 	void PauseAudioDevice() override;

src/engine/sound.h

@@ -87,6 +87,7 @@ public:
 	virtual void Stop(int SampleID) = 0;
 	virtual void StopAll() = 0;
 	virtual void StopVoice(CVoiceHandle Voice) = 0;
+	virtual bool IsPlaying(int SampleID) = 0;
 
 	virtual ISoundMixFunc GetSoundMixFunc() = 0;
 	// useful for thread synchronization

src/game/client/components/menus.cpp

@@ -994,6 +994,9 @@ void CMenus::OnInit()
 	Console()->Chain("add_friend", ConchainFriendlistUpdate, this);
 	Console()->Chain("remove_friend", ConchainFriendlistUpdate, this);
 
+	Console()->Chain("snd_enable", ConchainUpdateMusicState, this);
+	Console()->Chain("snd_enable_music", ConchainUpdateMusicState, this);
+
 	Console()->Chain("cl_assets_entities", ConchainAssetsEntities, this);
 	Console()->Chain("cl_asset_game", ConchainAssetGame, this);
 	Console()->Chain("cl_asset_emoticons", ConchainAssetEmoticons, this);
@@ -1017,6 +1020,23 @@ void CMenus::OnInit()
 	Storage()->ListDirectory(IStorage::TYPE_ALL, "menuimages", MenuImageScan, this);
 }
 
+void CMenus::ConchainUpdateMusicState(IConsole::IResult *pResult, void *pUserData, IConsole::FCommandCallback pfnCallback, void *pCallbackUserData)
+{
+	pfnCallback(pResult, pCallbackUserData);
+	auto *pSelf = (CMenus *)pUserData;
+	if(pResult->NumArguments())
+		pSelf->UpdateMusicState();
+}
+
+void CMenus::UpdateMusicState()
+{
+	const bool ShouldPlay = Client()->State() == IClient::STATE_OFFLINE && g_Config.m_SndEnable && g_Config.m_SndMusic;
+	if(ShouldPlay && !m_pClient->m_Sounds.IsPlaying(SOUND_MENU))
+		m_pClient->m_Sounds.Enqueue(CSounds::CHN_MUSIC, SOUND_MENU);
+	else if(!ShouldPlay && m_pClient->m_Sounds.IsPlaying(SOUND_MENU))
+		m_pClient->m_Sounds.Stop(SOUND_MENU);
+}
+
 void CMenus::PopupMessage(const char *pTopic, const char *pBody, const char *pButton)
 {
 	// reset active item
@@ -1264,7 +1284,7 @@ int CMenus::Render()
 	}
 	else if(s_Frame == 1)
 	{
-		m_pClient->m_Sounds.Enqueue(CSounds::CHN_MUSIC, SOUND_MENU);
+		UpdateMusicState();
 		s_Frame++;
 		m_DoubleClickIndex = -1;
 
@@ -2441,7 +2461,7 @@ void CMenus::OnStateChange(int NewState, int OldState)
 	if(NewState == IClient::STATE_OFFLINE)
 	{
 		if(OldState >= IClient::STATE_ONLINE && NewState < IClient::STATE_QUITTING)
-			m_pClient->m_Sounds.Play(CSounds::CHN_MUSIC, SOUND_MENU, 1.0f);
+			UpdateMusicState();
 		m_Popup = POPUP_NONE;
 		if(Client()->ErrorString() && Client()->ErrorString()[0] != 0)
 		{

src/game/client/components/menus.h

@@ -492,6 +492,8 @@ protected:
 	//void render_loading(float percent);
 	int RenderMenubar(CUIRect r);
 	void RenderNews(CUIRect MainView);
+	static void ConchainUpdateMusicState(IConsole::IResult *pResult, void *pUserData, IConsole::FCommandCallback pfnCallback, void *pCallbackUserData);
+	void UpdateMusicState();
 
 	// found in menus_demo.cpp
 	static bool DemoFilterChat(const void *pData, int Size, void *pUser);

src/game/client/components/menus_settings.cpp

@@ -1775,13 +1775,7 @@ void CMenus::RenderSettingsSound(CUIRect MainView)
 	if(DoButton_CheckBox(&g_Config.m_SndEnable, Localize("Use sounds"), g_Config.m_SndEnable, &Button))
 	{
 		g_Config.m_SndEnable ^= 1;
-		if(g_Config.m_SndEnable)
+		UpdateMusicState();
-		{
-			if(g_Config.m_SndMusic && Client()->State() == IClient::STATE_OFFLINE)
-				m_pClient->m_Sounds.Play(CSounds::CHN_MUSIC, SOUND_MENU, 1.0f);
-		}
-		else
-			m_pClient->m_Sounds.Stop(SOUND_MENU);
 		m_NeedRestartSound = g_Config.m_SndEnable && (!s_SndEnable || s_SndRate != g_Config.m_SndRate);
 	}
 
@@ -1792,13 +1786,7 @@ void CMenus::RenderSettingsSound(CUIRect MainView)
 	if(DoButton_CheckBox(&g_Config.m_SndMusic, Localize("Play background music"), g_Config.m_SndMusic, &Button))
 	{
 		g_Config.m_SndMusic ^= 1;
-		if(Client()->State() == IClient::STATE_OFFLINE)
+		UpdateMusicState();
-		{
-			if(g_Config.m_SndMusic)
-				m_pClient->m_Sounds.Play(CSounds::CHN_MUSIC, SOUND_MENU, 1.0f);
-			else
-				m_pClient->m_Sounds.Stop(SOUND_MENU);
-		}
 	}
 
 	MainView.HSplitTop(20.0f, &Button, &MainView);

src/game/client/components/sounds.cpp

@@ -228,10 +228,22 @@ void CSounds::Stop(int SetId)
 	if(m_WaitForSoundJob || SetId < 0 || SetId >= g_pData->m_NumSounds)
 		return;
 
-	CDataSoundset *pSet = &g_pData->m_aSounds[SetId];
+	const CDataSoundset *pSet = &g_pData->m_aSounds[SetId];
-
 	for(int i = 0; i < pSet->m_NumSounds; i++)
-		Sound()->Stop(pSet->m_aSounds[i].m_Id);
+		if(pSet->m_aSounds[i].m_Id != -1)
+			Sound()->Stop(pSet->m_aSounds[i].m_Id);
+}
+
+bool CSounds::IsPlaying(int SetId)
+{
+	if(m_WaitForSoundJob || SetId < 0 || SetId >= g_pData->m_NumSounds)
+		return false;
+
+	const CDataSoundset *pSet = &g_pData->m_aSounds[SetId];
+	for(int i = 0; i < pSet->m_NumSounds; i++)
+		if(pSet->m_aSounds[i].m_Id != -1 && Sound()->IsPlaying(pSet->m_aSounds[i].m_Id))
+			return true;
+	return false;
 }
 
 ISound::CVoiceHandle CSounds::PlaySample(int Channel, int SampleId, float Vol, int Flags)

src/game/client/components/sounds.h

@@ -64,6 +64,7 @@ public:
 	void PlayAt(int Channel, int SetId, float Vol, vec2 Pos);
 	void PlayAndRecord(int Channel, int SetId, float Vol, vec2 Pos);
 	void Stop(int SetId);
+	bool IsPlaying(int SetId);
 
 	ISound::CVoiceHandle PlaySample(int Channel, int SampleId, float Vol, int Flags = 0);
 	ISound::CVoiceHandle PlaySampleAt(int Channel, int SampleId, float Vol, vec2 Pos, int Flags = 0);