From 202fcfe08f4ded65bd66cd58165efced49bade27 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:29:50 +0100 Subject: [PATCH 1/8] espressif: add audiopwmio.PWMAudioOut for the ESP32-S2 The espressif port had no audiopwmio at all, so boards whose speaker is not wired to a DAC pin had no way to play samples. On the Adafruit FunHouse the piezo is on GPIO42, while the ESP32-S2 DAC is hardwired to GPIO17/GPIO18 (and GPIO18 is the light sensor), so audioio cannot reach it. LEDC has no DMA path for its duty registers, and the S2's I2S has no PDM transmitter (no SOC_I2S_SUPPORTS_PDM_TX), so samples are pushed from a GPTimer alarm ISR running at the sample rate. The ISR drains one of two device buffers; audiopwmout_background(), called from port_background_task(), refills the other from the audiosample protocol. The ISR deliberately touches nothing but its own object's volatile state. It must not queue a background callback: on this port that macro is taskENTER_CRITICAL(), which aborts when called from ISR context. Driving the refill by polling instead also means a buffer the ISR found empty is always picked up again, rather than relying on the ISR to have queued the refill before it stalled. Every gptimer call is checked, so a timer that cannot be configured raises instead of leaving `playing` true forever. alarm_config is static because CircuitPython builds with CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM, under which gptimer_set_alarm_action() rejects a config outside internal RAM. The carrier is APB_CLK / 2**10 = 78.125 kHz, which keeps it out of the audio band while leaving 10 bits of amplitude resolution. The GPTimer runs at 40 MHz (the finest the timer group divider allows), putting the sample-rate error below 0.01% at 8k/11k/22k/44.1k. A single-buffer sample that loops wraps inside the ISR rather than round-tripping through the background task, so looping RawSamples do not click. ledc_ll.h lives in the per-target esp_hal_ledc directory, which was not on the include path, so add it. Enabled for esp32s2 only; the S3 has neither DAC nor audiopwmio and could reuse this later. Costs 2336 bytes of flash and 648 bytes of internal SRAM on adafruit_funhouse, and nothing on boards that leave it off. Co-Authored-By: Claude Opus 5 --- ports/espressif/Makefile | 1 + ports/espressif/background.c | 6 + .../common-hal/audiopwmio/PWMAudioOut.c | 442 ++++++++++++++++++ .../common-hal/audiopwmio/PWMAudioOut.h | 61 +++ .../common-hal/audiopwmio/__init__.c | 7 + ports/espressif/mpconfigport.mk | 3 + ports/espressif/supervisor/port.c | 7 + 7 files changed, 527 insertions(+) create mode 100644 ports/espressif/common-hal/audiopwmio/PWMAudioOut.c create mode 100644 ports/espressif/common-hal/audiopwmio/PWMAudioOut.h create mode 100644 ports/espressif/common-hal/audiopwmio/__init__.c diff --git a/ports/espressif/Makefile b/ports/espressif/Makefile index 978a3b0e7c0..485aa81a56f 100644 --- a/ports/espressif/Makefile +++ b/ports/espressif/Makefile @@ -123,6 +123,7 @@ INC += \ -isystem esp-idf/components/esp_hal_i2s/include \ -isystem esp-idf/components/esp_hal_i2s/$(IDF_TARGET)/include \ -isystem esp-idf/components/esp_hal_ledc/include \ + -isystem esp-idf/components/esp_hal_ledc/$(IDF_TARGET)/include \ -isystem esp-idf/components/esp_hal_pcnt/include \ -isystem esp-idf/components/esp_hal_rmt/include \ -isystem esp-idf/components/esp_hal_rmt/$(IDF_TARGET)/include \ diff --git a/ports/espressif/background.c b/ports/espressif/background.c index d28b1bb3765..df63aeb3dd2 100644 --- a/ports/espressif/background.c +++ b/ports/espressif/background.c @@ -8,6 +8,9 @@ #include "supervisor/filesystem.h" #include "supervisor/port.h" #include "supervisor/shared/stack.h" +#if CIRCUITPY_AUDIOPWMIO +#include "common-hal/audiopwmio/PWMAudioOut.h" +#endif #if CIRCUITPY_USB_HOST #include "supervisor/usb.h" #include "tusb.h" @@ -28,6 +31,9 @@ void port_background_tick(void) { } void port_background_task(void) { + #if CIRCUITPY_AUDIOPWMIO + audiopwmout_background(); + #endif } void port_start_background_tick(void) { diff --git a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c new file mode 100644 index 00000000000..798a9232245 --- /dev/null +++ b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c @@ -0,0 +1,442 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Tyeth Gundry +// +// SPDX-License-Identifier: MIT + +// PWM audio for the ESP32 family. +// +// LEDC has no DMA path for its duty registers, and the ESP32-S2's I2S has no +// PDM transmitter, so samples are pushed one at a time from a GPTimer alarm +// ISR running at the sample rate. Two device buffers are used: the ISR drains +// one while audiopwmout_background() refills the other. +// +// The ISR deliberately touches nothing but this object's own volatile state. +// In particular it must not queue a background callback: that takes a FreeRTOS +// critical section, which aborts when called from ISR context on this port. + +#include "common-hal/audiopwmio/PWMAudioOut.h" + +#include +#include + +#include "py/gc.h" +#include "py/runtime.h" + +#include "shared-bindings/audiopwmio/PWMAudioOut.h" +#include "shared-bindings/microcontroller/Pin.h" +#include "shared-bindings/pwmio/PWMOut.h" +#include "shared-module/audiocore/__init__.h" + +#include "driver/gptimer.h" +#include "esp_attr.h" +#include "hal/ledc_ll.h" +#include "soc/soc.h" + +// Carrier frequency: 80 MHz / 1024 = 78125 Hz. pwmio derives PWM_BITS back out +// of this when it picks a duty resolution, so the two stay in step. +#define PWM_FREQUENCY (APB_CLK_FREQ >> PWM_BITS) + +// GPTimer tick rate. The timer group divider must be at least 2, so 40 MHz is +// the finest available, and it keeps the sample-rate error below a cent +// (44100 Hz -> 907 ticks -> 44101.4 Hz). +#define TIMER_RESOLUTION_HZ (APB_CLK_FREQ / 2) + +// Each object needs its own LEDC timer, so that bounds how many can play. +static audiopwmio_pwmaudioout_obj_t *active_audio[SOC_LEDC_TIMER_NUM]; + +// Write one duty value to a LEDC channel. This mirrors what ledc_update_duty() +// does, minus the argument checks and the spinlock, so it is cheap enough to +// run at the sample rate. The fade parameters it relies on (scale 0, so the +// duty is latched rather than ramped) are left set up by pwmio's construct. +static inline void IRAM_ATTR set_duty(uint8_t channel, uint32_t duty) { + ledc_ll_set_duty_int_part(&LEDC, LEDC_LOW_SPEED_MODE, channel, duty); + ledc_ll_set_sig_out_en(&LEDC, LEDC_LOW_SPEED_MODE, channel, true); + ledc_ll_set_duty_start(&LEDC, LEDC_LOW_SPEED_MODE, channel); + ledc_ll_ls_channel_update(&LEDC, LEDC_LOW_SPEED_MODE, channel); +} + +static bool IRAM_ATTR timer_alarm_cb(gptimer_handle_t timer, + const gptimer_alarm_event_data_t *edata, void *user_ctx) { + audiopwmio_pwmaudioout_obj_t *self = (audiopwmio_pwmaudioout_obj_t *)user_ctx; + (void)timer; + (void)edata; + + uint8_t buf_index = self->playing_buffer; + if (!self->buffer_ready[buf_index]) { + // Underrun, or the sample ended. Hold the quiescent level rather than + // leaving the last sample on the pin, which would be a DC offset. + set_duty(self->left_pwm.chan_handle.channel, self->quiescent_value); + if (self->stereo) { + set_duty(self->right_pwm.chan_handle.channel, self->quiescent_value); + } + return false; + } + + size_t frame = self->playing_frame; + const uint16_t *buf = self->buffers[buf_index]; + + set_duty(self->left_pwm.chan_handle.channel, buf[2 * frame]); + if (self->stereo) { + set_duty(self->right_pwm.chan_handle.channel, buf[2 * frame + 1]); + } + + frame++; + if (frame >= self->buffer_frames[buf_index]) { + if (self->single_buffer && self->loop) { + // The whole sample fits in one buffer and repeats forever, so wrap + // in place. This keeps looping RawSamples glitch-free: no refill, + // and no dependency on how promptly the background task runs. + self->playing_frame = 0; + return false; + } + // This buffer is spent. Flag it; the background task does the refill. + self->buffer_ready[buf_index] = false; + if (self->last_buffer[buf_index]) { + self->stop_queued = true; + } + self->playing_frame = 0; + if (!self->single_buffer) { + self->playing_buffer = buf_index ^ 1; + } + } else { + self->playing_frame = frame; + } + + return false; +} + +// Convert one source buffer into device buffer `buf_index`, scaling samples to +// PWM_BITS and always writing interleaved (left, right) pairs. +static void fill_buffer(audiopwmio_pwmaudioout_obj_t *self, uint8_t buf_index) { + uint8_t *source; + uint32_t source_length; + audioio_get_buffer_result_t result = + audiosample_get_buffer(self->sample, false, 0, &source, &source_length); + + if (result == GET_BUFFER_ERROR) { + self->stop_queued = true; + return; + } + + size_t frames = source_length / self->bytes_per_sample / self->sample_channel_count; + if (frames * 2 * sizeof(uint16_t) > self->buffer_size[buf_index]) { + // Should not happen: play() sized the buffers from max_buffer_length. + frames = self->buffer_size[buf_index] / (2 * sizeof(uint16_t)); + } + + uint16_t *dest = self->buffers[buf_index]; + if (self->bytes_per_sample == 1) { + uint8_t xor_mask = self->signed_to_unsigned ? 0x80 : 0; + for (size_t i = 0; i < frames; i++) { + uint16_t left = (uint16_t)((source[i * self->sample_channel_count] ^ xor_mask) << (PWM_BITS - 8)); + uint16_t right = left; + if (self->sample_channel_count == 2) { + right = (uint16_t)((source[i * 2 + 1] ^ xor_mask) << (PWM_BITS - 8)); + } + *dest++ = left; + *dest++ = right; + } + } else { + uint16_t xor_mask = self->signed_to_unsigned ? 0x8000 : 0; + // Cast via void * : the sample buffer is 16-bit aligned for 16-bit + // samples, but the protocol hands it over as uint8_t *. + const uint16_t *source16 = (const uint16_t *)(const void *)source; + for (size_t i = 0; i < frames; i++) { + uint16_t left = (uint16_t)((source16[i * self->sample_channel_count] ^ xor_mask) >> SAMPLE_BITS_TO_DISCARD); + uint16_t right = left; + if (self->sample_channel_count == 2) { + right = (uint16_t)((source16[i * 2 + 1] ^ xor_mask) >> SAMPLE_BITS_TO_DISCARD); + } + *dest++ = left; + *dest++ = right; + } + } + + self->buffer_frames[buf_index] = frames; + self->last_buffer[buf_index] = false; + + if (result == GET_BUFFER_DONE) { + if (self->loop) { + audiosample_reset_buffer(self->sample, false, 0); + } else { + self->last_buffer[buf_index] = true; + } + } + + // Publish last, so the ISR never sees a half-filled buffer. + __asm__ volatile ("" ::: "memory"); + self->buffer_ready[buf_index] = (frames > 0); + if (frames == 0 && result == GET_BUFFER_DONE && !self->loop) { + self->stop_queued = true; + } +} + +// Tear down the timer without touching the heap, so this is also safe to call +// while the VM is being reset. +static void stop_timer(audiopwmio_pwmaudioout_obj_t *self) { + if (self->timer != NULL) { + gptimer_stop(self->timer); + gptimer_disable(self->timer); + gptimer_del_timer(self->timer); + self->timer = NULL; + } + for (size_t i = 0; i < MP_ARRAY_SIZE(active_audio); i++) { + if (active_audio[i] == self) { + active_audio[i] = NULL; + } + } +} + +static void free_buffers(audiopwmio_pwmaudioout_obj_t *self) { + for (uint8_t i = 0; i < 2; i++) { + if (self->buffers[i] != NULL) { + #if MICROPY_MALLOC_USES_ALLOCATED_SIZE + m_free(self->buffers[i], self->buffer_size[i]); + #else + m_free(self->buffers[i]); + #endif + self->buffers[i] = NULL; + } + self->buffer_size[i] = 0; + self->buffer_frames[i] = 0; + self->buffer_ready[i] = false; + self->last_buffer[i] = false; + } +} + +void audiopwmout_background(void) { + for (size_t i = 0; i < MP_ARRAY_SIZE(active_audio); i++) { + audiopwmio_pwmaudioout_obj_t *self = active_audio[i]; + if (self == NULL || !self->playing) { + continue; + } + if (self->stop_queued) { + common_hal_audiopwmio_pwmaudioout_stop(self); + continue; + } + if (self->paused) { + continue; + } + for (uint8_t buf = 0; buf < 2; buf++) { + if (!self->buffer_ready[buf] && !self->stop_queued) { + fill_buffer(self, buf); + } + if (self->single_buffer) { + // A single-buffer sample hands back the same memory every + // time, so there is nothing to gain from a second buffer. + break; + } + } + } +} + +void audiopwmout_reset(void) { + for (size_t i = 0; i < MP_ARRAY_SIZE(active_audio); i++) { + if (active_audio[i] != NULL) { + active_audio[i]->playing = false; + stop_timer(active_audio[i]); + } + } +} + +// Caller validates that pins are free. +void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *self, + const mcu_pin_obj_t *left_channel, const mcu_pin_obj_t *right_channel, uint16_t quiescent_value) { + + self->stereo = right_channel != NULL; + self->left_pwm.deinited = true; + self->right_pwm.deinited = true; + self->timer = NULL; + self->sample = NULL; + self->playing = false; + self->paused = false; + + // variable_frequency is false so that the two channels of a stereo pair can + // share one LEDC timer. At PWM_FREQUENCY pwmio picks a PWM_BITS-wide duty. + pwmout_result_t result = common_hal_pwmio_pwmout_construct( + &self->left_pwm, left_channel, 0, PWM_FREQUENCY, false); + if (result == PWMOUT_OK && self->stereo) { + result = common_hal_pwmio_pwmout_construct( + &self->right_pwm, right_channel, 0, PWM_FREQUENCY, false); + if (result != PWMOUT_OK) { + common_hal_pwmio_pwmout_deinit(&self->left_pwm); + } + } + if (result != PWMOUT_OK) { + common_hal_pwmio_pwmout_raise_error(result); + } + + self->quiescent_value = quiescent_value >> SAMPLE_BITS_TO_DISCARD; + common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, quiescent_value); + if (self->stereo) { + common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, quiescent_value); + } +} + +bool common_hal_audiopwmio_pwmaudioout_deinited(audiopwmio_pwmaudioout_obj_t *self) { + return common_hal_pwmio_pwmout_deinited(&self->left_pwm); +} + +void common_hal_audiopwmio_pwmaudioout_deinit(audiopwmio_pwmaudioout_obj_t *self) { + if (common_hal_audiopwmio_pwmaudioout_deinited(self)) { + return; + } + + common_hal_audiopwmio_pwmaudioout_stop(self); + + common_hal_pwmio_pwmout_deinit(&self->left_pwm); + if (self->stereo) { + common_hal_pwmio_pwmout_deinit(&self->right_pwm); + } +} + +void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, + mp_obj_t sample, bool loop) { + + if (common_hal_audiopwmio_pwmaudioout_get_playing(self)) { + common_hal_audiopwmio_pwmaudioout_stop(self); + } + + audiosample_base_t *base = audiosample_check(sample); + + uint32_t sample_rate = audiosample_get_sample_rate(base); + if (sample_rate == 0 || sample_rate > TIMER_RESOLUTION_HZ) { + mp_arg_error_invalid(MP_QSTR_sample_rate); + } + + self->sample = sample; + self->loop = loop; + self->bytes_per_sample = audiosample_get_bits_per_sample(base) / 8; + self->sample_channel_count = audiosample_get_channel_count(base); + + uint32_t max_buffer_length; + uint8_t spacing; + audiosample_get_buffer_structure(base, false, &self->single_buffer, + &self->signed_to_unsigned, &max_buffer_length, &spacing); + + // Two device buffers of interleaved 16-bit pairs. + size_t frames = max_buffer_length / self->bytes_per_sample / self->sample_channel_count; + size_t buffer_size = frames * 2 * sizeof(uint16_t); + if (buffer_size == 0) { + mp_arg_error_invalid(MP_QSTR_sample); + } + + // Claim a slot before allocating, so a failure here frees nothing. + size_t slot = MP_ARRAY_SIZE(active_audio); + for (size_t i = 0; i < MP_ARRAY_SIZE(active_audio); i++) { + if (active_audio[i] == NULL) { + slot = i; + break; + } + } + if (slot == MP_ARRAY_SIZE(active_audio)) { + mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to start async audio")); + } + + free_buffers(self); + for (uint8_t i = 0; i < 2; i++) { + self->buffers[i] = m_malloc_without_collect(buffer_size); + self->buffer_size[i] = buffer_size; + if (self->single_buffer) { + break; + } + } + + audiosample_reset_buffer(sample, false, 0); + + self->playing_buffer = 0; + self->playing_frame = 0; + self->stop_queued = false; + self->paused = false; + self->playing = true; + + // Prime both buffers before the timer starts. + fill_buffer(self, 0); + if (!self->single_buffer && !self->stop_queued) { + fill_buffer(self, 1); + } + if (self->stop_queued) { + // Nothing to play (empty or broken sample). Don't start the timer, or + // there would be nothing to shut it down again. + common_hal_audiopwmio_pwmaudioout_stop(self); + return; + } + + gptimer_config_t timer_config = { + .clk_src = GPTIMER_CLK_SRC_DEFAULT, + .direction = GPTIMER_COUNT_UP, + .resolution_hz = TIMER_RESOLUTION_HZ, + .intr_priority = 0, + }; + gptimer_event_callbacks_t callbacks = { + .on_alarm = timer_alarm_cb, + }; + // CircuitPython builds with CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM, under which + // gptimer_set_alarm_action() rejects a config that is not in internal RAM. + // The IDF docs call for a static instance here, so give it one. + static gptimer_alarm_config_t alarm_config; + alarm_config.alarm_count = TIMER_RESOLUTION_HZ / sample_rate; + alarm_config.reload_count = 0; + alarm_config.flags.auto_reload_on_alarm = true; + + active_audio[slot] = self; + if (gptimer_new_timer(&timer_config, &self->timer) != ESP_OK || + gptimer_register_event_callbacks(self->timer, &callbacks, self) != ESP_OK || + gptimer_set_alarm_action(self->timer, &alarm_config) != ESP_OK || + gptimer_enable(self->timer) != ESP_OK || + gptimer_start(self->timer) != ESP_OK) { + // Leave nothing half-started: a stuck `playing` would hang any + // `while playing:` loop forever. + self->playing = false; + stop_timer(self); + free_buffers(self); + mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to start async audio")); + } +} + +void common_hal_audiopwmio_pwmaudioout_stop(audiopwmio_pwmaudioout_obj_t *self) { + stop_timer(self); + + self->playing = false; + self->paused = false; + self->stop_queued = false; + self->sample = NULL; + + // Park the output at the quiescent level so the speaker does not sit at a + // DC offset between samples. + if (!common_hal_pwmio_pwmout_deinited(&self->left_pwm)) { + common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, + self->quiescent_value << SAMPLE_BITS_TO_DISCARD); + } + if (self->stereo && !common_hal_pwmio_pwmout_deinited(&self->right_pwm)) { + common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, + self->quiescent_value << SAMPLE_BITS_TO_DISCARD); + } + + free_buffers(self); +} + +bool common_hal_audiopwmio_pwmaudioout_get_playing(audiopwmio_pwmaudioout_obj_t *self) { + return self->playing; +} + +void common_hal_audiopwmio_pwmaudioout_pause(audiopwmio_pwmaudioout_obj_t *self) { + if (!self->playing || self->paused || self->timer == NULL) { + return; + } + gptimer_stop(self->timer); + self->paused = true; +} + +void common_hal_audiopwmio_pwmaudioout_resume(audiopwmio_pwmaudioout_obj_t *self) { + if (!self->paused || self->timer == NULL) { + return; + } + self->paused = false; + gptimer_start(self->timer); +} + +bool common_hal_audiopwmio_pwmaudioout_get_paused(audiopwmio_pwmaudioout_obj_t *self) { + return self->paused; +} diff --git a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h new file mode 100644 index 00000000000..80f636573ba --- /dev/null +++ b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h @@ -0,0 +1,61 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Tyeth Gundry +// +// SPDX-License-Identifier: MIT + +#pragma once + +#include "common-hal/microcontroller/Pin.h" +#include "common-hal/pwmio/PWMOut.h" + +#include "py/obj.h" + +#include "driver/gptimer.h" + +// The LEDC carrier is APB_CLK (80 MHz) / 2**PWM_BITS. 10 bits puts the carrier +// at 78.125 kHz: comfortably above hearing, while leaving 10 bits of amplitude +// resolution. Going finer drags the carrier down into the audible band. +#define PWM_BITS (10) +#define SAMPLE_BITS_TO_DISCARD (16 - PWM_BITS) + +typedef struct { + mp_obj_base_t base; + + pwmio_pwmout_obj_t left_pwm; + pwmio_pwmout_obj_t right_pwm; + gptimer_handle_t timer; + + mp_obj_t sample; + + // Two device buffers of interleaved (left, right) duty values, already + // scaled to PWM_BITS. The ISR drains one while audiopwmout_background() + // refills the other. + uint16_t *buffers[2]; + size_t buffer_size[2]; // allocated size in bytes + size_t buffer_frames[2]; // valid frames currently in the buffer + + volatile size_t playing_frame; // read cursor within the active buffer + volatile uint8_t playing_buffer; // which buffer the ISR is draining + volatile bool buffer_ready[2]; // buffer holds data the ISR has not used + volatile bool last_buffer[2]; // this buffer ends the sample + volatile bool stop_queued; // the ISR reached the end of the sample + + uint16_t quiescent_value; // scaled to PWM_BITS + + uint8_t sample_channel_count; + uint8_t bytes_per_sample; + + bool stereo; // right channel in use + bool signed_to_unsigned; + bool single_buffer; + bool playing; + bool paused; + bool loop; +} audiopwmio_pwmaudioout_obj_t; + +// Called from port_background_task(): refills buffers the ISR has drained. +void audiopwmout_background(void); + +// Called from reset_port(): stops any playback left running by the VM. +void audiopwmout_reset(void); diff --git a/ports/espressif/common-hal/audiopwmio/__init__.c b/ports/espressif/common-hal/audiopwmio/__init__.c new file mode 100644 index 00000000000..98db6232125 --- /dev/null +++ b/ports/espressif/common-hal/audiopwmio/__init__.c @@ -0,0 +1,7 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Tyeth Gundry +// +// SPDX-License-Identifier: MIT + +// No audiopwmio module functions. diff --git a/ports/espressif/mpconfigport.mk b/ports/espressif/mpconfigport.mk index b635f193e1c..4dc09f6d85b 100644 --- a/ports/espressif/mpconfigport.mk +++ b/ports/espressif/mpconfigport.mk @@ -458,6 +458,9 @@ CIRCUITPY_MII = 0 CIRCUITPY_AUDIOIO ?= 1 CIRCUITPY_AUDIOFILEWRITER ?= 1 +# LEDC + GPTimer PWM audio, for boards whose speaker is not on a DAC pin. +CIRCUITPY_AUDIOPWMIO ?= 1 + # No I2S peripheral PDM-to-PCM hardware support CIRCUITPY_AUDIOBUSIO_PDMIN = 0 diff --git a/ports/espressif/supervisor/port.c b/ports/espressif/supervisor/port.c index 91fe8cf5a2e..0eff744c83e 100644 --- a/ports/espressif/supervisor/port.c +++ b/ports/espressif/supervisor/port.c @@ -29,6 +29,9 @@ #include "bindings/espulp/__init__.h" #include "common-hal/microcontroller/Pin.h" #include "common-hal/analogio/AnalogOut.h" +#if CIRCUITPY_AUDIOPWMIO +#include "common-hal/audiopwmio/PWMAudioOut.h" +#endif #include "common-hal/busio/SPI.h" #include "common-hal/busio/UART.h" #include "common-hal/dualbank/__init__.h" @@ -456,6 +459,10 @@ void reset_port(void) { analogout_reset(); #endif + #if CIRCUITPY_AUDIOPWMIO + audiopwmout_reset(); + #endif + #if CIRCUITPY_BUSIO uart_reset(); #endif From 1f6a9ddbb3078088d59c0a91000f4a037ae443f5 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:59:37 +0100 Subject: [PATCH 2/8] espressif/audiopwmio: keep all ISR state in internal RAM gptimer_register_event_callbacks() returned ESP_ERR_INVALID_ARG on the FunHouse, so the alarm callback was never installed: the timer ran, no alarm was ever delivered, nothing consumed samples, and play() left `playing` true forever. The driver is built with CONFIG_GPTIMER_ISR_CACHE_SAFE (selected by CONFIG_GPTIMER_ISR_HANDLER_IN_IRAM, which defaults on), so gptimer_register_event_callbacks() requires the callback in IRAM and its user context in internal RAM. The callback was fine; the user context was not. It was `self`, a MicroPython heap object, and port_malloc() takes MALLOC_CAP_SPIRAM first, so on any board with PSRAM the whole object is external. Clearing that check alone would not be enough. A cache-safe ISR can run while the flash cache is disabled, which happens during any flash write, including the user saving code.py, so everything it dereferences has to be in internal RAM. Follow the pattern common-hal/frequencyio already uses: put the ISR-reachable state in an audiopwmio_internal_data_t allocated with MALLOC_CAP_INTERNAL and pass that as the user context. The LEDC channel numbers are copied in too, so the ISR no longer reaches into the pwmio objects. The audio buffers move to heap_caps_malloc(MALLOC_CAP_ INTERNAL) for the same reason, capped so a large sample fails with ESP_ERR_NO_MEM rather than exhausting internal RAM. The refill path still runs in task context and reaches the object as before. Buffers now hold one value per output channel rather than always a stereo pair, which halves the internal RAM a mono output needs. play() asserts esp_ptr_in_iram() for the callback and esp_ptr_internal() for the control block and both buffers, so a regression surfaces there instead of as a fault at a random later moment. Error paths report the underlying esp_err_t through raise_esp_error() instead of a generic message. Co-Authored-By: Claude Opus 5 --- .../common-hal/audiopwmio/PWMAudioOut.c | 245 +++++++++++------- .../common-hal/audiopwmio/PWMAudioOut.h | 41 +-- 2 files changed, 177 insertions(+), 109 deletions(-) diff --git a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c index 798a9232245..21373bd134a 100644 --- a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c +++ b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.c @@ -11,9 +11,10 @@ // ISR running at the sample rate. Two device buffers are used: the ISR drains // one while audiopwmout_background() refills the other. // -// The ISR deliberately touches nothing but this object's own volatile state. -// In particular it must not queue a background callback: that takes a FreeRTOS -// critical section, which aborts when called from ISR context on this port. +// The ISR reads and writes only the internal-RAM control block, for the two +// reasons set out in PWMAudioOut.h and below. In particular it must not queue +// a background callback: on this port that macro is taskENTER_CRITICAL(), +// which aborts when called from ISR context. #include "common-hal/audiopwmio/PWMAudioOut.h" @@ -23,6 +24,7 @@ #include "py/gc.h" #include "py/runtime.h" +#include "bindings/espidf/__init__.h" #include "shared-bindings/audiopwmio/PWMAudioOut.h" #include "shared-bindings/microcontroller/Pin.h" #include "shared-bindings/pwmio/PWMOut.h" @@ -30,6 +32,8 @@ #include "driver/gptimer.h" #include "esp_attr.h" +#include "esp_heap_caps.h" +#include "esp_memory_utils.h" #include "hal/ledc_ll.h" #include "soc/soc.h" @@ -42,6 +46,11 @@ // (44100 Hz -> 907 ticks -> 44101.4 Hz). #define TIMER_RESOLUTION_HZ (APB_CLK_FREQ / 2) +// Internal RAM is scarce, and both buffers come out of it. This is generous +// for the buffer sizes the audiosample protocol actually hands over (a +// WaveFile uses 512 bytes) while still failing clearly on something silly. +#define MAX_BUFFER_BYTES (8192) + // Each object needs its own LEDC timer, so that bounds how many can play. static audiopwmio_pwmaudioout_obj_t *active_audio[SOC_LEDC_TIMER_NUM]; @@ -57,85 +66,89 @@ static inline void IRAM_ATTR set_duty(uint8_t channel, uint32_t duty) { } static bool IRAM_ATTR timer_alarm_cb(gptimer_handle_t timer, - const gptimer_alarm_event_data_t *edata, void *user_ctx) { - audiopwmio_pwmaudioout_obj_t *self = (audiopwmio_pwmaudioout_obj_t *)user_ctx; + const gptimer_alarm_event_data_t *edata, void *internal_data_ptr) { + audiopwmio_internal_data_t *internal_data = + (audiopwmio_internal_data_t *)internal_data_ptr; (void)timer; (void)edata; - uint8_t buf_index = self->playing_buffer; - if (!self->buffer_ready[buf_index]) { + uint8_t buf_index = internal_data->playing_buffer; + if (!internal_data->buffer_ready[buf_index]) { // Underrun, or the sample ended. Hold the quiescent level rather than // leaving the last sample on the pin, which would be a DC offset. - set_duty(self->left_pwm.chan_handle.channel, self->quiescent_value); - if (self->stereo) { - set_duty(self->right_pwm.chan_handle.channel, self->quiescent_value); + set_duty(internal_data->left_channel, internal_data->quiescent_value); + if (internal_data->stereo) { + set_duty(internal_data->right_channel, internal_data->quiescent_value); } return false; } - size_t frame = self->playing_frame; - const uint16_t *buf = self->buffers[buf_index]; + size_t frame = internal_data->playing_frame; + const uint16_t *sample = + &internal_data->buffers[buf_index][frame * internal_data->out_channels]; - set_duty(self->left_pwm.chan_handle.channel, buf[2 * frame]); - if (self->stereo) { - set_duty(self->right_pwm.chan_handle.channel, buf[2 * frame + 1]); + set_duty(internal_data->left_channel, sample[0]); + if (internal_data->stereo) { + set_duty(internal_data->right_channel, sample[1]); } frame++; - if (frame >= self->buffer_frames[buf_index]) { - if (self->single_buffer && self->loop) { + if (frame >= internal_data->buffer_frames[buf_index]) { + if (internal_data->single_buffer && internal_data->loop) { // The whole sample fits in one buffer and repeats forever, so wrap // in place. This keeps looping RawSamples glitch-free: no refill, // and no dependency on how promptly the background task runs. - self->playing_frame = 0; + internal_data->playing_frame = 0; return false; } // This buffer is spent. Flag it; the background task does the refill. - self->buffer_ready[buf_index] = false; - if (self->last_buffer[buf_index]) { - self->stop_queued = true; + internal_data->buffer_ready[buf_index] = false; + if (internal_data->last_buffer[buf_index]) { + internal_data->stop_queued = true; } - self->playing_frame = 0; - if (!self->single_buffer) { - self->playing_buffer = buf_index ^ 1; + internal_data->playing_frame = 0; + if (!internal_data->single_buffer) { + internal_data->playing_buffer = buf_index ^ 1; } } else { - self->playing_frame = frame; + internal_data->playing_frame = frame; } return false; } // Convert one source buffer into device buffer `buf_index`, scaling samples to -// PWM_BITS and always writing interleaved (left, right) pairs. +// PWM_BITS. Runs in task context, so it may touch the object freely. static void fill_buffer(audiopwmio_pwmaudioout_obj_t *self, uint8_t buf_index) { + audiopwmio_internal_data_t *internal_data = self->internal_data; uint8_t *source; uint32_t source_length; audioio_get_buffer_result_t result = audiosample_get_buffer(self->sample, false, 0, &source, &source_length); if (result == GET_BUFFER_ERROR) { - self->stop_queued = true; + internal_data->stop_queued = true; return; } + uint8_t out_channels = internal_data->out_channels; size_t frames = source_length / self->bytes_per_sample / self->sample_channel_count; - if (frames * 2 * sizeof(uint16_t) > self->buffer_size[buf_index]) { + if (frames * out_channels * sizeof(uint16_t) > self->buffer_size[buf_index]) { // Should not happen: play() sized the buffers from max_buffer_length. - frames = self->buffer_size[buf_index] / (2 * sizeof(uint16_t)); + frames = self->buffer_size[buf_index] / (out_channels * sizeof(uint16_t)); } - uint16_t *dest = self->buffers[buf_index]; + uint16_t *dest = internal_data->buffers[buf_index]; if (self->bytes_per_sample == 1) { uint8_t xor_mask = self->signed_to_unsigned ? 0x80 : 0; for (size_t i = 0; i < frames; i++) { uint16_t left = (uint16_t)((source[i * self->sample_channel_count] ^ xor_mask) << (PWM_BITS - 8)); - uint16_t right = left; - if (self->sample_channel_count == 2) { - right = (uint16_t)((source[i * 2 + 1] ^ xor_mask) << (PWM_BITS - 8)); - } *dest++ = left; - *dest++ = right; + if (out_channels == 2) { + *dest++ = (self->sample_channel_count == 2) + ? (uint16_t)((source[i * 2 + 1] ^ xor_mask) << (PWM_BITS - 8)) + : left; + } } } else { uint16_t xor_mask = self->signed_to_unsigned ? 0x8000 : 0; @@ -144,31 +157,31 @@ static void fill_buffer(audiopwmio_pwmaudioout_obj_t *self, uint8_t buf_index) { const uint16_t *source16 = (const uint16_t *)(const void *)source; for (size_t i = 0; i < frames; i++) { uint16_t left = (uint16_t)((source16[i * self->sample_channel_count] ^ xor_mask) >> SAMPLE_BITS_TO_DISCARD); - uint16_t right = left; - if (self->sample_channel_count == 2) { - right = (uint16_t)((source16[i * 2 + 1] ^ xor_mask) >> SAMPLE_BITS_TO_DISCARD); - } *dest++ = left; - *dest++ = right; + if (out_channels == 2) { + *dest++ = (self->sample_channel_count == 2) + ? (uint16_t)((source16[i * 2 + 1] ^ xor_mask) >> SAMPLE_BITS_TO_DISCARD) + : left; + } } } - self->buffer_frames[buf_index] = frames; - self->last_buffer[buf_index] = false; + internal_data->buffer_frames[buf_index] = frames; + internal_data->last_buffer[buf_index] = false; if (result == GET_BUFFER_DONE) { - if (self->loop) { + if (internal_data->loop) { audiosample_reset_buffer(self->sample, false, 0); } else { - self->last_buffer[buf_index] = true; + internal_data->last_buffer[buf_index] = true; } } // Publish last, so the ISR never sees a half-filled buffer. __asm__ volatile ("" ::: "memory"); - self->buffer_ready[buf_index] = (frames > 0); - if (frames == 0 && result == GET_BUFFER_DONE && !self->loop) { - self->stop_queued = true; + internal_data->buffer_ready[buf_index] = (frames > 0); + if (frames == 0 && result == GET_BUFFER_DONE && !internal_data->loop) { + internal_data->stop_queued = true; } } @@ -190,18 +203,14 @@ static void stop_timer(audiopwmio_pwmaudioout_obj_t *self) { static void free_buffers(audiopwmio_pwmaudioout_obj_t *self) { for (uint8_t i = 0; i < 2; i++) { - if (self->buffers[i] != NULL) { - #if MICROPY_MALLOC_USES_ALLOCATED_SIZE - m_free(self->buffers[i], self->buffer_size[i]); - #else - m_free(self->buffers[i]); - #endif - self->buffers[i] = NULL; + if (self->internal_data->buffers[i] != NULL) { + heap_caps_free(self->internal_data->buffers[i]); + self->internal_data->buffers[i] = NULL; } self->buffer_size[i] = 0; - self->buffer_frames[i] = 0; - self->buffer_ready[i] = false; - self->last_buffer[i] = false; + self->internal_data->buffer_frames[i] = 0; + self->internal_data->buffer_ready[i] = false; + self->internal_data->last_buffer[i] = false; } } @@ -211,7 +220,7 @@ void audiopwmout_background(void) { if (self == NULL || !self->playing) { continue; } - if (self->stop_queued) { + if (self->internal_data->stop_queued) { common_hal_audiopwmio_pwmaudioout_stop(self); continue; } @@ -219,10 +228,10 @@ void audiopwmout_background(void) { continue; } for (uint8_t buf = 0; buf < 2; buf++) { - if (!self->buffer_ready[buf] && !self->stop_queued) { + if (!self->internal_data->buffer_ready[buf] && !self->internal_data->stop_queued) { fill_buffer(self, buf); } - if (self->single_buffer) { + if (self->internal_data->single_buffer) { // A single-buffer sample hands back the same memory every // time, so there is nothing to gain from a second buffer. break; @@ -244,19 +253,27 @@ void audiopwmout_reset(void) { void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *self, const mcu_pin_obj_t *left_channel, const mcu_pin_obj_t *right_channel, uint16_t quiescent_value) { - self->stereo = right_channel != NULL; - self->left_pwm.deinited = true; - self->right_pwm.deinited = true; self->timer = NULL; self->sample = NULL; self->playing = false; self->paused = false; + self->internal_data = heap_caps_calloc(1, sizeof(audiopwmio_internal_data_t), + MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); + if (self->internal_data == NULL) { + raise_esp_error(ESP_ERR_NO_MEM); + } + self->internal_data->stereo = right_channel != NULL; + self->internal_data->out_channels = right_channel != NULL ? 2 : 1; + + self->left_pwm.deinited = true; + self->right_pwm.deinited = true; + // variable_frequency is false so that the two channels of a stereo pair can // share one LEDC timer. At PWM_FREQUENCY pwmio picks a PWM_BITS-wide duty. pwmout_result_t result = common_hal_pwmio_pwmout_construct( &self->left_pwm, left_channel, 0, PWM_FREQUENCY, false); - if (result == PWMOUT_OK && self->stereo) { + if (result == PWMOUT_OK && self->internal_data->stereo) { result = common_hal_pwmio_pwmout_construct( &self->right_pwm, right_channel, 0, PWM_FREQUENCY, false); if (result != PWMOUT_OK) { @@ -264,18 +281,27 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s } } if (result != PWMOUT_OK) { + heap_caps_free(self->internal_data); + self->internal_data = NULL; common_hal_pwmio_pwmout_raise_error(result); } - self->quiescent_value = quiescent_value >> SAMPLE_BITS_TO_DISCARD; + // Copy the channel numbers in: the ISR must not reach into the pwmio + // objects, which live on the (possibly PSRAM) MicroPython heap. + self->internal_data->left_channel = self->left_pwm.chan_handle.channel; + if (self->internal_data->stereo) { + self->internal_data->right_channel = self->right_pwm.chan_handle.channel; + } + + self->internal_data->quiescent_value = quiescent_value >> SAMPLE_BITS_TO_DISCARD; common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, quiescent_value); - if (self->stereo) { + if (self->internal_data->stereo) { common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, quiescent_value); } } bool common_hal_audiopwmio_pwmaudioout_deinited(audiopwmio_pwmaudioout_obj_t *self) { - return common_hal_pwmio_pwmout_deinited(&self->left_pwm); + return self->internal_data == NULL; } void common_hal_audiopwmio_pwmaudioout_deinit(audiopwmio_pwmaudioout_obj_t *self) { @@ -286,9 +312,12 @@ void common_hal_audiopwmio_pwmaudioout_deinit(audiopwmio_pwmaudioout_obj_t *self common_hal_audiopwmio_pwmaudioout_stop(self); common_hal_pwmio_pwmout_deinit(&self->left_pwm); - if (self->stereo) { + if (self->internal_data->stereo) { common_hal_pwmio_pwmout_deinit(&self->right_pwm); } + + heap_caps_free(self->internal_data); + self->internal_data = NULL; } void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, @@ -298,6 +327,7 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, common_hal_audiopwmio_pwmaudioout_stop(self); } + audiopwmio_internal_data_t *internal_data = self->internal_data; audiosample_base_t *base = audiosample_check(sample); uint32_t sample_rate = audiosample_get_sample_rate(base); @@ -306,21 +336,24 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, } self->sample = sample; - self->loop = loop; + internal_data->loop = loop; self->bytes_per_sample = audiosample_get_bits_per_sample(base) / 8; self->sample_channel_count = audiosample_get_channel_count(base); uint32_t max_buffer_length; uint8_t spacing; - audiosample_get_buffer_structure(base, false, &self->single_buffer, + audiosample_get_buffer_structure(base, false, &internal_data->single_buffer, &self->signed_to_unsigned, &max_buffer_length, &spacing); - // Two device buffers of interleaved 16-bit pairs. size_t frames = max_buffer_length / self->bytes_per_sample / self->sample_channel_count; - size_t buffer_size = frames * 2 * sizeof(uint16_t); + size_t buffer_size = frames * internal_data->out_channels * sizeof(uint16_t); if (buffer_size == 0) { mp_arg_error_invalid(MP_QSTR_sample); } + if (buffer_size > MAX_BUFFER_BYTES) { + // These come out of internal RAM, so refuse rather than exhaust it. + raise_esp_error(ESP_ERR_NO_MEM); + } // Claim a slot before allocating, so a failure here frees nothing. size_t slot = MP_ARRAY_SIZE(active_audio); @@ -331,38 +364,58 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, } } if (slot == MP_ARRAY_SIZE(active_audio)) { - mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to start async audio")); + raise_esp_error(ESP_ERR_NOT_FOUND); } free_buffers(self); for (uint8_t i = 0; i < 2; i++) { - self->buffers[i] = m_malloc_without_collect(buffer_size); + // The ISR reads these with the cache possibly disabled, so they have + // to be internal RAM too, not the MicroPython heap. + internal_data->buffers[i] = heap_caps_malloc(buffer_size, + MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); + if (internal_data->buffers[i] == NULL) { + free_buffers(self); + raise_esp_error(ESP_ERR_NO_MEM); + } self->buffer_size[i] = buffer_size; - if (self->single_buffer) { + if (internal_data->single_buffer) { break; } } audiosample_reset_buffer(sample, false, 0); - self->playing_buffer = 0; - self->playing_frame = 0; - self->stop_queued = false; + internal_data->playing_buffer = 0; + internal_data->playing_frame = 0; + internal_data->stop_queued = false; self->paused = false; self->playing = true; // Prime both buffers before the timer starts. fill_buffer(self, 0); - if (!self->single_buffer && !self->stop_queued) { + if (!internal_data->single_buffer && !internal_data->stop_queued) { fill_buffer(self, 1); } - if (self->stop_queued) { + if (internal_data->stop_queued) { // Nothing to play (empty or broken sample). Don't start the timer, or // there would be nothing to shut it down again. common_hal_audiopwmio_pwmaudioout_stop(self); return; } + // The GPTimer driver is built cache-safe here, so it requires the callback + // in IRAM and its user context in internal RAM, and it only checks the + // latter two. Check all of it, so a regression shows up as an error from + // play() rather than as a fault at a random later moment. + if (!esp_ptr_in_iram((const void *)timer_alarm_cb) || + !esp_ptr_internal(internal_data) || + !esp_ptr_internal(internal_data->buffers[0]) || + (internal_data->buffers[1] != NULL && !esp_ptr_internal(internal_data->buffers[1]))) { + self->playing = false; + free_buffers(self); + raise_esp_error(ESP_ERR_INVALID_ARG); + } + gptimer_config_t timer_config = { .clk_src = GPTIMER_CLK_SRC_DEFAULT, .direction = GPTIMER_COUNT_UP, @@ -381,17 +434,26 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, alarm_config.flags.auto_reload_on_alarm = true; active_audio[slot] = self; - if (gptimer_new_timer(&timer_config, &self->timer) != ESP_OK || - gptimer_register_event_callbacks(self->timer, &callbacks, self) != ESP_OK || - gptimer_set_alarm_action(self->timer, &alarm_config) != ESP_OK || - gptimer_enable(self->timer) != ESP_OK || - gptimer_start(self->timer) != ESP_OK) { + esp_err_t err = gptimer_new_timer(&timer_config, &self->timer); + if (err == ESP_OK) { + err = gptimer_register_event_callbacks(self->timer, &callbacks, internal_data); + } + if (err == ESP_OK) { + err = gptimer_set_alarm_action(self->timer, &alarm_config); + } + if (err == ESP_OK) { + err = gptimer_enable(self->timer); + } + if (err == ESP_OK) { + err = gptimer_start(self->timer); + } + if (err != ESP_OK) { // Leave nothing half-started: a stuck `playing` would hang any // `while playing:` loop forever. self->playing = false; stop_timer(self); free_buffers(self); - mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to start async audio")); + raise_esp_error(err); } } @@ -400,18 +462,17 @@ void common_hal_audiopwmio_pwmaudioout_stop(audiopwmio_pwmaudioout_obj_t *self) self->playing = false; self->paused = false; - self->stop_queued = false; + self->internal_data->stop_queued = false; self->sample = NULL; // Park the output at the quiescent level so the speaker does not sit at a // DC offset between samples. + uint16_t quiescent = self->internal_data->quiescent_value << SAMPLE_BITS_TO_DISCARD; if (!common_hal_pwmio_pwmout_deinited(&self->left_pwm)) { - common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, - self->quiescent_value << SAMPLE_BITS_TO_DISCARD); + common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, quiescent); } - if (self->stereo && !common_hal_pwmio_pwmout_deinited(&self->right_pwm)) { - common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, - self->quiescent_value << SAMPLE_BITS_TO_DISCARD); + if (self->internal_data->stereo && !common_hal_pwmio_pwmout_deinited(&self->right_pwm)) { + common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, quiescent); } free_buffers(self); diff --git a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h index 80f636573ba..98e90474cd9 100644 --- a/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h +++ b/ports/espressif/common-hal/audiopwmio/PWMAudioOut.h @@ -19,6 +19,28 @@ #define PWM_BITS (10) #define SAMPLE_BITS_TO_DISCARD (16 - PWM_BITS) +// Everything the alarm ISR touches. CircuitPython builds the GPTimer driver +// with CONFIG_GPTIMER_ISR_CACHE_SAFE, so the ISR can run while the flash cache +// is disabled - during any flash write, including the user saving code.py. +// Anything it dereferences must therefore live in internal RAM, which the +// MicroPython heap is not: port_malloc() prefers PSRAM when a board has it. +typedef struct { + uint16_t *buffers[2]; // interleaved duty values, internal RAM + size_t buffer_frames[2]; // valid frames currently in each buffer + volatile size_t playing_frame; // read cursor within the active buffer + volatile uint8_t playing_buffer; // which buffer the ISR is draining + volatile bool buffer_ready[2]; // buffer holds data the ISR has not used + volatile bool last_buffer[2]; // this buffer ends the sample + volatile bool stop_queued; // the ISR reached the end of the sample + uint16_t quiescent_value; // scaled to PWM_BITS + uint8_t left_channel; // LEDC channel numbers, copied in so the + uint8_t right_channel; // ISR never reaches into the pwmio objects + uint8_t out_channels; // 1 or 2; the stride of buffers[] + bool stereo; + bool single_buffer; + bool loop; +} audiopwmio_internal_data_t; + typedef struct { mp_obj_base_t base; @@ -27,31 +49,16 @@ typedef struct { gptimer_handle_t timer; mp_obj_t sample; + audiopwmio_internal_data_t *internal_data; - // Two device buffers of interleaved (left, right) duty values, already - // scaled to PWM_BITS. The ISR drains one while audiopwmout_background() - // refills the other. - uint16_t *buffers[2]; - size_t buffer_size[2]; // allocated size in bytes - size_t buffer_frames[2]; // valid frames currently in the buffer - - volatile size_t playing_frame; // read cursor within the active buffer - volatile uint8_t playing_buffer; // which buffer the ISR is draining - volatile bool buffer_ready[2]; // buffer holds data the ISR has not used - volatile bool last_buffer[2]; // this buffer ends the sample - volatile bool stop_queued; // the ISR reached the end of the sample - - uint16_t quiescent_value; // scaled to PWM_BITS + size_t buffer_size[2]; // allocated bytes, for the free path uint8_t sample_channel_count; uint8_t bytes_per_sample; - bool stereo; // right channel in use bool signed_to_unsigned; - bool single_buffer; bool playing; bool paused; - bool loop; } audiopwmio_pwmaudioout_obj_t; // Called from port_background_task(): refills buffers the ISR has drained. From b35623107201d6f311490a9171c84ec6eb2623d5 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:33:02 +0100 Subject: [PATCH 3/8] synthio: stop MidiTrack treating every 0xF. byte as End of Track The decoder read any byte with a high nibble of 15 as an "End of Track" event and stopped there. A MIDI track conventionally opens with a Set Tempo meta event, which begins 0xFF, so for most real files the track ended before a single note was played and the sample was silent. Meta and System Exclusive events both carry an explicit length, so skip exactly that many bytes instead of guessing, and end the track only on an actual End of Track meta event. Decoding the first 30 seconds of a stock Bach Toccata now yields 479 and 423 note events on its two note tracks, where it previously yielded 2 and none. Also report error_location as the offset where the error was found rather than the end of the track it was abandoned at. Co-Authored-By: Claude Opus 5 --- shared-module/synthio/MidiTrack.c | 90 +++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 23 deletions(-) diff --git a/shared-module/synthio/MidiTrack.c b/shared-module/synthio/MidiTrack.c index 7c54c36d47f..4988857c080 100644 --- a/shared-module/synthio/MidiTrack.c +++ b/shared-module/synthio/MidiTrack.c @@ -10,15 +10,25 @@ static void record_midi_stream_error(synthio_miditrack_obj_t *self) { - self->error_location = self->pos; + // keep the first error; later ones are usually fallout from this one + if (self->error_location < 0) { + self->error_location = self->pos; + } self->pos = self->track.len; } +// Check that `count` more bytes are available, ending the track if not. +static bool need_bytes(synthio_miditrack_obj_t *self, size_t count) { + if (count > self->track.len - self->pos) { + record_midi_stream_error(self); + return false; + } + return true; +} + static mp_obj_t parse_note(synthio_miditrack_obj_t *self) { uint8_t *buffer = self->track.buf; - size_t len = self->track.len; - if (self->pos + 1 >= len) { - record_midi_stream_error(self); + if (!need_bytes(self, 2)) { return MP_OBJ_NEW_SMALL_INT(0); } uint8_t note = buffer[(self->pos)++]; @@ -28,23 +38,58 @@ static mp_obj_t parse_note(synthio_miditrack_obj_t *self) { return MP_OBJ_NEW_SMALL_INT(note); } +// MIDI variable-length quantity: up to 4 bytes of 7 bits each. +static uint32_t decode_varint(synthio_miditrack_obj_t *self) { + uint8_t *buffer = self->track.buf; + uint32_t value = 0; + for (size_t i = 0; i < 4; i++) { + if (!need_bytes(self, 1)) { + return 0; + } + uint8_t c = buffer[self->pos++]; + value = (value << 7) | (c & 0x7f); + if (!(c & 0x80)) { + return value; + } + } + // a fifth continuation byte cannot be part of a valid quantity + record_midi_stream_error(self); + return 0; +} + static int decode_duration(synthio_miditrack_obj_t *self) { + uint32_t delta = decode_varint(self); + return delta * self->synth.base.sample_rate / self->tempo; +} + +// Meta (0xFF) and system exclusive (0xF0/0xF7) events both carry an explicit +// length, so they can be skipped exactly rather than guessed at. +static void decode_system_event(synthio_miditrack_obj_t *self, uint8_t status) { uint8_t *buffer = self->track.buf; - size_t len = self->track.len; - uint8_t c; - uint32_t delta = 0; - do { - c = buffer[self->pos++]; - delta <<= 7; - delta |= c & 0x7f; - } while ((c & 0x80) && (self->pos < len)); - - // errors cannot be raised from the background task, so simply end the track. - if (c & 0x80) { - self->pos = self->track.len; + if (status == 0xFF) { + if (!need_bytes(self, 1)) { + return; + } + uint8_t type = buffer[self->pos++]; + uint32_t length = decode_varint(self); + if (!need_bytes(self, length)) { + return; + } + if (type == 0x2F) { // End of Track + self->pos = self->track.len; + return; + } + self->pos += length; + } else if (status == 0xF0 || status == 0xF7) { + uint32_t length = decode_varint(self); + if (!need_bytes(self, length)) { + return; + } + self->pos += length; + } else { + // system common and real-time bytes have no meaning in a file record_midi_stream_error(self); } - return delta * self->synth.base.sample_rate / self->tempo; } // invariant: pointing at a MIDI message @@ -55,7 +100,8 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { if (self->pos >= len) { break; } - switch (buffer[self->pos++] >> 4) { + uint8_t status = buffer[self->pos++]; + switch (status >> 4) { case 8: { // Note Off mp_obj_t note = parse_note(self); synthio_span_change_note(&self->synth, note, SYNTHIO_SILENCE); @@ -73,15 +119,13 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { break; case 12: case 13: // one data byte to ignore - if (self->pos >= len || buffer[self->pos++] > 127) { + if (need_bytes(self, 1) && buffer[self->pos++] > 127) { record_midi_stream_error(self); } break; - case 15: // the full syntax is too complicated, just assume it's "End of Track" event - self->pos = len; + default: // 15: meta or system exclusive + decode_system_event(self, status); break; - default: // invalid event - record_midi_stream_error(self); } if (self->pos < len) { self->synth.span.dur = decode_duration(self); From f7b0ce9f15fe077bc1ced93c9bf60b7627562509 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:33:50 +0100 Subject: [PATCH 4/8] synthio: support running status in MidiTrack A MIDI file may omit the status byte when it repeats the previous one, so a run of notes on the same channel is encoded as data bytes alone. The decoder read each of those as a status byte, took the invalid-event branch and ended the track. Remember the last channel status byte and imply it when the next event begins with a data byte. System messages cancel it, as the specification requires. Co-Authored-By: Claude Opus 5 --- shared-module/synthio/MidiTrack.c | 16 +++++++++++++++- shared-module/synthio/MidiTrack.h | 2 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/shared-module/synthio/MidiTrack.c b/shared-module/synthio/MidiTrack.c index 4988857c080..96d9ac6168a 100644 --- a/shared-module/synthio/MidiTrack.c +++ b/shared-module/synthio/MidiTrack.c @@ -100,7 +100,20 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { if (self->pos >= len) { break; } - uint8_t status = buffer[self->pos++]; + uint8_t status = buffer[self->pos]; + if (status & 0x80) { + self->pos++; + // a channel message arms running status; a system one cancels it + self->running_status = (status < 0xF0) ? status : 0; + } else { + // running status: the status byte is omitted and the previous + // channel message is implied + status = self->running_status; + if (status == 0) { + record_midi_stream_error(self); + break; + } + } switch (status >> 4) { case 8: { // Note Off mp_obj_t note = parse_note(self); @@ -136,6 +149,7 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { static void start_parse(synthio_miditrack_obj_t *self) { self->pos = 0; self->error_location = -1; + self->running_status = 0; self->synth.span.dur = decode_duration(self); if (self->synth.span.dur == 0) { // the usual case: the file starts with some MIDI event, not a delay diff --git a/shared-module/synthio/MidiTrack.h b/shared-module/synthio/MidiTrack.h index 1e4d5cba2da..918ff147e3c 100644 --- a/shared-module/synthio/MidiTrack.h +++ b/shared-module/synthio/MidiTrack.h @@ -17,6 +17,8 @@ typedef struct { size_t pos; mp_int_t error_location; uint32_t tempo; + // last channel status byte, for events that omit it (running status) + uint8_t running_status; } synthio_miditrack_obj_t; From 760f02a71452b9a7d263fa027d9bdb8552932f8f Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:34:28 +0100 Subject: [PATCH 5/8] synthio: treat a Note On of velocity 0 as a Note Off in MidiTrack The MIDI specification lets a Note On with a velocity of zero stand in for a Note Off, and files commonly use it so that a run of notes can share one status byte. MidiTrack ignored velocity entirely, so such a note was started and never stopped, and the voice sounded until the track ended. Read the velocity that was already being parsed and discarded, and release the note when it is zero. Co-Authored-By: Claude Opus 5 --- shared-module/synthio/MidiTrack.c | 33 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/shared-module/synthio/MidiTrack.c b/shared-module/synthio/MidiTrack.c index 96d9ac6168a..22058976ae8 100644 --- a/shared-module/synthio/MidiTrack.c +++ b/shared-module/synthio/MidiTrack.c @@ -26,16 +26,20 @@ static bool need_bytes(synthio_miditrack_obj_t *self, size_t count) { return true; } -static mp_obj_t parse_note(synthio_miditrack_obj_t *self) { +// Returns the note in the low 8 bits and the velocity in the next 8, or -1 +// if the pair could not be read. +static int parse_note(synthio_miditrack_obj_t *self) { uint8_t *buffer = self->track.buf; if (!need_bytes(self, 2)) { - return MP_OBJ_NEW_SMALL_INT(0); + return -1; } - uint8_t note = buffer[(self->pos)++]; - if (note > 127 || buffer[(self->pos)++] > 127) { + uint8_t note = buffer[self->pos++]; + uint8_t velocity = buffer[self->pos++]; + if (note > 127 || velocity > 127) { record_midi_stream_error(self); + return -1; } - return MP_OBJ_NEW_SMALL_INT(note); + return note | (velocity << 8); } // MIDI variable-length quantity: up to 4 bytes of 7 bits each. @@ -115,14 +119,17 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { } } switch (status >> 4) { - case 8: { // Note Off - mp_obj_t note = parse_note(self); - synthio_span_change_note(&self->synth, note, SYNTHIO_SILENCE); - break; - } - case 9: { // Note On - mp_obj_t note = parse_note(self); - synthio_span_change_note(&self->synth, SYNTHIO_SILENCE, note); + case 8: // Note Off + case 9: { // Note On, but a velocity of zero means Note Off + int parsed = parse_note(self); + if (parsed >= 0) { + mp_obj_t note = MP_OBJ_NEW_SMALL_INT(parsed & 0xff); + if ((status >> 4) == 9 && (parsed >> 8) != 0) { + synthio_span_change_note(&self->synth, SYNTHIO_SILENCE, note); + } else { + synthio_span_change_note(&self->synth, note, SYNTHIO_SILENCE); + } + } break; } case 10: From 22fd15463cf8955c98bbbb9f509721a44e780978 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:35:22 +0100 Subject: [PATCH 6/8] synthio: keep MidiTrack's timing accurate for long delays span.dur is a uint16_t, so a delay longer than 65535 samples wrapped: at 44.1kHz that is any gap beyond 1.5 seconds, and a held note or a rest of ordinary length came out far too short. The conversion overflowed too, because delta * sample_rate does not fit in 32 bits for a long delay. Compute the duration in 64 bits and carry whatever does not fit in span.dur in a separate field, handing it over a chunk at a time. Co-Authored-By: Claude Opus 5 --- shared-module/synthio/MidiTrack.c | 31 +++++++++++++++++++++++-------- shared-module/synthio/MidiTrack.h | 2 ++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/shared-module/synthio/MidiTrack.c b/shared-module/synthio/MidiTrack.c index 22058976ae8..2d090be2696 100644 --- a/shared-module/synthio/MidiTrack.c +++ b/shared-module/synthio/MidiTrack.c @@ -26,6 +26,14 @@ static bool need_bytes(synthio_miditrack_obj_t *self, size_t count) { return true; } +// A duration can be longer than span.dur (a uint16_t) can hold, so the +// remainder is kept in `pending` and handed over a chunk at a time. +static void set_remaining(synthio_miditrack_obj_t *self, uint32_t samples) { + uint16_t chunk = samples > UINT16_MAX ? UINT16_MAX : (uint16_t)samples; + self->synth.span.dur = chunk; + self->pending = samples - chunk; +} + // Returns the note in the low 8 bits and the velocity in the next 8, or -1 // if the pair could not be read. static int parse_note(synthio_miditrack_obj_t *self) { @@ -61,9 +69,10 @@ static uint32_t decode_varint(synthio_miditrack_obj_t *self) { return 0; } -static int decode_duration(synthio_miditrack_obj_t *self) { +static void decode_duration(synthio_miditrack_obj_t *self) { uint32_t delta = decode_varint(self); - return delta * self->synth.base.sample_rate / self->tempo; + uint64_t samples = (uint64_t)delta * self->synth.base.sample_rate / self->tempo; + set_remaining(self, samples > UINT32_MAX ? UINT32_MAX : (uint32_t)samples); } // Meta (0xFF) and system exclusive (0xF0/0xF7) events both carry an explicit @@ -148,17 +157,18 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { break; } if (self->pos < len) { - self->synth.span.dur = decode_duration(self); + decode_duration(self); } - } while (self->pos < len && self->synth.span.dur == 0); + } while (self->pos < len && self->synth.span.dur == 0 && self->pending == 0); } static void start_parse(synthio_miditrack_obj_t *self) { self->pos = 0; self->error_location = -1; self->running_status = 0; - self->synth.span.dur = decode_duration(self); - if (self->synth.span.dur == 0) { + self->pending = 0; + decode_duration(self); + if (self->synth.span.dur == 0 && self->pending == 0) { // the usual case: the file starts with some MIDI event, not a delay decode_until_pause(self); } @@ -191,7 +201,9 @@ mp_int_t common_hal_synthio_miditrack_get_tempo(synthio_miditrack_obj_t *self) { void common_hal_synthio_miditrack_set_tempo(synthio_miditrack_obj_t *self, mp_int_t value) { mp_int_t val = mp_arg_validate_int_min(value, 1, MP_QSTR_tempo); - self->synth.span.dur = (uint32_t)self->synth.span.dur * self->tempo / val; + uint64_t remaining = (uint64_t)self->synth.span.dur + self->pending; + remaining = remaining * self->tempo / val; + set_remaining(self, remaining > UINT32_MAX ? UINT32_MAX : (uint32_t)remaining); self->tempo = val; } @@ -210,7 +222,10 @@ audioio_get_buffer_result_t synthio_miditrack_get_buffer(synthio_miditrack_obj_t synthio_synth_synthesize(&self->synth, buffer, buffer_length, single_channel_output ? 0 : channel); if (self->synth.span.dur == 0) { - if (self->pos == self->track.len) { + if (self->pending != 0) { + // still inside a delay too long for a single span + set_remaining(self, self->pending); + } else if (self->pos == self->track.len) { return GET_BUFFER_DONE; } else { decode_until_pause(self); diff --git a/shared-module/synthio/MidiTrack.h b/shared-module/synthio/MidiTrack.h index 918ff147e3c..674e73c6d4e 100644 --- a/shared-module/synthio/MidiTrack.h +++ b/shared-module/synthio/MidiTrack.h @@ -17,6 +17,8 @@ typedef struct { size_t pos; mp_int_t error_location; uint32_t tempo; + // samples of the current delay that did not fit in span.dur + uint32_t pending; // last channel status byte, for events that omit it (running status) uint8_t running_status; } synthio_miditrack_obj_t; From 68c4509d13c8d96308a2d668184576367abe47b3 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 01:35:55 +0100 Subject: [PATCH 7/8] synthio: follow the MIDI file's tempo map in MidiTrack MidiTrack played at one fixed rate for the whole track, so a file that changes tempo, or that simply does not use the 120 BPM default, played at the wrong speed throughout. The Set Tempo meta event was skipped. Act on Set Tempo. Its value is in microseconds per quarter note, which can only be converted to the ticks per second MidiTrack works in if the ticks per quarter note is known, so take that as a new optional ticks_per_beat argument. from_file() reads it from the file header; it is left at 0 for SMPTE timing, where the rate is already absolute and Set Tempo does not apply, and for callers that pass raw track bytes without it. The tempo is rewound along with the track when the sample restarts, so a looping track does not inherit the rate its last loop ended on. Co-Authored-By: Claude Opus 5 --- shared-bindings/synthio/MidiTrack.c | 19 +++++++++++++++---- shared-bindings/synthio/MidiTrack.h | 2 +- shared-bindings/synthio/__init__.c | 16 +++++++++++++--- shared-module/synthio/MidiTrack.c | 27 +++++++++++++++++++++------ shared-module/synthio/MidiTrack.h | 5 +++++ 5 files changed, 55 insertions(+), 14 deletions(-) diff --git a/shared-bindings/synthio/MidiTrack.c b/shared-bindings/synthio/MidiTrack.c index 89c50282bab..83b2fd94100 100644 --- a/shared-bindings/synthio/MidiTrack.c +++ b/shared-bindings/synthio/MidiTrack.c @@ -23,16 +23,25 @@ //| buffer: ReadableBuffer, //| tempo: int, //| *, +//| ticks_per_beat: int = 0, //| sample_rate: int = 11025, //| waveform: Optional[ReadableBuffer] = None, //| envelope: Optional[Envelope] = None, //| ) -> None: -//| """Create a MidiTrack from the given stream of MIDI events. Only "Note On" and "Note Off" events -//| are supported; channel numbers and key velocities are ignored. Up to two notes may be on at the -//| same time. +//| """Create a MidiTrack from the given stream of MIDI events. +//| +//| "Note On" and "Note Off" events are played; a "Note On" with a velocity of zero is +//| treated as a "Note Off", as the MIDI specification allows. Other channel events are +//| skipped, as are System Exclusive and meta events, except that "End of Track" stops +//| playback and "Set Tempo" adjusts it when ``ticks_per_beat`` is known. Events that omit +//| a repeated status byte ("running status") are understood. Channel numbers and key +//| velocities are otherwise ignored. //| //| :param ~circuitpython_typing.ReadableBuffer buffer: Stream of MIDI events, as stored in a MIDI file track chunk //| :param int tempo: Tempo of the streamed events, in MIDI ticks per second +//| :param int ticks_per_beat: Ticks per quarter note, from the MIDI file header. When this +//| is 0 (the default) any "Set Tempo" event in the stream is ignored, because its +//| microseconds-per-quarter-note value cannot be converted without it. //| :param int sample_rate: The desired playback sample rate; higher sample rate requires more memory //| :param ReadableBuffer waveform: A single-cycle waveform. Default is a 50% duty cycle square wave. If specified, must be a ReadableBuffer of type 'h' (signed 16 bit) //| :param Envelope envelope: An object that defines the loudness of a note over time. The default envelope provides no ramping, voices turn instantly on and off. @@ -55,10 +64,11 @@ //| ... //| static mp_obj_t synthio_miditrack_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { - enum { ARG_buffer, ARG_tempo, ARG_sample_rate, ARG_waveform, ARG_envelope }; + enum { ARG_buffer, ARG_tempo, ARG_ticks_per_beat, ARG_sample_rate, ARG_waveform, ARG_envelope }; static const mp_arg_t allowed_args[] = { { MP_QSTR_buffer, MP_ARG_OBJ | MP_ARG_REQUIRED, {} }, { MP_QSTR_tempo, MP_ARG_INT | MP_ARG_REQUIRED, {} }, + { MP_QSTR_ticks_per_beat, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0} }, { MP_QSTR_sample_rate, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 11025} }, { MP_QSTR_waveform, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none } }, { MP_QSTR_envelope, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none } }, @@ -73,6 +83,7 @@ static mp_obj_t synthio_miditrack_make_new(const mp_obj_type_t *type, size_t n_a common_hal_synthio_miditrack_construct(self, (uint8_t *)bufinfo.buf, bufinfo.len, mp_arg_validate_int_min(args[ARG_tempo].u_int, 1, MP_QSTR_tempo), + args[ARG_ticks_per_beat].u_int, args[ARG_sample_rate].u_int, args[ARG_waveform].u_obj, mp_const_none, diff --git a/shared-bindings/synthio/MidiTrack.h b/shared-bindings/synthio/MidiTrack.h index cb154d3996b..48e430156d5 100644 --- a/shared-bindings/synthio/MidiTrack.h +++ b/shared-bindings/synthio/MidiTrack.h @@ -11,7 +11,7 @@ extern const mp_obj_type_t synthio_miditrack_type; -void common_hal_synthio_miditrack_construct(synthio_miditrack_obj_t *self, const uint8_t *buffer, uint32_t len, uint32_t tempo, uint32_t sample_rate, mp_obj_t waveform_obj, mp_obj_t filter_obj, mp_obj_t envelope_obj); +void common_hal_synthio_miditrack_construct(synthio_miditrack_obj_t *self, const uint8_t *buffer, uint32_t len, uint32_t tempo, uint32_t ticks_per_beat, uint32_t sample_rate, mp_obj_t waveform_obj, mp_obj_t filter_obj, mp_obj_t envelope_obj); void common_hal_synthio_miditrack_deinit(synthio_miditrack_obj_t *self); mp_int_t common_hal_synthio_miditrack_get_error_location(synthio_miditrack_obj_t *self); diff --git a/shared-bindings/synthio/__init__.c b/shared-bindings/synthio/__init__.c index 3f9bc3f9274..b77e312974b 100644 --- a/shared-bindings/synthio/__init__.c +++ b/shared-bindings/synthio/__init__.c @@ -177,6 +177,9 @@ const mp_obj_namedtuple_type_t synthio_envelope_type_obj = { //| """Create an AudioSample from an already opened MIDI file. //| Currently, only single-track MIDI (type 0) is supported. //| +//| The file's tempo map is followed: the track starts at the MIDI default of 120 BPM and +//| each "Set Tempo" event changes the rate from that point on. +//| //| :param typing.BinaryIO file: Already opened MIDI file //| :param int sample_rate: The desired playback sample rate; higher sample rate requires more memory //| :param ReadableBuffer waveform: A single-cycle waveform. Default is a 50% duty cycle square wave. If specified, must be a ReadableBuffer of type 'h' (signed 16 bit) @@ -233,11 +236,18 @@ static mp_obj_t synthio_from_file(size_t n_args, const mp_obj_t *pos_args, mp_ma // TODO: for a multi-track MIDI (type 1), return an AudioMixer } - uint16_t tempo; + uint32_t tempo; + uint32_t ticks_per_beat; if (chunk_header[12] & 0x80) { + // SMPTE: frames per second times ticks per frame is already an + // absolute rate, so Set Tempo events do not apply to it tempo = -(int8_t)chunk_header[12] * chunk_header[13]; + ticks_per_beat = 0; } else { - tempo = 2 * ((chunk_header[12] << 8) | chunk_header[13]); + ticks_per_beat = (chunk_header[12] << 8) | chunk_header[13]; + // until a Set Tempo event says otherwise, MIDI defaults to 120 BPM, + // i.e. two quarter notes per second + tempo = 2 * ticks_per_beat; } bytes_read = mp_stream_rw(file_obj, chunk_header, 8, &errcode, MP_STREAM_RW_READ); @@ -260,7 +270,7 @@ static mp_obj_t synthio_from_file(size_t n_args, const mp_obj_t *pos_args, mp_ma synthio_miditrack_obj_t *result = mp_obj_malloc_with_finaliser(synthio_miditrack_obj_t, &synthio_miditrack_type); common_hal_synthio_miditrack_construct(result, buffer, track_size, - tempo, args[ARG_sample_rate].u_int, args[ARG_waveform].u_obj, + tempo, ticks_per_beat, args[ARG_sample_rate].u_int, args[ARG_waveform].u_obj, mp_const_none, args[ARG_envelope].u_obj ); diff --git a/shared-module/synthio/MidiTrack.c b/shared-module/synthio/MidiTrack.c index 2d090be2696..23641c12a2c 100644 --- a/shared-module/synthio/MidiTrack.c +++ b/shared-module/synthio/MidiTrack.c @@ -92,6 +92,16 @@ static void decode_system_event(synthio_miditrack_obj_t *self, uint8_t status) { self->pos = self->track.len; return; } + if (type == 0x51 && length == 3 && self->ticks_per_beat != 0) { // Set Tempo + uint32_t us_per_beat = (buffer[self->pos] << 16) | + (buffer[self->pos + 1] << 8) | buffer[self->pos + 2]; + if (us_per_beat != 0) { + uint32_t ticks = (uint32_t)(((uint64_t)self->ticks_per_beat * 1000000) / us_per_beat); + if (ticks != 0) { + self->tempo = ticks; + } + } + } self->pos += length; } else if (status == 0xF0 || status == 0xF7) { uint32_t length = decode_varint(self); @@ -165,8 +175,10 @@ static void decode_until_pause(synthio_miditrack_obj_t *self) { static void start_parse(synthio_miditrack_obj_t *self) { self->pos = 0; self->error_location = -1; - self->running_status = 0; self->pending = 0; + self->running_status = 0; + // a tempo map applies from the top of the track, so rewind it too + self->tempo = self->base_tempo; decode_duration(self); if (self->synth.span.dur == 0 && self->pending == 0) { // the usual case: the file starts with some MIDI event, not a delay @@ -175,10 +187,12 @@ static void start_parse(synthio_miditrack_obj_t *self) { } void common_hal_synthio_miditrack_construct(synthio_miditrack_obj_t *self, - const uint8_t *buffer, uint32_t len, uint32_t tempo, uint32_t sample_rate, - mp_obj_t waveform_obj, mp_obj_t filter_obj, mp_obj_t envelope_obj) { + const uint8_t *buffer, uint32_t len, uint32_t tempo, uint32_t ticks_per_beat, + uint32_t sample_rate, mp_obj_t waveform_obj, mp_obj_t filter_obj, + mp_obj_t envelope_obj) { - self->tempo = tempo; + self->tempo = self->base_tempo = tempo; + self->ticks_per_beat = ticks_per_beat; self->track.buf = (void *)buffer; self->track.len = len; @@ -204,7 +218,8 @@ void common_hal_synthio_miditrack_set_tempo(synthio_miditrack_obj_t *self, mp_in uint64_t remaining = (uint64_t)self->synth.span.dur + self->pending; remaining = remaining * self->tempo / val; set_remaining(self, remaining > UINT32_MAX ? UINT32_MAX : (uint32_t)remaining); - self->tempo = val; + // becomes the new baseline, so that looping does not undo it + self->tempo = self->base_tempo = val; } void synthio_miditrack_reset_buffer(synthio_miditrack_obj_t *self, @@ -223,7 +238,7 @@ audioio_get_buffer_result_t synthio_miditrack_get_buffer(synthio_miditrack_obj_t synthio_synth_synthesize(&self->synth, buffer, buffer_length, single_channel_output ? 0 : channel); if (self->synth.span.dur == 0) { if (self->pending != 0) { - // still inside a delay too long for a single span + // still inside a gap too long for a single span set_remaining(self, self->pending); } else if (self->pos == self->track.len) { return GET_BUFFER_DONE; diff --git a/shared-module/synthio/MidiTrack.h b/shared-module/synthio/MidiTrack.h index 674e73c6d4e..e8a6a9bf864 100644 --- a/shared-module/synthio/MidiTrack.h +++ b/shared-module/synthio/MidiTrack.h @@ -16,9 +16,14 @@ typedef struct { // invariant: after initial startup, pos always points just after an encoded duration, i.e., at a midi message (or at EOF) size_t pos; mp_int_t error_location; + // current tempo in ticks per second, and the value to rewind it to uint32_t tempo; + uint32_t base_tempo; // samples of the current delay that did not fit in span.dur uint32_t pending; + // ticks per quarter note (15 bits in the file header), or 0 when the + // timing is absolute (SMPTE) and Set Tempo events do not apply + uint16_t ticks_per_beat; // last channel status byte, for events that omit it (running status) uint8_t running_status; } synthio_miditrack_obj_t; From c8554c8b77c8e196991e8ebd5581ad445c2012c8 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 23 Oct 2026 13:53:48 +0100 Subject: [PATCH 8/8] ci_fetch_deps: fetch lib/mp3 and extmod/ulab for zephyr-cp zephyr-cp builds audiomp3 and ulab, but its PORT_DEPS left out both submodules. Board builds only worked when the scheduler job's submodules-common cache restored; with a cold cache they fail on lib/mp3/src/mp3common.h. Fixes #11655 Co-Authored-By: Claude Opus 5.5 --- tools/ci_fetch_deps.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/ci_fetch_deps.py b/tools/ci_fetch_deps.py index c50746651b6..312b87c5ba4 100644 --- a/tools/ci_fetch_deps.py +++ b/tools/ci_fetch_deps.py @@ -96,7 +96,9 @@ def _all_submodules(): "data/nvm.toml/", ], "zephyr-cp": [ + "extmod/ulab/", "lib/certificates/", + "lib/mp3/", "lib/tinyusb/", "lib/tlsf", ],