From 398eb459292ae9759875d2dd13c0c12112a8713a Mon Sep 17 00:00:00 2001 From: dcooperdalrymple Date: Fri, 6 Dec 2024 09:12:50 -0600 Subject: [PATCH 1/4] Always update `echo_buffer_len` during `recalculate_delay` when `freq_shift=False` to avoid `echo_buffer_len=0` when `delay_ms` is invalid within constructor. --- shared-module/audiodelays/Echo.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/shared-module/audiodelays/Echo.c b/shared-module/audiodelays/Echo.c index e4eeff48f4c3..2c499d1ac40f 100644 --- a/shared-module/audiodelays/Echo.c +++ b/shared-module/audiodelays/Echo.c @@ -132,23 +132,8 @@ void recalculate_delay(audiodelays_echo_obj_t *self, mp_float_t f_delay_ms) { self->echo_buffer_rate = (uint32_t)MAX(self->max_delay_ms / f_delay_ms * MICROPY_FLOAT_CONST(256.0), MICROPY_FLOAT_CONST(1.0)); self->echo_buffer_len = self->max_echo_buffer_len; } else { - // Calculate the current echo buffer length in bytes - uint32_t new_echo_buffer_len = (uint32_t)(self->sample_rate / MICROPY_FLOAT_CONST(1000.0) * f_delay_ms) * (self->channel_count * sizeof(uint16_t)); - - // Check if our new echo is too long for our maximum buffer - if (new_echo_buffer_len > self->max_echo_buffer_len) { - return; - } else if (new_echo_buffer_len < 0.0) { // or too short! - return; - } - - // If the echo buffer is larger then our audio buffer weird things happen - if (new_echo_buffer_len < self->buffer_len) { - return; - } - - self->echo_buffer_len = new_echo_buffer_len; - + // Calculate the current echo buffer length in bytes and limit to valid range + self->echo_buffer_len = (uint32_t)MIN(MAX((self->sample_rate / MICROPY_FLOAT_CONST(1000.0) * f_delay_ms) * (self->channel_count * sizeof(uint16_t)), self->channel_count * sizeof(uint16_t)), self->max_echo_buffer_len); // Clear the now unused part of the buffer or some weird artifacts appear memset(self->echo_buffer + self->echo_buffer_len, 0, self->max_echo_buffer_len - self->echo_buffer_len); } From cf6086ddcba602f3387426b57a9e331e4a975a94 Mon Sep 17 00:00:00 2001 From: dcooperdalrymple Date: Fri, 6 Dec 2024 10:02:31 -0600 Subject: [PATCH 2/4] Fix casting error in buffer length calculation. --- shared-module/audiodelays/Echo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shared-module/audiodelays/Echo.c b/shared-module/audiodelays/Echo.c index 2c499d1ac40f..d8f0b85ceb14 100644 --- a/shared-module/audiodelays/Echo.c +++ b/shared-module/audiodelays/Echo.c @@ -133,7 +133,8 @@ void recalculate_delay(audiodelays_echo_obj_t *self, mp_float_t f_delay_ms) { self->echo_buffer_len = self->max_echo_buffer_len; } else { // Calculate the current echo buffer length in bytes and limit to valid range - self->echo_buffer_len = (uint32_t)MIN(MAX((self->sample_rate / MICROPY_FLOAT_CONST(1000.0) * f_delay_ms) * (self->channel_count * sizeof(uint16_t)), self->channel_count * sizeof(uint16_t)), self->max_echo_buffer_len); + self->echo_buffer_len = MIN(MAX((uint32_t)(self->sample_rate / MICROPY_FLOAT_CONST(1000.0) * f_delay_ms) * (self->channel_count * sizeof(uint16_t)), self->channel_count * sizeof(uint16_t)), self->max_echo_buffer_len); + // Clear the now unused part of the buffer or some weird artifacts appear memset(self->echo_buffer + self->echo_buffer_len, 0, self->max_echo_buffer_len - self->echo_buffer_len); } From 83fce94b3b61c5dc9fb7eb3468c5850e1839fa0f Mon Sep 17 00:00:00 2001 From: dcooperdalrymple Date: Fri, 6 Dec 2024 10:04:55 -0600 Subject: [PATCH 3/4] Fix handling of echo buffer when `freq_shift=True` and `channel_count=2` and remove unnecessary position variables. --- shared-module/audiodelays/Echo.c | 94 +++++++++++++------------------- shared-module/audiodelays/Echo.h | 5 +- 2 files changed, 40 insertions(+), 59 deletions(-) diff --git a/shared-module/audiodelays/Echo.c b/shared-module/audiodelays/Echo.c index d8f0b85ceb14..ac77691c9f6a 100644 --- a/shared-module/audiodelays/Echo.c +++ b/shared-module/audiodelays/Echo.c @@ -91,11 +91,9 @@ void common_hal_audiodelays_echo_construct(audiodelays_echo_obj_t *self, uint32_ // read is where we read previous echo from delay_ms ago to play back now // write is where the store the latest playing sample to echo back later - self->echo_buffer_read_pos = self->buffer_len / sizeof(uint16_t); - self->echo_buffer_write_pos = 0; - - // where we read the previous echo from delay_ms ago to play back now (for freq shift) - self->echo_buffer_left_pos = self->echo_buffer_right_pos = 0; + self->echo_buffer_pos = 0; + // use a separate buffer position for the right channel when using freq_shift + self->echo_buffer_right_pos = 0; } bool common_hal_audiodelays_echo_deinited(audiodelays_echo_obj_t *self) { @@ -130,7 +128,8 @@ void recalculate_delay(audiodelays_echo_obj_t *self, mp_float_t f_delay_ms) { if (self->freq_shift) { // Calculate the rate of iteration over the echo buffer with 8 sub-bits self->echo_buffer_rate = (uint32_t)MAX(self->max_delay_ms / f_delay_ms * MICROPY_FLOAT_CONST(256.0), MICROPY_FLOAT_CONST(1.0)); - self->echo_buffer_len = self->max_echo_buffer_len; + // Only use half of the buffer per channel if stereo + self->echo_buffer_len = self->max_echo_buffer_len >> (self->channel_count - 1); } else { // Calculate the current echo buffer length in bytes and limit to valid range self->echo_buffer_len = MIN(MAX((uint32_t)(self->sample_rate / MICROPY_FLOAT_CONST(1000.0) * f_delay_ms) * (self->channel_count * sizeof(uint16_t)), self->channel_count * sizeof(uint16_t)), self->max_echo_buffer_len); @@ -290,14 +289,9 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * // The echo buffer is always stored as a 16-bit value internally int16_t *echo_buffer = (int16_t *)self->echo_buffer; uint32_t echo_buf_len = self->echo_buffer_len / sizeof(uint16_t); - - // Set our echo buffer position accounting for stereo - uint32_t echo_buffer_pos = 0; - if (self->freq_shift) { - echo_buffer_pos = self->echo_buffer_left_pos; - if (channel == 1) { - echo_buffer_pos = self->echo_buffer_right_pos; - } + uint32_t echo_buf_pos = self->echo_buffer_pos; + if (self->freq_shift && channel == 1) { + echo_buf_pos = self->echo_buffer_right_pos; } // Loop over the entire length of our buffer to fill it, this may require several calls to get data from the sample @@ -341,19 +335,20 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * for (uint32_t i = 0; i < length; i++) { int16_t echo, word = 0; uint32_t next_buffer_pos = 0; + bool echo_buf_offset = self->freq_shift && (channel == 1 || i % self->channel_count == 1); if (self->freq_shift) { - echo = echo_buffer[echo_buffer_pos >> 8]; - next_buffer_pos = echo_buffer_pos + self->echo_buffer_rate; + echo = echo_buffer[(echo_buf_pos >> 8) + echo_buf_len * echo_buf_offset]; + next_buffer_pos = echo_buf_pos + self->echo_buffer_rate; word = (int16_t)(echo * decay); - for (uint32_t j = echo_buffer_pos >> 8; j < next_buffer_pos >> 8; j++) { - echo_buffer[j % echo_buf_len] = word; + for (uint32_t j = echo_buf_pos >> 8; j < next_buffer_pos >> 8; j++) { + echo_buffer[(j % echo_buf_len) + echo_buf_len * echo_buf_offset] = word; } } else { - echo = echo_buffer[self->echo_buffer_read_pos++]; + echo = echo_buffer[echo_buf_pos]; word = (int16_t)(echo * decay); - echo_buffer[self->echo_buffer_write_pos++] = word; + echo_buffer[echo_buf_pos++] = word; } word = (int16_t)(echo * mix); @@ -370,15 +365,10 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * } } - if (self->freq_shift) { - echo_buffer_pos = next_buffer_pos % (echo_buf_len << 8); - } else { - if (self->echo_buffer_read_pos >= echo_buf_len) { - self->echo_buffer_read_pos = 0; - } - if (self->echo_buffer_write_pos >= echo_buf_len) { - self->echo_buffer_write_pos = 0; - } + if (self->freq_shift && (single_channel_output || echo_buf_offset)) { + echo_buf_pos = next_buffer_pos % (echo_buf_len << 8); + } else if (!self->freq_shift && echo_buf_pos >= echo_buf_len) { + echo_buf_pos = 0; } } } @@ -416,23 +406,23 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * int32_t echo, word = 0; uint32_t next_buffer_pos = 0; + bool echo_buf_offset = self->freq_shift && (channel == 1 || i % self->channel_count == 1); if (self->freq_shift) { - echo = echo_buffer[echo_buffer_pos >> 8]; - next_buffer_pos = echo_buffer_pos + self->echo_buffer_rate; - word = (int32_t)(echo * decay + sample_word); + echo = echo_buffer[(echo_buf_pos >> 8) + echo_buf_len * echo_buf_offset]; + next_buffer_pos = echo_buf_pos + self->echo_buffer_rate; } else { - echo = echo_buffer[self->echo_buffer_read_pos++]; - word = (int32_t)(echo * decay + sample_word); + echo = echo_buffer[echo_buf_pos]; } + word = (int32_t)(echo * decay + sample_word); if (MP_LIKELY(self->bits_per_sample == 16)) { word = mix_down_sample(word); if (self->freq_shift) { - for (uint32_t j = echo_buffer_pos >> 8; j < next_buffer_pos >> 8; j++) { - echo_buffer[j % echo_buf_len] = (int16_t)word; + for (uint32_t j = echo_buf_pos >> 8; j < next_buffer_pos >> 8; j++) { + echo_buffer[(j % echo_buf_len) + echo_buf_len * echo_buf_offset] = (int16_t)word; } } else { - echo_buffer[self->echo_buffer_write_pos++] = (int16_t)word; + echo_buffer[echo_buf_pos++] = (int16_t)word; } } else { // Do not have mix_down for 8 bit so just hard cap samples into 1 byte @@ -442,11 +432,11 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * word = -128; } if (self->freq_shift) { - for (uint32_t j = echo_buffer_pos >> 8; j < next_buffer_pos >> 8; j++) { - echo_buffer[j % echo_buf_len] = (int8_t)word; + for (uint32_t j = echo_buf_pos >> 8; j < next_buffer_pos >> 8; j++) { + echo_buffer[(j % echo_buf_len) + echo_buf_offset] = (int8_t)word; } } else { - echo_buffer[self->echo_buffer_write_pos++] = (int8_t)word; + echo_buffer[echo_buf_pos++] = (int8_t)word; } } @@ -466,15 +456,10 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * } } - if (self->freq_shift) { - echo_buffer_pos = next_buffer_pos % (echo_buf_len << 8); - } else { - if (self->echo_buffer_read_pos >= echo_buf_len) { - self->echo_buffer_read_pos = 0; - } - if (self->echo_buffer_write_pos >= echo_buf_len) { - self->echo_buffer_write_pos = 0; - } + if (self->freq_shift && (single_channel_output || echo_buf_offset)) { + echo_buf_pos = next_buffer_pos % (echo_buf_len << 8); + } else if (!self->freq_shift && echo_buf_pos >= echo_buf_len) { + echo_buf_pos = 0; } } } @@ -488,12 +473,11 @@ audioio_get_buffer_result_t audiodelays_echo_get_buffer(audiodelays_echo_obj_t * } } - if (self->freq_shift) { - if (channel == 0) { - self->echo_buffer_left_pos = echo_buffer_pos; - } else if (channel == 1) { - self->echo_buffer_right_pos = echo_buffer_pos; - } + // Update buffer position + if (self->freq_shift && channel == 1) { + self->echo_buffer_right_pos = echo_buf_pos; + } else { + self->echo_buffer_pos = echo_buf_pos; } // Finally pass our buffer and length to the calling audio function diff --git a/shared-module/audiodelays/Echo.h b/shared-module/audiodelays/Echo.h index 8742f83898a7..17aaec20a933 100644 --- a/shared-module/audiodelays/Echo.h +++ b/shared-module/audiodelays/Echo.h @@ -40,11 +40,8 @@ typedef struct { uint32_t echo_buffer_len; // bytes uint32_t max_echo_buffer_len; // bytes - uint32_t echo_buffer_read_pos; // words - uint32_t echo_buffer_write_pos; // words - + uint32_t echo_buffer_pos; // words (<< 8 when freq_shift=True) uint32_t echo_buffer_rate; // words << 8 - uint32_t echo_buffer_left_pos; // words << 8 uint32_t echo_buffer_right_pos; // words << 8 mp_obj_t sample; From 9e36143416c08b1c91d673ad5dc825b6391166fb Mon Sep 17 00:00:00 2001 From: dcooperdalrymple Date: Fri, 6 Dec 2024 10:05:11 -0600 Subject: [PATCH 4/4] Reset buffer if changing `freq_shift` modes. --- shared-module/audiodelays/Echo.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shared-module/audiodelays/Echo.c b/shared-module/audiodelays/Echo.c index ac77691c9f6a..6a56c59db4d5 100644 --- a/shared-module/audiodelays/Echo.c +++ b/shared-module/audiodelays/Echo.c @@ -162,6 +162,12 @@ bool common_hal_audiodelays_echo_get_freq_shift(audiodelays_echo_obj_t *self) { } void common_hal_audiodelays_echo_set_freq_shift(audiodelays_echo_obj_t *self, bool freq_shift) { + // Clear the echo buffer and reset buffer position if changing freq_shift modes + if (self->freq_shift != freq_shift) { + memset(self->echo_buffer, 0, self->max_echo_buffer_len); + self->echo_buffer_pos = 0; + self->echo_buffer_right_pos = 0; + } self->freq_shift = freq_shift; uint32_t delay_ms = (uint32_t)synthio_block_slot_get(&self->delay_ms); recalculate_delay(self, delay_ms);