From 8608f9367fc29e498f42f5249aa248dd2044d567 Mon Sep 17 00:00:00 2001 From: jacqueline Date: Tue, 26 Sep 2023 17:23:34 +1000 Subject: [PATCH] Tune buffer sizes and locations for I2S --- src/audio/audio_converter.cpp | 15 ++++++++------- src/audio/audio_decoder.cpp | 6 ++++-- src/audio/audio_fsm.cpp | 13 +++++++++++-- src/audio/bt_audio_output.cpp | 7 +++---- src/audio/i2s_audio_output.cpp | 5 +++-- src/audio/include/audio_sink.hpp | 5 ++--- src/audio/include/bt_audio_output.hpp | 2 +- src/audio/include/i2s_audio_output.hpp | 3 ++- src/drivers/i2s_dac.cpp | 9 +++++++-- src/drivers/include/i2s_dac.hpp | 8 +++++++- src/memory/allocator.cpp | 4 ++-- src/tasks/tasks.cpp | 4 ++-- 12 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/audio/audio_converter.cpp b/src/audio/audio_converter.cpp index 88017bf1..a57a1387 100644 --- a/src/audio/audio_converter.cpp +++ b/src/audio/audio_converter.cpp @@ -15,6 +15,7 @@ #include "esp_log.h" #include "freertos/portmacro.h" #include "freertos/projdefs.h" +#include "i2s_dac.hpp" #include "idf_additions.h" #include "resample.hpp" @@ -23,9 +24,9 @@ static constexpr char kTag[] = "mixer"; -static constexpr std::size_t kSampleBufferLength = 240 * 2 * 64; -static constexpr std::size_t kSourceBufferLength = - kSampleBufferLength * 2 * sizeof(sample::Sample); +static constexpr std::size_t kSampleBufferLength = + drivers::kI2SBufferLengthFrames * sizeof(sample::Sample) * 2; +static constexpr std::size_t kSourceBufferLength = kSampleBufferLength * 2; namespace audio { @@ -33,18 +34,18 @@ SampleConverter::SampleConverter() : commands_(xQueueCreate(1, sizeof(Args))), resampler_(nullptr), source_(xStreamBufferCreateWithCaps(kSourceBufferLength, - 1, - MALLOC_CAP_SPIRAM)) { + sizeof(sample::Sample) * 2, + MALLOC_CAP_DMA)) { input_buffer_ = { reinterpret_cast(heap_caps_calloc( - kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)), + kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_DMA)), kSampleBufferLength}; input_buffer_as_bytes_ = {reinterpret_cast(input_buffer_.data()), input_buffer_.size_bytes()}; resampled_buffer_ = { reinterpret_cast(heap_caps_calloc( - kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)), + kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_DMA)), kSampleBufferLength}; tasks::StartPersistent([&]() { Main(); }); diff --git a/src/audio/audio_decoder.cpp b/src/audio/audio_decoder.cpp index bacd5cde..7751bf37 100644 --- a/src/audio/audio_decoder.cpp +++ b/src/audio/audio_decoder.cpp @@ -26,6 +26,7 @@ #include "freertos/projdefs.h" #include "freertos/queue.h" #include "freertos/ringbuf.h" +#include "i2s_dac.hpp" #include "span.hpp" #include "audio_converter.hpp" @@ -46,7 +47,8 @@ namespace audio { static const char* kTag = "audio_dec"; -static constexpr std::size_t kCodecBufferLength = 240 * 4 * 64; +static constexpr std::size_t kCodecBufferLength = + drivers::kI2SBufferLengthFrames * sizeof(sample::Sample) * 2; Timer::Timer(const codecs::ICodec::OutputFormat& format) : current_seconds_(0), @@ -93,7 +95,7 @@ Decoder::Decoder(std::shared_ptr source, ESP_LOGI(kTag, "allocating codec buffer, %u KiB", kCodecBufferLength / 1024); codec_buffer_ = { reinterpret_cast(heap_caps_calloc( - kCodecBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)), + kCodecBufferLength, sizeof(sample::Sample), MALLOC_CAP_DMA)), kCodecBufferLength}; } diff --git a/src/audio/audio_fsm.cpp b/src/audio/audio_fsm.cpp index 06b98420..1b6b6cc8 100644 --- a/src/audio/audio_fsm.cpp +++ b/src/audio/audio_fsm.cpp @@ -26,7 +26,9 @@ #include "future_fetcher.hpp" #include "i2s_audio_output.hpp" #include "i2s_dac.hpp" +#include "idf_additions.h" #include "nvs.hpp" +#include "sample.hpp" #include "service_locator.hpp" #include "system_events.hpp" #include "track.hpp" @@ -103,10 +105,17 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) { return; } + constexpr size_t kDrainBufferSize = + drivers::kI2SBufferLengthFrames * sizeof(sample::Sample) * 2 * 8; + ESP_LOGI(kTag, "allocating drain buffer, size %u KiB", + kDrainBufferSize / 1024); + StreamBufferHandle_t stream = xStreamBufferCreateWithCaps( + kDrainBufferSize, sizeof(sample::Sample) * 2, MALLOC_CAP_DMA); + sFileSource.reset(new FatfsAudioInput(sServices->tag_parser())); - sI2SOutput.reset(new I2SAudioOutput(sServices->gpios(), + sI2SOutput.reset(new I2SAudioOutput(stream, sServices->gpios(), std::unique_ptr{*dac})); - sBtOutput.reset(new BluetoothAudioOutput(sServices->bluetooth())); + sBtOutput.reset(new BluetoothAudioOutput(stream, sServices->bluetooth())); auto& nvs = sServices->nvs(); sI2SOutput->SetMaxVolume(nvs.AmpMaxVolume().get()); diff --git a/src/audio/bt_audio_output.cpp b/src/audio/bt_audio_output.cpp index 374906fd..7be00cf5 100644 --- a/src/audio/bt_audio_output.cpp +++ b/src/audio/bt_audio_output.cpp @@ -27,10 +27,9 @@ static const char* kTag = "BTOUT"; namespace audio { -static constexpr size_t kDrainBufferSize = 48 * 1024; - -BluetoothAudioOutput::BluetoothAudioOutput(drivers::Bluetooth& bt) - : IAudioOutput(kDrainBufferSize, MALLOC_CAP_SPIRAM), bluetooth_(bt) {} +BluetoothAudioOutput::BluetoothAudioOutput(StreamBufferHandle_t s, + drivers::Bluetooth& bt) + : IAudioOutput(s), bluetooth_(bt) {} BluetoothAudioOutput::~BluetoothAudioOutput() {} diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp index 68b03145..e77c7c92 100644 --- a/src/audio/i2s_audio_output.cpp +++ b/src/audio/i2s_audio_output.cpp @@ -42,9 +42,10 @@ static constexpr uint16_t kDefaultVolume = 0x100; static constexpr size_t kDrainBufferSize = 8 * 1024; -I2SAudioOutput::I2SAudioOutput(drivers::IGpios& expander, +I2SAudioOutput::I2SAudioOutput(StreamBufferHandle_t s, + drivers::IGpios& expander, std::unique_ptr dac) - : IAudioOutput(kDrainBufferSize, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT), + : IAudioOutput(s), expander_(expander), dac_(std::move(dac)), current_config_(), diff --git a/src/audio/include/audio_sink.hpp b/src/audio/include/audio_sink.hpp index 4baa4aa1..6a7f95bf 100644 --- a/src/audio/include/audio_sink.hpp +++ b/src/audio/include/audio_sink.hpp @@ -27,10 +27,9 @@ class IAudioOutput { StreamBufferHandle_t stream_; public: - IAudioOutput(size_t buffer_size, uint32_t caps) - : stream_(xStreamBufferCreateWithCaps(buffer_size, 1, caps)) {} + IAudioOutput(StreamBufferHandle_t stream) : stream_(stream) {} - virtual ~IAudioOutput() { vStreamBufferDeleteWithCaps(stream_); } + virtual ~IAudioOutput() {} /* * Indicates whether this output is currently being sent samples. If this is diff --git a/src/audio/include/bt_audio_output.hpp b/src/audio/include/bt_audio_output.hpp index 734a7ed1..d3d2bc19 100644 --- a/src/audio/include/bt_audio_output.hpp +++ b/src/audio/include/bt_audio_output.hpp @@ -21,7 +21,7 @@ namespace audio { class BluetoothAudioOutput : public IAudioOutput { public: - BluetoothAudioOutput(drivers::Bluetooth& bt); + BluetoothAudioOutput(StreamBufferHandle_t, drivers::Bluetooth& bt); ~BluetoothAudioOutput(); auto SetInUse(bool) -> void override; diff --git a/src/audio/include/i2s_audio_output.hpp b/src/audio/include/i2s_audio_output.hpp index 17f6b71a..004cb8f8 100644 --- a/src/audio/include/i2s_audio_output.hpp +++ b/src/audio/include/i2s_audio_output.hpp @@ -20,7 +20,8 @@ namespace audio { class I2SAudioOutput : public IAudioOutput { public: - I2SAudioOutput(drivers::IGpios& expander, + I2SAudioOutput(StreamBufferHandle_t, + drivers::IGpios& expander, std::unique_ptr dac); ~I2SAudioOutput(); diff --git a/src/drivers/i2s_dac.cpp b/src/drivers/i2s_dac.cpp index 2fdd826f..b28a8ee1 100644 --- a/src/drivers/i2s_dac.cpp +++ b/src/drivers/i2s_dac.cpp @@ -41,8 +41,13 @@ static const i2s_port_t kI2SPort = I2S_NUM_0; auto I2SDac::create(IGpios& expander) -> std::optional { i2s_chan_handle_t i2s_handle; - i2s_chan_config_t channel_config = - I2S_CHANNEL_DEFAULT_CONFIG(kI2SPort, I2S_ROLE_MASTER); + i2s_chan_config_t channel_config{ + .id = kI2SPort, + .role = I2S_ROLE_MASTER, + .dma_desc_num = 2, + .dma_frame_num = kI2SBufferLengthFrames, + .auto_clear = false, + }; ESP_ERROR_CHECK(i2s_new_channel(&channel_config, &i2s_handle, NULL)); // diff --git a/src/drivers/include/i2s_dac.hpp b/src/drivers/include/i2s_dac.hpp index 6bc5b6a4..b61f7989 100644 --- a/src/drivers/include/i2s_dac.hpp +++ b/src/drivers/include/i2s_dac.hpp @@ -18,7 +18,6 @@ #include "esp_err.h" #include "freertos/FreeRTOS.h" #include "freertos/portmacro.h" -#include "freertos/ringbuf.h" #include "freertos/stream_buffer.h" #include "result.hpp" #include "span.hpp" @@ -28,6 +27,13 @@ namespace drivers { +// DMA max buffer size for I2S is 4092. We normalise to 2-channel, 16 bit +// audio, which gives us a max of 4092 / 2 / 2 (16 bits) frames. This in turn +// means that at 48kHz, we have about 21ms of budget to fill each buffer. +// We base this off of the maximum DMA size in order to minimise the amount of +// work the CPU has to do to service the DMA callbacks. +constexpr size_t kI2SBufferLengthFrames = 1023; + /** * Interface for a DAC that receives PCM samples over I2S. */ diff --git a/src/memory/allocator.cpp b/src/memory/allocator.cpp index d505ab3e..aaee08f8 100644 --- a/src/memory/allocator.cpp +++ b/src/memory/allocator.cpp @@ -13,7 +13,7 @@ void* operator new[](std::size_t sz) { ++sz; // avoid std::malloc(0) which may return nullptr on success } - if (sz > 256) { + if (sz > 512) { return heap_caps_malloc(sz, MALLOC_CAP_SPIRAM); } @@ -26,4 +26,4 @@ void operator delete[](void* ptr) noexcept { void operator delete[](void* ptr, std::size_t size) noexcept { heap_caps_free(ptr); -} \ No newline at end of file +} diff --git a/src/tasks/tasks.cpp b/src/tasks/tasks.cpp index dfcead06..689011ea 100644 --- a/src/tasks/tasks.cpp +++ b/src/tasks/tasks.cpp @@ -107,11 +107,11 @@ auto Priority() -> UBaseType_t; // highest priority. template <> auto Priority() -> UBaseType_t { - return 18; + return configMAX_PRIORITIES - 1; } template <> auto Priority() -> UBaseType_t { - return 18; + return configMAX_PRIORITIES - 1; } // After audio issues, UI jank is the most noticeable kind of scheduling-induced // slowness that the user is likely to notice or care about. Therefore we place