[PATCH 2/5] libcamera: rkisp1: Standardise frame number tracking

[Jacopo Mondi]

Hi Dan

On Mon, Feb 12, 2024 at 03:35:29PM +0000, Daniel Scally wrote:
> The rkisp1 driver uses a couple of different methods of tracking the
> number of the frame - standardise them on request->sequence() and
> remove the frame_ member of RkISP1CameraData which was being used to
> do the job.

Request::Private::sequence_ gets assigned to
Camera::Private::requestSequence_ by the PipelineHandler base class.

Camera::Private::requestSequence_ gets zeroed in
PipelineHandler::stop()

Camera::Private::requestSequence_ gets incremented in
PipelineHandler::doQueueRequest()

So I think this change is legit (and should be done on IPU3 as well if
it uses the same scheme that was implemented here)

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++++--------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 5460dc11..22e553fe 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -87,7 +87,7 @@ class RkISP1CameraData : public Camera::Private
>  public:
>  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
>  			 RkISP1SelfPath *selfPath)
> -		: Camera::Private(pipe), frame_(0), frameInfo_(pipe),
> +		: Camera::Private(pipe), frameInfo_(pipe),
>  		  mainPath_(mainPath), selfPath_(selfPath)
>  	{
>  	}
> @@ -99,7 +99,6 @@ public:
>  	Stream selfPathStream_;
>  	std::unique_ptr<CameraSensor> sensor_;
>  	std::unique_ptr<DelayedControls> delayedCtrls_;
> -	unsigned int frame_;
>  	std::vector<IPABuffer> ipaBuffers_;
>  	RkISP1Frames frameInfo_;
>
> @@ -212,7 +211,7 @@ RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
>  RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
>  				      bool isRaw)
>  {
> -	unsigned int frame = data->frame_;
> +	unsigned int frame = request->sequence();
>
>  	FrameBuffer *paramBuffer = nullptr;
>  	FrameBuffer *statBuffer = nullptr;
> @@ -233,6 +232,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>
>  		statBuffer = pipe_->availableStatBuffers_.front();
>  		pipe_->availableStatBuffers_.pop();
> +		statBuffer->_d()->setRequest(request);
>  	}
>
>  	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> @@ -932,8 +932,6 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
>  		return ret;
>  	}
>
> -	data->frame_ = 0;
> -
>  	if (!isRaw_) {
>  		ret = param_->streamOn();
>  		if (ret) {
> @@ -1025,7 +1023,7 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>  	if (!info)
>  		return -ENOENT;
>
> -	data->ipa_->queueRequest(data->frame_, request->controls());
> +	data->ipa_->queueRequest(request->sequence(), request->controls());
>  	if (isRaw_) {
>  		if (info->mainPathBuffer)
>  			data->mainPath_->queueBuffer(info->mainPathBuffer);
> @@ -1033,12 +1031,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>  		if (data->selfPath_ && info->selfPathBuffer)
>  			data->selfPath_->queueBuffer(info->selfPathBuffer);
>  	} else {
> -		data->ipa_->fillParamsBuffer(data->frame_,
> +		data->ipa_->fillParamsBuffer(request->sequence(),
>  					     info->paramBuffer->cookie());
>  	}
>
> -	data->frame_++;
> -
>  	return 0;
>  }
>
> @@ -1269,7 +1265,8 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  		if (isRaw_) {
>  			const ControlList &ctrls =
>  				data->delayedCtrls_->get(metadata.sequence);
> -			data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
> +			data->ipa_->processStatsBuffer(request->sequence(),
> +						       0, ctrls);
>  		}
>  	} else {
>  		if (isRaw_)
> @@ -1284,6 +1281,7 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> +	Request *request = buffer->request();
>
>  	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
>  	if (!info)
> @@ -1295,11 +1293,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  		return;
>  	}
>
> -	if (data->frame_ <= buffer->metadata().sequence)
> -		data->frame_ = buffer->metadata().sequence + 1;
> -
> -	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
> -				       data->delayedCtrls_->get(buffer->metadata().sequence));
> +	data->ipa_->processStatsBuffer(request->sequence(), info->statBuffer->cookie(),
> +				       data->delayedCtrls_->get(request->sequence()));
>  }
>
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)
> --
> 2.34.1
>