From 4b065a038f0ab9f6268bcc8b3441b6caa5cc8908 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sun, 18 Oct 2026 18:10:36 -0700 Subject: [PATCH 01/12] jpegio: decode JPEGs with the JPEG engine on ESP32-P4 and ESP32-S31 CIRCUITPY_JPEGIO_HW (default 0) means the port provides jpegio_jpegdecoder_decode_hw(). decode() calls it first when open() was given an in-memory buffer. If it returns false, TJpgDec decodes the image as before. The espressif port turns it on for esp32p4 and esp32s31, the targets with a JPEG decoder peripheral, and implements it with esp_driver_jpeg. Other targets are unchanged. CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT (default 4) sets the alignment of the pixel memory a Bitmap allocates. The two chips raise it to the 64 byte cache line, so a decode of the whole image at 0,0 into a Bitmap of the same size goes straight into the Bitmap. The pixels stay in the GC heap. Other decodes go through a temporary buffer that is freed before decode() returns. Skip indexes, and input the DMA can't read, use TJpgDec. The engine is created on the first decode and deleted on VM reset. The color conversion is full range BT601, as JFIF specifies. This needs yuv_range in jpeg_decode_cfg_t, added in adafruit/esp-idf. 1280x720, median of 10, hardware from bytes vs TJpgDec from io.BytesIO: ESP32-P4 (Adafruit P4 GPIO) 4:2:0 9.2 ms (302.1), 4:2:2 12.2 (327.5), 4:4:4 18.2 (385.2) ESP32-S31 (ESP32-S31-Korvo-1) 4:2:0 16.0 ms (374.8), 4:2:2 20.9 (409.4), 4:4:4 31.0 (477.4) Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/CMakeLists.txt | 2 +- ports/espressif/Makefile | 8 + .../espressif/common-hal/jpegio/JpegDecoder.c | 196 ++++++++++++++++++ .../espressif/common-hal/jpegio/JpegDecoder.h | 10 + ports/espressif/mpconfigport.h | 5 + ports/espressif/mpconfigport.mk | 6 + ports/espressif/supervisor/port.c | 8 + py/circuitpy_mpconfig.h | 6 + py/circuitpy_mpconfig.mk | 4 + shared-bindings/jpegio/JpegDecoder.c | 8 + shared-module/displayio/Bitmap.c | 18 +- shared-module/displayio/Bitmap.h | 4 + shared-module/jpegio/JpegDecoder.c | 11 + shared-module/jpegio/JpegDecoder.h | 10 + .../circuitpython-manual/jpegio/hw_decode.py | 74 +++++++ 15 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 ports/espressif/common-hal/jpegio/JpegDecoder.c create mode 100644 ports/espressif/common-hal/jpegio/JpegDecoder.h create mode 100644 tests/circuitpython-manual/jpegio/hw_decode.py diff --git a/ports/espressif/CMakeLists.txt b/ports/espressif/CMakeLists.txt index 608d4f4c7b3..563f953e7d8 100644 --- a/ports/espressif/CMakeLists.txt +++ b/ports/espressif/CMakeLists.txt @@ -5,7 +5,7 @@ cmake_minimum_required(VERSION 3.16) set(ENV{IDF_PATH} ${CMAKE_SOURCE_DIR}/esp-idf) # The component list here determines what options we get in menuconfig and what the ninja file can build. -set(COMPONENTS bt driver esp_driver_dac esp_driver_gpio esp_driver_gptimer esp_driver_i2c esp_driver_i2s esp_driver_ledc esp_driver_pcnt esp_driver_rmt esp_driver_sdmmc esp_driver_spi esp_driver_touch_sens esp_driver_tsens esp_driver_uart esp_eth esp-tls esp_adc esp_adc_cal esp_event esp_netif esp_psram esp_security esp_wifi esptool_py freertos log lwip main mbedtls mdns pthread soc ulp wpa_supplicant esp-camera esp_lcd vfs esp_vfs_console esp_stdio sdmmc usb) +set(COMPONENTS bt driver esp_driver_dac esp_driver_gpio esp_driver_gptimer esp_driver_i2c esp_driver_i2s esp_driver_jpeg esp_driver_ledc esp_driver_pcnt esp_driver_rmt esp_driver_sdmmc esp_driver_spi esp_driver_touch_sens esp_driver_tsens esp_driver_uart esp_eth esp-tls esp_adc esp_adc_cal esp_event esp_netif esp_psram esp_security esp_wifi esptool_py freertos log lwip main mbedtls mdns pthread soc ulp wpa_supplicant esp-camera esp_lcd vfs esp_vfs_console esp_stdio sdmmc usb) set(EXTRA_COMPONENT_DIRS "esp-protocols/components/mdns" "esp-camera") include($ENV{IDF_PATH}/tools/cmake/project.cmake) diff --git a/ports/espressif/Makefile b/ports/espressif/Makefile index 1d713916168..224edb091b2 100644 --- a/ports/espressif/Makefile +++ b/ports/espressif/Makefile @@ -65,6 +65,7 @@ INC += \ -isystem esp-idf/components/esp_driver_gptimer/include \ -isystem esp-idf/components/esp_driver_i2c/include \ -isystem esp-idf/components/esp_driver_i2s/include \ + -isystem esp-idf/components/esp_driver_jpeg/include \ -isystem esp-idf/components/esp_driver_ledc/include \ -isystem esp-idf/components/esp_driver_parlio/include \ -isystem esp-idf/components/esp_driver_pcnt/include \ @@ -119,6 +120,8 @@ INC += \ -isystem esp-idf/components/esp_hal_i2c/include \ -isystem esp-idf/components/esp_hal_i2s/include \ -isystem esp-idf/components/esp_hal_i2s/$(IDF_TARGET)/include \ + -isystem esp-idf/components/esp_hal_jpeg/include \ + -isystem esp-idf/components/esp_hal_jpeg/$(IDF_TARGET)/include \ -isystem esp-idf/components/esp_hal_ledc/include \ -isystem esp-idf/components/esp_hal_pcnt/include \ -isystem esp-idf/components/esp_hal_rmt/include \ @@ -815,6 +818,11 @@ SRC_ULP := \ SRC_C += $(SRC_ULP) endif +ifneq ($(CIRCUITPY_JPEGIO_HW),0) +SRC_C += common-hal/jpegio/JpegDecoder.c +CHIP_COMPONENTS += esp_driver_jpeg esp_hal_jpeg +endif + ifneq ($(CIRCUITPY_NEOPIXEL_WRITE),0) CHIP_COMPONENTS += esp_driver_rmt endif diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c new file mode 100644 index 00000000000..1feae565d75 --- /dev/null +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -0,0 +1,196 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Mikey Sklar +// +// SPDX-License-Identifier: MIT + +// Hardware JPEG decode for jpegio, on chips with SOC_JPEG_DECODE_SUPPORTED. +// The engine decodes a whole baseline JPEG to RGB565 with no scaling, so only +// decode() calls with scale=0 into a 16-bit Bitmap take this path. Anything the +// hardware does not handle returns false and jpegio falls back to TJpgDec. + +#include "py/runtime.h" + +#include "common-hal/jpegio/JpegDecoder.h" +#include "shared-bindings/bitmaptools/__init__.h" +#include "shared-module/displayio/Bitmap.h" +#include "shared-module/jpegio/JpegDecoder.h" + +#include "driver/jpeg_decode.h" +#include "esp_cache.h" +#include "esp_heap_caps.h" +#include "esp_memory_utils.h" + +// A bad image blocks the engine for at most this long. +#define JPEG_DECODE_TIMEOUT_MS (1000) + +static jpeg_decoder_handle_t decoder_engine; + +void espressif_jpegio_reset(void) { + if (decoder_engine != NULL) { + jpeg_del_decoder_engine(decoder_engine); + decoder_engine = NULL; + } +} + +// Bitmap pixels must start on a cache line for the engine to write them directly. +#ifdef CONFIG_CACHE_L2_CACHE_LINE_SIZE +_Static_assert(CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT % CONFIG_CACHE_L2_CACHE_LINE_SIZE == 0, "Bitmap alignment is not a cache line"); +#endif +#ifdef CONFIG_CACHE_L1_DCACHE_LINE_SIZE +_Static_assert(CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT % CONFIG_CACHE_L1_DCACHE_LINE_SIZE == 0, "Bitmap alignment is not a cache line"); +#endif + +// True if the hardware may write `size` bytes at `buf` directly: the start and +// length must both be whole cache lines. +static bool output_aligned(const void *buf, size_t size) { + size_t align = esp_cache_get_line_size_by_addr(buf); + if (align < 4) { + align = 4; + } + return ((uintptr_t)buf & (align - 1)) == 0 && (size & (align - 1)) == 0; +} + +bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale) { + displayio_bitmap_t *bitmap = self->dest; + if (scale != 0 || bitmap->bits_per_value != 16 || bitmap->read_only) { + return false; + } + // Skip indices are slower on this path than in TJpgDec. + if (!self->skip_source_index_none || !self->skip_dest_index_none) { + return false; + } + // The engine reads the input by DMA, so it must be in DMA-capable memory. + if (!esp_ptr_dma_capable(jpeg->buf) && !esp_ptr_dma_ext_capable(jpeg->buf)) { + return false; + } + + jpeg_decode_picture_info_t info; + if (jpeg_decoder_get_info(jpeg->buf, jpeg->len, &info) != ESP_OK) { + return false; + } + + // The engine pads its output to whole MCUs: 8x8 for 4:4:4, 16x8 for 4:2:2 + // and 16x16 for 4:2:0. Grayscale is left to TJpgDec. + uint32_t mcu_width, mcu_height; + switch (info.sample_method) { + case JPEG_DOWN_SAMPLING_YUV444: + mcu_width = 8; + mcu_height = 8; + break; + case JPEG_DOWN_SAMPLING_YUV422: + mcu_width = 16; + mcu_height = 8; + break; + case JPEG_DOWN_SAMPLING_YUV420: + mcu_width = 16; + mcu_height = 16; + break; + default: + return false; + } + uint32_t width = info.width; + uint32_t height = info.height; + // The driver rejects images whose pixel count is not a multiple of 8. + if ((width * height) % 8 != 0) { + return false; + } + uint32_t output_width = (width + mcu_width - 1) / mcu_width * mcu_width; + uint32_t output_height = (height + mcu_height - 1) / mcu_height * mcu_height; + size_t output_size = output_width * output_height * sizeof(uint16_t); + + // Decode straight into the Bitmap when it is exactly the image, unpadded and + // cache-line aligned, and the whole image lands at 0,0. + bool direct = width == output_width && height == output_height && + bitmap->width == width && bitmap->height == height && + self->x == 0 && self->y == 0 && + self->lim.x1 <= 0 && self->lim.y1 <= 0 && + self->lim.x2 >= (int)width && self->lim.y2 >= (int)height && + output_aligned(bitmap->data, output_size); + + if (decoder_engine == NULL) { + jpeg_decode_engine_cfg_t engine_config = { + .timeout_ms = JPEG_DECODE_TIMEOUT_MS, + }; + if (jpeg_new_decoder_engine(&engine_config, &decoder_engine) != ESP_OK) { + decoder_engine = NULL; + return false; + } + } + + // Otherwise decode into a temporary buffer and copy the visible part out of it. + uint8_t *output; + size_t output_buffer_size; + uint8_t *scratch = NULL; + if (direct) { + output = (uint8_t *)bitmap->data; + output_buffer_size = output_size; + } else { + jpeg_decode_memory_alloc_cfg_t mem_config = { + .buffer_direction = JPEG_DEC_ALLOC_OUTPUT_BUFFER, + }; + scratch = jpeg_alloc_decoder_mem(output_size, &mem_config, &output_buffer_size); + if (scratch == NULL) { + return false; + } + output = scratch; + } + + // RGB order with RGB565 makes the engine emit the high byte first, the same + // byte-swapped RGB565 that TJpgDec produces here. JFIF files use full range + // BT601 YCbCr. + jpeg_decode_cfg_t decode_config = { + .output_format = JPEG_DECODE_OUT_FORMAT_RGB565, + .rgb_order = JPEG_DEC_RGB_ELEMENT_ORDER_RGB, + .conv_std = JPEG_YUV_RGB_CONV_STD_BT601, + .yuv_range = COLOR_RANGE_FULL, + }; + uint32_t decoded_size; + if (jpeg_decoder_process(decoder_engine, &decode_config, jpeg->buf, jpeg->len, + output, output_buffer_size, &decoded_size) != ESP_OK) { + // A failed direct decode may have written part of the Bitmap. TJpgDec + // then decodes the whole image over it. + heap_caps_free(scratch); + return false; + } + + if (direct) { + displayio_area_t area = { 0, 0, width, height, NULL }; + displayio_bitmap_set_dirty_area(bitmap, &area); + return true; + } + + // Copy out the visible part with the same clipping TJpgDec's output callback + // applies, treating the whole image as a single block at 0,0. + displayio_bitmap_t src = { + .width = width, + .height = height, + .data = (uint32_t *)(void *)output, + .stride = output_width / 2, // in units of uint32_t + .bits_per_value = 16, + .x_shift = 1, + .x_mask = 1, + .bitmask = 0xffff, + }; + int x = self->x; + int y = self->y; + int x1 = self->lim.x1; + int y1 = self->lim.y1; + int x2 = MIN(self->lim.x2, (int)width); + int y2 = MIN(self->lim.y2, (int)height); + if (x1 < 0) { + x += -x1; + x1 = 0; + } + if (y1 < 0) { + y += -y1; + y1 = 0; + } + if (x2 > 0 && y2 > 0) { + common_hal_bitmaptools_blit(bitmap, &src, x, y, x1, y1, x2, y2, + self->skip_source_index, self->skip_source_index_none, + self->skip_dest_index, self->skip_dest_index_none); + } + heap_caps_free(scratch); + return true; +} diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.h b/ports/espressif/common-hal/jpegio/JpegDecoder.h new file mode 100644 index 00000000000..0ca7958dc5b --- /dev/null +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.h @@ -0,0 +1,10 @@ +// This file is part of the CircuitPython project: https://circuitpython.org +// +// SPDX-FileCopyrightText: Copyright (c) 2026 Mikey Sklar +// +// SPDX-License-Identifier: MIT + +#pragma once + +// Releases the JPEG engine. Called on VM reset. +void espressif_jpegio_reset(void); diff --git a/ports/espressif/mpconfigport.h b/ports/espressif/mpconfigport.h index 7de44130971..a8a577840a7 100644 --- a/ports/espressif/mpconfigport.h +++ b/ports/espressif/mpconfigport.h @@ -17,6 +17,11 @@ #define CIRCUITPY_DIGITALIO_HAVE_INPUT_ONLY (1) +#if CIRCUITPY_JPEGIO_HW +// The JPEG engine decodes straight into Bitmaps whose pixels start on a cache line. +#define CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT (64) +#endif + #include "py/circuitpy_mpconfig.h" #define MICROPY_NLR_SETJMP (1) diff --git a/ports/espressif/mpconfigport.mk b/ports/espressif/mpconfigport.mk index 87ba43d1ebf..44ef8e043fa 100644 --- a/ports/espressif/mpconfigport.mk +++ b/ports/espressif/mpconfigport.mk @@ -404,6 +404,9 @@ CIRCUITPY_ESPCAMERA = 0 # P4 has MIPI-DSI CIRCUITPY_MIPIDSI = 1 +# Hardware JPEG decoder +CIRCUITPY_JPEGIO_HW ?= $(CIRCUITPY_JPEGIO) + #### esp32s31 ######################################################### else ifeq ($(IDF_TARGET),esp32s31) @@ -438,6 +441,9 @@ CIRCUITPY_ESPULP = 0 # The S31 has five eFuse key blocks, not six. CIRCUITPY_HARDWAREKEY = 0 +# Hardware JPEG decoder +CIRCUITPY_JPEGIO_HW ?= $(CIRCUITPY_JPEGIO) + # No I80 support from the IDF CIRCUITPY_PARALLELDISPLAYBUS = 0 diff --git a/ports/espressif/supervisor/port.c b/ports/espressif/supervisor/port.c index 91fe8cf5a2e..0bca67d487a 100644 --- a/ports/espressif/supervisor/port.c +++ b/ports/espressif/supervisor/port.c @@ -63,6 +63,10 @@ #include "common-hal/espcamera/Camera.h" #endif +#if CIRCUITPY_JPEGIO_HW +#include "common-hal/jpegio/JpegDecoder.h" +#endif + #if CIRCUITPY_RCLCPY #include "common-hal/rclcpy/__init__.h" #endif @@ -484,6 +488,10 @@ void reset_port(void) { rclcpy_reset(); #endif + #if CIRCUITPY_JPEGIO_HW + espressif_jpegio_reset(); + #endif + #if CIRCUITPY_RTC rtc_reset(); #endif diff --git a/py/circuitpy_mpconfig.h b/py/circuitpy_mpconfig.h index 8649b3e65a2..770435c093e 100644 --- a/py/circuitpy_mpconfig.h +++ b/py/circuitpy_mpconfig.h @@ -431,6 +431,12 @@ typedef long mp_off_t; #define CIRCUITPY_DISPLAY_LIMIT (0) #endif +// Byte alignment of the pixel memory a Bitmap allocates. Ports raise it when hardware +// writes Bitmap pixels directly and needs them to start on a cache line. +#ifndef CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT +#define CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT (4) +#endif + // This is not a top-level module; it's microcontroller.nvm. #if CIRCUITPY_NVM extern const struct _mp_obj_module_t nvm_module; diff --git a/py/circuitpy_mpconfig.mk b/py/circuitpy_mpconfig.mk index 0fc82d03704..7137f5930f6 100755 --- a/py/circuitpy_mpconfig.mk +++ b/py/circuitpy_mpconfig.mk @@ -426,6 +426,10 @@ CFLAGS += -DCIRCUITPY_IS31FL3741=$(CIRCUITPY_IS31FL3741) CIRCUITPY_JPEGIO ?= $(CIRCUITPY_BITMAPTOOLS) CFLAGS += -DCIRCUITPY_JPEGIO=$(CIRCUITPY_JPEGIO) +# Port provides a hardware JPEG decode path for jpegio. +CIRCUITPY_JPEGIO_HW ?= 0 +CFLAGS += -DCIRCUITPY_JPEGIO_HW=$(CIRCUITPY_JPEGIO_HW) + CIRCUITPY_JSON ?= $(CIRCUITPY_FULL_BUILD) CFLAGS += -DCIRCUITPY_JSON=$(CIRCUITPY_JSON) diff --git a/shared-bindings/jpegio/JpegDecoder.c b/shared-bindings/jpegio/JpegDecoder.c index d55de7b67e0..b016babc4ab 100644 --- a/shared-bindings/jpegio/JpegDecoder.c +++ b/shared-bindings/jpegio/JpegDecoder.c @@ -124,6 +124,14 @@ MP_DEFINE_CONST_FUN_OBJ_2(jpegio_jpegdecoder_open_obj, jpegio_jpegdecoder_open); //| possible to repeatedly ``decode`` the same jpeg data, even if it is to //| select different scales or crop regions from it. //| +//| On chips with a JPEG decoder peripheral (ESP32-P4 and ESP32-S31), ``decode`` +//| uses the peripheral for baseline color JPEGs when the data was opened from a +//| buffer in DMA-capable memory, ``scale`` is 0, ``bitmap`` holds 16-bit values, +//| and no skip index is given. It is fastest when ``bitmap`` is exactly the image +//| size and the whole image is decoded at 0,0. Colors may differ slightly from the +//| software decoder's. Other images, and data opened from a file or stream, use +//| the software decoder. +//| //| :param Bitmap bitmap: Optional output buffer //| :param int scale: Scale factor from 0 to 3, inclusive. //| :param int x: Horizontal pixel location in bitmap where source_bitmap upper-left diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c index 29f32fd40ce..ec434ef5d75 100644 --- a/shared-module/displayio/Bitmap.c +++ b/shared-module/displayio/Bitmap.c @@ -31,7 +31,19 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, self->stride = stride(width, bits_per_value); self->data_alloc = false; if (!data) { - data = m_malloc_without_collect(self->stride * height * sizeof(uint32_t)); + size_t size = self->stride * height * sizeof(uint32_t); + #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 + // Pad the allocation so data can start on an alignment boundary, and round the size + // up so data ends on one too and shares no alignment unit with other memory. + const uintptr_t alignment = CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT; + size = (size + alignment - 1) & ~(alignment - 1); + const size_t padding = alignment > MICROPY_BYTES_PER_GC_BLOCK ? alignment - MICROPY_BYTES_PER_GC_BLOCK : 0; + uint8_t *allocation = m_malloc_without_collect(size + padding); + self->data_allocation = allocation; + data = (uint32_t *)(((uintptr_t)allocation + alignment - 1) & ~(alignment - 1)); + #else + data = m_malloc_without_collect(size); + #endif self->data_alloc = true; } self->data = data; @@ -64,7 +76,11 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self) { if (self->data_alloc && self->data != NULL) { + #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 + gc_free(self->data_allocation); + #else gc_free(self->data); + #endif } self->data = NULL; } diff --git a/shared-module/displayio/Bitmap.h b/shared-module/displayio/Bitmap.h index a4a57587207..df3b13b63c3 100644 --- a/shared-module/displayio/Bitmap.h +++ b/shared-module/displayio/Bitmap.h @@ -25,6 +25,10 @@ typedef struct { uint16_t bitmask; bool read_only; bool data_alloc; // did bitmap allocate data or someone else + #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 + // The GC allocation that holds data. data may start partway into it. + void *data_allocation; + #endif } displayio_bitmap_t; void displayio_bitmap_finish_refresh(displayio_bitmap_t *self); diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index b70cfccf22d..0ab295875a6 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -204,6 +204,17 @@ void common_hal_jpegio_jpegdecoder_decode_into( self->skip_dest_index_none = skip_dest_index_none; self->dest = bitmap; + + #if CIRCUITPY_JPEGIO_HW + mp_buffer_info_t jpeg; + if (self->decoder.infunc == buffer_input && + mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ) && + jpegio_jpegdecoder_decode_hw(self, &jpeg, scale)) { + common_hal_jpegio_jpegdecoder_close(self); + return; + } + #endif + JRESULT result = jd_decomp(&self->decoder, bitmap_output, scale); common_hal_jpegio_jpegdecoder_close(self); if (result != JDR_INTR) { diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index 2585b364545..6c9574ad1ca 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -25,3 +25,13 @@ typedef struct jpegio_jpegdecoder_obj { bool skip_source_index_none, skip_dest_index_none; uint8_t scale; } jpegio_jpegdecoder_obj_t; + +#if CIRCUITPY_JPEGIO_HW +// Ports with JPEG decode hardware provide this. It is called by decode() only when the +// source passed to open() is an in-memory buffer; `jpeg` is that whole buffer. The decode +// arguments (x, y, lim, skip indices) are already stored in `self`, and `self->dest` is +// the target bitmap. +// Returns true if the port decoded the image into `self->dest`. Returns false to decode +// with TJpgDec instead, which rewrites every pixel the image covers. Must not raise. +bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale); +#endif diff --git a/tests/circuitpython-manual/jpegio/hw_decode.py b/tests/circuitpython-manual/jpegio/hw_decode.py new file mode 100644 index 00000000000..63df3b5d4e4 --- /dev/null +++ b/tests/circuitpython-manual/jpegio/hw_decode.py @@ -0,0 +1,74 @@ +# SPDX-FileCopyrightText: 2026 Mikey Sklar +# SPDX-License-Identifier: MIT + +# Compare jpegio's hardware decode with TJpgDec and time both. +# Copy baseline JPEGs into /jpegtest/ first, for example 720p at 4:2:0, 4:2:2 and 4:4:4, +# a size that is not a multiple of 16, a grayscale and a progressive file. +# Opening a filename always uses TJpgDec; opening bytes uses the hardware when it can. +# Expect small differences, since the two IDCTs round differently. + +import os +import time + +import displayio +import jpegio + +DIR = "/jpegtest/" +decoder = jpegio.JpegDecoder() + + +def max_step(a, b): + worst = 0 + differ = 0 + for i in range(0, len(a), 2): + va = (a[i] << 8) | a[i + 1] + vb = (b[i] << 8) | b[i + 1] + if va != vb: + differ += 1 + worst = max( + worst, + abs((va >> 11) - (vb >> 11)) * 2, + abs(((va >> 5) & 63) - ((vb >> 5) & 63)), + abs((va & 31) - (vb & 31)) * 2, + ) + return differ, worst + + +def median_ms(source, bitmap, n=10): + times = [] + for _ in range(n): + t0 = time.monotonic_ns() + decoder.open(source) + decoder.decode(bitmap) + times.append(time.monotonic_ns() - t0) + times.sort() + return times[n // 2] / 1e6 + + +for name in sorted(os.listdir(DIR)): + path = DIR + name + data = open(path, "rb").read() + try: + width, height = decoder.open(data) + hw = displayio.Bitmap(width, height, 65535) + decoder.decode(hw) + except Exception as e: + print(name, "raises", repr(e)) + continue + sw = displayio.Bitmap(width, height, 65535) + decoder.open(path) + decoder.decode(sw) + differ, worst = max_step(bytes(memoryview(sw)), bytes(memoryview(hw))) + print( + "%s %dx%d: %d of %d pixels differ, max %d steps; bytes %.1f ms, file %.1f ms" + % ( + name, + width, + height, + differ, + width * height, + worst, + median_ms(data, hw), + median_ms(path, sw, 3), + ) + ) From 3994305a41f22307b7cce5831d46a3cc2ef96deb Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Mon, 19 Oct 2026 06:38:15 -0700 Subject: [PATCH 02/12] Update esp-idf for JPEG engine color range and S31 ROM fix Points at adafruit/esp-idf#19, which also merges #18. Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/esp-idf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/espressif/esp-idf b/ports/espressif/esp-idf index 0687c505dad..765b98b64a7 160000 --- a/ports/espressif/esp-idf +++ b/ports/espressif/esp-idf @@ -1 +1 @@ -Subproject commit 0687c505dad1ace31c664de2c5494c13c08b0b1d +Subproject commit 765b98b64a7f13d952b3724005162727cfed8ba8 From 5846a7c989ddd992ffd79bac9e5d31a5f53c3f6f Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Mon, 19 Oct 2026 12:15:33 -0700 Subject: [PATCH 03/12] espressif: align GC blocks to the cache line on P4 and S31 Use 64-byte GC blocks and align the PSRAM heap areas to them, as the camera change does, so Bitmap pixels start on a cache line without a Bitmap-specific alignment option. Revert the Bitmap allocation change. Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/Makefile | 7 +++++++ .../espressif/common-hal/jpegio/JpegDecoder.c | 6 +++--- ports/espressif/mpconfigport.h | 5 ----- ports/espressif/supervisor/port.c | 6 ++++-- py/circuitpy_mpconfig.h | 6 ------ shared-module/displayio/Bitmap.c | 18 +----------------- shared-module/displayio/Bitmap.h | 4 ---- 7 files changed, 15 insertions(+), 37 deletions(-) diff --git a/ports/espressif/Makefile b/ports/espressif/Makefile index 224edb091b2..fc4a1f30685 100644 --- a/ports/espressif/Makefile +++ b/ports/espressif/Makefile @@ -229,6 +229,13 @@ REGISTRATION_FUNCTIONS = \ -u esp_security_init_include_impl \ -u mbedtls_psa_crypto_init_include_impl +# JPEG engine DMA destinations come from the Python heap (Bitmaps), and +# the cache line is 64 bytes on the P4 and S31, so make GC blocks 64 bytes as well. +# supervisor/port.c aligns the PSRAM heap areas to the same value. +ifneq ($(filter esp32p4 esp32s31,$(IDF_TARGET)),) +CFLAGS += -DMICROPY_BYTES_PER_GC_BLOCK=64 +endif + # RISC-V picolibc ships optimized memcpy/memset/etc. that need forcing # (gated by CONFIG_LIBC_OPTIMIZED_MISALIGNED_ACCESS in IDF's esp_libc CMake). ifeq ($(IDF_TARGET_ARCH),riscv) diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c index 1feae565d75..3d080e30023 100644 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.c +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -33,12 +33,12 @@ void espressif_jpegio_reset(void) { } } -// Bitmap pixels must start on a cache line for the engine to write them directly. +// GC blocks start on a cache line, so the engine can write Bitmap pixels directly. #ifdef CONFIG_CACHE_L2_CACHE_LINE_SIZE -_Static_assert(CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT % CONFIG_CACHE_L2_CACHE_LINE_SIZE == 0, "Bitmap alignment is not a cache line"); +_Static_assert(MICROPY_BYTES_PER_GC_BLOCK % CONFIG_CACHE_L2_CACHE_LINE_SIZE == 0, "GC block is not a cache line"); #endif #ifdef CONFIG_CACHE_L1_DCACHE_LINE_SIZE -_Static_assert(CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT % CONFIG_CACHE_L1_DCACHE_LINE_SIZE == 0, "Bitmap alignment is not a cache line"); +_Static_assert(MICROPY_BYTES_PER_GC_BLOCK % CONFIG_CACHE_L1_DCACHE_LINE_SIZE == 0, "GC block is not a cache line"); #endif // True if the hardware may write `size` bytes at `buf` directly: the start and diff --git a/ports/espressif/mpconfigport.h b/ports/espressif/mpconfigport.h index a8a577840a7..7de44130971 100644 --- a/ports/espressif/mpconfigport.h +++ b/ports/espressif/mpconfigport.h @@ -17,11 +17,6 @@ #define CIRCUITPY_DIGITALIO_HAVE_INPUT_ONLY (1) -#if CIRCUITPY_JPEGIO_HW -// The JPEG engine decodes straight into Bitmaps whose pixels start on a cache line. -#define CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT (64) -#endif - #include "py/circuitpy_mpconfig.h" #define MICROPY_NLR_SETJMP (1) diff --git a/ports/espressif/supervisor/port.c b/ports/espressif/supervisor/port.c index 0bca67d487a..4524b6d1ec5 100644 --- a/ports/espressif/supervisor/port.c +++ b/ports/espressif/supervisor/port.c @@ -347,9 +347,11 @@ void *port_malloc(size_t size, bool dma_capable) { } void *ptr = NULL; - // Try SPIRAM first if available. + // Try SPIRAM first if available. Aligned to the GC block size so heap + // areas (and with 64-byte GC blocks, every GC allocation in them) are + // cache-line aligned, which video DMA destinations require. #ifdef CONFIG_SPIRAM - ptr = heap_caps_malloc(size, MALLOC_CAP_8BIT | MALLOC_CAP_SPIRAM); + ptr = heap_caps_aligned_alloc(MICROPY_BYTES_PER_GC_BLOCK, size, MALLOC_CAP_8BIT | MALLOC_CAP_SPIRAM); #endif if (ptr == NULL) { ptr = heap_caps_malloc(size, MALLOC_CAP_8BIT); diff --git a/py/circuitpy_mpconfig.h b/py/circuitpy_mpconfig.h index 770435c093e..8649b3e65a2 100644 --- a/py/circuitpy_mpconfig.h +++ b/py/circuitpy_mpconfig.h @@ -431,12 +431,6 @@ typedef long mp_off_t; #define CIRCUITPY_DISPLAY_LIMIT (0) #endif -// Byte alignment of the pixel memory a Bitmap allocates. Ports raise it when hardware -// writes Bitmap pixels directly and needs them to start on a cache line. -#ifndef CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT -#define CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT (4) -#endif - // This is not a top-level module; it's microcontroller.nvm. #if CIRCUITPY_NVM extern const struct _mp_obj_module_t nvm_module; diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c index ec434ef5d75..29f32fd40ce 100644 --- a/shared-module/displayio/Bitmap.c +++ b/shared-module/displayio/Bitmap.c @@ -31,19 +31,7 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, self->stride = stride(width, bits_per_value); self->data_alloc = false; if (!data) { - size_t size = self->stride * height * sizeof(uint32_t); - #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 - // Pad the allocation so data can start on an alignment boundary, and round the size - // up so data ends on one too and shares no alignment unit with other memory. - const uintptr_t alignment = CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT; - size = (size + alignment - 1) & ~(alignment - 1); - const size_t padding = alignment > MICROPY_BYTES_PER_GC_BLOCK ? alignment - MICROPY_BYTES_PER_GC_BLOCK : 0; - uint8_t *allocation = m_malloc_without_collect(size + padding); - self->data_allocation = allocation; - data = (uint32_t *)(((uintptr_t)allocation + alignment - 1) & ~(alignment - 1)); - #else - data = m_malloc_without_collect(size); - #endif + data = m_malloc_without_collect(self->stride * height * sizeof(uint32_t)); self->data_alloc = true; } self->data = data; @@ -76,11 +64,7 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self) { if (self->data_alloc && self->data != NULL) { - #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 - gc_free(self->data_allocation); - #else gc_free(self->data); - #endif } self->data = NULL; } diff --git a/shared-module/displayio/Bitmap.h b/shared-module/displayio/Bitmap.h index df3b13b63c3..a4a57587207 100644 --- a/shared-module/displayio/Bitmap.h +++ b/shared-module/displayio/Bitmap.h @@ -25,10 +25,6 @@ typedef struct { uint16_t bitmask; bool read_only; bool data_alloc; // did bitmap allocate data or someone else - #if CIRCUITPY_DISPLAYIO_BITMAP_DATA_ALIGNMENT > 4 - // The GC allocation that holds data. data may start partway into it. - void *data_allocation; - #endif } displayio_bitmap_t; void displayio_bitmap_finish_refresh(displayio_bitmap_t *self); From d69ef5f31da54b0afa7a9be5313ce67a05718f08 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Mon, 19 Oct 2026 12:17:17 -0700 Subject: [PATCH 04/12] jpegio: free the JPEG engine from a finaliser Each JpegDecoder now owns its JPEG engine and releases it when it is collected, instead of sharing one engine that is freed on VM reset. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../espressif/common-hal/jpegio/JpegDecoder.c | 20 +++++++++---------- .../espressif/common-hal/jpegio/JpegDecoder.h | 10 ---------- ports/espressif/supervisor/port.c | 8 -------- shared-bindings/jpegio/JpegDecoder.c | 11 +++++++++- shared-bindings/jpegio/JpegDecoder.h | 1 + shared-module/jpegio/JpegDecoder.c | 9 +++++++++ shared-module/jpegio/JpegDecoder.h | 7 +++++++ 7 files changed, 36 insertions(+), 30 deletions(-) delete mode 100644 ports/espressif/common-hal/jpegio/JpegDecoder.h diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c index 3d080e30023..0e53a916a03 100644 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.c +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -11,7 +11,6 @@ #include "py/runtime.h" -#include "common-hal/jpegio/JpegDecoder.h" #include "shared-bindings/bitmaptools/__init__.h" #include "shared-module/displayio/Bitmap.h" #include "shared-module/jpegio/JpegDecoder.h" @@ -24,12 +23,10 @@ // A bad image blocks the engine for at most this long. #define JPEG_DECODE_TIMEOUT_MS (1000) -static jpeg_decoder_handle_t decoder_engine; - -void espressif_jpegio_reset(void) { - if (decoder_engine != NULL) { - jpeg_del_decoder_engine(decoder_engine); - decoder_engine = NULL; +void jpegio_jpegdecoder_deinit_hw(jpegio_jpegdecoder_obj_t *self) { + if (self->hw_decoder != NULL) { + jpeg_del_decoder_engine(self->hw_decoder); + self->hw_decoder = NULL; } } @@ -108,14 +105,15 @@ bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffe self->lim.x2 >= (int)width && self->lim.y2 >= (int)height && output_aligned(bitmap->data, output_size); - if (decoder_engine == NULL) { + if (self->hw_decoder == NULL) { jpeg_decode_engine_cfg_t engine_config = { .timeout_ms = JPEG_DECODE_TIMEOUT_MS, }; - if (jpeg_new_decoder_engine(&engine_config, &decoder_engine) != ESP_OK) { - decoder_engine = NULL; + jpeg_decoder_handle_t engine; + if (jpeg_new_decoder_engine(&engine_config, &engine) != ESP_OK) { return false; } + self->hw_decoder = engine; } // Otherwise decode into a temporary buffer and copy the visible part out of it. @@ -146,7 +144,7 @@ bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffe .yuv_range = COLOR_RANGE_FULL, }; uint32_t decoded_size; - if (jpeg_decoder_process(decoder_engine, &decode_config, jpeg->buf, jpeg->len, + if (jpeg_decoder_process(self->hw_decoder, &decode_config, jpeg->buf, jpeg->len, output, output_buffer_size, &decoded_size) != ESP_OK) { // A failed direct decode may have written part of the Bitmap. TJpgDec // then decodes the whole image over it. diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.h b/ports/espressif/common-hal/jpegio/JpegDecoder.h deleted file mode 100644 index 0ca7958dc5b..00000000000 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.h +++ /dev/null @@ -1,10 +0,0 @@ -// This file is part of the CircuitPython project: https://circuitpython.org -// -// SPDX-FileCopyrightText: Copyright (c) 2026 Mikey Sklar -// -// SPDX-License-Identifier: MIT - -#pragma once - -// Releases the JPEG engine. Called on VM reset. -void espressif_jpegio_reset(void); diff --git a/ports/espressif/supervisor/port.c b/ports/espressif/supervisor/port.c index 4524b6d1ec5..81eb4714db6 100644 --- a/ports/espressif/supervisor/port.c +++ b/ports/espressif/supervisor/port.c @@ -63,10 +63,6 @@ #include "common-hal/espcamera/Camera.h" #endif -#if CIRCUITPY_JPEGIO_HW -#include "common-hal/jpegio/JpegDecoder.h" -#endif - #if CIRCUITPY_RCLCPY #include "common-hal/rclcpy/__init__.h" #endif @@ -490,10 +486,6 @@ void reset_port(void) { rclcpy_reset(); #endif - #if CIRCUITPY_JPEGIO_HW - espressif_jpegio_reset(); - #endif - #if CIRCUITPY_RTC rtc_reset(); #endif diff --git a/shared-bindings/jpegio/JpegDecoder.c b/shared-bindings/jpegio/JpegDecoder.c index b016babc4ab..d7ad8c494be 100644 --- a/shared-bindings/jpegio/JpegDecoder.c +++ b/shared-bindings/jpegio/JpegDecoder.c @@ -47,7 +47,7 @@ static mp_obj_t jpegio_jpegdecoder_make_new(const mp_obj_type_t *type, size_t n_ 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); - jpegio_jpegdecoder_obj_t *self = mp_obj_malloc(jpegio_jpegdecoder_obj_t, &jpegio_jpegdecoder_type); + jpegio_jpegdecoder_obj_t *self = mp_obj_malloc_with_finaliser(jpegio_jpegdecoder_obj_t, &jpegio_jpegdecoder_type); common_hal_jpegio_jpegdecoder_construct(self); return MP_OBJ_FROM_PTR(self); @@ -203,9 +203,18 @@ static mp_obj_t jpegio_jpegdecoder_decode(size_t n_args, const mp_obj_t *pos_arg } static MP_DEFINE_CONST_FUN_OBJ_KW(jpegio_jpegdecoder_decode_obj, 1, jpegio_jpegdecoder_decode); +// Releases any hardware decoder the object holds when it is collected. +static mp_obj_t jpegio_jpegdecoder_del(mp_obj_t self_in) { + jpegio_jpegdecoder_obj_t *self = MP_OBJ_TO_PTR(self_in); + common_hal_jpegio_jpegdecoder_deinit(self); + return mp_const_none; +} +static MP_DEFINE_CONST_FUN_OBJ_1(jpegio_jpegdecoder_del_obj, jpegio_jpegdecoder_del); + static const mp_rom_map_elem_t jpegio_jpegdecoder_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_open), MP_ROM_PTR(&jpegio_jpegdecoder_open_obj) }, { MP_ROM_QSTR(MP_QSTR_decode), MP_ROM_PTR(&jpegio_jpegdecoder_decode_obj) }, + { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&jpegio_jpegdecoder_del_obj) }, }; static MP_DEFINE_CONST_DICT(jpegio_jpegdecoder_locals_dict, jpegio_jpegdecoder_locals_dict_table); diff --git a/shared-bindings/jpegio/JpegDecoder.h b/shared-bindings/jpegio/JpegDecoder.h index 560586a871e..f36e6c56810 100644 --- a/shared-bindings/jpegio/JpegDecoder.h +++ b/shared-bindings/jpegio/JpegDecoder.h @@ -16,6 +16,7 @@ extern const mp_obj_type_t jpegio_jpegdecoder_type; typedef struct jpegio_jpegdecoder_obj jpegio_jpegdecoder_obj_t; void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self); +void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_buffer(jpegio_jpegdecoder_obj_t *self, mp_obj_t jpeg_data); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_file(jpegio_jpegdecoder_obj_t *self, mp_obj_t file_obj); diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index 0ab295875a6..a888c183436 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -54,6 +54,15 @@ static void check_jresult(JRESULT j) { void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; + #if CIRCUITPY_JPEGIO_HW + self->hw_decoder = NULL; + #endif +} + +void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self) { + #if CIRCUITPY_JPEGIO_HW + jpegio_jpegdecoder_deinit_hw(self); + #endif } void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self) { diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index 6c9574ad1ca..aeb9c2ca5e6 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -24,6 +24,9 @@ typedef struct jpegio_jpegdecoder_obj { uint32_t skip_source_index, skip_dest_index; bool skip_source_index_none, skip_dest_index_none; uint8_t scale; + #if CIRCUITPY_JPEGIO_HW + void *hw_decoder; // port state for the hardware decoder, NULL until first used + #endif } jpegio_jpegdecoder_obj_t; #if CIRCUITPY_JPEGIO_HW @@ -34,4 +37,8 @@ typedef struct jpegio_jpegdecoder_obj { // Returns true if the port decoded the image into `self->dest`. Returns false to decode // with TJpgDec instead, which rewrites every pixel the image covers. Must not raise. bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale); + +// Ports with JPEG decode hardware provide this. It releases `self->hw_decoder`, if set, +// and sets it to NULL. Called when the decoder object is collected. Must not raise. +void jpegio_jpegdecoder_deinit_hw(jpegio_jpegdecoder_obj_t *self); #endif From e381071ef4929cb5668aa2a7df3ed90b77a1bb46 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Mon, 19 Oct 2026 16:13:38 -0700 Subject: [PATCH 05/12] jpegio: add JpegDecoder.used_hardware Reports whether the last decode() used the JPEG decoder peripheral, so scripts and tests can check the path without timing it. Ports without the hardware always report False. Co-Authored-By: Claude Opus 5.5 (1M context) --- shared-bindings/jpegio/JpegDecoder.c | 15 +++++++++++++++ shared-bindings/jpegio/JpegDecoder.h | 1 + shared-module/jpegio/JpegDecoder.c | 7 +++++++ shared-module/jpegio/JpegDecoder.h | 1 + tests/circuitpython-manual/jpegio/hw_decode.py | 6 +++++- 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/shared-bindings/jpegio/JpegDecoder.c b/shared-bindings/jpegio/JpegDecoder.c index d7ad8c494be..200dbfd0430 100644 --- a/shared-bindings/jpegio/JpegDecoder.c +++ b/shared-bindings/jpegio/JpegDecoder.c @@ -6,6 +6,7 @@ #include "py/obj.h" #include "py/builtin.h" +#include "py/objproperty.h" #include "py/runtime.h" #include "shared-bindings/bitmaptools/__init__.h" @@ -203,6 +204,19 @@ static mp_obj_t jpegio_jpegdecoder_decode(size_t n_args, const mp_obj_t *pos_arg } static MP_DEFINE_CONST_FUN_OBJ_KW(jpegio_jpegdecoder_decode_obj, 1, jpegio_jpegdecoder_decode); +//| used_hardware: bool +//| """True if the last `decode` used the chip's JPEG decoder peripheral, False if it +//| used the software decoder. (read only)""" +//| +static mp_obj_t jpegio_jpegdecoder_obj_get_used_hardware(mp_obj_t self_in) { + jpegio_jpegdecoder_obj_t *self = MP_OBJ_TO_PTR(self_in); + return mp_obj_new_bool(common_hal_jpegio_jpegdecoder_get_used_hardware(self)); +} +MP_DEFINE_CONST_FUN_OBJ_1(jpegio_jpegdecoder_get_used_hardware_obj, jpegio_jpegdecoder_obj_get_used_hardware); + +MP_PROPERTY_GETTER(jpegio_jpegdecoder_used_hardware_obj, + (mp_obj_t)&jpegio_jpegdecoder_get_used_hardware_obj); + // Releases any hardware decoder the object holds when it is collected. static mp_obj_t jpegio_jpegdecoder_del(mp_obj_t self_in) { jpegio_jpegdecoder_obj_t *self = MP_OBJ_TO_PTR(self_in); @@ -214,6 +228,7 @@ static MP_DEFINE_CONST_FUN_OBJ_1(jpegio_jpegdecoder_del_obj, jpegio_jpegdecoder_ static const mp_rom_map_elem_t jpegio_jpegdecoder_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_open), MP_ROM_PTR(&jpegio_jpegdecoder_open_obj) }, { MP_ROM_QSTR(MP_QSTR_decode), MP_ROM_PTR(&jpegio_jpegdecoder_decode_obj) }, + { MP_ROM_QSTR(MP_QSTR_used_hardware), MP_ROM_PTR(&jpegio_jpegdecoder_used_hardware_obj) }, { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&jpegio_jpegdecoder_del_obj) }, }; static MP_DEFINE_CONST_DICT(jpegio_jpegdecoder_locals_dict, jpegio_jpegdecoder_locals_dict_table); diff --git a/shared-bindings/jpegio/JpegDecoder.h b/shared-bindings/jpegio/JpegDecoder.h index f36e6c56810..073141b8b1f 100644 --- a/shared-bindings/jpegio/JpegDecoder.h +++ b/shared-bindings/jpegio/JpegDecoder.h @@ -18,6 +18,7 @@ typedef struct jpegio_jpegdecoder_obj jpegio_jpegdecoder_obj_t; void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self); +bool common_hal_jpegio_jpegdecoder_get_used_hardware(jpegio_jpegdecoder_obj_t *self); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_buffer(jpegio_jpegdecoder_obj_t *self, mp_obj_t jpeg_data); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_file(jpegio_jpegdecoder_obj_t *self, mp_obj_t file_obj); void common_hal_jpegio_jpegdecoder_decode_into( diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index a888c183436..ad991b3aae1 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -54,6 +54,7 @@ static void check_jresult(JRESULT j) { void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; + self->used_hardware = false; #if CIRCUITPY_JPEGIO_HW self->hw_decoder = NULL; #endif @@ -65,6 +66,10 @@ void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self) { #endif } +bool common_hal_jpegio_jpegdecoder_get_used_hardware(jpegio_jpegdecoder_obj_t *self) { + return self->used_hardware; +} + void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; memset(&self->bufinfo, 0, sizeof(self->bufinfo)); @@ -213,12 +218,14 @@ void common_hal_jpegio_jpegdecoder_decode_into( self->skip_dest_index_none = skip_dest_index_none; self->dest = bitmap; + self->used_hardware = false; #if CIRCUITPY_JPEGIO_HW mp_buffer_info_t jpeg; if (self->decoder.infunc == buffer_input && mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ) && jpegio_jpegdecoder_decode_hw(self, &jpeg, scale)) { + self->used_hardware = true; common_hal_jpegio_jpegdecoder_close(self); return; } diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index aeb9c2ca5e6..06c42743662 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -24,6 +24,7 @@ typedef struct jpegio_jpegdecoder_obj { uint32_t skip_source_index, skip_dest_index; bool skip_source_index_none, skip_dest_index_none; uint8_t scale; + bool used_hardware; #if CIRCUITPY_JPEGIO_HW void *hw_decoder; // port state for the hardware decoder, NULL until first used #endif diff --git a/tests/circuitpython-manual/jpegio/hw_decode.py b/tests/circuitpython-manual/jpegio/hw_decode.py index 63df3b5d4e4..7991de064ff 100644 --- a/tests/circuitpython-manual/jpegio/hw_decode.py +++ b/tests/circuitpython-manual/jpegio/hw_decode.py @@ -5,6 +5,7 @@ # Copy baseline JPEGs into /jpegtest/ first, for example 720p at 4:2:0, 4:2:2 and 4:4:4, # a size that is not a multiple of 16, a grayscale and a progressive file. # Opening a filename always uses TJpgDec; opening bytes uses the hardware when it can. +# decoder.used_hardware says which one the last decode() used. # Expect small differences, since the two IDCTs round differently. import os @@ -52,19 +53,22 @@ def median_ms(source, bitmap, n=10): width, height = decoder.open(data) hw = displayio.Bitmap(width, height, 65535) decoder.decode(hw) + used_hardware = decoder.used_hardware except Exception as e: print(name, "raises", repr(e)) continue sw = displayio.Bitmap(width, height, 65535) decoder.open(path) decoder.decode(sw) + assert not decoder.used_hardware differ, worst = max_step(bytes(memoryview(sw)), bytes(memoryview(hw))) print( - "%s %dx%d: %d of %d pixels differ, max %d steps; bytes %.1f ms, file %.1f ms" + "%s %dx%d hardware=%s: %d of %d pixels differ, max %d steps; bytes %.1f ms, file %.1f ms" % ( name, width, height, + used_hardware, differ, width * height, worst, From e5a1af283f867b9b8b05b467a8e05fb286f859c3 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 12:25:25 -0700 Subject: [PATCH 06/12] espressif: set the GC block size in the P4 and S31 sections Match the Makefile style of one section per target. Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/Makefile | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ports/espressif/Makefile b/ports/espressif/Makefile index fc4a1f30685..4494840cc23 100644 --- a/ports/espressif/Makefile +++ b/ports/espressif/Makefile @@ -229,13 +229,6 @@ REGISTRATION_FUNCTIONS = \ -u esp_security_init_include_impl \ -u mbedtls_psa_crypto_init_include_impl -# JPEG engine DMA destinations come from the Python heap (Bitmaps), and -# the cache line is 64 bytes on the P4 and S31, so make GC blocks 64 bytes as well. -# supervisor/port.c aligns the PSRAM heap areas to the same value. -ifneq ($(filter esp32p4 esp32s31,$(IDF_TARGET)),) -CFLAGS += -DMICROPY_BYTES_PER_GC_BLOCK=64 -endif - # RISC-V picolibc ships optimized memcpy/memset/etc. that need forcing # (gated by CONFIG_LIBC_OPTIMIZED_MISALIGNED_ACCESS in IDF's esp_libc CMake). ifeq ($(IDF_TARGET_ARCH),riscv) @@ -495,6 +488,9 @@ CFLAGS += \ -isystem esp-idf/components/esp_hw_support/ldo/include \ -isystem esp-idf/components/esp_driver_touch_sens/hw_ver3/include +# JPEG engine DMA writes into GC blocks, so make them one 64-byte cache line. +CFLAGS += -DMICROPY_BYTES_PER_GC_BLOCK=64 + ifeq ($(CIRCUITPY_ESP32P4_REV),3) IDF_TARGET_ROM_FLOAT_LD = esp32p4.rom.rvfp.ld LDFLAGS += \ @@ -531,6 +527,9 @@ CHIP_COMPONENTS = \ else ifeq ($(IDF_TARGET),esp32s31) CFLAGS += -isystem esp-idf/components/esp_hw_support/ldo/include +# JPEG engine DMA writes into GC blocks, so make them one 64-byte cache line. +CFLAGS += -DMICROPY_BYTES_PER_GC_BLOCK=64 + IDF_TARGET_ROM_FLOAT_LD = esp32s31.rom.rvfp.ld LDFLAGS += \ -Tesp32s31.rom.heap.ld \ From cb1d88fd41baddd53f76b1071130d2a4cd4878a1 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 12:25:34 -0700 Subject: [PATCH 07/12] jpegio: remove JpegDecoder.used_hardware Removed per review. A software_ok option will cover the case of expecting hardware decode and not getting it. Co-Authored-By: Claude Opus 5.5 (1M context) --- shared-bindings/jpegio/JpegDecoder.c | 15 --------------- shared-bindings/jpegio/JpegDecoder.h | 1 - shared-module/jpegio/JpegDecoder.c | 7 ------- shared-module/jpegio/JpegDecoder.h | 1 - tests/circuitpython-manual/jpegio/hw_decode.py | 6 +----- 5 files changed, 1 insertion(+), 29 deletions(-) diff --git a/shared-bindings/jpegio/JpegDecoder.c b/shared-bindings/jpegio/JpegDecoder.c index 200dbfd0430..d7ad8c494be 100644 --- a/shared-bindings/jpegio/JpegDecoder.c +++ b/shared-bindings/jpegio/JpegDecoder.c @@ -6,7 +6,6 @@ #include "py/obj.h" #include "py/builtin.h" -#include "py/objproperty.h" #include "py/runtime.h" #include "shared-bindings/bitmaptools/__init__.h" @@ -204,19 +203,6 @@ static mp_obj_t jpegio_jpegdecoder_decode(size_t n_args, const mp_obj_t *pos_arg } static MP_DEFINE_CONST_FUN_OBJ_KW(jpegio_jpegdecoder_decode_obj, 1, jpegio_jpegdecoder_decode); -//| used_hardware: bool -//| """True if the last `decode` used the chip's JPEG decoder peripheral, False if it -//| used the software decoder. (read only)""" -//| -static mp_obj_t jpegio_jpegdecoder_obj_get_used_hardware(mp_obj_t self_in) { - jpegio_jpegdecoder_obj_t *self = MP_OBJ_TO_PTR(self_in); - return mp_obj_new_bool(common_hal_jpegio_jpegdecoder_get_used_hardware(self)); -} -MP_DEFINE_CONST_FUN_OBJ_1(jpegio_jpegdecoder_get_used_hardware_obj, jpegio_jpegdecoder_obj_get_used_hardware); - -MP_PROPERTY_GETTER(jpegio_jpegdecoder_used_hardware_obj, - (mp_obj_t)&jpegio_jpegdecoder_get_used_hardware_obj); - // Releases any hardware decoder the object holds when it is collected. static mp_obj_t jpegio_jpegdecoder_del(mp_obj_t self_in) { jpegio_jpegdecoder_obj_t *self = MP_OBJ_TO_PTR(self_in); @@ -228,7 +214,6 @@ static MP_DEFINE_CONST_FUN_OBJ_1(jpegio_jpegdecoder_del_obj, jpegio_jpegdecoder_ static const mp_rom_map_elem_t jpegio_jpegdecoder_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_open), MP_ROM_PTR(&jpegio_jpegdecoder_open_obj) }, { MP_ROM_QSTR(MP_QSTR_decode), MP_ROM_PTR(&jpegio_jpegdecoder_decode_obj) }, - { MP_ROM_QSTR(MP_QSTR_used_hardware), MP_ROM_PTR(&jpegio_jpegdecoder_used_hardware_obj) }, { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&jpegio_jpegdecoder_del_obj) }, }; static MP_DEFINE_CONST_DICT(jpegio_jpegdecoder_locals_dict, jpegio_jpegdecoder_locals_dict_table); diff --git a/shared-bindings/jpegio/JpegDecoder.h b/shared-bindings/jpegio/JpegDecoder.h index 073141b8b1f..f36e6c56810 100644 --- a/shared-bindings/jpegio/JpegDecoder.h +++ b/shared-bindings/jpegio/JpegDecoder.h @@ -18,7 +18,6 @@ typedef struct jpegio_jpegdecoder_obj jpegio_jpegdecoder_obj_t; void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self); -bool common_hal_jpegio_jpegdecoder_get_used_hardware(jpegio_jpegdecoder_obj_t *self); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_buffer(jpegio_jpegdecoder_obj_t *self, mp_obj_t jpeg_data); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_file(jpegio_jpegdecoder_obj_t *self, mp_obj_t file_obj); void common_hal_jpegio_jpegdecoder_decode_into( diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index ad991b3aae1..a888c183436 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -54,7 +54,6 @@ static void check_jresult(JRESULT j) { void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; - self->used_hardware = false; #if CIRCUITPY_JPEGIO_HW self->hw_decoder = NULL; #endif @@ -66,10 +65,6 @@ void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self) { #endif } -bool common_hal_jpegio_jpegdecoder_get_used_hardware(jpegio_jpegdecoder_obj_t *self) { - return self->used_hardware; -} - void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; memset(&self->bufinfo, 0, sizeof(self->bufinfo)); @@ -218,14 +213,12 @@ void common_hal_jpegio_jpegdecoder_decode_into( self->skip_dest_index_none = skip_dest_index_none; self->dest = bitmap; - self->used_hardware = false; #if CIRCUITPY_JPEGIO_HW mp_buffer_info_t jpeg; if (self->decoder.infunc == buffer_input && mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ) && jpegio_jpegdecoder_decode_hw(self, &jpeg, scale)) { - self->used_hardware = true; common_hal_jpegio_jpegdecoder_close(self); return; } diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index 06c42743662..aeb9c2ca5e6 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -24,7 +24,6 @@ typedef struct jpegio_jpegdecoder_obj { uint32_t skip_source_index, skip_dest_index; bool skip_source_index_none, skip_dest_index_none; uint8_t scale; - bool used_hardware; #if CIRCUITPY_JPEGIO_HW void *hw_decoder; // port state for the hardware decoder, NULL until first used #endif diff --git a/tests/circuitpython-manual/jpegio/hw_decode.py b/tests/circuitpython-manual/jpegio/hw_decode.py index 7991de064ff..63df3b5d4e4 100644 --- a/tests/circuitpython-manual/jpegio/hw_decode.py +++ b/tests/circuitpython-manual/jpegio/hw_decode.py @@ -5,7 +5,6 @@ # Copy baseline JPEGs into /jpegtest/ first, for example 720p at 4:2:0, 4:2:2 and 4:4:4, # a size that is not a multiple of 16, a grayscale and a progressive file. # Opening a filename always uses TJpgDec; opening bytes uses the hardware when it can. -# decoder.used_hardware says which one the last decode() used. # Expect small differences, since the two IDCTs round differently. import os @@ -53,22 +52,19 @@ def median_ms(source, bitmap, n=10): width, height = decoder.open(data) hw = displayio.Bitmap(width, height, 65535) decoder.decode(hw) - used_hardware = decoder.used_hardware except Exception as e: print(name, "raises", repr(e)) continue sw = displayio.Bitmap(width, height, 65535) decoder.open(path) decoder.decode(sw) - assert not decoder.used_hardware differ, worst = max_step(bytes(memoryview(sw)), bytes(memoryview(hw))) print( - "%s %dx%d hardware=%s: %d of %d pixels differ, max %d steps; bytes %.1f ms, file %.1f ms" + "%s %dx%d: %d of %d pixels differ, max %d steps; bytes %.1f ms, file %.1f ms" % ( name, width, height, - used_hardware, differ, width * height, worst, From 5dfb972eaef4aa8db5bdc3756ef4a3125243236c Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 12:27:10 -0700 Subject: [PATCH 08/12] jpegio: rename the hardware hooks to hw_decode and hw_deinit Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/common-hal/jpegio/JpegDecoder.c | 4 ++-- shared-module/jpegio/JpegDecoder.c | 4 ++-- shared-module/jpegio/JpegDecoder.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c index 0e53a916a03..0f39ac759c9 100644 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.c +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -23,7 +23,7 @@ // A bad image blocks the engine for at most this long. #define JPEG_DECODE_TIMEOUT_MS (1000) -void jpegio_jpegdecoder_deinit_hw(jpegio_jpegdecoder_obj_t *self) { +void jpegio_jpegdecoder_hw_deinit(jpegio_jpegdecoder_obj_t *self) { if (self->hw_decoder != NULL) { jpeg_del_decoder_engine(self->hw_decoder); self->hw_decoder = NULL; @@ -48,7 +48,7 @@ static bool output_aligned(const void *buf, size_t size) { return ((uintptr_t)buf & (align - 1)) == 0 && (size & (align - 1)) == 0; } -bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale) { +bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale) { displayio_bitmap_t *bitmap = self->dest; if (scale != 0 || bitmap->bits_per_value != 16 || bitmap->read_only) { return false; diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index a888c183436..979dcd10663 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -61,7 +61,7 @@ void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self) { #if CIRCUITPY_JPEGIO_HW - jpegio_jpegdecoder_deinit_hw(self); + jpegio_jpegdecoder_hw_deinit(self); #endif } @@ -218,7 +218,7 @@ void common_hal_jpegio_jpegdecoder_decode_into( mp_buffer_info_t jpeg; if (self->decoder.infunc == buffer_input && mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ) && - jpegio_jpegdecoder_decode_hw(self, &jpeg, scale)) { + jpegio_jpegdecoder_hw_decode(self, &jpeg, scale)) { common_hal_jpegio_jpegdecoder_close(self); return; } diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index aeb9c2ca5e6..d3f6034235c 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -36,9 +36,9 @@ typedef struct jpegio_jpegdecoder_obj { // the target bitmap. // Returns true if the port decoded the image into `self->dest`. Returns false to decode // with TJpgDec instead, which rewrites every pixel the image covers. Must not raise. -bool jpegio_jpegdecoder_decode_hw(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale); +bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale); // Ports with JPEG decode hardware provide this. It releases `self->hw_decoder`, if set, // and sets it to NULL. Called when the decoder object is collected. Must not raise. -void jpegio_jpegdecoder_deinit_hw(jpegio_jpegdecoder_obj_t *self); +void jpegio_jpegdecoder_hw_deinit(jpegio_jpegdecoder_obj_t *self); #endif From c4cc0b4c506fc3f85764cd345d6982c3ace1f443 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 12:27:49 -0700 Subject: [PATCH 09/12] jpegio: create the JPEG engine in hw_init, called from construct This matches hw_deinit, which frees it from the finaliser. If the engine cannot be created, hw_decoder stays NULL and decode() uses TJpgDec. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../espressif/common-hal/jpegio/JpegDecoder.c | 25 +++++++++++-------- shared-module/jpegio/JpegDecoder.c | 2 +- shared-module/jpegio/JpegDecoder.h | 7 +++++- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c index 0f39ac759c9..792d9f36aa1 100644 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.c +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -23,6 +23,17 @@ // A bad image blocks the engine for at most this long. #define JPEG_DECODE_TIMEOUT_MS (1000) +void jpegio_jpegdecoder_hw_init(jpegio_jpegdecoder_obj_t *self) { + jpeg_decode_engine_cfg_t engine_config = { + .timeout_ms = JPEG_DECODE_TIMEOUT_MS, + }; + jpeg_decoder_handle_t engine; + if (jpeg_new_decoder_engine(&engine_config, &engine) != ESP_OK) { + engine = NULL; + } + self->hw_decoder = engine; +} + void jpegio_jpegdecoder_hw_deinit(jpegio_jpegdecoder_obj_t *self) { if (self->hw_decoder != NULL) { jpeg_del_decoder_engine(self->hw_decoder); @@ -49,6 +60,9 @@ static bool output_aligned(const void *buf, size_t size) { } bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale) { + if (self->hw_decoder == NULL) { + return false; + } displayio_bitmap_t *bitmap = self->dest; if (scale != 0 || bitmap->bits_per_value != 16 || bitmap->read_only) { return false; @@ -105,17 +119,6 @@ bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffe self->lim.x2 >= (int)width && self->lim.y2 >= (int)height && output_aligned(bitmap->data, output_size); - if (self->hw_decoder == NULL) { - jpeg_decode_engine_cfg_t engine_config = { - .timeout_ms = JPEG_DECODE_TIMEOUT_MS, - }; - jpeg_decoder_handle_t engine; - if (jpeg_new_decoder_engine(&engine_config, &engine) != ESP_OK) { - return false; - } - self->hw_decoder = engine; - } - // Otherwise decode into a temporary buffer and copy the visible part out of it. uint8_t *output; size_t output_buffer_size; diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index 979dcd10663..8fd9da0b20a 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -55,7 +55,7 @@ static void check_jresult(JRESULT j) { void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { self->data_obj = MP_OBJ_NULL; #if CIRCUITPY_JPEGIO_HW - self->hw_decoder = NULL; + jpegio_jpegdecoder_hw_init(self); #endif } diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index d3f6034235c..85a3cbef1a6 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -25,11 +25,16 @@ typedef struct jpegio_jpegdecoder_obj { bool skip_source_index_none, skip_dest_index_none; uint8_t scale; #if CIRCUITPY_JPEGIO_HW - void *hw_decoder; // port state for the hardware decoder, NULL until first used + void *hw_decoder; // port state for the hardware decoder, NULL if unavailable #endif } jpegio_jpegdecoder_obj_t; #if CIRCUITPY_JPEGIO_HW +// Ports with JPEG decode hardware provide this. Called when the decoder object is +// constructed. Sets `self->hw_decoder`, or NULL if the hardware is unavailable, in which +// case decode() uses TJpgDec. Must not raise. +void jpegio_jpegdecoder_hw_init(jpegio_jpegdecoder_obj_t *self); + // Ports with JPEG decode hardware provide this. It is called by decode() only when the // source passed to open() is an in-memory buffer; `jpeg` is that whole buffer. The decode // arguments (x, y, lim, skip indices) are already stored in `self`, and `self->dest` is From 11c912efc7b5ec49d5eb2052e2265f0095ddebcd Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 12:30:49 -0700 Subject: [PATCH 10/12] jpegio: add software_ok to JpegDecoder software_ok=True (the default) keeps falling back to TJpgDec when the JPEG decoder peripheral cannot decode an image. software_ok=False raises ValueError instead, including on chips without the peripheral, so code that expects hardware decode finds out when it does not get it. The error messages reuse existing translated strings. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../espressif/common-hal/jpegio/JpegDecoder.c | 44 ++++++++++++++----- shared-bindings/jpegio/JpegDecoder.c | 15 +++++-- shared-bindings/jpegio/JpegDecoder.h | 2 +- shared-module/jpegio/JpegDecoder.c | 18 +++++--- shared-module/jpegio/JpegDecoder.h | 4 +- 5 files changed, 61 insertions(+), 22 deletions(-) diff --git a/ports/espressif/common-hal/jpegio/JpegDecoder.c b/ports/espressif/common-hal/jpegio/JpegDecoder.c index 792d9f36aa1..24c883d193d 100644 --- a/ports/espressif/common-hal/jpegio/JpegDecoder.c +++ b/ports/espressif/common-hal/jpegio/JpegDecoder.c @@ -59,26 +59,48 @@ static bool output_aligned(const void *buf, size_t size) { return ((uintptr_t)buf & (align - 1)) == 0 && (size & (align - 1)) == 0; } +// These return false so decode() uses TJpgDec, or raise ValueError if +// software decoding is not allowed. +static bool use_software(jpegio_jpegdecoder_obj_t *self, mp_rom_error_text_t msg) { + if (!self->software_ok) { + mp_raise_ValueError(msg); + } + return false; +} + +static bool use_software_for_arg(jpegio_jpegdecoder_obj_t *self, qstr arg_name) { + if (!self->software_ok) { + mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), arg_name); + } + return false; +} + bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale) { if (self->hw_decoder == NULL) { - return false; + return use_software(self, MP_ERROR_TEXT("Operation or feature not supported")); } displayio_bitmap_t *bitmap = self->dest; - if (scale != 0 || bitmap->bits_per_value != 16 || bitmap->read_only) { - return false; + if (scale != 0) { + return use_software_for_arg(self, MP_QSTR_scale); + } + if (bitmap->bits_per_value != 16 || bitmap->read_only) { + return use_software_for_arg(self, MP_QSTR_bitmap); } // Skip indices are slower on this path than in TJpgDec. - if (!self->skip_source_index_none || !self->skip_dest_index_none) { - return false; + if (!self->skip_source_index_none) { + return use_software_for_arg(self, MP_QSTR_skip_source_index); + } + if (!self->skip_dest_index_none) { + return use_software_for_arg(self, MP_QSTR_skip_dest_index); } // The engine reads the input by DMA, so it must be in DMA-capable memory. if (!esp_ptr_dma_capable(jpeg->buf) && !esp_ptr_dma_ext_capable(jpeg->buf)) { - return false; + return use_software_for_arg(self, MP_QSTR_data_source); } jpeg_decode_picture_info_t info; if (jpeg_decoder_get_info(jpeg->buf, jpeg->len, &info) != ESP_OK) { - return false; + return use_software(self, MP_ERROR_TEXT("Format not supported")); } // The engine pads its output to whole MCUs: 8x8 for 4:4:4, 16x8 for 4:2:2 @@ -98,13 +120,13 @@ bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffe mcu_height = 16; break; default: - return false; + return use_software(self, MP_ERROR_TEXT("Format not supported")); } uint32_t width = info.width; uint32_t height = info.height; // The driver rejects images whose pixel count is not a multiple of 8. if ((width * height) % 8 != 0) { - return false; + return use_software(self, MP_ERROR_TEXT("Format not supported")); } uint32_t output_width = (width + mcu_width - 1) / mcu_width * mcu_width; uint32_t output_height = (height + mcu_height - 1) / mcu_height * mcu_height; @@ -132,7 +154,7 @@ bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffe }; scratch = jpeg_alloc_decoder_mem(output_size, &mem_config, &output_buffer_size); if (scratch == NULL) { - return false; + return use_software(self, MP_ERROR_TEXT("Operation or feature not supported")); } output = scratch; } @@ -152,7 +174,7 @@ bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffe // A failed direct decode may have written part of the Bitmap. TJpgDec // then decodes the whole image over it. heap_caps_free(scratch); - return false; + return use_software(self, MP_ERROR_TEXT("Format not supported")); } if (direct) { diff --git a/shared-bindings/jpegio/JpegDecoder.c b/shared-bindings/jpegio/JpegDecoder.c index d7ad8c494be..a7475741343 100644 --- a/shared-bindings/jpegio/JpegDecoder.c +++ b/shared-bindings/jpegio/JpegDecoder.c @@ -37,18 +37,25 @@ //| //| """ //| -//| def __init__(self) -> None: -//| """Create a JpegDecoder""" +//| def __init__(self, *, software_ok: bool = True) -> None: +//| """Create a JpegDecoder +//| +//| :param bool software_ok: If True, `decode` uses the software decoder when the JPEG +//| decoder peripheral cannot decode the image. If False, `decode` raises `ValueError` +//| instead, including on chips without the peripheral. +//| """ //| ... //| static mp_obj_t jpegio_jpegdecoder_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { + enum { ARG_software_ok }; static const mp_arg_t allowed_args[] = { + { MP_QSTR_software_ok, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} }, }; 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); jpegio_jpegdecoder_obj_t *self = mp_obj_malloc_with_finaliser(jpegio_jpegdecoder_obj_t, &jpegio_jpegdecoder_type); - common_hal_jpegio_jpegdecoder_construct(self); + common_hal_jpegio_jpegdecoder_construct(self, args[ARG_software_ok].u_bool); return MP_OBJ_FROM_PTR(self); } @@ -130,7 +137,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(jpegio_jpegdecoder_open_obj, jpegio_jpegdecoder_open); //| and no skip index is given. It is fastest when ``bitmap`` is exactly the image //| size and the whole image is decoded at 0,0. Colors may differ slightly from the //| software decoder's. Other images, and data opened from a file or stream, use -//| the software decoder. +//| the software decoder, or raise `ValueError` if ``software_ok`` is False. //| //| :param Bitmap bitmap: Optional output buffer //| :param int scale: Scale factor from 0 to 3, inclusive. diff --git a/shared-bindings/jpegio/JpegDecoder.h b/shared-bindings/jpegio/JpegDecoder.h index f36e6c56810..98e7216d74a 100644 --- a/shared-bindings/jpegio/JpegDecoder.h +++ b/shared-bindings/jpegio/JpegDecoder.h @@ -15,7 +15,7 @@ extern const mp_obj_type_t jpegio_jpegdecoder_type; typedef struct jpegio_jpegdecoder_obj jpegio_jpegdecoder_obj_t; -void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self); +void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self, bool software_ok); void common_hal_jpegio_jpegdecoder_deinit(jpegio_jpegdecoder_obj_t *self); void common_hal_jpegio_jpegdecoder_close(jpegio_jpegdecoder_obj_t *self); mp_obj_t common_hal_jpegio_jpegdecoder_set_source_buffer(jpegio_jpegdecoder_obj_t *self, mp_obj_t jpeg_data); diff --git a/shared-module/jpegio/JpegDecoder.c b/shared-module/jpegio/JpegDecoder.c index 8fd9da0b20a..9245c65d822 100644 --- a/shared-module/jpegio/JpegDecoder.c +++ b/shared-module/jpegio/JpegDecoder.c @@ -52,8 +52,9 @@ static void check_jresult(JRESULT j) { mp_raise_RuntimeError(msg); } -void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self) { +void common_hal_jpegio_jpegdecoder_construct(jpegio_jpegdecoder_obj_t *self, bool software_ok) { self->data_obj = MP_OBJ_NULL; + self->software_ok = software_ok; #if CIRCUITPY_JPEGIO_HW jpegio_jpegdecoder_hw_init(self); #endif @@ -217,10 +218,17 @@ void common_hal_jpegio_jpegdecoder_decode_into( #if CIRCUITPY_JPEGIO_HW mp_buffer_info_t jpeg; if (self->decoder.infunc == buffer_input && - mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ) && - jpegio_jpegdecoder_hw_decode(self, &jpeg, scale)) { - common_hal_jpegio_jpegdecoder_close(self); - return; + mp_get_buffer(self->data_obj, &jpeg, MP_BUFFER_READ)) { + if (jpegio_jpegdecoder_hw_decode(self, &jpeg, scale)) { + common_hal_jpegio_jpegdecoder_close(self); + return; + } + } else if (!self->software_ok) { + mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_data_source); + } + #else + if (!self->software_ok) { + mp_raise_ValueError(MP_ERROR_TEXT("Operation or feature not supported")); } #endif diff --git a/shared-module/jpegio/JpegDecoder.h b/shared-module/jpegio/JpegDecoder.h index 85a3cbef1a6..33659168299 100644 --- a/shared-module/jpegio/JpegDecoder.h +++ b/shared-module/jpegio/JpegDecoder.h @@ -24,6 +24,7 @@ typedef struct jpegio_jpegdecoder_obj { uint32_t skip_source_index, skip_dest_index; bool skip_source_index_none, skip_dest_index_none; uint8_t scale; + bool software_ok; #if CIRCUITPY_JPEGIO_HW void *hw_decoder; // port state for the hardware decoder, NULL if unavailable #endif @@ -40,7 +41,8 @@ void jpegio_jpegdecoder_hw_init(jpegio_jpegdecoder_obj_t *self); // arguments (x, y, lim, skip indices) are already stored in `self`, and `self->dest` is // the target bitmap. // Returns true if the port decoded the image into `self->dest`. Returns false to decode -// with TJpgDec instead, which rewrites every pixel the image covers. Must not raise. +// with TJpgDec instead, which rewrites every pixel the image covers. If +// `self->software_ok` is false, raises ValueError instead of returning false. bool jpegio_jpegdecoder_hw_decode(jpegio_jpegdecoder_obj_t *self, const mp_buffer_info_t *jpeg, int scale); // Ports with JPEG decode hardware provide this. It releases `self->hw_decoder`, if set, From d7bab2ea977a560aeeed08c1783afcf18e3dc76d Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 14:31:23 -0700 Subject: [PATCH 11/12] jpegio: test software_ok=False in hw_decode.py Co-Authored-By: Claude Opus 5.5 (1M context) --- .../circuitpython-manual/jpegio/hw_decode.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/circuitpython-manual/jpegio/hw_decode.py b/tests/circuitpython-manual/jpegio/hw_decode.py index 63df3b5d4e4..01f19a3e660 100644 --- a/tests/circuitpython-manual/jpegio/hw_decode.py +++ b/tests/circuitpython-manual/jpegio/hw_decode.py @@ -5,6 +5,8 @@ # Copy baseline JPEGs into /jpegtest/ first, for example 720p at 4:2:0, 4:2:2 and 4:4:4, # a size that is not a multiple of 16, a grayscale and a progressive file. # Opening a filename always uses TJpgDec; opening bytes uses the hardware when it can. +# The last section checks that software_ok=False raises ValueError instead of using TJpgDec. +# On chips without JPEG hardware, every case in that section should raise. # Expect small differences, since the two IDCTs round differently. import os @@ -72,3 +74,23 @@ def median_ms(source, bitmap, n=10): median_ms(path, sw, 3), ) ) + + +strict = jpegio.JpegDecoder(software_ok=False) + + +def strict_decode(source, **kwargs): + try: + width, height = strict.open(source) + strict.decode(displayio.Bitmap(width, height, 65535), **kwargs) + return "decoded" + except (ValueError, RuntimeError) as e: + return "%s: %s" % (type(e).__name__, e) + + +for name in sorted(os.listdir(DIR)): + path = DIR + name + data = open(path, "rb").read() + print("software_ok=False", name, "bytes:", strict_decode(data)) + print("software_ok=False", name, "scale=1:", strict_decode(data, scale=1)) + print("software_ok=False", name, "file:", strict_decode(path)) From d7581f930ccf4a0a597bd72c30989ea42bbd6917 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Tue, 20 Oct 2026 17:49:41 -0700 Subject: [PATCH 12/12] espressif: update esp-idf to circuitpython-v6.1 tip Match the esp-idf pin in #11643 (29d36039) so the two PRs merge cleanly in either order. 29d36039 already contains the jpeg full-range change (adafruit/esp-idf#18) this PR needs, plus #19 and #21. Co-Authored-By: Claude Opus 5.5 (1M context) --- ports/espressif/esp-idf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/espressif/esp-idf b/ports/espressif/esp-idf index 765b98b64a7..29d36039e56 160000 --- a/ports/espressif/esp-idf +++ b/ports/espressif/esp-idf @@ -1 +1 @@ -Subproject commit 765b98b64a7f13d952b3724005162727cfed8ba8 +Subproject commit 29d36039e564db6df505d0582b94b230a17e63f3