android: generic_camera_buffer: Correct buffer mapping

buffer_handle_t doesn't provide sufficient info to map a buffer
properly. cros::CameraBufferManager enables handling the buffer on
ChromeOS, but no way is provided for other platforms.

Therefore, we put the assumption that planes are in the same buffer
and they are consecutive. This modifies the way of mapping in
generic_camera_buffer with the assumption.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

src/android/camera_buffer.h

@@ -11,13 +11,17 @@
 
 #include <libcamera/base/class.h>
 #include <libcamera/base/span.h>
+#include <libcamera/geometry.h>
+#include <libcamera/pixel_format.h>
 
 class CameraBuffer final : public libcamera::Extensible
 {
 	LIBCAMERA_DECLARE_PRIVATE()
 
 public:
-	CameraBuffer(buffer_handle_t camera3Buffer, int flags);
+	CameraBuffer(buffer_handle_t camera3Buffer,
+		     libcamera::PixelFormat pixelFormat,
+		     const libcamera::Size &size, int flags);
 	~CameraBuffer();
 
 	bool isValid() const;
@@ -31,8 +35,12 @@ public:
 };
 
 #define PUBLIC_CAMERA_BUFFER_IMPLEMENTATION				\
-CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)	\
+CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer,		\
-	: Extensible(std::make_unique<Private>(this, camera3Buffer, flags)) \
+			   libcamera::PixelFormat pixelFormat,		\
+			   const libcamera::Size &size, int flags)	\
+	: Extensible(std::make_unique<Private>(this, camera3Buffer,	\
+					       pixelFormat, size,	\
+					       flags))			\
 {									\
 }									\
 CameraBuffer::~CameraBuffer()						\

src/android/camera_stream.cpp

@@ -110,7 +110,9 @@ int CameraStream::process(const libcamera::FrameBuffer &source,
 	 * \todo Buffer mapping and processing should be moved to a
 	 * separate thread.
 	 */
-	CameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE);
+	const StreamConfiguration &output = configuration();
+	CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,
+			  PROT_READ | PROT_WRITE);
 	if (!dest.isValid()) {
 		LOG(HAL, Error) << "Failed to map android blob buffer";
 		return -EINVAL;

src/android/mm/cros_camera_buffer.cpp

@@ -20,8 +20,9 @@ class CameraBuffer::Private : public Extensible::Private
 	LIBCAMERA_DECLARE_PUBLIC(CameraBuffer)
 
 public:
-	Private(CameraBuffer *cameraBuffer,
+	Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
-		buffer_handle_t camera3Buffer, int flags);
+		libcamera::PixelFormat pixelFormat, const libcamera::Size &size,
+		int flags);
 	~Private();
 
 	bool isValid() const { return valid_; }
@@ -46,6 +47,8 @@ private:
 
 CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
 			       buffer_handle_t camera3Buffer,
+			       [[maybe_unused]] libcamera::PixelFormat pixelFormat,
+			       [[maybe_unused]] const libcamera::Size &size,
 			       [[maybe_unused]] int flags)
 	: handle_(camera3Buffer), numPlanes_(0), valid_(false),
 	  registered_(false)

src/android/mm/generic_camera_buffer.cpp

@@ -12,6 +12,7 @@
 
 #include <libcamera/base/log.h>
 
+#include "libcamera/internal/formats.h"
 #include "libcamera/internal/mapped_framebuffer.h"
 
 using namespace libcamera;
@@ -24,8 +25,9 @@ class CameraBuffer::Private : public Extensible::Private,
 	LIBCAMERA_DECLARE_PUBLIC(CameraBuffer)
 
 public:
-	Private(CameraBuffer *cameraBuffer,
+	Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
-		buffer_handle_t camera3Buffer, int flags);
+		libcamera::PixelFormat pixelFormat, const libcamera::Size &size,
+		int flags);
 	~Private();
 
 	unsigned int numPlanes() const;
@@ -33,35 +35,92 @@ public:
 	Span<uint8_t> plane(unsigned int plane);
 
 	size_t jpegBufferSize(size_t maxJpegBufferSize) const;
+
+private:
+	/* \todo Remove planes_ when it will be added to MappedBuffer */
+	std::vector<Span<uint8_t>> planes_;
 };
 
 CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
-			       buffer_handle_t camera3Buffer, int flags)
+			       buffer_handle_t camera3Buffer,
+			       libcamera::PixelFormat pixelFormat,
+			       const libcamera::Size &size, int flags)
 {
-	maps_.reserve(camera3Buffer->numFds);
 	error_ = 0;
 
-	for (int i = 0; i < camera3Buffer->numFds; i++) {
+	const auto &info = libcamera::PixelFormatInfo::info(pixelFormat);
-		if (camera3Buffer->data[i] == -1)
+	if (!info.isValid()) {
-			continue;
+		error_ = -EINVAL;
-
+		LOG(HAL, Error) << "Invalid pixel format: "
-		off_t length = lseek(camera3Buffer->data[i], 0, SEEK_END);
+				<< pixelFormat.toString();
-		if (length < 0) {
+		return;
-			error_ = -errno;
-			LOG(HAL, Error) << "Failed to query plane length";
-			break;
 	}
 
-		void *address = mmap(nullptr, length, flags, MAP_SHARED,
+	/*
-				     camera3Buffer->data[i], 0);
+	 * As Android doesn't offer an API to query buffer layouts, assume for
+	 * now that the buffer is backed by a single dmabuf, with planes being
+	 * stored contiguously.
+	 */
+	int fd = -1;
+	for (int i = 0; i < camera3Buffer->numFds; i++) {
+		if (camera3Buffer->data[i] == -1 || camera3Buffer->data[i] == fd)
+			continue;
+
+		if (fd != -1) {
+			error_ = -EINVAL;
+			LOG(HAL, Error) << "Discontiguous planes are not supported";
+			return;
+		}
+
+		fd = camera3Buffer->data[i];
+	}
+
+	if (fd == -1) {
+		error_ = -EINVAL;
+		LOG(HAL, Error) << "No valid file descriptor";
+		return;
+	}
+
+	off_t bufferLength = lseek(fd, 0, SEEK_END);
+	if (bufferLength < 0) {
+		error_ = -errno;
+		LOG(HAL, Error) << "Failed to get buffer length";
+		return;
+	}
+
+	void *address = mmap(nullptr, bufferLength, flags, MAP_SHARED, fd, 0);
 	if (address == MAP_FAILED) {
 		error_ = -errno;
 		LOG(HAL, Error) << "Failed to mmap plane";
-			break;
+		return;
 	}
+	maps_.emplace_back(static_cast<uint8_t *>(address), bufferLength);
 
-		maps_.emplace_back(static_cast<uint8_t *>(address),
+	const unsigned int numPlanes = info.numPlanes();
-				   static_cast<size_t>(length));
+	planes_.resize(numPlanes);
+	unsigned int offset = 0;
+	for (unsigned int i = 0; i < numPlanes; ++i) {
+		/*
+		 * \todo Remove if this plane size computation function is
+		 * added to PixelFormatInfo.
+		 */
+		const unsigned int vertSubSample = info.planes[i].verticalSubSampling;
+		const unsigned int stride = info.stride(size.width, i, 1u);
+		const unsigned int planeSize =
+			stride * ((size.height + vertSubSample - 1) / vertSubSample);
+
+		planes_[i] = libcamera::Span<uint8_t>(
+			static_cast<uint8_t *>(address) + offset, planeSize);
+
+		if (bufferLength < offset + planeSize) {
+			error_ = -EINVAL;
+			LOG(HAL, Error) << "Plane " << i << " is out of buffer"
+					<< ", buffer length=" << bufferLength
+					<< ", offset=" << offset
+					<< ", size=" << planeSize;
+			return;
+		}
+		offset += planeSize;
 	}
 }
 
@@ -71,15 +130,15 @@ CameraBuffer::Private::~Private()
 
 unsigned int CameraBuffer::Private::numPlanes() const
 {
-	return maps_.size();
+	return planes_.size();
 }
 
 Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
 {
-	if (plane >= maps_.size())
+	if (plane >= planes_.size())
 		return {};
 
-	return maps_[plane];
+	return planes_[plane];
 }
 
 size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const