From 41e0605f17a784e8f125b3ad10ddfe5ef63337d9 Mon Sep 17 00:00:00 2001 From: jacqueline Date: Mon, 8 Jul 2024 15:06:43 +1000 Subject: [PATCH] Give PcmBuffer pairs a name, and wire them up in the audio stack --- src/drivers/bluetooth.cpp | 35 ++++++++++------------ src/drivers/i2s_dac.cpp | 20 +++++++------ src/drivers/include/drivers/bluetooth.hpp | 6 ++-- src/drivers/include/drivers/i2s_dac.hpp | 8 ++--- src/drivers/include/drivers/pcm_buffer.hpp | 12 ++++++++ src/drivers/pcm_buffer.cpp | 2 +- src/tangara/audio/audio_fsm.cpp | 33 ++++++++++++-------- src/tangara/audio/audio_fsm.hpp | 2 +- src/tangara/audio/bt_audio_output.cpp | 8 ++--- src/tangara/audio/bt_audio_output.hpp | 4 +-- src/tangara/audio/i2s_audio_output.cpp | 6 ++-- src/tangara/audio/i2s_audio_output.hpp | 4 +-- 12 files changed, 79 insertions(+), 61 deletions(-) diff --git a/src/drivers/bluetooth.cpp b/src/drivers/bluetooth.cpp index 23c4f8d8..acb38ce4 100644 --- a/src/drivers/bluetooth.cpp +++ b/src/drivers/bluetooth.cpp @@ -37,8 +37,7 @@ namespace drivers { [[maybe_unused]] static constexpr char kTag[] = "bluetooth"; -DRAM_ATTR static PcmBuffer* sStream1 = nullptr; -DRAM_ATTR static PcmBuffer* sStream2 = nullptr; +DRAM_ATTR static OutputBuffers* sStreams = nullptr; DRAM_ATTR static std::atomic sVolumeFactor = 1.f; static tasks::WorkerPool* sBgWorker; @@ -97,15 +96,16 @@ IRAM_ATTR auto a2dp_data_cb(uint8_t* buf, int32_t buf_size) -> int32_t { if (buf == nullptr || buf_size <= 0) { return 0; } - PcmBuffer* stream1 = sStream1; - PcmBuffer* stream2 = sStream2; - if (stream1 == nullptr || stream2 == nullptr) { + OutputBuffers* streams = sStreams; + if (streams == nullptr) { return 0; } int16_t* samples = reinterpret_cast(buf); - stream1->receive({samples, static_cast(buf_size / 2)}, false, false); - stream2->receive({samples, static_cast(buf_size / 2)}, true, false); + streams->first.receive({samples, static_cast(buf_size / 2)}, false, + false); + streams->second.receive({samples, static_cast(buf_size / 2)}, true, + false); // Apply software volume scaling. float factor = sVolumeFactor.load(); @@ -184,14 +184,13 @@ auto Bluetooth::PreferredDevice() -> std::optional { return bluetooth::BluetoothState::preferred_device(); } -auto Bluetooth::SetSources(PcmBuffer* src1, PcmBuffer* src2) -> void { +auto Bluetooth::SetSources(OutputBuffers* src) -> void { auto lock = bluetooth::BluetoothState::lock(); - PcmBuffer *cur1, *cur2; - std::tie(cur1, cur2) = bluetooth::BluetoothState::sources(); - if (src1 == cur1 && src2 == cur2) { + OutputBuffers* cur = bluetooth::BluetoothState::sources(); + if (src == cur) { return; } - bluetooth::BluetoothState::sources(src1, src2); + bluetooth::BluetoothState::sources(src); tinyfsm::FsmList::dispatch( bluetooth::events::SourcesChanged{}); } @@ -381,13 +380,12 @@ auto BluetoothState::preferred_device(std::optional addr) -> void { sPreferredDevice_ = addr; } -auto BluetoothState::sources() -> std::pair { - return {sStream1, sStream2}; +auto BluetoothState::sources() -> OutputBuffers* { + return sStreams; } -auto BluetoothState::sources(PcmBuffer* src1, PcmBuffer* src2) -> void { - sStream1 = src1; - sStream2 = src2; +auto BluetoothState::sources(OutputBuffers* src) -> void { + sStreams = src; } auto BluetoothState::event_handler(std::function cb) -> void { @@ -715,7 +713,6 @@ void Connected::entry() { sPreferredDevice_->mac != stored_pref->mac)) { sStorage_->PreferredBluetoothDevice(sPreferredDevice_); } - // TODO: if we already have a source, immediately start playing } void Connected::exit() { @@ -733,7 +730,7 @@ void Connected::react(const events::PreferredDeviceChanged& ev) { } void Connected::react(const events::SourcesChanged& ev) { - if (sStream1 != nullptr && sStream2 != nullptr) { + if (sStreams != nullptr) { ESP_LOGI(kTag, "checking source is ready"); esp_a2d_media_ctrl(ESP_A2D_MEDIA_CTRL_CHECK_SRC_RDY); } else { diff --git a/src/drivers/i2s_dac.cpp b/src/drivers/i2s_dac.cpp index b1044896..4e2e171a 100644 --- a/src/drivers/i2s_dac.cpp +++ b/src/drivers/i2s_dac.cpp @@ -52,10 +52,12 @@ extern "C" IRAM_ATTR auto callback(i2s_chan_handle_t handle, assert(event->size % 4 == 0); uint8_t* buf = *reinterpret_cast(event->data); - auto* src = reinterpret_cast(user_ctx); + auto* src = reinterpret_cast(user_ctx); - BaseType_t ret = - src->receive({reinterpret_cast(buf), event->size / 2}, true); + BaseType_t ret1 = src->first.receive( + {reinterpret_cast(buf), event->size / 2}, false, true); + BaseType_t ret2 = src->second.receive( + {reinterpret_cast(buf), event->size / 2}, true, true); // The ESP32's I2S peripheral has a different endianness to its processors. // ESP-IDF handles this difference for stereo channels, but not for mono @@ -70,10 +72,10 @@ extern "C" IRAM_ATTR auto callback(i2s_chan_handle_t handle, } } - return ret; + return ret1 || ret2; } -auto I2SDac::create(IGpios& expander, PcmBuffer& buf) +auto I2SDac::create(IGpios& expander, OutputBuffers& bufs) -> std::optional { i2s_chan_handle_t i2s_handle; i2s_chan_config_t channel_config{ @@ -90,7 +92,7 @@ auto I2SDac::create(IGpios& expander, PcmBuffer& buf) // First, instantiate the instance so it can do all of its power on // configuration. std::unique_ptr dac = - std::make_unique(expander, buf, i2s_handle); + std::make_unique(expander, bufs, i2s_handle); // Whilst we wait for the initial boot, we can work on installing the I2S // driver. @@ -122,14 +124,14 @@ auto I2SDac::create(IGpios& expander, PcmBuffer& buf) .on_sent = callback, .on_send_q_ovf = NULL, }; - i2s_channel_register_event_callback(i2s_handle, &callbacks, &buf); + i2s_channel_register_event_callback(i2s_handle, &callbacks, &bufs); return dac.release(); } -I2SDac::I2SDac(IGpios& gpio, PcmBuffer& buf, i2s_chan_handle_t i2s_handle) +I2SDac::I2SDac(IGpios& gpio, OutputBuffers& bufs, i2s_chan_handle_t i2s_handle) : gpio_(gpio), - buffer_(buf), + buffers_(bufs), i2s_handle_(i2s_handle), i2s_active_(false), clock_config_(I2S_STD_CLK_DEFAULT_CONFIG(48000)), diff --git a/src/drivers/include/drivers/bluetooth.hpp b/src/drivers/include/drivers/bluetooth.hpp index b3b12ffc..eaecfb2b 100644 --- a/src/drivers/include/drivers/bluetooth.hpp +++ b/src/drivers/include/drivers/bluetooth.hpp @@ -43,7 +43,7 @@ class Bluetooth { auto SetPreferredDevice(std::optional dev) -> void; auto PreferredDevice() -> std::optional; - auto SetSources(PcmBuffer*, PcmBuffer*) -> void; + auto SetSources(OutputBuffers*) -> void; auto SetVolumeFactor(float) -> void; auto SetEventHandler(std::function cb) -> void; @@ -118,8 +118,8 @@ class BluetoothState : public tinyfsm::Fsm { static auto discovery() -> bool; static auto discovery(bool) -> void; - static auto sources() -> std::pair; - static auto sources(PcmBuffer*, PcmBuffer*) -> void; + static auto sources() -> OutputBuffers*; + static auto sources(OutputBuffers*) -> void; static auto event_handler(std::function) -> void; diff --git a/src/drivers/include/drivers/i2s_dac.hpp b/src/drivers/include/drivers/i2s_dac.hpp index 0fe462b4..891acb56 100644 --- a/src/drivers/include/drivers/i2s_dac.hpp +++ b/src/drivers/include/drivers/i2s_dac.hpp @@ -40,9 +40,10 @@ constexpr size_t kI2SBufferLengthFrames = 1024; */ class I2SDac { public: - static auto create(IGpios& expander, PcmBuffer&, PcmBuffer&) -> std::optional; + static auto create(IGpios& expander, OutputBuffers&) + -> std::optional; - I2SDac(IGpios& gpio, PcmBuffer&, i2s_chan_handle_t i2s_handle); + I2SDac(IGpios& gpio, OutputBuffers&, i2s_chan_handle_t i2s_handle); ~I2SDac(); auto SetPaused(bool) -> void; @@ -77,8 +78,7 @@ class I2SDac { auto set_channel(bool) -> void; IGpios& gpio_; - PcmBuffer& buffer1_; - PcmBuffer& buffer2_; + OutputBuffers& buffers_; i2s_chan_handle_t i2s_handle_; bool i2s_active_; diff --git a/src/drivers/include/drivers/pcm_buffer.hpp b/src/drivers/include/drivers/pcm_buffer.hpp index 27e9eec6..968c3398 100644 --- a/src/drivers/include/drivers/pcm_buffer.hpp +++ b/src/drivers/include/drivers/pcm_buffer.hpp @@ -74,4 +74,16 @@ class PcmBuffer { RingbufHandle_t ringbuf_; }; +/* + * Convenience type for a pair of PcmBuffers. Each audio output handles mixing + * streams together to ensure that low-latency sounds in one channel (e.g. a + * system notification bleep) aren't delayed by a large audio buffer in the + * other channel (e.g. a long-running track). + * + * By convention, the first buffer of this pair is used for tracks, whilst the + * second is reserved for 'system sounds'; usually TTS, but potentially maybe + * other informative noises. + */ +using OutputBuffers = std::pair; + } // namespace drivers diff --git a/src/drivers/pcm_buffer.cpp b/src/drivers/pcm_buffer.cpp index b619cefb..1d2bab1e 100644 --- a/src/drivers/pcm_buffer.cpp +++ b/src/drivers/pcm_buffer.cpp @@ -56,7 +56,7 @@ IRAM_ATTR auto PcmBuffer::receive(std::span dest, bool mix, bool isr) } size_t total_read = first_read + second_read; - if (total_read < dest.size()) { + if (total_read < dest.size() && !mix) { std::fill_n(dest.begin() + total_read, dest.size() - total_read, 0); } diff --git a/src/tangara/audio/audio_fsm.cpp b/src/tangara/audio/audio_fsm.cpp index 80611082..8f04c6c1 100644 --- a/src/tangara/audio/audio_fsm.cpp +++ b/src/tangara/audio/audio_fsm.cpp @@ -59,10 +59,16 @@ std::shared_ptr AudioState::sOutput; std::shared_ptr AudioState::sI2SOutput; std::shared_ptr AudioState::sBtOutput; -// Two seconds of samples for two channels, at a representative sample rate. -constexpr size_t kDrainLatencySamples = 48000 * 2 * 2; +// For tracks, keep about two seconds' worth of samples at 2ch 48kHz. This +// is more headroom than we need for small playback, but it doesn't hurt to +// keep some PSRAM in our pockets for a rainy day. +constexpr size_t kTrackDrainLatencySamples = 48000 * 2 * 2; -std::unique_ptr AudioState::sDrainBuffer; +// For system sounds, we intentionally choose codecs that are very fast to +// decode. This lets us get away with a much smaller drain buffer. +constexpr size_t kSystemDrainLatencySamples = 48000; + +std::unique_ptr AudioState::sDrainBuffers; std::optional AudioState::sDrainFormat; StreamCues AudioState::sStreamCues; @@ -237,11 +243,11 @@ void AudioState::react(const system_fsm::BluetoothEvent& ev) { break; } } - if (std::holds_alternative(ev.event)) { - auto volume_chg = std::get(ev.event).new_vol; - events::Ui().Dispatch(RemoteVolumeChanged{ - .value = volume_chg - }); + if (std::holds_alternative( + ev.event)) { + auto volume_chg = + std::get(ev.event).new_vol; + events::Ui().Dispatch(RemoteVolumeChanged{.value = volume_chg}); } } @@ -354,12 +360,13 @@ namespace states { void Uninitialised::react(const system_fsm::BootComplete& ev) { sServices = ev.services; - sDrainBuffer = std::make_unique(kDrainLatencySamples); + sDrainBuffers = std::make_unique( + kTrackDrainLatencySamples, kSystemDrainLatencySamples); sStreamFactory.reset(new FatfsStreamFactory(*sServices)); - sI2SOutput.reset(new I2SAudioOutput(sServices->gpios(), *sDrainBuffer)); + sI2SOutput.reset(new I2SAudioOutput(sServices->gpios(), *sDrainBuffers)); sBtOutput.reset(new BluetoothAudioOutput( - sServices->bluetooth(), *sDrainBuffer, sServices->bg_worker())); + sServices->bluetooth(), *sDrainBuffers, sServices->bg_worker())); auto& nvs = sServices->nvs(); sI2SOutput->SetMaxVolume(nvs.AmpMaxVolume()); @@ -390,7 +397,7 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) { .left_bias = nvs.AmpLeftBias(), }); - sSampleProcessor.reset(new SampleProcessor(*sDrainBuffer)); + sSampleProcessor.reset(new SampleProcessor(sDrainBuffers->first)); sSampleProcessor->SetOutput(sOutput); sDecoder.reset(Decoder::Start(sSampleProcessor)); @@ -507,7 +514,7 @@ void Playback::react(const system_fsm::SdStateChanged& ev) { } void Playback::react(const internal::StreamHeartbeat& ev) { - sStreamCues.update(sDrainBuffer->totalReceived()); + sStreamCues.update(sDrainBuffers->first.totalReceived()); if (sStreamCues.hasStream()) { emitPlaybackUpdate(false); diff --git a/src/tangara/audio/audio_fsm.hpp b/src/tangara/audio/audio_fsm.hpp index f949ce8a..1e5184b5 100644 --- a/src/tangara/audio/audio_fsm.hpp +++ b/src/tangara/audio/audio_fsm.hpp @@ -81,7 +81,7 @@ class AudioState : public tinyfsm::Fsm { static std::shared_ptr sBtOutput; static std::shared_ptr sOutput; - static std::unique_ptr sDrainBuffer; + static std::unique_ptr sDrainBuffers; static StreamCues sStreamCues; static std::optional sDrainFormat; diff --git a/src/tangara/audio/bt_audio_output.cpp b/src/tangara/audio/bt_audio_output.cpp index 616a385f..54547622 100644 --- a/src/tangara/audio/bt_audio_output.cpp +++ b/src/tangara/audio/bt_audio_output.cpp @@ -33,11 +33,11 @@ namespace audio { static constexpr uint16_t kVolumeRange = 60; BluetoothAudioOutput::BluetoothAudioOutput(drivers::Bluetooth& bt, - drivers::PcmBuffer& buffer, + drivers::OutputBuffers& bufs, tasks::WorkerPool& p) : IAudioOutput(), bluetooth_(bt), - buffer_(buffer), + buffers_(bufs), bg_worker_(p), volume_() {} @@ -45,9 +45,9 @@ BluetoothAudioOutput::~BluetoothAudioOutput() {} auto BluetoothAudioOutput::changeMode(Modes mode) -> void { if (mode == Modes::kOnPlaying) { - bluetooth_.SetSource(&buffer_); + bluetooth_.SetSources(&buffers_); } else { - bluetooth_.SetSource(nullptr); + bluetooth_.SetSources(nullptr); } } diff --git a/src/tangara/audio/bt_audio_output.hpp b/src/tangara/audio/bt_audio_output.hpp index f22f330a..53d2c1a4 100644 --- a/src/tangara/audio/bt_audio_output.hpp +++ b/src/tangara/audio/bt_audio_output.hpp @@ -25,7 +25,7 @@ namespace audio { class BluetoothAudioOutput : public IAudioOutput { public: BluetoothAudioOutput(drivers::Bluetooth& bt, - drivers::PcmBuffer& buf, + drivers::OutputBuffers& bufs, tasks::WorkerPool&); ~BluetoothAudioOutput(); @@ -54,7 +54,7 @@ class BluetoothAudioOutput : public IAudioOutput { private: drivers::Bluetooth& bluetooth_; - drivers::PcmBuffer& buffer_; + drivers::OutputBuffers& buffers_; tasks::WorkerPool& bg_worker_; uint16_t volume_; diff --git a/src/tangara/audio/i2s_audio_output.cpp b/src/tangara/audio/i2s_audio_output.cpp index 8222b8c9..55c8bdb8 100644 --- a/src/tangara/audio/i2s_audio_output.cpp +++ b/src/tangara/audio/i2s_audio_output.cpp @@ -42,10 +42,10 @@ static constexpr uint16_t kLineLevelVolume = 0x13d; static constexpr uint16_t kDefaultVolume = 0x100; I2SAudioOutput::I2SAudioOutput(drivers::IGpios& expander, - drivers::PcmBuffer& buffer) + drivers::OutputBuffers& buffers) : IAudioOutput(), expander_(expander), - buffer_(buffer), + buffers_(buffers), dac_(), current_mode_(Modes::kOff), current_config_(), @@ -72,7 +72,7 @@ auto I2SAudioOutput::changeMode(Modes mode) -> void { if (was_off) { // Ensure an I2SDac instance actually exists. if (!dac_) { - auto instance = drivers::I2SDac::create(expander_, buffer_); + auto instance = drivers::I2SDac::create(expander_, buffers_); if (!instance) { return; } diff --git a/src/tangara/audio/i2s_audio_output.hpp b/src/tangara/audio/i2s_audio_output.hpp index 35d888b9..2b768ddd 100644 --- a/src/tangara/audio/i2s_audio_output.hpp +++ b/src/tangara/audio/i2s_audio_output.hpp @@ -21,7 +21,7 @@ namespace audio { class I2SAudioOutput : public IAudioOutput { public: - I2SAudioOutput(drivers::IGpios&, drivers::PcmBuffer&); + I2SAudioOutput(drivers::IGpios&, drivers::OutputBuffers&); auto SetMaxVolume(uint16_t) -> void; auto SetVolumeDb(uint16_t) -> void; @@ -51,7 +51,7 @@ class I2SAudioOutput : public IAudioOutput { private: drivers::IGpios& expander_; - drivers::PcmBuffer& buffer_; + drivers::OutputBuffers& buffers_; std::unique_ptr dac_;