From 7679db36072cfbe35b59a535a7f1afb58c9f7dad Mon Sep 17 00:00:00 2001 From: Thomas Farstrike Date: Tue, 25 Nov 2025 14:28:09 +0100 Subject: [PATCH] Refactor camera code: DRY, fix memory leak, improve error handling - Extract RGB conversion helper to eliminate duplication - Unify save functions - Fix buffer cleanup on error (memory leak) - Move retry logic into init_internal_cam() - Add error handling for failed camera reinitialization - Consolidate button sizing with class variables - Remove redundant initialization and unnecessary copies --- c_mpos/src/webcam.c | 92 +++++++------------ .../assets/camera_app.py | 69 ++++++++------ 2 files changed, 76 insertions(+), 85 deletions(-) diff --git a/c_mpos/src/webcam.c b/c_mpos/src/webcam.c index f1c71adf..83f08c31 100644 --- a/c_mpos/src/webcam.c +++ b/c_mpos/src/webcam.c @@ -31,6 +31,25 @@ typedef struct _webcam_obj_t { int height; // Resolution height } webcam_obj_t; +// Helper function to convert single YUV pixel to RGB565 +static inline uint16_t yuv_to_rgb565(int y_val, int u, int v) { + int c = y_val - 16; + int d = u - 128; + int e = v - 128; + + int r = (298 * c + 409 * e + 128) >> 8; + int g = (298 * c - 100 * d - 208 * e + 128) >> 8; + int b = (298 * c + 516 * d + 128) >> 8; + + // Clamp to valid range + r = r < 0 ? 0 : (r > 255 ? 255 : r); + g = g < 0 ? 0 : (g > 255 ? 255 : g); + b = b < 0 ? 0 : (b > 255 ? 255 : b); + + // Convert to RGB565 + return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3); +} + static void yuyv_to_rgb565(unsigned char *yuyv, uint16_t *rgb565, int width, int height) { // Convert YUYV to RGB565 without scaling // YUYV format: Y0 U Y1 V (4 bytes for 2 pixels, chroma shared) @@ -45,43 +64,9 @@ static void yuyv_to_rgb565(unsigned char *yuyv, uint16_t *rgb565, int width, int int y1 = yuyv[base_index + 2]; int v = yuyv[base_index + 3]; - // YUV to RGB conversion (ITU-R BT.601) for first pixel - int c = y0 - 16; - int d = u - 128; - int e = v - 128; - - int r = (298 * c + 409 * e + 128) >> 8; - int g = (298 * c - 100 * d - 208 * e + 128) >> 8; - int b = (298 * c + 516 * d + 128) >> 8; - - // Clamp to valid range - r = r < 0 ? 0 : (r > 255 ? 255 : r); - g = g < 0 ? 0 : (g > 255 ? 255 : g); - b = b < 0 ? 0 : (b > 255 ? 255 : b); - - // Convert to RGB565 - uint16_t r5 = (r >> 3) & 0x1F; - uint16_t g6 = (g >> 2) & 0x3F; - uint16_t b5 = (b >> 3) & 0x1F; - - rgb565[y * width + x] = (r5 << 11) | (g6 << 5) | b5; - - // Second pixel (shares U/V with first) - c = y1 - 16; - - r = (298 * c + 409 * e + 128) >> 8; - g = (298 * c - 100 * d - 208 * e + 128) >> 8; - b = (298 * c + 516 * d + 128) >> 8; - - r = r < 0 ? 0 : (r > 255 ? 255 : r); - g = g < 0 ? 0 : (g > 255 ? 255 : g); - b = b < 0 ? 0 : (b > 255 ? 255 : b); - - r5 = (r >> 3) & 0x1F; - g6 = (g >> 2) & 0x3F; - b5 = (b >> 3) & 0x1F; - - rgb565[y * width + x + 1] = (r5 << 11) | (g6 << 5) | b5; + // Convert both pixels (sharing U/V chroma) + rgb565[y * width + x] = yuv_to_rgb565(y0, u, v); + rgb565[y * width + x + 1] = yuv_to_rgb565(y1, u, v); } } } @@ -98,23 +83,13 @@ static void yuyv_to_grayscale(unsigned char *yuyv, unsigned char *gray, int widt } } -static void save_raw(const char *filename, unsigned char *data, int width, int height) { +static void save_raw_generic(const char *filename, void *data, size_t elem_size, int width, int height) { FILE *fp = fopen(filename, "wb"); if (!fp) { WEBCAM_DEBUG_PRINT("Cannot open file %s: %s\n", filename, strerror(errno)); return; } - fwrite(data, 1, width * height, fp); - fclose(fp); -} - -static void save_raw_rgb565(const char *filename, uint16_t *data, int width, int height) { - FILE *fp = fopen(filename, "wb"); - if (!fp) { - WEBCAM_DEBUG_PRINT("Cannot open file %s: %s\n", filename, strerror(errno)); - return; - } - fwrite(data, sizeof(uint16_t), width * height, fp); + fwrite(data, elem_size, width * height, fp); fclose(fp); } @@ -162,6 +137,10 @@ static int init_webcam(webcam_obj_t *self, const char *device, int width, int he buf.index = i; if (ioctl(self->fd, VIDIOC_QUERYBUF, &buf) < 0) { WEBCAM_DEBUG_PRINT("Cannot query buffer: %s\n", strerror(errno)); + // Unmap any already-mapped buffers + for (int j = 0; j < i; j++) { + munmap(self->buffers[j], self->buffer_length); + } close(self->fd); return -errno; } @@ -169,6 +148,10 @@ static int init_webcam(webcam_obj_t *self, const char *device, int width, int he self->buffers[i] = mmap(NULL, buf.length, PROT_READ | PROT_WRITE, MAP_SHARED, self->fd, buf.m.offset); if (self->buffers[i] == MAP_FAILED) { WEBCAM_DEBUG_PRINT("Cannot map buffer: %s\n", strerror(errno)); + // Unmap any already-mapped buffers + for (int j = 0; j < i; j++) { + munmap(self->buffers[j], self->buffer_length); + } close(self->fd); return -errno; } @@ -301,10 +284,6 @@ static mp_obj_t webcam_init(size_t n_args, const mp_obj_t *pos_args, mp_map_t *k webcam_obj_t *self = m_new_obj(webcam_obj_t); self->base.type = &webcam_type; self->fd = -1; - self->gray_buffer = NULL; - self->rgb565_buffer = NULL; - self->width = 0; // Will be set from V4L2 format in init_webcam - self->height = 0; // Will be set from V4L2 format in init_webcam int res = init_webcam(self, device, width, height); if (res < 0) { @@ -380,13 +359,10 @@ static mp_obj_t webcam_reconfigure(size_t n_args, const mp_obj_t *pos_args, mp_m WEBCAM_DEBUG_PRINT("Reconfiguring webcam: %dx%d -> %dx%d\n", self->width, self->height, new_width, new_height); - // Remember device path before deinit (which closes fd) - char device[64]; - strncpy(device, self->device, sizeof(device)); - // Clean shutdown and reinitialize with new resolution + // Note: deinit_webcam doesn't touch self->device, so it's safe to use directly deinit_webcam(self); - int res = init_webcam(self, device, new_width, new_height); + int res = init_webcam(self, self->device, new_width, new_height); if (res < 0) { mp_raise_OSError(-res); diff --git a/internal_filesystem/apps/com.micropythonos.camera/assets/camera_app.py b/internal_filesystem/apps/com.micropythonos.camera/assets/camera_app.py index 0bb080fa..6ae0d6ea 100644 --- a/internal_filesystem/apps/com.micropythonos.camera/assets/camera_app.py +++ b/internal_filesystem/apps/com.micropythonos.camera/assets/camera_app.py @@ -20,6 +20,7 @@ import mpos.time class CameraApp(Activity): button_width = 40 + button_height = 40 width = 320 height = 240 @@ -71,7 +72,7 @@ class CameraApp(Activity): # Initialize LVGL image widget self.create_preview_image() close_button = lv.button(self.main_screen) - close_button.set_size(self.button_width,40) + close_button.set_size(self.button_width, self.button_height) close_button.align(lv.ALIGN.TOP_RIGHT, 0, 0) close_label = lv.label(close_button) close_label.set_text(lv.SYMBOL.CLOSE) @@ -79,15 +80,15 @@ class CameraApp(Activity): close_button.add_event_cb(lambda e: self.finish(),lv.EVENT.CLICKED,None) # Settings button settings_button = lv.button(self.main_screen) - settings_button.set_size(self.button_width,40) - settings_button.align(lv.ALIGN.TOP_RIGHT, 0, 50) + settings_button.set_size(self.button_width, self.button_height) + settings_button.align(lv.ALIGN.TOP_RIGHT, 0, self.button_height + 10) settings_label = lv.label(settings_button) settings_label.set_text(lv.SYMBOL.SETTINGS) settings_label.center() settings_button.add_event_cb(lambda e: self.open_settings(),lv.EVENT.CLICKED,None) self.snap_button = lv.button(self.main_screen) - self.snap_button.set_size(self.button_width, 40) + self.snap_button.set_size(self.button_width, self.button_height) self.snap_button.align(lv.ALIGN.RIGHT_MID, 0, 0) self.snap_button.add_flag(lv.obj.FLAG.HIDDEN) self.snap_button.add_event_cb(self.snap_button_click,lv.EVENT.CLICKED,None) @@ -95,7 +96,7 @@ class CameraApp(Activity): snap_label.set_text(lv.SYMBOL.OK) snap_label.center() self.qr_button = lv.button(self.main_screen) - self.qr_button.set_size(self.button_width, 40) + self.qr_button.set_size(self.button_width, self.button_height) self.qr_button.add_flag(lv.obj.FLAG.HIDDEN) self.qr_button.align(lv.ALIGN.BOTTOM_RIGHT, 0, 0) self.qr_button.add_event_cb(self.qr_button_click,lv.EVENT.CLICKED,None) @@ -121,9 +122,6 @@ class CameraApp(Activity): def onResume(self, screen): self.cam = init_internal_cam(self.width, self.height) - if not self.cam: - # try again because the manual i2c poweroff leaves it in a bad state - self.cam = init_internal_cam(self.width, self.height) if self.cam: self.image.set_rotation(900) # internal camera is rotated 90 degrees else: @@ -332,6 +330,11 @@ class CameraApp(Activity): if self.cam: self.capture_timer = lv.timer_create(self.try_capture, 100, None) print("Internal camera reinitialized, capture timer resumed") + else: + print("ERROR: Failed to reinitialize camera after resolution change") + self.status_label.set_text("Failed to reinitialize camera.\nPlease restart the app.") + self.status_label_cont.remove_flag(lv.obj.FLAG.HIDDEN) + return # Don't continue if camera failed self.set_image_size() @@ -364,8 +367,11 @@ class CameraApp(Activity): # Non-class functions: -def init_internal_cam(width=320, height=240): - """Initialize internal camera with specified resolution.""" +def init_internal_cam(width, height): + """Initialize internal camera with specified resolution. + + Automatically retries once if initialization fails (to handle I2C poweroff issue). + """ try: from camera import Camera, GrabMode, PixelFormat, FrameSize, GainCeiling @@ -394,23 +400,32 @@ def init_internal_cam(width=320, height=240): frame_size = resolution_map.get((width, height), FrameSize.QVGA) print(f"init_internal_cam: Using FrameSize for {width}x{height}") - cam = Camera( - data_pins=[12,13,15,11,14,10,7,2], - vsync_pin=6, - href_pin=4, - sda_pin=21, - scl_pin=16, - pclk_pin=9, - xclk_pin=8, - xclk_freq=20000000, - powerdown_pin=-1, - reset_pin=-1, - pixel_format=PixelFormat.RGB565, - frame_size=frame_size, - grab_mode=GrabMode.LATEST - ) - cam.set_vflip(True) - return cam + # Try to initialize, with one retry for I2C poweroff issue + for attempt in range(2): + try: + cam = Camera( + data_pins=[12,13,15,11,14,10,7,2], + vsync_pin=6, + href_pin=4, + sda_pin=21, + scl_pin=16, + pclk_pin=9, + xclk_pin=8, + xclk_freq=20000000, + powerdown_pin=-1, + reset_pin=-1, + pixel_format=PixelFormat.RGB565, + frame_size=frame_size, + grab_mode=GrabMode.LATEST + ) + cam.set_vflip(True) + return cam + except Exception as e: + if attempt == 0: + print(f"init_cam attempt {attempt + 1} failed: {e}, retrying...") + else: + print(f"init_cam exception: {e}") + return None except Exception as e: print(f"init_cam exception: {e}") return None