Migrate from ESP8266Audio to BackgroundAudio - #11272
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (31)
Build artifacts expire on 2026-08-28. Updated for |
39b18a2 to
a710f58
Compare
a710f58 to
a80181d
Compare
The 8x128 DMA ring held 46ms of audio and was fed from the cooperative main loop, so any stall longer than that drained it and auto_clear played silence - audible as stuttering exactly when a notification redraws the screen. Worst on MUI builds: the tft task holds spiLock across whole LVGL cycles (render + flush), and on shared-SPI boards like T-Deck the main loop then blocks behind it on every radio op. - Deepen the ring to 16x256 (185ms; 16KB DMA RAM held only during playback), falling back to the old geometry when DMA RAM is tight. - Preload the ring with real samples before enabling the channel, via a begin()/preloadFrames()/start() split. Startup latency no longer scales with ring depth: a click is audible after a 10ms amp lead-in instead of after a full ring of silence, and playback starts at maximum ring depth - right when the notification redraw begins. - Feed from a small per-playback task (4KB stack, priority 2, pinned to loopTask's core) that touches only the generator and the I2S channel, so display/radio spiLock contention cannot starve it. Falls back to main-loop feeding if task creation fails. This is not upstream's task coming back: that one exists to maintain availableForWrite() from ISR notifications, the machinery MeshtasticI2SOut deliberately avoids. - Warn with the measured stall length whenever the feed gap exceeds what the ring holds - a proven dropout, attributable in tester logs. Validated on T-Deck MUI (DEFCON build): full ringtone with nag over live mesh traffic, clean audio, zero dropout warnings.
Without it this file's setup()/loop() definitions get plain C++ linkage while portduino's main.cpp expects Arduino.h's declarations, and the coverage build fails with 'undefined reference to setup/loop' (other suites include it transitively via meshUtils.h/TestUtil.h). Fixes the failing test-native CI job on this PR; suite runs 30/30.
|
This is a much better end state than what's on I'd been working the same problem from the other direction (fixing the existing The boot melody still doesn't play on I2S boards
if (moduleConfig.external_notification.use_i2s_as_buzzer && audioThread) {
audioThread->beginTones(tone_durations, (size_t)size);
return;
}But This is pre-existing on I have a one-commit fix on a branch off Confirmed on a T-Deck ( One judgement call in there that's yours to make: dreamcatcher defines both Happy to open it as a PR against Things I checked that turned out fineMentioning these only so you know they've had a second pair of eyes: the octave 4–7 clamp initially looked like a gap to me, but HardwareI've flashed this branch on two in-tree boards — a T-Deck ( I also have a T-Watch Ultra on an out-of-tree variant of my own; not something I'm proposing here, but it's a third differently-wired board on the same path (different DAC pinout, no MCLK) so I'm happy to run this there too if that's a data point you want. The gap I can't cover: none of my boards define |
|
Separate topic from my last comment: text-to-speech, and a way to keep it for ~16KB. What goes awayThe PR description says "text-to-speech support is dropped for now," which undersells it slightly — what disappears is user-visible:
So a T-Deck or Cardputer Adv owner on espeak-ng isn't the way backJust to close this off, since BackgroundAudio already vendors it: I counted the actual array bytes rather than trusting file sizes.
Your ~947KB figure is about right. Worth noting the Why naively re-adding SAM failsI tried it: re-added ESP8266Audio + ESP8266SAM and wrote a shim from Your What does workBoth blockers dissolve if SAM stops being a library dependency:
A ~35-line adapter then bridges that interface to Measured on
16.6KB of flash and 216 bytes of RAM, and I've flashed it on a Cardputer Adv and used Read Aloud — it works. I went ahead and built the pull-style version, since the blocking one wasn't really proposable against this architecture. It's on What it does
The commits are split so the two decisions can be judged separately:
The design constraint, since it shaped the resultSAM can't be made resumable — So the render runs on its own task pushing into a 4KB ring (~186ms) and One subtlety worth flagging for review: your feeder reads Cost
Plus 4KB of heap for the ring and a 4KB task stack that only exists while speaking. Against that, BackgroundAudio's own espeak-ng is ~827KB of tables before any code, and doesn't fit the 8MB app partition at all. TestingRead Aloud works on both boards I have:
The Cardputer is the one that matters — no PSRAM is exactly the case the ring design exists for, and it's the tighter partition of the two. The UI also stays responsive during an utterance, which the old blocking To be clear about what this isYou closed this decision already, and the ~947KB figure was right for espeak. My only real claim is that "espeak costs ~947KB" and "TTS costs ~947KB" turn out to be different statements, and 17KB seemed worth putting on the table before the feature disappears. Entirely your call whether it's worth carrying vendored SAM for speech that sounds like a 1982 answering machine. If you'd rather TTS just go, say so and I'll drop it — no argument from me. If you want it, happy to open it as a PR against this branch, rebase it onto whatever this becomes, or hand it over however suits. |
3 participants
🤖
This pull request introduces a major refactor of the audio playback system, replacing the ESP8266Audio-based RTTTL playback with a new, dependency-free, asynchronous PCM generator and playback thread. The new implementation is more memory-efficient, testable, and fixes several subtle bugs in the previous approach. The public API remains mostly unchanged, but text-to-speech support is dropped for now. The most important changes are summarized below.
Audio playback system refactor:
RtttlPcmclass, which parses RTTTL strings and generates square-wave PCM samples directly, matching the pitch and timing of the old implementation but without external library dependencies. [1] [2]AudioThreadclass that manages asynchronous playback, amplifier power control, and DMA feeding. Playback is now non-blocking, and the amplifier is only powered when needed, reducing power usage and avoiding audible pops. [1] [2]readAloud()), as the new system does not include a speech synthesizer due to resource constraints and symbol conflicts.API and code structure improvements:
ToneDuration), allowing system sounds to be played without converting them to RTTTL strings. [1] [2]ToneDurationtoaudio/RtttlPcm.hfor better encapsulation and to support both RTTTL and direct tone playback. [1] [2]Bug fixes and safety improvements:
Overall, these changes modernize the audio system, reduce dependencies, and improve reliability and maintainability.