[libcamera-devel] [RFCv2 6/7] libcamera: pipeline: vimc: Call IPA start() and stop()

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2020-03-26 23:29:23 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Mar 26, 2020 at 05:08:18PM +0100, Niklas Söderlund wrote:
> > Call the IPA start()/stop() functions before/after the camera is
> > started. This makes sure the IPA functions properly once thread
> > management is moved into the start/stop interface.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > index b04a9726efa5ade6..c5eea3a01b0e9446 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -274,8 +274,15 @@ int PipelineHandlerVimc::start(Camera *camera)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	ret = data->ipa_->start();
> > +	if (ret) {
> > +		data->video_->releaseBuffers();
> > +		return ret;
> > +	}
> > +
> >  	ret = data->video_->streamOn();
> >  	if (ret < 0) {
> > +		data->ipa_->stop();
> >  		data->video_->releaseBuffers();
> >  		return ret;
> >  	}
> > @@ -287,6 +294,7 @@ void PipelineHandlerVimc::stop(Camera *camera)
> >  {
> >  	VimcCameraData *data = cameraData(camera);
> >  	data->video_->streamOff();
> > +	data->ipa_->stop();
> 
> Conceptually speaking I think the IPA should be stopped first (and
> started last), as I see it "depending" on the operation of the pipeline
> handler, but that makes absolutely no difference in practice here. It
> should however be consistent between the rkisp1 and vimc pipeline
> handlers.

I wrestled a bit with this when writing the IPA. As the ipa_ is stored 
in the base class CameraData shall we move the ipa_-{start,stop}() calls 
to the camera class start/stop to force them being called uniformly for 
all pipelines?

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> >  	data->video_->releaseBuffers();
> >  }
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund