[libcamera-devel,v4,4/8] libcamera: pipeline: rkisp1: Add clear() to RkISP1Frames

Message ID 20200413212700.1373247-5-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: ipa_manager: Proxy open-source IPAs to a thread
Related show

Commit Message

Niklas Söderlund April 13, 2020, 9:26 p.m. UTC
When the camera is stopping the helper that keeps track of which buffers
are associated with a request is not cleared. Add a clear operation and
call it when the camera is stopped.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Laurent Pinchart April 13, 2020, 9:36 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Mon, Apr 13, 2020 at 11:26:56PM +0200, Niklas Söderlund wrote:
> When the camera is stopping the helper that keeps track of which buffers
> are associated with a request is not cleared. Add a clear operation and
> call it when the camera is stopped.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index de90615edf217cca..c4e46bea1cc9a47f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -64,6 +64,7 @@ public:
>  
>  	RkISP1FrameInfo *create(unsigned int frame, Request *request, Stream *stream);
>  	int destroy(unsigned int frame);
> +	void clear();
>  
>  	RkISP1FrameInfo *find(unsigned int frame);
>  	RkISP1FrameInfo *find(FrameBuffer *buffer);
> @@ -279,6 +280,21 @@ int RkISP1Frames::destroy(unsigned int frame)
>  	return 0;
>  }
>  
> +void RkISP1Frames::clear()
> +{
> +	auto it = frameInfo_.cbegin();
> +	while (it != frameInfo_.cend()) {
> +		RkISP1FrameInfo *info = it->second;
> +
> +		pipe_->availableParamBuffers_.push(info->paramBuffer);
> +		pipe_->availableStatBuffers_.push(info->statBuffer);
> +
> +		it = frameInfo_.erase(it);
> +
> +		delete info;
> +	}

I think you can simplify this:

	for (const auto &entry : frameInfo_) {
		RkISP1FrameInfo *info = entry.second;

		pipe_->availableParamBuffers_.push(info->paramBuffer);
		pipe_->availableStatBuffers_.push(info->statBuffer);

		delete info;
	}

	frameInfo_.clear();

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

> +}
> +
>  RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
>  {
>  	auto itInfo = frameInfo_.find(frame);
> @@ -832,6 +848,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
>  
>  	data->timeline_.reset();
>  
> +	data->frameInfo_.clear();
> +
>  	freeBuffers(camera);
>  
>  	activeCamera_ = nullptr;

Patch

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index de90615edf217cca..c4e46bea1cc9a47f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -64,6 +64,7 @@  public:
 
 	RkISP1FrameInfo *create(unsigned int frame, Request *request, Stream *stream);
 	int destroy(unsigned int frame);
+	void clear();
 
 	RkISP1FrameInfo *find(unsigned int frame);
 	RkISP1FrameInfo *find(FrameBuffer *buffer);
@@ -279,6 +280,21 @@  int RkISP1Frames::destroy(unsigned int frame)
 	return 0;
 }
 
+void RkISP1Frames::clear()
+{
+	auto it = frameInfo_.cbegin();
+	while (it != frameInfo_.cend()) {
+		RkISP1FrameInfo *info = it->second;
+
+		pipe_->availableParamBuffers_.push(info->paramBuffer);
+		pipe_->availableStatBuffers_.push(info->statBuffer);
+
+		it = frameInfo_.erase(it);
+
+		delete info;
+	}
+}
+
 RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
 {
 	auto itInfo = frameInfo_.find(frame);
@@ -832,6 +848,8 @@  void PipelineHandlerRkISP1::stop(Camera *camera)
 
 	data->timeline_.reset();
 
+	data->frameInfo_.clear();
+
 	freeBuffers(camera);
 
 	activeCamera_ = nullptr;