From 0fae0599b74fc3984b9a5b4c59719ff1019c8ec0 Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Mon, 7 Nov 2022 21:26:03 +0000 Subject: [PATCH] Fix: Data race on effect volume setting with mixer thread --- src/mixer.cpp | 13 ++++++++++--- src/mixer.h | 2 ++ src/music_gui.cpp | 7 ++++++- src/openttd.cpp | 4 +++- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/mixer.cpp b/src/mixer.cpp index 2d8afa3f14..b862353383 100644 --- a/src/mixer.cpp +++ b/src/mixer.cpp @@ -41,6 +41,7 @@ static uint32 _play_rate = 11025; static uint32 _max_size = UINT_MAX; static MxStreamCallback _music_stream = nullptr; static std::mutex _music_stream_mutex; +static std::atomic _effect_vol; /** * The theoretical maximum volume for a single sound sample. Multiple sound @@ -162,9 +163,10 @@ void MxMixSamples(void *buffer, uint samples) * perceived difference in loudness to better match expectations. effect_vol * is expected to be in the range 0-127 hence the division by 127 * 127 to * get back into range. */ - uint8 effect_vol = (_settings_client.music.effect_vol * - _settings_client.music.effect_vol * - _settings_client.music.effect_vol) / (127 * 127); + uint8 effect_vol_setting = _effect_vol.load(std::memory_order_relaxed); + uint8 effect_vol = (effect_vol_setting * + effect_vol_setting * + effect_vol_setting) / (127 * 127); /* Mix each channel */ uint8 active = _active_channels.load(std::memory_order_acquire); @@ -255,3 +257,8 @@ bool MxInitialize(uint rate) _music_stream = nullptr; /* rate may have changed, any music source is now invalid */ return true; } + +void SetEffectVolume(uint8 volume) +{ + _effect_vol.store(volume, std::memory_order_relaxed); +} diff --git a/src/mixer.h b/src/mixer.h index bd7b18a44a..51669da71c 100644 --- a/src/mixer.h +++ b/src/mixer.h @@ -30,4 +30,6 @@ void MxActivateChannel(MixerChannel*); uint32 MxSetMusicSource(MxStreamCallback music_callback); +void SetEffectVolume(uint8 volume); + #endif /* MIXER_H */ diff --git a/src/music_gui.cpp b/src/music_gui.cpp index b4a4de5196..392ffcb636 100644 --- a/src/music_gui.cpp +++ b/src/music_gui.cpp @@ -27,6 +27,7 @@ #include "widgets/dropdown_func.h" #include "widgets/dropdown_type.h" #include "widgets/slider_func.h" +#include "mixer.h" #include "widgets/music_widget.h" @@ -788,7 +789,11 @@ struct MusicWindow : public Window { case WID_M_MUSIC_VOL: case WID_M_EFFECT_VOL: { // volume sliders byte &vol = (widget == WID_M_MUSIC_VOL) ? _settings_client.music.music_vol : _settings_client.music.effect_vol; if (ClickVolumeSliderWidget(this->GetWidget(widget)->GetCurrentRect(), pt, vol)) { - if (widget == WID_M_MUSIC_VOL) MusicDriver::GetInstance()->SetVolume(vol); + if (widget == WID_M_MUSIC_VOL) { + MusicDriver::GetInstance()->SetVolume(vol); + } else { + SetEffectVolume(vol); + } this->SetWidgetDirty(widget); SetWindowClassesDirty(WC_GAME_OPTIONS); } diff --git a/src/openttd.cpp b/src/openttd.cpp index 06ff78c713..1e7cd9ae0e 100644 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -13,6 +13,7 @@ #include "sound/sound_driver.hpp" #include "music/music_driver.hpp" #include "video/video_driver.hpp" +#include "mixer.h" #include "fontcache.h" #include "error.h" @@ -452,8 +453,9 @@ struct AfterNewGRFScan : NewGRFScanCallback { /* We have loaded the config, so we may possibly save it. */ _save_config = save_config; - /* restore saved music volume */ + /* restore saved music and effects volumes */ MusicDriver::GetInstance()->SetVolume(_settings_client.music.music_vol); + SetEffectVolume(_settings_client.music.effect_vol); if (startyear != INVALID_YEAR) IConsoleSetSetting("game_creation.starting_year", startyear); if (generation_seed != GENERATE_NEW_SEED) _settings_newgame.game_creation.generation_seed = generation_seed;