[3/5] libcamera: rkisp1: Replace usage of RkISP1FrameInfo
diff mbox series

Message ID 20240221174015.52958-4-jacopo.mondi@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Replace IPU3/RkISP1FrameInfo
Related show

Commit Message

Jacopo Mondi Feb. 21, 2024, 5:40 p.m. UTC 
Now that the pipeline handler can create a private derived class of
Request::Private, use it to store the pipeline specific data.

In the case of RkISP1 we associate the statistic and paramters buffers
with a Request.

As the IPA sends notifications for paramters and statistics buffer by
identifying them by frame id, associate the frame ids and the Request
in the RkISP1CameraData class.

This replaces the functionalities of RkISP1FrameInfo which can now be
removed.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 358 ++++++++---------------
 1 file changed, 128 insertions(+), 230 deletions(-)

Comments

Daniel Scally March 5, 2024, 11:50 a.m. UTC | #1 
Hi Jacopo

On 21/02/2024 17:40, Jacopo Mondi wrote:
> Now that the pipeline handler can create a private derived class of
> Request::Private, use it to store the pipeline specific data.
>
> In the case of RkISP1 we associate the statistic and paramters buffers
> with a Request.
>
> As the IPA sends notifications for paramters and statistics buffer by
> identifying them by frame id, associate the frame ids and the Request
> in the RkISP1CameraData class.
>
> This replaces the functionalities of RkISP1FrameInfo which can now be
> removed.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---


Code here looks fine to me so:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

and

Tested-by: Daniel Scally <dan.scally@ideasonboard.com>


However I do wonder if the change in queueRequestDevice() to stop unconditionally queuing frames to 
the IPA should go into its own commit before this one - it's a good change but also could stand 
alone. Up to you though.

>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 358 ++++++++---------------
>   1 file changed, 128 insertions(+), 230 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e981e60758f7..f2163f528251 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -49,76 +49,63 @@ namespace libcamera {
>   
>   LOG_DEFINE_CATEGORY(RkISP1)
>   
> -class PipelineHandlerRkISP1;
> -class RkISP1CameraData;
> +class RkISP1Request : public Request::Private
> +{
> +public:
> +	RkISP1Request(Camera *camera)
> +		: Request::Private(camera)
> +	{
> +	}
>   
> -struct RkISP1FrameInfo {
> -	unsigned int frame;
> -	Request *request;
> +	bool hasPendingBuffers(bool isRaw) const;
>   
> -	FrameBuffer *paramBuffer;
>   	FrameBuffer *statBuffer;
> -	FrameBuffer *mainPathBuffer;
> -	FrameBuffer *selfPathBuffer;
> +	FrameBuffer *paramBuffer;
> +
> +	/* The frame number this request is associated with. */
> +	unsigned int frame;
>   
>   	bool paramDequeued;
>   	bool metadataProcessed;
>   };
>   
> -class RkISP1Frames
> -{
> -public:
> -	RkISP1Frames(PipelineHandler *pipe);
> -
> -	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
> -				bool isRaw);
> -	int destroy(unsigned int frame);
> -	void clear();
> -
> -	RkISP1FrameInfo *find(unsigned int frame);
> -	RkISP1FrameInfo *find(FrameBuffer *buffer);
> -	RkISP1FrameInfo *find(Request *request);
> -
> -private:
> -	PipelineHandlerRkISP1 *pipe_;
> -	std::map<unsigned int, RkISP1FrameInfo *> frameInfo_;
> -};
> -
> -class RkISP1Request : public Request::Private
> +bool RkISP1Request::hasPendingBuffers(bool isRaw) const
>   {
> -public:
> -	RkISP1Request(Camera *camera)
> -		: Request::Private(camera)
> -	{
> -	}
> -};
> +	return Request::Private::hasPendingBuffers() ||
> +	       !metadataProcessed || (!isRaw && !paramDequeued);
> +}
>   
> +class PipelineHandlerRkISP1;
>   class RkISP1CameraData : public Camera::Private
>   {
>   public:
>   	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
>   			 RkISP1SelfPath *selfPath)
> -		: Camera::Private(pipe), frame_(0), frameInfo_(pipe),
> -		  mainPath_(mainPath), selfPath_(selfPath)
> +		: Camera::Private(pipe), frame_(0), mainPath_(mainPath),
> +		  selfPath_(selfPath)
>   	{
>   	}
>   
>   	PipelineHandlerRkISP1 *pipe();
>   	int loadIPA(unsigned int hwRevision);
>   
> +	void addRequest(RkISP1Request *request);
> +
>   	Stream mainPathStream_;
>   	Stream selfPathStream_;
>   	std::unique_ptr<CameraSensor> sensor_;
>   	std::unique_ptr<DelayedControls> delayedCtrls_;
>   	unsigned int frame_;
>   	std::vector<IPABuffer> ipaBuffers_;
> -	RkISP1Frames frameInfo_;
>   
>   	RkISP1MainPath *mainPath_;
>   	RkISP1SelfPath *selfPath_;
>   
>   	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
>   
> +	/* Associate a frame id with a Request. */
> +	std::map<unsigned int, RkISP1Request *> requestMap_;
> +
>   private:
>   	void paramFilled(unsigned int frame);
>   	void setSensorControls(unsigned int frame,
> @@ -181,13 +168,17 @@ private:
>   		return static_cast<RkISP1CameraData *>(camera->_d());
>   	}
>   
> +	RkISP1Request *cameraRequest(Request *request)
> +	{
> +		return static_cast<RkISP1Request *>(request->_d());
> +	}
> +
>   	friend RkISP1CameraData;
> -	friend RkISP1Frames;
>   
>   	int initLinks(Camera *camera, const CameraSensor *sensor,
>   		      const RkISP1CameraConfiguration &config);
>   	int createCamera(MediaEntity *sensor);
> -	void tryCompleteRequest(RkISP1FrameInfo *info);
> +	void tryCompleteRequest(RkISP1Request *request);
>   	void bufferReady(FrameBuffer *buffer);
>   	void paramReady(FrameBuffer *buffer);
>   	void statReady(FrameBuffer *buffer);
> @@ -218,129 +209,6 @@ private:
>   	const MediaPad *ispSink_;
>   };
>   
> -RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> -	: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))
> -{
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
> -				      bool isRaw)
> -{
> -	unsigned int frame = data->frame_;
> -
> -	FrameBuffer *paramBuffer = nullptr;
> -	FrameBuffer *statBuffer = nullptr;
> -
> -	if (!isRaw) {
> -		if (pipe_->availableParamBuffers_.empty()) {
> -			LOG(RkISP1, Error) << "Parameters buffer underrun";
> -			return nullptr;
> -		}
> -
> -		if (pipe_->availableStatBuffers_.empty()) {
> -			LOG(RkISP1, Error) << "Statistic buffer underrun";
> -			return nullptr;
> -		}
> -
> -		paramBuffer = pipe_->availableParamBuffers_.front();
> -		pipe_->availableParamBuffers_.pop();
> -
> -		statBuffer = pipe_->availableStatBuffers_.front();
> -		pipe_->availableStatBuffers_.pop();
> -	}
> -
> -	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> -	FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
> -
> -	RkISP1FrameInfo *info = new RkISP1FrameInfo;
> -
> -	info->frame = frame;
> -	info->request = request;
> -	info->paramBuffer = paramBuffer;
> -	info->mainPathBuffer = mainPathBuffer;
> -	info->selfPathBuffer = selfPathBuffer;
> -	info->statBuffer = statBuffer;
> -	info->paramDequeued = false;
> -	info->metadataProcessed = false;
> -
> -	frameInfo_[frame] = info;
> -
> -	return info;
> -}
> -
> -int RkISP1Frames::destroy(unsigned int frame)
> -{
> -	RkISP1FrameInfo *info = find(frame);
> -	if (!info)
> -		return -ENOENT;
> -
> -	pipe_->availableParamBuffers_.push(info->paramBuffer);
> -	pipe_->availableStatBuffers_.push(info->statBuffer);
> -
> -	frameInfo_.erase(info->frame);
> -
> -	delete info;
> -
> -	return 0;
> -}
> -
> -void RkISP1Frames::clear()
> -{
> -	for (const auto &entry : frameInfo_) {
> -		RkISP1FrameInfo *info = entry.second;
> -
> -		pipe_->availableParamBuffers_.push(info->paramBuffer);
> -		pipe_->availableStatBuffers_.push(info->statBuffer);
> -
> -		delete info;
> -	}
> -
> -	frameInfo_.clear();
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
> -{
> -	auto itInfo = frameInfo_.find(frame);
> -
> -	if (itInfo != frameInfo_.end())
> -		return itInfo->second;
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from frame";
> -
> -	return nullptr;
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> -{
> -	for (auto &itInfo : frameInfo_) {
> -		RkISP1FrameInfo *info = itInfo.second;
> -
> -		if (info->paramBuffer == buffer ||
> -		    info->statBuffer == buffer ||
> -		    info->mainPathBuffer == buffer ||
> -		    info->selfPathBuffer == buffer)
> -			return info;
> -	}
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from buffer";
> -
> -	return nullptr;
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(Request *request)
> -{
> -	for (auto &itInfo : frameInfo_) {
> -		RkISP1FrameInfo *info = itInfo.second;
> -
> -		if (info->request == request)
> -			return info;
> -	}
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from request";
> -
> -	return nullptr;
> -}
> -
>   PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
>   {
>   	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> @@ -391,23 +259,34 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>   	return 0;
>   }
>   
> +void RkISP1CameraData::addRequest(RkISP1Request *request)
> +{
> +	/* Associate the request and the frame number. */
> +	request->frame = frame_;
> +	requestMap_[frame_] = request;
> +	frame_++;
> +}
> +
>   void RkISP1CameraData::paramFilled(unsigned int frame)
>   {
>   	PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = requestMap_.at(frame);
> +	ASSERT(request);
>   
> -	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> +	request->paramBuffer->_d()->metadata().planes()[0].bytesused =
>   		sizeof(struct rkisp1_params_cfg);
> -	pipe->param_->queueBuffer(info->paramBuffer);
> -	pipe->stat_->queueBuffer(info->statBuffer);
> -
> -	if (info->mainPathBuffer)
> -		mainPath_->queueBuffer(info->mainPathBuffer);
> -
> -	if (selfPath_ && info->selfPathBuffer)
> -		selfPath_->queueBuffer(info->selfPathBuffer);
> +	pipe->param_->queueBuffer(request->paramBuffer);
> +	pipe->stat_->queueBuffer(request->statBuffer);
> +
> +	FrameBuffer *mainPathBuffer =
> +		request->_o<Request>()->findBuffer(&mainPathStream_);
> +	if (mainPathBuffer)
> +		mainPath_->queueBuffer(mainPathBuffer);
> +
> +	FrameBuffer *selfPathBuffer =
> +		request->_o<Request>()->findBuffer(&selfPathStream_);
> +	if (selfPath_ && selfPathBuffer)
> +		selfPath_->queueBuffer(selfPathBuffer);
>   }
>   
>   void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> @@ -418,14 +297,13 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
>   
>   void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
>   {
> -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = requestMap_.at(frame);
> +	ASSERT(request);
>   
> -	info->request->metadata().merge(metadata);
> -	info->metadataProcessed = true;
> +	request->_o<Request>()->metadata().merge(metadata);
> +	request->metadataProcessed = true;
>   
> -	pipe()->tryCompleteRequest(info);
> +	pipe()->tryCompleteRequest(request);
>   }
>   
>   /* -----------------------------------------------------------------------------
> @@ -1025,7 +903,7 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
>   	}
>   
>   	ASSERT(data->queuedRequests_.empty());
> -	data->frameInfo_.clear();
> +	data->requestMap_.clear();
>   
>   	freeBuffers(camera);
>   
> @@ -1042,24 +920,60 @@ std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *came
>   int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>   {
>   	RkISP1CameraData *data = cameraData(camera);
> +	RkISP1Request *rkisp1Request = cameraRequest(request);
>   
> -	RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
> -	if (!info)
> -		return -ENOENT;
> +	data->addRequest(rkisp1Request);
>   
> -	data->ipa_->queueRequest(data->frame_, request->controls());
> +	/*
> +	 * If we're operating in RAW mode (only one RAW stream is captured)
> +	 * then we simply queue buffers to the video devices as we don't
> +	 * need to run the IPA.
> +	 */
>   	if (isRaw_) {
> -		if (info->mainPathBuffer)
> -			data->mainPath_->queueBuffer(info->mainPathBuffer);
> +		FrameBuffer *mainPathBuffer =
> +			request->findBuffer(&data->mainPathStream_);
> +		if (mainPathBuffer)
> +			data->mainPath_->queueBuffer(mainPathBuffer);
>   
> -		if (data->selfPath_ && info->selfPathBuffer)
> -			data->selfPath_->queueBuffer(info->selfPathBuffer);
> -	} else {
> -		data->ipa_->fillParamsBuffer(data->frame_,
> -					     info->paramBuffer->cookie());
> +		FrameBuffer *selfPathBuffer =
> +			request->findBuffer(&data->selfPathStream_);
> +		if (data->selfPath_ && selfPathBuffer)
> +			data->selfPath_->queueBuffer(selfPathBuffer);
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * If we run the IPA we need to associate a parameters and a statistics
> +	 * buffer with the Request and associate the request with the current
> +	 * frame number.
> +	 *
> +	 * Associate the stat and frame buffers to a Request (if available)
> +	 * and then run the IPA.
> +	 */
> +	if (availableParamBuffers_.empty()) {
> +		LOG(RkISP1, Error) << "Parameters buffer underrun";
> +		return -ENOENT;
>   	}
>   
> -	data->frame_++;
> +	if (availableStatBuffers_.empty()) {
> +		LOG(RkISP1, Error) << "Statistic buffer underrun";
> +		return -ENOENT;
> +	}
> +
> +	rkisp1Request->paramBuffer = availableParamBuffers_.front();
> +	rkisp1Request->paramDequeued = false;
> +	rkisp1Request->paramBuffer->_d()->setRequest(request);
> +	availableParamBuffers_.pop();
> +
> +	rkisp1Request->statBuffer = availableStatBuffers_.front();
> +	rkisp1Request->metadataProcessed = false;
> +	rkisp1Request->statBuffer->_d()->setRequest(request);
> +	availableStatBuffers_.pop();
> +
> +	data->ipa_->queueRequest(rkisp1Request->frame, request->controls());
> +	data->ipa_->fillParamsBuffer(rkisp1Request->frame,
> +				     rkisp1Request->paramBuffer->cookie());
>   
>   	return 0;
>   }
> @@ -1251,37 +1165,28 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>    * Buffer Handling
>    */
>   
> -void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
> +void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1Request *request)
>   {
>   	RkISP1CameraData *data = cameraData(activeCamera_);
> -	Request *request = info->request;
> -
> -	if (request->hasPendingBuffers())
> -		return;
> -
> -	if (!info->metadataProcessed)
> -		return;
>   
> -	if (!isRaw_ && !info->paramDequeued)
> +	if (request->hasPendingBuffers(isRaw_))
>   		return;
>   
> -	data->frameInfo_.destroy(info->frame);
> +	/* Return the stat and param buffers to the pipeline. */
> +	availableParamBuffers_.push(request->paramBuffer);
> +	availableStatBuffers_.push(request->statBuffer);
> +	data->requestMap_.erase(request->frame);
>   
> -	completeRequest(request);
> +	completeRequest(request->_o<Request>());
>   }
>   
>   void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>   {
>   	ASSERT(activeCamera_);
>   	RkISP1CameraData *data = cameraData(activeCamera_);
> -
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = cameraRequest(buffer->request());
>   
>   	const FrameMetadata &metadata = buffer->metadata();
> -	Request *request = buffer->request();
> -
>   	if (metadata.status != FrameMetadata::FrameCancelled) {
>   		/*
>   		 * Record the sensor's timestamp in the request metadata.
> @@ -1289,55 +1194,48 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>   		 * \todo The sensor timestamp should be better estimated by connecting
>   		 * to the V4L2Device::frameStart signal.
>   		 */
> -		request->metadata().set(controls::SensorTimestamp,
> -					metadata.timestamp);
> +		request->_o<Request>()->metadata().set(controls::SensorTimestamp,
> +						       metadata.timestamp);
>   
>   		if (isRaw_) {
>   			const ControlList &ctrls =
>   				data->delayedCtrls_->get(metadata.sequence);
> -			data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
> +			data->ipa_->processStatsBuffer(request->frame, 0, ctrls);
>   		}
>   	} else {
>   		if (isRaw_)
> -			info->metadataProcessed = true;
> +			request->metadataProcessed = true;
>   	}
>   
> -	completeBuffer(request, buffer);
> -	tryCompleteRequest(info);
> +	completeBuffer(request->_o<Request>(), buffer);
> +	tryCompleteRequest(request);
>   }
>   
>   void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>   {
>   	ASSERT(activeCamera_);
> -	RkISP1CameraData *data = cameraData(activeCamera_);
> +	RkISP1Request *request = cameraRequest(buffer->request());
>   
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> -
> -	info->paramDequeued = true;
> -	tryCompleteRequest(info);
> +	request->paramDequeued = true;
> +	tryCompleteRequest(request);
>   }
>   
>   void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>   {
>   	ASSERT(activeCamera_);
>   	RkISP1CameraData *data = cameraData(activeCamera_);
> -
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = cameraRequest(buffer->request());
>   
>   	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> -		info->metadataProcessed = true;
> -		tryCompleteRequest(info);
> +		request->metadataProcessed = true;
> +		tryCompleteRequest(request);
>   		return;
>   	}
>   
>   	if (data->frame_ <= buffer->metadata().sequence)
>   		data->frame_ = buffer->metadata().sequence + 1;
>   
> -	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
> +	data->ipa_->processStatsBuffer(request->frame, request->statBuffer->cookie(),
>   				       data->delayedCtrls_->get(buffer->metadata().sequence));
>   }
>
Stefan Klug March 6, 2024, 3:48 p.m. UTC | #2 
Hi Jacopo,

On Wed, Feb 21, 2024 at 06:40:11PM +0100, Jacopo Mondi wrote:
> Now that the pipeline handler can create a private derived class of
> Request::Private, use it to store the pipeline specific data.
> 
> In the case of RkISP1 we associate the statistic and paramters buffers
> with a Request.
> 
> As the IPA sends notifications for paramters and statistics buffer by
> identifying them by frame id, associate the frame ids and the Request
> in the RkISP1CameraData class.
> 
> This replaces the functionalities of RkISP1FrameInfo which can now be
> removed.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Stefan Klug<stefan.klug@ideasonboard.com>

Cheers,
Stefan

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 358 ++++++++---------------
>  1 file changed, 128 insertions(+), 230 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e981e60758f7..f2163f528251 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -49,76 +49,63 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(RkISP1)
>  
> -class PipelineHandlerRkISP1;
> -class RkISP1CameraData;
> +class RkISP1Request : public Request::Private
> +{
> +public:
> +	RkISP1Request(Camera *camera)
> +		: Request::Private(camera)
> +	{
> +	}
>  
> -struct RkISP1FrameInfo {
> -	unsigned int frame;
> -	Request *request;
> +	bool hasPendingBuffers(bool isRaw) const;
>  
> -	FrameBuffer *paramBuffer;
>  	FrameBuffer *statBuffer;
> -	FrameBuffer *mainPathBuffer;
> -	FrameBuffer *selfPathBuffer;
> +	FrameBuffer *paramBuffer;
> +
> +	/* The frame number this request is associated with. */
> +	unsigned int frame;
>  
>  	bool paramDequeued;
>  	bool metadataProcessed;
>  };
>  
> -class RkISP1Frames
> -{
> -public:
> -	RkISP1Frames(PipelineHandler *pipe);
> -
> -	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
> -				bool isRaw);
> -	int destroy(unsigned int frame);
> -	void clear();
> -
> -	RkISP1FrameInfo *find(unsigned int frame);
> -	RkISP1FrameInfo *find(FrameBuffer *buffer);
> -	RkISP1FrameInfo *find(Request *request);
> -
> -private:
> -	PipelineHandlerRkISP1 *pipe_;
> -	std::map<unsigned int, RkISP1FrameInfo *> frameInfo_;
> -};
> -
> -class RkISP1Request : public Request::Private
> +bool RkISP1Request::hasPendingBuffers(bool isRaw) const
>  {
> -public:
> -	RkISP1Request(Camera *camera)
> -		: Request::Private(camera)
> -	{
> -	}
> -};
> +	return Request::Private::hasPendingBuffers() ||
> +	       !metadataProcessed || (!isRaw && !paramDequeued);
> +}
>  
> +class PipelineHandlerRkISP1;
>  class RkISP1CameraData : public Camera::Private
>  {
>  public:
>  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
>  			 RkISP1SelfPath *selfPath)
> -		: Camera::Private(pipe), frame_(0), frameInfo_(pipe),
> -		  mainPath_(mainPath), selfPath_(selfPath)
> +		: Camera::Private(pipe), frame_(0), mainPath_(mainPath),
> +		  selfPath_(selfPath)
>  	{
>  	}
>  
>  	PipelineHandlerRkISP1 *pipe();
>  	int loadIPA(unsigned int hwRevision);
>  
> +	void addRequest(RkISP1Request *request);
> +
>  	Stream mainPathStream_;
>  	Stream selfPathStream_;
>  	std::unique_ptr<CameraSensor> sensor_;
>  	std::unique_ptr<DelayedControls> delayedCtrls_;
>  	unsigned int frame_;
>  	std::vector<IPABuffer> ipaBuffers_;
> -	RkISP1Frames frameInfo_;
>  
>  	RkISP1MainPath *mainPath_;
>  	RkISP1SelfPath *selfPath_;
>  
>  	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
>  
> +	/* Associate a frame id with a Request. */
> +	std::map<unsigned int, RkISP1Request *> requestMap_;
> +
>  private:
>  	void paramFilled(unsigned int frame);
>  	void setSensorControls(unsigned int frame,
> @@ -181,13 +168,17 @@ private:
>  		return static_cast<RkISP1CameraData *>(camera->_d());
>  	}
>  
> +	RkISP1Request *cameraRequest(Request *request)
> +	{
> +		return static_cast<RkISP1Request *>(request->_d());
> +	}
> +
>  	friend RkISP1CameraData;
> -	friend RkISP1Frames;
>  
>  	int initLinks(Camera *camera, const CameraSensor *sensor,
>  		      const RkISP1CameraConfiguration &config);
>  	int createCamera(MediaEntity *sensor);
> -	void tryCompleteRequest(RkISP1FrameInfo *info);
> +	void tryCompleteRequest(RkISP1Request *request);
>  	void bufferReady(FrameBuffer *buffer);
>  	void paramReady(FrameBuffer *buffer);
>  	void statReady(FrameBuffer *buffer);
> @@ -218,129 +209,6 @@ private:
>  	const MediaPad *ispSink_;
>  };
>  
> -RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> -	: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))
> -{
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
> -				      bool isRaw)
> -{
> -	unsigned int frame = data->frame_;
> -
> -	FrameBuffer *paramBuffer = nullptr;
> -	FrameBuffer *statBuffer = nullptr;
> -
> -	if (!isRaw) {
> -		if (pipe_->availableParamBuffers_.empty()) {
> -			LOG(RkISP1, Error) << "Parameters buffer underrun";
> -			return nullptr;
> -		}
> -
> -		if (pipe_->availableStatBuffers_.empty()) {
> -			LOG(RkISP1, Error) << "Statistic buffer underrun";
> -			return nullptr;
> -		}
> -
> -		paramBuffer = pipe_->availableParamBuffers_.front();
> -		pipe_->availableParamBuffers_.pop();
> -
> -		statBuffer = pipe_->availableStatBuffers_.front();
> -		pipe_->availableStatBuffers_.pop();
> -	}
> -
> -	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> -	FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
> -
> -	RkISP1FrameInfo *info = new RkISP1FrameInfo;
> -
> -	info->frame = frame;
> -	info->request = request;
> -	info->paramBuffer = paramBuffer;
> -	info->mainPathBuffer = mainPathBuffer;
> -	info->selfPathBuffer = selfPathBuffer;
> -	info->statBuffer = statBuffer;
> -	info->paramDequeued = false;
> -	info->metadataProcessed = false;
> -
> -	frameInfo_[frame] = info;
> -
> -	return info;
> -}
> -
> -int RkISP1Frames::destroy(unsigned int frame)
> -{
> -	RkISP1FrameInfo *info = find(frame);
> -	if (!info)
> -		return -ENOENT;
> -
> -	pipe_->availableParamBuffers_.push(info->paramBuffer);
> -	pipe_->availableStatBuffers_.push(info->statBuffer);
> -
> -	frameInfo_.erase(info->frame);
> -
> -	delete info;
> -
> -	return 0;
> -}
> -
> -void RkISP1Frames::clear()
> -{
> -	for (const auto &entry : frameInfo_) {
> -		RkISP1FrameInfo *info = entry.second;
> -
> -		pipe_->availableParamBuffers_.push(info->paramBuffer);
> -		pipe_->availableStatBuffers_.push(info->statBuffer);
> -
> -		delete info;
> -	}
> -
> -	frameInfo_.clear();
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
> -{
> -	auto itInfo = frameInfo_.find(frame);
> -
> -	if (itInfo != frameInfo_.end())
> -		return itInfo->second;
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from frame";
> -
> -	return nullptr;
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> -{
> -	for (auto &itInfo : frameInfo_) {
> -		RkISP1FrameInfo *info = itInfo.second;
> -
> -		if (info->paramBuffer == buffer ||
> -		    info->statBuffer == buffer ||
> -		    info->mainPathBuffer == buffer ||
> -		    info->selfPathBuffer == buffer)
> -			return info;
> -	}
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from buffer";
> -
> -	return nullptr;
> -}
> -
> -RkISP1FrameInfo *RkISP1Frames::find(Request *request)
> -{
> -	for (auto &itInfo : frameInfo_) {
> -		RkISP1FrameInfo *info = itInfo.second;
> -
> -		if (info->request == request)
> -			return info;
> -	}
> -
> -	LOG(RkISP1, Fatal) << "Can't locate info from request";
> -
> -	return nullptr;
> -}
> -
>  PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
>  {
>  	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> @@ -391,23 +259,34 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  	return 0;
>  }
>  
> +void RkISP1CameraData::addRequest(RkISP1Request *request)
> +{
> +	/* Associate the request and the frame number. */
> +	request->frame = frame_;
> +	requestMap_[frame_] = request;
> +	frame_++;
> +}
> +
>  void RkISP1CameraData::paramFilled(unsigned int frame)
>  {
>  	PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = requestMap_.at(frame);
> +	ASSERT(request);
>  
> -	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> +	request->paramBuffer->_d()->metadata().planes()[0].bytesused =
>  		sizeof(struct rkisp1_params_cfg);
> -	pipe->param_->queueBuffer(info->paramBuffer);
> -	pipe->stat_->queueBuffer(info->statBuffer);
> -
> -	if (info->mainPathBuffer)
> -		mainPath_->queueBuffer(info->mainPathBuffer);
> -
> -	if (selfPath_ && info->selfPathBuffer)
> -		selfPath_->queueBuffer(info->selfPathBuffer);
> +	pipe->param_->queueBuffer(request->paramBuffer);
> +	pipe->stat_->queueBuffer(request->statBuffer);
> +
> +	FrameBuffer *mainPathBuffer =
> +		request->_o<Request>()->findBuffer(&mainPathStream_);
> +	if (mainPathBuffer)
> +		mainPath_->queueBuffer(mainPathBuffer);
> +
> +	FrameBuffer *selfPathBuffer =
> +		request->_o<Request>()->findBuffer(&selfPathStream_);
> +	if (selfPath_ && selfPathBuffer)
> +		selfPath_->queueBuffer(selfPathBuffer);
>  }
>  
>  void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> @@ -418,14 +297,13 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
>  
>  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
>  {
> -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = requestMap_.at(frame);
> +	ASSERT(request);
>  
> -	info->request->metadata().merge(metadata);
> -	info->metadataProcessed = true;
> +	request->_o<Request>()->metadata().merge(metadata);
> +	request->metadataProcessed = true;
>  
> -	pipe()->tryCompleteRequest(info);
> +	pipe()->tryCompleteRequest(request);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -1025,7 +903,7 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
>  	}
>  
>  	ASSERT(data->queuedRequests_.empty());
> -	data->frameInfo_.clear();
> +	data->requestMap_.clear();
>  
>  	freeBuffers(camera);
>  
> @@ -1042,24 +920,60 @@ std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *came
>  int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
> +	RkISP1Request *rkisp1Request = cameraRequest(request);
>  
> -	RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
> -	if (!info)
> -		return -ENOENT;
> +	data->addRequest(rkisp1Request);
>  
> -	data->ipa_->queueRequest(data->frame_, request->controls());
> +	/*
> +	 * If we're operating in RAW mode (only one RAW stream is captured)
> +	 * then we simply queue buffers to the video devices as we don't
> +	 * need to run the IPA.
> +	 */
>  	if (isRaw_) {
> -		if (info->mainPathBuffer)
> -			data->mainPath_->queueBuffer(info->mainPathBuffer);
> +		FrameBuffer *mainPathBuffer =
> +			request->findBuffer(&data->mainPathStream_);
> +		if (mainPathBuffer)
> +			data->mainPath_->queueBuffer(mainPathBuffer);
>  
> -		if (data->selfPath_ && info->selfPathBuffer)
> -			data->selfPath_->queueBuffer(info->selfPathBuffer);
> -	} else {
> -		data->ipa_->fillParamsBuffer(data->frame_,
> -					     info->paramBuffer->cookie());
> +		FrameBuffer *selfPathBuffer =
> +			request->findBuffer(&data->selfPathStream_);
> +		if (data->selfPath_ && selfPathBuffer)
> +			data->selfPath_->queueBuffer(selfPathBuffer);
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * If we run the IPA we need to associate a parameters and a statistics
> +	 * buffer with the Request and associate the request with the current
> +	 * frame number.
> +	 *
> +	 * Associate the stat and frame buffers to a Request (if available)
> +	 * and then run the IPA.
> +	 */
> +	if (availableParamBuffers_.empty()) {
> +		LOG(RkISP1, Error) << "Parameters buffer underrun";
> +		return -ENOENT;
>  	}
>  
> -	data->frame_++;
> +	if (availableStatBuffers_.empty()) {
> +		LOG(RkISP1, Error) << "Statistic buffer underrun";
> +		return -ENOENT;
> +	}
> +
> +	rkisp1Request->paramBuffer = availableParamBuffers_.front();
> +	rkisp1Request->paramDequeued = false;
> +	rkisp1Request->paramBuffer->_d()->setRequest(request);
> +	availableParamBuffers_.pop();
> +
> +	rkisp1Request->statBuffer = availableStatBuffers_.front();
> +	rkisp1Request->metadataProcessed = false;
> +	rkisp1Request->statBuffer->_d()->setRequest(request);
> +	availableStatBuffers_.pop();
> +
> +	data->ipa_->queueRequest(rkisp1Request->frame, request->controls());
> +	data->ipa_->fillParamsBuffer(rkisp1Request->frame,
> +				     rkisp1Request->paramBuffer->cookie());
>  
>  	return 0;
>  }
> @@ -1251,37 +1165,28 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>   * Buffer Handling
>   */
>  
> -void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
> +void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1Request *request)
>  {
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> -	Request *request = info->request;
> -
> -	if (request->hasPendingBuffers())
> -		return;
> -
> -	if (!info->metadataProcessed)
> -		return;
>  
> -	if (!isRaw_ && !info->paramDequeued)
> +	if (request->hasPendingBuffers(isRaw_))
>  		return;
>  
> -	data->frameInfo_.destroy(info->frame);
> +	/* Return the stat and param buffers to the pipeline. */
> +	availableParamBuffers_.push(request->paramBuffer);
> +	availableStatBuffers_.push(request->statBuffer);
> +	data->requestMap_.erase(request->frame);
>  
> -	completeRequest(request);
> +	completeRequest(request->_o<Request>());
>  }
>  
>  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  {
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> -
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = cameraRequest(buffer->request());
>  
>  	const FrameMetadata &metadata = buffer->metadata();
> -	Request *request = buffer->request();
> -
>  	if (metadata.status != FrameMetadata::FrameCancelled) {
>  		/*
>  		 * Record the sensor's timestamp in the request metadata.
> @@ -1289,55 +1194,48 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  		 * \todo The sensor timestamp should be better estimated by connecting
>  		 * to the V4L2Device::frameStart signal.
>  		 */
> -		request->metadata().set(controls::SensorTimestamp,
> -					metadata.timestamp);
> +		request->_o<Request>()->metadata().set(controls::SensorTimestamp,
> +						       metadata.timestamp);
>  
>  		if (isRaw_) {
>  			const ControlList &ctrls =
>  				data->delayedCtrls_->get(metadata.sequence);
> -			data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
> +			data->ipa_->processStatsBuffer(request->frame, 0, ctrls);
>  		}
>  	} else {
>  		if (isRaw_)
> -			info->metadataProcessed = true;
> +			request->metadataProcessed = true;
>  	}
>  
> -	completeBuffer(request, buffer);
> -	tryCompleteRequest(info);
> +	completeBuffer(request->_o<Request>(), buffer);
> +	tryCompleteRequest(request);
>  }
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  {
>  	ASSERT(activeCamera_);
> -	RkISP1CameraData *data = cameraData(activeCamera_);
> +	RkISP1Request *request = cameraRequest(buffer->request());
>  
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> -
> -	info->paramDequeued = true;
> -	tryCompleteRequest(info);
> +	request->paramDequeued = true;
> +	tryCompleteRequest(request);
>  }
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> -
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> +	RkISP1Request *request = cameraRequest(buffer->request());
>  
>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> -		info->metadataProcessed = true;
> -		tryCompleteRequest(info);
> +		request->metadataProcessed = true;
> +		tryCompleteRequest(request);
>  		return;
>  	}
>  
>  	if (data->frame_ <= buffer->metadata().sequence)
>  		data->frame_ = buffer->metadata().sequence + 1;
>  
> -	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
> +	data->ipa_->processStatsBuffer(request->frame, request->statBuffer->cookie(),
>  				       data->delayedCtrls_->get(buffer->metadata().sequence));
>  }
>  
> -- 
> 2.43.0
>
Jacopo Mondi March 11, 2024, 11:40 a.m. UTC | #3 
Hi Dan

On Tue, Mar 05, 2024 at 11:50:11AM +0000, Dan Scally wrote:
> Hi Jacopo
>
> On 21/02/2024 17:40, Jacopo Mondi wrote:
> > Now that the pipeline handler can create a private derived class of
> > Request::Private, use it to store the pipeline specific data.
> >
> > In the case of RkISP1 we associate the statistic and paramters buffers
> > with a Request.
> >
> > As the IPA sends notifications for paramters and statistics buffer by
> > identifying them by frame id, associate the frame ids and the Request
> > in the RkISP1CameraData class.
> >
> > This replaces the functionalities of RkISP1FrameInfo which can now be
> > removed.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
>
>
> Code here looks fine to me so:
>
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
>
> and
>
> Tested-by: Daniel Scally <dan.scally@ideasonboard.com>
>
>
> However I do wonder if the change in queueRequestDevice() to stop
> unconditionally queuing frames to the IPA should go into its own commit
> before this one - it's a good change but also could stand alone. Up to you
> though.
>

Good catch!

The difference is that we used to queue Request to the IPA even for
RAW frames, even if we didn't provide any paramters buffer to fill so
not ISP (or sensor) paramters where computed, but the algorithms were
run anyway.

With this new version we only queue a request to the IPA for non-RAW
requests.

I would rather maintain the existing behaviour and hence change this
patch in my next version

Thanks
  j

> >   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 358 ++++++++---------------
> >   1 file changed, 128 insertions(+), 230 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index e981e60758f7..f2163f528251 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -49,76 +49,63 @@ namespace libcamera {
> >   LOG_DEFINE_CATEGORY(RkISP1)
> > -class PipelineHandlerRkISP1;
> > -class RkISP1CameraData;
> > +class RkISP1Request : public Request::Private
> > +{
> > +public:
> > +	RkISP1Request(Camera *camera)
> > +		: Request::Private(camera)
> > +	{
> > +	}
> > -struct RkISP1FrameInfo {
> > -	unsigned int frame;
> > -	Request *request;
> > +	bool hasPendingBuffers(bool isRaw) const;
> > -	FrameBuffer *paramBuffer;
> >   	FrameBuffer *statBuffer;
> > -	FrameBuffer *mainPathBuffer;
> > -	FrameBuffer *selfPathBuffer;
> > +	FrameBuffer *paramBuffer;
> > +
> > +	/* The frame number this request is associated with. */
> > +	unsigned int frame;
> >   	bool paramDequeued;
> >   	bool metadataProcessed;
> >   };
> > -class RkISP1Frames
> > -{
> > -public:
> > -	RkISP1Frames(PipelineHandler *pipe);
> > -
> > -	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
> > -				bool isRaw);
> > -	int destroy(unsigned int frame);
> > -	void clear();
> > -
> > -	RkISP1FrameInfo *find(unsigned int frame);
> > -	RkISP1FrameInfo *find(FrameBuffer *buffer);
> > -	RkISP1FrameInfo *find(Request *request);
> > -
> > -private:
> > -	PipelineHandlerRkISP1 *pipe_;
> > -	std::map<unsigned int, RkISP1FrameInfo *> frameInfo_;
> > -};
> > -
> > -class RkISP1Request : public Request::Private
> > +bool RkISP1Request::hasPendingBuffers(bool isRaw) const
> >   {
> > -public:
> > -	RkISP1Request(Camera *camera)
> > -		: Request::Private(camera)
> > -	{
> > -	}
> > -};
> > +	return Request::Private::hasPendingBuffers() ||
> > +	       !metadataProcessed || (!isRaw && !paramDequeued);
> > +}
> > +class PipelineHandlerRkISP1;
> >   class RkISP1CameraData : public Camera::Private
> >   {
> >   public:
> >   	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
> >   			 RkISP1SelfPath *selfPath)
> > -		: Camera::Private(pipe), frame_(0), frameInfo_(pipe),
> > -		  mainPath_(mainPath), selfPath_(selfPath)
> > +		: Camera::Private(pipe), frame_(0), mainPath_(mainPath),
> > +		  selfPath_(selfPath)
> >   	{
> >   	}
> >   	PipelineHandlerRkISP1 *pipe();
> >   	int loadIPA(unsigned int hwRevision);
> > +	void addRequest(RkISP1Request *request);
> > +
> >   	Stream mainPathStream_;
> >   	Stream selfPathStream_;
> >   	std::unique_ptr<CameraSensor> sensor_;
> >   	std::unique_ptr<DelayedControls> delayedCtrls_;
> >   	unsigned int frame_;
> >   	std::vector<IPABuffer> ipaBuffers_;
> > -	RkISP1Frames frameInfo_;
> >   	RkISP1MainPath *mainPath_;
> >   	RkISP1SelfPath *selfPath_;
> >   	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
> > +	/* Associate a frame id with a Request. */
> > +	std::map<unsigned int, RkISP1Request *> requestMap_;
> > +
> >   private:
> >   	void paramFilled(unsigned int frame);
> >   	void setSensorControls(unsigned int frame,
> > @@ -181,13 +168,17 @@ private:
> >   		return static_cast<RkISP1CameraData *>(camera->_d());
> >   	}
> > +	RkISP1Request *cameraRequest(Request *request)
> > +	{
> > +		return static_cast<RkISP1Request *>(request->_d());
> > +	}
> > +
> >   	friend RkISP1CameraData;
> > -	friend RkISP1Frames;
> >   	int initLinks(Camera *camera, const CameraSensor *sensor,
> >   		      const RkISP1CameraConfiguration &config);
> >   	int createCamera(MediaEntity *sensor);
> > -	void tryCompleteRequest(RkISP1FrameInfo *info);
> > +	void tryCompleteRequest(RkISP1Request *request);
> >   	void bufferReady(FrameBuffer *buffer);
> >   	void paramReady(FrameBuffer *buffer);
> >   	void statReady(FrameBuffer *buffer);
> > @@ -218,129 +209,6 @@ private:
> >   	const MediaPad *ispSink_;
> >   };
> > -RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> > -	: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))
> > -{
> > -}
> > -
> > -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
> > -				      bool isRaw)
> > -{
> > -	unsigned int frame = data->frame_;
> > -
> > -	FrameBuffer *paramBuffer = nullptr;
> > -	FrameBuffer *statBuffer = nullptr;
> > -
> > -	if (!isRaw) {
> > -		if (pipe_->availableParamBuffers_.empty()) {
> > -			LOG(RkISP1, Error) << "Parameters buffer underrun";
> > -			return nullptr;
> > -		}
> > -
> > -		if (pipe_->availableStatBuffers_.empty()) {
> > -			LOG(RkISP1, Error) << "Statistic buffer underrun";
> > -			return nullptr;
> > -		}
> > -
> > -		paramBuffer = pipe_->availableParamBuffers_.front();
> > -		pipe_->availableParamBuffers_.pop();
> > -
> > -		statBuffer = pipe_->availableStatBuffers_.front();
> > -		pipe_->availableStatBuffers_.pop();
> > -	}
> > -
> > -	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> > -	FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
> > -
> > -	RkISP1FrameInfo *info = new RkISP1FrameInfo;
> > -
> > -	info->frame = frame;
> > -	info->request = request;
> > -	info->paramBuffer = paramBuffer;
> > -	info->mainPathBuffer = mainPathBuffer;
> > -	info->selfPathBuffer = selfPathBuffer;
> > -	info->statBuffer = statBuffer;
> > -	info->paramDequeued = false;
> > -	info->metadataProcessed = false;
> > -
> > -	frameInfo_[frame] = info;
> > -
> > -	return info;
> > -}
> > -
> > -int RkISP1Frames::destroy(unsigned int frame)
> > -{
> > -	RkISP1FrameInfo *info = find(frame);
> > -	if (!info)
> > -		return -ENOENT;
> > -
> > -	pipe_->availableParamBuffers_.push(info->paramBuffer);
> > -	pipe_->availableStatBuffers_.push(info->statBuffer);
> > -
> > -	frameInfo_.erase(info->frame);
> > -
> > -	delete info;
> > -
> > -	return 0;
> > -}
> > -
> > -void RkISP1Frames::clear()
> > -{
> > -	for (const auto &entry : frameInfo_) {
> > -		RkISP1FrameInfo *info = entry.second;
> > -
> > -		pipe_->availableParamBuffers_.push(info->paramBuffer);
> > -		pipe_->availableStatBuffers_.push(info->statBuffer);
> > -
> > -		delete info;
> > -	}
> > -
> > -	frameInfo_.clear();
> > -}
> > -
> > -RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
> > -{
> > -	auto itInfo = frameInfo_.find(frame);
> > -
> > -	if (itInfo != frameInfo_.end())
> > -		return itInfo->second;
> > -
> > -	LOG(RkISP1, Fatal) << "Can't locate info from frame";
> > -
> > -	return nullptr;
> > -}
> > -
> > -RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> > -{
> > -	for (auto &itInfo : frameInfo_) {
> > -		RkISP1FrameInfo *info = itInfo.second;
> > -
> > -		if (info->paramBuffer == buffer ||
> > -		    info->statBuffer == buffer ||
> > -		    info->mainPathBuffer == buffer ||
> > -		    info->selfPathBuffer == buffer)
> > -			return info;
> > -	}
> > -
> > -	LOG(RkISP1, Fatal) << "Can't locate info from buffer";
> > -
> > -	return nullptr;
> > -}
> > -
> > -RkISP1FrameInfo *RkISP1Frames::find(Request *request)
> > -{
> > -	for (auto &itInfo : frameInfo_) {
> > -		RkISP1FrameInfo *info = itInfo.second;
> > -
> > -		if (info->request == request)
> > -			return info;
> > -	}
> > -
> > -	LOG(RkISP1, Fatal) << "Can't locate info from request";
> > -
> > -	return nullptr;
> > -}
> > -
> >   PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
> >   {
> >   	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> > @@ -391,23 +259,34 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> >   	return 0;
> >   }
> > +void RkISP1CameraData::addRequest(RkISP1Request *request)
> > +{
> > +	/* Associate the request and the frame number. */
> > +	request->frame = frame_;
> > +	requestMap_[frame_] = request;
> > +	frame_++;
> > +}
> > +
> >   void RkISP1CameraData::paramFilled(unsigned int frame)
> >   {
> >   	PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> > -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> > -	if (!info)
> > -		return;
> > +	RkISP1Request *request = requestMap_.at(frame);
> > +	ASSERT(request);
> > -	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> > +	request->paramBuffer->_d()->metadata().planes()[0].bytesused =
> >   		sizeof(struct rkisp1_params_cfg);
> > -	pipe->param_->queueBuffer(info->paramBuffer);
> > -	pipe->stat_->queueBuffer(info->statBuffer);
> > -
> > -	if (info->mainPathBuffer)
> > -		mainPath_->queueBuffer(info->mainPathBuffer);
> > -
> > -	if (selfPath_ && info->selfPathBuffer)
> > -		selfPath_->queueBuffer(info->selfPathBuffer);
> > +	pipe->param_->queueBuffer(request->paramBuffer);
> > +	pipe->stat_->queueBuffer(request->statBuffer);
> > +
> > +	FrameBuffer *mainPathBuffer =
> > +		request->_o<Request>()->findBuffer(&mainPathStream_);
> > +	if (mainPathBuffer)
> > +		mainPath_->queueBuffer(mainPathBuffer);
> > +
> > +	FrameBuffer *selfPathBuffer =
> > +		request->_o<Request>()->findBuffer(&selfPathStream_);
> > +	if (selfPath_ && selfPathBuffer)
> > +		selfPath_->queueBuffer(selfPathBuffer);
> >   }
> >   void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> > @@ -418,14 +297,13 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> >   void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
> >   {
> > -	RkISP1FrameInfo *info = frameInfo_.find(frame);
> > -	if (!info)
> > -		return;
> > +	RkISP1Request *request = requestMap_.at(frame);
> > +	ASSERT(request);
> > -	info->request->metadata().merge(metadata);
> > -	info->metadataProcessed = true;
> > +	request->_o<Request>()->metadata().merge(metadata);
> > +	request->metadataProcessed = true;
> > -	pipe()->tryCompleteRequest(info);
> > +	pipe()->tryCompleteRequest(request);
> >   }
> >   /* -----------------------------------------------------------------------------
> > @@ -1025,7 +903,7 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
> >   	}
> >   	ASSERT(data->queuedRequests_.empty());
> > -	data->frameInfo_.clear();
> > +	data->requestMap_.clear();
> >   	freeBuffers(camera);
> > @@ -1042,24 +920,60 @@ std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *came
> >   int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
> >   {
> >   	RkISP1CameraData *data = cameraData(camera);
> > +	RkISP1Request *rkisp1Request = cameraRequest(request);
> > -	RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
> > -	if (!info)
> > -		return -ENOENT;
> > +	data->addRequest(rkisp1Request);
> > -	data->ipa_->queueRequest(data->frame_, request->controls());
> > +	/*
> > +	 * If we're operating in RAW mode (only one RAW stream is captured)
> > +	 * then we simply queue buffers to the video devices as we don't
> > +	 * need to run the IPA.
> > +	 */
> >   	if (isRaw_) {
> > -		if (info->mainPathBuffer)
> > -			data->mainPath_->queueBuffer(info->mainPathBuffer);
> > +		FrameBuffer *mainPathBuffer =
> > +			request->findBuffer(&data->mainPathStream_);
> > +		if (mainPathBuffer)
> > +			data->mainPath_->queueBuffer(mainPathBuffer);
> > -		if (data->selfPath_ && info->selfPathBuffer)
> > -			data->selfPath_->queueBuffer(info->selfPathBuffer);
> > -	} else {
> > -		data->ipa_->fillParamsBuffer(data->frame_,
> > -					     info->paramBuffer->cookie());
> > +		FrameBuffer *selfPathBuffer =
> > +			request->findBuffer(&data->selfPathStream_);
> > +		if (data->selfPath_ && selfPathBuffer)
> > +			data->selfPath_->queueBuffer(selfPathBuffer);
> > +
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * If we run the IPA we need to associate a parameters and a statistics
> > +	 * buffer with the Request and associate the request with the current
> > +	 * frame number.
> > +	 *
> > +	 * Associate the stat and frame buffers to a Request (if available)
> > +	 * and then run the IPA.
> > +	 */
> > +	if (availableParamBuffers_.empty()) {
> > +		LOG(RkISP1, Error) << "Parameters buffer underrun";
> > +		return -ENOENT;
> >   	}
> > -	data->frame_++;
> > +	if (availableStatBuffers_.empty()) {
> > +		LOG(RkISP1, Error) << "Statistic buffer underrun";
> > +		return -ENOENT;
> > +	}
> > +
> > +	rkisp1Request->paramBuffer = availableParamBuffers_.front();
> > +	rkisp1Request->paramDequeued = false;
> > +	rkisp1Request->paramBuffer->_d()->setRequest(request);
> > +	availableParamBuffers_.pop();
> > +
> > +	rkisp1Request->statBuffer = availableStatBuffers_.front();
> > +	rkisp1Request->metadataProcessed = false;
> > +	rkisp1Request->statBuffer->_d()->setRequest(request);
> > +	availableStatBuffers_.pop();
> > +
> > +	data->ipa_->queueRequest(rkisp1Request->frame, request->controls());
> > +	data->ipa_->fillParamsBuffer(rkisp1Request->frame,
> > +				     rkisp1Request->paramBuffer->cookie());
> >   	return 0;
> >   }
> > @@ -1251,37 +1165,28 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >    * Buffer Handling
> >    */
> > -void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
> > +void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1Request *request)
> >   {
> >   	RkISP1CameraData *data = cameraData(activeCamera_);
> > -	Request *request = info->request;
> > -
> > -	if (request->hasPendingBuffers())
> > -		return;
> > -
> > -	if (!info->metadataProcessed)
> > -		return;
> > -	if (!isRaw_ && !info->paramDequeued)
> > +	if (request->hasPendingBuffers(isRaw_))
> >   		return;
> > -	data->frameInfo_.destroy(info->frame);
> > +	/* Return the stat and param buffers to the pipeline. */
> > +	availableParamBuffers_.push(request->paramBuffer);
> > +	availableStatBuffers_.push(request->statBuffer);
> > +	data->requestMap_.erase(request->frame);
> > -	completeRequest(request);
> > +	completeRequest(request->_o<Request>());
> >   }
> >   void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> >   {
> >   	ASSERT(activeCamera_);
> >   	RkISP1CameraData *data = cameraData(activeCamera_);
> > -
> > -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> > -	if (!info)
> > -		return;
> > +	RkISP1Request *request = cameraRequest(buffer->request());
> >   	const FrameMetadata &metadata = buffer->metadata();
> > -	Request *request = buffer->request();
> > -
> >   	if (metadata.status != FrameMetadata::FrameCancelled) {
> >   		/*
> >   		 * Record the sensor's timestamp in the request metadata.
> > @@ -1289,55 +1194,48 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> >   		 * \todo The sensor timestamp should be better estimated by connecting
> >   		 * to the V4L2Device::frameStart signal.
> >   		 */
> > -		request->metadata().set(controls::SensorTimestamp,
> > -					metadata.timestamp);
> > +		request->_o<Request>()->metadata().set(controls::SensorTimestamp,
> > +						       metadata.timestamp);
> >   		if (isRaw_) {
> >   			const ControlList &ctrls =
> >   				data->delayedCtrls_->get(metadata.sequence);
> > -			data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
> > +			data->ipa_->processStatsBuffer(request->frame, 0, ctrls);
> >   		}
> >   	} else {
> >   		if (isRaw_)
> > -			info->metadataProcessed = true;
> > +			request->metadataProcessed = true;
> >   	}
> > -	completeBuffer(request, buffer);
> > -	tryCompleteRequest(info);
> > +	completeBuffer(request->_o<Request>(), buffer);
> > +	tryCompleteRequest(request);
> >   }
> >   void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
> >   {
> >   	ASSERT(activeCamera_);
> > -	RkISP1CameraData *data = cameraData(activeCamera_);
> > +	RkISP1Request *request = cameraRequest(buffer->request());
> > -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> > -	if (!info)
> > -		return;
> > -
> > -	info->paramDequeued = true;
> > -	tryCompleteRequest(info);
> > +	request->paramDequeued = true;
> > +	tryCompleteRequest(request);
> >   }
> >   void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
> >   {
> >   	ASSERT(activeCamera_);
> >   	RkISP1CameraData *data = cameraData(activeCamera_);
> > -
> > -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> > -	if (!info)
> > -		return;
> > +	RkISP1Request *request = cameraRequest(buffer->request());
> >   	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> > -		info->metadataProcessed = true;
> > -		tryCompleteRequest(info);
> > +		request->metadataProcessed = true;
> > +		tryCompleteRequest(request);
> >   		return;
> >   	}
> >   	if (data->frame_ <= buffer->metadata().sequence)
> >   		data->frame_ = buffer->metadata().sequence + 1;
> > -	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
> > +	data->ipa_->processStatsBuffer(request->frame, request->statBuffer->cookie(),
> >   				       data->delayedCtrls_->get(buffer->metadata().sequence));
> >   }

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index e981e60758f7..f2163f528251 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -49,76 +49,63 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(RkISP1)
 
-class PipelineHandlerRkISP1;
-class RkISP1CameraData;
+class RkISP1Request : public Request::Private
+{
+public:
+	RkISP1Request(Camera *camera)
+		: Request::Private(camera)
+	{
+	}
 
-struct RkISP1FrameInfo {
-	unsigned int frame;
-	Request *request;
+	bool hasPendingBuffers(bool isRaw) const;
 
-	FrameBuffer *paramBuffer;
 	FrameBuffer *statBuffer;
-	FrameBuffer *mainPathBuffer;
-	FrameBuffer *selfPathBuffer;
+	FrameBuffer *paramBuffer;
+
+	/* The frame number this request is associated with. */
+	unsigned int frame;
 
 	bool paramDequeued;
 	bool metadataProcessed;
 };
 
-class RkISP1Frames
-{
-public:
-	RkISP1Frames(PipelineHandler *pipe);
-
-	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
-				bool isRaw);
-	int destroy(unsigned int frame);
-	void clear();
-
-	RkISP1FrameInfo *find(unsigned int frame);
-	RkISP1FrameInfo *find(FrameBuffer *buffer);
-	RkISP1FrameInfo *find(Request *request);
-
-private:
-	PipelineHandlerRkISP1 *pipe_;
-	std::map<unsigned int, RkISP1FrameInfo *> frameInfo_;
-};
-
-class RkISP1Request : public Request::Private
+bool RkISP1Request::hasPendingBuffers(bool isRaw) const
 {
-public:
-	RkISP1Request(Camera *camera)
-		: Request::Private(camera)
-	{
-	}
-};
+	return Request::Private::hasPendingBuffers() ||
+	       !metadataProcessed || (!isRaw && !paramDequeued);
+}
 
+class PipelineHandlerRkISP1;
 class RkISP1CameraData : public Camera::Private
 {
 public:
 	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
 			 RkISP1SelfPath *selfPath)
-		: Camera::Private(pipe), frame_(0), frameInfo_(pipe),
-		  mainPath_(mainPath), selfPath_(selfPath)
+		: Camera::Private(pipe), frame_(0), mainPath_(mainPath),
+		  selfPath_(selfPath)
 	{
 	}
 
 	PipelineHandlerRkISP1 *pipe();
 	int loadIPA(unsigned int hwRevision);
 
+	void addRequest(RkISP1Request *request);
+
 	Stream mainPathStream_;
 	Stream selfPathStream_;
 	std::unique_ptr<CameraSensor> sensor_;
 	std::unique_ptr<DelayedControls> delayedCtrls_;
 	unsigned int frame_;
 	std::vector<IPABuffer> ipaBuffers_;
-	RkISP1Frames frameInfo_;
 
 	RkISP1MainPath *mainPath_;
 	RkISP1SelfPath *selfPath_;
 
 	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
 
+	/* Associate a frame id with a Request. */
+	std::map<unsigned int, RkISP1Request *> requestMap_;
+
 private:
 	void paramFilled(unsigned int frame);
 	void setSensorControls(unsigned int frame,
@@ -181,13 +168,17 @@  private:
 		return static_cast<RkISP1CameraData *>(camera->_d());
 	}
 
+	RkISP1Request *cameraRequest(Request *request)
+	{
+		return static_cast<RkISP1Request *>(request->_d());
+	}
+
 	friend RkISP1CameraData;
-	friend RkISP1Frames;
 
 	int initLinks(Camera *camera, const CameraSensor *sensor,
 		      const RkISP1CameraConfiguration &config);
 	int createCamera(MediaEntity *sensor);
-	void tryCompleteRequest(RkISP1FrameInfo *info);
+	void tryCompleteRequest(RkISP1Request *request);
 	void bufferReady(FrameBuffer *buffer);
 	void paramReady(FrameBuffer *buffer);
 	void statReady(FrameBuffer *buffer);
@@ -218,129 +209,6 @@  private:
 	const MediaPad *ispSink_;
 };
 
-RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
-	: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))
-{
-}
-
-RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
-				      bool isRaw)
-{
-	unsigned int frame = data->frame_;
-
-	FrameBuffer *paramBuffer = nullptr;
-	FrameBuffer *statBuffer = nullptr;
-
-	if (!isRaw) {
-		if (pipe_->availableParamBuffers_.empty()) {
-			LOG(RkISP1, Error) << "Parameters buffer underrun";
-			return nullptr;
-		}
-
-		if (pipe_->availableStatBuffers_.empty()) {
-			LOG(RkISP1, Error) << "Statistic buffer underrun";
-			return nullptr;
-		}
-
-		paramBuffer = pipe_->availableParamBuffers_.front();
-		pipe_->availableParamBuffers_.pop();
-
-		statBuffer = pipe_->availableStatBuffers_.front();
-		pipe_->availableStatBuffers_.pop();
-	}
-
-	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
-	FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
-
-	RkISP1FrameInfo *info = new RkISP1FrameInfo;
-
-	info->frame = frame;
-	info->request = request;
-	info->paramBuffer = paramBuffer;
-	info->mainPathBuffer = mainPathBuffer;
-	info->selfPathBuffer = selfPathBuffer;
-	info->statBuffer = statBuffer;
-	info->paramDequeued = false;
-	info->metadataProcessed = false;
-
-	frameInfo_[frame] = info;
-
-	return info;
-}
-
-int RkISP1Frames::destroy(unsigned int frame)
-{
-	RkISP1FrameInfo *info = find(frame);
-	if (!info)
-		return -ENOENT;
-
-	pipe_->availableParamBuffers_.push(info->paramBuffer);
-	pipe_->availableStatBuffers_.push(info->statBuffer);
-
-	frameInfo_.erase(info->frame);
-
-	delete info;
-
-	return 0;
-}
-
-void RkISP1Frames::clear()
-{
-	for (const auto &entry : frameInfo_) {
-		RkISP1FrameInfo *info = entry.second;
-
-		pipe_->availableParamBuffers_.push(info->paramBuffer);
-		pipe_->availableStatBuffers_.push(info->statBuffer);
-
-		delete info;
-	}
-
-	frameInfo_.clear();
-}
-
-RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
-{
-	auto itInfo = frameInfo_.find(frame);
-
-	if (itInfo != frameInfo_.end())
-		return itInfo->second;
-
-	LOG(RkISP1, Fatal) << "Can't locate info from frame";
-
-	return nullptr;
-}
-
-RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
-{
-	for (auto &itInfo : frameInfo_) {
-		RkISP1FrameInfo *info = itInfo.second;
-
-		if (info->paramBuffer == buffer ||
-		    info->statBuffer == buffer ||
-		    info->mainPathBuffer == buffer ||
-		    info->selfPathBuffer == buffer)
-			return info;
-	}
-
-	LOG(RkISP1, Fatal) << "Can't locate info from buffer";
-
-	return nullptr;
-}
-
-RkISP1FrameInfo *RkISP1Frames::find(Request *request)
-{
-	for (auto &itInfo : frameInfo_) {
-		RkISP1FrameInfo *info = itInfo.second;
-
-		if (info->request == request)
-			return info;
-	}
-
-	LOG(RkISP1, Fatal) << "Can't locate info from request";
-
-	return nullptr;
-}
-
 PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
 {
 	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
@@ -391,23 +259,34 @@  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 	return 0;
 }
 
+void RkISP1CameraData::addRequest(RkISP1Request *request)
+{
+	/* Associate the request and the frame number. */
+	request->frame = frame_;
+	requestMap_[frame_] = request;
+	frame_++;
+}
+
 void RkISP1CameraData::paramFilled(unsigned int frame)
 {
 	PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
-	RkISP1FrameInfo *info = frameInfo_.find(frame);
-	if (!info)
-		return;
+	RkISP1Request *request = requestMap_.at(frame);
+	ASSERT(request);
 
-	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
+	request->paramBuffer->_d()->metadata().planes()[0].bytesused =
 		sizeof(struct rkisp1_params_cfg);
-	pipe->param_->queueBuffer(info->paramBuffer);
-	pipe->stat_->queueBuffer(info->statBuffer);
-
-	if (info->mainPathBuffer)
-		mainPath_->queueBuffer(info->mainPathBuffer);
-
-	if (selfPath_ && info->selfPathBuffer)
-		selfPath_->queueBuffer(info->selfPathBuffer);
+	pipe->param_->queueBuffer(request->paramBuffer);
+	pipe->stat_->queueBuffer(request->statBuffer);
+
+	FrameBuffer *mainPathBuffer =
+		request->_o<Request>()->findBuffer(&mainPathStream_);
+	if (mainPathBuffer)
+		mainPath_->queueBuffer(mainPathBuffer);
+
+	FrameBuffer *selfPathBuffer =
+		request->_o<Request>()->findBuffer(&selfPathStream_);
+	if (selfPath_ && selfPathBuffer)
+		selfPath_->queueBuffer(selfPathBuffer);
 }
 
 void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
@@ -418,14 +297,13 @@  void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
 
 void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
 {
-	RkISP1FrameInfo *info = frameInfo_.find(frame);
-	if (!info)
-		return;
+	RkISP1Request *request = requestMap_.at(frame);
+	ASSERT(request);
 
-	info->request->metadata().merge(metadata);
-	info->metadataProcessed = true;
+	request->_o<Request>()->metadata().merge(metadata);
+	request->metadataProcessed = true;
 
-	pipe()->tryCompleteRequest(info);
+	pipe()->tryCompleteRequest(request);
 }
 
 /* -----------------------------------------------------------------------------
@@ -1025,7 +903,7 @@  void PipelineHandlerRkISP1::stopDevice(Camera *camera)
 	}
 
 	ASSERT(data->queuedRequests_.empty());
-	data->frameInfo_.clear();
+	data->requestMap_.clear();
 
 	freeBuffers(camera);
 
@@ -1042,24 +920,60 @@  std::unique_ptr<Request> PipelineHandlerRkISP1::createRequestDevice(Camera *came
 int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 {
 	RkISP1CameraData *data = cameraData(camera);
+	RkISP1Request *rkisp1Request = cameraRequest(request);
 
-	RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
-	if (!info)
-		return -ENOENT;
+	data->addRequest(rkisp1Request);
 
-	data->ipa_->queueRequest(data->frame_, request->controls());
+	/*
+	 * If we're operating in RAW mode (only one RAW stream is captured)
+	 * then we simply queue buffers to the video devices as we don't
+	 * need to run the IPA.
+	 */
 	if (isRaw_) {
-		if (info->mainPathBuffer)
-			data->mainPath_->queueBuffer(info->mainPathBuffer);
+		FrameBuffer *mainPathBuffer =
+			request->findBuffer(&data->mainPathStream_);
+		if (mainPathBuffer)
+			data->mainPath_->queueBuffer(mainPathBuffer);
 
-		if (data->selfPath_ && info->selfPathBuffer)
-			data->selfPath_->queueBuffer(info->selfPathBuffer);
-	} else {
-		data->ipa_->fillParamsBuffer(data->frame_,
-					     info->paramBuffer->cookie());
+		FrameBuffer *selfPathBuffer =
+			request->findBuffer(&data->selfPathStream_);
+		if (data->selfPath_ && selfPathBuffer)
+			data->selfPath_->queueBuffer(selfPathBuffer);
+
+		return 0;
+	}
+
+	/*
+	 * If we run the IPA we need to associate a parameters and a statistics
+	 * buffer with the Request and associate the request with the current
+	 * frame number.
+	 *
+	 * Associate the stat and frame buffers to a Request (if available)
+	 * and then run the IPA.
+	 */
+	if (availableParamBuffers_.empty()) {
+		LOG(RkISP1, Error) << "Parameters buffer underrun";
+		return -ENOENT;
 	}
 
-	data->frame_++;
+	if (availableStatBuffers_.empty()) {
+		LOG(RkISP1, Error) << "Statistic buffer underrun";
+		return -ENOENT;
+	}
+
+	rkisp1Request->paramBuffer = availableParamBuffers_.front();
+	rkisp1Request->paramDequeued = false;
+	rkisp1Request->paramBuffer->_d()->setRequest(request);
+	availableParamBuffers_.pop();
+
+	rkisp1Request->statBuffer = availableStatBuffers_.front();
+	rkisp1Request->metadataProcessed = false;
+	rkisp1Request->statBuffer->_d()->setRequest(request);
+	availableStatBuffers_.pop();
+
+	data->ipa_->queueRequest(rkisp1Request->frame, request->controls());
+	data->ipa_->fillParamsBuffer(rkisp1Request->frame,
+				     rkisp1Request->paramBuffer->cookie());
 
 	return 0;
 }
@@ -1251,37 +1165,28 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
  * Buffer Handling
  */
 
-void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
+void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1Request *request)
 {
 	RkISP1CameraData *data = cameraData(activeCamera_);
-	Request *request = info->request;
-
-	if (request->hasPendingBuffers())
-		return;
-
-	if (!info->metadataProcessed)
-		return;
 
-	if (!isRaw_ && !info->paramDequeued)
+	if (request->hasPendingBuffers(isRaw_))
 		return;
 
-	data->frameInfo_.destroy(info->frame);
+	/* Return the stat and param buffers to the pipeline. */
+	availableParamBuffers_.push(request->paramBuffer);
+	availableStatBuffers_.push(request->statBuffer);
+	data->requestMap_.erase(request->frame);
 
-	completeRequest(request);
+	completeRequest(request->_o<Request>());
 }
 
 void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
 	RkISP1CameraData *data = cameraData(activeCamera_);
-
-	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
-	if (!info)
-		return;
+	RkISP1Request *request = cameraRequest(buffer->request());
 
 	const FrameMetadata &metadata = buffer->metadata();
-	Request *request = buffer->request();
-
 	if (metadata.status != FrameMetadata::FrameCancelled) {
 		/*
 		 * Record the sensor's timestamp in the request metadata.
@@ -1289,55 +1194,48 @@  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 		 * \todo The sensor timestamp should be better estimated by connecting
 		 * to the V4L2Device::frameStart signal.
 		 */
-		request->metadata().set(controls::SensorTimestamp,
-					metadata.timestamp);
+		request->_o<Request>()->metadata().set(controls::SensorTimestamp,
+						       metadata.timestamp);
 
 		if (isRaw_) {
 			const ControlList &ctrls =
 				data->delayedCtrls_->get(metadata.sequence);
-			data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
+			data->ipa_->processStatsBuffer(request->frame, 0, ctrls);
 		}
 	} else {
 		if (isRaw_)
-			info->metadataProcessed = true;
+			request->metadataProcessed = true;
 	}
 
-	completeBuffer(request, buffer);
-	tryCompleteRequest(info);
+	completeBuffer(request->_o<Request>(), buffer);
+	tryCompleteRequest(request);
 }
 
 void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
-	RkISP1CameraData *data = cameraData(activeCamera_);
+	RkISP1Request *request = cameraRequest(buffer->request());
 
-	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
-	if (!info)
-		return;
-
-	info->paramDequeued = true;
-	tryCompleteRequest(info);
+	request->paramDequeued = true;
+	tryCompleteRequest(request);
 }
 
 void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
 	RkISP1CameraData *data = cameraData(activeCamera_);
-
-	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
-	if (!info)
-		return;
+	RkISP1Request *request = cameraRequest(buffer->request());
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
-		info->metadataProcessed = true;
-		tryCompleteRequest(info);
+		request->metadataProcessed = true;
+		tryCompleteRequest(request);
 		return;
 	}
 
 	if (data->frame_ <= buffer->metadata().sequence)
 		data->frame_ = buffer->metadata().sequence + 1;
 
-	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
+	data->ipa_->processStatsBuffer(request->frame, request->statBuffer->cookie(),
 				       data->delayedCtrls_->get(buffer->metadata().sequence));
 }