[libcamera-devel,13/23] libcamera: IPAInterface: Remove all functions from IPAInterface

9623 diff mbox series

Now that all the functions in the IPA interface are defined in the data
definition file and a specialized IPAInterface is generated per pipeline
handler, remove all the functions from the case IPAInterface.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/ipa/ipa_interface.h | 16 ----------------
 1 file changed, 16 deletions(-)

Hi Paul,

Thanks for your work.

On 2020-09-15 23:20:28 +0900, Paul Elder wrote:
> Now that all the functions in the IPA interface are defined in the data
> definition file and a specialized IPAInterface is generated per pipeline
> handler, remove all the functions from the case IPAInterface.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

I'm sure a lot of documentation shall be removed as well as part of this 
change. With that addressed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/ipa/ipa_interface.h | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> index 5016ec25..cbe325ea 100644
> --- a/include/libcamera/ipa/ipa_interface.h
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -151,22 +151,6 @@ class IPAInterface
>  {
>  public:
>  	virtual ~IPAInterface() {}
> -
> -	virtual int init(const IPASettings &settings) = 0;
> -	virtual int start() = 0;
> -	virtual void stop() = 0;
> -
> -	virtual void configure(const CameraSensorInfo &sensorInfo,
> -			       const std::map<unsigned int, IPAStream> &streamConfig,
> -			       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> -			       const IPAOperationData &ipaConfig,
> -			       IPAOperationData *result) = 0;
> -
> -	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
> -	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> -
> -	virtual void processEvent(const IPAOperationData &data) = 0;
> -	Signal<unsigned int, const IPAOperationData &> queueFrameAction;
>  };
>  
>  } /* namespace libcamera */
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Niklas,

Thank you for the review.

On Sat, Sep 19, 2020 at 01:42:38PM +0200, Niklas Söderlund wrote:
> Hi Paul,
> 
> Thanks for your work.
> 
> On 2020-09-15 23:20:28 +0900, Paul Elder wrote:
> > Now that all the functions in the IPA interface are defined in the data
> > definition file and a specialized IPAInterface is generated per pipeline
> > handler, remove all the functions from the case IPAInterface.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> I'm sure a lot of documentation shall be removed as well as part of this 
> change. With that addressed,

Yes, it will. I was, however, still wondering where to move it to. I
want to keep init(), start(), and stop() as required IPA functions, but
there doesn't seem to be a place to mandate that technically, since mojo
(the language) doesn't support interface inheritance. I suppose I could
spit out an error at the code generation stage if these functions don't
exist. (Technically correctly we should intervene at the parsing stage,
but we don't want to touch mojo code since that would ruin ease of
updating)

The other functions will become pipeline-specific, so I would move the
documentation to their mojom interface definition files... which means
if a lot of pipelines use the same interface (as they do now), they'd
all duplicate the interface documentation.


Paul

> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> > ---
> >  include/libcamera/ipa/ipa_interface.h | 16 ----------------
> >  1 file changed, 16 deletions(-)
> > 
> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> > index 5016ec25..cbe325ea 100644
> > --- a/include/libcamera/ipa/ipa_interface.h
> > +++ b/include/libcamera/ipa/ipa_interface.h
> > @@ -151,22 +151,6 @@ class IPAInterface
> >  {
> >  public:
> >  	virtual ~IPAInterface() {}
> > -
> > -	virtual int init(const IPASettings &settings) = 0;
> > -	virtual int start() = 0;
> > -	virtual void stop() = 0;
> > -
> > -	virtual void configure(const CameraSensorInfo &sensorInfo,
> > -			       const std::map<unsigned int, IPAStream> &streamConfig,
> > -			       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> > -			       const IPAOperationData &ipaConfig,
> > -			       IPAOperationData *result) = 0;
> > -
> > -	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
> > -	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> > -
> > -	virtual void processEvent(const IPAOperationData &data) = 0;
> > -	Signal<unsigned int, const IPAOperationData &> queueFrameAction;
> >  };
> >  
> >  } /* namespace libcamera */
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> 
> -- 
> Regards,
> Niklas Söderlund