Fix playback and console issues

src/dev_console/include/console.hpp

@@ -12,7 +12,7 @@ class Console {
   auto Launch() -> void;
 
  protected:
-  virtual auto GetStackSizeKiB() -> uint16_t { return 0; }
+  virtual auto GetStackSizeKiB() -> uint16_t { return 4; }
   virtual auto RegisterExtraComponents() -> void {}
 
  private:

src/drivers/audio_playback.cpp

@@ -25,9 +25,7 @@ static const char* kSource = "src";
 static const char* kDecoder = "dec";
 static const char* kSink = "sink";
 
-namespace drivers {
+static audio_element_status_t toStatus(void* status) {
-
-static audio_element_status_t status_from_the_void(void* status) {
   uintptr_t as_pointer_int = reinterpret_cast<uintptr_t>(status);
   return static_cast<audio_element_status_t>(as_pointer_int);
 }
@@ -42,6 +40,8 @@ static void toLower(std::string& str) {
                  [](unsigned char c) { return std::tolower(c); });
 }
 
+namespace drivers {
+
 auto AudioPlayback::create(std::unique_ptr<IAudioOutput> output)
     -> cpp::result<std::unique_ptr<AudioPlayback>, Error> {
   audio_pipeline_handle_t pipeline;
@@ -108,58 +108,67 @@ AudioPlayback::~AudioPlayback() {
 }
 
 void AudioPlayback::Play(const std::string& filename) {
-  if (GetPlaybackState() != STOPPED) {
+  output_->SetSoftMute(true);
+
+  if (playback_state_ != STOPPED) {
     audio_pipeline_stop(pipeline_);
     audio_pipeline_wait_for_stop(pipeline_);
     audio_pipeline_terminate(pipeline_);
   }
 
-  current_state_ = PLAYING;
+  playback_state_ = PLAYING;
   Decoder decoder = GetDecoderForFilename(filename);
   ReconfigurePipeline(decoder);
   audio_element_set_uri(source_element_, filename.c_str());
   audio_pipeline_reset_ringbuffer(pipeline_);
   audio_pipeline_reset_elements(pipeline_);
   audio_pipeline_run(pipeline_);
-  output_->SetVolume(volume_);
+
+  output_->SetSoftMute(false);
 }
 
 void AudioPlayback::Toggle() {
-  if (GetPlaybackState() == PLAYING) {
+  if (playback_state_ == PLAYING) {
     Pause();
-  } else if (GetPlaybackState() == PAUSED) {
+  } else if (playback_state_ == PAUSED) {
     Resume();
   }
 }
 
 void AudioPlayback::Resume() {
-  if (GetPlaybackState() == PAUSED) {
+  if (playback_state_ == PAUSED) {
-    current_state_ = PLAYING;
+    ESP_LOGI(kTag, "resuming");
+    playback_state_ = PLAYING;
     audio_pipeline_resume(pipeline_);
+    output_->SetSoftMute(false);
   }
 }
 void AudioPlayback::Pause() {
   if (GetPlaybackState() == PLAYING) {
-    current_state_ = PAUSED;
+    ESP_LOGI(kTag, "pausing");
+    output_->SetSoftMute(true);
+    playback_state_ = PAUSED;
     audio_pipeline_pause(pipeline_);
   }
 }
 
-auto AudioPlayback::GetPlaybackState() -> PlaybackState {
+auto AudioPlayback::GetPlaybackState() const -> PlaybackState {
-  return current_state_;
+  return playback_state_;
 }
 
 void AudioPlayback::ProcessEvents(uint16_t max_time_ms) {
-  if (current_state_ == STOPPED) {
+  if (playback_state_ == STOPPED) {
     return;
   }
+
   while (1) {
     audio_event_iface_msg_t event;
     esp_err_t err = audio_event_iface_listen(event_interface_, &event,
                                              pdMS_TO_TICKS(max_time_ms));
     if (err != ESP_OK) {
-      ESP_LOGE(kTag, "error listening for event:%x", err);
+      // Error should only be timeouts, so use a 'failure' as an indication that
-      continue;
+      // we're out of events to process.
+      break;
     }
 
     if (event.source_type == AUDIO_ELEMENT_TYPE_ELEMENT &&
@@ -174,61 +183,65 @@ void AudioPlayback::ProcessEvents(uint16_t max_time_ms) {
     if (event.source_type == AUDIO_ELEMENT_TYPE_ELEMENT &&
         event.source == (void*)source_element_ &&
         event.cmd == AEL_MSG_CMD_REPORT_STATUS) {
-      audio_element_status_t status = status_from_the_void(event.data);
+      audio_element_status_t status = toStatus(event.data);
       if (status == AEL_STATUS_STATE_FINISHED) {
         // TODO: Could we change the uri here? hmm.
+        ESP_LOGI(kTag, "finished reading input.");
       }
     }
 
     if (event.source_type == AUDIO_ELEMENT_TYPE_ELEMENT &&
         event.source == (void*)output_->GetAudioElement() &&
         event.cmd == AEL_MSG_CMD_REPORT_STATUS) {
-      audio_element_status_t status = status_from_the_void(event.data);
+      audio_element_status_t status = toStatus(event.data);
       if (status == AEL_STATUS_STATE_FINISHED) {
         if (next_filename_ != "") {
+          ESP_LOGI(kTag, "finished writing output. enqueing next.");
           Decoder decoder = GetDecoderForFilename(next_filename_);
           if (decoder == decoder_type_) {
+            output_->SetSoftMute(true);
             audio_element_set_uri(source_element_, next_filename_.c_str());
             audio_pipeline_reset_ringbuffer(pipeline_);
             audio_pipeline_reset_elements(pipeline_);
             audio_pipeline_change_state(pipeline_, AEL_STATE_INIT);
             audio_pipeline_run(pipeline_);
+            output_->SetSoftMute(true);
           } else {
             Play(next_filename_);
           }
           next_filename_ = "";
         } else {
-          audio_pipeline_stop(pipeline_);
+          ESP_LOGI(kTag, "finished writing output. stopping.");
           audio_pipeline_wait_for_stop(pipeline_);
           audio_pipeline_terminate(pipeline_);
-          current_state_ = STOPPED;
+          playback_state_ = STOPPED;
         }
         return;
       }
     }
 
     if (event.need_free_data) {
-      ESP_LOGI(kTag, "freeing event data");
+      // AFAICT this never happens in practice, but it doesn't hurt to follow
+      // the api here anyway.
       free(event.data);
     }
   }
 }
 
-void AudioPlayback::set_next_file(const std::string& filename) {
+void AudioPlayback::SetNextFile(const std::string& filename) {
   next_filename_ = filename;
 }
 
-void AudioPlayback::set_volume(uint8_t volume) {
+void AudioPlayback::SetVolume(uint8_t volume) {
-  volume_ = volume;
-  // TODO: don't write immediately if we're muted to change track or similar.
   output_->SetVolume(volume);
 }
 
-auto AudioPlayback::volume() -> uint8_t {
+auto AudioPlayback::GetVolume() const -> uint8_t {
-  return volume_;
+  return output_->GetVolume();
 }
 
-auto AudioPlayback::GetDecoderForFilename(std::string filename) -> Decoder {
+auto AudioPlayback::GetDecoderForFilename(std::string filename) const
+    -> Decoder {
   toLower(filename);
   if (endsWith(filename, "mp3")) {
     return MP3;
@@ -255,7 +268,8 @@ auto AudioPlayback::GetDecoderForFilename(std::string filename) -> Decoder {
   return NONE;
 }
 
-auto AudioPlayback::CreateDecoder(Decoder decoder) -> audio_element_handle_t {
+auto AudioPlayback::CreateDecoder(Decoder decoder) const
+    -> audio_element_handle_t {
   if (decoder == MP3) {
     mp3_decoder_cfg_t config = DEFAULT_MP3_DECODER_CONFIG();
     return mp3_decoder_init(&config);

src/drivers/i2s_audio_output.cpp

@@ -82,13 +82,28 @@ auto I2SAudioOutput::create(GpioExpander* expander)
 
 I2SAudioOutput::I2SAudioOutput(std::unique_ptr<AudioDac>& dac,
                                audio_element_handle_t element)
-    : IAudioOutput(element), dac_(std::move(dac)) {}
+    : IAudioOutput(element), dac_(std::move(dac)) {
+  volume_ = 255;
+}
 I2SAudioOutput::~I2SAudioOutput() {
   // TODO: power down the DAC.
 }
 
 auto I2SAudioOutput::SetVolume(uint8_t volume) -> void {
-  dac_->WriteVolume(255);
+  volume_ = volume;
+  if (!is_soft_muted_) {
+    dac_->WriteVolume(volume);
+  }
+}
+
+auto I2SAudioOutput::SetSoftMute(bool enabled) -> void {
+  if (enabled) {
+    is_soft_muted_ = true;
+    dac_->WriteVolume(255);
+  } else {
+    is_soft_muted_ = false;
+    dac_->WriteVolume(volume_);
+  }
 }
 
 auto I2SAudioOutput::Configure(audio_element_info_t& info) -> void {

src/drivers/include/audio_output.hpp

@@ -15,10 +15,14 @@ class IAudioOutput {
   auto GetAudioElement() -> audio_element_handle_t { return element_; }
 
   virtual auto SetVolume(uint8_t volume) -> void = 0;
+  virtual auto GetVolume() const -> uint8_t { return volume_; }
+
   virtual auto Configure(audio_element_info_t& info) -> void = 0;
+  virtual auto SetSoftMute(bool enabled) -> void = 0;
 
  protected:
   audio_element_handle_t element_;
+  uint8_t volume_;
 };
 
 }  // namespace drivers

src/drivers/include/audio_playback.hpp

@@ -44,14 +44,14 @@ class AudioPlayback {
    *
    * Any value set in `set_next_file` is cleared by this method.
    */
-  void Play(const std::string& filename);
+  auto Play(const std::string& filename) -> void;
   /* Toogle between resumed and paused. */
-  void Toggle();
+  auto Toggle() -> void;
-  void Resume();
+  auto Resume() -> void;
-  void Pause();
+  auto Pause() -> void;
 
   enum PlaybackState { PLAYING, PAUSED, STOPPED };
-  auto GetPlaybackState() -> PlaybackState;
+  auto GetPlaybackState() const -> PlaybackState;
 
   /*
    * Handles any pending events from the underlying audio pipeline. This must
@@ -59,34 +59,32 @@ class AudioPlayback {
    * different audio types (e.g. sample rate, bit depth), and for gapless
    * playback.
    */
-  void ProcessEvents(uint16_t max_time_ms);
+  auto ProcessEvents(uint16_t max_time_ms) -> void;
 
   /*
    * Sets the file that should be played immediately after the current file
    * finishes. This is used for gapless playback
    */
-  void set_next_file(const std::string& filename);
+  auto SetNextFile(const std::string& filename) -> void;
 
-  void set_volume(uint8_t volume);
+  auto SetVolume(uint8_t volume) -> void;
-  auto volume() -> uint8_t;
+  auto GetVolume() const -> uint8_t;
 
   // Not copyable or movable.
   AudioPlayback(const AudioPlayback&) = delete;
   AudioPlayback& operator=(const AudioPlayback&) = delete;
 
  private:
-  PlaybackState current_state_;
+  PlaybackState playback_state_;
 
   enum Decoder { NONE, MP3, AMR, OPUS, OGG, FLAC, WAV, AAC };
-  auto GetDecoderForFilename(std::string filename) -> Decoder;
+  auto GetDecoderForFilename(std::string filename) const -> Decoder;
-  auto CreateDecoder(Decoder decoder) -> audio_element_handle_t;
+  auto CreateDecoder(Decoder decoder) const -> audio_element_handle_t;
   auto ReconfigurePipeline(Decoder decoder) -> void;
 
   std::unique_ptr<IAudioOutput> output_;
-  std::mutex playback_lock_;
 
   std::string next_filename_ = "";
-  uint8_t volume_;
 
   audio_pipeline_handle_t pipeline_;
   audio_element_handle_t source_element_;

src/drivers/include/i2s_audio_output.hpp

@@ -24,9 +24,11 @@ class I2SAudioOutput : public IAudioOutput {
 
   virtual auto SetVolume(uint8_t volume) -> void;
   virtual auto Configure(audio_element_info_t& info) -> void;
+  virtual auto SetSoftMute(bool enabled) -> void;
 
  private:
   std::unique_ptr<AudioDac> dac_;
+  bool is_soft_muted_ = false;
 };
 
 }  // namespace drivers

src/main/app_console.cpp

@@ -1,7 +1,9 @@
 #include "app_console.hpp"
 
 #include <dirent.h>
+#include <cstdint>
 #include <cstdio>
+#include <cstdlib>
 #include <iostream>
 #include <string>
 #include "esp_console.h"
@@ -10,16 +12,22 @@ namespace console {
 
 static AppConsole* sInstance = nullptr;
 
+std::string toSdPath(std::string filepath) {
+  return std::string(drivers::kStoragePath) + "/" + filepath;
+}
+
 int CmdListDir(int argc, char** argv) {
   static const std::string usage = "usage: ls [directory]";
   if (argc > 2) {
     std::cout << usage << std::endl;
     return 1;
   }
-  std::string path = drivers::kStoragePath;
+
+  std::string path;
   if (argc == 2) {
-    path += "/";
+    path = toSdPath(argv[1]);
-    path += argv[1];
+  } else {
+    path = toSdPath("");
   }
 
   DIR* dir;
@@ -44,15 +52,15 @@ void RegisterListDir() {
 
 int CmdPlayFile(int argc, char** argv) {
   static const std::string usage = "usage: play [file]";
-  if (argc != 2) {
+  if (argc < 2 || argc > 3) {
     std::cout << usage << std::endl;
     return 1;
   }
-  std::string path = drivers::kStoragePath;
-  path += "/";
-  path += argv[1];
 
-  sInstance->playback_->Play(path.c_str());
+  sInstance->playback_->Play(toSdPath(argv[1]));
+  if (argc == 3) {
+    sInstance->playback_->SetNextFile(toSdPath(argv[2]));
+  }
 
   return 0;
 }
@@ -87,8 +95,35 @@ void RegisterToggle() {
   esp_console_cmd_register(&cmd);
 }
 
-AppConsole::AppConsole(std::unique_ptr<drivers::AudioPlayback> playback)
+int CmdVolume(int argc, char** argv) {
-    : playback_(std::move(playback)) {
+  static const std::string usage = "usage: volume [0-255]";
+  if (argc != 2) {
+    std::cout << usage << std::endl;
+    return 1;
+  }
+
+  long int raw_vol = strtol(argv[1], NULL, 10);
+  if (raw_vol < 0 || raw_vol > 255) {
+    std::cout << usage << std::endl;
+    return 1;
+  }
+
+  sInstance->playback_->SetVolume((uint8_t)raw_vol);
+
+  return 0;
+}
+
+void RegisterVolume() {
+  esp_console_cmd_t cmd{
+      .command = "vol",
+      .help = "Changes the volume (between 0 and 254. 255 is mute.)",
+      .hint = NULL,
+      .func = &CmdVolume,
+      .argtable = NULL};
+  esp_console_cmd_register(&cmd);
+}
+
+AppConsole::AppConsole(drivers::AudioPlayback* playback) : playback_(playback) {
   sInstance = this;
 }
 AppConsole::~AppConsole() {
@@ -99,6 +134,7 @@ auto AppConsole::RegisterExtraComponents() -> void {
   RegisterListDir();
   RegisterPlayFile();
   RegisterToggle();
+  RegisterVolume();
 }
 
 }  // namespace console

src/main/app_console.hpp

@@ -9,10 +9,10 @@ namespace console {
 
 class AppConsole : public Console {
  public:
-  AppConsole(std::unique_ptr<drivers::AudioPlayback> playback);
+  AppConsole(drivers::AudioPlayback* playback);
   virtual ~AppConsole();
 
-  std::unique_ptr<drivers::AudioPlayback> playback_;
+  drivers::AudioPlayback* playback_;
 
  protected:
   virtual auto RegisterExtraComponents() -> void;

src/main/main.cpp

@@ -130,9 +130,10 @@ extern "C" void app_main(void) {
   }
   std::unique_ptr<drivers::AudioPlayback> playback =
       std::move(playback_res.value());
+  playback->SetVolume(130);
 
   ESP_LOGI(TAG, "Launch console");
-  console::AppConsole console(std::move(playback));
+  console::AppConsole console(playback.get());
   console.Launch();
 
   while (1) {