[libcamera-devel,v3,14/14] libcamera: pipeline: Cast to derived pipeline handler with helpers
diff mbox series

Message ID 20210811232625.17280-15-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Replace CameraData with Camera::Private
Related show

Commit Message

Laurent Pinchart Aug. 11, 2021, 11:26 p.m. UTC 
Replace manual static casts from the PipelineHandler pointer to a
derived class pointer with helper functions in the camera data classes.
This simplifies code accessing the pipeline from the camera data.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++------
 src/libcamera/pipeline/simple/simple.cpp | 16 +++++++++-------
 2 files changed, 17 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi Aug. 12, 2021, 7:56 a.m. UTC | #1 
Hi Laurent,

On Thu, Aug 12, 2021 at 02:26:25AM +0300, Laurent Pinchart wrote:
> Replace manual static casts from the PipelineHandler pointer to a
> derived class pointer with helper functions in the camera data classes.
> This simplifies code accessing the pipeline from the camera data.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks

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


> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++------
>  src/libcamera/pipeline/simple/simple.cpp | 16 +++++++++-------
>  2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 3c37a8ccad8f..2dbd6304acce 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -87,6 +87,7 @@ public:
>  	{
>  	}
>
> +	PipelineHandlerRkISP1 *pipe();
>  	int loadIPA(unsigned int hwRevision);
>
>  	Stream mainPathStream_;
> @@ -304,6 +305,11 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)
>  	return nullptr;
>  }
>
> +PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
> +{
> +	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> +}
> +
>  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  {
>  	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
> @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  		break;
>  	}
>  	case ipa::rkisp1::ActionParamFilled: {
> -		PipelineHandlerRkISP1 *pipe =
> -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
>  		RkISP1FrameInfo *info = frameInfo_.find(frame);
>  		if (!info)
>  			break;
> @@ -360,9 +365,6 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>
>  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
>  {
> -	PipelineHandlerRkISP1 *pipe =
> -		static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> -
>  	RkISP1FrameInfo *info = frameInfo_.find(frame);
>  	if (!info)
>  		return;
> @@ -370,7 +372,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
>  	info->request->metadata().merge(metadata);
>  	info->metadataProcessed = true;
>
> -	pipe->tryCompleteRequest(info->request);
> +	pipe()->tryCompleteRequest(info->request);
>  }
>
>  RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index ef7687eaf502..8ff954722fed 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -155,6 +155,7 @@ public:
>  			 MediaEntity *sensor);
>
>  	bool isValid() const { return sensor_ != nullptr; }
> +	SimplePipelineHandler *pipe();
>
>  	int init();
>  	int setupLinks();
> @@ -352,11 +353,14 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  			       [](const Entity &e) { return e.entity->name(); });
>  }
>
> +SimplePipelineHandler *SimpleCameraData::pipe()
> +{
> +	return static_cast<SimplePipelineHandler *>(Camera::Private::pipe());
> +}
> +
>  int SimpleCameraData::init()
>  {
> -	SimplePipelineHandler *pipe =
> -		static_cast<SimplePipelineHandler *>(this->pipe());
> -	SimpleConverter *converter = pipe->converter();
> +	SimpleConverter *converter = pipe()->converter();
>  	int ret;
>
>  	/*
> @@ -480,8 +484,7 @@ int SimpleCameraData::setupLinks()
>  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  				   V4L2Subdevice::Whence whence)
>  {
> -	SimplePipelineHandler *pipe =
> -		static_cast<SimplePipelineHandler *>(this->pipe());
> +	SimplePipelineHandler *pipe = SimpleCameraData::pipe();
>  	int ret;
>
>  	/*
> @@ -582,8 +585,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  	}
>
>  	/* Adjust the requested streams. */
> -	SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
> -	SimpleConverter *converter = pipe->converter();
> +	SimpleConverter *converter = data_->pipe()->converter();
>
>  	/*
>  	 * Enable usage of the converter when producing multiple streams, as
> --
> Regards,
>
> Laurent Pinchart
>
Kieran Bingham Aug. 16, 2021, 12:20 p.m. UTC | #2 
On 12/08/2021 00:26, Laurent Pinchart wrote:
> Replace manual static casts from the PipelineHandler pointer to a
> derived class pointer with helper functions in the camera data classes.
> This simplifies code accessing the pipeline from the camera data.

And helps reduce the number of places someone might do casting wrong, so
I think that's a good idea, and cleaner code.


> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++------
>  src/libcamera/pipeline/simple/simple.cpp | 16 +++++++++-------

I assume the other pipelines are not affected by this ...



>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 3c37a8ccad8f..2dbd6304acce 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -87,6 +87,7 @@ public:
>  	{
>  	}
>  
> +	PipelineHandlerRkISP1 *pipe();
>  	int loadIPA(unsigned int hwRevision);
>  
>  	Stream mainPathStream_;
> @@ -304,6 +305,11 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)
>  	return nullptr;
>  }
>  
> +PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
> +{
> +	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> +}
> +
>  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  {
>  	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
> @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  		break;
>  	}
>  	case ipa::rkisp1::ActionParamFilled: {
> -		PipelineHandlerRkISP1 *pipe =
> -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();


Why the RkISP1CameraData:: prefix?

Isn't this just pipe(); here?

We're in RkISP1CameraData::queueFrameAction() right ?

With that resolved,

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


>  		RkISP1FrameInfo *info = frameInfo_.find(frame);
>  		if (!info)
>  			break;
> @@ -360,9 +365,6 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  
>  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
>  {
> -	PipelineHandlerRkISP1 *pipe =
> -		static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> -
>  	RkISP1FrameInfo *info = frameInfo_.find(frame);
>  	if (!info)
>  		return;
> @@ -370,7 +372,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
>  	info->request->metadata().merge(metadata);
>  	info->metadataProcessed = true;
>  
> -	pipe->tryCompleteRequest(info->request);
> +	pipe()->tryCompleteRequest(info->request);
>  }
>  
>  RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index ef7687eaf502..8ff954722fed 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -155,6 +155,7 @@ public:
>  			 MediaEntity *sensor);
>  
>  	bool isValid() const { return sensor_ != nullptr; }
> +	SimplePipelineHandler *pipe();
>  
>  	int init();
>  	int setupLinks();
> @@ -352,11 +353,14 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  			       [](const Entity &e) { return e.entity->name(); });
>  }
>  
> +SimplePipelineHandler *SimpleCameraData::pipe()
> +{
> +	return static_cast<SimplePipelineHandler *>(Camera::Private::pipe());
> +}
> +
>  int SimpleCameraData::init()
>  {
> -	SimplePipelineHandler *pipe =
> -		static_cast<SimplePipelineHandler *>(this->pipe());
> -	SimpleConverter *converter = pipe->converter();
> +	SimpleConverter *converter = pipe()->converter();
>  	int ret;
>  
>  	/*
> @@ -480,8 +484,7 @@ int SimpleCameraData::setupLinks()
>  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  				   V4L2Subdevice::Whence whence)
>  {
> -	SimplePipelineHandler *pipe =
> -		static_cast<SimplePipelineHandler *>(this->pipe());
> +	SimplePipelineHandler *pipe = SimpleCameraData::pipe();
>  	int ret;
>  
>  	/*
> @@ -582,8 +585,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  	}
>  
>  	/* Adjust the requested streams. */
> -	SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
> -	SimpleConverter *converter = pipe->converter();
> +	SimpleConverter *converter = data_->pipe()->converter();
>  
>  	/*
>  	 * Enable usage of the converter when producing multiple streams, as
>
Laurent Pinchart Aug. 16, 2021, 12:26 p.m. UTC | #3 
Hi Kieran,

On Mon, Aug 16, 2021 at 01:20:21PM +0100, Kieran Bingham wrote:
> On 12/08/2021 00:26, Laurent Pinchart wrote:
> > Replace manual static casts from the PipelineHandler pointer to a
> > derived class pointer with helper functions in the camera data classes.
> > This simplifies code accessing the pipeline from the camera data.
> 
> And helps reduce the number of places someone might do casting wrong, so
> I think that's a good idea, and cleaner code.
>
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++------
> >  src/libcamera/pipeline/simple/simple.cpp | 16 +++++++++-------
> 
> I assume the other pipelines are not affected by this ...

Correct, none of the other pipeline handler perform such casts at this
point.

> >  2 files changed, 17 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 3c37a8ccad8f..2dbd6304acce 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -87,6 +87,7 @@ public:
> >  	{
> >  	}
> >  
> > +	PipelineHandlerRkISP1 *pipe();
> >  	int loadIPA(unsigned int hwRevision);
> >  
> >  	Stream mainPathStream_;
> > @@ -304,6 +305,11 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)
> >  	return nullptr;
> >  }
> >  
> > +PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
> > +{
> > +	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
> > +}
> > +
> >  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> >  {
> >  	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
> > @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
> >  		break;
> >  	}
> >  	case ipa::rkisp1::ActionParamFilled: {
> > -		PipelineHandlerRkISP1 *pipe =
> > -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> > +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> 
> 
> Why the RkISP1CameraData:: prefix?

Because an unqualified 'pipe' refers to the local variable, so

		PipelineHandlerRkISP1 *pipe = pipe();

makes the compiler complain that 'PipelineHandlerRkISP1 *' has no
operator() defined. It doesn't resolve 'pipe' to the function, but the
local variable.

> Isn't this just pipe(); here?
> 
> We're in RkISP1CameraData::queueFrameAction() right ?
> 
> With that resolved,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> 
> >  		RkISP1FrameInfo *info = frameInfo_.find(frame);
> >  		if (!info)
> >  			break;
> > @@ -360,9 +365,6 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
> >  
> >  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
> >  {
> > -	PipelineHandlerRkISP1 *pipe =
> > -		static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> > -
> >  	RkISP1FrameInfo *info = frameInfo_.find(frame);
> >  	if (!info)
> >  		return;
> > @@ -370,7 +372,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
> >  	info->request->metadata().merge(metadata);
> >  	info->metadataProcessed = true;
> >  
> > -	pipe->tryCompleteRequest(info->request);
> > +	pipe()->tryCompleteRequest(info->request);
> >  }
> >  
> >  RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index ef7687eaf502..8ff954722fed 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -155,6 +155,7 @@ public:
> >  			 MediaEntity *sensor);
> >  
> >  	bool isValid() const { return sensor_ != nullptr; }
> > +	SimplePipelineHandler *pipe();
> >  
> >  	int init();
> >  	int setupLinks();
> > @@ -352,11 +353,14 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> >  			       [](const Entity &e) { return e.entity->name(); });
> >  }
> >  
> > +SimplePipelineHandler *SimpleCameraData::pipe()
> > +{
> > +	return static_cast<SimplePipelineHandler *>(Camera::Private::pipe());
> > +}
> > +
> >  int SimpleCameraData::init()
> >  {
> > -	SimplePipelineHandler *pipe =
> > -		static_cast<SimplePipelineHandler *>(this->pipe());
> > -	SimpleConverter *converter = pipe->converter();
> > +	SimpleConverter *converter = pipe()->converter();
> >  	int ret;
> >  
> >  	/*
> > @@ -480,8 +484,7 @@ int SimpleCameraData::setupLinks()
> >  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> >  				   V4L2Subdevice::Whence whence)
> >  {
> > -	SimplePipelineHandler *pipe =
> > -		static_cast<SimplePipelineHandler *>(this->pipe());
> > +	SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> >  	int ret;
> >  
> >  	/*
> > @@ -582,8 +585,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> >  	}
> >  
> >  	/* Adjust the requested streams. */
> > -	SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
> > -	SimpleConverter *converter = pipe->converter();
> > +	SimpleConverter *converter = data_->pipe()->converter();
> >  
> >  	/*
> >  	 * Enable usage of the converter when producing multiple streams, as
Kieran Bingham Aug. 16, 2021, 12:30 p.m. UTC | #4 
On 16/08/2021 13:26, Laurent Pinchart wrote:
>>> @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>>>  		break;
>>>  	}
>>>  	case ipa::rkisp1::ActionParamFilled: {
>>> -		PipelineHandlerRkISP1 *pipe =
>>> -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
>>> +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
>>
>>
>> Why the RkISP1CameraData:: prefix?
> 
> Because an unqualified 'pipe' refers to the local variable, so
> 
> 		PipelineHandlerRkISP1 *pipe = pipe();
> 
> makes the compiler complain that 'PipelineHandlerRkISP1 *' has no
> operator() defined. It doesn't resolve 'pipe' to the function, but the
> local variable.

But is the local variable needed?

You could change the two lines :

 		pipe->param_->queueBuffer(info->paramBuffer);
 		pipe->stat_->queueBuffer(info->statBuffer);

to
 		pipe()->param_->queueBuffer(info->paramBuffer);
 		pipe()->stat_->queueBuffer(info->statBuffer);


Which would match the usage everywhere else...


Anyway, it's a valid construct as you have it - it just looked out of place.

Tag still holds however you decide to do this.


> 
>> Isn't this just pipe(); here?
>>
>> We're in RkISP1CameraData::queueFrameAction() right ?
>>
>> With that resolved,
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Laurent Pinchart Aug. 16, 2021, 12:55 p.m. UTC | #5 
Hi Kieran,

On Mon, Aug 16, 2021 at 01:30:36PM +0100, Kieran Bingham wrote:
> On 16/08/2021 13:26, Laurent Pinchart wrote:
> >>> @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
> >>>  		break;
> >>>  	}
> >>>  	case ipa::rkisp1::ActionParamFilled: {
> >>> -		PipelineHandlerRkISP1 *pipe =
> >>> -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
> >>> +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> >>
> >>
> >> Why the RkISP1CameraData:: prefix?
> > 
> > Because an unqualified 'pipe' refers to the local variable, so
> > 
> > 		PipelineHandlerRkISP1 *pipe = pipe();
> > 
> > makes the compiler complain that 'PipelineHandlerRkISP1 *' has no
> > operator() defined. It doesn't resolve 'pipe' to the function, but the
> > local variable.
> 
> But is the local variable needed?
> 
> You could change the two lines :
> 
>  		pipe->param_->queueBuffer(info->paramBuffer);
>  		pipe->stat_->queueBuffer(info->statBuffer);
> 
> to
>  		pipe()->param_->queueBuffer(info->paramBuffer);
>  		pipe()->stat_->queueBuffer(info->statBuffer);
> 
> 
> Which would match the usage everywhere else...

Yes indeed. I overall tried to use a local variable when pipe() would be
called multiple times, but got lazy in a few places I suppose. I haven't
removed existing variables, and was actually thinking about adding more
in the few places where I haven't yet :-)

> Anyway, it's a valid construct as you have it - it just looked out of place.
> 
> Tag still holds however you decide to do this.
> 
> >> Isn't this just pipe(); here?
> >>
> >> We're in RkISP1CameraData::queueFrameAction() right ?
> >>
> >> With that resolved,
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 3c37a8ccad8f..2dbd6304acce 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -87,6 +87,7 @@  public:
 	{
 	}
 
+	PipelineHandlerRkISP1 *pipe();
 	int loadIPA(unsigned int hwRevision);
 
 	Stream mainPathStream_;
@@ -304,6 +305,11 @@  RkISP1FrameInfo *RkISP1Frames::find(Request *request)
 	return nullptr;
 }
 
+PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
+{
+	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
+}
+
 int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 {
 	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
@@ -332,8 +338,7 @@  void RkISP1CameraData::queueFrameAction(unsigned int frame,
 		break;
 	}
 	case ipa::rkisp1::ActionParamFilled: {
-		PipelineHandlerRkISP1 *pipe =
-			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
+		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
 		RkISP1FrameInfo *info = frameInfo_.find(frame);
 		if (!info)
 			break;
@@ -360,9 +365,6 @@  void RkISP1CameraData::queueFrameAction(unsigned int frame,
 
 void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
 {
-	PipelineHandlerRkISP1 *pipe =
-		static_cast<PipelineHandlerRkISP1 *>(this->pipe());
-
 	RkISP1FrameInfo *info = frameInfo_.find(frame);
 	if (!info)
 		return;
@@ -370,7 +372,7 @@  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
 	info->request->metadata().merge(metadata);
 	info->metadataProcessed = true;
 
-	pipe->tryCompleteRequest(info->request);
+	pipe()->tryCompleteRequest(info->request);
 }
 
 RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index ef7687eaf502..8ff954722fed 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -155,6 +155,7 @@  public:
 			 MediaEntity *sensor);
 
 	bool isValid() const { return sensor_ != nullptr; }
+	SimplePipelineHandler *pipe();
 
 	int init();
 	int setupLinks();
@@ -352,11 +353,14 @@  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
 			       [](const Entity &e) { return e.entity->name(); });
 }
 
+SimplePipelineHandler *SimpleCameraData::pipe()
+{
+	return static_cast<SimplePipelineHandler *>(Camera::Private::pipe());
+}
+
 int SimpleCameraData::init()
 {
-	SimplePipelineHandler *pipe =
-		static_cast<SimplePipelineHandler *>(this->pipe());
-	SimpleConverter *converter = pipe->converter();
+	SimpleConverter *converter = pipe()->converter();
 	int ret;
 
 	/*
@@ -480,8 +484,7 @@  int SimpleCameraData::setupLinks()
 int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
 				   V4L2Subdevice::Whence whence)
 {
-	SimplePipelineHandler *pipe =
-		static_cast<SimplePipelineHandler *>(this->pipe());
+	SimplePipelineHandler *pipe = SimpleCameraData::pipe();
 	int ret;
 
 	/*
@@ -582,8 +585,7 @@  CameraConfiguration::Status SimpleCameraConfiguration::validate()
 	}
 
 	/* Adjust the requested streams. */
-	SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
-	SimpleConverter *converter = pipe->converter();
+	SimpleConverter *converter = data_->pipe()->converter();
 
 	/*
 	 * Enable usage of the converter when producing multiple streams, as