From 47ae601d417d0ef99eb6fe433ef695614d8d2786 Mon Sep 17 00:00:00 2001 From: jacqueline Date: Tue, 21 Feb 2023 14:40:18 +1100 Subject: [PATCH] Tidy up pipeline and use arena capacity to test for overruns --- src/audio/CMakeLists.txt | 3 +- src/audio/audio_decoder.cpp | 35 ++++------ src/audio/audio_element.cpp | 7 +- src/audio/audio_element_handle.cpp | 80 ---------------------- src/audio/audio_playback.cpp | 16 ++--- src/audio/audio_task.cpp | 35 +++------- src/audio/fatfs_audio_input.cpp | 28 ++++---- src/audio/i2s_audio_output.cpp | 47 ++++--------- src/audio/include/audio_decoder.hpp | 17 ++--- src/audio/include/audio_element.hpp | 48 ++----------- src/audio/include/audio_element_handle.hpp | 41 ----------- src/audio/include/audio_playback.hpp | 7 +- src/audio/include/audio_task.hpp | 4 +- src/audio/include/fatfs_audio_input.hpp | 9 ++- src/audio/include/i2s_audio_output.hpp | 13 ++-- src/codecs/mad.cpp | 2 +- src/drivers/dac.cpp | 46 ++++++------- src/memory/arena.cpp | 7 +- 18 files changed, 112 insertions(+), 333 deletions(-) delete mode 100644 src/audio/audio_element_handle.cpp delete mode 100644 src/audio/include/audio_element_handle.hpp diff --git a/src/audio/CMakeLists.txt b/src/audio/CMakeLists.txt index 80532542..d4c89214 100644 --- a/src/audio/CMakeLists.txt +++ b/src/audio/CMakeLists.txt @@ -1,8 +1,7 @@ idf_component_register( SRCS "audio_decoder.cpp" "audio_task.cpp" "chunk.cpp" "fatfs_audio_input.cpp" "stream_message.cpp" "i2s_audio_output.cpp" "stream_buffer.cpp" - "audio_playback.cpp" "audio_element_handle.cpp" "stream_event.cpp" - "audio_element.cpp" + "audio_playback.cpp" "stream_event.cpp" "audio_element.cpp" INCLUDE_DIRS "include" REQUIRES "codecs" "drivers" "cbor" "result" "tasks" "span" "memory") diff --git a/src/audio/audio_decoder.cpp b/src/audio/audio_decoder.cpp index 07e05653..9879b042 100644 --- a/src/audio/audio_decoder.cpp +++ b/src/audio/audio_decoder.cpp @@ -37,15 +37,18 @@ auto AudioDecoder::HasUnprocessedInput() -> bool { return !needs_more_input_ || has_samples_to_send_; } -auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info) - -> cpp::result { +auto AudioDecoder::IsOverBuffered() -> bool { + return arena_.BlocksFree() == 0; +} + +auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info) -> void { stream_info_ = info; if (info.chunk_size) { chunk_reader_.emplace(info.chunk_size.value()); } else { ESP_LOGE(kTag, "no chunk size given"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } // Reuse the existing codec if we can. This will help with gapless playback, @@ -54,7 +57,7 @@ auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info) if (current_codec_ != nullptr && current_codec_->CanHandleFile(info.path.value_or(""))) { current_codec_->ResetForNewStream(); - return {}; + return; } auto result = codecs::CreateCodecForFile(info.path.value_or("")); @@ -62,28 +65,23 @@ auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info) current_codec_ = std::move(result.value()); } else { ESP_LOGE(kTag, "no codec for this file"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } stream_info_ = info; has_sent_stream_info_ = false; - - return {}; } -auto AudioDecoder::ProcessChunk(const cpp::span& chunk) - -> cpp::result { +auto AudioDecoder::ProcessChunk(const cpp::span& chunk) -> void { if (current_codec_ == nullptr || !chunk_reader_) { // Should never happen, but fail explicitly anyway. ESP_LOGW(kTag, "received chunk without chunk size or codec"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } ESP_LOGI(kTag, "received new chunk (size %u)", chunk.size()); current_codec_->SetInput(chunk_reader_->HandleNewData(chunk)); needs_more_input_ = false; - - return {}; } auto AudioDecoder::ProcessEndOfStream() -> void { @@ -95,7 +93,7 @@ auto AudioDecoder::ProcessEndOfStream() -> void { StreamEvent::CreateEndOfStream(input_events_))); } -auto AudioDecoder::Process() -> cpp::result { +auto AudioDecoder::Process() -> void { if (has_samples_to_send_) { // Writing samples is relatively quick (it's just a bunch of memcopy's), so // do them all at once. @@ -115,7 +113,7 @@ auto AudioDecoder::Process() -> cpp::result { auto block = arena_.Acquire(); if (!block) { - return {}; + return; } auto write_res = @@ -126,18 +124,17 @@ auto AudioDecoder::Process() -> cpp::result { auto chunk = std::unique_ptr( StreamEvent::CreateArenaChunk(input_events_, *block)); if (!SendOrBufferEvent(std::move(chunk))) { - return {}; + return; } } // We will process the next frame during the next call to this method. - return {}; } if (!needs_more_input_) { auto res = current_codec_->ProcessNextFrame(); if (res.has_error()) { - // todo - return {}; + // TODO(jacqueline): Handle errors. + return; } needs_more_input_ = res.value(); has_samples_to_send_ = true; @@ -146,8 +143,6 @@ auto AudioDecoder::Process() -> cpp::result { chunk_reader_->HandleBytesUsed(current_codec_->GetInputPosition()); } } - - return {}; } } // namespace audio diff --git a/src/audio/audio_element.cpp b/src/audio/audio_element.cpp index 6b04b892..cef54631 100644 --- a/src/audio/audio_element.cpp +++ b/src/audio/audio_element.cpp @@ -6,9 +6,7 @@ namespace audio { IAudioElement::IAudioElement() : input_events_(xQueueCreate(kEventQueueSize, sizeof(void*))), output_events_(nullptr), - unprocessed_output_chunks_(0), - buffered_output_(), - current_state_(STATE_RUN) {} + buffered_output_() {} IAudioElement::~IAudioElement() { // Ensure we don't leak any memory from events leftover in the queue. @@ -28,9 +26,6 @@ IAudioElement::~IAudioElement() { auto IAudioElement::SendOrBufferEvent(std::unique_ptr event) -> bool { - if (event->tag == StreamEvent::ARENA_CHUNK) { - unprocessed_output_chunks_++; - } if (!buffered_output_.empty()) { // To ensure we send data in order, don't try to send if we've already // failed to send something. diff --git a/src/audio/audio_element_handle.cpp b/src/audio/audio_element_handle.cpp deleted file mode 100644 index c5f0c374..00000000 --- a/src/audio/audio_element_handle.cpp +++ /dev/null @@ -1,80 +0,0 @@ -#include "audio_element_handle.hpp" -#include "audio_element.hpp" -#include "freertos/projdefs.h" -#include "freertos/task.h" - -namespace audio { - -AudioElementHandle::AudioElementHandle(std::unique_ptr task, - std::shared_ptr element) - : task_(std::move(task)), element_(std::move(element)) {} - -AudioElementHandle::~AudioElementHandle() { - Quit(); -} - -auto AudioElementHandle::CurrentState() -> ElementState { - return element_->ElementState(); -} - -auto AudioElementHandle::PlayPause(enum PlayPause state) -> void { - ElementState s = CurrentState(); - if (state == PLAY && s == STATE_PAUSE) { - // Ensure we actually finished any previous pause command. - // TODO: really? - PauseSync(); - SetStateAndWakeUp(STATE_RUN); - return; - } - if (state == PAUSE && s == STATE_RUN) { - element_->ElementState(STATE_PAUSE); - SetStateAndWakeUp(STATE_PAUSE); - return; - } -} - -auto AudioElementHandle::Quit() -> void { - SetStateAndWakeUp(STATE_QUIT); -} - -auto AudioElementHandle::PauseSync() -> void { - PlayPause(PAUSE); - MonitorUntilState(eSuspended); -} - -auto AudioElementHandle::QuitSync() -> void { - Quit(); - MonitorUntilState(eDeleted); -} - -auto AudioElementHandle::MonitorUntilState(eTaskState desired) -> void { - while (eTaskGetState(*task_) != desired) { - WakeUpTask(); - vTaskDelay(pdMS_TO_TICKS(1)); - } -} - -auto AudioElementHandle::SetStateAndWakeUp(ElementState state) -> void { - element_->ElementState(state); - WakeUpTask(); -} - -auto AudioElementHandle::WakeUpTask() -> void { - // TODO: various races where the task isn't blocked yet, but there is a block - // between now and its next element state check. Also think about chunk blocks - // nested in element bodies. - // Maybe we need a big mutex or semaphore somewhere in here. - switch (eTaskGetState(*task_)) { - case eBlocked: - // TODO: when is this safe? - xTaskAbortDelay(*task_); - break; - case eSuspended: - vTaskResume(*task_); - break; - default: - return; - } -} - -} // namespace audio diff --git a/src/audio/audio_playback.cpp b/src/audio/audio_playback.cpp index edbdcea7..c95a5d63 100644 --- a/src/audio/audio_playback.cpp +++ b/src/audio/audio_playback.cpp @@ -4,11 +4,13 @@ #include #include #include + +#include "freertos/portmacro.h" + #include "audio_decoder.hpp" #include "audio_task.hpp" #include "chunk.hpp" #include "fatfs_audio_input.hpp" -#include "freertos/portmacro.h" #include "gpio_expander.hpp" #include "i2s_audio_output.hpp" #include "storage.hpp" @@ -38,9 +40,9 @@ auto AudioPlayback::create(drivers::GpioExpander* expander, playback->ConnectElements(codec.get(), sink.get()); // Launch! - playback->element_handles_.push_back(StartAudioTask("src", {}, source)); - playback->element_handles_.push_back(StartAudioTask("dec", {}, codec)); - playback->element_handles_.push_back(StartAudioTask("sink", 0, sink)); + StartAudioTask("src", {}, source); + StartAudioTask("dec", {}, codec); + StartAudioTask("sink", 0, sink); playback->input_handle_ = source->InputEventQueue(); @@ -49,11 +51,7 @@ auto AudioPlayback::create(drivers::GpioExpander* expander, AudioPlayback::AudioPlayback() {} -AudioPlayback::~AudioPlayback() { - for (auto& element : element_handles_) { - element->Quit(); - } -} +AudioPlayback::~AudioPlayback() {} auto AudioPlayback::Play(const std::string& filename) -> void { StreamInfo info; diff --git a/src/audio/audio_task.cpp b/src/audio/audio_task.cpp index 7c91b8cc..ce6d724e 100644 --- a/src/audio/audio_task.cpp +++ b/src/audio/audio_task.cpp @@ -6,8 +6,6 @@ #include #include -#include "arena.hpp" -#include "audio_element_handle.hpp" #include "cbor.h" #include "esp_heap_caps.h" #include "esp_log.h" @@ -16,6 +14,7 @@ #include "freertos/queue.h" #include "span.hpp" +#include "arena.hpp" #include "audio_element.hpp" #include "chunk.hpp" #include "stream_event.hpp" @@ -30,8 +29,7 @@ static const char* kTag = "task"; auto StartAudioTask(const std::string& name, std::optional core_id, - std::shared_ptr element) - -> std::unique_ptr { + std::shared_ptr element) -> void { auto task_handle = std::make_unique(); // Newly created task will free this. @@ -46,8 +44,6 @@ auto StartAudioTask(const std::string& name, xTaskCreate(&AudioTaskMain, name.c_str(), element->StackSizeBytes(), args, kTaskPriorityAudio, task_handle.get()); } - - return std::make_unique(std::move(task_handle), element); } void AudioTaskMain(void* args) { @@ -62,7 +58,8 @@ void AudioTaskMain(void* args) { // processed. std::deque> pending_events; - while (element->ElementState() != STATE_QUIT) { + // TODO(jacqueline): quit event + while (true) { // First, we pull events from our input queue into pending_events. This // keeps us responsive to any events that need to be handled immediately. // Then we check if there's any events to flush downstream. @@ -91,8 +88,6 @@ void AudioTaskMain(void* args) { if (new_event->tag == StreamEvent::UNINITIALISED) { ESP_LOGE(kTag, "discarding invalid event!!"); } else if (new_event->tag == StreamEvent::CHUNK_NOTIFICATION) { - ESP_LOGD(kTag, "marking chunk as used"); - element->OnChunkProcessed(); delete new_event; } else if (new_event->tag == StreamEvent::LOG_STATUS) { element->ProcessLogStatus(); @@ -129,10 +124,7 @@ void AudioTaskMain(void* args) { if (element->HasUnprocessedInput()) { ESP_LOGD(kTag, "processing input events"); - auto process_res = element->Process(); - if (!process_res.has_error() || process_res.error() != OUT_OF_DATA) { - // TODO: log! - } + element->Process(); continue; } @@ -146,13 +138,12 @@ void AudioTaskMain(void* args) { if (event->tag == StreamEvent::STREAM_INFO) { ESP_LOGD(kTag, "processing stream info"); - auto process_res = element->ProcessStreamInfo(*event->stream_info); - if (process_res.has_error()) { - // TODO(jacqueline) - ESP_LOGE(kTag, "failed to process stream info"); - } + + element->ProcessStreamInfo(*event->stream_info); + } else if (event->tag == StreamEvent::ARENA_CHUNK) { ESP_LOGD(kTag, "processing arena data"); + memory::ArenaRef ref(event->arena_chunk); auto callback = StreamEvent::CreateChunkNotification(element->InputEventQueue()); @@ -163,13 +154,7 @@ void AudioTaskMain(void* args) { // TODO(jacqueline): Consider giving the element a full ArenaRef here, // so that it can hang on to it and potentially save an alloc+copy. - auto process_chunk_res = - element->ProcessChunk({ref.ptr.start, ref.ptr.used_size}); - if (process_chunk_res.has_error()) { - // TODO(jacqueline) - ESP_LOGE(kTag, "failed to process chunk"); - continue; - } + element->ProcessChunk({ref.ptr.start, ref.ptr.used_size}); // TODO: think about whether to do the whole queue break; diff --git a/src/audio/fatfs_audio_input.cpp b/src/audio/fatfs_audio_input.cpp index 829064d8..5354c5fd 100644 --- a/src/audio/fatfs_audio_input.cpp +++ b/src/audio/fatfs_audio_input.cpp @@ -36,22 +36,27 @@ auto FatfsAudioInput::HasUnprocessedInput() -> bool { return is_file_open_; } -auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info) - -> cpp::result { +auto FatfsAudioInput::IsOverBuffered() -> bool { + return arena_.BlocksFree() == 0; +} + +auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info) -> void { if (is_file_open_) { f_close(¤t_file_); is_file_open_ = false; } if (!info.path) { - return cpp::fail(UNSUPPORTED_STREAM); + // TODO(jacqueline): Handle errors. + return; } ESP_LOGI(kTag, "opening file %s", info.path->c_str()); std::string path = *info.path; FRESULT res = f_open(¤t_file_, path.c_str(), FA_READ); if (res != FR_OK) { ESP_LOGE(kTag, "failed to open file! res: %i", res); - return cpp::fail(IO_ERROR); + // TODO(jacqueline): Handle errors. + return; } is_file_open_ = true; @@ -62,14 +67,9 @@ auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info) auto event = StreamEvent::CreateStreamInfo(input_events_, new_info); SendOrBufferEvent(std::unique_ptr(event)); - - return {}; } -auto FatfsAudioInput::ProcessChunk(const cpp::span& chunk) - -> cpp::result { - return cpp::fail(UNSUPPORTED_STREAM); -} +auto FatfsAudioInput::ProcessChunk(const cpp::span& chunk) -> void {} auto FatfsAudioInput::ProcessEndOfStream() -> void { if (is_file_open_) { @@ -80,18 +80,19 @@ auto FatfsAudioInput::ProcessEndOfStream() -> void { } } -auto FatfsAudioInput::Process() -> cpp::result { +auto FatfsAudioInput::Process() -> void { if (is_file_open_) { auto dest_block = memory::ArenaRef::Acquire(&arena_); if (!dest_block) { - return {}; + return; } FRESULT result = f_read(¤t_file_, dest_block->ptr.start, dest_block->ptr.size, &dest_block->ptr.used_size); if (result != FR_OK) { ESP_LOGE(kTag, "file I/O error %d", result); - return cpp::fail(IO_ERROR); + // TODO(jacqueline): Handle errors. + return; } if (dest_block->ptr.used_size < dest_block->ptr.size || @@ -105,7 +106,6 @@ auto FatfsAudioInput::Process() -> cpp::result { SendOrBufferEvent(std::move(dest_event)); } - return {}; } } // namespace audio diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp index 9a41adff..110227cf 100644 --- a/src/audio/i2s_audio_output.cpp +++ b/src/audio/i2s_audio_output.cpp @@ -40,8 +40,6 @@ I2SAudioOutput::I2SAudioOutput(drivers::GpioExpander* expander, std::unique_ptr dac) : expander_(expander), dac_(std::move(dac)), - volume_(255), - is_soft_muted_(false), chunk_reader_(), latest_chunk_() {} @@ -51,18 +49,21 @@ auto I2SAudioOutput::HasUnprocessedInput() -> bool { return latest_chunk_.size() > 0; } -auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) - -> cpp::result { +auto I2SAudioOutput::IsOverBuffered() -> bool { + return false; +} + +auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) -> void { // TODO(jacqueline): probs do something with the channel hey if (!info.bits_per_sample || !info.sample_rate) { ESP_LOGE(kTag, "audio stream missing bits or sample rate"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } if (!info.chunk_size) { ESP_LOGE(kTag, "audio stream missing chunk size"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } chunk_reader_.emplace(*info.chunk_size); @@ -82,7 +83,7 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) break; default: ESP_LOGE(kTag, "dropping stream with unknown bps"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } drivers::AudioDac::SampleRate sample_rate; @@ -95,18 +96,14 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) break; default: ESP_LOGE(kTag, "dropping stream with unknown rate"); - return cpp::fail(UNSUPPORTED_STREAM); + return; } dac_->Reconfigure(bps, sample_rate); - - return {}; } -auto I2SAudioOutput::ProcessChunk(const cpp::span& chunk) - -> cpp::result { +auto I2SAudioOutput::ProcessChunk(const cpp::span& chunk) -> void { latest_chunk_ = chunk_reader_->HandleNewData(chunk); - return 0; } auto I2SAudioOutput::ProcessEndOfStream() -> void { @@ -119,8 +116,9 @@ auto I2SAudioOutput::ProcessLogStatus() -> void { dac_->LogStatus(); } -auto I2SAudioOutput::Process() -> cpp::result { - // Note: no logging here! +auto I2SAudioOutput::Process() -> void { + // Note: avoid logging here! We need to get bytes from the chunk buffer into + // the I2S DMA buffer as fast as possible, to avoid running out of samples. std::size_t bytes_written = dac_->WriteData(latest_chunk_); if (bytes_written == latest_chunk_.size_bytes()) { latest_chunk_ = cpp::span(); @@ -128,26 +126,11 @@ auto I2SAudioOutput::Process() -> cpp::result { } else { latest_chunk_ = latest_chunk_.subspan(bytes_written); } - return {}; + return; } auto I2SAudioOutput::SetVolume(uint8_t volume) -> void { - volume_ = volume; - if (!is_soft_muted_) { - dac_->WriteVolume(volume); - } -} - -auto I2SAudioOutput::SetSoftMute(bool enabled) -> void { - if (enabled == is_soft_muted_) { - return; - } - is_soft_muted_ = enabled; - if (is_soft_muted_) { - dac_->WriteVolume(255); - } else { - dac_->WriteVolume(volume_); - } + dac_->WriteVolume(volume); } } // namespace audio diff --git a/src/audio/include/audio_decoder.hpp b/src/audio/include/audio_decoder.hpp index 9cc40162..47642469 100644 --- a/src/audio/include/audio_decoder.hpp +++ b/src/audio/include/audio_decoder.hpp @@ -24,21 +24,13 @@ class AudioDecoder : public IAudioElement { auto StackSizeBytes() const -> std::size_t override { return 10 * 1024; }; - auto InputMinChunkSize() const -> std::size_t override { - // 128 kbps MPEG-1 @ 44.1 kHz is approx. 418 bytes according to the - // internet. - // TODO(jacqueline): tune as more codecs are added. - return 1024; - } - auto HasUnprocessedInput() -> bool override; + auto IsOverBuffered() -> bool override; - auto ProcessStreamInfo(const StreamInfo& info) - -> cpp::result override; - auto ProcessChunk(const cpp::span& chunk) - -> cpp::result override; + auto ProcessStreamInfo(const StreamInfo& info) -> void override; + auto ProcessChunk(const cpp::span& chunk) -> void override; auto ProcessEndOfStream() -> void override; - auto Process() -> cpp::result override; + auto Process() -> void override; AudioDecoder(const AudioDecoder&) = delete; AudioDecoder& operator=(const AudioDecoder&) = delete; @@ -50,7 +42,6 @@ class AudioDecoder : public IAudioElement { std::optional chunk_reader_; bool has_sent_stream_info_; - std::size_t chunk_size_; bool has_samples_to_send_; bool needs_more_input_; }; diff --git a/src/audio/include/audio_element.hpp b/src/audio/include/audio_element.hpp index b881404c..91036348 100644 --- a/src/audio/include/audio_element.hpp +++ b/src/audio/include/audio_element.hpp @@ -20,25 +20,6 @@ namespace audio { -enum ElementState { - STATE_RUN, - STATE_PAUSE, - STATE_QUIT, -}; - -/* - * Errors that may be returned by any of the Process* methods of an audio - * element. - */ -enum AudioProcessingError { - // Indicates that this element is unable to handle the upcoming chunks. - UNSUPPORTED_STREAM, - // Indicates an error with reading or writing stream data. - IO_ERROR, - // Indicates that the element has run out of data to process. - OUT_OF_DATA, -}; - static const size_t kEventQueueSize = 8; /* @@ -66,35 +47,24 @@ class IAudioElement { */ virtual auto StackSizeBytes() const -> std::size_t { return 4096; }; - virtual auto InputMinChunkSize() const -> std::size_t { return 0; } - /* Returns this element's input buffer. */ auto InputEventQueue() const -> QueueHandle_t { return input_events_; } - /* Returns this element's output buffer. */ auto OutputEventQueue() const -> QueueHandle_t { return output_events_; } - auto OutputEventQueue(const QueueHandle_t q) -> void { output_events_ = q; } - auto HasUnflushedOutput() -> bool { return !buffered_output_.empty(); } - virtual auto HasUnprocessedInput() -> bool = 0; - auto IsOverBuffered() -> bool { return unprocessed_output_chunks_ > 4; } + virtual auto IsOverBuffered() -> bool { return false; } + auto HasUnflushedOutput() -> bool { return !buffered_output_.empty(); } auto FlushBufferedOutput() -> bool; - auto ElementState() const -> ElementState { return current_state_; } - auto ElementState(enum ElementState e) -> void { current_state_ = e; } - - virtual auto OnChunkProcessed() -> void { unprocessed_output_chunks_--; } - /* * Called when a StreamInfo message is received. Used to configure this * element in preperation for incoming chunks. */ - virtual auto ProcessStreamInfo(const StreamInfo& info) - -> cpp::result = 0; + virtual auto ProcessStreamInfo(const StreamInfo& info) -> void = 0; /* * Called when a ChunkHeader message is received. Includes the data associated @@ -102,8 +72,7 @@ class IAudioElement { * bytes in this chunk that were actually used; leftover bytes will be * prepended to the next call. */ - virtual auto ProcessChunk(const cpp::span& chunk) - -> cpp::result = 0; + virtual auto ProcessChunk(const cpp::span& chunk) -> void = 0; virtual auto ProcessEndOfStream() -> void = 0; @@ -114,7 +83,7 @@ class IAudioElement { * time. This could be used to synthesize output, or to save memory by * releasing unused resources. */ - virtual auto Process() -> cpp::result = 0; + virtual auto Process() -> void = 0; protected: auto SendOrBufferEvent(std::unique_ptr event) -> bool; @@ -125,15 +94,8 @@ class IAudioElement { // if we're not yet in a pipeline. // FIXME: it would be nicer if this was non-nullable. QueueHandle_t output_events_; - - // The number of output chunks that we have generated, but have not yet been - // processed by the next element in the pipeline. This includes any chunks - // that are currently help in buffered_output_. - int unprocessed_output_chunks_; // Output events that have been generated, but are yet to be sent downstream. std::deque> buffered_output_; - - enum ElementState current_state_; }; } // namespace audio diff --git a/src/audio/include/audio_element_handle.hpp b/src/audio/include/audio_element_handle.hpp deleted file mode 100644 index e4d66491..00000000 --- a/src/audio/include/audio_element_handle.hpp +++ /dev/null @@ -1,41 +0,0 @@ -#pragma once - -#include -#include "audio_element.hpp" - -namespace audio { - -class AudioElementHandle { - public: - AudioElementHandle(std::unique_ptr task, - std::shared_ptr element); - ~AudioElementHandle(); - - auto CurrentState() -> ElementState; - - // TODO: think about this contract. Would it ever make sense to pause and - // then walk away? Things could keep running for a whole loop if data comes - // through, so probably not? - enum PlayPause { - PLAY, - PAUSE, - }; - auto PlayPause(PlayPause state) -> void; - auto Quit() -> void; - - auto PauseSync() -> void; - auto QuitSync() -> void; - - AudioElementHandle(const AudioElementHandle&) = delete; - AudioElementHandle& operator=(const AudioElementHandle&) = delete; - - private: - std::unique_ptr task_; - std::shared_ptr element_; - - auto MonitorUntilState(eTaskState desired) -> void; - auto SetStateAndWakeUp(ElementState state) -> void; - auto WakeUpTask() -> void; -}; - -} // namespace audio diff --git a/src/audio/include/audio_playback.hpp b/src/audio/include/audio_playback.hpp index fc266c9b..a1bea64f 100644 --- a/src/audio/include/audio_playback.hpp +++ b/src/audio/include/audio_playback.hpp @@ -5,12 +5,12 @@ #include #include -#include "audio_element.hpp" -#include "audio_element_handle.hpp" #include "esp_err.h" -#include "gpio_expander.hpp" #include "result.hpp" #include "span.hpp" + +#include "audio_element.hpp" +#include "gpio_expander.hpp" #include "storage.hpp" #include "stream_buffer.hpp" @@ -45,7 +45,6 @@ class AudioPlayback { private: auto ConnectElements(IAudioElement* src, IAudioElement* sink) -> void; - std::vector> element_handles_; QueueHandle_t input_handle_; }; diff --git a/src/audio/include/audio_task.hpp b/src/audio/include/audio_task.hpp index 0b353735..df70ebaa 100644 --- a/src/audio/include/audio_task.hpp +++ b/src/audio/include/audio_task.hpp @@ -5,7 +5,6 @@ #include #include "audio_element.hpp" -#include "audio_element_handle.hpp" #include "freertos/portmacro.h" namespace audio { @@ -16,8 +15,7 @@ struct AudioTaskArgs { auto StartAudioTask(const std::string& name, std::optional core_id, - std::shared_ptr element) - -> std::unique_ptr; + std::shared_ptr element) -> void; void AudioTaskMain(void* args); diff --git a/src/audio/include/fatfs_audio_input.hpp b/src/audio/include/fatfs_audio_input.hpp index 06b0b7ea..9f2d676c 100644 --- a/src/audio/include/fatfs_audio_input.hpp +++ b/src/audio/include/fatfs_audio_input.hpp @@ -24,13 +24,12 @@ class FatfsAudioInput : public IAudioElement { ~FatfsAudioInput(); auto HasUnprocessedInput() -> bool override; + auto IsOverBuffered() -> bool override; - auto ProcessStreamInfo(const StreamInfo& info) - -> cpp::result override; - auto ProcessChunk(const cpp::span& chunk) - -> cpp::result override; + auto ProcessStreamInfo(const StreamInfo& info) -> void override; + auto ProcessChunk(const cpp::span& chunk) -> void override; auto ProcessEndOfStream() -> void override; - auto Process() -> cpp::result override; + auto Process() -> void override; FatfsAudioInput(const FatfsAudioInput&) = delete; FatfsAudioInput& operator=(const FatfsAudioInput&) = delete; diff --git a/src/audio/include/i2s_audio_output.hpp b/src/audio/include/i2s_audio_output.hpp index de2f1f58..2bea091b 100644 --- a/src/audio/include/i2s_audio_output.hpp +++ b/src/audio/include/i2s_audio_output.hpp @@ -23,28 +23,23 @@ class I2SAudioOutput : public IAudioElement { ~I2SAudioOutput(); auto HasUnprocessedInput() -> bool override; + auto IsOverBuffered() -> bool override; - auto ProcessStreamInfo(const StreamInfo& info) - -> cpp::result override; - auto ProcessChunk(const cpp::span& chunk) - -> cpp::result override; + auto ProcessStreamInfo(const StreamInfo& info) -> void override; + auto ProcessChunk(const cpp::span& chunk) -> void override; auto ProcessEndOfStream() -> void override; auto ProcessLogStatus() -> void override; - auto Process() -> cpp::result override; + auto Process() -> void override; I2SAudioOutput(const I2SAudioOutput&) = delete; I2SAudioOutput& operator=(const I2SAudioOutput&) = delete; private: auto SetVolume(uint8_t volume) -> void; - auto SetSoftMute(bool enabled) -> void; drivers::GpioExpander* expander_; std::unique_ptr dac_; - uint8_t volume_; - bool is_soft_muted_; - std::optional chunk_reader_; cpp::span latest_chunk_; }; diff --git a/src/codecs/mad.cpp b/src/codecs/mad.cpp index 3fef3bbf..1112bd62 100644 --- a/src/codecs/mad.cpp +++ b/src/codecs/mad.cpp @@ -121,7 +121,7 @@ auto MadMp3Decoder::WriteOutputSamples(cpp::span output) uint32_t sample_24 = scaleTo24Bits(synth_.pcm.samples[channel][current_sample_]); output[output_byte++] = static_cast((sample_24 >> 8) & 0xFF); - output[output_byte++] = static_cast((sample_24) & 0xFF); + output[output_byte++] = static_cast((sample_24)&0xFF); } current_sample_++; } diff --git a/src/drivers/dac.cpp b/src/drivers/dac.cpp index fc0f0791..c53c0b53 100644 --- a/src/drivers/dac.cpp +++ b/src/drivers/dac.cpp @@ -50,19 +50,19 @@ auto AudioDac::create(GpioExpander* expander) i2s_std_config_t i2s_config = { .clk_cfg = dac->clock_config_, .slot_cfg = dac->slot_config_, - .gpio_cfg = { - // TODO: investigate running in three wire mode for less noise - .mclk = GPIO_NUM_0, - .bclk = GPIO_NUM_26, - .ws = GPIO_NUM_27, - .dout = GPIO_NUM_5, - .din = I2S_GPIO_UNUSED, - .invert_flags = - { - .mclk_inv = false, - .bclk_inv = false, - .ws_inv = false, - }}, + .gpio_cfg = + {// TODO: investigate running in three wire mode for less noise + .mclk = GPIO_NUM_0, + .bclk = GPIO_NUM_26, + .ws = GPIO_NUM_27, + .dout = GPIO_NUM_5, + .din = I2S_GPIO_UNUSED, + .invert_flags = + { + .mclk_inv = false, + .bclk_inv = false, + .ws_inv = false, + }}, }; if (esp_err_t err = @@ -89,11 +89,12 @@ auto AudioDac::create(GpioExpander* expander) dac->WriteRegister(Register::DAC_CLOCK_SOURCE, 0b11 << 5); // Enable auto clocking, and do your best to carry on despite errors. - //dac->WriteRegister(Register::CLOCK_ERRORS, 0b1111101); + // dac->WriteRegister(Register::CLOCK_ERRORS, 0b1111101); i2s_channel_enable(dac->i2s_handle_); - dac->WaitForPowerState([](bool booted, PowerState state) { return state == STANDBY; }); + dac->WaitForPowerState( + [](bool booted, PowerState state) { return state == STANDBY; }); return dac; } @@ -208,16 +209,11 @@ auto AudioDac::Stop() -> void { } #define BYTE_TO_BINARY_PATTERN "%c%c%c%c%c%c%c%c" -#define BYTE_TO_BINARY(byte) \ - (byte & 0x80 ? '1' : '0'), \ - (byte & 0x40 ? '1' : '0'), \ - (byte & 0x20 ? '1' : '0'), \ - (byte & 0x10 ? '1' : '0'), \ - (byte & 0x08 ? '1' : '0'), \ - (byte & 0x04 ? '1' : '0'), \ - (byte & 0x02 ? '1' : '0'), \ - (byte & 0x01 ? '1' : '0') - +#define BYTE_TO_BINARY(byte) \ + (byte & 0x80 ? '1' : '0'), (byte & 0x40 ? '1' : '0'), \ + (byte & 0x20 ? '1' : '0'), (byte & 0x10 ? '1' : '0'), \ + (byte & 0x08 ? '1' : '0'), (byte & 0x04 ? '1' : '0'), \ + (byte & 0x02 ? '1' : '0'), (byte & 0x01 ? '1' : '0') auto AudioDac::LogStatus() -> void { uint8_t res; diff --git a/src/memory/arena.cpp b/src/memory/arena.cpp index 450ac4f2..1e82e064 100644 --- a/src/memory/arena.cpp +++ b/src/memory/arena.cpp @@ -23,7 +23,8 @@ Arena::Arena(std::size_t block_size, } Arena::~Arena() { - // TODO: assert queue is full? + // We shouldn't have any blocks in use when destroying an arena. + assert(uxQueueSpacesAvailable(free_blocks_) == 0); vQueueDelete(free_blocks_); free(pool_); } @@ -44,6 +45,10 @@ auto Arena::Return(ArenaPtr ptr) -> void { xQueueSend(free_blocks_, &ptr.start, 0); } +auto Arena::BlocksFree() -> std::size_t { + return uxQueueMessagesWaiting(free_blocks_); +} + auto ArenaRef::Acquire(Arena* a) -> std::optional { auto ptr = a->Acquire(); if (ptr) {