From 06fcfe03b05b5d3d524d396fdaf821af2a65b338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 3 Oct 2024 21:35:43 +0200 Subject: [PATCH] Improve map sound validation and performance - Add check for map sound data being loaded from map to avoid null pointer access. - Add check for size of sound source data to avoid out-of-bounds reads. - Avoid iterating over all groups and layers and then performing a linear search on the sound queue entries every frame to update the sounds. Instead, store a pointer to the respective `CMapItemGroup` for every queue entry, so iterating over the queue once is enough to update all sound queue entries. - Move check for sound layer index outside of loop as the value does not change. - Avoid creating source queue entries for sounds that could not be loaded. - Mark pointers as `const` when possible. --- src/game/client/components/mapsounds.cpp | 141 +++++++++-------------- src/game/client/components/mapsounds.h | 14 +-- 2 files changed, 57 insertions(+), 98 deletions(-) diff --git a/src/game/client/components/mapsounds.cpp b/src/game/client/components/mapsounds.cpp index fd65b9d4b..cf8d0b491 100644 --- a/src/game/client/components/mapsounds.cpp +++ b/src/game/client/components/mapsounds.cpp @@ -70,8 +70,14 @@ void CMapSounds::OnMapLoad() } else { - const int SoundDataSize = pMap->GetDataSize(pSound->m_SoundData); const void *pData = pMap->GetData(pSound->m_SoundData); + if(pData == nullptr) + { + log_error("mapsounds", "Failed to load map sound %d: failed to load data.", i); + ShowWarning = true; + continue; + } + const int SoundDataSize = pMap->GetDataSize(pSound->m_SoundData); m_aSounds[i] = Sound()->LoadOpusFromMem(pData, SoundDataSize); pMap->UnloadData(pSound->m_SoundData); } @@ -83,47 +89,39 @@ void CMapSounds::OnMapLoad() } // enqueue sound sources - for(int g = 0; g < Layers()->NumGroups(); g++) + for(int GroupIndex = 0; GroupIndex < Layers()->NumGroups(); GroupIndex++) { - CMapItemGroup *pGroup = Layers()->GetGroup(g); - + const CMapItemGroup *pGroup = Layers()->GetGroup(GroupIndex); if(!pGroup) continue; - for(int l = 0; l < pGroup->m_NumLayers; l++) + for(int LayerIndex = 0; LayerIndex < pGroup->m_NumLayers; LayerIndex++) { - CMapItemLayer *pLayer = Layers()->GetLayer(pGroup->m_StartLayer + l); - + const CMapItemLayer *pLayer = Layers()->GetLayer(pGroup->m_StartLayer + LayerIndex); if(!pLayer) continue; + if(pLayer->m_Type != LAYERTYPE_SOUNDS) + continue; - if(pLayer->m_Type == LAYERTYPE_SOUNDS) + const CMapItemLayerSounds *pSoundLayer = reinterpret_cast(pLayer); + if(pSoundLayer->m_Version < 1 || pSoundLayer->m_Version > CMapItemLayerSounds::CURRENT_VERSION) + continue; + if(pSoundLayer->m_Sound < 0 || pSoundLayer->m_Sound >= m_Count || m_aSounds[pSoundLayer->m_Sound] == -1) + continue; + + const CSoundSource *pSources = static_cast(Layers()->Map()->GetDataSwapped(pSoundLayer->m_Data)); + if(!pSources) + continue; + + const size_t NumSources = minimum(pSoundLayer->m_NumSources, Layers()->Map()->GetDataSize(pSoundLayer->m_Data) / sizeof(CSoundSource)); + for(size_t SourceIndex = 0; SourceIndex < NumSources; SourceIndex++) { - CMapItemLayerSounds *pSoundLayer = (CMapItemLayerSounds *)pLayer; - - if(pSoundLayer->m_Version < 1 || pSoundLayer->m_Version > CMapItemLayerSounds::CURRENT_VERSION) - continue; - - if(pSoundLayer->m_Sound == -1) - continue; - - CSoundSource *pSources = (CSoundSource *)Layers()->Map()->GetDataSwapped(pSoundLayer->m_Data); - - if(!pSources) - continue; - - for(int i = 0; i < pSoundLayer->m_NumSources; i++) - { - CSourceQueueEntry Source; - Source.m_Sound = pSoundLayer->m_Sound; - Source.m_pSource = &pSources[i]; - Source.m_HighDetail = pLayer->m_Flags & LAYERFLAG_DETAIL; - - if(!Source.m_pSource || Source.m_Sound < 0 || Source.m_Sound >= m_Count) - continue; - - m_vSourceQueue.push_back(Source); - } + CSourceQueueEntry Source; + Source.m_Sound = pSoundLayer->m_Sound; + Source.m_HighDetail = pLayer->m_Flags & LAYERFLAG_DETAIL; + Source.m_pGroup = pGroup; + Source.m_pSource = &pSources[SourceIndex]; + m_vSourceQueue.push_back(Source); } } } @@ -190,66 +188,31 @@ void CMapSounds::OnRender() } } - vec2 Center = m_pClient->m_Camera.m_Center; - for(int g = 0; g < Layers()->NumGroups(); g++) + const vec2 Center = m_pClient->m_Camera.m_Center; + for(const auto &Source : m_vSourceQueue) { - CMapItemGroup *pGroup = Layers()->GetGroup(g); - - if(!pGroup) + if(!Source.m_Voice.IsValid()) continue; - for(int l = 0; l < pGroup->m_NumLayers; l++) + ColorRGBA Position = ColorRGBA(0.0f, 0.0f, 0.0f, 0.0f); + CMapLayers::EnvelopeEval(Source.m_pSource->m_PosEnvOffset, Source.m_pSource->m_PosEnv, Position, 2, &m_pClient->m_MapLayersBackground); + + float x = fx2f(Source.m_pSource->m_Position.x) + Position.r; + float y = fx2f(Source.m_pSource->m_Position.y) + Position.g; + + x += Center.x * (1.0f - Source.m_pGroup->m_ParallaxX / 100.0f); + y += Center.y * (1.0f - Source.m_pGroup->m_ParallaxY / 100.0f); + + x -= Source.m_pGroup->m_OffsetX; + y -= Source.m_pGroup->m_OffsetY; + + Sound()->SetVoicePosition(Source.m_Voice, vec2(x, y)); + + ColorRGBA Volume = ColorRGBA(1.0f, 0.0f, 0.0f, 0.0f); + CMapLayers::EnvelopeEval(Source.m_pSource->m_SoundEnvOffset, Source.m_pSource->m_SoundEnv, Volume, 1, &m_pClient->m_MapLayersBackground); + if(Volume.r < 1.0f) { - CMapItemLayer *pLayer = Layers()->GetLayer(pGroup->m_StartLayer + l); - - if(!pLayer) - continue; - - if(pLayer->m_Type == LAYERTYPE_SOUNDS) - { - CMapItemLayerSounds *pSoundLayer = (CMapItemLayerSounds *)pLayer; - - if(pSoundLayer->m_Version < 1 || pSoundLayer->m_Version > CMapItemLayerSounds::CURRENT_VERSION) - continue; - - CSoundSource *pSources = (CSoundSource *)Layers()->Map()->GetDataSwapped(pSoundLayer->m_Data); - - if(!pSources) - continue; - - for(int s = 0; s < pSoundLayer->m_NumSources; s++) - { - for(auto &Voice : m_vSourceQueue) - { - if(Voice.m_pSource != &pSources[s]) - continue; - - if(!Voice.m_Voice.IsValid()) - continue; - - ColorRGBA Position = ColorRGBA(0.0f, 0.0f, 0.0f, 0.0f); - CMapLayers::EnvelopeEval(Voice.m_pSource->m_PosEnvOffset, Voice.m_pSource->m_PosEnv, Position, 2, &m_pClient->m_MapLayersBackground); - - float x = fx2f(Voice.m_pSource->m_Position.x) + Position.r; - float y = fx2f(Voice.m_pSource->m_Position.y) + Position.g; - - x += Center.x * (1.0f - pGroup->m_ParallaxX / 100.0f); - y += Center.y * (1.0f - pGroup->m_ParallaxY / 100.0f); - - x -= pGroup->m_OffsetX; - y -= pGroup->m_OffsetY; - - Sound()->SetVoicePosition(Voice.m_Voice, vec2(x, y)); - - ColorRGBA Volume = ColorRGBA(1.0f, 0.0f, 0.0f, 0.0f); - CMapLayers::EnvelopeEval(Voice.m_pSource->m_SoundEnvOffset, Voice.m_pSource->m_SoundEnv, Volume, 1, &m_pClient->m_MapLayersBackground); - if(Volume.r < 1.0f) - { - Sound()->SetVoiceVolume(Voice.m_Voice, Volume.r); - } - } - } - } + Sound()->SetVoiceVolume(Source.m_Voice, Volume.r); } } } diff --git a/src/game/client/components/mapsounds.h b/src/game/client/components/mapsounds.h index ffca022e5..d5e8b1fbb 100644 --- a/src/game/client/components/mapsounds.h +++ b/src/game/client/components/mapsounds.h @@ -1,32 +1,28 @@ #ifndef GAME_CLIENT_COMPONENTS_MAPSOUNDS_H #define GAME_CLIENT_COMPONENTS_MAPSOUNDS_H -#include - #include #include #include -struct CSoundSource; +#include class CMapSounds : public CComponent { int m_aSounds[MAX_MAPSOUNDS]; int m_Count; - struct CSourceQueueEntry + class CSourceQueueEntry { + public: int m_Sound; bool m_HighDetail; ISound::CVoiceHandle m_Voice; - CSoundSource *m_pSource; - - bool operator==(const CSourceQueueEntry &Other) const { return (m_Sound == Other.m_Sound) && (m_Voice == Other.m_Voice) && (m_pSource == Other.m_pSource); } + const CMapItemGroup *m_pGroup; + const CSoundSource *m_pSource; }; - std::vector m_vSourceQueue; - void Clear(); public: