[v3,3/7] android: Correctly support multiple Mapped streams
diff mbox series

Message ID 20241204164137.3938891-4-chenghaoyang@chromium.org
State New
Headers show
Series
  • Refactor Android HAL before supporting partial result
Related show

Commit Message

Cheng-Hao Yang Dec. 4, 2024, 4:36 p.m. UTC 
The Android camera HAL supports creating streams for the Android
framework by post-processing streams produced by libcamera.

However at the moment a single Mapped stream can be associated with a
Direct stream because, after the first post-processing, the data from
the internal stream are returned preventing further post-processing
passes.

Fix this by storing the list of Direct stream buffers used as the
post-processing source in an Camera3RequestDescriptor::internalBuffers_
map to replace the single internalBuffer_ pointer and return the
internal buffers when the capture request is deleted.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/android/camera_device.cpp  | 66 +++++++++++++++++++---------------
 src/android/camera_request.cpp | 11 +++---
 src/android/camera_request.h   |  3 +-
 3 files changed, 47 insertions(+), 33 deletions(-)

Comments

Jacopo Mondi Dec. 5, 2024, 4:13 p.m. UTC | #1 
Hi Harvey

On Wed, Dec 04, 2024 at 04:36:28PM +0000, Harvey Yang wrote:
> The Android camera HAL supports creating streams for the Android
> framework by post-processing streams produced by libcamera.
>
> However at the moment a single Mapped stream can be associated with a
> Direct stream because, after the first post-processing, the data from
> the internal stream are returned preventing further post-processing
> passes.
>
> Fix this by storing the list of Direct stream buffers used as the

Have I suggested this ? The streams in internalBuffers_ are not Direct
but Internal. So s/Direct/Internal/

> post-processing source in an Camera3RequestDescriptor::internalBuffers_
> map to replace the single internalBuffer_ pointer and return the
> internal buffers when the capture request is deleted.
>
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/android/camera_device.cpp  | 66 +++++++++++++++++++---------------
>  src/android/camera_request.cpp | 11 +++---
>  src/android/camera_request.h   |  3 +-
>  3 files changed, 47 insertions(+), 33 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f6dadaf22..f2dd8d4fd 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -966,9 +966,10 @@ 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:map<>, no duplications can happen.
> +	 * Since directBuffers is an std:map<>, no duplications can
> +	 * happen.

nit: fits on the previous line

>  	 */
> -	std::map<CameraStream *, libcamera::FrameBuffer *> requestedBuffers;
> +	std::map<CameraStream *, libcamera::FrameBuffer *> directBuffers;
>  	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
>  		CameraStream *cameraStream = buffer.stream;
>  		camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> @@ -1007,6 +1008,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  						  cameraStream->configuration().size);
>  			frameBuffer = buffer.frameBuffer.get();
>  			acquireFence = std::move(buffer.fence);
> +
> +			directBuffers[cameraStream] = frameBuffer;
>  			LOG(HAL, Debug) << ss.str() << " (direct)";
>  			break;
>
> @@ -1015,13 +1018,17 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			 * Get the frame buffer from the CameraStream internal
>  			 * buffer pool.
>  			 *
> -			 * The buffer has to be returned to the CameraStream
> -			 * once it has been processed.
> +			 * The buffer will be returned to the CameraStream when
> +			 * the request is destroyed.
>  			 */
>  			frameBuffer = cameraStream->getBuffer();
> -			buffer.internalBuffer = frameBuffer;
>  			buffer.srcBuffer = frameBuffer;
>
> +			/*
> +			 * Track the allocated internal buffers, which will be
> +			 * recycled when the descriptor destroyed.

nit: "is destroyed"

> +			 */
> +			descriptor->internalBuffers_[cameraStream] = frameBuffer;
>  			LOG(HAL, Debug) << ss.str() << " (internal)";
>
>  			descriptor->pendingStreamsToProcess_.insert(
> @@ -1037,8 +1044,6 @@ 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));
> -
> -		requestedBuffers[cameraStream] = frameBuffer;
>  	}
>
>  	/*
> @@ -1076,24 +1081,43 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		 * post-processing. No need to recycle the buffer since it's
>  		 * owned by Android.
>  		 */
> -		auto iterDirectBuffer = requestedBuffers.find(sourceStream);
> -		if (iterDirectBuffer != requestedBuffers.end()) {
> +		auto iterDirectBuffer = directBuffers.find(sourceStream);
> +		if (iterDirectBuffer != directBuffers.end()) {
>  			buffer.srcBuffer = iterDirectBuffer->second;
>  			continue;
>  		}
>
>  		/*
> -		 * If that's not the case, we need to add a buffer to the request
> -		 * for this stream.
> +		 * If that's not the case, we use an internal buffer allocated
> +		 * from the source stream.
> +		 */
> +
> +		/*
> +		 * If an internal buffer has been requested for the source
> +		 * stream before, we should reuse it.
>  		 */
> -		FrameBuffer *frameBuffer = cameraStream->getBuffer();
> -		buffer.internalBuffer = frameBuffer;
> +		auto iterInternalBuffer = descriptor->internalBuffers_.find(sourceStream);
> +		if (iterInternalBuffer != descriptor->internalBuffers_.end()) {
> +			buffer.srcBuffer = iterInternalBuffer->second;
> +			continue;
> +		}
> +
> +		/*
> +		 * Otherwise, we need to create an internal buffer to the
> +		 * request for the source stream. Get the frame buffer from the
> +		 * source stream's internal buffer pool.
> +		 *
> +		 * The buffer will be returned to the CameraStream when the
> +		 * request is destructed.
> +		 */
> +		FrameBuffer *frameBuffer = sourceStream->getBuffer();
>  		buffer.srcBuffer = frameBuffer;
>
>  		descriptor->request_->addBuffer(sourceStream->stream(),
>  						frameBuffer, nullptr);
>
> -		requestedBuffers[sourceStream] = frameBuffer;
> +		/* Track the allocated internal buffer. */
> +		descriptor->internalBuffers_[sourceStream] = frameBuffer;

Fine with me.

I wonder if inverting the logic wouldn't make it more clear


		/*
		 * If that's not the case, we use an internal buffer allocated
		 * from the source stream.
		 */

		auto iterInternalBuffer = descriptor->internalBuffers_.find(sourceStream);
		if (iterInternalBuffer == descriptor->internalBuffers_.end()) {
			/*
			 * We need to create an internal buffer to the request
			 * for the source stream. Get the frame buffer from the
			 * source stream's internal buffer pool.
			 *
			 * The buffer will be returned to the CameraStream when
			 * the request is destructed.
			 */
			FrameBuffer *frameBuffer = sourceStream->getBuffer();
			buffer.srcBuffer = frameBuffer;

			descriptor->request_->addBuffer(sourceStream->stream(),
							frameBuffer, nullptr);

			/* Track the allocated internal buffer. */
			descriptor->internalBuffers_[sourceStream] = frameBuffer;

			continue;
		}

		/*
		 * Otherwise if an internal buffer has already been requested
		 * for the source stream before, we should reuse it.
		 */
		buffer.srcBuffer = iterInternalBuffer->second;

Up to you.

With the two nits fixed, and with or without the above change
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>  	}
>
>  	/*
> @@ -1253,13 +1277,6 @@ void CameraDevice::requestComplete(Request *request)
>  		if (ret) {
>  			setBufferStatus(*buffer, StreamBuffer::Status::Error);
>  			descriptor->pendingStreamsToProcess_.erase(stream);
> -
> -			/*
> -			 * If the framebuffer is internal to CameraStream return
> -			 * it back now that we're done processing it.
> -			 */
> -			if (buffer->internalBuffer)
> -				stream->putBuffer(buffer->internalBuffer);
>  		}
>  	}
>
> @@ -1378,13 +1395,6 @@ void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,
>  {
>  	setBufferStatus(*streamBuffer, status);
>
> -	/*
> -	 * If the framebuffer is internal to CameraStream return it back now
> -	 * that we're done processing it.
> -	 */
> -	if (streamBuffer->internalBuffer)
> -		streamBuffer->stream->putBuffer(streamBuffer->internalBuffer);
> -
>  	Camera3RequestDescriptor *request = streamBuffer->request;
>
>  	{
> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> index 52a3ac1f7..92e74ab6a 100644
> --- a/src/android/camera_request.cpp
> +++ b/src/android/camera_request.cpp
> @@ -10,6 +10,7 @@
>  #include <libcamera/base/span.h>
>
>  #include "camera_buffer.h"
> +#include "camera_stream.h"
>
>  using namespace libcamera;
>
> @@ -138,7 +139,12 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
>  	request_ = camera->createRequest(reinterpret_cast<uint64_t>(this));
>  }
>
> -Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> +Camera3RequestDescriptor::~Camera3RequestDescriptor()
> +{
> +	/* Recycle the allocated internal buffer back to its source stream. */
> +	for (auto &[sourceStream, frameBuffer] : internalBuffers_)
> +		sourceStream->putBuffer(frameBuffer);
> +}
>
>  /**
>   * \class StreamBuffer
> @@ -166,9 +172,6 @@ Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
>   * \var StreamBuffer::status
>   * \brief Track the status of the buffer
>   *
> - * \var StreamBuffer::internalBuffer
> - * \brief Pointer to a buffer internally handled by CameraStream (if any)
> - *
>   * \var StreamBuffer::srcBuffer
>   * \brief Pointer to the source frame buffer used for post-processing
>   *
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index 335f1985d..6b2a00795 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -49,7 +49,6 @@ public:
>  	std::unique_ptr<HALFrameBuffer> frameBuffer;
>  	libcamera::UniqueFD fence;
>  	Status status = Status::Success;
> -	libcamera::FrameBuffer *internalBuffer = nullptr;
>  	const libcamera::FrameBuffer *srcBuffer = nullptr;
>  	std::unique_ptr<CameraBuffer> dstBuffer;
>  	Camera3RequestDescriptor *request;
> @@ -85,6 +84,8 @@ public:
>  	std::unique_ptr<libcamera::Request> request_;
>  	std::unique_ptr<CameraMetadata> resultMetadata_;
>
> +	std::map<CameraStream *, libcamera::FrameBuffer *> internalBuffers_;
> +
>  	bool complete_ = false;
>  	Status status_ = Status::Success;
>
> --
> 2.47.0.338.g60cca15819-goog
>
Cheng-Hao Yang Dec. 9, 2024, 5:23 a.m. UTC | #2 
Hi Jacopo,

Will upload a new version later.

On Fri, Dec 6, 2024 at 12:13 AM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Wed, Dec 04, 2024 at 04:36:28PM +0000, Harvey Yang wrote:
> > The Android camera HAL supports creating streams for the Android
> > framework by post-processing streams produced by libcamera.
> >
> > However at the moment a single Mapped stream can be associated with a
> > Direct stream because, after the first post-processing, the data from
> > the internal stream are returned preventing further post-processing
> > passes.
> >
> > Fix this by storing the list of Direct stream buffers used as the
>
> Have I suggested this ? The streams in internalBuffers_ are not Direct
> but Internal. So s/Direct/Internal/

Ah right, I should've fixed it. Thanks!

>
> > post-processing source in an Camera3RequestDescriptor::internalBuffers_
> > map to replace the single internalBuffer_ pointer and return the
> > internal buffers when the capture request is deleted.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  src/android/camera_device.cpp  | 66 +++++++++++++++++++---------------
> >  src/android/camera_request.cpp | 11 +++---
> >  src/android/camera_request.h   |  3 +-
> >  3 files changed, 47 insertions(+), 33 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index f6dadaf22..f2dd8d4fd 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -966,9 +966,10 @@ 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:map<>, no duplications can happen.
> > +      * Since directBuffers is an std:map<>, no duplications can
> > +      * happen.
>
> nit: fits on the previous line

Done

>
> >        */
> > -     std::map<CameraStream *, libcamera::FrameBuffer *> requestedBuffers;
> > +     std::map<CameraStream *, libcamera::FrameBuffer *> directBuffers;
> >       for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
> >               CameraStream *cameraStream = buffer.stream;
> >               camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
> > @@ -1007,6 +1008,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                                                 cameraStream->configuration().size);
> >                       frameBuffer = buffer.frameBuffer.get();
> >                       acquireFence = std::move(buffer.fence);
> > +
> > +                     directBuffers[cameraStream] = frameBuffer;
> >                       LOG(HAL, Debug) << ss.str() << " (direct)";
> >                       break;
> >
> > @@ -1015,13 +1018,17 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                        * Get the frame buffer from the CameraStream internal
> >                        * buffer pool.
> >                        *
> > -                      * The buffer has to be returned to the CameraStream
> > -                      * once it has been processed.
> > +                      * The buffer will be returned to the CameraStream when
> > +                      * the request is destroyed.
> >                        */
> >                       frameBuffer = cameraStream->getBuffer();
> > -                     buffer.internalBuffer = frameBuffer;
> >                       buffer.srcBuffer = frameBuffer;
> >
> > +                     /*
> > +                      * Track the allocated internal buffers, which will be
> > +                      * recycled when the descriptor destroyed.
>
> nit: "is destroyed"

Done

>
> > +                      */
> > +                     descriptor->internalBuffers_[cameraStream] = frameBuffer;
> >                       LOG(HAL, Debug) << ss.str() << " (internal)";
> >
> >                       descriptor->pendingStreamsToProcess_.insert(
> > @@ -1037,8 +1044,6 @@ 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));
> > -
> > -             requestedBuffers[cameraStream] = frameBuffer;
> >       }
> >
> >       /*
> > @@ -1076,24 +1081,43 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                * post-processing. No need to recycle the buffer since it's
> >                * owned by Android.
> >                */
> > -             auto iterDirectBuffer = requestedBuffers.find(sourceStream);
> > -             if (iterDirectBuffer != requestedBuffers.end()) {
> > +             auto iterDirectBuffer = directBuffers.find(sourceStream);
> > +             if (iterDirectBuffer != directBuffers.end()) {
> >                       buffer.srcBuffer = iterDirectBuffer->second;
> >                       continue;
> >               }
> >
> >               /*
> > -              * If that's not the case, we need to add a buffer to the request
> > -              * for this stream.
> > +              * If that's not the case, we use an internal buffer allocated
> > +              * from the source stream.
> > +              */
> > +
> > +             /*
> > +              * If an internal buffer has been requested for the source
> > +              * stream before, we should reuse it.
> >                */
> > -             FrameBuffer *frameBuffer = cameraStream->getBuffer();
> > -             buffer.internalBuffer = frameBuffer;
> > +             auto iterInternalBuffer = descriptor->internalBuffers_.find(sourceStream);
> > +             if (iterInternalBuffer != descriptor->internalBuffers_.end()) {
> > +                     buffer.srcBuffer = iterInternalBuffer->second;
> > +                     continue;
> > +             }
> > +
> > +             /*
> > +              * Otherwise, we need to create an internal buffer to the
> > +              * request for the source stream. Get the frame buffer from the
> > +              * source stream's internal buffer pool.
> > +              *
> > +              * The buffer will be returned to the CameraStream when the
> > +              * request is destructed.
> > +              */
> > +             FrameBuffer *frameBuffer = sourceStream->getBuffer();
> >               buffer.srcBuffer = frameBuffer;
> >
> >               descriptor->request_->addBuffer(sourceStream->stream(),
> >                                               frameBuffer, nullptr);
> >
> > -             requestedBuffers[sourceStream] = frameBuffer;
> > +             /* Track the allocated internal buffer. */
> > +             descriptor->internalBuffers_[sourceStream] = frameBuffer;
>
> Fine with me.
>
> I wonder if inverting the logic wouldn't make it more clear
>
>
>                 /*
>                  * If that's not the case, we use an internal buffer allocated
>                  * from the source stream.
>                  */
>
>                 auto iterInternalBuffer = descriptor->internalBuffers_.find(sourceStream);
>                 if (iterInternalBuffer == descriptor->internalBuffers_.end()) {
>                         /*
>                          * We need to create an internal buffer to the request
>                          * for the source stream. Get the frame buffer from the
>                          * source stream's internal buffer pool.
>                          *
>                          * The buffer will be returned to the CameraStream when
>                          * the request is destructed.
>                          */
>                         FrameBuffer *frameBuffer = sourceStream->getBuffer();
>                         buffer.srcBuffer = frameBuffer;
>
>                         descriptor->request_->addBuffer(sourceStream->stream(),
>                                                         frameBuffer, nullptr);
>
>                         /* Track the allocated internal buffer. */
>                         descriptor->internalBuffers_[sourceStream] = frameBuffer;
>
>                         continue;
>                 }
>
>                 /*
>                  * Otherwise if an internal buffer has already been requested
>                  * for the source stream before, we should reuse it.
>                  */
>                 buffer.srcBuffer = iterInternalBuffer->second;
>
> Up to you.

Ah, I understand that reverting the logic would make the comment
clearer, while I still prefer the current order, which I believe also
involves less line changes.


>
> With the two nits fixed, and with or without the above change
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks!

BR,
Harvey

>
> Thanks
>   j
>
> >       }
> >
> >       /*
> > @@ -1253,13 +1277,6 @@ void CameraDevice::requestComplete(Request *request)
> >               if (ret) {
> >                       setBufferStatus(*buffer, StreamBuffer::Status::Error);
> >                       descriptor->pendingStreamsToProcess_.erase(stream);
> > -
> > -                     /*
> > -                      * If the framebuffer is internal to CameraStream return
> > -                      * it back now that we're done processing it.
> > -                      */
> > -                     if (buffer->internalBuffer)
> > -                             stream->putBuffer(buffer->internalBuffer);
> >               }
> >       }
> >
> > @@ -1378,13 +1395,6 @@ void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,
> >  {
> >       setBufferStatus(*streamBuffer, status);
> >
> > -     /*
> > -      * If the framebuffer is internal to CameraStream return it back now
> > -      * that we're done processing it.
> > -      */
> > -     if (streamBuffer->internalBuffer)
> > -             streamBuffer->stream->putBuffer(streamBuffer->internalBuffer);
> > -
> >       Camera3RequestDescriptor *request = streamBuffer->request;
> >
> >       {
> > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> > index 52a3ac1f7..92e74ab6a 100644
> > --- a/src/android/camera_request.cpp
> > +++ b/src/android/camera_request.cpp
> > @@ -10,6 +10,7 @@
> >  #include <libcamera/base/span.h>
> >
> >  #include "camera_buffer.h"
> > +#include "camera_stream.h"
> >
> >  using namespace libcamera;
> >
> > @@ -138,7 +139,12 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
> >       request_ = camera->createRequest(reinterpret_cast<uint64_t>(this));
> >  }
> >
> > -Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> > +Camera3RequestDescriptor::~Camera3RequestDescriptor()
> > +{
> > +     /* Recycle the allocated internal buffer back to its source stream. */
> > +     for (auto &[sourceStream, frameBuffer] : internalBuffers_)
> > +             sourceStream->putBuffer(frameBuffer);
> > +}
> >
> >  /**
> >   * \class StreamBuffer
> > @@ -166,9 +172,6 @@ Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> >   * \var StreamBuffer::status
> >   * \brief Track the status of the buffer
> >   *
> > - * \var StreamBuffer::internalBuffer
> > - * \brief Pointer to a buffer internally handled by CameraStream (if any)
> > - *
> >   * \var StreamBuffer::srcBuffer
> >   * \brief Pointer to the source frame buffer used for post-processing
> >   *
> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> > index 335f1985d..6b2a00795 100644
> > --- a/src/android/camera_request.h
> > +++ b/src/android/camera_request.h
> > @@ -49,7 +49,6 @@ public:
> >       std::unique_ptr<HALFrameBuffer> frameBuffer;
> >       libcamera::UniqueFD fence;
> >       Status status = Status::Success;
> > -     libcamera::FrameBuffer *internalBuffer = nullptr;
> >       const libcamera::FrameBuffer *srcBuffer = nullptr;
> >       std::unique_ptr<CameraBuffer> dstBuffer;
> >       Camera3RequestDescriptor *request;
> > @@ -85,6 +84,8 @@ public:
> >       std::unique_ptr<libcamera::Request> request_;
> >       std::unique_ptr<CameraMetadata> resultMetadata_;
> >
> > +     std::map<CameraStream *, libcamera::FrameBuffer *> internalBuffers_;
> > +
> >       bool complete_ = false;
> >       Status status_ = Status::Success;
> >
> > --
> > 2.47.0.338.g60cca15819-goog
> >

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index f6dadaf22..f2dd8d4fd 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -966,9 +966,10 @@  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:map<>, no duplications can happen.
+	 * Since directBuffers is an std:map<>, no duplications can
+	 * happen.
 	 */
-	std::map<CameraStream *, libcamera::FrameBuffer *> requestedBuffers;
+	std::map<CameraStream *, libcamera::FrameBuffer *> directBuffers;
 	for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
 		CameraStream *cameraStream = buffer.stream;
 		camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
@@ -1007,6 +1008,8 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 						  cameraStream->configuration().size);
 			frameBuffer = buffer.frameBuffer.get();
 			acquireFence = std::move(buffer.fence);
+
+			directBuffers[cameraStream] = frameBuffer;
 			LOG(HAL, Debug) << ss.str() << " (direct)";
 			break;
 
@@ -1015,13 +1018,17 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			 * Get the frame buffer from the CameraStream internal
 			 * buffer pool.
 			 *
-			 * The buffer has to be returned to the CameraStream
-			 * once it has been processed.
+			 * The buffer will be returned to the CameraStream when
+			 * the request is destroyed.
 			 */
 			frameBuffer = cameraStream->getBuffer();
-			buffer.internalBuffer = frameBuffer;
 			buffer.srcBuffer = frameBuffer;
 
+			/*
+			 * Track the allocated internal buffers, which will be
+			 * recycled when the descriptor destroyed.
+			 */
+			descriptor->internalBuffers_[cameraStream] = frameBuffer;
 			LOG(HAL, Debug) << ss.str() << " (internal)";
 
 			descriptor->pendingStreamsToProcess_.insert(
@@ -1037,8 +1044,6 @@  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));
-
-		requestedBuffers[cameraStream] = frameBuffer;
 	}
 
 	/*
@@ -1076,24 +1081,43 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		 * post-processing. No need to recycle the buffer since it's
 		 * owned by Android.
 		 */
-		auto iterDirectBuffer = requestedBuffers.find(sourceStream);
-		if (iterDirectBuffer != requestedBuffers.end()) {
+		auto iterDirectBuffer = directBuffers.find(sourceStream);
+		if (iterDirectBuffer != directBuffers.end()) {
 			buffer.srcBuffer = iterDirectBuffer->second;
 			continue;
 		}
 
 		/*
-		 * If that's not the case, we need to add a buffer to the request
-		 * for this stream.
+		 * If that's not the case, we use an internal buffer allocated
+		 * from the source stream.
+		 */
+
+		/*
+		 * If an internal buffer has been requested for the source
+		 * stream before, we should reuse it.
 		 */
-		FrameBuffer *frameBuffer = cameraStream->getBuffer();
-		buffer.internalBuffer = frameBuffer;
+		auto iterInternalBuffer = descriptor->internalBuffers_.find(sourceStream);
+		if (iterInternalBuffer != descriptor->internalBuffers_.end()) {
+			buffer.srcBuffer = iterInternalBuffer->second;
+			continue;
+		}
+
+		/*
+		 * Otherwise, we need to create an internal buffer to the
+		 * request for the source stream. Get the frame buffer from the
+		 * source stream's internal buffer pool.
+		 *
+		 * The buffer will be returned to the CameraStream when the
+		 * request is destructed.
+		 */
+		FrameBuffer *frameBuffer = sourceStream->getBuffer();
 		buffer.srcBuffer = frameBuffer;
 
 		descriptor->request_->addBuffer(sourceStream->stream(),
 						frameBuffer, nullptr);
 
-		requestedBuffers[sourceStream] = frameBuffer;
+		/* Track the allocated internal buffer. */
+		descriptor->internalBuffers_[sourceStream] = frameBuffer;
 	}
 
 	/*
@@ -1253,13 +1277,6 @@  void CameraDevice::requestComplete(Request *request)
 		if (ret) {
 			setBufferStatus(*buffer, StreamBuffer::Status::Error);
 			descriptor->pendingStreamsToProcess_.erase(stream);
-
-			/*
-			 * If the framebuffer is internal to CameraStream return
-			 * it back now that we're done processing it.
-			 */
-			if (buffer->internalBuffer)
-				stream->putBuffer(buffer->internalBuffer);
 		}
 	}
 
@@ -1378,13 +1395,6 @@  void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,
 {
 	setBufferStatus(*streamBuffer, status);
 
-	/*
-	 * If the framebuffer is internal to CameraStream return it back now
-	 * that we're done processing it.
-	 */
-	if (streamBuffer->internalBuffer)
-		streamBuffer->stream->putBuffer(streamBuffer->internalBuffer);
-
 	Camera3RequestDescriptor *request = streamBuffer->request;
 
 	{
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
index 52a3ac1f7..92e74ab6a 100644
--- a/src/android/camera_request.cpp
+++ b/src/android/camera_request.cpp
@@ -10,6 +10,7 @@ 
 #include <libcamera/base/span.h>
 
 #include "camera_buffer.h"
+#include "camera_stream.h"
 
 using namespace libcamera;
 
@@ -138,7 +139,12 @@  Camera3RequestDescriptor::Camera3RequestDescriptor(
 	request_ = camera->createRequest(reinterpret_cast<uint64_t>(this));
 }
 
-Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
+Camera3RequestDescriptor::~Camera3RequestDescriptor()
+{
+	/* Recycle the allocated internal buffer back to its source stream. */
+	for (auto &[sourceStream, frameBuffer] : internalBuffers_)
+		sourceStream->putBuffer(frameBuffer);
+}
 
 /**
  * \class StreamBuffer
@@ -166,9 +172,6 @@  Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
  * \var StreamBuffer::status
  * \brief Track the status of the buffer
  *
- * \var StreamBuffer::internalBuffer
- * \brief Pointer to a buffer internally handled by CameraStream (if any)
- *
  * \var StreamBuffer::srcBuffer
  * \brief Pointer to the source frame buffer used for post-processing
  *
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index 335f1985d..6b2a00795 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -49,7 +49,6 @@  public:
 	std::unique_ptr<HALFrameBuffer> frameBuffer;
 	libcamera::UniqueFD fence;
 	Status status = Status::Success;
-	libcamera::FrameBuffer *internalBuffer = nullptr;
 	const libcamera::FrameBuffer *srcBuffer = nullptr;
 	std::unique_ptr<CameraBuffer> dstBuffer;
 	Camera3RequestDescriptor *request;
@@ -85,6 +84,8 @@  public:
 	std::unique_ptr<libcamera::Request> request_;
 	std::unique_ptr<CameraMetadata> resultMetadata_;
 
+	std::map<CameraStream *, libcamera::FrameBuffer *> internalBuffers_;
+
 	bool complete_ = false;
 	Status status_ = Status::Success;