Fix: Data race on effect volume setting with mixer thread

OpenTTD · Nov 7, 2022 · ec02451 · ec02451
1 parent e6e3b87
commit ec02451
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 5 deletions.
diff --git 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<uint8> _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
@@ -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
@@ -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<NWidgetBase>(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
@@ -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;