Skip to content

Commit 7679db3

Browse files
ThomasFarstrikeThomasFarstrike
committed
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
1 parent 385d551 commit 7679db3

File tree

2 files changed

+76
-85
lines changed

2 files changed

+76
-85
lines changed

‎c_mpos/src/webcam.c‎

Lines changed: 34 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,25 @@ typedef struct _webcam_obj_t {
3131
int height; // Resolution height
3232
} webcam_obj_t;
3333

34+
// Helper function to convert single YUV pixel to RGB565
35+
static inline uint16_t yuv_to_rgb565(int y_val, int u, int v) {
36+
int c = y_val - 16;
37+
int d = u - 128;
38+
int e = v - 128;
39+
40+
int r = (298 * c + 409 * e + 128) >> 8;
41+
int g = (298 * c - 100 * d - 208 * e + 128) >> 8;
42+
int b = (298 * c + 516 * d + 128) >> 8;
43+
44+
// Clamp to valid range
45+
r = r < 0 ? 0 : (r > 255 ? 255 : r);
46+
g = g < 0 ? 0 : (g > 255 ? 255 : g);
47+
b = b < 0 ? 0 : (b > 255 ? 255 : b);
48+
49+
// Convert to RGB565
50+
return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3);
51+
}
52+
3453
static void yuyv_to_rgb565(unsigned char *yuyv, uint16_t *rgb565, int width, int height) {
3554
// Convert YUYV to RGB565 without scaling
3655
// 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
4564
int y1 = yuyv[base_index + 2];
4665
int v = yuyv[base_index + 3];
4766

48-
// YUV to RGB conversion (ITU-R BT.601) for first pixel
49-
int c = y0 - 16;
50-
int d = u - 128;
51-
int e = v - 128;
52-
53-
int r = (298 * c + 409 * e + 128) >> 8;
54-
int g = (298 * c - 100 * d - 208 * e + 128) >> 8;
55-
int b = (298 * c + 516 * d + 128) >> 8;
56-
57-
// Clamp to valid range
58-
r = r < 0 ? 0 : (r > 255 ? 255 : r);
59-
g = g < 0 ? 0 : (g > 255 ? 255 : g);
60-
b = b < 0 ? 0 : (b > 255 ? 255 : b);
61-
62-
// Convert to RGB565
63-
uint16_t r5 = (r >> 3) & 0x1F;
64-
uint16_t g6 = (g >> 2) & 0x3F;
65-
uint16_t b5 = (b >> 3) & 0x1F;
66-
67-
rgb565[y * width + x] = (r5 << 11) | (g6 << 5) | b5;
68-
69-
// Second pixel (shares U/V with first)
70-
c = y1 - 16;
71-
72-
r = (298 * c + 409 * e + 128) >> 8;
73-
g = (298 * c - 100 * d - 208 * e + 128) >> 8;
74-
b = (298 * c + 516 * d + 128) >> 8;
75-
76-
r = r < 0 ? 0 : (r > 255 ? 255 : r);
77-
g = g < 0 ? 0 : (g > 255 ? 255 : g);
78-
b = b < 0 ? 0 : (b > 255 ? 255 : b);
79-
80-
r5 = (r >> 3) & 0x1F;
81-
g6 = (g >> 2) & 0x3F;
82-
b5 = (b >> 3) & 0x1F;
83-
84-
rgb565[y * width + x + 1] = (r5 << 11) | (g6 << 5) | b5;
67+
// Convert both pixels (sharing U/V chroma)
68+
rgb565[y * width + x] = yuv_to_rgb565(y0, u, v);
69+
rgb565[y * width + x + 1] = yuv_to_rgb565(y1, u, v);
8570
}
8671
}
8772
}
@@ -98,23 +83,13 @@ static void yuyv_to_grayscale(unsigned char *yuyv, unsigned char *gray, int widt
9883
}
9984
}
10085

101-
static void save_raw(const char *filename, unsigned char *data, int width, int height) {
86+
static void save_raw_generic(const char *filename, void *data, size_t elem_size, int width, int height) {
10287
FILE *fp = fopen(filename, "wb");
10388
if (!fp) {
10489
WEBCAM_DEBUG_PRINT("Cannot open file %s: %s\n", filename, strerror(errno));
10590
return;
10691
}
107-
fwrite(data, 1, width * height, fp);
108-
fclose(fp);
109-
}
110-
111-
static void save_raw_rgb565(const char *filename, uint16_t *data, int width, int height) {
112-
FILE *fp = fopen(filename, "wb");
113-
if (!fp) {
114-
WEBCAM_DEBUG_PRINT("Cannot open file %s: %s\n", filename, strerror(errno));
115-
return;
116-
}
117-
fwrite(data, sizeof(uint16_t), width * height, fp);
92+
fwrite(data, elem_size, width * height, fp);
11893
fclose(fp);
11994
}
12095

@@ -162,13 +137,21 @@ static int init_webcam(webcam_obj_t *self, const char *device, int width, int he
162137
buf.index = i;
163138
if (ioctl(self->fd, VIDIOC_QUERYBUF, &buf) < 0) {
164139
WEBCAM_DEBUG_PRINT("Cannot query buffer: %s\n", strerror(errno));
140+
// Unmap any already-mapped buffers
141+
for (int j = 0; j < i; j++) {
142+
munmap(self->buffers[j], self->buffer_length);
143+
}
165144
close(self->fd);
166145
return -errno;
167146
}
168147
self->buffer_length = buf.length;
169148
self->buffers[i] = mmap(NULL, buf.length, PROT_READ | PROT_WRITE, MAP_SHARED, self->fd, buf.m.offset);
170149
if (self->buffers[i] == MAP_FAILED) {
171150
WEBCAM_DEBUG_PRINT("Cannot map buffer: %s\n", strerror(errno));
151+
// Unmap any already-mapped buffers
152+
for (int j = 0; j < i; j++) {
153+
munmap(self->buffers[j], self->buffer_length);
154+
}
172155
close(self->fd);
173156
return -errno;
174157
}
@@ -301,10 +284,6 @@ static mp_obj_t webcam_init(size_t n_args, const mp_obj_t *pos_args, mp_map_t *k
301284
webcam_obj_t *self = m_new_obj(webcam_obj_t);
302285
self->base.type = &webcam_type;
303286
self->fd = -1;
304-
self->gray_buffer = NULL;
305-
self->rgb565_buffer = NULL;
306-
self->width = 0; // Will be set from V4L2 format in init_webcam
307-
self->height = 0; // Will be set from V4L2 format in init_webcam
308287

309288
int res = init_webcam(self, device, width, height);
310289
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
380359
WEBCAM_DEBUG_PRINT("Reconfiguring webcam: %dx%d -> %dx%d\n",
381360
self->width, self->height, new_width, new_height);
382361

383-
// Remember device path before deinit (which closes fd)
384-
char device[64];
385-
strncpy(device, self->device, sizeof(device));
386-
387362
// Clean shutdown and reinitialize with new resolution
363+
// Note: deinit_webcam doesn't touch self->device, so it's safe to use directly
388364
deinit_webcam(self);
389-
int res = init_webcam(self, device, new_width, new_height);
365+
int res = init_webcam(self, self->device, new_width, new_height);
390366

391367
if (res < 0) {
392368
mp_raise_OSError(-res);

‎internal_filesystem/apps/com.micropythonos.camera/assets/camera_app.py‎

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
class CameraApp(Activity):
2121

2222
button_width = 40
23+
button_height = 40
2324
width = 320
2425
height = 240
2526

@@ -71,31 +72,31 @@ def onCreate(self):
7172
# Initialize LVGL image widget
7273
self.create_preview_image()
7374
close_button = lv.button(self.main_screen)
74-
close_button.set_size(self.button_width,40)
75+
close_button.set_size(self.button_width, self.button_height)
7576
close_button.align(lv.ALIGN.TOP_RIGHT, 0, 0)
7677
close_label = lv.label(close_button)
7778
close_label.set_text(lv.SYMBOL.CLOSE)
7879
close_label.center()
7980
close_button.add_event_cb(lambda e: self.finish(),lv.EVENT.CLICKED,None)
8081
# Settings button
8182
settings_button = lv.button(self.main_screen)
82-
settings_button.set_size(self.button_width,40)
83-
settings_button.align(lv.ALIGN.TOP_RIGHT, 0, 50)
83+
settings_button.set_size(self.button_width, self.button_height)
84+
settings_button.align(lv.ALIGN.TOP_RIGHT, 0, self.button_height + 10)
8485
settings_label = lv.label(settings_button)
8586
settings_label.set_text(lv.SYMBOL.SETTINGS)
8687
settings_label.center()
8788
settings_button.add_event_cb(lambda e: self.open_settings(),lv.EVENT.CLICKED,None)
8889

8990
self.snap_button = lv.button(self.main_screen)
90-
self.snap_button.set_size(self.button_width, 40)
91+
self.snap_button.set_size(self.button_width, self.button_height)
9192
self.snap_button.align(lv.ALIGN.RIGHT_MID, 0, 0)
9293
self.snap_button.add_flag(lv.obj.FLAG.HIDDEN)
9394
self.snap_button.add_event_cb(self.snap_button_click,lv.EVENT.CLICKED,None)
9495
snap_label = lv.label(self.snap_button)
9596
snap_label.set_text(lv.SYMBOL.OK)
9697
snap_label.center()
9798
self.qr_button = lv.button(self.main_screen)
98-
self.qr_button.set_size(self.button_width, 40)
99+
self.qr_button.set_size(self.button_width, self.button_height)
99100
self.qr_button.add_flag(lv.obj.FLAG.HIDDEN)
100101
self.qr_button.align(lv.ALIGN.BOTTOM_RIGHT, 0, 0)
101102
self.qr_button.add_event_cb(self.qr_button_click,lv.EVENT.CLICKED,None)
@@ -121,9 +122,6 @@ def onCreate(self):
121122

122123
def onResume(self, screen):
123124
self.cam = init_internal_cam(self.width, self.height)
124-
if not self.cam:
125-
# try again because the manual i2c poweroff leaves it in a bad state
126-
self.cam = init_internal_cam(self.width, self.height)
127125
if self.cam:
128126
self.image.set_rotation(900) # internal camera is rotated 90 degrees
129127
else:
@@ -332,6 +330,11 @@ def handle_settings_result(self, result):
332330
if self.cam:
333331
self.capture_timer = lv.timer_create(self.try_capture, 100, None)
334332
print("Internal camera reinitialized, capture timer resumed")
333+
else:
334+
print("ERROR: Failed to reinitialize camera after resolution change")
335+
self.status_label.set_text("Failed to reinitialize camera.\nPlease restart the app.")
336+
self.status_label_cont.remove_flag(lv.obj.FLAG.HIDDEN)
337+
return # Don't continue if camera failed
335338

336339
self.set_image_size()
337340

@@ -364,8 +367,11 @@ def try_capture(self, event):
364367

365368

366369
# Non-class functions:
367-
def init_internal_cam(width=320, height=240):
368-
"""Initialize internal camera with specified resolution."""
370+
def init_internal_cam(width, height):
371+
"""Initialize internal camera with specified resolution.
372+
373+
Automatically retries once if initialization fails (to handle I2C poweroff issue).
374+
"""
369375
try:
370376
from camera import Camera, GrabMode, PixelFormat, FrameSize, GainCeiling
371377

@@ -394,23 +400,32 @@ def init_internal_cam(width=320, height=240):
394400
frame_size = resolution_map.get((width, height), FrameSize.QVGA)
395401
print(f"init_internal_cam: Using FrameSize for {width}x{height}")
396402

397-
cam = Camera(
398-
data_pins=[12,13,15,11,14,10,7,2],
399-
vsync_pin=6,
400-
href_pin=4,
401-
sda_pin=21,
402-
scl_pin=16,
403-
pclk_pin=9,
404-
xclk_pin=8,
405-
xclk_freq=20000000,
406-
powerdown_pin=-1,
407-
reset_pin=-1,
408-
pixel_format=PixelFormat.RGB565,
409-
frame_size=frame_size,
410-
grab_mode=GrabMode.LATEST
411-
)
412-
cam.set_vflip(True)
413-
return cam
403+
# Try to initialize, with one retry for I2C poweroff issue
404+
for attempt in range(2):
405+
try:
406+
cam = Camera(
407+
data_pins=[12,13,15,11,14,10,7,2],
408+
vsync_pin=6,
409+
href_pin=4,
410+
sda_pin=21,
411+
scl_pin=16,
412+
pclk_pin=9,
413+
xclk_pin=8,
414+
xclk_freq=20000000,
415+
powerdown_pin=-1,
416+
reset_pin=-1,
417+
pixel_format=PixelFormat.RGB565,
418+
frame_size=frame_size,
419+
grab_mode=GrabMode.LATEST
420+
)
421+
cam.set_vflip(True)
422+
return cam
423+
except Exception as e:
424+
if attempt == 0:
425+
print(f"init_cam attempt {attempt + 1} failed: {e}, retrying...")
426+
else:
427+
print(f"init_cam exception: {e}")
428+
return None
414429
except Exception as e:
415430
print(f"init_cam exception: {e}")
416431
return None

0 commit comments

Comments
 (0)