[libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store IPA in pipeline data

[Paul Elder]

Hi Laurent,

On Tue, Jun 04, 2019 at 03:07:44PM +0300, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.

Thank you for the review.

> On Mon, Jun 03, 2019 at 07:16:36PM -0400, Paul Elder wrote:
> > After the pipeline handler acquires an IPA, it should save it for future
> > use. Store it in the pipeline data.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> > New patch
> > 
> >  src/libcamera/include/pipeline_handler.h | 3 +++
> >  src/libcamera/pipeline_handler.cpp       | 8 ++++++++
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index 67971b3..a2d9624 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -14,6 +14,7 @@
> >  #include <string>
> >  #include <vector>
> >  
> > +#include <libcamera/ipa_interface.h>
> >  #include <libcamera/stream.h>
> >  
> >  namespace libcamera {
> > @@ -93,6 +94,8 @@ protected:
> >  	const char *name_;
> >  	uint32_t version_;
> >  
> > +	std::unique_ptr<IPAInterface> ipa_;
> > +
> >  private:
> >  	void mediaDeviceDisconnected(MediaDevice *media);
> >  	virtual void disconnect();
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index b18f14d..d6e68b0 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -530,6 +530,14 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)
> >   * \brief Pipeline handler version
> >   */
> >  
> > +/**
> > + * \var PipelineHandler::ipa_
> > + * \brief The IPA that the pipeline handler will use
> 
> "The IPA interface instance for this pipeline handler" ?
> 
> And thinking about it, will we always have a 1:1 mapping between
> pipeline handlers and IPA instances ? In the IPU3 case, if we support

tbh, I was doubting a 1:1 mapping between pipeline handlers and IPA
instances.

> capturing from two cameras simultaneously, I think we'll have one IPA
> instance per camera, while both cameras will be handled by the same
> pipeline handler. We could store the IPA instance in the camera data
> instead. Or, for now, until we figure the details out, store it in the
> vimc pipeline handler in patch 10/10 (thus dropping this patch). What do
> you think ?

If the mapping is 1:1 between cameras and IPA instances then I think we
can put the IPA instance in the camera data.

> > + *
> > + * The pipeline handler can acquire an IPA from the IPAManager. Once it is
> > + * acquired, it will be stored here.
> > + */
> > +
> >  /**
> >   * \class PipelineHandlerFactory
> >   * \brief Registration of PipelineHandler classes and creation of instances

Thanks,

Paul