Repository navigation
Conversation
New components/audio: espp::Audio drives one I2S port (standard mode, 16-bit PCM) for playback, recording or both, full duplex. It owns what every BSP copied: the I2S channels and DMA rings, the playback stream buffer and whole-block task (codec/i2s_tx_feed.hpp), the recording task, volume / mute / microphone gain (in the codec, or in software without one), sample rate changes, and a full unwind on failure plus deinitialize() and the destructor. Codecs plug in through the small espp::AudioCodec interface, with adapters for the ES8388 (playback) and ES7210 (recording) on the codec component's drivers. The ES7210 adapter also follows sample rate changes (its coefficient table), which the Tab5 did not before. The M5Stack Tab5 BSP now holds an espp::Audio and keeps the board parts (pins, codec devices, speaker amplifier, charger pause via Config::on_playback_block). Its audio API is the canonical BSP audio API: adds deinitialize_audio(), is_audio_initialized(), audio(), clear_audio(), and audio_sample_rate(uint32_t) returns bool; initialize_audio(sample_rate, task_config) is deprecated in favor of initialize_audio(AudioConfig). volume() / mute() before initialize_audio() no longer call into an unwired codec driver. Docs (new Audio section), Doxyfile, CI build / upload entries, and example overrides for espp/audio until it is published. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFCMzomCZ5wRHJN4sxwkvY
|
✅Static analysis result - no issues found! ✅ |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
What changed in this PR
Adds a new espp::Audio component that centralizes I2S playback/recording and codec integration, and migrates the M5Stack Tab5 BSP to use it.
Changes:
- Introduces
components/audiowithespp::Audio,espp::AudioCodec, and ES8388/ES7210 codec adapters plus an example project. - Refactors the Tab5 BSP audio implementation to delegate to
espp::Audio, adding new BSP audio API methods and deprecating the old init overload. - Extends documentation/CI to build and publish the new component and include Audio docs.
| File | Description |
|---|---|
| doc/en/index.rst | Adds an “Audio” section to the docs toctree. |
| doc/en/audio/index.rst | Introduces the Audio docs landing page and references. |
| doc/en/audio/audio_example.md | Includes the component’s example README into docs. |
| doc/en/audio/audio.rst | Documents espp::Audio, BSP audio API, and API reference includes. |
| doc/Doxyfile | Adds Audio headers and example to Doxygen inputs. |
| components/usb_host/tab5_example/main/idf_component.yml | Adds example-only override for in-repo espp/audio. |
| components/usb_host/tab5_example/CMakeLists.txt | Adds components/audio to extra component dirs for the example. |
| components/m5stack-tab5/src/audio.cpp | Refactors Tab5 audio to wrap/use espp::Audio and codec adapters. |
| components/m5stack-tab5/include/m5stack-tab5.hpp | Updates Tab5 public audio API, adds new methods, deprecates old init signature. |
| components/m5stack-tab5/idf_component.yml | Declares dependency on espp/audio and removes direct espp/codec dependency. |
| components/m5stack-tab5/example/main/idf_component.yml | Adds example-only override for in-repo espp/audio. |
| components/m5stack-tab5/example/CMakeLists.txt | Adds components/audio to extra component dirs for the example. |
| components/m5stack-tab5/README.md | Notes that Tab5 audio is built on espp::Audio and exposes it. |
| components/m5stack-tab5/CMakeLists.txt | Switches build requirements from codec to audio. |
| components/audio/src/es8388_codec.cpp | Adds ES8388 playback adapter implementing AudioCodec. |
| components/audio/src/es7210_codec.cpp | Adds ES7210 recording adapter implementing AudioCodec, including HPF handling. |
| components/audio/src/audio.cpp | Adds the core espp::Audio implementation (I2S, tasks, buffering, teardown). |
| components/audio/include/es8388_codec.hpp | Declares the ES8388 codec adapter API. |
| components/audio/include/es7210_codec.hpp | Declares the ES7210 codec adapter API. |
| components/audio/include/audio_codec.hpp | Defines the codec abstraction interface used by espp::Audio. |
| components/audio/include/audio.hpp | Declares espp::Audio public API and configuration. |
| components/audio/idf_component.yml | Defines the new component’s metadata and dependencies. |
| components/audio/example/sdkconfig.defaults | Sets example target and FreeRTOS tick rate defaults. |
| components/audio/example/main/audio_example.cpp | Adds a playback-only example using software volume/mute. |
| components/audio/example/main/Kconfig.projbuild | Adds example pin/power configuration options. |
| components/audio/example/main/CMakeLists.txt | Registers the example’s main component. |
| components/audio/example/README.md | Documents building/running the audio example. |
| components/audio/example/CMakeLists.txt | Adds top-level CMake for the audio example project. |
| components/audio/README.md | Adds component README describing features, codecs, and BSP API. |
| components/audio/CMakeLists.txt | Registers the new audio component and dependencies. |
| .github/workflows/upload_components.yml | Adds components/audio to the publish job list. |
| .github/workflows/build.yml | Adds the audio example to CI builds. |
Review effort: Lite
Comment on lines
+328
to
+338
| std::lock_guard<std::mutex> stream_lock(stream_mutex_); | ||
| if (!tx_stream_) { | ||
| return 0; | ||
| } | ||
| // Queue only whole frames that actually fit: xStreamBufferSend can accept | ||
| // fewer bytes than requested when the buffer is nearly full, and a partial | ||
| // frame would corrupt the framing of everything queued after it. With a 0 | ||
| // timeout the send never blocks. | ||
| size_t sendable = std::min<size_t>(num_bytes, xStreamBufferSpacesAvailable(tx_stream_)); | ||
| sendable -= sendable % tx_feed_.frame_bytes; | ||
| if (sendable == 0) { |
Comment on lines
+453
to
+457
| // wait for a callback in flight to return, so that none runs after this | ||
| // returns - unless this is that callback | ||
| if (recording_task_ && espp::Task::get_current_id() != recording_task_->get_id()) { | ||
| std::lock_guard<std::recursive_mutex> callback_lock(recording_callback_mutex_); | ||
| } |
Comment on lines
+75
to
+83
| static constexpr uint8_t ES7210_ADDR = 0x40; // the driver's (AD1/AD0 = 00) | ||
| bool hpf_ok = true; | ||
| for (auto [reg, value] : {std::pair<uint8_t, uint8_t>{0x22, 0x0a}, // ADC1/2 HPF1 | ||
| {0x23, 0x2a}, // ADC1/2 HPF2 | ||
| {0x20, 0x0a}, // ADC3/4 HPF2 | ||
| {0x21, 0x2a}}) { // ADC3/4 HPF1 | ||
| uint8_t data[] = {reg, value}; | ||
| hpf_ok &= config_.write(ES7210_ADDR, data, sizeof(data)); | ||
| } |
On this platform every std::mutex / std::recursive_mutex allocates its FreeRTOS semaphore (about 116 B of internal RAM) and every espp::Logger carries one too, so the component's four mutexes and the adapters' loggers were most of its overhead over the old in-BSP code. - espp::Audio uses a single mutex. Teardown releases it while it joins the tasks (tearing_down_ keeps initialize() / deinitialize() out meanwhile), so a callback that calls a method cannot deadlock against it, and stop_recording() waits for a callback in flight through an atomic flag instead of a mutex. - The ES8388 / ES7210 adapters log through ESP_LOGx instead of an espp::Logger, and the ES7210 keeps only what start() needs. - Only the I2S clock configuration is kept (for sample_rate()), not two full standard-mode configurations. M5Stack Tab5 initialize_audio(): 66,076 -> 65,244 B of internal RAM (main's in-BSP code: 64,312 B). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFCMzomCZ5wRHJN4sxwkvY
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
First step of moving the BSP audio code into its own component, the same way
espp::Ethernetwas split out.New
components/audioespp::Audiodrives one I2S port in standard mode with 16-bit PCM: playback, recording, or both (full duplex). It owns what every BSP used to copy:i2s_tx_feed::ring_outlasts_wait)codec/i2s_tx_feed.hpp)initialize()fails, plusdeinitialize()and the destructorMono or stereo can be set per direction.
Config::on_playback_blocktells the BSP once per block whether that block carried audio.Locking:
play()andclear()hold a stream mutex.Because of this, a recording callback may call
play(), the setters, andstart_recording()/stop_recording().stop_recording()waits for a callback that is still running, unless it is called from inside that callback.Codecs: chips plug in through a small virtual
espp::AudioCodecinterface (start/stop/set_sample_rate/set_volume/set_mute/set_gain). It is only called on init, teardown and setter calls, never per sample. This PR adds adapters on the existingcodecdrivers:espp::Es8388Codec(playback)espp::Es7210Codec(recording, with its high-pass filter and gain mapping). It now also reprograms its coefficient table when the sample rate changes; the Tab5 didn't do that before.The component also has an example (a tone through a codec-less I2S amplifier, with the T-Deck's pins as defaults), docs (a new Audio section), Doxyfile entries and CI build/upload entries.
M5Stack Tab5
The BSP holds an
espp::Audioand keeps only the board-specific parts: the pins, the codec I2C devices, the speaker amplifier, and the charger pause, driven byon_playback_block.Its audio methods are now the canonical BSP audio API (documented in the component README):
deinitialize_audio(),is_audio_initialized(),audio()andclear_audio().audio_sample_rate(uint32_t)now returnsbool.play_audio(const uint8_t *, size_t)now takes asize_tlength.initialize_audio(sample_rate, task_config)is[[deprecated]]in favor ofinitialize_audio(const AudioConfig &).volume()/mute()beforeinitialize_audio()now only store the value. Before, they called the ES8388 driver before its I2C functions were set.The Tab5 example and the
usb_hostTab5 example get a temporarymain/idf_component.ymlthat overridesespp/audioto the in-repo path, asespp/ethernetdid until it was published.Memory
Internal RAM used by the Tab5's
initialize_audio(), measured in the Tab5 example:Each
std::mutex/std::recursive_mutexallocates its FreeRTOS semaphore (about 116 B of internal RAM) when it is constructed, and eachespp::Loggerholds one, so the second commit:espp::Audio. Teardown releases it while it joins the tasks, andstop_recording()waits for a callback that is still running through an atomic flag.ESP_LOGx.Most of the remaining ~930 B is the
Audioobject itself (376 B, part of which used to be fields of the Tab5 object in static storage) and two semaphores: its own mutex's and its logger's.tab5-emu with its current settings (recording and DMA2D on, about 17.5 KB of DMA-capable RAM free at the menu) has very little headroom; with the first push of this PR an SD save failed. With the #878 settings (recording and DMA2D off) it has about 43 KB and runs cleanly; see Testing.
Follow-ups (planned)
initialize_sound/enable_sound/initialize_microphone.Testing
All on a Tab5 with ESP-IDF 6.1.
deinitialize_audio()freed all 65 KB of internal RAM. Re-initializing used the same amount again (180,431 → 245,467 → 180,287 bytes free).play_audio()after deinit returned 0.stop_audio_recording()on itself ran exactly once, and that recording restarted afterwards.🤖 Generated with Claude Code
https://claude.ai/code/session_01SFCMzomCZ5wRHJN4sxwkvY