android: camera_device: Create struct to track per stream buffer

The Camera3RequestDescriptor structure stores, for each stream, the
camera3_stream_buffer_t and the libcamera FrameBuffer in two separate
vectors. This complicates buffer handling, as the code needs to keep
both vectors in sync. Create a new structure to group all data about
per-stream buffers to simplify this.

As a side effect, we need to create a local vector of
camera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the
camera3_stream_buffer_t instances stored in the new structure in
Camera3RequestDescriptor are not contiguous anymore. This is a small
price to pay for easier handling of buffers, and will be refactored in
subsequent commits anyway.

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

3 files changed, 55 insertions, 44 deletions

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index a8c66a79..74d9b952 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -802,9 +802,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
 	notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
 
 	for (auto &buffer : descriptor->buffers_) {
-		buffer.release_fence = buffer.acquire_fence;
-		buffer.acquire_fence = -1;
-		buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+		buffer.buffer.release_fence = buffer.buffer.acquire_fence;
+		buffer.buffer.acquire_fence = -1;
+		buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
 	}
 
 	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
@@ -902,8 +902,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie()
 			<< " with " << descriptor->buffers_.size() << " streams";
 
-	for (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) {
-		camera3_stream *camera3Stream = camera3Buffer.stream;
+	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
+		camera3_stream *camera3Stream = buffer.buffer.stream;
 		CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);
 
 		std::stringstream ss;
@@ -920,7 +920,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		 * while fences for streams of type Internal and Mapped are
 		 * handled at post-processing time.
 		 */
-		FrameBuffer *buffer = nullptr;
+		FrameBuffer *frameBuffer = nullptr;
 		int acquireFence = -1;
 		switch (cameraStream->type()) {
 		case CameraStream::Type::Mapped:
@@ -938,13 +938,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			 * associate it with the Camera3RequestDescriptor for
 			 * lifetime management only.
 			 */
-			descriptor->frameBuffers_.push_back(
-				createFrameBuffer(*camera3Buffer.buffer,
+			buffer.frameBuffer =
+				createFrameBuffer(*buffer.buffer.buffer,
 						  cameraStream->configuration().pixelFormat,
-						  cameraStream->configuration().size));
-
-			buffer = descriptor->frameBuffers_.back().get();
-			acquireFence = camera3Buffer.acquire_fence;
+						  cameraStream->configuration().size);
+			frameBuffer = buffer.frameBuffer.get();
+			acquireFence = buffer.buffer.acquire_fence;
 			LOG(HAL, Debug) << ss.str() << " (direct)";
 			break;
 
@@ -956,18 +955,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			 * The buffer has to be returned to the CameraStream
 			 * once it has been processed.
 			 */
-			buffer = cameraStream->getBuffer();
+			frameBuffer = cameraStream->getBuffer();
 			LOG(HAL, Debug) << ss.str() << " (internal)";
 			break;
 		}
 
-		if (!buffer) {
-			LOG(HAL, Error) << "Failed to create buffer";
+		if (!frameBuffer) {
+			LOG(HAL, Error) << "Failed to create frame buffer";
 			return -ENOMEM;
 		}
 
-		descriptor->request_->addBuffer(cameraStream->stream(), buffer,
-					       acquireFence);
+		descriptor->request_->addBuffer(cameraStream->stream(),
+						frameBuffer, acquireFence);
 	}
 
 	/*
@@ -1054,9 +1053,9 @@ void CameraDevice::requestComplete(Request *request)
 	 * The buffer status is set to OK and later changed to ERROR if
 	 * post-processing/compression fails.
 	 */
-	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
+	for (auto &buffer : descriptor->buffers_) {
 		CameraStream *cameraStream =
-			static_cast<CameraStream *>(buffer.stream->priv);
+			static_cast<CameraStream *>(buffer.buffer.stream->priv);
 
 		/*
 		 * Streams of type Direct have been queued to the
@@ -1070,9 +1069,9 @@ void CameraDevice::requestComplete(Request *request)
 		 * fence to -1 once it has handled it and remove this check.
 		 */
 		if (cameraStream->type() == CameraStream::Type::Direct)
-			buffer.acquire_fence = -1;
-		buffer.release_fence = -1;
-		buffer.status = CAMERA3_BUFFER_STATUS_OK;
+			buffer.buffer.acquire_fence = -1;
+		buffer.buffer.release_fence = -1;
+		buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK;
 	}
 
 	/*
@@ -1087,15 +1086,15 @@ void CameraDevice::requestComplete(Request *request)
 		notifyError(descriptor->frameNumber_, nullptr,
 			    CAMERA3_MSG_ERROR_REQUEST);
 
-		for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
+		for (auto &buffer : descriptor->buffers_) {
 			/*
 			 * Signal to the framework it has to handle fences that
 			 * have not been waited on by setting the release fence
 			 * to the acquire fence value.
 			 */
-			buffer.release_fence = buffer.acquire_fence;
-			buffer.acquire_fence = -1;
-			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+			buffer.buffer.release_fence = buffer.buffer.acquire_fence;
+			buffer.buffer.acquire_fence = -1;
+			buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
 		}
 
 		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
@@ -1137,9 +1136,9 @@ void CameraDevice::requestComplete(Request *request)
 	}
 
 	/* Handle post-processing. */
-	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
+	for (auto &buffer : descriptor->buffers_) {
 		CameraStream *cameraStream =
-			static_cast<CameraStream *>(buffer.stream->priv);
+			static_cast<CameraStream *>(buffer.buffer.stream->priv);
 
 		if (cameraStream->type() == CameraStream::Type::Direct)
 			continue;
@@ -1147,13 +1146,13 @@ void CameraDevice::requestComplete(Request *request)
 		FrameBuffer *src = request->findBuffer(cameraStream->stream());
 		if (!src) {
 			LOG(HAL, Error) << "Failed to find a source stream buffer";
-			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
-			notifyError(descriptor->frameNumber_, buffer.stream,
+			buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+			notifyError(descriptor->frameNumber_, buffer.buffer.stream,
 				    CAMERA3_MSG_ERROR_BUFFER);
 			continue;
 		}
 
-		int ret = cameraStream->process(*src, buffer, descriptor);
+		int ret = cameraStream->process(*src, buffer.buffer, descriptor);
 
 		/*
 		 * Return the FrameBuffer to the CameraStream now that we're
@@ -1163,8 +1162,8 @@ void CameraDevice::requestComplete(Request *request)
 			cameraStream->putBuffer(src);
 
 		if (ret) {
-			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
-			notifyError(descriptor->frameNumber_, buffer.stream,
+			buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+			notifyError(descriptor->frameNumber_, buffer.buffer.stream,
 				    CAMERA3_MSG_ERROR_BUFFER);
 		}
 	}
@@ -1190,10 +1189,16 @@ void CameraDevice::sendCaptureResults()
 		camera3_capture_result_t captureResult = {};
 
 		captureResult.frame_number = descriptor->frameNumber_;
+
 		if (descriptor->resultMetadata_)
 			captureResult.result = descriptor->resultMetadata_->get();
-		captureResult.num_output_buffers = descriptor->buffers_.size();
-		captureResult.output_buffers = descriptor->buffers_.data();
+
+		std::vector<camera3_stream_buffer_t> resultBuffers;
+		for (const auto &buffer : descriptor->buffers_)
+			resultBuffers.emplace_back(buffer.buffer);
+
+		captureResult.num_output_buffers = resultBuffers.size();
+		captureResult.output_buffers = resultBuffers.data();
 
 		if (descriptor->status_ == Camera3RequestDescriptor::Status::Success)
 			captureResult.partial_result = 1;
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
index 16a632b3..614baed4 100644
--- a/src/android/camera_request.cpp
+++ b/src/android/camera_request.cpp
@@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
 
 	/* Copy the camera3 request stream information for later access. */
 	const uint32_t numBuffers = camera3Request->num_output_buffers;
+
 	buffers_.resize(numBuffers);
 	for (uint32_t i = 0; i < numBuffers; i++)
-		buffers_[i] = camera3Request->output_buffers[i];
-
-	/*
-	 * FrameBuffer instances created by wrapping a camera3 provided dmabuf
-	 * are emplaced in this vector of unique_ptr<> for lifetime management.
-	 */
-	frameBuffers_.reserve(numBuffers);
+		buffers_[i].buffer = camera3Request->output_buffers[i];
 
 	/* Clone the controls associated with the camera3 request. */
 	settings_ = CameraMetadata(camera3Request->settings);
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index db13f624..a030febf 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -29,6 +29,16 @@ public:
 		Error,
 	};
 
+	struct StreamBuffer {
+		camera3_stream_buffer_t buffer;
+		/*
+		 * FrameBuffer instances created by wrapping a camera3 provided
+		 * dmabuf are emplaced in this vector of unique_ptr<> for
+		 * lifetime management.
+		 */
+		std::unique_ptr<libcamera::FrameBuffer> frameBuffer;
+	};
+
 	Camera3RequestDescriptor(libcamera::Camera *camera,
 				 const camera3_capture_request_t *camera3Request);
 	~Camera3RequestDescriptor();
@@ -36,8 +46,9 @@ public:
 	bool isPending() const { return status_ == Status::Pending; }
 
 	uint32_t frameNumber_ = 0;
-	std::vector<camera3_stream_buffer_t> buffers_;
-	std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_;
+
+	std::vector<StreamBuffer> buffers_;
+
 	CameraMetadata settings_;
 	std::unique_ptr<CaptureRequest> request_;
 	std::unique_ptr<CameraMetadata> resultMetadata_;