[libcamera-devel] [PATCH v3 3/8] libcamera: frame_buffer_allocator: Add clear()

[Kieran Bingham]

Hi Jacopo,

On 22/09/2020 10:47, Jacopo Mondi wrote:
> Add a clear() method to the FrameBufferAllocator class that
> frees all the buffers previously reserved by the allocator.
> 
> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/framebuffer_allocator.h | 1 +
>  src/libcamera/framebuffer_allocator.cpp   | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> index 78f1353964eb..2a4d538a0cb2 100644
> --- a/include/libcamera/framebuffer_allocator.h
> +++ b/include/libcamera/framebuffer_allocator.h
> @@ -28,6 +28,7 @@ public:
>  
>  	int allocate(Stream *stream);
>  	int free(Stream *stream);
> +	void clear();
>  
>  	bool allocated() const { return !buffers_.empty(); }
>  	const std::vector<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;
> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> index 2fbba37a1b0b..7ed80011c845 100644
> --- a/src/libcamera/framebuffer_allocator.cpp
> +++ b/src/libcamera/framebuffer_allocator.cpp
> @@ -125,6 +125,14 @@ int FrameBufferAllocator::free(Stream *stream)
>  	return 0;
>  }
>  
> +/**
> + * \brief Free all the buffers previously allocated
> + */
> +void FrameBufferAllocator::clear()
> +{
> +	buffers_.clear();
> +}

Throwing more opportunities into the bike-shed..

clear doesn't fit well for me either, I see Laurent suggested freeAll()
or such.

We could have an overload for free(no-arg) which clears 'all streams',
i.e. free(stream) is just a filtered subset.

Otherwise, reset() would also feel appropriate here.

But I can see the need to be able to reset a FrameBufferAllocator() in
some form, so if you're settled on the name:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +
>  /**
>   * \fn FrameBufferAllocator::allocated()
>   * \brief Check if the allocator has allocated buffers for any stream
> 

-- 
Regards
--
Kieran