From a8614a61dcf8c4b2f323140f629aa11a7c9bb267 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 27 Oct 2021 16:10:20 -0500 Subject: [PATCH] ParallelImageCapture: Add continuous capture on espressif By having a pair of buffers, the capture hardware can fill one buffer while Python code (including displayio, etc) operates on the other buffer. This increases the responsiveness of camera-using code. On the Kaluga it makes the following improvements: * 320x240 viewfinder at 30fps instead of 15fps using directio * 240x240 animated gif capture at 10fps instead of 7.5fps As discussed at length on Discord, the "usual end user" code will look like this: camera = ... with camera.continuous_capture(buffer1, buffer2) as capture: for frame in capture: # Do something with frame However, rather than presenting a context manager, the core code consists of three new functions to start & stop continuous capture, and to get the next frame. The reason is twofold. First, it's simply easier to implement the context manager object in pure Python. Second, for more advanced usage, the context manager may be too limiting, and it's easier to iterate on the right design in Python code. In particular, I noticed that adapting the JPEG-capturing programs to use continuous capture mode needed a change in program structure. The camera app was structured as ```python while True: if shutter button was just pressed: capture a jpeg frame else: update the viewfinder ``` However, "capture a jpeg frame" needs to (A) switch the camera settings and (B) capture into a different, larger buffer then (C) return to the earlier settings. This can't be done during continuous capture mode. So just restructuring it as follows isn't going to work: ```python with camera.continuous_capture(buffer1, buffer2) as capture: for frame in capture: if shutter button was just pressed: capture a jpeg frame, without disturbing continuous capture mode else: update the viewfinder ``` The continuous mode is only implemented in the espressif port; others will throw an exception if the associated methods are invoked. It's not impossible to implement there, just not a priority, since these micros don't have enough RAM for two framebuffer copies at any resonable sizes. The capture code, including single-shot capture, now take mp_obj_t in the common-hal layer, instead of a buffer & length. This was done for the continuous capture mode because it has to identify & return to the user the proper Python object representing the original buffer. In the Espressif port, it was convenient to implement single capture in terms of a multi-capture, which is why I changed the singleshot routine's signature too. --- locale/circuitpython.pot | 17 +++- .../imagecapture/ParallelImageCapture.c | 10 +-- .../imagecapture/ParallelImageCapture.c | 82 +++++++++++++++---- .../imagecapture/ParallelImageCapture.h | 3 + .../imagecapture/ParallelImageCapture.c | 7 +- py/circuitpy_defns.mk | 2 + .../imagecapture/ParallelImageCapture.c | 58 +++++++++++-- .../imagecapture/ParallelImageCapture.h | 5 +- .../imagecapture/ParallelImageCapture.c | 44 ++++++++++ 9 files changed, 197 insertions(+), 31 deletions(-) create mode 100644 shared-module/imagecapture/ParallelImageCapture.c diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index f1f747e7e6..1d7fc6854d 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -347,6 +347,7 @@ msgstr "" msgid "64 bit types" msgstr "" +#: ports/atmel-samd/common-hal/alarm/pin/PinAlarm.c #: ports/atmel-samd/common-hal/countio/Counter.c #: ports/atmel-samd/common-hal/rotaryio/IncrementalEncoder.c msgid "A hardware interrupt channel is already in use" @@ -625,6 +626,10 @@ msgstr "" msgid "Buffer too short by %d bytes" msgstr "" +#: ports/espressif/common-hal/imagecapture/ParallelImageCapture.c +msgid "Buffers must be same size" +msgstr "" + #: ports/atmel-samd/common-hal/paralleldisplay/ParallelBus.c #: ports/espressif/common-hal/paralleldisplay/ParallelBus.c #: ports/nrf/common-hal/paralleldisplay/ParallelBus.c @@ -1587,6 +1592,10 @@ msgstr "" msgid "No available clocks" msgstr "" +#: ports/espressif/common-hal/imagecapture/ParallelImageCapture.c +msgid "No capture in progress" +msgstr "" + #: shared-bindings/_bleio/PacketBuffer.c msgid "No connection: length cannot be determined" msgstr "" @@ -1607,6 +1616,7 @@ msgstr "" msgid "No hardware support on clk pin" msgstr "" +#: ports/atmel-samd/common-hal/alarm/pin/PinAlarm.c #: ports/atmel-samd/common-hal/frequencyio/FrequencyIn.c #: ports/atmel-samd/common-hal/pulseio/PulseIn.c msgid "No hardware support on pin" @@ -1727,7 +1737,6 @@ msgstr "" msgid "Only connectable advertisements can be directed" msgstr "" -#: ports/atmel-samd/common-hal/alarm/pin/PinAlarm.c #: ports/stm/common-hal/alarm/pin/PinAlarm.c msgid "Only edge detection is available on this hardware" msgstr "" @@ -1822,6 +1831,7 @@ msgstr "" msgid "Permission denied" msgstr "" +#: ports/atmel-samd/common-hal/alarm/pin/PinAlarm.c #: ports/stm/common-hal/alarm/pin/PinAlarm.c msgid "Pin cannot wake from Deep Sleep" msgstr "" @@ -2175,6 +2185,10 @@ msgstr "" msgid "The sample's signedness does not match the mixer's" msgstr "" +#: shared-module/imagecapture/ParallelImageCapture.c +msgid "This microcontroller does not support continuous capture." +msgstr "" + #: shared-module/paralleldisplay/ParallelBus.c msgid "" "This microcontroller only supports data0=, not data_pins=, because it " @@ -3880,6 +3894,7 @@ msgstr "" msgid "pow() with 3 arguments requires integers" msgstr "" +#: ports/espressif/boards/adafruit_feather_esp32s2/mpconfigboard.h #: ports/espressif/boards/adafruit_feather_esp32s2_nopsram/mpconfigboard.h #: ports/espressif/boards/adafruit_feather_esp32s2_tftback_nopsram/mpconfigboard.h #: ports/espressif/boards/adafruit_funhouse/mpconfigboard.h diff --git a/ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c b/ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c index acc172fa22..8e41f50d8d 100644 --- a/ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c +++ b/ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c @@ -151,14 +151,14 @@ static void setup_dma(DmacDescriptor *descriptor, size_t count, uint32_t *buffer descriptor->DESCADDR.reg = 0; } -#include - -void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_parallelimagecapture_obj_t *self, void *buffer, size_t bufsize) { +void common_hal_imagecapture_parallelimagecapture_singleshot_capture(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer) { + mp_buffer_info_t bufinfo; + mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_RW); uint8_t dma_channel = dma_allocate_channel(); - uint32_t *dest = buffer; - size_t count = bufsize / 4; // PCC receives 4 bytes (2 pixels) at a time + uint32_t *dest = bufinfo.buf; + size_t count = bufinfo.len / 4; // PCC receives 4 bytes (2 pixels) at a time turn_on_event_system(); diff --git a/ports/espressif/common-hal/imagecapture/ParallelImageCapture.c b/ports/espressif/common-hal/imagecapture/ParallelImageCapture.c index 6d347080bd..b167b5c734 100644 --- a/ports/espressif/common-hal/imagecapture/ParallelImageCapture.c +++ b/ports/espressif/common-hal/imagecapture/ParallelImageCapture.c @@ -79,6 +79,11 @@ void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_paralle } void common_hal_imagecapture_parallelimagecapture_deinit(imagecapture_parallelimagecapture_obj_t *self) { + cam_deinit(); + + self->buffer1 = NULL; + self->buffer2 = NULL; + reset_pin_number(self->data_clock); self->data_clock = NO_PIN; @@ -102,28 +107,73 @@ bool common_hal_imagecapture_parallelimagecapture_deinited(imagecapture_parallel return self->data_clock == NO_PIN; } -void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_parallelimagecapture_obj_t *self, void *buffer, size_t bufsize) { - size_t size = bufsize / 2; // count is in pixels - if (size != self->config.size || buffer != self->config.frame1_buffer) { - cam_deinit(); - self->config.size = bufsize / 2; // count is in pixels(?) - self->config.frame1_buffer = buffer; - - cam_init(&self->config); - cam_start(); - } else { - cam_give(buffer); +void common_hal_imagecapture_parallelimagecapture_continuous_capture_start(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer1, mp_obj_t buffer2) { + if (buffer1 == self->buffer1 && buffer2 == self->buffer2) { + return; } + mp_buffer_info_t bufinfo1, bufinfo2 = {}; + mp_get_buffer_raise(buffer1, &bufinfo1, MP_BUFFER_RW); + if (buffer2 != mp_const_none) { + mp_get_buffer_raise(buffer2, &bufinfo2, MP_BUFFER_RW); + if (bufinfo1.len != bufinfo2.len) { + mp_raise_ValueError(translate("Buffers must be same size")); + } + } + + self->buffer1 = buffer1; + self->buffer2 = buffer2; + + + cam_deinit(); + self->config.size = bufinfo1.len / 2; // count is in pixels + self->config.frame1_buffer = bufinfo1.buf; + self->config.frame2_buffer = bufinfo2.buf; + self->buffer_to_give = NULL; + + cam_init(&self->config); + cam_start(); +} + +void common_hal_imagecapture_parallelimagecapture_continuous_capture_stop(imagecapture_parallelimagecapture_obj_t *self) { + cam_deinit(); + self->buffer1 = self->buffer2 = NULL; + self->buffer_to_give = NULL; +} + +STATIC void common_hal_imagecapture_parallelimagecapture_continuous_capture_give_frame(imagecapture_parallelimagecapture_obj_t *self) { + if (self->buffer_to_give) { + cam_give(self->buffer_to_give); + self->buffer_to_give = NULL; + } +} + +mp_obj_t common_hal_imagecapture_parallelimagecapture_continuous_capture_get_frame(imagecapture_parallelimagecapture_obj_t *self) { + if (self->buffer1 == NULL) { + mp_raise_RuntimeError(translate("No capture in progress")); + } + common_hal_imagecapture_parallelimagecapture_continuous_capture_give_frame(self); + while (!cam_ready()) { RUN_BACKGROUND_TASKS; if (mp_hal_is_interrupted()) { - self->config.size = 0; // force re-init next time - cam_stop(); - return; + return mp_const_none; } } - uint8_t *unused; - cam_take(&unused); // this just "returns" buffer + cam_take(&self->buffer_to_give); + + if (self->buffer_to_give == self->config.frame1_buffer) { + return self->buffer1; + } + if (self->buffer_to_give == self->config.frame2_buffer) { + return self->buffer2; + } + + return mp_const_none; // should be unreachable +} + +void common_hal_imagecapture_parallelimagecapture_singleshot_capture(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer) { + common_hal_imagecapture_parallelimagecapture_continuous_capture_start(self, buffer, mp_const_none); + common_hal_imagecapture_parallelimagecapture_continuous_capture_get_frame(self); } diff --git a/ports/espressif/common-hal/imagecapture/ParallelImageCapture.h b/ports/espressif/common-hal/imagecapture/ParallelImageCapture.h index 4dd65bc904..45f7a2734e 100644 --- a/ports/espressif/common-hal/imagecapture/ParallelImageCapture.h +++ b/ports/espressif/common-hal/imagecapture/ParallelImageCapture.h @@ -26,6 +26,7 @@ #pragma once +#include "py/obj.h" #include "shared-bindings/imagecapture/ParallelImageCapture.h" #include "cam.h" @@ -36,4 +37,6 @@ struct imagecapture_parallelimagecapture_obj { gpio_num_t vertical_sync; gpio_num_t horizontal_reference; uint8_t data_count; + mp_obj_t buffer1, buffer2; + uint8_t *buffer_to_give; }; diff --git a/ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c b/ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c index 1a294f6886..280c87e234 100644 --- a/ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c +++ b/ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c @@ -137,7 +137,10 @@ bool common_hal_imagecapture_parallelimagecapture_deinited(imagecapture_parallel return common_hal_rp2pio_statemachine_deinited(&self->state_machine); } -void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_parallelimagecapture_obj_t *self, void *buffer, size_t bufsize) { +void common_hal_imagecapture_parallelimagecapture_singleshot_capture(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer) { + mp_buffer_info_t bufinfo; + mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_RW); + PIO pio = self->state_machine.pio; uint sm = self->state_machine.state_machine; uint8_t offset = rp2pio_statemachine_program_offset(&self->state_machine); @@ -149,7 +152,7 @@ void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_paralleli pio_sm_exec(pio, sm, pio_encode_jmp(offset)); pio_sm_set_enabled(pio, sm, true); - common_hal_rp2pio_statemachine_readinto(&self->state_machine, buffer, bufsize, 4); + common_hal_rp2pio_statemachine_readinto(&self->state_machine, bufinfo.buf, bufinfo.len, 4); pio_sm_set_enabled(pio, sm, false); } diff --git a/py/circuitpy_defns.mk b/py/circuitpy_defns.mk index 7d2d128a73..bcbe6a9cfb 100644 --- a/py/circuitpy_defns.mk +++ b/py/circuitpy_defns.mk @@ -466,6 +466,7 @@ $(filter $(SRC_PATTERNS), \ digitalio/DriveMode.c \ digitalio/Pull.c \ fontio/Glyph.c \ + imagecapture/ParallelImageCapture.c \ math/__init__.c \ microcontroller/ResetReason.c \ microcontroller/RunMode.c \ @@ -535,6 +536,7 @@ SRC_SHARED_MODULE_ALL = \ gamepadshift/GamePadShift.c \ gamepadshift/__init__.c \ getpass/__init__.c \ + imagecapture/ParallelImageCapture.c \ ipaddress/IPv4Address.c \ ipaddress/__init__.c \ keypad/__init__.c \ diff --git a/shared-bindings/imagecapture/ParallelImageCapture.c b/shared-bindings/imagecapture/ParallelImageCapture.c index e999fba850..071501919f 100644 --- a/shared-bindings/imagecapture/ParallelImageCapture.c +++ b/shared-bindings/imagecapture/ParallelImageCapture.c @@ -25,6 +25,7 @@ */ #include "py/obj.h" +#include "py/objproperty.h" #include "py/runtime.h" #include "shared/runtime/context_manager_helpers.h" @@ -82,19 +83,61 @@ STATIC mp_obj_t imagecapture_parallelimagecapture_make_new(const mp_obj_type_t * return self; } -//| def capture(self, buffer: WriteableBuffer, width: int, height: int, bpp: int=16) -> None: -//| """Capture a single frame into the given buffer""" +//| def capture(self, buffer: WriteableBuffer) -> WriteableBuffer: +//| """Capture a single frame into the given buffer. +//| +//| This will stop a continuous-mode capture, if one is in progress.""" //| ... //| STATIC mp_obj_t imagecapture_parallelimagecapture_capture(mp_obj_t self_in, mp_obj_t buffer) { imagecapture_parallelimagecapture_obj_t *self = (imagecapture_parallelimagecapture_obj_t *)self_in; - mp_buffer_info_t bufinfo; - mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_RW); - common_hal_imagecapture_parallelimagecapture_capture(self, bufinfo.buf, bufinfo.len); + common_hal_imagecapture_parallelimagecapture_singleshot_capture(self, buffer); + + return buffer; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_2(imagecapture_parallelimagecapture_capture_obj, imagecapture_parallelimagecapture_capture); + +//| def continuous_capture_start(self, buffer1: WriteableBuffer, buffer2: WriteableBuffer) -> None: +//| """Begin capturing into the given buffers in the background. +//| +//| Call `continuous_capture_get_frame` to get the next available +//| frame, and `continuous_capture_stop` to stop capturing.""" +//| ... +//| +STATIC mp_obj_t imagecapture_parallelimagecapture_continuous_capture_start(mp_obj_t self_in, mp_obj_t buffer1, mp_obj_t buffer2) { + imagecapture_parallelimagecapture_obj_t *self = (imagecapture_parallelimagecapture_obj_t *)self_in; + common_hal_imagecapture_parallelimagecapture_continuous_capture_start(self, buffer1, buffer2); return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_2(imagecapture_parallelimagecapture_capture_obj, imagecapture_parallelimagecapture_capture); +STATIC MP_DEFINE_CONST_FUN_OBJ_3(imagecapture_parallelimagecapture_continuous_capture_start_obj, imagecapture_parallelimagecapture_continuous_capture_start); + +//| def continuous_capture_get_frame(self) -> WritableBuffer: +//| """Return the next available frame, one of the two buffers passed to `continuous_capture_start`""" +//| ... +//| +STATIC mp_obj_t imagecapture_parallelimagecapture_continuous_capture_get_frame(mp_obj_t self_in) { + imagecapture_parallelimagecapture_obj_t *self = (imagecapture_parallelimagecapture_obj_t *)self_in; + return common_hal_imagecapture_parallelimagecapture_continuous_capture_get_frame(self); +} +STATIC MP_DEFINE_CONST_FUN_OBJ_1(imagecapture_parallelimagecapture_continuous_capture_get_frame_obj, imagecapture_parallelimagecapture_continuous_capture_get_frame); + + + +//| def continuous_capture_stop(self) -> None: +//| """Stop continuous capture""" +//| ... +//| +STATIC mp_obj_t imagecapture_parallelimagecapture_continuous_capture_stop(mp_obj_t self_in) { + imagecapture_parallelimagecapture_obj_t *self = (imagecapture_parallelimagecapture_obj_t *)self_in; + common_hal_imagecapture_parallelimagecapture_continuous_capture_stop(self); + + return mp_const_none; +} +STATIC MP_DEFINE_CONST_FUN_OBJ_1(imagecapture_parallelimagecapture_continuous_capture_stop_obj, imagecapture_parallelimagecapture_continuous_capture_stop); + + + //| def deinit(self) -> None: //| """Deinitialize this instance""" @@ -134,6 +177,9 @@ STATIC const mp_rom_map_elem_t imagecapture_parallelimagecapture_locals_dict_tab { MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&imagecapture_parallelimagecapture___exit___obj) }, { MP_ROM_QSTR(MP_QSTR_capture), MP_ROM_PTR(&imagecapture_parallelimagecapture_capture_obj) }, + { MP_ROM_QSTR(MP_QSTR_continuous_capture_start), MP_ROM_PTR(&imagecapture_parallelimagecapture_continuous_capture_start_obj) }, + { MP_ROM_QSTR(MP_QSTR_continuous_capture_stop), MP_ROM_PTR(&imagecapture_parallelimagecapture_continuous_capture_stop_obj) }, + { MP_ROM_QSTR(MP_QSTR_continuous_capture_get_frame), MP_ROM_PTR(&imagecapture_parallelimagecapture_continuous_capture_get_frame_obj) }, }; STATIC MP_DEFINE_CONST_DICT(imagecapture_parallelimagecapture_locals_dict, imagecapture_parallelimagecapture_locals_dict_table); diff --git a/shared-bindings/imagecapture/ParallelImageCapture.h b/shared-bindings/imagecapture/ParallelImageCapture.h index bcd827aa23..72ddc23f65 100644 --- a/shared-bindings/imagecapture/ParallelImageCapture.h +++ b/shared-bindings/imagecapture/ParallelImageCapture.h @@ -40,4 +40,7 @@ void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_paralle const mcu_pin_obj_t *horizontal_sync); void common_hal_imagecapture_parallelimagecapture_deinit(imagecapture_parallelimagecapture_obj_t *self); bool common_hal_imagecapture_parallelimagecapture_deinited(imagecapture_parallelimagecapture_obj_t *self); -void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_parallelimagecapture_obj_t *self, void *buffer, size_t bufsize); +void common_hal_imagecapture_parallelimagecapture_singleshot_capture(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer); +void common_hal_imagecapture_parallelimagecapture_continuous_capture_start(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer1, mp_obj_t buffer2); +void common_hal_imagecapture_parallelimagecapture_continuous_capture_stop(imagecapture_parallelimagecapture_obj_t *self); +mp_obj_t common_hal_imagecapture_parallelimagecapture_continuous_capture_get_frame(imagecapture_parallelimagecapture_obj_t *self); diff --git a/shared-module/imagecapture/ParallelImageCapture.c b/shared-module/imagecapture/ParallelImageCapture.c new file mode 100644 index 0000000000..346bc76479 --- /dev/null +++ b/shared-module/imagecapture/ParallelImageCapture.c @@ -0,0 +1,44 @@ +/* + * This file is part of the Micro Python project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2019 Scott Shawcroft for Adafruit Industries + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "shared-bindings/imagecapture/ParallelImageCapture.h" +#include "py/runtime.h" + +// If the continuous-capture mode isn't supported, then this default (weak) implementation will raise exceptions for you +__attribute__((weak)) +void common_hal_imagecapture_parallelimagecapture_continuous_capture_start(imagecapture_parallelimagecapture_obj_t *self, mp_obj_t buffer1, mp_obj_t buffer2) { + mp_raise_NotImplementedError(translate("This microcontroller does not support continuous capture.")); +} + +__attribute__((weak)) +void common_hal_imagecapture_parallelimagecapture_continuous_capture_stop(imagecapture_parallelimagecapture_obj_t *self) { + mp_raise_NotImplementedError(translate("This microcontroller does not support continuous capture.")); +} + +__attribute__((weak)) +mp_obj_t common_hal_imagecapture_parallelimagecapture_continuous_capture_get_frame(imagecapture_parallelimagecapture_obj_t *self) { + mp_raise_NotImplementedError(translate("This microcontroller does not support continuous capture.")); +}