From abb6a8b9fa3bbefad17a624ce629e4fc7ccda6d0 Mon Sep 17 00:00:00 2001 From: foamyguy Date: Wed, 5 Aug 2026 10:31:55 -0500 Subject: [PATCH] remove i2sin invert_bit_clock argument, update comment for left_justified change --- ports/espressif/common-hal/audioi2sin/I2SIn.c | 2 +- .../raspberrypi/common-hal/audioi2sin/I2SIn.c | 22 ++++++++----------- shared-bindings/audioi2sin/I2SIn.c | 14 ++---------- shared-bindings/audioi2sin/I2SIn.h | 2 +- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/ports/espressif/common-hal/audioi2sin/I2SIn.c b/ports/espressif/common-hal/audioi2sin/I2SIn.c index 6da790d9736..9a85d11f493 100644 --- a/ports/espressif/common-hal/audioi2sin/I2SIn.c +++ b/ports/espressif/common-hal/audioi2sin/I2SIn.c @@ -25,7 +25,7 @@ void common_hal_audioi2sin_i2sin_construct(audioi2sin_i2sin_obj_t *self, const mcu_pin_obj_t *data, const mcu_pin_obj_t *main_clock, uint32_t sample_rate, uint8_t bit_depth, uint8_t output_bit_depth, bool mono, bool left_justified, bool samples_signed, - bool external_clock, bool invert_bit_clock) { + bool external_clock) { if (external_clock) { mp_raise_NotImplementedError_varg(MP_ERROR_TEXT("%q"), MP_QSTR_external_clock); } diff --git a/ports/raspberrypi/common-hal/audioi2sin/I2SIn.c b/ports/raspberrypi/common-hal/audioi2sin/I2SIn.c index 0032e6fd7be..bb0b0204111 100644 --- a/ports/raspberrypi/common-hal/audioi2sin/I2SIn.c +++ b/ports/raspberrypi/common-hal/audioi2sin/I2SIn.c @@ -212,11 +212,10 @@ static const uint16_t i2sin_program_left_justified_swap_32[] = { // a 32-BCLK frame is one push (right<<16 | left), at 24/32 a 64-BCLK frame is // two pushes (right then left). // -// The resync's own `wait 0/1 gpio B` already lands on the first data bit, so -// against a CircuitPython clock source this program recovers the transmitted word -// bit-exactly with no instruction for the Philips delay bit. The -// `left_justified` variant is that program plus one more BCLK of skew, -// the other of the two possible alignments; +// The WS resync lands on the first data bit, so the program above is +// the `left_justified` alignment: data starts on the WS edge and no instruction +// is spent on a delay bit. The default (Philips) alignment is that program plus +// one more BCLK of skew, the other of the two possible alignments. // // Free-running after the initial sync: the external frame must be exactly // 2 x bits_per_channel BCLKs, the same assumption internal clock mode already bakes @@ -226,12 +225,9 @@ static const uint16_t i2sin_program_left_justified_swap_32[] = { #define I2SIN_EXT_CLOCK_WRAP_TARGET(len) ((int)(len) - 5) static size_t build_i2sin_ext_clock_program(uint16_t *prog, uint8_t bclk, uint8_t ws, - bool left_justified, bool invert_bit_clock) { - // Sampling on the falling edge of BCLK is the same program with the - // polarity of every BCLK wait flipped. - const uint16_t invert = invert_bit_clock ? 0x0080 : 0x0000; - const uint16_t wait_0_bclk = (0x2000 | bclk) ^ invert; - const uint16_t wait_1_bclk = (0x2080 | bclk) ^ invert; + bool left_justified) { + const uint16_t wait_0_bclk = 0x2000 | bclk; + const uint16_t wait_1_bclk = 0x2080 | bclk; size_t len = 0; prog[len++] = 0x2000 | ws; // wait 0 gpio W prog[len++] = 0x2080 | ws; // wait 1 gpio W @@ -262,7 +258,7 @@ void common_hal_audioi2sin_i2sin_construct(audioi2sin_i2sin_obj_t *self, const mcu_pin_obj_t *data, const mcu_pin_obj_t *main_clock, uint32_t sample_rate, uint8_t bit_depth, uint8_t output_bit_depth, bool mono, bool left_justified, bool samples_signed, - bool external_clock, bool invert_bit_clock) { + bool external_clock) { if (main_clock != NULL) { mp_raise_NotImplementedError_varg(MP_ERROR_TEXT("%q"), MP_QSTR_main_clock); @@ -295,7 +291,7 @@ void common_hal_audioi2sin_i2sin_construct(audioi2sin_i2sin_obj_t *self, program_len = build_i2sin_ext_clock_program(ext_clock_program, i2s_wait_gpio_index(bit_clock, gpio_offset), i2s_wait_gpio_index(word_select, gpio_offset), - left_justified, invert_bit_clock); + left_justified); program = ext_clock_program; wait_gpio_mask = PIO_PINMASK_OR(PIO_PINMASK_FROM_PIN(bit_clock->number), PIO_PINMASK_FROM_PIN(word_select->number)); diff --git a/shared-bindings/audioi2sin/I2SIn.c b/shared-bindings/audioi2sin/I2SIn.c index ad369574c68..22f1fb2aaad 100644 --- a/shared-bindings/audioi2sin/I2SIn.c +++ b/shared-bindings/audioi2sin/I2SIn.c @@ -35,7 +35,6 @@ //| left_justified: bool = False, //| samples_signed: bool = True, //| external_clock: bool = False, -//| invert_bit_clock: bool = False, //| ) -> None: //| """Create an I2SIn object associated with the given pins. This allows you to //| record audio signals from an external I2S source (e.g. an I2S MEMS microphone @@ -95,9 +94,6 @@ //| declaration rather than a measurement: the real rate is whatever the external word select //| runs at, and `sample_rate` still reports the declared value. If the incoming clock stops, //| `record` blocks (interruptible with Ctrl-C). -//| :param bool invert_bit_clock: Sample ``data`` on the falling edge of ``bit_clock`` instead of -//| the rising edge. Needed when the external clock source drives its data on the rising edge. -//| Only valid together with ``external_clock``. //| //| Example, recording 16-bit mono samples from an INMP441:: //| @@ -121,7 +117,7 @@ static mp_obj_t audioi2sin_i2sin_make_new(const mp_obj_type_t *type, size_t n_ar enum { ARG_bit_clock, ARG_word_select, ARG_data, ARG_main_clock, ARG_sample_rate, ARG_bit_depth, ARG_output_bit_depth, ARG_mono, ARG_left_justified, ARG_samples_signed, - ARG_external_clock, ARG_invert_bit_clock }; + ARG_external_clock }; static const mp_arg_t allowed_args[] = { { MP_QSTR_bit_clock, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_word_select, MP_ARG_REQUIRED | MP_ARG_OBJ }, @@ -134,17 +130,11 @@ static mp_obj_t audioi2sin_i2sin_make_new(const mp_obj_type_t *type, size_t n_ar { MP_QSTR_left_justified, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, { MP_QSTR_samples_signed, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} }, { MP_QSTR_external_clock, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, - { MP_QSTR_invert_bit_clock, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, }; mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); bool external_clock = args[ARG_external_clock].u_bool; - bool invert_bit_clock = args[ARG_invert_bit_clock].u_bool; - if (invert_bit_clock && !external_clock) { - mp_raise_ValueError_varg(MP_ERROR_TEXT("%q requires %q"), - MP_QSTR_invert_bit_clock, MP_QSTR_external_clock); - } // In external clock mode the clock pins are only read, so they may already // be owned by whatever is driving them; let the port decide if the sharing @@ -181,7 +171,7 @@ static mp_obj_t audioi2sin_i2sin_make_new(const mp_obj_type_t *type, size_t n_ar audioi2sin_i2sin_obj_t *self = mp_obj_malloc_with_finaliser(audioi2sin_i2sin_obj_t, &audioi2sin_i2sin_type); common_hal_audioi2sin_i2sin_construct(self, bit_clock, word_select, data, main_clock, sample_rate, bit_depth, output_bit_depth, mono, left_justified, samples_signed, - external_clock, invert_bit_clock); + external_clock); return MP_OBJ_FROM_PTR(self); #endif diff --git a/shared-bindings/audioi2sin/I2SIn.h b/shared-bindings/audioi2sin/I2SIn.h index 7877653858f..25f5489015d 100644 --- a/shared-bindings/audioi2sin/I2SIn.h +++ b/shared-bindings/audioi2sin/I2SIn.h @@ -21,7 +21,7 @@ void common_hal_audioi2sin_i2sin_construct(audioi2sin_i2sin_obj_t *self, const mcu_pin_obj_t *data, const mcu_pin_obj_t *main_clock, uint32_t sample_rate, uint8_t bit_depth, uint8_t output_bit_depth, bool mono, bool left_justified, bool samples_signed, - bool external_clock, bool invert_bit_clock); + bool external_clock); void common_hal_audioi2sin_i2sin_deinit(audioi2sin_i2sin_obj_t *self); bool common_hal_audioi2sin_i2sin_deinited(audioi2sin_i2sin_obj_t *self); uint32_t common_hal_audioi2sin_i2sin_record_to_buffer(audioi2sin_i2sin_obj_t *self,