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

@@ -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);
+  auto result = codecs::CreateCodecForFile(info.path.value_or(""));
-  if (result) {
+  if (result.has_value()) {
-    current_codec_ = std::move(*result);
+    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
+  stream_info_ = info;
-  // sample rate, chunk size, etc.
+  has_sent_stream_info_ = false;
-  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<StreamEvent>(event));
 
   return {};
 }
@@ -78,10 +73,13 @@ auto AudioDecoder::ProcessChunk(const cpp::span<std::byte>& chunk)
     -> cpp::result<size_t, AudioProcessingError> {
   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<void, AudioProcessingError> {
     // 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<StreamEvent>(event));
+      }
+
       auto chunk = std::unique_ptr<StreamEvent>(
           StreamEvent::CreateChunkData(input_events_, chunk_size_));
       auto write_res =

src/audio/audio_element.cpp

@@ -1,4 +1,5 @@
 #include "audio_element.hpp"
+#include <memory>
 
 namespace audio {
 
@@ -38,7 +39,8 @@ auto IAudioElement::SendOrBufferEvent(std::unique_ptr<StreamEvent> 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;

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();
+        std::unique_ptr<StreamEvent> event;
-        ESP_LOGI(kTag, "processing event, tag %i", event->tag);
+        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");

src/audio/chunk.cpp

@@ -25,6 +25,7 @@ ChunkReader::~ChunkReader() {
 
 auto ChunkReader::HandleNewData(cpp::span<std::byte> data)
     -> cpp::span<std::byte> {
+  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,9 +41,7 @@ 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
+    std::copy(data_to_keep.begin(), data_to_keep.end(),
-    // source and destination will overlap.
-    std::copy_backward(data_to_keep.begin(), data_to_keep.end(),
               working_buffer_.begin());
   }
 }

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<drivers::SdStorage> storage)
     : IAudioElement(),
@@ -48,6 +48,7 @@ auto FatfsAudioInput::ProcessStreamInfo(const StreamInfo& info)
   std::string path = *info.path;
   FRESULT res = f_open(&current_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<StreamEvent>(event));
@@ -81,7 +83,7 @@ auto FatfsAudioInput::Process() -> cpp::result<void, AudioProcessingError> {
       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));

src/audio/i2s_audio_output.cpp

@@ -48,7 +48,8 @@ auto I2SAudioOutput::ProcessStreamInfo(const StreamInfo& info)
     -> cpp::result<void, AudioProcessingError> {
   // 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<std::byte>& 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<void, AudioProcessingError> {
-  // TODO(jacqueline): Consider powering down the dac completely maybe?
+  // TODO(jacqueline): Play the stream in smaller sections
   return {};
 }
 

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<StreamInfo> stream_info_;
   std::optional<ChunkReader> chunk_reader_;
 
+  bool has_sent_stream_info_;
   std::size_t chunk_size_;
   bool has_samples_to_send_;
   bool needs_more_input_;

src/codecs/mad.cpp

@@ -43,7 +43,8 @@ auto MadMp3Decoder::GetOutputFormat() -> OutputFormat {
   return OutputFormat{
       .num_channels = static_cast<uint8_t>(synth_.pcm.channels),
       .bits_per_sample = 24,
-      .sample_rate_hz = synth_.pcm.samplerate,
+      .sample_rate_hz =
+          synth_.pcm.samplerate == 0 ? 44100 : synth_.pcm.samplerate,
   };
 }
 

src/drivers/dac.cpp

@@ -1,6 +1,7 @@
 #include "dac.hpp"
 
 #include <cstdint>
+#include <cstring>
 
 #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_CHANNEL_DEFAULT_CONFIG(kI2SPort, I2S_ROLE_MASTER);
-  i2s_new_channel(&channel_config, &i2s_handle, NULL);
+  // 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
+  ESP_ERROR_CHECK(i2s_channel_enable(dac->i2s_handle_));
-  // meaningful state?
-  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 ||
+        return state == STANDBY || state == WAIT_FOR_CP;
-               state == STANDBY;
       });
   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)),
+      clock_config_(I2S_STD_CLK_DEFAULT_CONFIG(44100)),
-      slot_config_(I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_32BIT,
+      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<std::byte>& data, TickType_t max_wait)
     -> std::size_t {
   std::size_t res = 0;
+  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;
 }
 

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;
 }