[libcamera-devel,v2,7/8] android: camera_device: Use libcamera buffer pool

Message ID 20200918135041.91492-8-jacopo@jmondi.org
State Superseded
Headers show
Series
  • android: camera_device: Add support for internal buffers
Related show

Commit Message

Jacopo Mondi Sept. 18, 2020, 1:50 p.m. UTC
Now that we have reserved and made available to the camera HAL a
pool of libcamera allocated buffers, use them when a CameraStream
instance that requires internal allocation is processed.

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 47 +++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 13 deletions(-)

Comments

Niklas Söderlund Sept. 20, 2020, 12:48 a.m. UTC | #1
Hi Jacopo,

Thanks for your patch,

On 2020-09-18 15:50:40 +0200, Jacopo Mondi wrote:
> Now that we have reserved and made available to the camera HAL a
> pool of libcamera allocated buffers, use them when a CameraStream
> instance that requires internal allocation is processed.
> 
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_device.cpp | 47 +++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 6ed56ff57dab..f7e9a1e5f494 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1477,6 +1477,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
>  		CameraStream *cameraStream =
>  			static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> +		const StreamConfiguration &config = config_->at(cameraStream->index());
> +		Stream *stream = config.stream();
>  
>  		/*
>  		 * Keep track of which stream the request belongs to and store
> @@ -1485,27 +1487,39 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		descriptor->buffers[i].stream = camera3Buffers[i].stream;
>  		descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
>  
> -		/* Software streams are handled after hardware streams complete. */
> -		if (cameraStream->format() == formats::MJPEG)
> +		/* Mapped streams don't need to be added to the Request. */
> +		if (cameraStream->type() == CameraStream::Type::Mapped)
>  			continue;
>  
> -		/*
> -		 * Create a libcamera buffer using the dmabuf descriptors of
> -		 * the camera3Buffer for each stream. The FrameBuffer is
> -		 * directly associated with the Camera3RequestDescriptor for
> -		 * lifetime management only.
> -		 */
> -		FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> +		FrameBuffer *buffer;
> +		if (cameraStream->type() == CameraStream::Type::Direct) {
> +			/*
> +			 * Create a libcamera buffer using the dmabuf
> +			 * descriptors of the camera3Buffer for each stream and
> +			 * associate it with the Camera3RequestDescriptor for
> +			 * lifetime management only.
> +			 */
> +			buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> +			descriptor->frameBuffers.emplace_back(buffer);
> +
> +		} else {
> +			/*
> +			 * Get the frame buffer from the CameraStream internal
> +			 * buffer pool. The lifetime management of internal
> +			 * buffers is connected to the one of the
> +			 * FrameBufferAllocator instance.
> +			 *
> +			 * The retrieved buffer has to be returned to the
> +			 * allocator once it has been processed.
> +			 */
> +			buffer = getBuffer(stream);
> +		}

I think this could be turned into a switch (cameraStream->type()) 
statement?

>  		if (!buffer) {
>  			LOG(HAL, Error) << "Failed to create buffer";
>  			delete request;
>  			delete descriptor;
>  			return -ENOMEM;
>  		}
> -		descriptor->frameBuffers.emplace_back(buffer);
> -
> -		StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());
> -		Stream *stream = streamConfiguration->stream();
>  
>  		request->addBuffer(stream, buffer);
>  	}
> @@ -1632,6 +1646,13 @@ void CameraDevice::requestComplete(Request *request)
>  		const uint32_t jpeg_orientation = 0;
>  		resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,
>  					 &jpeg_orientation, 1);
> +
> +		/*
> +		 * Return the FrameBuffer to the CameraStream now that we're
> +		 * done processing it.
> +		 */
> +		if (cameraStream->type() == CameraStream::Type::Internal)
> +			returnBuffer(stream, buffer);
>  	}
>  
>  	/* Prepare to call back the Android camera stack. */
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 6ed56ff57dab..f7e9a1e5f494 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1477,6 +1477,8 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
 		CameraStream *cameraStream =
 			static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
+		const StreamConfiguration &config = config_->at(cameraStream->index());
+		Stream *stream = config.stream();
 
 		/*
 		 * Keep track of which stream the request belongs to and store
@@ -1485,27 +1487,39 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		descriptor->buffers[i].stream = camera3Buffers[i].stream;
 		descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
 
-		/* Software streams are handled after hardware streams complete. */
-		if (cameraStream->format() == formats::MJPEG)
+		/* Mapped streams don't need to be added to the Request. */
+		if (cameraStream->type() == CameraStream::Type::Mapped)
 			continue;
 
-		/*
-		 * Create a libcamera buffer using the dmabuf descriptors of
-		 * the camera3Buffer for each stream. The FrameBuffer is
-		 * directly associated with the Camera3RequestDescriptor for
-		 * lifetime management only.
-		 */
-		FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[i].buffer);
+		FrameBuffer *buffer;
+		if (cameraStream->type() == CameraStream::Type::Direct) {
+			/*
+			 * Create a libcamera buffer using the dmabuf
+			 * descriptors of the camera3Buffer for each stream and
+			 * associate it with the Camera3RequestDescriptor for
+			 * lifetime management only.
+			 */
+			buffer = createFrameBuffer(*camera3Buffers[i].buffer);
+			descriptor->frameBuffers.emplace_back(buffer);
+
+		} else {
+			/*
+			 * Get the frame buffer from the CameraStream internal
+			 * buffer pool. The lifetime management of internal
+			 * buffers is connected to the one of the
+			 * FrameBufferAllocator instance.
+			 *
+			 * The retrieved buffer has to be returned to the
+			 * allocator once it has been processed.
+			 */
+			buffer = getBuffer(stream);
+		}
 		if (!buffer) {
 			LOG(HAL, Error) << "Failed to create buffer";
 			delete request;
 			delete descriptor;
 			return -ENOMEM;
 		}
-		descriptor->frameBuffers.emplace_back(buffer);
-
-		StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());
-		Stream *stream = streamConfiguration->stream();
 
 		request->addBuffer(stream, buffer);
 	}
@@ -1632,6 +1646,13 @@  void CameraDevice::requestComplete(Request *request)
 		const uint32_t jpeg_orientation = 0;
 		resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION,
 					 &jpeg_orientation, 1);
+
+		/*
+		 * Return the FrameBuffer to the CameraStream now that we're
+		 * done processing it.
+		 */
+		if (cameraStream->type() == CameraStream::Type::Internal)
+			returnBuffer(stream, buffer);
 	}
 
 	/* Prepare to call back the Android camera stack. */