[libcamera-devel] [PATCH v2] android: Camera3RequestDescriptor: Provide a constructor for StreamBuffer

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Wed, Oct 27, 2021 at 07:46:23PM +0530, Umang Jain wrote:
> Provide a constructor for StreamBuffer and use that while populating
> Camera3RequestDescriptor::buffers_ vector. Also provide the default
> move-constructor (required as StreamBuffer is stored in a vector in
> Camera3RequestDescriptor) and destructor for the StreamBuffer struct.
> 
> Also declare a default move assignment operator and disable the
> copy constructor and move operator explicitly with
> LIBCAMERA_DISABLE_COPY().
> 
> While at it, initialize pointers members in the StreamBuffer struct
> to nullptr, with StreamBuffer::status set to Status::Success.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> Changes in v2:
>  - Declare default move-assignment operator which is then explicitly
>    disabled by LIBCAMERA_DISABLE_MOVE() macro along with copy
>    constructor (enforced by the macro).
> ---
>  src/android/camera_request.cpp | 16 +++++++++++++---
>  src/android/camera_request.h   | 16 +++++++++++++---
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> index 5bac1b8f..0b2946fb 100644
> --- a/src/android/camera_request.cpp
> +++ b/src/android/camera_request.cpp
> @@ -37,9 +37,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
>  		CameraStream *stream =
>  			static_cast<CameraStream *>(buffer.stream->priv);
>  
> -		buffers_.push_back({ stream, buffer.buffer, nullptr,
> -				     buffer.acquire_fence, Status::Success,
> -				     nullptr, nullptr, nullptr, this });
> +		buffers_.emplace_back(stream, buffer, this);
>  	}
>  
>  	/* Clone the controls associated with the camera3 request. */
> @@ -54,3 +52,15 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
>  }
>  
>  Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> +
> +Camera3RequestDescriptor::StreamBuffer::StreamBuffer(
> +	CameraStream *stream, const camera3_stream_buffer_t &buffer,
> +	Camera3RequestDescriptor *request)
> +	: camera3Buffer(buffer.buffer), fence(buffer.acquire_fence),
> +	  stream(stream), request(request)
> +{
> +}
> +
> +Camera3RequestDescriptor::StreamBuffer::~StreamBuffer() = default;
> +
> +Camera3RequestDescriptor::StreamBuffer::StreamBuffer(StreamBuffer &&other) = default;
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index cfafa445..618e1fc1 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -34,15 +34,25 @@ public:
>  	};
>  
>  	struct StreamBuffer {
> +		StreamBuffer(CameraStream *stream,
> +			     const camera3_stream_buffer_t &buffer,
> +			     Camera3RequestDescriptor *request);
> +		~StreamBuffer();

I'd add a blank line here.

> +		StreamBuffer(StreamBuffer &&other);
> +		StreamBuffer &operator=(StreamBuffer &&) = default;

The = default should go to the .cpp file, the function doesn't need to
be inlined.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
>  		CameraStream *stream;
>  		buffer_handle_t *camera3Buffer;
>  		std::unique_ptr<libcamera::FrameBuffer> frameBuffer;
>  		int fence;
> -		Status status;
> -		libcamera::FrameBuffer *internalBuffer;
> -		const libcamera::FrameBuffer *srcBuffer;
> +		Status status = Status::Success;
> +		libcamera::FrameBuffer *internalBuffer = nullptr;
> +		const libcamera::FrameBuffer *srcBuffer = nullptr;
>  		std::unique_ptr<CameraBuffer> dstBuffer;
>  		Camera3RequestDescriptor *request;
> +
> +	private:
> +		LIBCAMERA_DISABLE_COPY(StreamBuffer)
>  	};
>  
>  	/* Keeps track of streams requiring post-processing. */

-- 
Regards,

Laurent Pinchart