[libcamera-devel] [PATCH v2 13/27] libcamera: v4l2_videodevice: Coalesce planes when queuing buffer

[Laurent Pinchart]

When queueing a buffer to a V4L2VideoDevice, the number of planes in the
FrameBuffer may not match the number of V4L2 buffer planes if the
PixelFormat is multi-planar (has multiple colour planes) and the V4L2
format is single-planar (has a single buffer plane). In this case, we
need to coalesce all FrameBuffer planes into a single V4L2 buffer plane.
Do so, and add validity checks to reject frame buffers that can't be
described using a single V4L2 buffer plane.

This change prepares for proper multi-planar support, but isn't expected
to result in a change of behaviour with existing pipeline handlers, as
none of them queue an output buffer with multiple FrameBuffer planes or
use non-contiguous buffers for either capture or output.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
---
Changes since v1:

- Make numV4l2Planes const
- Use format_.planesCount
---
 src/libcamera/v4l2_videodevice.cpp | 67 +++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 10 deletions(-)

diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index c6c9263c49e9..625d5da40337 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -22,10 +22,12 @@
 
 #include <libcamera/base/event_notifier.h>
 #include <libcamera/base/log.h>
+#include <libcamera/base/utils.h>
 
 #include <libcamera/file_descriptor.h>
 
 #include "libcamera/internal/formats.h"
+#include "libcamera/internal/framebuffer.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/media_object.h"
 
@@ -1496,10 +1498,20 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
 
 	bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);
 	const std::vector<FrameBuffer::Plane> &planes = buffer->planes();
+	const unsigned int numV4l2Planes = format_.planesCount;
+
+	/*
+	 * If the frame buffer has multiple planes and the V4L2 format requires
+	 * contiguous planes, ensure that's the case.
+	 */
+	if (planes.size() != numV4l2Planes && !buffer->_d()->isContiguous()) {
+		LOG(V4L2, Error) << "Device format requires contiguous buffer";
+		return -EINVAL;
+	}
 
 	if (buf.memory == V4L2_MEMORY_DMABUF) {
 		if (multiPlanar) {
-			for (unsigned int p = 0; p < planes.size(); ++p)
+			for (unsigned int p = 0; p < numV4l2Planes; ++p)
 				v4l2Planes[p].m.fd = planes[p].fd.fd();
 		} else {
 			buf.m.fd = planes[0].fd.fd();
@@ -1507,23 +1519,58 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
 	}
 
 	if (multiPlanar) {
-		buf.length = planes.size();
+		buf.length = numV4l2Planes;
 		buf.m.planes = v4l2Planes;
 	}
 
 	if (V4L2_TYPE_IS_OUTPUT(buf.type)) {
 		const FrameMetadata &metadata = buffer->metadata();
 
-		if (multiPlanar) {
-			unsigned int nplane = 0;
-			for (const FrameMetadata::Plane &plane : metadata.planes) {
-				v4l2Planes[nplane].bytesused = plane.bytesused;
-				v4l2Planes[nplane].length = buffer->planes()[nplane].length;
-				nplane++;
+		if (numV4l2Planes != planes.size()) {
+			/*
+			 * If we have a multi-planar buffer with a V4L2
+			 * single-planar format, coalesce all planes. The length
+			 * and number of bytes used may only differ in the last
+			 * plane as any other situation can't be represented.
+			 */
+			unsigned int bytesused = 0;
+			unsigned int length = 0;
+
+			for (auto [i, plane] : utils::enumerate(planes)) {
+				bytesused += metadata.planes[i].bytesused;
+				length += plane.length;
+
+				if (i != planes.size() - 1 && bytesused != length) {
+					LOG(V4L2, Error)
+						<< "Holes in multi-planar buffer not supported";
+					return -EINVAL;
+				}
+			}
+
+			if (multiPlanar) {
+				v4l2Planes[0].bytesused = bytesused;
+				v4l2Planes[0].length = length;
+			} else {
+				buf.bytesused = bytesused;
+				buf.length = length;
+			}
+		} else if (multiPlanar) {
+			/*
+			 * If we use the multi-planar API, fill in the planes.
+			 * The number of planes in the frame buffer and in the
+			 * V4L2 buffer is guaranteed to be equal at this point.
+			 */
+			for (auto [i, plane] : utils::enumerate(planes)) {
+				v4l2Planes[i].bytesused = metadata.planes[i].bytesused;
+				v4l2Planes[i].length = plane.length;
 			}
 		} else {
-			if (metadata.planes.size())
-				buf.bytesused = metadata.planes[0].bytesused;
+			/*
+			 * Single-planar API with a single plane in the buffer
+			 * is trivial to handle.
+			 */
+			buf.bytesused = metadata.planes[0].bytesused;
+			buf.length = planes[0].length;
 		}
 
 		buf.sequence = metadata.sequence;
-- 
Regards,

Laurent Pinchart