Tune buffer sizes and locations for I2S

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<sample::Sample*>(heap_caps_calloc(
-          kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)),
+          kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_DMA)),
       kSampleBufferLength};
   input_buffer_as_bytes_ = {reinterpret_cast<std::byte*>(input_buffer_.data()),
                             input_buffer_.size_bytes()};
 
   resampled_buffer_ = {
       reinterpret_cast<sample::Sample*>(heap_caps_calloc(
-          kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)),
+          kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_DMA)),
       kSampleBufferLength};
 
   tasks::StartPersistent<tasks::Type::kAudioConverter>([&]() { 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<IAudioSource> source,
   ESP_LOGI(kTag, "allocating codec buffer, %u KiB", kCodecBufferLength / 1024);
   codec_buffer_ = {
       reinterpret_cast<sample::Sample*>(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<drivers::I2SDac>{*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<drivers::I2SDac> 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<drivers::I2SDac> 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<I2SDac*> {
   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<Type::kAudioDecoder>() -> UBaseType_t {
-  return 18;
+  return configMAX_PRIORITIES - 1;
 }
 template <>
 auto Priority<Type::kAudioConverter>() -> 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