[libcamera-devel,v4,1/3] android: generic_camera_buffer: Correct buffer mapping

13531 diff mbox series

diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
index c4e3a9e7..87df2570 100644
--- a/src/android/camera_buffer.h
+++ b/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)	\
-	: Extensible(std::make_unique<Private>(this, camera3Buffer, flags)) \
+CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer,		\
+			   libcamera::PixelFormat pixelFormat,		\
+			   const libcamera::Size &size, int flags)	\
+	: Extensible(std::make_unique<Private>(this, camera3Buffer,	\
+					       pixelFormat, size,	\
+					       flags))			\
 {									\
 }									\
 CameraBuffer::~CameraBuffer()						\
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 61b44183..01909ec7 100644
--- a/src/android/camera_stream.cpp
+++ b/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;
diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
index e8783ff8..50732637 100644
--- a/src/android/mm/cros_camera_buffer.cpp
+++ b/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,
-		buffer_handle_t camera3Buffer, int flags);
+	Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
+		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)
diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
index b3af194c..22753490 100644
--- a/src/android/mm/generic_camera_buffer.cpp
+++ b/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,
-		buffer_handle_t camera3Buffer, int flags);
+	Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
+		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;
 
+	const auto &info = libcamera::PixelFormatInfo::info(pixelFormat);
+	if (!info.isValid()) {
+		error_ = -EINVAL;
+		LOG(HAL, Error) << "Invalid pixel format: "
+				<< pixelFormat.toString();
+		return;
+	}
+
+	/*
+	 * 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)
+		if (camera3Buffer->data[i] == -1 || camera3Buffer->data[i] == fd)
 			continue;
 
-		off_t length = lseek(camera3Buffer->data[i], 0, SEEK_END);
-		if (length < 0) {
-			error_ = -errno;
-			LOG(HAL, Error) << "Failed to query plane length";
-			break;
+		if (fd != -1) {
+			error_ = -EINVAL;
+			LOG(HAL, Error) << "Discontiguous planes are not supported";
+			return;
 		}
 
-		void *address = mmap(nullptr, length, flags, MAP_SHARED,
-				     camera3Buffer->data[i], 0);
-		if (address == MAP_FAILED) {
-			error_ = -errno;
-			LOG(HAL, Error) << "Failed to mmap plane";
-			break;
-		}
+		fd = camera3Buffer->data[i];
+	}
 
-		maps_.emplace_back(static_cast<uint8_t *>(address),
-				   static_cast<size_t>(length));
+	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";
+		return;
+	}
+	maps_.emplace_back(static_cast<uint8_t *>(address), bufferLength);
+
+	const unsigned int numPlanes = info.numPlanes();
+	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