Skip to content

Commit db76fbd

Browse files
dhalbertdhalbert
authored
Merge pull request adafruit#7712 from gamblor21/ondiskgif_freemem_fix
Add deinit to OnDiskGif
2 parents 2492fb0 + 52631bb commit db76fbd

File tree

8 files changed

+124
-15
lines changed

8 files changed

+124
-15
lines changed

‎ports/atmel-samd/Makefile‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ INC += -I. \
5959
ifeq ($(CHIP_FAMILY), samd21)
6060
PERIPHERALS_CHIP_FAMILY=samd21
6161
OPTIMIZATION_FLAGS ?= -Os
62+
6263
# TinyUSB defines
6364
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
6465
endif
@@ -101,24 +102,26 @@ ifeq ($(DEBUG), 1)
101102
endif
102103
else
103104
CFLAGS += -DNDEBUG
104-
# -finline-limit can shrink the image size.
105-
# -finline-limit=80 or so is similar to not having it on.
106-
# There is no simple default value, though.
107105

108-
# Do a default shrink for small builds.
109-
ifndef CFLAGS_INLINE_LIMIT
110-
ifeq ($(CIRCUITPY_FULL_BUILD),0)
111-
CFLAGS_INLINE_LIMIT = 50
106+
# Do a default shrink for small builds, including all SAMD21 builds.
107+
ifeq ($(CIRCUITPY_FULL_BUILD),0)
108+
SHRINK_BUILD = 1
109+
else
110+
ifeq ($(CHIP_FAMILY), samd21)
111+
SHRINK_BUILD = 1
112112
endif
113113
endif
114114

115-
ifdef CFLAGS_INLINE_LIMIT
116-
CFLAGS += -finline-limit=$(CFLAGS_INLINE_LIMIT)
115+
# -finline-limit can shrink the image size.
116+
# -finline-limit=80 or so is similar to not having it on.
117+
# There is no simple default value, though.
118+
ifeq ($(SHRINK_BUILD), 1)
119+
CFLAGS += -finline-limit=45
117120
endif
118121

119-
ifeq ($(CIRCUITPY_FULL_BUILD),0)
120-
CFLAGS += --param inline-unit-growth=15 --param max-inline-insns-auto=20
121-
endif
122+
# We used to do this but it seems to not reduce space any more, at least in gcc 11.
123+
# Leave it here, commented out, just for reference.
124+
# --param inline-unit-growth=15 --param max-inline-insns-auto=20
122125

123126
ifdef CFLAGS_BOARD
124127
CFLAGS += $(CFLAGS_BOARD)

‎shared-bindings/displayio/Bitmap.c‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,19 @@ STATIC mp_obj_t displayio_bitmap_make_new(const mp_obj_type_t *type, size_t n_ar
8484

8585
return MP_OBJ_FROM_PTR(self);
8686
}
87+
88+
STATIC void check_for_deinit(displayio_bitmap_t *self) {
89+
if (common_hal_displayio_bitmap_deinited(self)) {
90+
raise_deinited_error();
91+
}
92+
}
93+
8794
//| width: int
8895
//| """Width of the bitmap. (read only)"""
8996
STATIC mp_obj_t displayio_bitmap_obj_get_width(mp_obj_t self_in) {
9097
displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
9198

99+
check_for_deinit(self);
92100
return MP_OBJ_NEW_SMALL_INT(common_hal_displayio_bitmap_get_width(self));
93101
}
94102

@@ -102,6 +110,7 @@ MP_PROPERTY_GETTER(displayio_bitmap_width_obj,
102110
STATIC mp_obj_t displayio_bitmap_obj_get_height(mp_obj_t self_in) {
103111
displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
104112

113+
check_for_deinit(self);
105114
return MP_OBJ_NEW_SMALL_INT(common_hal_displayio_bitmap_get_height(self));
106115
}
107116

@@ -134,6 +143,7 @@ STATIC mp_obj_t bitmap_subscr(mp_obj_t self_in, mp_obj_t index_obj, mp_obj_t val
134143
}
135144

136145
displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
146+
check_for_deinit(self);
137147

138148
if (mp_obj_is_type(index_obj, &mp_type_slice)) {
139149
// 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
214224
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
215225

216226
displayio_bitmap_t *self = MP_OBJ_TO_PTR(pos_args[0]);
227+
check_for_deinit(self);
217228

218229
int16_t x = args[ARG_x].u_int;
219230
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
288299
//| ...
289300
STATIC mp_obj_t displayio_bitmap_obj_fill(mp_obj_t self_in, mp_obj_t value_obj) {
290301
displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
302+
check_for_deinit(self);
291303

292304
mp_uint_t value = (mp_uint_t)mp_obj_get_int(value_obj);
293305
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);
318330
//| notified of the "dirty rectangle" that encloses all modified
319331
//| pixels."""
320332
//| ...
321-
//|
322333
STATIC mp_obj_t displayio_bitmap_obj_dirty(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
323334
displayio_bitmap_t *self = MP_OBJ_TO_PTR(pos_args[0]);
335+
check_for_deinit(self);
336+
324337
enum { ARG_x1, ARG_y1, ARG_x2, ARG_y2 };
325338
static const mp_arg_t allowed_args[] = {
326339
{ 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
344357
}
345358
MP_DEFINE_CONST_FUN_OBJ_KW(displayio_bitmap_dirty_obj, 0, displayio_bitmap_obj_dirty);
346359

360+
//| def deinit(self) -> None:
361+
//| """Release resources allocated by Bitmap."""
362+
//| ...
363+
//|
364+
STATIC mp_obj_t displayio_bitmap_obj_deinit(mp_obj_t self_in) {
365+
displayio_bitmap_t *self = MP_OBJ_TO_PTR(self_in);
366+
common_hal_displayio_bitmap_deinit(self);
367+
return mp_const_none;
368+
}
369+
MP_DEFINE_CONST_FUN_OBJ_1(displayio_bitmap_deinit_obj, displayio_bitmap_obj_deinit);
370+
347371
STATIC const mp_rom_map_elem_t displayio_bitmap_locals_dict_table[] = {
348372
{ MP_ROM_QSTR(MP_QSTR_height), MP_ROM_PTR(&displayio_bitmap_height_obj) },
349373
{ MP_ROM_QSTR(MP_QSTR_width), MP_ROM_PTR(&displayio_bitmap_width_obj) },
350374
{ MP_ROM_QSTR(MP_QSTR_blit), MP_ROM_PTR(&displayio_bitmap_blit_obj) },
351375
{ MP_ROM_QSTR(MP_QSTR_fill), MP_ROM_PTR(&displayio_bitmap_fill_obj) },
352376
{ MP_ROM_QSTR(MP_QSTR_dirty), MP_ROM_PTR(&displayio_bitmap_dirty_obj) },
353-
377+
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&displayio_bitmap_deinit_obj) },
354378
};
355379
STATIC MP_DEFINE_CONST_DICT(displayio_bitmap_locals_dict, displayio_bitmap_locals_dict_table);
356380

‎shared-bindings/displayio/Bitmap.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,7 @@ void common_hal_displayio_bitmap_blit(displayio_bitmap_t *self, int16_t x, int16
4848
uint32_t common_hal_displayio_bitmap_get_pixel(displayio_bitmap_t *bitmap, int16_t x, int16_t y);
4949
void common_hal_displayio_bitmap_fill(displayio_bitmap_t *bitmap, uint32_t value);
5050
int common_hal_displayio_bitmap_get_buffer(displayio_bitmap_t *self, mp_buffer_info_t *bufinfo, mp_uint_t flags);
51+
void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self);
52+
bool common_hal_displayio_bitmap_deinited(displayio_bitmap_t *self);
5153

5254
#endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_BITMAP_H

‎shared-bindings/gifio/OnDiskGif.c‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
#include "py/runtime.h"
3232
#include "py/objproperty.h"
33+
#include "shared/runtime/context_manager_helpers.h"
34+
#include "shared-bindings/util.h"
3335
#include "supervisor/shared/translate/translate.h"
3436
#include "shared-bindings/gifio/OnDiskGif.h"
3537

@@ -96,6 +98,14 @@
9698
//| display_bus.send(42, struct.pack(">hh", 0, odg.bitmap.width - 1))
9799
//| display_bus.send(43, struct.pack(">hh", 0, odg.bitmap.height - 1))
98100
//| display_bus.send(44, d.bitmap)
101+
//|
102+
//| # The following optional code will free the OnDiskGif and allocated resources
103+
//| # after use. This may be required before loading a new GIF in situations
104+
//| # where RAM is limited and the first GIF took most of the RAM.
105+
//| odg.deinit()
106+
//| odg = None
107+
//| gc.collect()
108+
//|
99109
//| """
100110
//|
101111
//| 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
125135
return MP_OBJ_FROM_PTR(self);
126136
}
127137

138+
STATIC void check_for_deinit(gifio_ondiskgif_t *self) {
139+
if (common_hal_gifio_ondiskgif_deinited(self)) {
140+
raise_deinited_error();
141+
}
142+
}
143+
144+
//| def __enter__(self) -> OnDiskGif:
145+
//| """No-op used by Context Managers."""
146+
//| ...
147+
// Provided by context manager helper.
148+
149+
//| def __exit__(self) -> None:
150+
//| """Automatically deinitializes the GIF when exiting a context. See
151+
//| :ref:`lifetime-and-contextmanagers` for more info."""
152+
//| ...
153+
STATIC mp_obj_t gifio_ondiskgif_obj___exit__(size_t n_args, const mp_obj_t *args) {
154+
(void)n_args;
155+
common_hal_gifio_ondiskgif_deinit(args[0]);
156+
return mp_const_none;
157+
}
158+
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(gifio_ondiskgif___exit___obj, 4, 4, gifio_ondiskgif_obj___exit__);
159+
128160
//| width: int
129161
//| """Width of the gif. (read only)"""
130162
STATIC mp_obj_t gifio_ondiskgif_obj_get_width(mp_obj_t self_in) {
131163
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
132164

165+
check_for_deinit(self);
133166
return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_width(self));
134167
}
135168

@@ -143,6 +176,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_width_obj,
143176
STATIC mp_obj_t gifio_ondiskgif_obj_get_height(mp_obj_t self_in) {
144177
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
145178

179+
check_for_deinit(self);
146180
return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_height(self));
147181
}
148182

@@ -155,6 +189,8 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_height_obj,
155189
//| """The bitmap used to hold the current frame."""
156190
STATIC mp_obj_t gifio_ondiskgif_obj_get_bitmap(mp_obj_t self_in) {
157191
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
192+
193+
check_for_deinit(self);
158194
return common_hal_gifio_ondiskgif_get_bitmap(self);
159195
}
160196

@@ -168,6 +204,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_bitmap_obj,
168204
STATIC mp_obj_t gifio_ondiskgif_obj_next_frame(mp_obj_t self_in) {
169205
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
170206

207+
check_for_deinit(self);
171208
return mp_obj_new_float((float)common_hal_gifio_ondiskgif_next_frame(self, true) / 1000);
172209
}
173210

@@ -179,6 +216,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_next_frame_obj, gifio_ondiskgif_obj_ne
179216
STATIC mp_obj_t gifio_ondiskgif_obj_get_duration(mp_obj_t self_in) {
180217
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
181218

219+
check_for_deinit(self);
182220
return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_duration(self) / 1000);
183221
}
184222

@@ -192,6 +230,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_duration_obj,
192230
STATIC mp_obj_t gifio_ondiskgif_obj_get_frame_count(mp_obj_t self_in) {
193231
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
194232

233+
check_for_deinit(self);
195234
return MP_OBJ_NEW_SMALL_INT(common_hal_gifio_ondiskgif_get_frame_count(self));
196235
}
197236

@@ -205,6 +244,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_frame_count_obj,
205244
STATIC mp_obj_t gifio_ondiskgif_obj_get_min_delay(mp_obj_t self_in) {
206245
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
207246

247+
check_for_deinit(self);
208248
return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_min_delay(self) / 1000);
209249
}
210250

@@ -219,6 +259,7 @@ MP_PROPERTY_GETTER(gifio_ondiskgif_min_delay_obj,
219259
STATIC mp_obj_t gifio_ondiskgif_obj_get_max_delay(mp_obj_t self_in) {
220260
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
221261

262+
check_for_deinit(self);
222263
return mp_obj_new_float((float)common_hal_gifio_ondiskgif_get_max_delay(self) / 1000);
223264
}
224265

@@ -227,7 +268,21 @@ MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_get_max_delay_obj, gifio_ondiskgif_obj
227268
MP_PROPERTY_GETTER(gifio_ondiskgif_max_delay_obj,
228269
(mp_obj_t)&gifio_ondiskgif_get_max_delay_obj);
229270

271+
//| def deinit(self) -> None:
272+
//| """Release resources allocated by OnDiskGif."""
273+
//| ...
274+
//|
275+
STATIC mp_obj_t gifio_ondiskgif_obj_deinit(mp_obj_t self_in) {
276+
gifio_ondiskgif_t *self = MP_OBJ_TO_PTR(self_in);
277+
common_hal_gifio_ondiskgif_deinit(self);
278+
return mp_const_none;
279+
}
280+
MP_DEFINE_CONST_FUN_OBJ_1(gifio_ondiskgif_deinit_obj, gifio_ondiskgif_obj_deinit);
281+
230282
STATIC const mp_rom_map_elem_t gifio_ondiskgif_locals_dict_table[] = {
283+
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&gifio_ondiskgif_deinit_obj) },
284+
{ MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&default___enter___obj) },
285+
{ MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&gifio_ondiskgif___exit___obj) },
231286
{ MP_ROM_QSTR(MP_QSTR_height), MP_ROM_PTR(&gifio_ondiskgif_height_obj) },
232287
{ MP_ROM_QSTR(MP_QSTR_bitmap), MP_ROM_PTR(&gifio_ondiskgif_bitmap_obj) },
233288
{ MP_ROM_QSTR(MP_QSTR_width), MP_ROM_PTR(&gifio_ondiskgif_width_obj) },

‎shared-bindings/gifio/OnDiskGif.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,6 @@ int32_t common_hal_gifio_ondiskgif_get_duration(gifio_ondiskgif_t *self);
4545
int32_t common_hal_gifio_ondiskgif_get_frame_count(gifio_ondiskgif_t *self);
4646
int32_t common_hal_gifio_ondiskgif_get_min_delay(gifio_ondiskgif_t *self);
4747
int32_t common_hal_gifio_ondiskgif_get_max_delay(gifio_ondiskgif_t *self);
48+
void common_hal_gifio_ondiskgif_deinit(gifio_ondiskgif_t *self);
49+
bool common_hal_gifio_ondiskgif_deinited(gifio_ondiskgif_t *self);
4850
#endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_ONDISKGIF_H

‎shared-module/displayio/Bitmap.c‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <string.h>
3030

3131
#include "py/runtime.h"
32+
#include "py/gc.h"
3233

3334
enum { ALIGN_BITS = 8 * sizeof(uint32_t) };
3435

@@ -48,8 +49,10 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self,
4849
self->width = width;
4950
self->height = height;
5051
self->stride = stride(width, bits_per_value);
52+
self->data_alloc = false;
5153
if (!data) {
5254
data = m_malloc(self->stride * height * sizeof(uint32_t), false);
55+
self->data_alloc = true;
5356
}
5457
self->data = data;
5558
self->read_only = read_only;
@@ -79,6 +82,16 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self,
7982
self->dirty_area.y2 = height;
8083
}
8184

85+
void common_hal_displayio_bitmap_deinit(displayio_bitmap_t *self) {
86+
if (self->data_alloc) {
87+
gc_free(self->data);
88+
}
89+
self->data = NULL;
90+
}
91+
92+
bool common_hal_displayio_bitmap_deinited(displayio_bitmap_t *self) {
93+
return self->data == NULL;
94+
}
8295

8396
uint16_t common_hal_displayio_bitmap_get_height(displayio_bitmap_t *self) {
8497
return self->height;

‎shared-module/displayio/Bitmap.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ typedef struct {
4545
displayio_area_t dirty_area;
4646
uint16_t bitmask;
4747
bool read_only;
48+
bool data_alloc; // did bitmap allocate data or someone else
4849
} displayio_bitmap_t;
4950

5051
void displayio_bitmap_finish_refresh(displayio_bitmap_t *self);

‎shared-module/gifio/OnDiskGif.c‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "py/mperrno.h"
3333
#include "py/runtime.h"
3434

35+
3536
static int32_t GIFReadFile(GIFFILE *pFile, uint8_t *pBuf, int32_t iLen) {
3637
// mp_printf(&mp_plat_print, "GifReadFile len %d ", iLen);
3738
uint32_t iBytesRead;
@@ -153,8 +154,16 @@ void common_hal_gifio_ondiskgif_construct(gifio_ondiskgif_t *self, pyb_file_obj_
153154
self->frame_count = info.iFrameCount;
154155
self->min_delay = info.iMinDelay;
155156
self->max_delay = info.iMaxDelay;
157+
}
158+
159+
void common_hal_gifio_ondiskgif_deinit(gifio_ondiskgif_t *self) {
160+
self->file = NULL;
161+
common_hal_displayio_bitmap_deinit(self->bitmap);
162+
self->bitmap = NULL;
163+
}
156164

157-
// mp_printf(&mp_plat_print, "GIF_init returned %d %x\n", result, bitmap->data);
165+
bool common_hal_gifio_ondiskgif_deinited(gifio_ondiskgif_t *self) {
166+
return self->bitmap == NULL;
158167
}
159168

160169
uint16_t common_hal_gifio_ondiskgif_get_height(gifio_ondiskgif_t *self) {

0 commit comments

Comments
 (0)