diff --git a/src/utility/Mic_Class.cpp b/src/utility/Mic_Class.cpp index aef60f1..285f4fd 100644 --- a/src/utility/Mic_Class.cpp +++ b/src/utility/Mic_Class.cpp @@ -708,18 +708,46 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { bool Mic_Class::begin(void) { + // _rec_sample_rate was written before _begun was released, so the + // acquire load makes this pair of reads safe without the lock. + if (_begun.load(std::memory_order_acquire) && _rec_sample_rate == _calc_rec_rate()) { return true; } + + // record() calls begin() lazily from whichever task gets there first, + // and both the setup and the sample-rate change tear the port down: two + // of these racing rip the live channel out from under the running task. + // One caller goes through at a time; the others wait for its outcome. + bool zero = false; + while (!_begin_lock.compare_exchange_strong(zero, true)) + { + zero = false; + vTaskDelay(1); + } + + bool res = true; if (_task_running) { auto rate = _calc_rec_rate(); - if (_rec_sample_rate == rate) + if (_rec_sample_rate != rate) { - return true; + do { vTaskDelay(1); } while (isRecording()); + end(); + _rec_sample_rate = rate; } - do { vTaskDelay(1); } while (isRecording()); - end(); - _rec_sample_rate = rate; } + if (!_task_running) + { + // Record the rate the port is being built for; without this the next + // begin() at the very same rate tears the port down and rebuilds it. + _rec_sample_rate = _calc_rec_rate(); + res = _begin_locked(); + } + _begin_lock.store(false); + return res; + } + + bool Mic_Class::_begin_locked(void) + { if (_task_semaphore == nullptr) { _task_semaphore = xSemaphoreCreateBinary(); } bool res = true; @@ -743,6 +771,7 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { // end() takes the driver and the callback back down; it still sees the // class as running, which is what lets it do that. if (!res) { end(); } + else { _begun.store(true, std::memory_order_release); } } return res; @@ -750,6 +779,7 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { void Mic_Class::end(void) { + _begun.store(false, std::memory_order_release); if (!_task_running) { return; } _task_running = false; if (_task_handle) diff --git a/src/utility/Mic_Class.hpp b/src/utility/Mic_Class.hpp index 343d6f7..751bb9a 100644 --- a/src/utility/Mic_Class.hpp +++ b/src/utility/Mic_Class.hpp @@ -185,6 +185,7 @@ namespace m5 static void mic_task(void* args); uint32_t _calc_rec_rate(void) const; + bool _begin_locked(void); esp_err_t _setup_i2s(void); bool _rec_raw(void* recdata, size_t array_len, bool flg_16bit, uint32_t sample_rate, bool stereo); @@ -196,6 +197,12 @@ namespace m5 int32_t _offset = 0; volatile bool _task_running = false; + /// begin() runs from whichever task records first, and setup starts by + /// tearing the port down - so only one call may go through. + std::atomic _begin_lock { false }; + /// True only once begin() has fully finished; the lock-free early return + /// keys on this, so a caller can never see a half-built port as ready. + std::atomic _begun { false }; #if defined (SDL_h_) SDL_Thread* _task_handle = nullptr; #else diff --git a/src/utility/Speaker_Class.cpp b/src/utility/Speaker_Class.cpp index 49d4727..d611abb 100644 --- a/src/utility/Speaker_Class.cpp +++ b/src/utility/Speaker_Class.cpp @@ -633,40 +633,73 @@ namespace m5 int ch_diff = ch_info->diff; size_t ch_index = ch_info->index; - wav_info_t* current_wav = &(ch_info->wavinfo[!ch_info->flip]); - wav_info_t* next_wav = &(ch_info->wavinfo[ ch_info->flip]); + wav_info_t* current_wav = &(ch_info->current); + bool flip = ch_info->flip.load(std::memory_order_relaxed); + uint8_t next_state = ch_info->wavinfo[flip].state.load(std::memory_order_acquire); size_t idx = 0; - if (current_wav->repeat == 0 || next_wav->stop_current) + if (current_wav->repeat == 0 + || ((next_state & (wav_phase_mask | wav_state_stop_current)) == (wav_phase_published | wav_state_stop_current))) { label_next_wav: - bool clear_idx = (next_wav->repeat == 0 - || !next_wav->no_clear_index - || (next_wav->data != current_wav->data)); - current_wav->clear(); - ch_info->flip = !ch_info->flip; + next_state = ch_info->wavinfo[flip].state.load(std::memory_order_acquire); + if ((next_state & wav_phase_mask) == wav_phase_published + && ch_info->wavinfo[flip].state.compare_exchange_strong(next_state + , (uint8_t)((next_state & ~wav_phase_mask) | wav_phase_playing) + , std::memory_order_acquire, std::memory_order_relaxed)) + { // the claim above is what makes the payload of the slot readable. + wav_info_t& incoming = ch_info->wavinfo[flip].info; + bool clear_idx = ((next_state & wav_state_stop_marker) + || !incoming.no_clear_index + || (incoming.data != current_wav->data)); + *current_wav = incoming; + if (next_state & wav_state_stop_marker) + { // a pure stop: nothing to play. The slot itself is retired + // further below, once flip has moved off of it - freeing it + // here would let a writer claim it while flip still points at + // it, and the later retirement would wipe that claim out. + current_wav->clear(); + } + // the finished (or cut) request goes back to the writers before + // flip moves, so a claim through the fresh flip cannot miss it; + // the wakeup comes last so a woken writer finds flip already moved. + ch_info->wavinfo[!flip].state.store(wav_phase_empty, std::memory_order_release); + ch_info->flip.store(!flip, std::memory_order_relaxed); + flip = !flip; #if !defined (SDL_h_) - xSemaphoreGive(self->_task_semaphore); + xSemaphoreGive(self->_task_semaphore); #endif - std::swap(current_wav, next_wav); - if (clear_idx) - { - ch_index = 0; - if (current_wav->repeat == 0) + if (clear_idx) { - self->_play_channel_bits.fetch_and(~(1 << ch)); - if (current_wav->repeat == 0) - { - ch_info->diff = 0; - ch_info->index = 0; - continue; - } - self->_play_channel_bits.fetch_or(1 << ch); + ch_index = 0; } } + else if (current_wav->repeat != 0) + { // a writer snatched the request away first: the current sound + // stays until whatever they are publishing arrives. + goto label_play; + } + if (current_wav->repeat == 0) + { + ch_info->wavinfo[!flip].state.store(wav_phase_empty, std::memory_order_release); +#if !defined (SDL_h_) + xSemaphoreGive(self->_task_semaphore); +#endif + self->_play_channel_bits.fetch_and(~(1 << ch)); + next_state = ch_info->wavinfo[flip].state.load(std::memory_order_acquire); + if ((next_state & wav_phase_mask) != wav_phase_published) + { // nothing to do; a writer caught mid-publish raises the bit itself. + ch_info->diff = 0; + ch_info->index = 0; + continue; + } + self->_play_channel_bits.fetch_or(1 << ch); + goto label_next_wav; + } } +label_play: auto data = (const uint8_t*)current_wav->data; const bool in_stereo = current_wav->is_stereo; const int32_t in_rate = current_wav->sample_rate_x256; @@ -915,7 +948,23 @@ namespace m5 bool Speaker_Class::begin(void) { - if (_task_running) { return true; } + if (_begun.load(std::memory_order_acquire)) { return true; } + + // Playback calls begin() lazily from whichever task gets there first, + // and _setup_i2s starts by uninstalling the port: two of these racing + // rip the live channel out from under the playback task. One caller + // goes through at a time; the others wait and find the work done. + bool zero = false; + while (!_begin_lock.compare_exchange_strong(zero, true)) + { + zero = false; +#if defined (SDL_h_) + SDL_Delay(1); +#else + vTaskDelay(1); +#endif + } + if (_begun.load(std::memory_order_acquire)) { _begin_lock.store(false); return true; } #if !defined (SDL_h_) if (_task_semaphore == nullptr) { _task_semaphore = xSemaphoreCreateBinary(); } @@ -948,18 +997,23 @@ namespace m5 // end() takes the driver and the callback back down; it still sees the // class as running, which is what lets it do that. if (!res) { end(); } + else { _begun.store(true, std::memory_order_release); } } + _begin_lock.store(false); return res; } void Speaker_Class::end(void) { + _begun.store(false, std::memory_order_release); if (_cb_set_enabled) { _cb_set_enabled(_cb_set_enabled_args, false); } if (_task_running) { + // No stop() here: it would publish markers and notify a task that may + // already be tearing its handle down. The slots are reset below once + // the task is gone, which is all the stop would have achieved. _task_running = false; - stop(); if (_task_handle) { #if defined (SDL_h_) @@ -975,8 +1029,11 @@ namespace m5 for (size_t ch = 0; ch < sound_channel_max; ++ch) { auto chinfo = &_ch_info[ch]; - chinfo->wavinfo[0].clear(); - chinfo->wavinfo[1].clear(); + chinfo->wavinfo[0].info.clear(); + chinfo->wavinfo[0].state.store(wav_phase_empty); + chinfo->wavinfo[1].info.clear(); + chinfo->wavinfo[1].state.store(wav_phase_empty); + chinfo->current.clear(); } #if !defined (SDL_h_) _i2s_driver_uninstall(_cfg.i2s_port); @@ -987,10 +1044,10 @@ namespace m5 { wav_info_t tmp; tmp.stop_current = 1; + uint8_t bits = _play_channel_bits.load(); for (size_t ch = 0; ch < sound_channel_max; ++ch) { - auto chinfo = &_ch_info[ch]; - chinfo->wavinfo[chinfo->flip] = tmp; + if (bits & (1 << ch)) { _set_next_wav(ch, tmp); } } } @@ -1000,12 +1057,11 @@ namespace m5 { stop(); } - else + else if (_play_channel_bits.load() & (1 << ch)) { wav_info_t tmp; tmp.stop_current = 1; - auto chinfo = &_ch_info[ch]; - chinfo->wavinfo[chinfo->flip] = tmp; + _set_next_wav(ch, tmp); } } @@ -1022,25 +1078,55 @@ namespace m5 { auto chinfo = &_ch_info[ch]; uint8_t chmask = 1 << ch; - if (!wav.stop_current) + const uint8_t claimed = wav_phase_writing | ((wav.repeat == 0) ? wav_state_stop_marker : 0); + for (;;) { - while ((_play_channel_bits.load() & chmask) && (chinfo->wavinfo[chinfo->flip].repeat)) + bool f = chinfo->flip.load(std::memory_order_relaxed); + auto slot = &(chinfo->wavinfo[f]); + uint8_t st = slot->state.load(std::memory_order_relaxed); + uint8_t phase = st & wav_phase_mask; + // a preempting request may take a queued one's place; anything else + // needs the slot back from the task first. + if (phase == wav_phase_empty || (wav.stop_current && phase == wav_phase_published)) { - if (chinfo->wavinfo[!chinfo->flip].repeat == ~0u) { return false; } + if (slot->state.compare_exchange_strong(st, claimed + , std::memory_order_acquire, std::memory_order_relaxed)) + { + // holding the claim pins flip: the task cannot adopt a slot in + // writing. So if flip already points elsewhere, this was the stale + // slot - put it back exactly as found and take the fresh target. + if (chinfo->flip.load(std::memory_order_relaxed) != f) + { + slot->state.store(st, std::memory_order_release); + continue; + } + slot->info = wav; + slot->state.store(wav_phase_published + | (wav.stop_current ? wav_state_stop_current : 0) + | (wav.repeat == ~0u ? wav_state_infinite : 0) + | (wav.repeat == 0 ? wav_state_stop_marker : 0) + , std::memory_order_release); + _play_channel_bits.fetch_or(chmask); #if !defined (SDL_h_) - xSemaphoreTake(_task_semaphore, 1); -#else - SDL_Delay(1); + xTaskNotifyGive(_task_handle); #endif + return true; + } + continue; // lost the claim to whoever changed the state; they made + // progress, so trying again right away cannot spin for long. + } + if (!wav.stop_current + && ((chinfo->wavinfo[!f].state.load(std::memory_order_relaxed) + & (wav_phase_mask | wav_state_infinite)) == (wav_phase_playing | wav_state_infinite))) + { // never a turn behind an endless request. + return false; } - } - chinfo->wavinfo[chinfo->flip] = wav; - _play_channel_bits.fetch_or(chmask); - #if !defined (SDL_h_) - xTaskNotifyGive(_task_handle); + xSemaphoreTake(_task_semaphore, 1); +#else + SDL_Delay(1); #endif - return true; + } } bool Speaker_Class::_play_raw(const void* data, size_t array_len, bool flg_16bit, bool flg_signed, float sample_rate, bool flg_stereo, uint32_t repeat_count, int channel, bool stop_current_sound, bool no_clear_index) diff --git a/src/utility/Speaker_Class.hpp b/src/utility/Speaker_Class.hpp index d8c7681..aa48a5a 100644 --- a/src/utility/Speaker_Class.hpp +++ b/src/utility/Speaker_Class.hpp @@ -110,7 +110,7 @@ namespace m5 /// now in playing or not. /// @param channel virtual channel number. (0~7), (default = automatically selected) /// @return 0=not playing / 1=playing (There's room in the queue) / 2=playing (There's no room in the queue.) - size_t isPlaying(uint8_t channel) const volatile { return (channel < sound_channel_max) ? ((bool)_ch_info[channel].wavinfo[0].repeat) + ((bool)_ch_info[channel].wavinfo[1].repeat) : 0; } + size_t isPlaying(uint8_t channel) const volatile { return (channel < sound_channel_max) ? _slot_occupied(_ch_info[channel].wavinfo[0]) + _slot_occupied(_ch_info[channel].wavinfo[1]) : 0; } /// Get the number of channels that are playing. /// @return number of channels that are playing. @@ -243,13 +243,13 @@ namespace m5 struct wav_info_t { - volatile uint32_t repeat = 0; /// -1 mean infinity repeat + uint32_t repeat = 0; /// -1 mean infinity repeat uint32_t sample_rate_x256 = 0; const void* data = nullptr; size_t length = 0; union { - volatile uint8_t flg = 0; + uint8_t flg = 0; struct { uint8_t is_stereo : 1; @@ -262,13 +262,43 @@ namespace m5 void clear(void); }; + // A slot moves empty -> writing -> published (owned by a writer), then + // published -> playing -> empty (owned by the task). Both claims are a CAS + // on the state byte, so no one ever reads or writes the payload of a slot + // someone else holds. The flags of the request ride along in the same byte + // for the decisions that must not look at the payload: cutting the current + // sound, refusing to queue behind an endless one, counting isPlaying(). + static constexpr uint8_t wav_phase_mask = 0x03; + static constexpr uint8_t wav_phase_empty = 0x00; + static constexpr uint8_t wav_phase_writing = 0x01; + static constexpr uint8_t wav_phase_published = 0x02; + static constexpr uint8_t wav_phase_playing = 0x03; + static constexpr uint8_t wav_state_stop_current = 0x04; + static constexpr uint8_t wav_state_infinite = 0x08; + static constexpr uint8_t wav_state_stop_marker = 0x10; + + struct wav_slot_t + { + wav_info_t info; + std::atomic state { 0 }; + }; + + static bool _slot_occupied(const volatile wav_slot_t& slot) + { + uint8_t s = slot.state.load(std::memory_order_relaxed); + return ((s & wav_phase_mask) != wav_phase_empty) && !(s & wav_state_stop_marker); + } + struct channel_info_t { - wav_info_t wavinfo[2]; // current/next flip info. + wav_slot_t wavinfo[2]; // request queue slots. + wav_info_t current; // the request being played; only the task touches it. size_t index = 0; int diff = 0; volatile uint8_t volume = 255; // channel volume (not master volume) - volatile bool flip = false; + /// Which slot the next request goes into. The task moves it as it adopts + /// a request; a writer reloads it whenever its claim fails. + std::atomic flip { false }; float liner_buf[2][2] = { { 0, 0 }, { 0, 0 } }; }; @@ -289,6 +319,12 @@ namespace m5 volatile bool _task_running = false; std::atomic _play_channel_bits = { 0 }; + /// begin() runs from whichever task touches the speaker first, and setup + /// starts by tearing the port down - so only one call may go through. + std::atomic _begin_lock { false }; + /// True only once begin() has fully finished; the lock-free early return + /// keys on this, so a caller can never see a half-built port as ready. + std::atomic _begun { false }; #if defined (SDL_h_) SDL_Thread* _task_handle = nullptr; #else