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

@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
+#include <unistd.h>
 
 #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;
 }
 

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;
 

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);

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);

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);

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);

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

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);

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);