Merge pull request #7712 from gamblor21/ondiskgif_freemem_fix

dhalbert · web-flow · commit db76fbd55f8f · 2023-03-23T09:03:07.000-04:00
Add deinit to OnDiskGif
diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile
@@ -59,6 +59,7 @@ INC += -I. \
 ifeq ($(CHIP_FAMILY), samd21)
 PERIPHERALS_CHIP_FAMILY=samd21
 OPTIMIZATION_FLAGS ?= -Os
+
 # TinyUSB defines
 CFLAGS += -DCFG_TUSB_MCU=OPT_MCU_SAMD21 -DCFG_TUD_MIDI_RX_BUFSIZE=128 -DCFG_TUD_CDC_RX_BUFSIZE=128 -DCFG_TUD_MIDI_TX_BUFSIZE=128 -DCFG_TUD_CDC_TX_BUFSIZE=128 -DCFG_TUD_MSC_BUFSIZE=512
 endif
@@ -101,24 +102,26 @@ ifeq ($(DEBUG), 1)
   endif
 else
   CFLAGS += -DNDEBUG
-  # -finline-limit can shrink the image size.
-  # -finline-limit=80 or so is similar to not having it on.
-  # There is no simple default value, though.
 
-  # Do a default shrink for small builds.
-  ifndef CFLAGS_INLINE_LIMIT
-    ifeq ($(CIRCUITPY_FULL_BUILD),0)
-      CFLAGS_INLINE_LIMIT = 50
+  # Do a default shrink for small builds, including all SAMD21 builds.
+  ifeq ($(CIRCUITPY_FULL_BUILD),0)
+    SHRINK_BUILD = 1
+  else
+    ifeq ($(CHIP_FAMILY), samd21)
+      SHRINK_BUILD = 1
     endif
   endif
 
-  ifdef CFLAGS_INLINE_LIMIT
-    CFLAGS += -finline-limit=$(CFLAGS_INLINE_LIMIT)
+  # -finline-limit can shrink the image size.
+  # -finline-limit=80 or so is similar to not having it on.
+  # There is no simple default value, though.
+  ifeq ($(SHRINK_BUILD), 1)
+    CFLAGS += -finline-limit=45
   endif
 
-  ifeq ($(CIRCUITPY_FULL_BUILD),0)
-    CFLAGS += --param inline-unit-growth=15 --param max-inline-insns-auto=20
-  endif
+  # We used to do this but it seems to not reduce space any more, at least in gcc 11.
+  # Leave it here, commented out, just for reference.
+  # --param inline-unit-growth=15 --param max-inline-insns-auto=20
 
   ifdef CFLAGS_BOARD
     CFLAGS += $(CFLAGS_BOARD)
diff --git a/shared-bindings/displayio/Bitmap.c b/shared-bindings/displayio/Bitmap.c
@@ -84,11 +84,19 @@ STATIC mp_obj_t displayio_bitmap_make_new(const mp_obj_type_t *type, size_t n_ar
 
     return MP_OBJ_FROM_PTR(self);
 }
+
+STATIC void check_for_deinit(displayio_bitmap_t *self) {
+    if (common_hal_displayio_bitmap_deinited(self)) {
+        raise_deinited_error();
+    }
+}
+
 //|     width: int
 //|     """Width of the bitmap. (read only)"""
 STATIC mp_obj_t displayio_bitmap_obj_get_width(mp_obj_t self_in) {
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return MP_OBJ_NEW_SMALL_INT(common_hal_displayio_bitmap_get_width(self));
 }
 
@@ -102,6 +110,7 @@ MP_PROPERTY_GETTER(displayio_bitmap_width_obj,
 STATIC mp_obj_t displayio_bitmap_obj_get_height(mp_obj_t self_in) {
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return MP_OBJ_NEW_SMALL_INT(common_hal_displayio_bitmap_get_height(self));
 }
 
@@ -134,6 +143,7 @@ STATIC mp_obj_t bitmap_subscr(mp_obj_t self_in, mp_obj_t index_obj, mp_obj_t val
     }
 
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
+    check_for_deinit(self);
 
     if (mp_obj_is_type(index_obj, &mp_type_slice)) {
         // TODO(tannewt): Implement subscr after slices support start, stop and step tuples.
@@ -214,6 +224,7 @@ STATIC mp_obj_t displayio_bitmap_obj_blit(size_t n_args, const mp_obj_t *pos_arg
     mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+    check_for_deinit(self);
 
     int16_t x = args[ARG_x].u_int;
     int16_t y = args[ARG_y].u_int;
@@ -288,6 +299,7 @@ MP_DEFINE_CONST_FUN_OBJ_KW(displayio_bitmap_blit_obj, 1, displayio_bitmap_obj_bl
 //|         ...
 STATIC mp_obj_t displayio_bitmap_obj_fill(mp_obj_t self_in, mp_obj_t value_obj) {
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
+    check_for_deinit(self);
 
     mp_uint_t value = (mp_uint_t)mp_obj_get_int(value_obj);
     if ((value >> common_hal_displayio_bitmap_get_bits_per_value(self)) != 0) {
@@ -318,9 +330,10 @@ MP_DEFINE_CONST_FUN_OBJ_2(displayio_bitmap_fill_obj, displayio_bitmap_obj_fill);
 //|         notified of the "dirty rectangle" that encloses all modified
 //|         pixels."""
 //|         ...
-//|
 STATIC mp_obj_t displayio_bitmap_obj_dirty(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     displayio_bitmap_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+    check_for_deinit(self);
+
     enum { ARG_x1, ARG_y1, ARG_x2, ARG_y2 };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_x1, MP_ARG_INT, {.u_int = 0} },
@@ -344,13 +357,24 @@ STATIC mp_obj_t displayio_bitmap_obj_dirty(size_t n_args, const mp_obj_t *pos_ar
 }
 MP_DEFINE_CONST_FUN_OBJ_KW(displayio_bitmap_dirty_obj, 0, displayio_bitmap_obj_dirty);
 
+//|     def deinit(self) -> None:
+//|         """Release resources allocated by Bitmap."""
+//|         ...
+//|
+STATIC mp_obj_t displayio_bitmap_obj_deinit(mp_obj_t self_in) {
+    displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
+    common_hal_displayio_bitmap_deinit(self);
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_1(displayio_bitmap_deinit_obj, displayio_bitmap_obj_deinit);
+
 STATIC const mp_rom_map_elem_t displayio_bitmap_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_height), MP_ROM_PTR(&displayio_bitmap_height_obj) },
     { MP_ROM_QSTR(MP_QSTR_width), MP_ROM_PTR(&displayio_bitmap_width_obj) },
     { MP_ROM_QSTR(MP_QSTR_blit), MP_ROM_PTR(&displayio_bitmap_blit_obj) },
     { MP_ROM_QSTR(MP_QSTR_fill), MP_ROM_PTR(&displayio_bitmap_fill_obj) },
     { MP_ROM_QSTR(MP_QSTR_dirty), MP_ROM_PTR(&displayio_bitmap_dirty_obj) },
-
+    { MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&displayio_bitmap_deinit_obj) },
 };
 STATIC MP_DEFINE_CONST_DICT(displayio_bitmap_locals_dict, displayio_bitmap_locals_dict_table);
 
diff --git a/shared-bindings/displayio/Bitmap.h b/shared-bindings/displayio/Bitmap.h
@@ -48,5 +48,7 @@ void common_hal_displayio_bitmap_blit(displayio_bitmap_t *self, int16_t x, int16
 uint32_t common_hal_displayio_bitmap_get_pixel(displayio_bitmap_t *bitmap, int16_t x, int16_t y);
 void common_hal_displayio_bitmap_fill(displayio_bitmap_t *bitmap, uint32_t value);
 int common_hal_displayio_bitmap_get_buffer(displayio_bitmap_t *self, mp_buffer_info_t *bufinfo, mp_uint_t flags);
+void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self);
+bool common_hal_displayio_bitmap_deinited(displayio_bitmap_t *self);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_BITMAP_H
diff --git a/shared-bindings/gifio/OnDiskGif.c b/shared-bindings/gifio/OnDiskGif.c
@@ -30,6 +30,8 @@
 
 #include "py/runtime.h"
 #include "py/objproperty.h"
+#include "shared/runtime/context_manager_helpers.h"
+#include "shared-bindings/util.h"
 #include "supervisor/shared/translate/translate.h"
 #include "shared-bindings/gifio/OnDiskGif.h"
 
@@ -96,6 +98,14 @@
 //|           display_bus.send(42, struct.pack(">hh", 0, odg.bitmap.width - 1))
 //|           display_bus.send(43, struct.pack(">hh", 0, odg.bitmap.height - 1))
 //|           display_bus.send(44, d.bitmap)
+//|
+//|       # The following optional code will free the OnDiskGif and allocated resources
+//|       # after use. This may be required before loading a new GIF in situations
+//|       # where RAM is limited and the first GIF took most of the RAM.
+//|       odg.deinit()
+//|       odg = None
+//|       gc.collect()
+//|
 //|     """
 //|
 //|     def __init__(self, file: str) -> None:
@@ -125,11 +135,34 @@ STATIC mp_obj_t gifio_ondiskgif_make_new(const mp_obj_type_t *type, size_t n_arg
     return MP_OBJ_FROM_PTR(self);
 }
 
+STATIC void check_for_deinit(gifio_ondiskgif_t *self) {
+    if (common_hal_gifio_ondiskgif_deinited(self)) {
+        raise_deinited_error();
+    }
+}
+
+//|     def __enter__(self) -> OnDiskGif:
+//|         """No-op used by Context Managers."""
+//|         ...
+//  Provided by context manager helper.
+
+//|     def __exit__(self) -> None:
+//|         """Automatically deinitializes the GIF when exiting a context. See
+//|         :ref:`lifetime-and-contextmanagers` for more info."""
+//|         ...
+STATIC mp_obj_t gifio_ondiskgif_obj___exit__(size_t n_args, const mp_obj_t *args) {
+    (void)n_args;
+    common_hal_gifio_ondiskgif_deinit(args[0]);
+    return mp_const_none;
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(gifio_ondiskgif___exit___obj, 4, 4, gifio_ondiskgif_obj___exit__);
+
 //|     width: int
 //|     """Width of the gif. (read only)"""
 STATIC mp_obj_t gifio_ondiskgif_obj_get_width(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_width(self));
 }
 
@@ -143,6 +176,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_width_obj,
 STATIC mp_obj_t gifio_ondiskgif_obj_get_height(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_height(self));
 }
 
@@ -155,6 +189,8 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_height_obj,
 //|     """The bitmap used to hold the current frame."""
 STATIC mp_obj_t gifio_ondiskgif_obj_get_bitmap(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
+
+    check_for_deinit(self);
     return common_hal_gifio_ondiskgif_get_bitmap(self);
 }
 
@@ -168,6 +204,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_bitmap_obj,
 STATIC mp_obj_t gifio_ondiskgif_obj_next_frame(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return mp_obj_new_float((float)common_hal_gifio_ondiskgif_next_frame(self, true) / 1000);
 }
 
@@ -179,6 +216,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_next_frame_obj, gifio_ondiskgif_obj_ne
 STATIC mp_obj_t gifio_ondiskgif_obj_get_duration(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_duration(self) / 1000);
 }
 
@@ -192,6 +230,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_duration_obj,
 STATIC mp_obj_t gifio_ondiskgif_obj_get_frame_count(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_frame_count(self));
 }
 
@@ -205,6 +244,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_frame_count_obj,
 STATIC mp_obj_t gifio_ondiskgif_obj_get_min_delay(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_min_delay(self) / 1000);
 }
 
@@ -219,6 +259,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_min_delay_obj,
 STATIC mp_obj_t gifio_ondiskgif_obj_get_max_delay(mp_obj_t self_in) {
     gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
 
+    check_for_deinit(self);
     return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_max_delay(self) / 1000);
 }
 
@@ -227,7 +268,21 @@ MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_get_max_delay_obj, gifio_ondiskgif_obj
 MP_PROPERTY_GETTER(gifio_ondiskgif_max_delay_obj,
     (mp_obj_t)&gifio_ondiskgif_get_max_delay_obj);
 
+//|     def deinit(self) -> None:
+//|         """Release resources allocated by OnDiskGif."""
+//|         ...
+//|
+STATIC mp_obj_t gifio_ondiskgif_obj_deinit(mp_obj_t self_in) {
+    gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
+    common_hal_gifio_ondiskgif_deinit(self);
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_deinit_obj, gifio_ondiskgif_obj_deinit);
+
 STATIC const mp_rom_map_elem_t gifio_ondiskgif_locals_dict_table[] = {
+    { MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&gifio_ondiskgif_deinit_obj) },
+    { MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&default___enter___obj) },
+    { MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&gifio_ondiskgif___exit___obj) },
     { MP_ROM_QSTR(MP_QSTR_height), MP_ROM_PTR(&gifio_ondiskgif_height_obj) },
     { MP_ROM_QSTR(MP_QSTR_bitmap), MP_ROM_PTR(&gifio_ondiskgif_bitmap_obj) },
     { MP_ROM_QSTR(MP_QSTR_width), MP_ROM_PTR(&gifio_ondiskgif_width_obj) },
diff --git a/shared-bindings/gifio/OnDiskGif.h b/shared-bindings/gifio/OnDiskGif.h
@@ -45,4 +45,6 @@ int32_t common_hal_gifio_ondiskgif_get_duration(gifio_ondiskgif_t *self);
 int32_t common_hal_gifio_ondiskgif_get_frame_count(gifio_ondiskgif_t *self);
 int32_t common_hal_gifio_ondiskgif_get_min_delay(gifio_ondiskgif_t *self);
 int32_t common_hal_gifio_ondiskgif_get_max_delay(gifio_ondiskgif_t *self);
+void common_hal_gifio_ondiskgif_deinit(gifio_ondiskgif_t *self);
+bool common_hal_gifio_ondiskgif_deinited(gifio_ondiskgif_t *self);
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_ONDISKGIF_H
diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c
@@ -29,6 +29,7 @@
 #include <string.h>
 
 #include "py/runtime.h"
+#include "py/gc.h"
 
 enum { ALIGN_BITS = 8 * sizeof(uint32_t) };
 
@@ -48,8 +49,10 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self,
     self->width = width;
     self->height = height;
     self->stride = stride(width, bits_per_value);
+    self->data_alloc = false;
     if (!data) {
         data = m_malloc(self->stride * height * sizeof(uint32_t), false);
+        self->data_alloc = true;
     }
     self->data = data;
     self->read_only = read_only;
@@ -79,6 +82,16 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self,
     self->dirty_area.y2 = height;
 }
 
+void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self) {
+    if (self->data_alloc) {
+        gc_free(self->data);
+    }
+    self->data = NULL;
+}
+
+bool common_hal_displayio_bitmap_deinited(displayio_bitmap_t *self) {
+    return self->data == NULL;
+}
 
 uint16_t common_hal_displayio_bitmap_get_height(displayio_bitmap_t *self) {
     return self->height;
diff --git a/shared-module/displayio/Bitmap.h b/shared-module/displayio/Bitmap.h
@@ -45,6 +45,7 @@ typedef struct {
     displayio_area_t dirty_area;
     uint16_t bitmask;
     bool read_only;
+    bool data_alloc; // did bitmap allocate data or someone else
 } displayio_bitmap_t;
 
 void displayio_bitmap_finish_refresh(displayio_bitmap_t *self);
diff --git a/shared-module/gifio/OnDiskGif.c b/shared-module/gifio/OnDiskGif.c
@@ -32,6 +32,7 @@
 #include "py/mperrno.h"
 #include "py/runtime.h"
 
+
 static int32_t GIFReadFile(GIFFILE *pFile, uint8_t *pBuf, int32_t iLen) {
     // mp_printf(&mp_plat_print, "GifReadFile len %d ", iLen);
     uint32_t iBytesRead;
@@ -153,8 +154,16 @@ void common_hal_gifio_ondiskgif_construct(gifio_ondiskgif_t *self, pyb_file_obj_
     self->frame_count = info.iFrameCount;
     self->min_delay = info.iMinDelay;
     self->max_delay = info.iMaxDelay;
+}
+
+void common_hal_gifio_ondiskgif_deinit(gifio_ondiskgif_t *self) {
+    self->file = NULL;
+    common_hal_displayio_bitmap_deinit(self->bitmap);
+    self->bitmap = NULL;
+}
 
-    // mp_printf(&mp_plat_print, "GIF_init returned %d %x\n", result, bitmap->data);
+bool common_hal_gifio_ondiskgif_deinited(gifio_ondiskgif_t *self) {
+    return self->bitmap == NULL;
 }
 
 uint16_t common_hal_gifio_ondiskgif_get_height(gifio_ondiskgif_t *self) {
