From 7523772886ca37cf05d0ad5b24f6d520e314242e Mon Sep 17 00:00:00 2001 From: jacqueline Date: Mon, 16 Oct 2023 13:52:43 +1100 Subject: [PATCH] Decouple play/pause from output on/off I think this was the cause of toggling play/pause making audio go away. Or at least I can't repro that bug anymore, anyway. --- src/audio/audio_fsm.cpp | 7 ++-- src/audio/bt_audio_output.cpp | 4 +-- src/audio/i2s_audio_output.cpp | 15 +++++--- src/audio/include/audio_sink.hpp | 8 ++++- src/audio/include/bt_audio_output.hpp | 2 +- src/audio/include/i2s_audio_output.hpp | 3 +- src/drivers/i2s_dac.cpp | 47 ++++++++++++-------------- src/drivers/include/i2s_dac.hpp | 3 ++ 8 files changed, 53 insertions(+), 36 deletions(-) diff --git a/src/audio/audio_fsm.cpp b/src/audio/audio_fsm.cpp index e470300f..5020a6ef 100644 --- a/src/audio/audio_fsm.cpp +++ b/src/audio/audio_fsm.cpp @@ -81,6 +81,7 @@ void AudioState::react(const OutputModeChanged& ev) { // TODO: handle SetInUse ESP_LOGI(kTag, "output mode changed"); auto new_mode = sServices->nvs().OutputMode(); + sOutput->SetMode(IAudioOutput::Modes::kOff); switch (new_mode) { case drivers::NvsStorage::Output::kBluetooth: sOutput = sBtOutput; @@ -89,6 +90,7 @@ void AudioState::react(const OutputModeChanged& ev) { sOutput = sI2SOutput; break; } + sOutput->SetMode(IAudioOutput::Modes::kOnPaused); } namespace states { @@ -125,6 +127,7 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) { } else { sOutput = sBtOutput; } + sOutput->SetMode(IAudioOutput::Modes::kOnPaused); sSampleConverter.reset(new SampleConverter()); sSampleConverter->SetOutput(sOutput); @@ -170,7 +173,7 @@ void Standby::react(const TogglePlayPause& ev) { void Playback::entry() { ESP_LOGI(kTag, "beginning playback"); - sOutput->SetInUse(true); + sOutput->SetMode(IAudioOutput::Modes::kOnPlaying); events::System().Dispatch(PlaybackStarted{}); events::Ui().Dispatch(PlaybackStarted{}); @@ -181,7 +184,7 @@ void Playback::exit() { // TODO(jacqueline): Second case where it's useful to wait for the i2s buffer // to drain. vTaskDelay(pdMS_TO_TICKS(10)); - sOutput->SetInUse(false); + sOutput->SetMode(IAudioOutput::Modes::kOnPaused); events::System().Dispatch(PlaybackFinished{}); events::Ui().Dispatch(PlaybackFinished{}); diff --git a/src/audio/bt_audio_output.cpp b/src/audio/bt_audio_output.cpp index 7be00cf5..367710db 100644 --- a/src/audio/bt_audio_output.cpp +++ b/src/audio/bt_audio_output.cpp @@ -33,8 +33,8 @@ BluetoothAudioOutput::BluetoothAudioOutput(StreamBufferHandle_t s, BluetoothAudioOutput::~BluetoothAudioOutput() {} -auto BluetoothAudioOutput::SetInUse(bool in_use) -> void { - if (in_use) { +auto BluetoothAudioOutput::SetMode(Modes mode) -> void { + if (mode == Modes::kOnPlaying) { bluetooth_.SetSource(stream()); } else { bluetooth_.SetSource(nullptr); diff --git a/src/audio/i2s_audio_output.cpp b/src/audio/i2s_audio_output.cpp index e77c7c92..fb52a7a8 100644 --- a/src/audio/i2s_audio_output.cpp +++ b/src/audio/i2s_audio_output.cpp @@ -48,6 +48,7 @@ I2SAudioOutput::I2SAudioOutput(StreamBufferHandle_t s, : IAudioOutput(s), expander_(expander), dac_(std::move(dac)), + current_mode_(Modes::kOff), current_config_(), left_difference_(0), current_volume_(0), @@ -60,12 +61,18 @@ I2SAudioOutput::~I2SAudioOutput() { dac_->SetSource(nullptr); } -auto I2SAudioOutput::SetInUse(bool in_use) -> void { - if (in_use) { - dac_->Start(); - } else { +auto I2SAudioOutput::SetMode(Modes mode) -> void { + if (mode == current_mode_) { + return; + } + if (mode == Modes::kOff) { dac_->Stop(); + return; + } else if (current_mode_ == Modes::kOff) { + dac_->Start(); } + current_mode_ = mode; + dac_->SetPaused(mode == Modes::kOnPaused); } auto I2SAudioOutput::SetVolumeImbalance(int_fast8_t balance) -> void { diff --git a/src/audio/include/audio_sink.hpp b/src/audio/include/audio_sink.hpp index 6a7f95bf..c37e51fb 100644 --- a/src/audio/include/audio_sink.hpp +++ b/src/audio/include/audio_sink.hpp @@ -31,11 +31,17 @@ class IAudioOutput { virtual ~IAudioOutput() {} + enum class Modes { + kOff, + kOnPaused, + kOnPlaying, + }; + /* * Indicates whether this output is currently being sent samples. If this is * false, the output should place itself into a low power state. */ - virtual auto SetInUse(bool) -> void = 0; + virtual auto SetMode(Modes) -> void = 0; virtual auto SetVolumeImbalance(int_fast8_t balance) -> void = 0; virtual auto SetVolume(uint_fast8_t percent) -> void = 0; diff --git a/src/audio/include/bt_audio_output.hpp b/src/audio/include/bt_audio_output.hpp index d3d2bc19..6e5d0720 100644 --- a/src/audio/include/bt_audio_output.hpp +++ b/src/audio/include/bt_audio_output.hpp @@ -24,7 +24,7 @@ class BluetoothAudioOutput : public IAudioOutput { BluetoothAudioOutput(StreamBufferHandle_t, drivers::Bluetooth& bt); ~BluetoothAudioOutput(); - auto SetInUse(bool) -> void override; + auto SetMode(Modes) -> void override; auto SetVolumeImbalance(int_fast8_t balance) -> void override; auto SetVolume(uint_fast8_t percent) -> void override; diff --git a/src/audio/include/i2s_audio_output.hpp b/src/audio/include/i2s_audio_output.hpp index 004cb8f8..0f51a11c 100644 --- a/src/audio/include/i2s_audio_output.hpp +++ b/src/audio/include/i2s_audio_output.hpp @@ -25,7 +25,7 @@ class I2SAudioOutput : public IAudioOutput { std::unique_ptr dac); ~I2SAudioOutput(); - auto SetInUse(bool) -> void override; + auto SetMode(Modes) -> void override; auto SetMaxVolume(uint16_t) -> void; auto SetVolumeDb(uint16_t) -> void; @@ -46,6 +46,7 @@ class I2SAudioOutput : public IAudioOutput { drivers::IGpios& expander_; std::unique_ptr dac_; + Modes current_mode_; std::optional current_config_; int_fast8_t left_difference_; uint16_t current_volume_; diff --git a/src/drivers/i2s_dac.cpp b/src/drivers/i2s_dac.cpp index b28a8ee1..9ff6e380 100644 --- a/src/drivers/i2s_dac.cpp +++ b/src/drivers/i2s_dac.cpp @@ -111,38 +111,23 @@ I2SDac::~I2SDac() { auto I2SDac::Start() -> void { std::lock_guard lock(configure_mutex_); - - gpio_.WriteSync(IGpios::Pin::kAmplifierUnmute, false); - - // Ensure the DAC powers up to a muted state. - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b10); - - // Enable MCLK; this has the side effect of triggering the DAC's startup - // sequence. - i2s_channel_enable(i2s_handle_); - i2s_active_ = true; - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b11); - gpio_.WriteSync(IGpios::Pin::kAmplifierUnmute, true); } auto I2SDac::Stop() -> void { std::lock_guard lock(configure_mutex_); - - // Mute the DAC. - wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b10); - vTaskDelay(pdMS_TO_TICKS(5)); - // Silence the output. - gpio_.WriteSync(IGpios::Pin::kAmplifierUnmute, false); - - vTaskDelay(pdMS_TO_TICKS(5)); - - // Turn everything off. + set_channel(false); wm8523::WriteRegister(wm8523::Register::kPsCtrl, 0b0); - i2s_channel_disable(i2s_handle_); - i2s_active_ = false; +} - gpio_.WriteSync(IGpios::Pin::kAmplifierEnable, false); +auto I2SDac::SetPaused(bool paused) -> void { + if (paused) { + gpio_.WriteSync(IGpios::Pin::kAmplifierUnmute, false); + set_channel(false); + } else { + set_channel(true); + gpio_.WriteSync(IGpios::Pin::kAmplifierUnmute, true); + } } auto I2SDac::Reconfigure(Channels ch, BitsPerSample bps, SampleRate rate) @@ -261,4 +246,16 @@ auto I2SDac::SetSource(StreamBufferHandle_t buffer) -> void { } } +auto I2SDac::set_channel(bool enabled) -> void { + if (i2s_active_ == enabled) { + return; + } + i2s_active_ = enabled; + if (enabled) { + i2s_channel_enable(i2s_handle_); + } else { + i2s_channel_disable(i2s_handle_); + } +} + } // namespace drivers diff --git a/src/drivers/include/i2s_dac.hpp b/src/drivers/include/i2s_dac.hpp index b61f7989..d66d9762 100644 --- a/src/drivers/include/i2s_dac.hpp +++ b/src/drivers/include/i2s_dac.hpp @@ -46,6 +46,7 @@ class I2SDac { auto Start() -> void; auto Stop() -> void; + auto SetPaused(bool) -> void; enum Channels { CHANNELS_MONO, @@ -75,6 +76,8 @@ class I2SDac { I2SDac& operator=(const I2SDac&) = delete; private: + auto set_channel(bool) -> void; + IGpios& gpio_; i2s_chan_handle_t i2s_handle_; bool i2s_active_;