Move UI task to priority 0 during playback

Also other misc task cleanup

src/audio/audio_converter.cpp

@@ -23,8 +23,9 @@
 
 static constexpr char kTag[] = "mixer";
 
-static constexpr std::size_t kSourceBufferLength = 8 * 1024;
-static constexpr std::size_t kSampleBufferLength = 240 * 2 * 4;
+static constexpr std::size_t kSampleBufferLength = 240 * 2 * 8;
+static constexpr std::size_t kSourceBufferLength =
+    kSampleBufferLength * 2 * sizeof(sample::Sample);
 
 namespace audio {
 
@@ -46,7 +47,7 @@ SampleConverter::SampleConverter()
           kSampleBufferLength, sizeof(sample::Sample), MALLOC_CAP_SPIRAM)),
       kSampleBufferLength};
 
-  tasks::StartPersistent<tasks::Type::kMixer>([&]() { Main(); });
+  tasks::StartPersistent<tasks::Type::kAudioConverter>([&]() { Main(); });
 }
 
 SampleConverter::~SampleConverter() {

src/audio/audio_decoder.cpp

@@ -46,7 +46,7 @@ namespace audio {
 
 static const char* kTag = "audio_dec";
 
-static constexpr std::size_t kCodecBufferLength = 240 * 4 * 4;
+static constexpr std::size_t kCodecBufferLength = 240 * 4 * 16;
 
 Timer::Timer(const codecs::ICodec::OutputFormat& format)
     : current_seconds_(0),
@@ -79,7 +79,8 @@ auto Timer::AddSamples(std::size_t samples) -> void {
 auto Decoder::Start(std::shared_ptr<IAudioSource> source,
                     std::shared_ptr<SampleConverter> sink) -> Decoder* {
   Decoder* task = new Decoder(source, sink);
-  tasks::StartPersistent<tasks::Type::kAudio>(1, [=]() { task->Main(); });
+  tasks::StartPersistent<tasks::Type::kAudioDecoder>(1,
+                                                     [=]() { task->Main(); });
   return task;
 }
 

src/audio/audio_fsm.cpp

@@ -143,6 +143,9 @@ void Standby::react(const TogglePlayPause& ev) {
 void Playback::entry() {
   ESP_LOGI(kTag, "beginning playback");
   sOutput->SetInUse(true);
+
+  events::System().Dispatch(PlaybackStarted{});
+  events::Ui().Dispatch(PlaybackStarted{});
 }
 
 void Playback::exit() {
@@ -153,6 +156,7 @@ void Playback::exit() {
   sOutput->SetInUse(false);
 
   events::System().Dispatch(PlaybackFinished{});
+  events::Ui().Dispatch(PlaybackFinished{});
 }
 
 void Playback::react(const QueueUpdate& ev) {

src/audio/include/audio_events.hpp

@@ -17,9 +17,7 @@
 
 namespace audio {
 
-struct PlaybackStarted : tinyfsm::Event {
-  database::Track track;
-};
+struct PlaybackStarted : tinyfsm::Event {};
 
 struct PlaybackUpdate : tinyfsm::Event {
   uint32_t seconds_elapsed;

src/drivers/display.cpp

@@ -60,19 +60,6 @@ static const int kDisplayBufferSize = (kDisplayWidth * kDisplayHeight) / 10;
 DMA_ATTR static lv_color_t sBuffer1[kDisplayBufferSize];
 DMA_ATTR static lv_color_t sBuffer2[kDisplayBufferSize];
 
-struct RenderTaskArgs {
-  std::atomic<bool>* quit;
-  QueueHandle_t work_queue;
-};
-
-struct FlushArgs {
-  lv_disp_drv_t* driver;
-  const lv_area_t* area;
-  lv_color_t* color_map;
-};
-
-void RenderMain(void* raw_args);
-
 namespace drivers {
 
 /*
@@ -81,8 +68,10 @@ namespace drivers {
 extern "C" void FlushDataCallback(lv_disp_drv_t* disp_drv,
                                   const lv_area_t* area,
                                   lv_color_t* color_map) {
+  taskYIELD();
   Display* instance = static_cast<Display*>(disp_drv->user_data);
   instance->OnLvglFlush(disp_drv, area, color_map);
+  taskYIELD();
 }
 
 auto Display::Create(IGpios& expander,
@@ -155,7 +144,6 @@ auto Display::Create(IGpios& expander,
   auto display = std::make_unique<Display>(expander, handle);
 
   // Now we reset the display into a known state, then configure it
-  // TODO(jacqueline): set rotation
   ESP_LOGI(kTag, "Sending init sequences");
   for (int i = 0; i < init_data.num_sequences; i++) {
     display->SendInitialisationSequence(init_data.sequences[i]);
@@ -183,13 +171,7 @@ auto Display::Create(IGpios& expander,
 }
 
 Display::Display(IGpios& gpio, spi_device_handle_t handle)
-    : gpio_(gpio),
-      handle_(handle),
-      worker_task_(tasks::Worker::Start<tasks::Type::kUiFlush>()),
-      display_on_(false),
-      brightness_(0) {
-  SetBrightness(50);
-}
+    : gpio_(gpio), handle_(handle), display_on_(false), brightness_(0) {}
 
 Display::~Display() {
   ledc_fade_func_uninstall();
@@ -303,10 +285,6 @@ void Display::SendTransaction(TransactionType type,
 void Display::OnLvglFlush(lv_disp_drv_t* disp_drv,
                           const lv_area_t* area,
                           lv_color_t* color_map) {
-  // area is stack-allocated, so it isn't safe to reference from the flush
-  // thread.
-  lv_area_t area_copy = *area;
-  // worker_task_->Dispatch<void>([=, this]() {
   //  Ideally we want to complete a single flush as quickly as possible, so
   //  grab the bus for this entire transaction sequence.
   spi_device_acquire_bus(handle_, portMAX_DELAY);
@@ -314,13 +292,13 @@ void Display::OnLvglFlush(lv_disp_drv_t* disp_drv,
   // First we need to specify the rectangle of the display we're writing into.
   uint16_t data[2] = {0, 0};
 
-  data[0] = SPI_SWAP_DATA_TX(area_copy.x1, 16);
-  data[1] = SPI_SWAP_DATA_TX(area_copy.x2, 16);
+  data[0] = SPI_SWAP_DATA_TX(area->x1, 16);
+  data[1] = SPI_SWAP_DATA_TX(area->x2, 16);
   SendCommandWithData(displays::ST77XX_CASET, reinterpret_cast<uint8_t*>(data),
                       4);
 
-  data[0] = SPI_SWAP_DATA_TX(area_copy.y1, 16);
-  data[1] = SPI_SWAP_DATA_TX(area_copy.y2, 16);
+  data[0] = SPI_SWAP_DATA_TX(area->y1, 16);
+  data[1] = SPI_SWAP_DATA_TX(area->y2, 16);
   SendCommandWithData(displays::ST77XX_RASET, reinterpret_cast<uint8_t*>(data),
                       4);
 
@@ -332,22 +310,6 @@ void Display::OnLvglFlush(lv_disp_drv_t* disp_drv,
   spi_device_release_bus(handle_);
 
   lv_disp_flush_ready(&driver_);
-  //});
-}
-
-void RenderMain(void* raw_args) {
-  RenderTaskArgs* args = reinterpret_cast<RenderTaskArgs*>(raw_args);
-  QueueHandle_t queue = args->work_queue;
-  std::atomic<bool>* quit = args->quit;
-  delete args;
-
-  while (!quit->load()) {
-    // TODO: flush data here! Yay speed.
-  }
-
-  vQueueDelete(queue);
-  delete quit;
-  vTaskDelete(NULL);
 }
 
 }  // namespace drivers

src/drivers/include/display.hpp

@@ -52,8 +52,6 @@ class Display {
   IGpios& gpio_;
   spi_device_handle_t handle_;
 
-  std::unique_ptr<tasks::Worker> worker_task_;
-
   bool display_on_;
   uint_fast8_t brightness_;
 

src/tasks/tasks.cpp

@@ -19,44 +19,36 @@ auto Name() -> std::string;
 
 template <>
 auto Name<Type::kUi>() -> std::string {
-  return "LVGL";
+  return "ui";
 }
 template <>
-auto Name<Type::kUiFlush>() -> std::string {
-  return "DISPLAY";
+auto Name<Type::kAudioDecoder>() -> std::string {
+  return "audio_dec";
 }
 template <>
-auto Name<Type::kFileStreamer>() -> std::string {
-  return "FSTREAMER";
-}
-template <>
-auto Name<Type::kAudio>() -> std::string {
-  return "AUDIO";
-}
-template <>
-auto Name<Type::kMixer>() -> std::string {
-  return "MIXER";
+auto Name<Type::kAudioConverter>() -> std::string {
+  return "audio_conv";
 }
 template <>
 auto Name<Type::kDatabase>() -> std::string {
-  return "DB";
+  return "db_fg";
 }
 template <>
 auto Name<Type::kDatabaseBackground>() -> std::string {
-  return "DB_BG";
+  return "db_bg";
 }
 template <>
 auto Name<Type::kNvsWriter>() -> std::string {
-  return "NVS";
+  return "nvs";
 }
 
 template <Type t>
 auto AllocateStack() -> cpp::span<StackType_t>;
 
-// Decoders run on the audio task, and these sometimes require a fairly large
-// amount of stack space.
+// Decoders often require a very large amount of stack space, since they aren't
+// usually written with embedded use cases in mind.
 template <>
-auto AllocateStack<Type::kAudio>() -> cpp::span<StackType_t> {
+auto AllocateStack<Type::kAudioDecoder>() -> cpp::span<StackType_t> {
   std::size_t size = 64 * 1024;
   return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_SPIRAM)),
           size};
@@ -69,29 +61,15 @@ auto AllocateStack<Type::kUi>() -> cpp::span<StackType_t> {
   return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_SPIRAM)),
           size};
 }
-// UI flushes *must* be done from internal RAM. Thankfully, there is very little
-// stack required to perform them, and the amount of stack needed is fixed.
 template <>
-auto AllocateStack<Type::kUiFlush>() -> cpp::span<StackType_t> {
-  std::size_t size = 1024;
-  return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_DEFAULT)),
-          size};
-}
-
-template <>
-auto AllocateStack<Type::kFileStreamer>() -> cpp::span<StackType_t> {
+// PCM conversion and resampling uses a very small amount of stack. It works
+// entirely with PSRAM-allocated buffers, so no real speed gain from allocating
+// it internally.
+auto AllocateStack<Type::kAudioConverter>() -> cpp::span<StackType_t> {
   std::size_t size = 4 * 1024;
   return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_SPIRAM)),
           size};
 }
-
-template <>
-auto AllocateStack<Type::kMixer>() -> cpp::span<StackType_t> {
-  std::size_t size = 4 * 1024;
-  return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_SPIRAM)),
-          size};
-}
-
 // Leveldb is designed for non-embedded use cases, where stack space isn't so
 // much of a concern. It therefore uses an eye-wateringly large amount of stack.
 template <>
@@ -114,8 +92,8 @@ auto AllocateStack<Type::kNvsWriter>() -> cpp::span<StackType_t> {
           size};
 }
 
-// 2048 bytes in internal ram
-// 302 KiB in external ram.
+// 2 KiB in internal ram
+// 612 KiB in external ram.
 
 /*
  * Please keep the priorities below in descending order for better readability.
@@ -124,39 +102,22 @@ auto AllocateStack<Type::kNvsWriter>() -> cpp::span<StackType_t> {
 template <Type t>
 auto Priority() -> UBaseType_t;
 
-// NVS writing requires suspending one of our cores, and disabling tasks with
-// their stacks in PSRAM. Get it over and done with as soon as possible.
-template <>
-auto Priority<Type::kNvsWriter>() -> UBaseType_t {
-  return 13;
-}
-// Realtime audio is the entire point of this device, so give this task the
+// Realtime audio is the entire point of this device, so give these tasks the
 // highest priority.
 template <>
-auto Priority<Type::kMixer>() -> UBaseType_t {
-  return 12;
+auto Priority<Type::kAudioDecoder>() -> UBaseType_t {
+  return 18;
 }
 template <>
-auto Priority<Type::kAudio>() -> UBaseType_t {
-  return 11;
+auto Priority<Type::kAudioConverter>() -> UBaseType_t {
+  return 18;
 }
 // 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
-// this task directly below audio in terms of priority.
+// this task directly below audio in terms of priority. Note that during audio
+// playback, this priority will be downgraded.
 template <>
 auto Priority<Type::kUi>() -> UBaseType_t {
-  return 9;
-}
-// UI flushing should use the same priority as the UI task, so as to maximise
-// the chance of the happy case: one of our cores is writing to the screen,
-// whilst the other is simultaneously preparing the next buffer to be flushed.
-template <>
-auto Priority<Type::kUiFlush>() -> UBaseType_t {
-  return 9;
-}
-
-template <>
-auto Priority<Type::kFileStreamer>() -> UBaseType_t {
   return 10;
 }
 // Database interactions are all inherently async already, due to their
@@ -165,11 +126,18 @@ auto Priority<Type::kFileStreamer>() -> UBaseType_t {
 // priority.
 template <>
 auto Priority<Type::kDatabase>() -> UBaseType_t {
-  return 8;
+  return 2;
 }
 template <>
 auto Priority<Type::kDatabaseBackground>() -> UBaseType_t {
-  return 7;
+  return 1;
+}
+// NVS writing requires suspending one of our cores, and disabling tasks with
+// their stacks in PSRAM. Only do it when there's not more important work
+// pending.
+template <>
+auto Priority<Type::kNvsWriter>() -> UBaseType_t {
+  return 2;
 }
 
 template <Type t>
@@ -184,10 +152,6 @@ auto WorkerQueueSize<Type::kDatabaseBackground>() -> std::size_t {
   return 8;
 }
 
-template <>
-auto WorkerQueueSize<Type::kUiFlush>() -> std::size_t {
-  return 2;
-}
 template <>
 auto WorkerQueueSize<Type::kNvsWriter>() -> std::size_t {
   return 2;

src/tasks/tasks.hpp

@@ -30,18 +30,17 @@ namespace tasks {
 enum class Type {
   // The main UI task. This runs the LVGL main loop.
   kUi,
-  // Task for flushing graphics buffers to the display.
-  kUiFlush,
-  // TODO.
-  kFileStreamer,
-  // The main audio pipeline task.
-  kAudio,
-  // TODO
-  kMixer,
+  // The main audio pipeline task. Decodes files into PCM stream.
+  kAudioDecoder,
+  // Second audio task. Converts the PCM stream into one suitable for the
+  // current output (e.g. downsampling for bluetooth).
+  kAudioConverter,
   // Task for running database queries.
   kDatabase,
   // Task for internal database operations
   kDatabaseBackground,
+  // Task for interacting with NVS -- this needs to be done with an internal
+  // stack.
   kNvsWriter,
 };
 

src/ui/include/lvgl_task.hpp

@@ -40,10 +40,6 @@ class UiTask {
 
   std::shared_ptr<TouchWheelEncoder> input_device_;
   std::shared_ptr<Screen> current_screen_;
-
-  std::atomic<bool> quit_;
-  SemaphoreHandle_t frame_semaphore_;
-  TimerHandle_t frame_timer_;
 };
 
 }  // namespace ui

src/ui/include/ui_fsm.hpp

@@ -48,10 +48,9 @@ class UiState : public tinyfsm::Fsm<UiState> {
   void react(const tinyfsm::Event& ev) {}
 
   virtual void react(const system_fsm::BatteryStateChanged&);
-
-  virtual void react(const audio::PlaybackStarted&) {}
+  virtual void react(const audio::PlaybackStarted&);
+  virtual void react(const audio::PlaybackFinished&);
   virtual void react(const audio::PlaybackUpdate&) {}
-  virtual void react(const audio::PlaybackFinished&) {}
   virtual void react(const audio::QueueUpdate&) {}
 
   virtual void react(const system_fsm::KeyLockChanged&);

src/ui/lvgl_task.cpp

@@ -49,24 +49,8 @@
 namespace ui {
 
 static const char* kTag = "ui_task";
-static const TickType_t kMaxFrameRate = pdMS_TO_TICKS(100);
 
-static auto next_frame(TimerHandle_t t) {
-  SemaphoreHandle_t sem =
-      reinterpret_cast<SemaphoreHandle_t>(pvTimerGetTimerID(t));
-  xSemaphoreGive(sem);
-}
-
-UiTask::UiTask()
-    : quit_(false),
-      frame_semaphore_(xSemaphoreCreateBinary()),
-      frame_timer_(xTimerCreate("ui_frame",
-                                kMaxFrameRate,
-                                pdTRUE,
-                                frame_semaphore_,
-                                next_frame)) {
-  xTimerStart(frame_timer_, portMAX_DELAY);
-}
+UiTask::UiTask() {}
 
 UiTask::~UiTask() {
   assert(false);
@@ -98,10 +82,8 @@ auto UiTask::Main() -> void {
       current_screen_->Tick();
     }
 
-    lv_task_handler();
-
-    // Wait for the signal to loop again.
-    xSemaphoreTake(frame_semaphore_, portMAX_DELAY);
+    TickType_t delay = lv_timer_handler();
+    vTaskDelay(pdMS_TO_TICKS(std::clamp<TickType_t>(delay, 0, 100)));
   }
 }
 
@@ -114,7 +96,7 @@ auto UiTask::SetInputDevice(std::shared_ptr<TouchWheelEncoder> dev) -> void {
 
 auto UiTask::Start() -> UiTask* {
   UiTask* ret = new UiTask();
-  tasks::StartPersistent<tasks::Type::kUi>(0, [=]() { ret->Main(); });
+  tasks::StartPersistent<tasks::Type::kUi>([=]() { ret->Main(); });
   return ret;
 }
 

src/ui/ui_fsm.cpp

@@ -90,6 +90,13 @@ void UiState::react(const system_fsm::BatteryStateChanged&) {
   UpdateTopBar();
 }
 
+void UiState::react(const audio::PlaybackStarted&) {
+  vTaskPrioritySet(NULL, 0);
+}
+void UiState::react(const audio::PlaybackFinished&) {
+  vTaskPrioritySet(NULL, 10);
+}
+
 void UiState::UpdateTopBar() {
   auto battery_state = sServices->battery().State();
   bool has_queue = sServices->track_queue().GetCurrent().has_value();
@@ -251,6 +258,7 @@ void Playing::exit() {
 }
 
 void Playing::react(const audio::PlaybackStarted& ev) {
+  vTaskPrioritySet(NULL, 0);
   UpdateTopBar();
   sPlayingScreen->OnTrackUpdate();
 }
@@ -261,6 +269,7 @@ void Playing::react(const audio::PlaybackUpdate& ev) {
 
 void Playing::react(const audio::PlaybackFinished& ev) {
   UpdateTopBar();
+  vTaskPrioritySet(NULL, 10);
 }
 
 void Playing::react(const audio::QueueUpdate& ev) {