[PATCH v1] libcamera: request: addBuffer(): Do not destroy fence on failure

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Mon, Mar 03, 2025 at 04:16:34PM +0100, Barnabás Pőcze wrote:
> Take the unique pointer to the `Fence` object by rvalue reference
> so that it is not destroyed if the function returns an error code
> and does not take ownership of the unique pointer.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  include/libcamera/request.h | 2 +-
>  src/libcamera/request.cpp   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index e214a9d13..0c5939f7b 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -53,7 +53,7 @@ public:
>  	ControlList &metadata() { return *metadata_; }
>  	const BufferMap &buffers() const { return bufferMap_; }
>  	int addBuffer(const Stream *stream, FrameBuffer *buffer,
> -		      std::unique_ptr<Fence> fence = nullptr);
> +		      std::unique_ptr<Fence> &&fence = {});

There's one caller in src/android/camera_device.cpp that passes nullptr
as the third argument. Could you fix that ?

The other caller in the same file creates the unique_ptr right before
calling the function, so the fence will be destroyed anyway. I suppose
that's fine for now.

Should the unit test also be extended to validate the new behaviour ?
The function documentation should also be updated.

>  	FrameBuffer *findBuffer(const Stream *stream) const;
>  
>  	uint32_t sequence() const;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index b206ac132..7d02f09fd 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -468,7 +468,7 @@ void Request::reuse(ReuseFlag flags)
>   * \retval -EINVAL The buffer does not reference a valid Stream
>   */
>  int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,
> -		       std::unique_ptr<Fence> fence)
> +		       std::unique_ptr<Fence> &&fence)
>  {
>  	if (!stream) {
>  		LOG(Request, Error) << "Invalid stream reference";

-- 
Regards,

Laurent Pinchart