From ef8d404b15e6d32506617a95aa3161fbe59ecdaf Mon Sep 17 00:00:00 2001 From: jacqueline Date: Mon, 30 Jan 2023 15:52:54 +1100 Subject: Continue ironing out i2s pipeline still at least one heap corruption issue, plus the i2s write method seems to block forever :/ --- src/audio/audio_decoder.cpp | 46 ++++++++++++++++++++++++------------- src/audio/audio_element.cpp | 4 +++- src/audio/audio_task.cpp | 24 +++++++++---------- src/audio/chunk.cpp | 7 +++--- src/audio/fatfs_audio_input.cpp | 6 +++-- src/audio/i2s_audio_output.cpp | 11 ++++++--- src/audio/include/audio_decoder.hpp | 3 ++- 7 files changed, 62 insertions(+), 39 deletions(-) (limited to 'src/audio') diff --git a/src/audio/audio_decoder.cpp b/src/audio/audio_decoder.cpp index 4478b2c4..b3415647 100644 --- a/src/audio/audio_decoder.cpp +++ b/src/audio/audio_decoder.cpp @@ -40,36 +40,31 @@ auto AudioDecoder::ProcessStreamInfo(const StreamInfo& info) stream_info_ = info; if (info.chunk_size) { - chunk_reader_.emplace(*info.chunk_size); + chunk_reader_ = ChunkReader(info.chunk_size.value()); } else { - // TODO. + ESP_LOGE(kTag, "no chunk size given"); + return cpp::fail(UNSUPPORTED_STREAM); } // Reuse the existing codec if we can. This will help with gapless playback, // since we can potentially just continue to decode as we were before, // without any setup overhead. - if (current_codec_->CanHandleFile(info.path.value_or(""))) { + if (current_codec_ != nullptr && + current_codec_->CanHandleFile(info.path.value_or(""))) { current_codec_->ResetForNewStream(); return {}; } - auto result = codecs::CreateCodecForFile(*info.path); - if (result) { - current_codec_ = std::move(*result); + auto result = codecs::CreateCodecForFile(info.path.value_or("")); + if (result.has_value()) { + current_codec_ = std::move(result.value()); } else { + ESP_LOGE(kTag, "no codec for this file"); return cpp::fail(UNSUPPORTED_STREAM); } - // TODO: defer until first header read, so we can give better info about - // sample rate, chunk size, etc. - StreamInfo downstream_info(info); - downstream_info.bits_per_sample = 32; - downstream_info.sample_rate = 48'000; - chunk_size_ = 128; - downstream_info.chunk_size = chunk_size_; - - auto event = StreamEvent::CreateStreamInfo(input_events_, downstream_info); - SendOrBufferEvent(std::unique_ptr(event)); + stream_info_ = info; + has_sent_stream_info_ = false; return {}; } @@ -78,10 +73,13 @@ auto AudioDecoder::ProcessChunk(const cpp::span& chunk) -> cpp::result { 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); } + ESP_LOGI(kTag, "received new chunk (size %u)", chunk.size()); current_codec_->SetInput(chunk_reader_->HandleNewData(chunk)); + needs_more_input_ = false; return {}; } @@ -92,6 +90,22 @@ auto AudioDecoder::Process() -> cpp::result { // Writing samples is relatively quick (it's just a bunch of memcopy's), so // do them all at once. while (has_samples_to_send_ && !IsOverBuffered()) { + if (!has_sent_stream_info_) { + has_sent_stream_info_ = true; + auto format = current_codec_->GetOutputFormat(); + stream_info_->bits_per_sample = format.bits_per_sample; + stream_info_->sample_rate = format.sample_rate_hz; + stream_info_->channels = format.num_channels; + + chunk_size_ = kSamplesPerChunk * (*stream_info_->bits_per_sample); + stream_info_->chunk_size = chunk_size_; + ESP_LOGI(kTag, "pcm stream chunk size: %u bytes", chunk_size_); + + auto event = + StreamEvent::CreateStreamInfo(input_events_, *stream_info_); + SendOrBufferEvent(std::unique_ptr(event)); + } + auto chunk = std::unique_ptr( StreamEvent::CreateChunkData(input_events_, chunk_size_)); auto write_res = diff --git a/src/audio/audio_element.cpp b/src/audio/audio_element.cpp index 90d62e76..70a59a51 100644 --- a/src/audio/audio_element.cpp +++ b/src/audio/audio_element.cpp @@ -1,4 +1,5 @@ #include "audio_element.hpp" +#include namespace audio { @@ -38,7 +39,8 @@ auto IAudioElement::SendOrBufferEvent(std::unique_ptr event) } StreamEvent* raw_event = event.release(); if (!xQueueSend(output_events_, &raw_event, 0)) { - buffered_output_.emplace_front(raw_event); + event.reset(raw_event); + buffered_output_.push_back(std::move(event)); return false; } return true; diff --git a/src/audio/audio_task.cpp b/src/audio/audio_task.cpp index 538cb201..8af3e7ff 100644 --- a/src/audio/audio_task.cpp +++ b/src/audio/audio_task.cpp @@ -66,9 +66,9 @@ void AudioTaskMain(void* args) { !element->IsOverBuffered(); if (has_work_to_do) { - ESP_LOGI(kTag, "checking for events"); + ESP_LOGD(kTag, "checking for events"); } else { - ESP_LOGI(kTag, "waiting for events"); + ESP_LOGD(kTag, "waiting for events"); } // If we have no new events to process and the element has nothing left to @@ -83,13 +83,13 @@ 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_LOGI(kTag, "marking chunk as used"); + ESP_LOGD(kTag, "marking chunk as used"); element->OnChunkProcessed(); } else { // This isn't an event that needs to be actioned immediately. Add it // to our work queue. pending_events.emplace_back(new_event); - ESP_LOGI(kTag, "deferring event"); + ESP_LOGD(kTag, "deferring event"); } // Loop again, so that we service all incoming events before doing our // possibly expensive processing. @@ -97,7 +97,7 @@ void AudioTaskMain(void* args) { } if (element->HasUnflushedOutput()) { - ESP_LOGI(kTag, "flushing output"); + ESP_LOGD(kTag, "flushing output"); } // We have no new events. Next, see if there's anything that needs to be @@ -112,7 +112,7 @@ void AudioTaskMain(void* args) { } if (element->HasUnprocessedInput()) { - ESP_LOGI(kTag, "processing input events"); + ESP_LOGD(kTag, "processing input events"); auto process_res = element->Process(); if (!process_res.has_error() || process_res.error() != OUT_OF_DATA) { // TODO: log! @@ -123,19 +123,20 @@ void AudioTaskMain(void* args) { // The element ran out of data, so now it's time to let it process more // input. while (!pending_events.empty()) { - auto& event = pending_events.front(); - ESP_LOGI(kTag, "processing event, tag %i", event->tag); + std::unique_ptr event; + pending_events.front().swap(event); + pending_events.pop_front(); + ESP_LOGD(kTag, "processing event, tag %i", event->tag); if (event->tag == StreamEvent::STREAM_INFO) { - ESP_LOGI(kTag, "processing stream info"); + ESP_LOGD(kTag, "processing stream info"); auto process_res = element->ProcessStreamInfo(*event->stream_info); - pending_events.pop_front(); if (process_res.has_error()) { // TODO(jacqueline) ESP_LOGE(kTag, "failed to process stream info"); } } else if (event->tag == StreamEvent::CHUNK_DATA) { - ESP_LOGI(kTag, "processing chunk data"); + ESP_LOGD(kTag, "processing chunk data"); auto callback = StreamEvent::CreateChunkNotification(element->InputEventQueue()); if (!xQueueSend(event->source, &callback, 0)) { @@ -145,7 +146,6 @@ void AudioTaskMain(void* args) { auto process_chunk_res = element->ProcessChunk(event->chunk_data.bytes); - pending_events.pop_front(); if (process_chunk_res.has_error()) { // TODO(jacqueline) ESP_LOGE(kTag, "failed to process chunk"); diff --git a/src/audio/chunk.cpp b/src/audio/chunk.cpp index baf2aba5..fb7f1354 100644 --- a/src/audio/chunk.cpp +++ b/src/audio/chunk.cpp @@ -25,6 +25,7 @@ ChunkReader::~ChunkReader() { auto ChunkReader::HandleNewData(cpp::span data) -> cpp::span { + assert(leftover_bytes_ + data.size() <= working_buffer_.size()); // Copy the new data onto the front for anything that was left over from the // last portion. Note: this could be optimised for the '0 leftover bytes' // case, which technically shouldn't need a copy. @@ -40,10 +41,8 @@ auto ChunkReader::HandleLeftovers(std::size_t bytes_used) -> void { leftover_bytes_ = last_data_in_working_buffer_.size() - bytes_used; if (leftover_bytes_ > 0) { auto data_to_keep = last_data_in_working_buffer_.last(leftover_bytes_); - // Copy backwards, since if more than half of the data was unused then the - // source and destination will overlap. - std::copy_backward(data_to_keep.begin(), data_to_keep.end(), - working_buffer_.begin()); + std::copy(data_to_keep.begin(), data_to_keep.end(), + working_buffer_.begin()); } } diff --git a/src/audio/fatfs_audio_input.cpp b/src/audio/fatfs_audio_input.cpp index 15823202..14176eae 100644 --- a/src/audio/fatfs_audio_input.cpp +++ b/src/audio/fatfs_audio_input.cpp @@ -20,7 +20,7 @@ static const char* kTag = "SRC"; namespace audio { // 32KiB to match the minimum himen region size. -static const std::size_t kChunkSize = 1024; +static const std::size_t kChunkSize = 32 * 1024; FatfsAudioInput::FatfsAudioInput(std::shared_ptr storage) : IAudioElement(), @@ -48,6 +48,7 @@ auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info) 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); } @@ -55,6 +56,7 @@ auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info) StreamInfo new_info(info); new_info.chunk_size = kChunkSize; + ESP_LOGI(kTag, "chunk size: %u bytes", kChunkSize); auto event = StreamEvent::CreateStreamInfo(input_events_, new_info); SendOrBufferEvent(std::unique_ptr(event)); @@ -81,7 +83,7 @@ auto FatfsAudioInput::Process() -> cpp::result { return cpp::fail(IO_ERROR); } - ESP_LOGI(kTag, "sending file data"); + ESP_LOGI(kTag, "sending file data (%u bytes)", bytes_read); dest_event->chunk_data.bytes = dest_event->chunk_data.bytes.first(bytes_read); SendOrBufferEvent(std::move(dest_event)); diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp index f499d50f..bfc88588 100644 --- a/src/audio/i2s_audio_output.cpp +++ b/src/audio/i2s_audio_output.cpp @@ -48,7 +48,8 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) -> cpp::result { // TODO(jacqueline): probs do something with the channel hey - if (!info.bits_per_sample && !info.sample_rate) { + if (!info.bits_per_sample || !info.sample_rate) { + ESP_LOGE(kTag, "audio stream missing bits or sample rate"); return cpp::fail(UNSUPPORTED_STREAM); } @@ -67,6 +68,7 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) bps = drivers::AudioDac::BPS_32; break; default: + ESP_LOGE(kTag, "dropping stream with unknown bps"); return cpp::fail(UNSUPPORTED_STREAM); } @@ -79,6 +81,7 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info) sample_rate = drivers::AudioDac::SAMPLE_RATE_48; break; default: + ESP_LOGE(kTag, "dropping stream with unknown rate"); return cpp::fail(UNSUPPORTED_STREAM); } @@ -93,11 +96,13 @@ auto I2SAudioOutput::ProcessChunk(const cpp::span& chunk) SetSoftMute(false); // TODO(jacqueline): write smaller parts with a small delay so that we can // be responsive to pause and seek commands. - return dac_->WriteData(chunk, portMAX_DELAY); + std::size_t bytes_written = dac_->WriteData(chunk, portMAX_DELAY); + ESP_LOGI(kTag, "played %u bytes", bytes_written); + return 0; } auto I2SAudioOutput::Process() -> cpp::result { - // TODO(jacqueline): Consider powering down the dac completely maybe? + // TODO(jacqueline): Play the stream in smaller sections return {}; } diff --git a/src/audio/include/audio_decoder.hpp b/src/audio/include/audio_decoder.hpp index a2591d25..d0f7469b 100644 --- a/src/audio/include/audio_decoder.hpp +++ b/src/audio/include/audio_decoder.hpp @@ -22,7 +22,7 @@ class AudioDecoder : public IAudioElement { AudioDecoder(); ~AudioDecoder(); - auto StackSizeBytes() const -> std::size_t override { return 8196; }; + 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 @@ -47,6 +47,7 @@ class AudioDecoder : public IAudioElement { std::optional stream_info_; std::optional chunk_reader_; + bool has_sent_stream_info_; std::size_t chunk_size_; bool has_samples_to_send_; bool needs_more_input_; -- cgit v1.2.3