From 1e2d00598d1e31ecc6d011f901a0fcccc39ce19d Mon Sep 17 00:00:00 2001 From: Ivan Mogilko Date: Sat, 2 Mar 2024 11:52:35 +0300 Subject: [PATCH] Engine: rewrote audio_core to return a locked AudioPlayer Although the original idea of "audio core" interface was to have everything behind the set of functions, in practice it proved to be rather annoying, as we had to have a duplicate function per each "audio player" action. This increases amount of necessary code and creates a mostly redundant layer of complexity. Besides that, there's a technical issue: each "audio core" function's call locks the mutex, and sometimes we need to do several things at once. This means either call several functions with a lock/unlock pair in a succession, or write more cumbersome helper functions that do a number of things under one lock/unlock pair. If calling several functions, the state of the audio player may actually change in between. (In fact, it may even get disposed in between, although this probably never happened in practice because of how our synchronization is organized.) The new design follows this outline: 1. AudioCoreSlot is picked out to a separate code unit and renamed to AudioPlayer. 2. Majority of audio core's playback control and state reading functions are REMOVED. 3. Instead, there's ONE function called audio_core_get_player() that returns a AudioPlayer's pointer wrapped into a AudioPlayerLock class, which holds a mutex lock, and unlocks on destruction. AudioPlayerLock class lets user to work with AudioPlayer, doing any manipulation necessary, and unlocks the provided mutex as soon as it gets out of scope. --- Engine/media/audio/audio_core.cpp | 121 +++++------------------------- Engine/media/audio/audio_core.h | 55 ++++++++------ Engine/media/audio/sound.cpp | 6 +- Engine/media/audio/soundclip.cpp | 35 +++++---- Engine/media/audio/soundclip.h | 2 +- 5 files changed, 77 insertions(+), 142 deletions(-) diff --git a/Engine/media/audio/audio_core.cpp b/Engine/media/audio/audio_core.cpp index 4397c029128..70487b3c696 100644 --- a/Engine/media/audio/audio_core.cpp +++ b/Engine/media/audio/audio_core.cpp @@ -130,6 +130,19 @@ void audio_core_shutdown() } +// ------------------------------------------------------------------------------------------------- +// AUDIO CORE CONFIG +// ------------------------------------------------------------------------------------------------- + +void audio_core_set_master_volume(float newvol) +{ + // TODO: review this later; how do we apply master volume + // if we use alternate audio output (e.g. from plugin)? + alListenerf(AL_GAIN, newvol); + dump_al_errors(); +} + + // ------------------------------------------------------------------------------------------------- // SLOTS // ------------------------------------------------------------------------------------------------- @@ -164,111 +177,15 @@ int audio_core_slot_init(std::unique_ptr in, const String &extension_hin return audio_core_slot_init(std::move(decoder)); } -// ------------------------------------------------------------------------------------------------- -// SLOT CONTROL -// ------------------------------------------------------------------------------------------------- - -PlaybackState audio_core_slot_play(int slot_handle) +AudioPlayerLock audio_core_get_player(int slot_handle) { - std::lock_guard lk(g_acore.mixer_mutex_m); - g_acore.slots_[slot_handle]->Play(); - auto state = g_acore.slots_[slot_handle]->GetPlayState(); - g_acore.mixer_cv.notify_all(); - return state; + std::unique_lock ulk(g_acore.mixer_mutex_m); + auto it = g_acore.slots_.find(slot_handle); + if (it == g_acore.slots_.end()) + return AudioPlayerLock(std::move(ulk), nullptr); + return AudioPlayerLock(std::move(ulk), it->second.get()); } -PlaybackState audio_core_slot_pause(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - g_acore.slots_[slot_handle]->Pause(); - auto state = g_acore.slots_[slot_handle]->GetPlayState(); - g_acore.mixer_cv.notify_all(); - return state; -} - -void audio_core_slot_stop(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - g_acore.slots_[slot_handle]->Stop(); - g_acore.slots_.erase(slot_handle); - g_acore.mixer_cv.notify_all(); -} - -void audio_core_slot_seek_ms(int slot_handle, float pos_ms) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - g_acore.slots_[slot_handle]->Seek(pos_ms); - g_acore.mixer_cv.notify_all(); -} - - -// ------------------------------------------------------------------------------------------------- -// SLOT CONFIG -// ------------------------------------------------------------------------------------------------- - -void audio_core_set_master_volume(float newvol) -{ - // TODO: review this later; how do we apply master volume - // if we use alternate audio output (e.g. from plugin)? - alListenerf(AL_GAIN, newvol); - dump_al_errors(); -} - -void audio_core_slot_configure(int slot_handle, float volume, float speed, float panning) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto *player = g_acore.slots_[slot_handle].get(); - player->SetVolume(volume); - player->SetSpeed(speed); - player->SetPanning(panning); -} - -// ------------------------------------------------------------------------------------------------- -// SLOT STATUS -// ------------------------------------------------------------------------------------------------- - -float audio_core_slot_get_pos_ms(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto pos = g_acore.slots_[slot_handle]->GetPositionMs(); - g_acore.mixer_cv.notify_all(); - return pos; -} - -float audio_core_slot_get_duration(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto dur = g_acore.slots_[slot_handle]->GetDurationMs(); - g_acore.mixer_cv.notify_all(); - return dur; -} - -int audio_core_slot_get_freq(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto dur = g_acore.slots_[slot_handle]->GetFrequency(); - g_acore.mixer_cv.notify_all(); - return dur; -} - -PlaybackState audio_core_slot_get_play_state(int slot_handle) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto state = g_acore.slots_[slot_handle]->GetPlayState(); - g_acore.mixer_cv.notify_all(); - return state; -} - -PlaybackState audio_core_slot_get_play_state(int slot_handle, float &pos_ms) -{ - std::lock_guard lk(g_acore.mixer_mutex_m); - auto state = g_acore.slots_[slot_handle]->GetPlayState(); - pos_ms = g_acore.slots_[slot_handle]->GetPositionMs(); - g_acore.mixer_cv.notify_all(); - return state; -} - - // ------------------------------------------------------------------------------------------------- // AUDIO PROCESSING // ------------------------------------------------------------------------------------------------- diff --git a/Engine/media/audio/audio_core.h b/Engine/media/audio/audio_core.h index 2978600bbae..3fe9ce05d9b 100644 --- a/Engine/media/audio/audio_core.h +++ b/Engine/media/audio/audio_core.h @@ -18,10 +18,38 @@ #ifndef __AGS_EE_MEDIA__AUDIOCORE_H #define __AGS_EE_MEDIA__AUDIOCORE_H #include +#include #include #include "media/audio/audiodefines.h" +#include "media/audio/audioplayer.h" #include "util/string.h" + +namespace AGS +{ +namespace Engine +{ + +class AudioPlayerLock +{ +public: + AudioPlayerLock(std::unique_lock &&ulk, AudioPlayer *player) + : _ulock(std::move(ulk)) + , _player(player) + { + } + + const AudioPlayer *operator ->() const { return _player; } + AudioPlayer *operator ->() { return _player; } + +private: + std::unique_lock _ulock; + AudioPlayer * const _player; +}; + +} // namespace Engine +} // namespace AGS + // Initializes audio core system; // starts polling on a background thread. void audio_core_init(/*config, soundlib*/); @@ -29,6 +57,10 @@ void audio_core_init(/*config, soundlib*/); // stops any associated threads. void audio_core_shutdown(); +// Audio core config +// Set new master volume, affects all slots +void audio_core_set_master_volume(float newvol); + // Audio slot controls: slots are abstract holders for a playback. // // Initializes playback on a free playback slot (reuses spare one or allocates new if there's none). @@ -36,31 +68,12 @@ void audio_core_shutdown(); int audio_core_slot_init(std::shared_ptr> &data, const AGS::Common::String &extension_hint, bool repeat); // Initializes playback streaming int audio_core_slot_init(std::unique_ptr in, const AGS::Common::String &extension_hint, bool repeat); -// Start playback on a slot -PlaybackState audio_core_slot_play(int slot_handle); -// Pause playback on a slot, resume with 'audio_core_slot_play' -PlaybackState audio_core_slot_pause(int slot_handle); -// Stop playback on a slot, disposes sound data, frees a slot -void audio_core_slot_stop(int slot_handle); -// Seek on a slot, new position in milliseconds -void audio_core_slot_seek_ms(int slot_handle, float pos_ms); +// Returns a AudioPlayer from the given slot, wrapped in a auto-locking struct. +AGS::Engine::AudioPlayerLock audio_core_get_player(int slot_handle); #if defined(AGS_DISABLE_THREADS) // polls the audio core if we have no threads, polled in Engine/ac/timer.cpp void audio_core_entry_poll(); #endif -// Audio core config -// Set new master volume, affects all slots -void audio_core_set_master_volume(float newvol); -// Sets up single playback parameters -void audio_core_slot_configure(int slot_handle, float volume, float speed, float panning); - -PlaybackState audio_core_slot_get_play_state(int slot_handle); -PlaybackState audio_core_slot_get_play_state(int slot_handle, float &pos_ms); -float audio_core_slot_get_pos_ms(int slot_handle); -// Returns sound duration in milliseconds -float audio_core_slot_get_duration(int slot_handle); -int audio_core_slot_get_freq(int slot_handle); - #endif // __AGS_EE_MEDIA__AUDIOCORE_H diff --git a/Engine/media/audio/sound.cpp b/Engine/media/audio/sound.cpp index 7a4a5803a75..95333188a20 100644 --- a/Engine/media/audio/sound.cpp +++ b/Engine/media/audio/sound.cpp @@ -25,6 +25,7 @@ #include "util/string_types.h" using namespace AGS::Common; +using namespace AGS::Engine; static int GuessSoundTypeFromExt(const String &extension) { @@ -157,11 +158,8 @@ SOUNDCLIP *load_sound_clip(const AssetPath &apath, const char *extension_hint, b if (slot < 0) { return nullptr; } const auto sound_type = GuessSoundTypeFromExt(ext_hint); - const auto lengthMs = (int)std::round(audio_core_slot_get_duration(slot)); - auto clip = new SOUNDCLIP(slot); + auto clip = new SOUNDCLIP(slot, sound_type); clip->repeat = loop; - clip->soundType = sound_type; - clip->lengthMs = lengthMs; return clip; } diff --git a/Engine/media/audio/soundclip.cpp b/Engine/media/audio/soundclip.cpp index cd94e556343..a1432517829 100644 --- a/Engine/media/audio/soundclip.cpp +++ b/Engine/media/audio/soundclip.cpp @@ -14,12 +14,12 @@ #include "media/audio/soundclip.h" #include "media/audio/audio_core.h" -SOUNDCLIP::SOUNDCLIP(int slot) +SOUNDCLIP::SOUNDCLIP(int slot, int snd_type) : slot_(slot) + , soundType(snd_type) { sourceClipID = -1; sourceClipType = 0; - soundType = 0; repeat = false; priority = 50; vol255 = 0; @@ -33,18 +33,19 @@ SOUNDCLIP::SOUNDCLIP(int slot) panning = 0; speed = 1000; - lengthMs = 0; state = PlaybackState::PlayStateInitial; pos = posMs = -1; paramsChanged = true; - freq = audio_core_slot_get_freq(slot_); + const auto player = audio_core_get_player(slot); + lengthMs = (int)std::round(player->GetDurationMs()); + freq = player->GetFrequency(); } SOUNDCLIP::~SOUNDCLIP() { if (slot_ >= 0) - audio_core_slot_stop(slot_); + audio_core_get_player(slot_)->Stop(); } int SOUNDCLIP::play() @@ -59,7 +60,9 @@ void SOUNDCLIP::pause() { if (!is_ready()) return; - state = audio_core_slot_pause(slot_); + auto player = audio_core_get_player(slot_); + player->Pause(); + state = player->GetPlayState(); } void SOUNDCLIP::resume() @@ -106,13 +109,13 @@ void SOUNDCLIP::seek(int pos_) void SOUNDCLIP::seek_ms(int pos_ms) { if (slot_ < 0) { return; } - audio_core_slot_pause(slot_); + auto player = audio_core_get_player(slot_); + player->Pause(); // TODO: for backward compatibility and MOD/XM music support // need to reimplement seeking to a position which units // are defined according to the sound type - audio_core_slot_seek_ms(slot_, (float)pos_ms); - float posms_f; - audio_core_slot_get_play_state(slot_, posms_f); + player->Seek((float)pos_ms); + float posms_f = player->GetPositionMs(); posMs = static_cast(posms_f); pos = posms_to_pos(posMs); } @@ -121,6 +124,7 @@ bool SOUNDCLIP::update() { if (!is_ready()) return false; + auto player = audio_core_get_player(slot_); if (paramsChanged) { auto vol_f = static_cast(get_final_volume()) / 255.0f; @@ -135,12 +139,14 @@ bool SOUNDCLIP::update() if (panning_f < -1.0f) { panning_f = -1.0f; } if (panning_f > 1.0f) { panning_f = 1.0f; } - audio_core_slot_configure(slot_, vol_f, speed_f, panning_f); + player->SetVolume(vol_f); + player->SetSpeed(speed_f); + player->SetPanning(panning_f); paramsChanged = false; } - float posms_f; - PlaybackState core_state = audio_core_slot_get_play_state(slot_, posms_f); + PlaybackState core_state = player->GetPlayState(); + float posms_f = player->GetPositionMs(); posMs = static_cast(posms_f); pos = posms_to_pos(posMs); if (state == core_state || IsPlaybackDone(core_state)) @@ -152,7 +158,8 @@ bool SOUNDCLIP::update() switch (state) { case PlaybackState::PlayStatePlaying: - state = audio_core_slot_play(slot_); + player->Play(); + state = player->GetPlayState(); break; default: /* do nothing */ break; diff --git a/Engine/media/audio/soundclip.h b/Engine/media/audio/soundclip.h index bb220d82b33..1201c99863c 100644 --- a/Engine/media/audio/soundclip.h +++ b/Engine/media/audio/soundclip.h @@ -41,7 +41,7 @@ class SOUNDCLIP final { public: - SOUNDCLIP(int slot); + SOUNDCLIP(int slot, int snd_type); ~SOUNDCLIP(); // TODO: move these to private