From e51f474613c5d4bd53a8d213785bcb51f5cf447f Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 9 Jul 2022 22:32:24 +0200 Subject: [PATCH] Sounds: Various little improvements (#12486) Use SimpleSoundSpec where reasonable (OpenAL) Ensure the sound IDs do not underflow or get overwritten -> loop in u16 Proper use of an enum. --- src/client/sound.h | 28 ++-------- src/client/sound_openal.cpp | 82 +++++++++++++++++------------ src/gui/guiFormSpecMenu.cpp | 8 +-- src/network/clientpackethandler.cpp | 22 ++++---- src/script/common/c_content.cpp | 4 +- src/script/lua_api/l_client.cpp | 26 ++++----- src/server.cpp | 36 +++++++------ src/server.h | 6 +-- src/sound.h | 8 +++ 9 files changed, 112 insertions(+), 108 deletions(-) diff --git a/src/client/sound.h b/src/client/sound.h index a3bb3ce87..213d20831 100644 --- a/src/client/sound.h +++ b/src/client/sound.h @@ -51,10 +51,8 @@ public: // playSound functions return -1 on failure, otherwise a handle to the // sound. If name=="", call should be ignored without error. - virtual int playSound(const std::string &name, bool loop, float volume, - float fade = 0.0f, float pitch = 1.0f) = 0; - virtual int playSoundAt(const std::string &name, bool loop, float volume, v3f pos, - float pitch = 1.0f) = 0; + virtual int playSound(const SimpleSoundSpec &spec) = 0; + virtual int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) = 0; virtual void stopSound(int sound) = 0; virtual bool soundExists(int sound) = 0; virtual void updateSoundPosition(int sound, v3f pos) = 0; @@ -62,15 +60,6 @@ public: virtual float getSoundGain(int id) = 0; virtual void step(float dtime) = 0; virtual void fadeSound(int sound, float step, float gain) = 0; - - int playSound(const SimpleSoundSpec &spec) - { - return playSound(spec.name, spec.loop, spec.gain, spec.fade, spec.pitch); - } - int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) - { - return playSoundAt(spec.name, spec.loop, spec.gain, pos, spec.pitch); - } }; class DummySoundManager : public ISoundManager @@ -88,16 +77,9 @@ public: { } void setListenerGain(float gain) {} - int playSound(const std::string &name, bool loop, float volume, float fade, - float pitch) - { - return 0; - } - int playSoundAt(const std::string &name, bool loop, float volume, v3f pos, - float pitch) - { - return 0; - } + + int playSound(const SimpleSoundSpec &spec) { return -1; } + int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) { return -1; } void stopSound(int sound) {} bool soundExists(int sound) { return false; } void updateSoundPosition(int sound, v3f pos) {} diff --git a/src/client/sound_openal.cpp b/src/client/sound_openal.cpp index 0eda8842b..b015aba25 100644 --- a/src/client/sound_openal.cpp +++ b/src/client/sound_openal.cpp @@ -322,7 +322,7 @@ private: OnDemandSoundFetcher *m_fetcher; ALCdevice *m_device; ALCcontext *m_context; - int m_next_id; + u16 m_last_used_id = 0; // only access within getFreeId() ! std::unordered_map> m_buffers; std::unordered_map m_sounds_playing; struct FadeState { @@ -342,8 +342,7 @@ public: OpenALSoundManager(SoundManagerSingleton *smg, OnDemandSoundFetcher *fetcher): m_fetcher(fetcher), m_device(smg->m_device.get()), - m_context(smg->m_context.get()), - m_next_id(1) + m_context(smg->m_context.get()) { infostream << "Audio: Initialized: OpenAL " << std::endl; } @@ -379,6 +378,22 @@ public: infostream << "Audio: Deinitialized." << std::endl; } + u16 getFreeId() + { + u16 startid = m_last_used_id; + while (!isFreeId(++m_last_used_id)) { + if (m_last_used_id == startid) + return 0; + } + + return m_last_used_id; + } + + inline bool isFreeId(int id) const + { + return id > 0 && m_sounds_playing.find(id) == m_sounds_playing.end(); + } + void step(float dtime) { doFades(dtime); @@ -437,7 +452,7 @@ public: << std::endl; assert(buf); PlayingSound *sound = new PlayingSound; - assert(sound); + warn_if_error(alGetError(), "before createPlayingSoundAt"); alGenSources(1, &sound->source_id); alSourcei(sound->source_id, AL_BUFFER, buf->buffer_id); @@ -463,28 +478,17 @@ public: { assert(buf); PlayingSound *sound = createPlayingSound(buf, loop, volume, pitch); - if(!sound) + if (!sound) return -1; - int id = m_next_id++; - m_sounds_playing[id] = sound; - return id; - } - int playSoundRawAt(SoundBuffer *buf, bool loop, float volume, const v3f &pos, - float pitch) - { - assert(buf); - PlayingSound *sound = createPlayingSoundAt(buf, loop, volume, pos, pitch); - if(!sound) - return -1; - int id = m_next_id++; - m_sounds_playing[id] = sound; - return id; + int handle = getFreeId(); + m_sounds_playing[handle] = sound; + return handle; } void deleteSound(int id) { - std::unordered_map::iterator i = m_sounds_playing.find(id); + auto i = m_sounds_playing.find(id); if(i == m_sounds_playing.end()) return; PlayingSound *sound = i->second; @@ -580,39 +584,46 @@ public: alListenerf(AL_GAIN, gain); } - int playSound(const std::string &name, bool loop, float volume, float fade, float pitch) + int playSound(const SimpleSoundSpec &spec) { maintain(); - if (name.empty()) + if (spec.name.empty()) return 0; - SoundBuffer *buf = getFetchBuffer(name); + SoundBuffer *buf = getFetchBuffer(spec.name); if(!buf){ - infostream << "OpenALSoundManager: \"" << name << "\" not found." + infostream << "OpenALSoundManager: \"" << spec.name << "\" not found." << std::endl; return -1; } + int handle = -1; - if (fade > 0) { - handle = playSoundRaw(buf, loop, 0.0f, pitch); - fadeSound(handle, fade, volume); + if (spec.fade > 0) { + handle = playSoundRaw(buf, spec.loop, 0.0f, spec.pitch); + fadeSound(handle, spec.fade, spec.gain); } else { - handle = playSoundRaw(buf, loop, volume, pitch); + handle = playSoundRaw(buf, spec.loop, spec.gain, spec.pitch); } return handle; } - int playSoundAt(const std::string &name, bool loop, float volume, v3f pos, float pitch) + int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) { maintain(); - if (name.empty()) + if (spec.name.empty()) return 0; - SoundBuffer *buf = getFetchBuffer(name); - if(!buf){ - infostream << "OpenALSoundManager: \"" << name << "\" not found." + SoundBuffer *buf = getFetchBuffer(spec.name); + if (!buf) { + infostream << "OpenALSoundManager: \"" << spec.name << "\" not found." << std::endl; return -1; } - return playSoundRawAt(buf, loop, volume, pos, pitch); + + PlayingSound *sound = createPlayingSoundAt(buf, spec.loop, spec.gain, pos, spec.pitch); + if (!sound) + return -1; + int handle = getFreeId(); + m_sounds_playing[handle] = sound; + return handle; } void stopSound(int sound) @@ -624,8 +635,9 @@ public: void fadeSound(int soundid, float step, float gain) { // Ignore the command if step isn't valid. - if (step == 0) + if (step == 0 || soundid < 0) return; + float current_gain = getSoundGain(soundid); step = gain - current_gain > 0 ? abs(step) : -abs(step); if (m_sounds_fading.find(soundid) != m_sounds_fading.end()) { diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp index e4ea9ff58..342f74b16 100644 --- a/src/gui/guiFormSpecMenu.cpp +++ b/src/gui/guiFormSpecMenu.cpp @@ -4498,7 +4498,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event) if ((s.ftype == f_TabHeader) && (s.fid == event.GUIEvent.Caller->getID())) { if (!s.sound.empty() && m_sound_manager) - m_sound_manager->playSound(s.sound, false, 1.0f); + m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f)); s.send = true; acceptInput(); s.send = false; @@ -4543,7 +4543,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event) if (s.ftype == f_Button || s.ftype == f_CheckBox) { if (!s.sound.empty() && m_sound_manager) - m_sound_manager->playSound(s.sound, false, 1.0f); + m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f)); s.send = true; if (s.is_exit) { @@ -4568,7 +4568,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event) } } if (!s.sound.empty() && m_sound_manager) - m_sound_manager->playSound(s.sound, false, 1.0f); + m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f)); s.send = true; acceptInput(quit_mode_no); @@ -4586,7 +4586,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event) s.fdefault = L""; } else if (s.ftype == f_Unknown || s.ftype == f_HyperText) { if (!s.sound.empty() && m_sound_manager) - m_sound_manager->playSound(s.sound, false, 1.0f); + m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f)); s.send = true; acceptInput(); s.send = false; diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index fba0fe72d..1901c6675 100644 --- a/src/network/clientpackethandler.cpp +++ b/src/network/clientpackethandler.cpp @@ -820,12 +820,12 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt) s32 server_id; SimpleSoundSpec spec; - u8 type; // 0=local, 1=positional, 2=object + SoundLocation type; // 0=local, 1=positional, 2=object v3f pos; u16 object_id; bool ephemeral = false; - *pkt >> server_id >> spec.name >> spec.gain >> type >> pos >> object_id >> spec.loop; + *pkt >> server_id >> spec.name >> spec.gain >> (u8 &)type >> pos >> object_id >> spec.loop; try { *pkt >> spec.fade; @@ -836,22 +836,20 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt) // Start playing int client_id = -1; switch(type) { - case 0: // local - client_id = m_sound->playSound(spec); - break; - case 1: // positional - client_id = m_sound->playSoundAt(spec, pos); - break; - case 2: - { // object + case SoundLocation::Local: + client_id = m_sound->playSound(spec); + break; + case SoundLocation::Position: + client_id = m_sound->playSoundAt(spec, pos); + break; + case SoundLocation::Object: + { ClientActiveObject *cao = m_env.getActiveObject(object_id); if (cao) pos = cao->getPosition(); client_id = m_sound->playSoundAt(spec, pos); break; } - default: - break; } if (client_id != -1) { diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index 1f9f97781..5595f3b14 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -1056,7 +1056,7 @@ void read_server_sound_params(lua_State *L, int index, if(!lua_isnil(L, -1)){ v3f p = read_v3f(L, -1)*BS; params.pos = p; - params.type = ServerPlayingSound::SSP_POSITIONAL; + params.type = SoundLocation::Position; } lua_pop(L, 1); lua_getfield(L, index, "object"); @@ -1065,7 +1065,7 @@ void read_server_sound_params(lua_State *L, int index, ServerActiveObject *sao = ObjectRef::getobject(ref); if(sao){ params.object = sao->getId(); - params.type = ServerPlayingSound::SSP_OBJECT; + params.type = SoundLocation::Object; } } lua_pop(L, 1); diff --git a/src/script/lua_api/l_client.cpp b/src/script/lua_api/l_client.cpp index aaced7cd0..05ac53cbb 100644 --- a/src/script/lua_api/l_client.cpp +++ b/src/script/lua_api/l_client.cpp @@ -268,30 +268,32 @@ int ModApiClient::l_sound_play(lua_State *L) SimpleSoundSpec spec; read_soundspec(L, 1, spec); + SoundLocation type = SoundLocation::Local; float gain = 1.0f; - float pitch = 1.0f; - bool looped = false; - s32 handle; + v3f position; if (lua_istable(L, 2)) { getfloatfield(L, 2, "gain", gain); - getfloatfield(L, 2, "pitch", pitch); - getboolfield(L, 2, "loop", looped); + getfloatfield(L, 2, "pitch", spec.pitch); + getboolfield(L, 2, "loop", spec.loop); lua_getfield(L, 2, "pos"); if (!lua_isnil(L, -1)) { - v3f pos = read_v3f(L, -1) * BS; + position = read_v3f(L, -1) * BS; + type = SoundLocation::Position; lua_pop(L, 1); - handle = sound->playSoundAt( - spec.name, looped, gain * spec.gain, pos, pitch); - lua_pushinteger(L, handle); - return 1; } } - handle = sound->playSound(spec.name, looped, gain * spec.gain, spec.fade, pitch); - lua_pushinteger(L, handle); + spec.gain *= gain; + s32 handle; + if (type == SoundLocation::Local) + handle = sound->playSound(spec); + else + handle = sound->playSoundAt(spec, position); + + lua_pushinteger(L, handle); return 1; } diff --git a/src/server.cpp b/src/server.cpp index 24e352658..4eee14a30 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -138,21 +138,27 @@ void *ServerThread::run() v3f ServerPlayingSound::getPos(ServerEnvironment *env, bool *pos_exists) const { - if(pos_exists) *pos_exists = false; - switch(type){ - case SSP_LOCAL: + if (pos_exists) + *pos_exists = false; + + switch (type ){ + case SoundLocation::Local: return v3f(0,0,0); - case SSP_POSITIONAL: - if(pos_exists) *pos_exists = true; + case SoundLocation::Position: + if (pos_exists) + *pos_exists = true; return pos; - case SSP_OBJECT: { - if(object == 0) - return v3f(0,0,0); - ServerActiveObject *sao = env->getActiveObject(object); - if(!sao) - return v3f(0,0,0); - if(pos_exists) *pos_exists = true; - return sao->getBasePosition(); } + case SoundLocation::Object: + { + if (object == 0) + return v3f(0,0,0); + ServerActiveObject *sao = env->getActiveObject(object); + if (!sao) + return v3f(0,0,0); + if (pos_exists) + *pos_exists = true; + return sao->getBasePosition(); + } } return v3f(0,0,0); } @@ -2071,9 +2077,9 @@ s32 Server::playSound(ServerPlayingSound ¶ms, bool ephemeral) { // Find out initial position of sound bool pos_exists = false; - v3f pos = params.getPos(m_env, &pos_exists); + const v3f pos = params.getPos(m_env, &pos_exists); // If position is not found while it should be, cancel sound - if(pos_exists != (params.type != ServerPlayingSound::SSP_LOCAL)) + if(pos_exists != (params.type != SoundLocation::Local)) return -1; // Filter destination clients diff --git a/src/server.h b/src/server.h index 00b6f897d..cb7d77067 100644 --- a/src/server.h +++ b/src/server.h @@ -99,11 +99,7 @@ struct MediaInfo // Combines the pure sound (SimpleSoundSpec) with positional information struct ServerPlayingSound { - enum Type { - SSP_LOCAL, - SSP_POSITIONAL, - SSP_OBJECT - } type = SSP_LOCAL; + SoundLocation type = SoundLocation::Local; float gain = 1.0f; // for amplification of the base sound float max_hear_distance = 32 * BS; diff --git a/src/sound.h b/src/sound.h index ddb4e3dc2..801c552a9 100644 --- a/src/sound.h +++ b/src/sound.h @@ -59,3 +59,11 @@ struct SimpleSoundSpec float pitch = 1.0f; bool loop = false; }; + + +// The order must not be changed. This is sent over the network. +enum class SoundLocation : u8 { + Local, + Position, + Object +};