From 6c4937f495f1a1389c6f12cee4545e004c535514 Mon Sep 17 00:00:00 2001 From: Steven Noonan Date: Tue, 9 Mar 2021 04:43:59 -0800 Subject: [PATCH 1/2] engine: use atomics for setting live pitch and spatialization variables Thread Sanitizer was unhappy about these variables being modified on one thread and read on another (data race). Signed-off-by: Steven Noonan --- research/miniaudio_engine.h | 42 +++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/research/miniaudio_engine.h b/research/miniaudio_engine.h index 112f8d57..b3cd0469 100644 --- a/research/miniaudio_engine.h +++ b/research/miniaudio_engine.h @@ -1645,18 +1645,18 @@ MA_API ma_engine_node_config ma_engine_node_config_init(ma_engine* pEngine, ma_e /* Base node object for both ma_sound and ma_sound_group. */ typedef struct { - ma_node_base baseNode; /* Must be the first member for compatiblity with the ma_node API. */ - ma_engine* pEngine; /* A pointer to the engine. Set based on the value from the config. */ + ma_node_base baseNode; /* Must be the first member for compatiblity with the ma_node API. */ + ma_engine* pEngine; /* A pointer to the engine. Set based on the value from the config. */ ma_fader fader; - ma_resampler resampler; /* For pitch shift. May change this to ma_linear_resampler later. */ + ma_resampler resampler; /* For pitch shift. May change this to ma_linear_resampler later. */ ma_spatializer spatializer; ma_panner panner; - float pitch; - float oldPitch; /* For determining whether or not the resampler needs to be updated to reflect the new pitch. The resampler will be updated on the mixing thread. */ - float oldDopplerPitch; /* For determining whether or not the resampler needs to be updated to take a new doppler pitch into account. */ - ma_bool8 isPitchDisabled; /* When set to true, pitching will be disabled which will allow the resampler to be bypassed to save some computation. */ - ma_bool8 isSpatializationDisabled; /* Set to false by default. When set to false, will not have spatialisation applied. */ - ma_uint8 pinnedListenerIndex; /* The index of the listener this node should always use for spatialization. If set to (ma_uint8)-1 the engine will use the closest listener. */ + MA_ATOMIC float pitch; + float oldPitch; /* For determining whether or not the resampler needs to be updated to reflect the new pitch. The resampler will be updated on the mixing thread. */ + float oldDopplerPitch; /* For determining whether or not the resampler needs to be updated to take a new doppler pitch into account. */ + MA_ATOMIC ma_bool8 isPitchDisabled; /* When set to true, pitching will be disabled which will allow the resampler to be bypassed to save some computation. */ + MA_ATOMIC ma_bool8 isSpatializationDisabled; /* Set to false by default. When set to false, will not have spatialisation applied. */ + MA_ATOMIC ma_uint8 pinnedListenerIndex; /* The index of the listener this node should always use for spatialization. If set to (ma_uint8)-1 the engine will use the closest listener. */ } ma_engine_node; MA_API ma_result ma_engine_node_init(const ma_engine_node_config* pConfig, const ma_allocation_callbacks* pAllocationCallbacks, ma_engine_node* pEngineNode); @@ -9901,8 +9901,10 @@ static void ma_engine_node_update_pitch_if_required(ma_engine_node* pEngineNode) MA_ASSERT(pEngineNode != NULL); - if (pEngineNode->oldPitch != pEngineNode->pitch) { - pEngineNode->oldPitch = pEngineNode->pitch; + float newPitch = c89atomic_load_explicit_f32(&pEngineNode->pitch, c89atomic_memory_order_acquire); + + if (pEngineNode->oldPitch != newPitch) { + pEngineNode->oldPitch = newPitch; isUpdateRequired = MA_TRUE; } @@ -9921,14 +9923,14 @@ static ma_bool32 ma_engine_node_is_pitching_enabled(const ma_engine_node* pEngin MA_ASSERT(pEngineNode != NULL); /* Don't try to be clever by skiping resampling in the pitch=1 case or else you'll glitch when moving away from 1. */ - return !pEngineNode->isPitchDisabled; + return !c89atomic_load_explicit_8(&pEngineNode->isPitchDisabled, c89atomic_memory_order_acquire); } static ma_bool32 ma_engine_node_is_spatialization_enabled(const ma_engine_node* pEngineNode) { MA_ASSERT(pEngineNode != NULL); - return !pEngineNode->isSpatializationDisabled; + return !c89atomic_load_explicit_8(&pEngineNode->isSpatializationDisabled, c89atomic_memory_order_acquire); } static ma_uint64 ma_engine_node_get_required_input_frame_count(const ma_engine_node* pEngineNode, ma_uint64 outputFrameCount) @@ -11196,7 +11198,7 @@ MA_API ma_result ma_sound_set_pitch(ma_sound* pSound, float pitch) return MA_INVALID_ARGS; } - pSound->engineNode.pitch = pitch; + c89atomic_exchange_explicit_f32(&pSound->engineNode.pitch, pitch, c89atomic_memory_order_release); return MA_SUCCESS; } @@ -11225,7 +11227,7 @@ MA_API void ma_sound_set_spatialization_enabled(ma_sound* pSound, ma_bool32 enab return; } - pSound->engineNode.isSpatializationDisabled = !enabled; + c89atomic_exchange_explicit_8(&pSound->engineNode.isSpatializationDisabled, !enabled, c89atomic_memory_order_release); } MA_API void ma_sound_set_pinned_listener_index(ma_sound* pSound, ma_uint8 listenerIndex) @@ -11234,7 +11236,7 @@ MA_API void ma_sound_set_pinned_listener_index(ma_sound* pSound, ma_uint8 listen return; } - pSound->engineNode.pinnedListenerIndex = listenerIndex; + c89atomic_exchange_explicit_8(&pSound->engineNode.pinnedListenerIndex, listenerIndex, c89atomic_memory_order_release); } MA_API ma_uint8 ma_sound_get_pinned_listener_index(const ma_sound* pSound) @@ -11243,7 +11245,7 @@ MA_API ma_uint8 ma_sound_get_pinned_listener_index(const ma_sound* pSound) return (ma_uint8)-1; } - return pSound->engineNode.pinnedListenerIndex; + return c89atomic_load_explicit_8(&pSound->engineNode.pinnedListenerIndex, c89atomic_memory_order_acquire); } MA_API void ma_sound_set_position(ma_sound* pSound, float x, float y, float z) @@ -11791,7 +11793,7 @@ MA_API void ma_sound_group_set_spatialization_enabled(ma_sound_group* pGroup, ma return; } - pGroup->engineNode.isSpatializationDisabled = !enabled; + c89atomic_exchange_explicit_8(&pGroup->engineNode.isSpatializationDisabled, !enabled, c89atomic_memory_order_release); } MA_API void ma_sound_group_set_pinned_listener_index(ma_sound_group* pGroup, ma_uint8 listenerIndex) @@ -11800,7 +11802,7 @@ MA_API void ma_sound_group_set_pinned_listener_index(ma_sound_group* pGroup, ma_ return; } - pGroup->engineNode.pinnedListenerIndex = listenerIndex; + c89atomic_exchange_explicit_8(&pGroup->engineNode.pinnedListenerIndex, listenerIndex, c89atomic_memory_order_release); } MA_API ma_uint8 ma_sound_group_get_pinned_listener_index(const ma_sound_group* pGroup) @@ -11809,7 +11811,7 @@ MA_API ma_uint8 ma_sound_group_get_pinned_listener_index(const ma_sound_group* p return (ma_uint8)-1; } - return pGroup->engineNode.pinnedListenerIndex; + return c89atomic_load_explicit_8(&pGroup->engineNode.pinnedListenerIndex, c89atomic_memory_order_acquire); } MA_API void ma_sound_group_set_position(ma_sound_group* pGroup, float x, float y, float z) From 7a1cc44170cd36d1163a832c7d6b19c02c0d6b11 Mon Sep 17 00:00:00 2001 From: Steven Noonan Date: Tue, 9 Mar 2021 10:52:09 -0800 Subject: [PATCH 2/2] atomics: use "const" pointers for the load-only atomics on MSVC Signed-off-by: Steven Noonan --- miniaudio.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miniaudio.h b/miniaudio.h index fb666587..d7a04c49 100644 --- a/miniaudio.h +++ b/miniaudio.h @@ -8720,28 +8720,28 @@ typedef unsigned char c89atomic_bool; #define c89atomic_compiler_fence() c89atomic_thread_fence(c89atomic_memory_order_seq_cst) #define c89atomic_signal_fence(order) c89atomic_thread_fence(order) #if defined(C89ATOMIC_HAS_8) - static C89ATOMIC_INLINE c89atomic_uint8 c89atomic_load_explicit_8(volatile c89atomic_uint8* ptr, c89atomic_memory_order order) + static C89ATOMIC_INLINE c89atomic_uint8 c89atomic_load_explicit_8(volatile const c89atomic_uint8* ptr, c89atomic_memory_order order) { (void)order; return c89atomic_compare_and_swap_8(ptr, 0, 0); } #endif #if defined(C89ATOMIC_HAS_16) - static C89ATOMIC_INLINE c89atomic_uint16 c89atomic_load_explicit_16(volatile c89atomic_uint16* ptr, c89atomic_memory_order order) + static C89ATOMIC_INLINE c89atomic_uint16 c89atomic_load_explicit_16(volatile const c89atomic_uint16* ptr, c89atomic_memory_order order) { (void)order; return c89atomic_compare_and_swap_16(ptr, 0, 0); } #endif #if defined(C89ATOMIC_HAS_32) - static C89ATOMIC_INLINE c89atomic_uint32 c89atomic_load_explicit_32(volatile c89atomic_uint32* ptr, c89atomic_memory_order order) + static C89ATOMIC_INLINE c89atomic_uint32 c89atomic_load_explicit_32(volatile const c89atomic_uint32* ptr, c89atomic_memory_order order) { (void)order; return c89atomic_compare_and_swap_32(ptr, 0, 0); } #endif #if defined(C89ATOMIC_HAS_64) - static C89ATOMIC_INLINE c89atomic_uint64 c89atomic_load_explicit_64(volatile c89atomic_uint64* ptr, c89atomic_memory_order order) + static C89ATOMIC_INLINE c89atomic_uint64 c89atomic_load_explicit_64(volatile const c89atomic_uint64* ptr, c89atomic_memory_order order) { (void)order; return c89atomic_compare_and_swap_64(ptr, 0, 0);