From f674e4509c1970692bcae3bfdfec180015579307 Mon Sep 17 00:00:00 2001 From: NP v/d Spek Date: Fri, 25 Oct 2024 12:22:30 +0200 Subject: [PATCH] Implement audio component locking and resolve issue with not playing all data. The finish method is now obsolute. --- esphome/components/i2s_audio/i2s_audio.cpp | 17 +++++++ esphome/components/i2s_audio/i2s_audio.h | 46 +++--------------- .../i2s_audio/speaker/i2s_audio_speaker.cpp | 48 ++++++++++--------- 3 files changed, 50 insertions(+), 61 deletions(-) diff --git a/esphome/components/i2s_audio/i2s_audio.cpp b/esphome/components/i2s_audio/i2s_audio.cpp index ad73b383fe..507e43cf52 100644 --- a/esphome/components/i2s_audio/i2s_audio.cpp +++ b/esphome/components/i2s_audio/i2s_audio.cpp @@ -28,6 +28,23 @@ void I2SAudioComponent::setup() { ESP_LOGCONFIG(TAG, "Setting up I2S Audio..."); } +bool I2SAudioComponent::lock_component(I2SAudioBase *audio) { + if (!this->is_compoment_locked(audio)) { + this->audio_base_ = audio; + return true; + } + return false; +} +void I2SAudioComponent::unlock_component(I2SAudioBase *audio) { + if (!this->is_compoment_locked(audio)) { + this->audio_base_ = nullptr; + } +} + +bool I2SAudioComponent::is_compoment_locked(I2SAudioBase *audio) { + return !(this->audio_base_ == nullptr || this->audio_base_ == audio); +} + } // namespace i2s_audio } // namespace esphome diff --git a/esphome/components/i2s_audio/i2s_audio.h b/esphome/components/i2s_audio/i2s_audio.h index 2f93b43caf..e39cade356 100644 --- a/esphome/components/i2s_audio/i2s_audio.h +++ b/esphome/components/i2s_audio/i2s_audio.h @@ -21,9 +21,6 @@ class I2SAudioBase : public Parented { void set_use_apll(uint32_t use_apll) { this->use_apll_ = use_apll; } protected: - virtual bool lock_component(uint32_t hash) = 0; - virtual void unlock_component(uint32_t hash) = 0; - i2s_mode_t i2s_mode_{}; i2s_channel_fmt_t channel_; uint32_t sample_rate_; @@ -32,37 +29,9 @@ class I2SAudioBase : public Parented { bool use_apll_; }; -class I2SAudioIn : public I2SAudioBase { - protected: - void lock_component(uint32_t hash) overload { - if (this->parent_->component_in_lock_hash_ == 0 || this->parent_->component_in_lock_hash_ == hash) { - this->parent_->component_in_lock_hash_ = hash; - return true; - } - return false; - } - void unlock_component(uint32_t hash) { - if (this->parent_->component_in_lock_hash_ == hash) { - this->parent_->component_in_lock_hash_ = 0; - } - } -}; +class I2SAudioIn : public I2SAudioBase {}; -class I2SAudioOut : public I2SAudioBase { - protected: - void lock_component(uint32_t hash) overload { - if (this->parent_->component_out_lock_hash_ == 0 || this->parent_->component_out_lock_hash_ == hash) { - this->parent_->component_out_lock_hash_ = hash; - return true; - } - return false; - } - void unlock_component(uint32_t hash) { - if (this->parent_->component_out_lock_hash_ == hash) { - this->parent_->component_out_lock_hash_ = 0; - } - } -}; +class I2SAudioOut : public I2SAudioBase {}; class I2SAudioComponent : public Component { public: @@ -86,17 +55,16 @@ class I2SAudioComponent : public Component { bool try_lock() { return this->lock_.try_lock(); } void unlock() { this->lock_.unlock(); } + virtual bool lock_component(I2SAudioBase *audio); + virtual void unlock_component(I2SAudioBase *audio); + virtual bool is_compoment_locked(I2SAudioBase *audio); + i2s_port_t get_port() const { return this->port_; } protected: Mutex lock_; - /* the following property is used to allow locking a component to the i2s service.*/ - uint32_t component_out_lock_hash_{0}; - uint32_t component_in_lock_hash_{0}; - - I2SAudioIn *audio_in_{nullptr}; - I2SAudioOut *audio_out_{nullptr}; + I2SAudioBase *audio_base_{nullptr}; int mclk_pin_{I2S_PIN_NO_CHANGE}; int bclk_pin_{I2S_PIN_NO_CHANGE}; diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp index 4fc489d0a3..4c1040c4c4 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp @@ -26,7 +26,6 @@ static const char *const TAG = "i2s_audio.speaker"; enum SpeakerEventGroupBits : uint32_t { COMMAND_START = (1 << 0), // Starts the main task purpose COMMAND_STOP = (1 << 1), // stops the main task - COMMAND_STOP_GRACEFULLY = (1 << 2), // Stops the task once all data has been written MESSAGE_RING_BUFFER_AVAILABLE_TO_WRITE = (1 << 5), // Locks the ring buffer when not set STATE_STARTING = (1 << 10), STATE_RUNNING = (1 << 11), @@ -138,6 +137,7 @@ void I2SAudioSpeaker::loop() { xEventGroupClearBits(this->event_group_, SpeakerEventGroupBits::ALL_BITS); this->speaker_task_handle_ = nullptr; } + this->parent_->unlock_component(this); } } @@ -152,6 +152,15 @@ size_t I2SAudioSpeaker::play(const uint8_t *data, size_t length, TickType_t tick ESP_LOGE(TAG, "Cannot play audio, speaker failed to setup"); return 0; } + // prevent adding new data until the speaker has stopped. + if (!this->parent_->lock_component(this)) { + ESP_LOGE(TAG, "Cannot play new audio, it being used by an other audio component."); + return 0; + } + if (this->state_ == speaker::STATE_STOPPING) { + return 0; + } + if (this->state_ != speaker::STATE_RUNNING && this->state_ != speaker::STATE_STARTING) { this->start(); } @@ -184,15 +193,14 @@ bool I2SAudioSpeaker::has_buffered_data() const { void I2SAudioSpeaker::speaker_task(void *params) { I2SAudioSpeaker *this_speaker = (I2SAudioSpeaker *) params; - uint32_t event_group_bits = - xEventGroupWaitBits(this_speaker->event_group_, - SpeakerEventGroupBits::COMMAND_START | SpeakerEventGroupBits::COMMAND_STOP | - SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY, // Bit message to read - pdTRUE, // Clear the bits on exit - pdFALSE, // Don't wait for all the bits, - portMAX_DELAY); // Block indefinitely until a bit is set + uint32_t event_group_bits = xEventGroupWaitBits( + this_speaker->event_group_, + SpeakerEventGroupBits::COMMAND_START | SpeakerEventGroupBits::COMMAND_STOP, // Bit message to read + pdTRUE, // Clear the bits on exit + pdFALSE, // Don't wait for all the bits, + portMAX_DELAY); // Block indefinitely until a bit is set - if (event_group_bits & (SpeakerEventGroupBits::COMMAND_STOP | SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY)) { + if (event_group_bits & (SpeakerEventGroupBits::COMMAND_STOP)) { // Received a stop signal before the task was requested to start this_speaker->delete_task_(0); } @@ -234,9 +242,6 @@ void I2SAudioSpeaker::speaker_task(void *params) { if (event_group_bits & SpeakerEventGroupBits::COMMAND_STOP) { break; } - if (event_group_bits & SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY) { - stop_gracefully = true; - } size_t bytes_to_read = dma_buffers_size; size_t bytes_read = this_speaker->audio_ring_buffer_->read((void *) this_speaker->data_buffer_, bytes_to_read, @@ -264,15 +269,6 @@ void I2SAudioSpeaker::speaker_task(void *params) { if (bytes_written != bytes_read) { xEventGroupSetBits(this_speaker->event_group_, SpeakerEventGroupBits::ERR_ESP_INVALID_SIZE); } - - } else { - // No data received - - if (stop_gracefully) { - break; - } - - i2s_zero_dma_buffer(this_speaker->parent_->get_port()); } } } @@ -293,6 +289,12 @@ void I2SAudioSpeaker::start() { if ((this->state_ == speaker::STATE_STARTING) || (this->state_ == speaker::STATE_RUNNING)) return; + // prevent adding new data until the speaker has stopped. + if (!this->parent_->lock_component(this)) { + ESP_LOGE(TAG, "Cannot play audio, it being used by an other audio component."); + return; + } + if (this->speaker_task_handle_ == nullptr) { xTaskCreate(I2SAudioSpeaker::speaker_task, "speaker_task", TASK_STACK_SIZE, (void *) this, TASK_PRIORITY, &this->speaker_task_handle_); @@ -315,9 +317,11 @@ void I2SAudioSpeaker::stop_(bool wait_on_empty) { return; if (this->state_ == speaker::STATE_STOPPED) return; + if (this->parent_->is_compoment_locked(this)) + return; if (wait_on_empty) { - xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY); + this->state_ = speaker::STATE_STOPPING; } else { xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::COMMAND_STOP); }