Skip to content

Add support for tuple of Biquad objects within audiofilters.Filter. #9772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 10, 2024
Merged

Add support for tuple of Biquad objects within audiofilters.Filter. #9772

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6badb92
Add support for list of Biquad objects within `audiofilters.Filter`.
relic-se Oct 29, 2024
14b1383
Remove trailing whitespace to fix pre-commit.
relic-se Nov 5, 2024
941e122
Rename `filters` back to `filter` and support individual Biquad objec…
relic-se Nov 7, 2024
5f8ec0a
Validate type of filter tuple items.
relic-se Nov 7, 2024
c96d142
Remove unnecessary `m_malloc_fail` and deinit from memory allocation …
relic-se Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 26 additions & 22 deletions shared-bindings/audiofilters/Filter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
//|
//| def __init__(
//| self,
//| filter: Optional[synthio.Biquad] = None,
//| filters: Optional[List[synthio.Biquad]] = None,
//| mix: synthio.BlockInput = 1.0,
//| buffer_size: int = 512,
//| sample_rate: int = 8000,
Expand All @@ -38,7 +38,7 @@
//| The mix parameter allows you to change how much of the unchanged sample passes through to
//| the output to how much of the effect audio you hear as the output.
//|
//| :param Optional[synthio.Biquad] filter: The normalized biquad filter object used to process the signal.
//| :param Optional[List[synthio.Biquad]] filters: A list of normalized biquad filter objects used to process the signal.
//| :param synthio.BlockInput mix: The mix as a ratio of the sample (0.0) to the effect (1.0).
//| :param int buffer_size: The total size in bytes of each of the two playback buffers to use
//| :param int sample_rate: The sample rate to be used
Expand All @@ -56,9 +56,10 @@
//|
//| audio = audiobusio.I2SOut(bit_clock=board.GP20, word_select=board.GP21, data=board.GP22)
//| synth = synthio.Synthesizer(channel_count=1, sample_rate=44100)
//| filter = audiofilters.Filter(filter=synth.low_pass_filter(frequency=2000, Q=1.25), buffer_size=1024, channel_count=1, sample_rate=44100, mix=1.0)
//| filter.play(synth)
//| audio.play(filter)
//| effect = audiofilters.Filter(buffer_size=1024, channel_count=1, sample_rate=44100, mix=1.0)
//| effect.filters.append(synth.low_pass_filter(frequency=2000, Q=1.25))
//| effect.play(synth)
//| audio.play(effect)
//|
//| note = synthio.Note(261)
//| while True:
Expand All @@ -68,9 +69,9 @@
//| time.sleep(5)"""
//| ...
static mp_obj_t audiofilters_filter_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
enum { ARG_filter, ARG_mix, ARG_buffer_size, ARG_sample_rate, ARG_bits_per_sample, ARG_samples_signed, ARG_channel_count, };
enum { ARG_filters, ARG_mix, ARG_buffer_size, ARG_sample_rate, ARG_bits_per_sample, ARG_samples_signed, ARG_channel_count, };
static const mp_arg_t allowed_args[] = {
{ MP_QSTR_filter, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NULL} },
{ MP_QSTR_filters, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NULL} },
{ MP_QSTR_mix, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NULL} },
{ MP_QSTR_buffer_size, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 512} },
{ MP_QSTR_sample_rate, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 8000} },
Expand All @@ -90,7 +91,7 @@ static mp_obj_t audiofilters_filter_make_new(const mp_obj_type_t *type, size_t n
}

audiofilters_filter_obj_t *self = mp_obj_malloc(audiofilters_filter_obj_t, &audiofilters_filter_type);
common_hal_audiofilters_filter_construct(self, args[ARG_filter].u_obj, args[ARG_mix].u_obj, args[ARG_buffer_size].u_int, bits_per_sample, args[ARG_samples_signed].u_bool, channel_count, sample_rate);
common_hal_audiofilters_filter_construct(self, args[ARG_filters].u_obj, args[ARG_mix].u_obj, args[ARG_buffer_size].u_int, bits_per_sample, args[ARG_samples_signed].u_bool, channel_count, sample_rate);

return MP_OBJ_FROM_PTR(self);
}
Expand Down Expand Up @@ -128,31 +129,34 @@ static mp_obj_t audiofilters_filter_obj___exit__(size_t n_args, const mp_obj_t *
static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(audiofilters_filter___exit___obj, 4, 4, audiofilters_filter_obj___exit__);


//| filter: Optional[synthio.Biquad]
//| """The normalized biquad filter object used to process the signal."""
static mp_obj_t audiofilters_filter_obj_get_filter(mp_obj_t self_in) {
return common_hal_audiofilters_filter_get_filter(self_in);
//| filters: List[synthio.Biquad]
//| """A list of normalized biquad filter objects used to process the signal."""
//|
static mp_obj_t audiofilters_filter_obj_get_filters(mp_obj_t self_in) {
audiofilters_filter_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);
return common_hal_audiofilters_filter_get_filters(self);
}
MP_DEFINE_CONST_FUN_OBJ_1(audiofilters_filter_get_filter_obj, audiofilters_filter_obj_get_filter);
MP_DEFINE_CONST_FUN_OBJ_1(audiofilters_filter_get_filters_obj, audiofilters_filter_obj_get_filters);

static mp_obj_t audiofilters_filter_obj_set_filter(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
enum { ARG_filter };
static mp_obj_t audiofilters_filter_obj_set_filters(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
enum { ARG_filters };
static const mp_arg_t allowed_args[] = {
{ MP_QSTR_filter, MP_ARG_OBJ | MP_ARG_REQUIRED, {} },
{ MP_QSTR_filters, MP_ARG_OBJ | MP_ARG_REQUIRED, {} },
};
audiofilters_filter_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

common_hal_audiofilters_filter_set_filter(self, args[ARG_filter].u_obj);
common_hal_audiofilters_filter_set_filters(self, args[ARG_filters].u_obj);

return mp_const_none;
}
MP_DEFINE_CONST_FUN_OBJ_KW(audiofilters_filter_set_filter_obj, 1, audiofilters_filter_obj_set_filter);
MP_DEFINE_CONST_FUN_OBJ_KW(audiofilters_filter_set_filters_obj, 1, audiofilters_filter_obj_set_filters);

MP_PROPERTY_GETSET(audiofilters_filter_filter_obj,
(mp_obj_t)&audiofilters_filter_get_filter_obj,
(mp_obj_t)&audiofilters_filter_set_filter_obj);
MP_PROPERTY_GETSET(audiofilters_filter_filters_obj,
(mp_obj_t)&audiofilters_filter_get_filters_obj,
(mp_obj_t)&audiofilters_filter_set_filters_obj);


//| mix: synthio.BlockInput
Expand Down Expand Up @@ -241,7 +245,7 @@ static const mp_rom_map_elem_t audiofilters_filter_locals_dict_table[] = {

// Properties
{ MP_ROM_QSTR(MP_QSTR_playing), MP_ROM_PTR(&audiofilters_filter_playing_obj) },
{ MP_ROM_QSTR(MP_QSTR_filter), MP_ROM_PTR(&audiofilters_filter_filter_obj) },
{ MP_ROM_QSTR(MP_QSTR_filters), MP_ROM_PTR(&audiofilters_filter_filters_obj) },
{ MP_ROM_QSTR(MP_QSTR_mix), MP_ROM_PTR(&audiofilters_filter_mix_obj) },
};
static MP_DEFINE_CONST_DICT(audiofilters_filter_locals_dict, audiofilters_filter_locals_dict_table);
Expand Down
6 changes: 3 additions & 3 deletions shared-bindings/audiofilters/Filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
extern const mp_obj_type_t audiofilters_filter_type;

void common_hal_audiofilters_filter_construct(audiofilters_filter_obj_t *self,
mp_obj_t filter, mp_obj_t mix,
mp_obj_t filters, mp_obj_t mix,
uint32_t buffer_size, uint8_t bits_per_sample, bool samples_signed,
uint8_t channel_count, uint32_t sample_rate);

Expand All @@ -22,8 +22,8 @@ uint32_t common_hal_audiofilters_filter_get_sample_rate(audiofilters_filter_obj_
uint8_t common_hal_audiofilters_filter_get_channel_count(audiofilters_filter_obj_t *self);
uint8_t common_hal_audiofilters_filter_get_bits_per_sample(audiofilters_filter_obj_t *self);

mp_obj_t common_hal_audiofilters_filter_get_filter(audiofilters_filter_obj_t *self);
void common_hal_audiofilters_filter_set_filter(audiofilters_filter_obj_t *self, mp_obj_t arg);
mp_obj_t common_hal_audiofilters_filter_get_filters(audiofilters_filter_obj_t *self);
void common_hal_audiofilters_filter_set_filters(audiofilters_filter_obj_t *self, mp_obj_t arg);

mp_obj_t common_hal_audiofilters_filter_get_mix(audiofilters_filter_obj_t *self);
void common_hal_audiofilters_filter_set_mix(audiofilters_filter_obj_t *self, mp_obj_t arg);
Expand Down
59 changes: 45 additions & 14 deletions shared-module/audiofilters/Filter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "py/runtime.h"

void common_hal_audiofilters_filter_construct(audiofilters_filter_obj_t *self,
mp_obj_t filter, mp_obj_t mix,
mp_obj_t filters, mp_obj_t mix,
uint32_t buffer_size, uint8_t bits_per_sample,
bool samples_signed, uint8_t channel_count, uint32_t sample_rate) {

Expand Down Expand Up @@ -60,11 +60,11 @@ void common_hal_audiofilters_filter_construct(audiofilters_filter_obj_t *self,

// The below section sets up the effect's starting values.

if (filter == MP_OBJ_NULL) {
filter = mp_const_none;
if (filters == MP_OBJ_NULL) {
filters = mp_obj_new_list(0, NULL);
}
synthio_biquad_filter_assign(&self->filter_state, filter);
self->filter_obj = filter;
self->filters = filters;
reset_filter_states(self);

// If we did not receive a BlockInput we need to create a default float value
if (mix == MP_OBJ_NULL) {
Expand All @@ -87,15 +87,37 @@ void common_hal_audiofilters_filter_deinit(audiofilters_filter_obj_t *self) {
self->buffer[0] = NULL;
self->buffer[1] = NULL;
self->filter_buffer = NULL;
self->filter_states = NULL;
}

mp_obj_t common_hal_audiofilters_filter_get_filter(audiofilters_filter_obj_t *self) {
return self->filter_obj;
void reset_filter_states(audiofilters_filter_obj_t *self) {
self->filter_states_len = self->filters->len;
self->filter_states = NULL;

if (self->filter_states_len) {
self->filter_states = m_malloc(self->filter_states_len * sizeof(biquad_filter_state));
if (self->filter_states == NULL) {
common_hal_audiofilters_filter_deinit(self);
m_malloc_fail(self->filter_states_len * sizeof(biquad_filter_state));
Copy link

@jepler jepler Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm relatively sure that m_malloc already throws in case of failed allocation, so the call to m_malloc_fail is not needed; but also, the call to common_hal_audiofilters_filter_deinit is ineffective. I see this idiom has been used across the audiofilters so it may need to be revised elsewhere as well.

Copy link
Author

@relic-se relic-se Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this issue and apply changes as necessary throughout. I'm still learning about how best to program within the MicroPython/CircuitPython ecosystem so bear with me! :)

Copy link
Author

@relic-se relic-se Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I began removing the ..._deinit and m_malloc_fail statements, but I'm finding that this idiom is fairly common within multiple modules of CircuitPython. As you certain that this isn't the proper handling of m_malloc?

Here are a few examples:

circuitpython/shared-module/audiomixer/Mixer.c

Lines 28 to 38 in ddfa519

self->first_buffer = m_malloc(self->len);
if (self->first_buffer == NULL) {
common_hal_audiomixer_mixer_deinit(self);
m_malloc_fail(self->len);
}
self->second_buffer = m_malloc(self->len);
if (self->second_buffer == NULL) {
common_hal_audiomixer_mixer_deinit(self);
m_malloc_fail(self->len);
}

circuitpython/shared-module/audiomp3/MP3Decoder.c

Lines 306 to 327 in ddfa519

self->inbuf.buf = m_malloc(DEFAULT_INPUT_BUFFER_SIZE);
if (self->inbuf.buf == NULL) {
common_hal_audiomp3_mp3file_deinit(self);
m_malloc_fail(DEFAULT_INPUT_BUFFER_SIZE);
}
if (buffer_size >= 2 * MAX_BUFFER_LEN) {
self->pcm_buffer[0] = (int16_t *)(void *)buffer;
self->pcm_buffer[1] = (int16_t *)(void *)(buffer + MAX_BUFFER_LEN);
} else {
self->pcm_buffer[0] = m_malloc(MAX_BUFFER_LEN);
if (self->pcm_buffer[0] == NULL) {
common_hal_audiomp3_mp3file_deinit(self);
m_malloc_fail(MAX_BUFFER_LEN);
}
self->pcm_buffer[1] = m_malloc(MAX_BUFFER_LEN);
if (self->pcm_buffer[1] == NULL) {
common_hal_audiomp3_mp3file_deinit(self);
m_malloc_fail(MAX_BUFFER_LEN);
}
}

circuitpython/shared-module/audiocore/WaveFile.c

Lines 100 to 110 in ddfa519

self->buffer = m_malloc(self->len);
if (self->buffer == NULL) {
common_hal_audioio_wavefile_deinit(self);
m_malloc_fail(self->len);
}
self->second_buffer = m_malloc(self->len);
if (self->second_buffer == NULL) {
common_hal_audioio_wavefile_deinit(self);
m_malloc_fail(self->len);
}

circuitpython/ports/stm/common-hal/pulseio/PulseIn.c

Lines 87 to 91 in ddfa519

self->buffer = (uint16_t *)m_malloc(maxlen * sizeof(uint16_t));
if (self->buffer == NULL) {
// TODO: free the EXTI here?
m_malloc_fail(maxlen * sizeof(uint16_t));
}
(also in other ports with pulseio)

There are other cases where instead of m_malloc(...) there are calls to port_malloc(...), malloc(...), realloc(...), malloc_with_finaliser(...), ringbuf_alloc(...), and gc_alloc(...) before m_malloc_fail(...). Maybe someone down the line copied that over but with m_malloc(...) and it stuck within audio and pulseio modules?

Rather than experimenting and going outside of the accepted formatting, I'm copying a lot of my homework from other portions of CircuitPython. This is something I borrowed from audiodelays without fully understanding the underlying principles. I plan on keeping it as is for now until we develop a plan for all other instances of this methodology (likely another PR).

Copy link
Author

@relic-se relic-se Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally looked this up, and I see what you mean. The additional check and m_malloc_fail is definitely not necessary. I'll remove it from this class, but let me know if you'd like me to go around and remove it throughout CircuitPython or save it for a separate PR. @jepler

circuitpython/py/malloc.c

Lines 86 to 89 in d7a7221

void *ptr = malloc(num_bytes);
if (ptr == NULL && num_bytes != 0) {
m_malloc_fail(num_bytes);
}

}

mp_obj_iter_buf_t iter_buf;
mp_obj_t iterable = mp_getiter(self->filters, &iter_buf);
mp_obj_t item;
uint8_t i = 0;
while ((item = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
synthio_biquad_filter_assign(&self->filter_states[i++], item);
}
}
}

void common_hal_audiofilters_filter_set_filter(audiofilters_filter_obj_t *self, mp_obj_t arg) {
synthio_biquad_filter_assign(&self->filter_state, arg);
self->filter_obj = arg;
mp_obj_t common_hal_audiofilters_filter_get_filters(audiofilters_filter_obj_t *self) {
return self->filters;
}

void common_hal_audiofilters_filter_set_filters(audiofilters_filter_obj_t *self, mp_obj_t arg) {
self->filters = arg;
reset_filter_states(self);
}

mp_obj_t common_hal_audiofilters_filter_get_mix(audiofilters_filter_obj_t *self) {
Expand Down Expand Up @@ -126,7 +148,9 @@ void audiofilters_filter_reset_buffer(audiofilters_filter_obj_t *self,
memset(self->buffer[1], 0, self->buffer_len);
memset(self->filter_buffer, 0, SYNTHIO_MAX_DUR * sizeof(int32_t));

synthio_biquad_filter_reset(&self->filter_state);
for (uint8_t i = 0; i < self->filter_states_len; i++) {
synthio_biquad_filter_reset(&self->filter_states[i]);
}
}

bool common_hal_audiofilters_filter_get_playing(audiofilters_filter_obj_t *self) {
Expand Down Expand Up @@ -209,6 +233,11 @@ audioio_get_buffer_result_t audiofilters_filter_get_buffer(audiofilters_filter_o
channel = 0;
}

// Update filter_states if filters list has been appended or removed
if (self->filters->len != self->filter_states_len) {
reset_filter_states(self);
}

// get the effect values we need from the BlockInput. These may change at run time so you need to do bounds checking if required
mp_float_t mix = MIN(1.0, MAX(synthio_block_slot_get(&self->mix), 0.0));

Expand Down Expand Up @@ -248,7 +277,7 @@ audioio_get_buffer_result_t audiofilters_filter_get_buffer(audiofilters_filter_o
int16_t *sample_src = (int16_t *)self->sample_remaining_buffer; // for 16-bit samples
int8_t *sample_hsrc = (int8_t *)self->sample_remaining_buffer; // for 8-bit samples

if (mix <= 0.01 || self->filter_obj == mp_const_none) { // if mix is zero pure sample only or no biquad filter object is provided
if (mix <= 0.01 || !self->filter_states_len) { // if mix is zero pure sample only or no biquad filter objects are provided
for (uint32_t i = 0; i < n; i++) {
if (MP_LIKELY(self->bits_per_sample == 16)) {
word_buffer[i] = sample_src[i];
Expand All @@ -275,8 +304,10 @@ audioio_get_buffer_result_t audiofilters_filter_get_buffer(audiofilters_filter_o
}
}

// Process biquad filter
synthio_biquad_filter_samples(&self->filter_state, self->filter_buffer, n_samples);
// Process biquad filters
for (uint8_t j = 0; j < self->filter_states_len; j++) {
synthio_biquad_filter_samples(&self->filter_states[j], self->filter_buffer, n_samples);
}

// Mix processed signal with original sample and transfer to output buffer
for (uint32_t j = 0; j < n_samples; j++) {
Expand Down
7 changes: 5 additions & 2 deletions shared-module/audiofilters/Filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ extern const mp_obj_type_t audiofilters_filter_type;

typedef struct {
mp_obj_base_t base;
mp_obj_t filter_obj;
mp_obj_list_t *filters;
synthio_block_slot_t mix;

biquad_filter_state filter_state;
uint8_t filter_states_len;
biquad_filter_state *filter_states;

uint8_t bits_per_sample;
bool samples_signed;
Expand All @@ -40,6 +41,8 @@ typedef struct {
mp_obj_t sample;
} audiofilters_filter_obj_t;

void reset_filter_states(audiofilters_filter_obj_t *self);

void audiofilters_filter_reset_buffer(audiofilters_filter_obj_t *self,
bool single_channel_output,
uint8_t channel);
Expand Down