[libcamera-devel,3/5] android: camera_device: Post-pone mapped streams handling
diff mbox series

Message ID 20220110165524.72978-4-jacopo@jmondi.org
State Not Applicable
Delegated to: Jacopo Mondi
Headers show
Series
  • android: Plumb the YUV processor in
Related show

Commit Message

Jacopo Mondi Jan. 10, 2022, 4:55 p.m. UTC
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@jmondi.org>
---
 src/android/camera_device.cpp | 81 +++++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 8 deletions(-)

Comments

Hirokazu Honda Jan. 12, 2022, 7:31 a.m. UTC | #1
Hi Jacopo,


On Tue, Jan 11, 2022 at 1:54 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> 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@jmondi.org>
> ---
>  src/android/camera_device.cpp | 81 +++++++++++++++++++++++++++++++----
>  1 file changed, 73 insertions(+), 8 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 005d95b51a0c..a44f199d25d8 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>
> @@ -924,6 +925,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.
> +        * Being requestedStreams an std::set<> no duplications can happen.
> +        */
> +       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();
> @@ -946,14 +973,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:
> @@ -997,6 +1016,52 @@ 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 addded for them

nit: the Mapped streams
s/added/addded/

> +        * 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();
> +               CameraStream *sourceStream = cameraStream->sourceStream();
> +               FrameBuffer *frameBuffer = nullptr;
> +
> +               if (cameraStream->type() != CameraStream::Type::Mapped)
> +                       continue;

I would move camera3Stream here.

> +
> +               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 });
> +

I move sourceStrfeam here.

> +               /*
> +                * Make sure the CameraStream this stream is mapped on has been
> +                * added to the request.
> +                */
> +               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 = cameraStream->getBuffer();
nit: FrameBuffer *frameBuffer =

> +               buffer.internalBuffer = frameBuffer;
> +
> +               descriptor->request_->addBuffer(sourceStream->stream(),
> +                                               frameBuffer, nullptr);
> +
> +               requestedStreams.erase(sourceStream);
>         }

Shall we check requestedStreams is empty?

Best Regards,
-Hiro
>
>         /*
> --
> 2.34.1
>

Patch
diff mbox series

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 005d95b51a0c..a44f199d25d8 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>
@@ -924,6 +925,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.
+	 * Being requestedStreams an std::set<> no duplications can happen.
+	 */
+	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();
@@ -946,14 +973,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:
@@ -997,6 +1016,52 @@  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 addded 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();
+		CameraStream *sourceStream = cameraStream->sourceStream();
+		FrameBuffer *frameBuffer = nullptr;
+
+		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.
+		 */
+		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 = cameraStream->getBuffer();
+		buffer.internalBuffer = frameBuffer;
+
+		descriptor->request_->addBuffer(sourceStream->stream(),
+						frameBuffer, nullptr);
+
+		requestedStreams.erase(sourceStream);
 	}
 
 	/*