[libcamera-devel,v3,2/5] ipa: libipa: Add init() function to the Algorithm class
diff mbox series

Message ID 20220617092315.120781-3-fsylvestre@baylibre.com
State Superseded
Headers show
Series
  • Add tuning file support for rkisp1 blc algo
Related show

Commit Message

Florian Sylvestre June 17, 2022, 9:23 a.m. UTC 
Add the init() function that will be called during algorithm initialization
to provide each algorithm the list of algorithms tuning data.
Each algorithm will be responsible to grab their corresponding parameters.

Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/algorithm.h   |  8 +++++---
 src/ipa/libipa/algorithm.cpp          | 14 ++++++++++++++
 src/ipa/libipa/algorithm.h            | 11 +++++++++--
 src/ipa/rkisp1/algorithms/algorithm.h |  8 +++++---
 4 files changed, 33 insertions(+), 8 deletions(-)

Comments

Jean-Michel Hautbois June 17, 2022, 9:33 a.m. UTC | #1 
Hi Florian,

Thanks for the patch !

On 17/06/2022 11:23, Florian Sylvestre via libcamera-devel wrote:
> Add the init() function that will be called during algorithm initialization
> to provide each algorithm the list of algorithms tuning data.
> Each algorithm will be responsible to grab their corresponding parameters.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>   src/ipa/ipu3/algorithms/algorithm.h   |  8 +++++---
>   src/ipa/libipa/algorithm.cpp          | 14 ++++++++++++++
>   src/ipa/libipa/algorithm.h            | 11 +++++++++--
>   src/ipa/rkisp1/algorithms/algorithm.h |  8 +++++---
>   4 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
> index 234b2bd7..8b562e5b 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.h
> +++ b/src/ipa/ipu3/algorithms/algorithm.h
> @@ -15,11 +15,13 @@
>   
>   namespace libcamera {
>   
> +class YamlObject;
> +
>   namespace ipa::ipu3 {
>   
> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> -					    IPAConfigInfo, ipu3_uapi_params,
> -					    ipu3_uapi_stats_3a>;
> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
> +					    IPAFrameContext, IPAConfigInfo,
> +					    ipu3_uapi_params, ipu3_uapi_stats_3a>;
>   
>   } /* namespace ipa::ipu3 */
>   
> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> index cce2ed62..f1b04160 100644
> --- a/src/ipa/libipa/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -29,6 +29,20 @@ namespace ipa {
>    * to manage algorithms regardless of their specific type.
>    */
>   
> +/**
> + * \fn Algorithm::init()
> + * \brief Configure the Algorithm with default parameters
> + * \param[in] context The shared IPA context
> + * \param[in] params The initial parameters used to tune algorithms

This is v3, sorry for not having said it before...

I am a bit annoyed by this name, params, as it is already used for the 
parameters buffer in the prepare() call which does not contain the same 
type of objects. What about tuningParams ?

> + *
> + * This function is called once, when the IPA module is initialized, to
> + * initialize the algorithm. The \a params YamlObject contains IPA module
> + * parameters, typically tuning data for all algorithms. The Algorithm is
> + * responsible for reading the parameters relevant to its configuration.
> + *
> + * \return 0 if successful, an error code otherwise
> + */
> +
>   /**
>    * \fn Algorithm::configure()
>    * \brief Configure the Algorithm given an IPAConfigInfo
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> index 032a05b5..2945719e 100644
> --- a/src/ipa/libipa/algorithm.h
> +++ b/src/ipa/libipa/algorithm.h
> @@ -10,13 +10,20 @@ namespace libcamera {
>   
>   namespace ipa {
>   
> -template<typename Context, typename FrameContext, typename Config,
> -	 typename Params, typename Stats>
> +template<typename Context, typename TuningData, typename FrameContext,
> +	 typename Config, typename Params, typename Stats>
> +
>   class Algorithm
>   {
>   public:
>   	virtual ~Algorithm() {}
>   
> +	virtual int init([[maybe_unused]] Context &context,
> +			 [[maybe_unused]] const TuningData &params)
> +	{
> +		return 0;
> +	}
> +
>   	virtual int configure([[maybe_unused]] Context &context,
>   			      [[maybe_unused]] const Config &configInfo)
>   	{
> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> index 68e3a44e..e813756d 100644
> --- a/src/ipa/rkisp1/algorithms/algorithm.h
> +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> @@ -17,11 +17,13 @@
>   
>   namespace libcamera {
>   
> +class YamlObject;
> +
>   namespace ipa::rkisp1 {
>   
> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> -					    IPACameraSensorInfo, rkisp1_params_cfg,
> -					    rkisp1_stat_buffer>;
> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
> +					    IPAFrameContext, IPACameraSensorInfo,
> +					    rkisp1_params_cfg, rkisp1_stat_buffer>;
>   
>   } /* namespace ipa::rkisp1 */
>
Laurent Pinchart June 17, 2022, 5:29 p.m. UTC | #2 
On Fri, Jun 17, 2022 at 11:33:21AM +0200, Jean-Michel Hautbois via libcamera-devel wrote:
> Hi Florian,
> 
> Thanks for the patch !
> 
> On 17/06/2022 11:23, Florian Sylvestre via libcamera-devel wrote:
> > Add the init() function that will be called during algorithm initialization
> > to provide each algorithm the list of algorithms tuning data.
> > Each algorithm will be responsible to grab their corresponding parameters.
> > 
> > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >   src/ipa/ipu3/algorithms/algorithm.h   |  8 +++++---
> >   src/ipa/libipa/algorithm.cpp          | 14 ++++++++++++++
> >   src/ipa/libipa/algorithm.h            | 11 +++++++++--
> >   src/ipa/rkisp1/algorithms/algorithm.h |  8 +++++---
> >   4 files changed, 33 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
> > index 234b2bd7..8b562e5b 100644
> > --- a/src/ipa/ipu3/algorithms/algorithm.h
> > +++ b/src/ipa/ipu3/algorithms/algorithm.h
> > @@ -15,11 +15,13 @@
> >   
> >   namespace libcamera {
> >   
> > +class YamlObject;
> > +
> >   namespace ipa::ipu3 {
> >   
> > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> > -					    IPAConfigInfo, ipu3_uapi_params,
> > -					    ipu3_uapi_stats_3a>;
> > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
> > +					    IPAFrameContext, IPAConfigInfo,
> > +					    ipu3_uapi_params, ipu3_uapi_stats_3a>;
> >   
> >   } /* namespace ipa::ipu3 */
> >   
> > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> > index cce2ed62..f1b04160 100644
> > --- a/src/ipa/libipa/algorithm.cpp
> > +++ b/src/ipa/libipa/algorithm.cpp
> > @@ -29,6 +29,20 @@ namespace ipa {
> >    * to manage algorithms regardless of their specific type.
> >    */
> >   
> > +/**
> > + * \fn Algorithm::init()
> > + * \brief Configure the Algorithm with default parameters
> > + * \param[in] context The shared IPA context
> > + * \param[in] params The initial parameters used to tune algorithms
> 
> This is v3, sorry for not having said it before...
> 
> I am a bit annoyed by this name, params, as it is already used for the 
> parameters buffer in the prepare() call which does not contain the same 
> type of objects. What about tuningParams ?

Or, given that the type name is TuningData, maybe tuningData for the
parameter name ?

> > + *
> > + * This function is called once, when the IPA module is initialized, to
> > + * initialize the algorithm. The \a params YamlObject contains IPA module
> > + * parameters, typically tuning data for all algorithms. The Algorithm is
> > + * responsible for reading the parameters relevant to its configuration.
> > + *
> > + * \return 0 if successful, an error code otherwise
> > + */
> > +
> >   /**
> >    * \fn Algorithm::configure()
> >    * \brief Configure the Algorithm given an IPAConfigInfo
> > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> > index 032a05b5..2945719e 100644
> > --- a/src/ipa/libipa/algorithm.h
> > +++ b/src/ipa/libipa/algorithm.h
> > @@ -10,13 +10,20 @@ namespace libcamera {
> >   
> >   namespace ipa {
> >   
> > -template<typename Context, typename FrameContext, typename Config,
> > -	 typename Params, typename Stats>
> > +template<typename Context, typename TuningData, typename FrameContext,
> > +	 typename Config, typename Params, typename Stats>
> > +
> >   class Algorithm
> >   {
> >   public:
> >   	virtual ~Algorithm() {}
> >   
> > +	virtual int init([[maybe_unused]] Context &context,
> > +			 [[maybe_unused]] const TuningData &params)
> > +	{
> > +		return 0;
> > +	}
> > +
> >   	virtual int configure([[maybe_unused]] Context &context,
> >   			      [[maybe_unused]] const Config &configInfo)
> >   	{
> > diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> > index 68e3a44e..e813756d 100644
> > --- a/src/ipa/rkisp1/algorithms/algorithm.h
> > +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> > @@ -17,11 +17,13 @@
> >   
> >   namespace libcamera {
> >   
> > +class YamlObject;
> > +
> >   namespace ipa::rkisp1 {
> >   
> > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> > -					    IPACameraSensorInfo, rkisp1_params_cfg,
> > -					    rkisp1_stat_buffer>;
> > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
> > +					    IPAFrameContext, IPACameraSensorInfo,
> > +					    rkisp1_params_cfg, rkisp1_stat_buffer>;
> >   
> >   } /* namespace ipa::rkisp1 */
> >

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
index 234b2bd7..8b562e5b 100644
--- a/src/ipa/ipu3/algorithms/algorithm.h
+++ b/src/ipa/ipu3/algorithms/algorithm.h
@@ -15,11 +15,13 @@ 
 
 namespace libcamera {
 
+class YamlObject;
+
 namespace ipa::ipu3 {
 
-using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
-					    IPAConfigInfo, ipu3_uapi_params,
-					    ipu3_uapi_stats_3a>;
+using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
+					    IPAFrameContext, IPAConfigInfo,
+					    ipu3_uapi_params, ipu3_uapi_stats_3a>;
 
 } /* namespace ipa::ipu3 */
 
diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
index cce2ed62..f1b04160 100644
--- a/src/ipa/libipa/algorithm.cpp
+++ b/src/ipa/libipa/algorithm.cpp
@@ -29,6 +29,20 @@  namespace ipa {
  * to manage algorithms regardless of their specific type.
  */
 
+/**
+ * \fn Algorithm::init()
+ * \brief Configure the Algorithm with default parameters
+ * \param[in] context The shared IPA context
+ * \param[in] params The initial parameters used to tune algorithms
+ *
+ * This function is called once, when the IPA module is initialized, to
+ * initialize the algorithm. The \a params YamlObject contains IPA module
+ * parameters, typically tuning data for all algorithms. The Algorithm is
+ * responsible for reading the parameters relevant to its configuration.
+ *
+ * \return 0 if successful, an error code otherwise
+ */
+
 /**
  * \fn Algorithm::configure()
  * \brief Configure the Algorithm given an IPAConfigInfo
diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
index 032a05b5..2945719e 100644
--- a/src/ipa/libipa/algorithm.h
+++ b/src/ipa/libipa/algorithm.h
@@ -10,13 +10,20 @@  namespace libcamera {
 
 namespace ipa {
 
-template<typename Context, typename FrameContext, typename Config,
-	 typename Params, typename Stats>
+template<typename Context, typename TuningData, typename FrameContext,
+	 typename Config, typename Params, typename Stats>
+
 class Algorithm
 {
 public:
 	virtual ~Algorithm() {}
 
+	virtual int init([[maybe_unused]] Context &context,
+			 [[maybe_unused]] const TuningData &params)
+	{
+		return 0;
+	}
+
 	virtual int configure([[maybe_unused]] Context &context,
 			      [[maybe_unused]] const Config &configInfo)
 	{
diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
index 68e3a44e..e813756d 100644
--- a/src/ipa/rkisp1/algorithms/algorithm.h
+++ b/src/ipa/rkisp1/algorithms/algorithm.h
@@ -17,11 +17,13 @@ 
 
 namespace libcamera {
 
+class YamlObject;
+
 namespace ipa::rkisp1 {
 
-using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
-					    IPACameraSensorInfo, rkisp1_params_cfg,
-					    rkisp1_stat_buffer>;
+using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject,
+					    IPAFrameContext, IPACameraSensorInfo,
+					    rkisp1_params_cfg, rkisp1_stat_buffer>;
 
 } /* namespace ipa::rkisp1 */