From f1947f134e35e6ec76641f4d9294a2b7236ef5b0 Mon Sep 17 00:00:00 2001 From: Benjamin Schaaf Date: Thu, 8 Apr 2021 00:16:17 +1000 Subject: [PATCH] Rework pipeline to allow for DMA support The process pipeline is now tasked with releasing the camera buffer, rather than being given a copy. This enables being able to avoid the copy in the future if it has a performance advantage. --- camera.c | 53 ++++++++++++++++++++++++++++------------- camera.h | 12 +++++----- io_pipeline.c | 33 +++++++++++++++----------- io_pipeline.h | 2 ++ pipeline.c | 30 ++++++++++++++++++++--- pipeline.h | 4 +++- process_pipeline.c | 58 +++++++++++++++++++++++++++------------------ process_pipeline.h | 3 ++- tools/camera_test.c | 31 ++++++++++++------------ 9 files changed, 146 insertions(+), 80 deletions(-) diff --git a/camera.c b/camera.c index 6e80781..f6fbf37 100644 --- a/camera.c +++ b/camera.c @@ -6,6 +6,7 @@ #include #include #include +#include #define MAX_VIDEO_BUFFERS 20 @@ -205,6 +206,7 @@ xioctl(int fd, int request, void *arg) struct video_buffer { uint32_t length; uint8_t *data; + int fd; }; struct _MPCamera { @@ -465,6 +467,17 @@ mp_camera_start_capture(MPCamera *camera) break; } + struct v4l2_exportbuffer expbuf = { + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, + .index = i, + }; + if (xioctl(camera->video_fd, VIDIOC_EXPBUF, &expbuf) == -1) { + errno_printerr("VIDIOC_EXPBUF"); + break; + } + + camera->buffers[i].fd = expbuf.fd; + ++camera->num_buffers; } @@ -510,6 +523,10 @@ error: -1) { errno_printerr("munmap"); } + + if (close(camera->buffers[i].fd) == -1) { + errno_printerr("close"); + } } // Reset allocated buffers @@ -543,6 +560,10 @@ mp_camera_stop_capture(MPCamera *camera) -1) { errno_printerr("munmap"); } + + if (close(camera->buffers[i].fd) == -1) { + errno_printerr("close"); + } } camera->num_buffers = 0; @@ -565,8 +586,7 @@ mp_camera_is_capturing(MPCamera *camera) } bool -mp_camera_capture_image(MPCamera *camera, void (*callback)(MPImage, void *), - void *user_data) +mp_camera_capture_buffer(MPCamera *camera, MPBuffer *buffer) { struct v4l2_buffer buf = {}; buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; @@ -606,24 +626,23 @@ mp_camera_capture_image(MPCamera *camera, void (*callback)(MPImage, void *), mp_pixel_format_width_to_bytes(pixel_format, width) * height); assert(bytesused == camera->buffers[buf.index].length); - MPImage image = { - .pixel_format = pixel_format, - .width = width, - .height = height, - .data = camera->buffers[buf.index].data, - }; + buffer->index = buf.index; + buffer->data = camera->buffers[buf.index].data; + buffer->fd = camera->buffers[buf.index].fd; - callback(image, user_data); + return true; +} - // The callback may have stopped the capture, only queue the buffer if we're - // still capturing. - if (mp_camera_is_capturing(camera)) { - if (xioctl(camera->video_fd, VIDIOC_QBUF, &buf) == -1) { - errno_printerr("VIDIOC_QBUF"); - return false; - } +bool mp_camera_release_buffer(MPCamera *camera, uint32_t buffer_index) +{ + struct v4l2_buffer buf = {}; + buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + buf.memory = V4L2_MEMORY_MMAP; + buf.index = buffer_index; + if (xioctl(camera->video_fd, VIDIOC_QBUF, &buf) == -1) { + errno_printerr("VIDIOC_QBUF"); + return false; } - return true; } diff --git a/camera.h b/camera.h index 3004e8e..323ac63 100644 --- a/camera.h +++ b/camera.h @@ -45,11 +45,11 @@ typedef struct { bool mp_camera_mode_is_equivalent(const MPCameraMode *m1, const MPCameraMode *m2); typedef struct { - uint32_t pixel_format; - uint32_t width; - uint32_t height; + uint32_t index; + uint8_t *data; -} MPImage; + int fd; +} MPBuffer; typedef struct _MPCamera MPCamera; @@ -67,8 +67,8 @@ bool mp_camera_set_mode(MPCamera *camera, MPCameraMode *mode); bool mp_camera_start_capture(MPCamera *camera); bool mp_camera_stop_capture(MPCamera *camera); bool mp_camera_is_capturing(MPCamera *camera); -bool mp_camera_capture_image(MPCamera *camera, void (*callback)(MPImage, void *), - void *user_data); +bool mp_camera_capture_buffer(MPCamera *camera, MPBuffer *buffer); +bool mp_camera_release_buffer(MPCamera *camera, uint32_t buffer_index); typedef struct _MPCameraModeList MPCameraModeList; diff --git a/io_pipeline.c b/io_pipeline.c index 980a722..1ebb93d 100644 --- a/io_pipeline.c +++ b/io_pipeline.c @@ -324,6 +324,19 @@ mp_io_pipeline_capture() mp_pipeline_invoke(pipeline, capture, NULL, 0); } +static void +release_buffer(MPPipeline *pipeline, const uint32_t *buffer_index) +{ + struct camera_info *info = &cameras[camera->index]; + + mp_camera_release_buffer(info->camera, *buffer_index); +} + +void mp_io_pipeline_release_buffer(uint32_t buffer_index) +{ + mp_pipeline_invoke(pipeline, (MPPipelineCallback) release_buffer, &buffer_index, sizeof(uint32_t)); +} + static void update_controls() { @@ -375,7 +388,7 @@ update_controls() } static void -on_frame(MPImage image, void *data) +on_frame(MPBuffer buffer, void * _data) { // Only update controls right after a frame was captured update_controls(); @@ -384,13 +397,13 @@ on_frame(MPImage image, void *data) // presumably from buffers made ready during the switch. Ignore these. if (just_switched_mode) { if (blank_frame_count < 20) { - // Only check a 50x50 area + // Only check a 10x10 area size_t test_size = - MIN(50, image.width) * MIN(50, image.height); + MIN(10, mode.width) * MIN(10, mode.height); bool image_is_blank = true; for (size_t i = 0; i < test_size; ++i) { - if (image.data[i] != 0) { + if (buffer.data[i] != 0) { image_is_blank = false; } } @@ -407,17 +420,8 @@ on_frame(MPImage image, void *data) blank_frame_count = 0; } - // Copy from the camera buffer - size_t size = - mp_pixel_format_width_to_bytes(image.pixel_format, image.width) * - image.height; - uint8_t *buffer = malloc(size); - memcpy(buffer, image.data, size); - - image.data = buffer; - // Send the image off for processing - mp_process_pipeline_process_image(image); + mp_process_pipeline_process_image(buffer); if (captures_remaining > 0) { --captures_remaining; @@ -465,6 +469,7 @@ update_state(MPPipeline *pipeline, const struct mp_io_pipeline_state *state) struct camera_info *info = &cameras[camera->index]; struct device_info *dev_info = &devices[info->device_index]; + mp_process_pipeline_sync(); mp_camera_stop_capture(info->camera); mp_device_setup_link(dev_info->device, info->pad_id, dev_info->interface_pad_id, false); diff --git a/io_pipeline.h b/io_pipeline.h index 10a6298..1814f84 100644 --- a/io_pipeline.h +++ b/io_pipeline.h @@ -23,4 +23,6 @@ void mp_io_pipeline_stop(); void mp_io_pipeline_focus(); void mp_io_pipeline_capture(); +void mp_io_pipeline_release_buffer(uint32_t buffer_index); + void mp_io_pipeline_update_state(const struct mp_io_pipeline_state *state); diff --git a/pipeline.c b/pipeline.c index fcfa4c2..80715f0 100644 --- a/pipeline.c +++ b/pipeline.c @@ -65,6 +65,27 @@ mp_pipeline_invoke(MPPipeline *pipeline, MPPipelineCallback callback, } } +static bool +unlock_mutex(GMutex *mutex) +{ + g_mutex_unlock(mutex); + return false; +} + +void +mp_pipeline_sync(MPPipeline *pipeline) +{ + GMutex mutex; + g_mutex_init(&mutex); + g_mutex_lock(&mutex); + + g_main_context_invoke_full(pipeline->main_context, G_PRIORITY_LOW, (GSourceFunc)unlock_mutex, &mutex, NULL); + g_mutex_lock(&mutex); + g_mutex_unlock(&mutex); + + g_mutex_clear(&mutex); +} + void mp_pipeline_free(MPPipeline *pipeline) { @@ -80,21 +101,24 @@ mp_pipeline_free(MPPipeline *pipeline) struct capture_source_args { MPCamera *camera; - void (*callback)(MPImage, void *); + void (*callback)(MPBuffer, void *); void *user_data; }; static bool on_capture(int fd, GIOCondition condition, struct capture_source_args *args) { - mp_camera_capture_image(args->camera, args->callback, args->user_data); + MPBuffer buffer; + if (mp_camera_capture_buffer(args->camera, &buffer)) { + args->callback(buffer, args->user_data); + } return true; } // Not thread safe GSource * mp_pipeline_add_capture_source(MPPipeline *pipeline, MPCamera *camera, - void (*callback)(MPImage, void *), void *user_data) + void (*callback)(MPBuffer, void *), void *user_data) { int video_fd = mp_camera_get_video_fd(camera); GSource *video_source = g_unix_fd_source_new(video_fd, G_IO_IN); diff --git a/pipeline.h b/pipeline.h index 6f71faa..67eaeff 100644 --- a/pipeline.h +++ b/pipeline.h @@ -11,8 +11,10 @@ typedef void (*MPPipelineCallback)(MPPipeline *, const void *); MPPipeline *mp_pipeline_new(); void mp_pipeline_invoke(MPPipeline *pipeline, MPPipelineCallback callback, const void *data, size_t size); +// Wait until all pending tasks have completed +void mp_pipeline_sync(MPPipeline *pipeline); void mp_pipeline_free(MPPipeline *pipeline); GSource *mp_pipeline_add_capture_source(MPPipeline *pipeline, MPCamera *camera, - void (*callback)(MPImage, void *), + void (*callback)(MPBuffer, void *), void *user_data); diff --git a/process_pipeline.c b/process_pipeline.c index 64003a2..7a43c59 100644 --- a/process_pipeline.c +++ b/process_pipeline.c @@ -2,6 +2,7 @@ #include "pipeline.h" #include "zbar_pipeline.h" +#include "io_pipeline.h" #include "main.h" #include "config.h" #include "quickpreview.h" @@ -134,21 +135,27 @@ mp_process_pipeline_stop() mp_zbar_pipeline_stop(); } +void +mp_process_pipeline_sync() +{ + mp_pipeline_sync(pipeline); +} + static cairo_surface_t * -process_image_for_preview(const MPImage *image) +process_image_for_preview(const uint8_t *image) { uint32_t surface_width, surface_height, skip; quick_preview_size(&surface_width, &surface_height, &skip, preview_width, - preview_height, image->width, image->height, - image->pixel_format, camera->rotate); + preview_height, mode.width, mode.height, + mode.pixel_format, camera->rotate); cairo_surface_t *surface = cairo_image_surface_create( CAIRO_FORMAT_RGB24, surface_width, surface_height); uint8_t *pixels = cairo_image_surface_get_data(surface); - quick_preview((uint32_t *)pixels, surface_width, surface_height, image->data, - image->width, image->height, image->pixel_format, + quick_preview((uint32_t *)pixels, surface_width, surface_height, image, + mode.width, mode.height, mode.pixel_format, camera->rotate, camera->mirrored, camera->previewmatrix[0] == 0 ? NULL : camera->previewmatrix, camera->blacklevel, skip); @@ -174,7 +181,7 @@ process_image_for_preview(const MPImage *image) } static void -process_image_for_capture(const MPImage *image, int count) +process_image_for_capture(const uint8_t *image, int count) { time_t rawtime; time(&rawtime); @@ -193,8 +200,8 @@ process_image_for_capture(const MPImage *image, int count) // Define TIFF thumbnail TIFFSetField(tif, TIFFTAG_SUBFILETYPE, 1); - TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, image->width >> 4); - TIFFSetField(tif, TIFFTAG_IMAGELENGTH, image->height >> 4); + TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, mode.width >> 4); + TIFFSetField(tif, TIFFTAG_IMAGELENGTH, mode.height >> 4); TIFFSetField(tif, TIFFTAG_BITSPERSAMPLE, 8); TIFFSetField(tif, TIFFTAG_COMPRESSION, COMPRESSION_NONE); TIFFSetField(tif, TIFFTAG_PHOTOMETRIC, PHOTOMETRIC_RGB); @@ -241,8 +248,8 @@ process_image_for_capture(const MPImage *image, int count) // Write black thumbnail, only windows uses this { unsigned char *buf = - (unsigned char *)calloc(1, (int)image->width >> 4); - for (int row = 0; row < (image->height >> 4); row++) { + (unsigned char *)calloc(1, (int)mode.width >> 4); + for (int row = 0; row < (mode.height >> 4); row++) { TIFFWriteScanline(tif, buf, row, 0); } free(buf); @@ -251,8 +258,8 @@ process_image_for_capture(const MPImage *image, int count) // Define main photo TIFFSetField(tif, TIFFTAG_SUBFILETYPE, 0); - TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, image->width); - TIFFSetField(tif, TIFFTAG_IMAGELENGTH, image->height); + TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, mode.width); + TIFFSetField(tif, TIFFTAG_IMAGELENGTH, mode.height); TIFFSetField(tif, TIFFTAG_BITSPERSAMPLE, 8); TIFFSetField(tif, TIFFTAG_PHOTOMETRIC, PHOTOMETRIC_CFA); TIFFSetField(tif, TIFFTAG_SAMPLESPERPIXEL, 1); @@ -274,9 +281,9 @@ process_image_for_capture(const MPImage *image, int count) TIFFCheckpointDirectory(tif); printf("Writing frame to %s\n", fname); - unsigned char *pLine = (unsigned char *)malloc(image->width); - for (int row = 0; row < image->height; row++) { - TIFFWriteScanline(tif, image->data + (row * image->width), row, 0); + unsigned char *pLine = (unsigned char *)malloc(mode.width); + for (int row = 0; row < mode.height; row++) { + TIFFWriteScanline(tif, (void *) image + (row * mode.width), row, 0); } free(pLine); TIFFWriteDirectory(tif); @@ -293,7 +300,7 @@ process_image_for_capture(const MPImage *image, int count) TIFFSetField(tif, EXIFTAG_EXPOSURETIME, (mode.frame_interval.numerator / (float)mode.frame_interval.denominator) / - ((float)image->height / (float)exposure)); + ((float)mode.height / (float)exposure)); uint16_t isospeed[1]; isospeed[0] = (uint16_t)remap(gain - 1, 0, gain_max, camera->iso_min, camera->iso_max); @@ -401,9 +408,14 @@ process_capture_burst(cairo_surface_t *thumb) } static void -process_image(MPPipeline *pipeline, const MPImage *image) +process_image(MPPipeline *pipeline, const MPBuffer *buffer) { - assert(image->width == mode.width && image->height == mode.height); + size_t size = + mp_pixel_format_width_to_bytes(mode.pixel_format, mode.width) * + mode.height; + uint8_t *image = malloc(size); + memcpy(image, buffer->data, size); + mp_io_pipeline_release_buffer(buffer->index); cairo_surface_t *thumb = process_image_for_preview(image); @@ -423,7 +435,7 @@ process_image(MPPipeline *pipeline, const MPImage *image) assert(!thumb); } - free(image->data); + free(image); ++frames_processed; if (captures_remaining == 0) { @@ -432,19 +444,19 @@ process_image(MPPipeline *pipeline, const MPImage *image) } void -mp_process_pipeline_process_image(MPImage image) +mp_process_pipeline_process_image(MPBuffer buffer) { // If we haven't processed the previous frame yet, drop this one if (frames_received != frames_processed && !is_capturing) { printf("Dropped frame at capture\n"); - free(image.data); + mp_io_pipeline_release_buffer(buffer.index); return; } ++frames_received; - mp_pipeline_invoke(pipeline, (MPPipelineCallback)process_image, &image, - sizeof(MPImage)); + mp_pipeline_invoke(pipeline, (MPPipelineCallback)process_image, &buffer, + sizeof(MPBuffer)); } static void diff --git a/process_pipeline.h b/process_pipeline.h index 40cadd7..351a576 100644 --- a/process_pipeline.h +++ b/process_pipeline.h @@ -24,7 +24,8 @@ struct mp_process_pipeline_state { void mp_process_pipeline_start(); void mp_process_pipeline_stop(); +void mp_process_pipeline_sync(); -void mp_process_pipeline_process_image(MPImage image); +void mp_process_pipeline_process_image(MPBuffer buffer); void mp_process_pipeline_capture(); void mp_process_pipeline_update_state(const struct mp_process_pipeline_state *state); diff --git a/tools/camera_test.c b/tools/camera_test.c index df88dbf..df7d923 100644 --- a/tools/camera_test.c +++ b/tools/camera_test.c @@ -15,20 +15,6 @@ get_time() return t.tv_sec + t.tv_usec * 1e-6; } -void -on_capture(MPImage image, void *user_data) -{ - size_t num_bytes = - mp_pixel_format_width_to_bytes(image.pixel_format, image.width) * - image.height; - uint8_t *data = malloc(num_bytes); - memcpy(data, image.data, num_bytes); - - printf(" first byte: %d.", data[0]); - - free(data); -} - int main(int argc, char *argv[]) { @@ -184,7 +170,22 @@ main(int argc, char *argv[]) (last - start_capture) * 1000); for (int i = 0; i < 10; ++i) { - mp_camera_capture_image(camera, on_capture, NULL); + MPBuffer buffer; + if (!mp_camera_capture_buffer(camera, &buffer)) { + printf(" Failed to capture buffer\n"); + } + + size_t num_bytes = + mp_pixel_format_width_to_bytes(m->pixel_format, m->width) * + m->height; + uint8_t *data = malloc(num_bytes); + memcpy(data, buffer.data, num_bytes); + + printf(" first byte: %d.", data[0]); + + free(data); + + mp_camera_release_buffer(camera, buffer.index); double now = get_time(); printf(" capture took %fms\n", (now - last) * 1000);