[PATCH v4 2/7] android: Set StreamBuffer::srcBuffer in CameraDevice::processCaptureRequest

[Jacopo Mondi]

Hi Harvey

On Tue, Dec 10, 2024 at 02:23:55PM +0000, Harvey Yang wrote:
> StreamBuffer::srcBuffer was set right before being processed, while it
> was already determined when being constructed. This patch sets the value
> in CameraDevice::processCaptureRequest. `requestedStreams` is refactored
> from std::set to std::map to find the correct srcBuffer accordingly.
>
> The change also helps cleaning up the post process pipeline's dependency
> on `Camera3RequestDescriptor::pendingStreamsToProcess_`, which is going
> to be cleaned up in the following patches.

Does this still apply ?

>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  src/android/camera_device.cpp | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4e3bdc9cc..f6dadaf22 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -9,7 +9,6 @@
>
>  #include <algorithm>
>  #include <fstream>
> -#include <set>
>  #include <sys/mman.h>
>  #include <unistd.h>
>  #include <vector>
> @@ -967,9 +966,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	 * to a libcamera stream. Streams of type Mapped will be handled later.
>  	 *
>  	 * Collect the CameraStream associated to each requested capture stream.
> -	 * Since requestedStreams is an std:set<>, no duplications can happen.
> +	 * Since requestedStreams is an std:map<>, no duplications can happen.
>  	 */
> -	std::set<CameraStream *> requestedStreams;
> +	std::map<CameraStream *, libcamera::FrameBuffer *> requestedBuffers;
>  	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>  		CameraStream *cameraStream = buffer.stream;
>  		camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> @@ -1021,6 +1020,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			 */
>  			frameBuffer = cameraStream->getBuffer();
>  			buffer.internalBuffer = frameBuffer;
> +			buffer.srcBuffer = frameBuffer;
> +
>  			LOG(HAL, Debug) << ss.str() << " (internal)";
>
>  			descriptor->pendingStreamsToProcess_.insert(
> @@ -1037,7 +1038,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		descriptor->request_->addBuffer(cameraStream->stream(),
>  						frameBuffer, std::move(fence));
>
> -		requestedStreams.insert(cameraStream);
> +		requestedBuffers[cameraStream] = frameBuffer;
>  	}
>
>  	/*
> @@ -1067,9 +1068,19 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		 * added to the request.
>  		 */
>  		CameraStream *sourceStream = cameraStream->sourceStream();
> -		ASSERT(sourceStream);
> -		if (requestedStreams.find(sourceStream) != requestedStreams.end())
> +		ASSERT(sourceStream && sourceStream->type() == CameraStream::Type::Direct);
> +
> +		/*
> +		 * If the buffer for the source stream has been requested as
> +		 * Direct, use its framebuffer as the source buffer for
> +		 * post-processing. No need to recycle the buffer since it's
> +		 * owned by Android.
> +		 */
> +		auto iterDirectBuffer = requestedBuffers.find(sourceStream);
> +		if (iterDirectBuffer != requestedBuffers.end()) {
> +			buffer.srcBuffer = iterDirectBuffer->second;
>  			continue;
> +		}
>
>  		/*
>  		 * If that's not the case, we need to add a buffer to the request
> @@ -1077,11 +1088,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		 */
>  		FrameBuffer *frameBuffer = cameraStream->getBuffer();
>  		buffer.internalBuffer = frameBuffer;
> +		buffer.srcBuffer = frameBuffer;
>
>  		descriptor->request_->addBuffer(sourceStream->stream(),
>  						frameBuffer, nullptr);
>
> -		requestedStreams.insert(sourceStream);
> +		requestedBuffers[sourceStream] = frameBuffer;
>  	}
>
>  	/*
> @@ -1236,8 +1248,6 @@ void CameraDevice::requestComplete(Request *request)
>  			continue;
>  		}
>
> -		buffer->srcBuffer = src;
> -

In this new version of the series I see that setting srcBuffer earlire
allows to validate the buffer error status in 5/7, so

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>  		++iter;
>  		int ret = stream->process(buffer);
>  		if (ret) {
> --
> 2.47.0.338.g60cca15819-goog
>