[libcamera-devel] [PATCH v2 3/5] android: camera_device: Postpone mapped streams handling

[Jacopo Mondi]

Hi Paul

On Fri, May 27, 2022 at 06:34:38PM +0900, Paul Elder wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> Mapped streams are generated by post-processing and always require a
> source buffer from where to process image data from.
>
> In case a Mapped stream is requested but its source stream is not, it
> is required to allocate a buffer on the fly and add it to the
> libcamera::Request.
>
> Make sure a source stream is available for all mapped streams, and if
> that's not the case, add a dedicated buffer to the request for that
> purpose.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - cosmetic changes in code
> - fix typo in the commit message
> ---
>  src/android/camera_device.cpp | 80 +++++++++++++++++++++++++++++++----
>  1 file changed, 72 insertions(+), 8 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 20599665..95c14f60 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -9,6 +9,7 @@
>
>  #include <algorithm>
>  #include <fstream>
> +#include <set>
>  #include <sys/mman.h>
>  #include <unistd.h>
>  #include <vector>
> @@ -923,6 +924,32 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie()
>  			<< " with " << descriptor->buffers_.size() << " streams";
>
> +	/*
> +	 * Collect the CameraStream associated to each requested capture stream.
> +	 * Since requestedStreams is an std:set<>, no duplications can happen.
                                        std::set<>

> +	 */
> +	std::set<CameraStream *> requestedStreams;
> +	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
> +		CameraStream *cameraStream = buffer.stream;
> +
> +		switch (cameraStream->type()) {
> +		case CameraStream::Type::Mapped:
> +			requestedStreams.insert(cameraStream->sourceStream());
> +			break;
> +
> +		case CameraStream::Type::Direct:
> +		case CameraStream::Type::Internal:
> +			requestedStreams.insert(cameraStream);
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * Process all the Direct and Internal streams, for which the CameraStream
> +	 * they refer to is the one that points to the right libcamera::Stream.
> +	 *
> +	 * Streams of type Mapped will be handled later.
> +	 */
>  	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>  		CameraStream *cameraStream = buffer.stream;
>  		camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> @@ -945,14 +972,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>
>  		switch (cameraStream->type()) {
>  		case CameraStream::Type::Mapped:
> -			/*
> -			 * Mapped streams don't need buffers added to the
> -			 * Request.
> -			 */
> -			LOG(HAL, Debug) << ss.str() << " (mapped)";
> -
> -			descriptor->pendingStreamsToProcess_.insert(
> -				{ cameraStream, &buffer });
>  			continue;
>
>  		case CameraStream::Type::Direct:
> @@ -996,6 +1015,51 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		auto fence = std::make_unique<Fence>(std::move(acquireFence));
>  		descriptor->request_->addBuffer(cameraStream->stream(),
>  						frameBuffer, std::move(fence));
> +
> +		requestedStreams.erase(cameraStream);
> +	}
> +
> +	/*
> +	 * Now handle the Mapped streams. If no buffer has been added for them
> +	 * as their corresponding direct source stream has not been requested,
> +	 * add it here.
> +	 */
> +	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
> +		CameraStream *cameraStream = buffer.stream;
> +		camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> +
> +		if (cameraStream->type() != CameraStream::Type::Mapped)
> +			continue;
> +
> +		LOG(HAL, Debug) << i << " - (" << camera3Stream->width << "x"
> +				<< camera3Stream->height << ")"
> +				<< "[" << utils::hex(camera3Stream->format) << "] -> "
> +				<< "(" << cameraStream->configuration().size.toString() << ")["
> +				<< cameraStream->configuration().pixelFormat.toString() << "]"
> +				<< " (mapped)";
> +
> +		MutexLocker lock(descriptor->streamsProcessMutex_);
> +		descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });
> +
> +		/*
> +		 * Make sure the CameraStream this stream is mapped on has been
> +		 * added to the request.
> +		 */
> +		CameraStream *sourceStream = cameraStream->sourceStream();
> +		if (requestedStreams.find(sourceStream) == requestedStreams.end())
> +			continue;
> +
> +		/*
> +		 * If that's not the case, we need to add a buffer to the request
> +		 * for this stream.
> +		 */
> +		FrameBuffer *frameBuffer = cameraStream->getBuffer();
> +		buffer.internalBuffer = frameBuffer;
> +
> +		descriptor->request_->addBuffer(sourceStream->stream(),
> +						frameBuffer, nullptr);
> +
> +		requestedStreams.erase(sourceStream);
>  	}

We could possibily make sure here that now requestedStreams is empty,
but that's just an additional safety check.

The patch still looks ok to me.

>
>  	/*
> --
> 2.30.2
>