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
This commit is contained in:
Thomas Farstrike
2025-11-25 14:28:09 +01:00
parent 385d551f3d
commit 7679db3607
2 changed files with 76 additions and 85 deletions
+34 -58
View File
@@ -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);
@@ -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