[libcamera-devel] [PATCH v2 08/11] libcamera: pipeline: ipu3: Migrate to Camera::Private

[Jacopo Mondi]

Hi Laurent

On Thu, Aug 05, 2021 at 08:58:45PM +0300, Laurent Pinchart wrote:
> As part of the effort to remove the CameraData class, migrate the
> pipeline handler-specific camera data from CameraData to the
> Camera::Private class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 38 +++++++++++++---------------
>  1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 15d6cca609ff..6d097ac91d2e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -53,11 +53,11 @@ static const ControlInfoMap::Map IPU3Controls = {
>  	{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },
>  };
>
> -class IPU3CameraData : public CameraData
> +class IPU3CameraData : public Camera::Private
>  {
>  public:
>  	IPU3CameraData(PipelineHandler *pipe)
> -		: CameraData(pipe), exposureTime_(0), supportsFlips_(false)
> +		: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)
>  	{
>  	}
>
> @@ -146,10 +146,9 @@ public:
>  	bool match(DeviceEnumerator *enumerator) override;
>
>  private:
> -	IPU3CameraData *cameraData(const Camera *camera)
> +	IPU3CameraData *cameraData(Camera *camera)
>  	{
> -		return static_cast<IPU3CameraData *>(
> -			PipelineHandler::cameraData(camera));
> +		return static_cast<IPU3CameraData *>(camera->_d());
>  	}
>
>  	int initControls(IPU3CameraData *data);
> @@ -800,10 +799,10 @@ void IPU3CameraData::cancelPendingRequests()
>  		for (auto it : request->buffers()) {
>  			FrameBuffer *buffer = it.second;
>  			buffer->cancel();
> -			pipe_->completeBuffer(request, buffer);
> +			pipe()->completeBuffer(request, buffer);

I wonder, being the Private class only visible internally, do we need
to keep Camera::Private::pipe_ private ?

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

>  		}
>
> -		pipe_->completeRequest(request);
> +		pipe()->completeRequest(request);
>  		pendingRequests_.pop();
>  	}
>  }
> @@ -1184,12 +1183,11 @@ int PipelineHandlerIPU3::registerCameras()
>  					&IPU3CameraData::statBufferReady);
>
>  		/* Create and register the Camera instance. */
> -		std::string cameraId = cio2->sensor()->id();
> +		const std::string &cameraId = cio2->sensor()->id();
>  		std::shared_ptr<Camera> camera =
> -			Camera::create(new Camera::Private(this), cameraId,
> -				       streams);
> +			Camera::create(data.release(), cameraId, streams);
>
> -		registerCamera(std::move(camera), std::move(data));
> +		registerCamera(std::move(camera), nullptr);
>
>  		LOG(IPU3, Info)
>  			<< "Registered Camera[" << numCameras << "] \""
> @@ -1204,7 +1202,7 @@ int PipelineHandlerIPU3::registerCameras()
>
>  int IPU3CameraData::loadIPA()
>  {
> -	ipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe_, 1, 1);
> +	ipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe(), 1, 1);
>  	if (!ipa_)
>  		return -ENOENT;
>
> @@ -1261,7 +1259,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,
>
>  		info->metadataProcessed = true;
>  		if (frameInfos_.tryComplete(info))
> -			pipe_->completeRequest(request);
> +			pipe()->completeRequest(request);
>
>  		break;
>  	}
> @@ -1289,7 +1287,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>
>  	Request *request = info->request;
>
> -	pipe_->completeBuffer(request, buffer);
> +	pipe()->completeBuffer(request, buffer);
>
>  	request->metadata().set(controls::draft::PipelineDepth, 3);
>  	/* \todo Move the ExposureTime control to the IPA. */
> @@ -1300,7 +1298,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  	request->metadata().set(controls::ScalerCrop, cropRegion_);
>
>  	if (frameInfos_.tryComplete(info))
> -		pipe_->completeRequest(request);
> +		pipe()->completeRequest(request);
>  }
>
>  /**
> @@ -1332,16 +1330,16 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>  		for (auto it : request->buffers()) {
>  			FrameBuffer *b = it.second;
>  			b->cancel();
> -			pipe_->completeBuffer(request, b);
> +			pipe()->completeBuffer(request, b);
>  		}
>
>  		frameInfos_.remove(info);
> -		pipe_->completeRequest(request);
> +		pipe()->completeRequest(request);
>  		return;
>  	}
>
>  	if (request->findBuffer(&rawStream_))
> -		pipe_->completeBuffer(request, buffer);
> +		pipe()->completeBuffer(request, buffer);
>
>  	ipa::ipu3::IPU3Event ev;
>  	ev.op = ipa::ipu3::EventFillParams;
> @@ -1367,7 +1365,7 @@ void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
>  	Request *request = info->request;
>
>  	if (frameInfos_.tryComplete(info))
> -		pipe_->completeRequest(request);
> +		pipe()->completeRequest(request);
>  }
>
>  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> @@ -1386,7 +1384,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>  		 * In that event, we must have obtained the Request before hand.
>  		 */
>  		if (frameInfos_.tryComplete(info))
> -			pipe_->completeRequest(request);
> +			pipe()->completeRequest(request);
>
>  		return;
>  	}
> --
> Regards,
>
> Laurent Pinchart
>