From ef8d404b15e6d32506617a95aa3161fbe59ecdaf Mon Sep 17 00:00:00 2001 From: jacqueline Date: Mon, 30 Jan 2023 15:52:54 +1100 Subject: [PATCH] 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 +- src/codecs/mad.cpp | 3 +- src/drivers/dac.cpp | 41 +++++++++++++++---------- src/main/app_console.cpp | 5 ++-- 10 files changed, 92 insertions(+), 58 deletions(-) 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_; diff --git a/src/codecs/mad.cpp b/src/codecs/mad.cpp index a8c2fb11..dc75a892 100644 --- a/src/codecs/mad.cpp +++ b/src/codecs/mad.cpp @@ -43,7 +43,8 @@ auto MadMp3Decoder::GetOutputFormat() -> OutputFormat { return OutputFormat{ .num_channels = static_cast(synth_.pcm.channels), .bits_per_sample = 24, - .sample_rate_hz = synth_.pcm.samplerate, + .sample_rate_hz = + synth_.pcm.samplerate == 0 ? 44100 : synth_.pcm.samplerate, }; } diff --git a/src/drivers/dac.cpp b/src/drivers/dac.cpp index 96a204a7..214feb28 100644 --- a/src/drivers/dac.cpp +++ b/src/drivers/dac.cpp @@ -1,6 +1,7 @@ #include "dac.hpp" #include +#include #include "assert.h" #include "driver/i2c.h" @@ -31,8 +32,10 @@ auto AudioDac::create(GpioExpander* expander) // TODO: tune. i2s_chan_handle_t i2s_handle; i2s_chan_config_t channel_config = - I2S_CHANNEL_DEFAULT_CONFIG(I2S_NUM_AUTO, I2S_ROLE_MASTER); - i2s_new_channel(&channel_config, &i2s_handle, NULL); + I2S_CHANNEL_DEFAULT_CONFIG(kI2SPort, I2S_ROLE_MASTER); + // Auto clear to trigger soft-mute when we run out of data. + channel_config.auto_clear = true; + ESP_ERROR_CHECK(i2s_new_channel(&channel_config, &i2s_handle, NULL)); // // First, instantiate the instance so it can do all of its power on // configuration. @@ -44,7 +47,7 @@ auto AudioDac::create(GpioExpander* expander) i2s_std_config_t i2s_config = { .clk_cfg = dac->clock_config_, .slot_cfg = dac->slot_config_, - .gpio_cfg = {.mclk = GPIO_NUM_0, + .gpio_cfg = {.mclk = I2S_GPIO_UNUSED, // PCM5122 is self-clocking .bclk = GPIO_NUM_26, .ws = GPIO_NUM_27, .dout = GPIO_NUM_5, @@ -63,9 +66,7 @@ auto AudioDac::create(GpioExpander* expander) return cpp::fail(Error::FAILED_TO_INSTALL_I2S); } - // TODO: does starting the channel mean the dac will boot into a more - // meaningful state? - i2s_channel_enable(dac->i2s_handle_); + ESP_ERROR_CHECK(i2s_channel_enable(dac->i2s_handle_)); // Now let's double check that the DAC itself came up whilst we we working. bool is_booted = dac->WaitForPowerState( @@ -79,10 +80,12 @@ auto AudioDac::create(GpioExpander* expander) dac->WriteRegister(Register::DE_EMPHASIS, 1 << 4); dac->WriteVolume(255); + // We already started the I2S channel with a default clock rate, but sending + // only zeros. The DAC should see this and automatically enter standby (if + // it's still waiting for the charge pump then that's also okay.) bool is_configured = dac->WaitForPowerState([](bool booted, PowerState state) { - return state == WAIT_FOR_CP || state == RAMP_UP || state == RUN || - state == STANDBY; + return state == STANDBY || state == WAIT_FOR_CP; }); if (!is_configured) { return cpp::fail(Error::FAILED_TO_CONFIGURE); @@ -94,8 +97,8 @@ auto AudioDac::create(GpioExpander* expander) AudioDac::AudioDac(GpioExpander* gpio, i2s_chan_handle_t i2s_handle) : gpio_(gpio), i2s_handle_(i2s_handle), - clock_config_(I2S_STD_CLK_DEFAULT_CONFIG(48000)), - slot_config_(I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_32BIT, + clock_config_(I2S_STD_CLK_DEFAULT_CONFIG(44100)), + slot_config_(I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_STEREO)) { gpio_->set_pin(GpioExpander::AUDIO_POWER_ENABLE, true); gpio_->Write(); @@ -158,23 +161,29 @@ auto AudioDac::Reconfigure(BitsPerSample bps, SampleRate rate) -> bool { // TODO(jacqueline): investigate how reliable the auto-clocking of the dac // is. We might need to explicit reconfigure the dac here as well if it's not // good enough. - i2s_channel_disable(i2s_handle_); + ESP_ERROR_CHECK(i2s_channel_disable(i2s_handle_)); slot_config_.slot_bit_width = (i2s_slot_bit_width_t)bps; - i2s_channel_reconfig_std_slot(i2s_handle_, &slot_config_); + ESP_ERROR_CHECK(i2s_channel_reconfig_std_slot(i2s_handle_, &slot_config_)); - // TODO: update mclk multiple as well if needed? clock_config_.sample_rate_hz = rate; - i2s_channel_reconfig_std_clock(i2s_handle_, &clock_config_); + clock_config_.mclk_multiple = + bps == BPS_24 ? I2S_MCLK_MULTIPLE_384 : I2S_MCLK_MULTIPLE_256; + ESP_ERROR_CHECK(i2s_channel_reconfig_std_clock(i2s_handle_, &clock_config_)); - i2s_channel_enable(i2s_handle_); + ESP_ERROR_CHECK(i2s_channel_enable(i2s_handle_)); return true; } auto AudioDac::WriteData(const cpp::span& data, TickType_t max_wait) -> std::size_t { std::size_t res = 0; - i2s_channel_write(i2s_handle_, data.data(), data.size(), &res, max_wait); + esp_err_t err = + i2s_channel_write(i2s_handle_, data.data(), data.size(), &res, max_wait); + if (err == ESP_ERR_TIMEOUT) { + return res; + } + ESP_ERROR_CHECK(err); return res; } diff --git a/src/main/app_console.cpp b/src/main/app_console.cpp index 281454dc..a0ada735 100644 --- a/src/main/app_console.cpp +++ b/src/main/app_console.cpp @@ -55,12 +55,13 @@ void RegisterListDir() { int CmdPlayFile(int argc, char** argv) { static const std::string usage = "usage: play [file]"; - if (argc < 2 || argc > 3) { + if (argc != 2) { std::cout << usage << std::endl; return 1; } - sInstance->playback_->Play(toSdPath(argv[1])); + std::string path = "/"; + sInstance->playback_->Play(path + argv[1]); return 0; }