| Message ID | 20211123091451.67404-7-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Tue, Nov 23, 2021 at 10:14:46AM +0100, Jean-Michel Hautbois wrote: > The algorithms are using the same function names with specialized > parameters. Instead of duplicating code, introduce a libipa Algorithm > class which implements a base class with template parameters in libipa, > and use it in each IPA. > > As we now won't need an algorithm class for each IPA, move the > documentation to libipa, and make it agnostic of the IPA used. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/algorithm.h | 12 ++---- > src/ipa/ipu3/algorithms/meson.build | 1 - > .../{ipu3/algorithms => libipa}/algorithm.cpp | 35 ++++++---------- > src/ipa/libipa/algorithm.h | 41 +++++++++++++++++++ > src/ipa/libipa/meson.build | 1 + > src/ipa/rkisp1/algorithms/algorithm.h | 28 +++++++++++++ > src/ipa/rkisp1/algorithms/meson.build | 4 ++ > src/ipa/rkisp1/meson.build | 4 ++ > src/ipa/rkisp1/rkisp1.cpp | 5 ++- Shouldn't this have been split in two, with the introduction of src/ipa/libipa/algorithm.h and porting of IPU3 first, and then the rkisp1 additions ? > 9 files changed, 97 insertions(+), 34 deletions(-) > rename src/ipa/{ipu3/algorithms => libipa}/algorithm.cpp (74%) > create mode 100644 src/ipa/libipa/algorithm.h > create mode 100644 src/ipa/rkisp1/algorithms/algorithm.h > create mode 100644 src/ipa/rkisp1/algorithms/meson.build > > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h > index 43f5d8b0..3c0ce461 100644 > --- a/src/ipa/ipu3/algorithms/algorithm.h > +++ b/src/ipa/ipu3/algorithms/algorithm.h > @@ -9,21 +9,15 @@ > > #include <libcamera/ipa/ipu3_ipa_interface.h> > > +#include <libipa/algorithm.h> > + > #include "ipa_context.h" > > namespace libcamera { > > namespace ipa::ipu3 { > > -class Algorithm > -{ > -public: > - virtual ~Algorithm() {} > - > - virtual int configure(IPAContext &context, const IPAConfigInfo &configInfo); > - virtual void prepare(IPAContext &context, ipu3_uapi_params *params); > - virtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats); > -}; > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; > > } /* namespace ipa::ipu3 */ > > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build > index 3ec42f72..4db6ae1d 100644 > --- a/src/ipa/ipu3/algorithms/meson.build > +++ b/src/ipa/ipu3/algorithms/meson.build > @@ -2,7 +2,6 @@ > > ipu3_ipa_algorithms = files([ > 'agc.cpp', > - 'algorithm.cpp', > 'awb.cpp', > 'blc.cpp', > 'tone_mapping.cpp', > diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > similarity index 74% > rename from src/ipa/ipu3/algorithms/algorithm.cpp > rename to src/ipa/libipa/algorithm.cpp > index 3e7e3018..888ffb57 100644 > --- a/src/ipa/ipu3/algorithms/algorithm.cpp > +++ b/src/ipa/libipa/algorithm.cpp > @@ -2,7 +2,7 @@ > /* > * Copyright (C) 2021, Ideas On Board > * > - * algorithm.cpp - IPU3 control algorithm interface > + * algorithm.cpp - IPA control algorithm interface > */ > > #include "algorithm.h" > @@ -14,11 +14,11 @@ > > namespace libcamera { > > -namespace ipa::ipu3 { > +namespace ipa { > > /** > * \class Algorithm > - * \brief The base class for all IPU3 algorithms > + * \brief The base class for all IPA algorithms The template parameters need to be documented with \tparam. > * > * The Algorithm class defines a standard interface for IPA algorithms. By > * abstracting algorithms, it makes possible the implementation of generic code > @@ -26,6 +26,7 @@ namespace ipa::ipu3 { > */ > > /** > + * \fn Algorithm::configure() > * \brief Configure the Algorithm given an IPAConfigInfo > * \param[in] context The shared IPA context > * \param[in] configInfo The IPA configuration data, received from the pipeline > @@ -39,37 +40,29 @@ namespace ipa::ipu3 { > * > * \return 0 if successful, an error code otherwise > */ > -int Algorithm::configure([[maybe_unused]] IPAContext &context, > - [[maybe_unused]] const IPAConfigInfo &configInfo) > -{ > - return 0; > -} > > /** > + * \fn Algorithm::prepare() > * \brief Fill the \a params buffer with ISP processing parameters for a frame > * \param[in] context The shared IPA context > - * \param[out] params The IPU3 specific parameters. > + * \param[out] params The ISP specific parameters. > * > * This function is called for every frame when the camera is running before it > - * is processed by the ImgU to prepare the ImgU processing parameters for that > - * frame. > + * is processed by the ISP. * is processed by the ISP to prepare the ISP processing parameters for that * frame. > * > * Algorithms shall fill in the parameter structure fields appropriately to > - * configure the ImgU processing blocks that they are responsible for. This > + * configure the ISP processing blocks that they are responsible for. This > * includes setting fields and flags that enable those processing blocks. > */ > -void Algorithm::prepare([[maybe_unused]] IPAContext &context, > - [[maybe_unused]] ipu3_uapi_params *params) > -{ > -} > > /** > + * \fn Algorithm::process() > * \brief Process ISP statistics, and run algorithm operations > * \param[in] context The shared IPA context > - * \param[in] stats The IPU3 statistics and ISP results > + * \param[in] stats The IPA statistics and ISP results > * > * This function is called while camera is running for every frame processed by > - * the ImgU, to process statistics generated from that frame by the ImgU. > + * the ISP, to process statistics generated from that frame by the ISP. > * Algorithms shall use this data to run calculations and update their state > * accordingly. > * > @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context, > * Care shall be taken to ensure the ordering of access to the information > * such that the algorithms use up to date state as required. > */ > -void Algorithm::process([[maybe_unused]] IPAContext &context, > - [[maybe_unused]] const ipu3_uapi_stats_3a *stats) > -{ > -} > > -} /* namespace ipa::ipu3 */ > +} /* namespace ipa */ > > } /* namespace libcamera */ > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h > new file mode 100644 > index 00000000..74dc49c4 > --- /dev/null > +++ b/src/ipa/libipa/algorithm.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Ideas On Board > + * > + * algorithm.h - ISP control algorithm interface > + */ > +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ > +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ > + > +namespace libcamera { > + > +namespace ipa { > + > +template<typename Context, typename IPAConfigInfo, typename Params, typename Stats> s/IPAConfigInfo/Config/ > +class Algorithm > +{ > +public: > + virtual ~Algorithm() {} > + > + virtual int configure([[maybe_unused]] Context &context, > + [[maybe_unused]] const IPAConfigInfo &configInfo) > + { > + return 0; > + } > + > + virtual void prepare([[maybe_unused]] Context &context, > + [[maybe_unused]] Params *params) > + { > + } > + > + virtual void process([[maybe_unused]] Context &context, > + [[maybe_unused]] const Stats *stats) > + { > + } > +}; > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > + > +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */ > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > index 4d073a03..161cc5a1 100644 > --- a/src/ipa/libipa/meson.build > +++ b/src/ipa/libipa/meson.build > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: CC0-1.0 > > libipa_headers = files([ > + 'algorithm.h', > 'camera_sensor_helper.h', > 'histogram.h' > ]) > diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h > new file mode 100644 > index 00000000..dfa58727 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/algorithm.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Ideas On Board > + * > + * algorithm.h - RkISP1 control algorithm interface > + */ > +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ > +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ > + > +#include <linux/rkisp1-config.h> > + > +#include <libcamera/ipa/rkisp1_ipa_interface.h> > + > +#include <libipa/algorithm.h> > + > +#include "ipa_context.h" > + > +namespace libcamera { > + > +namespace ipa::rkisp1 { > + > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; > + > +} /* namespace ipa::rkisp1 */ > + > +} /* namespace libcamera */ > + > +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */ > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > new file mode 100644 > index 00000000..1c6c59cf > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/meson.build > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +rkisp1_ipa_algorithms = files([ > +]) > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build > index 3683c922..8c822fbb 100644 > --- a/src/ipa/rkisp1/meson.build > +++ b/src/ipa/rkisp1/meson.build > @@ -1,5 +1,7 @@ > # SPDX-License-Identifier: CC0-1.0 > > +subdir('algorithms') > + > ipa_name = 'ipa_rkisp1' > > rkisp1_ipa_sources = files([ > @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([ > 'rkisp1.cpp', > ]) > > +rkisp1_ipa_sources += rkisp1_ipa_algorithms > + > mod = shared_module(ipa_name, > [rkisp1_ipa_sources, libcamera_generated_ipa_headers], > name_prefix : '', > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 34c3f9a2..0c54d8ec 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -25,7 +25,7 @@ > > #include <libcamera/internal/mapped_framebuffer.h> > > -#include "ipa_context.h" > +#include "algorithms/algorithm.h" > #include "libipa/camera_sensor_helper.h" > > namespace libcamera { > @@ -82,6 +82,9 @@ private: > > /* Local parameter storage */ > struct IPAContext context_; > + > + /* Maintain the algorithms used by the IPA */ > + std::list<std::unique_ptr<ipa::rkisp1::Algorithm>> algorithms_; > }; > > int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
Hi Laurent, On 23/11/2021 11:02, Laurent Pinchart wrote: > Hi Jean-Michel, > > Thank you for the patch. > > On Tue, Nov 23, 2021 at 10:14:46AM +0100, Jean-Michel Hautbois wrote: >> The algorithms are using the same function names with specialized >> parameters. Instead of duplicating code, introduce a libipa Algorithm >> class which implements a base class with template parameters in libipa, >> and use it in each IPA. >> >> As we now won't need an algorithm class for each IPA, move the >> documentation to libipa, and make it agnostic of the IPA used. >> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/algorithm.h | 12 ++---- >> src/ipa/ipu3/algorithms/meson.build | 1 - >> .../{ipu3/algorithms => libipa}/algorithm.cpp | 35 ++++++---------- >> src/ipa/libipa/algorithm.h | 41 +++++++++++++++++++ >> src/ipa/libipa/meson.build | 1 + >> src/ipa/rkisp1/algorithms/algorithm.h | 28 +++++++++++++ >> src/ipa/rkisp1/algorithms/meson.build | 4 ++ >> src/ipa/rkisp1/meson.build | 4 ++ >> src/ipa/rkisp1/rkisp1.cpp | 5 ++- > > Shouldn't this have been split in two, with the introduction of > src/ipa/libipa/algorithm.h and porting of IPU3 first, and then the > rkisp1 additions ? OK, I will split it in two :-). > >> 9 files changed, 97 insertions(+), 34 deletions(-) >> rename src/ipa/{ipu3/algorithms => libipa}/algorithm.cpp (74%) >> create mode 100644 src/ipa/libipa/algorithm.h >> create mode 100644 src/ipa/rkisp1/algorithms/algorithm.h >> create mode 100644 src/ipa/rkisp1/algorithms/meson.build >> >> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h >> index 43f5d8b0..3c0ce461 100644 >> --- a/src/ipa/ipu3/algorithms/algorithm.h >> +++ b/src/ipa/ipu3/algorithms/algorithm.h >> @@ -9,21 +9,15 @@ >> >> #include <libcamera/ipa/ipu3_ipa_interface.h> >> >> +#include <libipa/algorithm.h> >> + >> #include "ipa_context.h" >> >> namespace libcamera { >> >> namespace ipa::ipu3 { >> >> -class Algorithm >> -{ >> -public: >> - virtual ~Algorithm() {} >> - >> - virtual int configure(IPAContext &context, const IPAConfigInfo &configInfo); >> - virtual void prepare(IPAContext &context, ipu3_uapi_params *params); >> - virtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats); >> -}; >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; >> >> } /* namespace ipa::ipu3 */ >> >> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build >> index 3ec42f72..4db6ae1d 100644 >> --- a/src/ipa/ipu3/algorithms/meson.build >> +++ b/src/ipa/ipu3/algorithms/meson.build >> @@ -2,7 +2,6 @@ >> >> ipu3_ipa_algorithms = files([ >> 'agc.cpp', >> - 'algorithm.cpp', >> 'awb.cpp', >> 'blc.cpp', >> 'tone_mapping.cpp', >> diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp >> similarity index 74% >> rename from src/ipa/ipu3/algorithms/algorithm.cpp >> rename to src/ipa/libipa/algorithm.cpp >> index 3e7e3018..888ffb57 100644 >> --- a/src/ipa/ipu3/algorithms/algorithm.cpp >> +++ b/src/ipa/libipa/algorithm.cpp >> @@ -2,7 +2,7 @@ >> /* >> * Copyright (C) 2021, Ideas On Board >> * >> - * algorithm.cpp - IPU3 control algorithm interface >> + * algorithm.cpp - IPA control algorithm interface >> */ >> >> #include "algorithm.h" >> @@ -14,11 +14,11 @@ >> >> namespace libcamera { >> >> -namespace ipa::ipu3 { >> +namespace ipa { >> >> /** >> * \class Algorithm >> - * \brief The base class for all IPU3 algorithms >> + * \brief The base class for all IPA algorithms > > The template parameters need to be documented with \tparam. > I haven't been warned by Doxygen, I will change that (did not know there is a specific template parameters keyword ;-)). >> * >> * The Algorithm class defines a standard interface for IPA algorithms. By >> * abstracting algorithms, it makes possible the implementation of generic code >> @@ -26,6 +26,7 @@ namespace ipa::ipu3 { >> */ >> >> /** >> + * \fn Algorithm::configure() >> * \brief Configure the Algorithm given an IPAConfigInfo >> * \param[in] context The shared IPA context >> * \param[in] configInfo The IPA configuration data, received from the pipeline >> @@ -39,37 +40,29 @@ namespace ipa::ipu3 { >> * >> * \return 0 if successful, an error code otherwise >> */ >> -int Algorithm::configure([[maybe_unused]] IPAContext &context, >> - [[maybe_unused]] const IPAConfigInfo &configInfo) >> -{ >> - return 0; >> -} >> >> /** >> + * \fn Algorithm::prepare() >> * \brief Fill the \a params buffer with ISP processing parameters for a frame >> * \param[in] context The shared IPA context >> - * \param[out] params The IPU3 specific parameters. >> + * \param[out] params The ISP specific parameters. >> * >> * This function is called for every frame when the camera is running before it >> - * is processed by the ImgU to prepare the ImgU processing parameters for that >> - * frame. >> + * is processed by the ISP. > > * is processed by the ISP to prepare the ISP processing parameters for that > * frame. > >> * >> * Algorithms shall fill in the parameter structure fields appropriately to >> - * configure the ImgU processing blocks that they are responsible for. This >> + * configure the ISP processing blocks that they are responsible for. This >> * includes setting fields and flags that enable those processing blocks. >> */ >> -void Algorithm::prepare([[maybe_unused]] IPAContext &context, >> - [[maybe_unused]] ipu3_uapi_params *params) >> -{ >> -} >> >> /** >> + * \fn Algorithm::process() >> * \brief Process ISP statistics, and run algorithm operations >> * \param[in] context The shared IPA context >> - * \param[in] stats The IPU3 statistics and ISP results >> + * \param[in] stats The IPA statistics and ISP results >> * >> * This function is called while camera is running for every frame processed by >> - * the ImgU, to process statistics generated from that frame by the ImgU. >> + * the ISP, to process statistics generated from that frame by the ISP. >> * Algorithms shall use this data to run calculations and update their state >> * accordingly. >> * >> @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context, >> * Care shall be taken to ensure the ordering of access to the information >> * such that the algorithms use up to date state as required. >> */ >> -void Algorithm::process([[maybe_unused]] IPAContext &context, >> - [[maybe_unused]] const ipu3_uapi_stats_3a *stats) >> -{ >> -} >> >> -} /* namespace ipa::ipu3 */ >> +} /* namespace ipa */ >> >> } /* namespace libcamera */ >> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h >> new file mode 100644 >> index 00000000..74dc49c4 >> --- /dev/null >> +++ b/src/ipa/libipa/algorithm.h >> @@ -0,0 +1,41 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> +/* >> + * Copyright (C) 2021, Ideas On Board >> + * >> + * algorithm.h - ISP control algorithm interface >> + */ >> +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ >> +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ >> + >> +namespace libcamera { >> + >> +namespace ipa { >> + >> +template<typename Context, typename IPAConfigInfo, typename Params, typename Stats> > > s/IPAConfigInfo/Config/ > >> +class Algorithm >> +{ >> +public: >> + virtual ~Algorithm() {} >> + >> + virtual int configure([[maybe_unused]] Context &context, >> + [[maybe_unused]] const IPAConfigInfo &configInfo) >> + { >> + return 0; >> + } >> + >> + virtual void prepare([[maybe_unused]] Context &context, >> + [[maybe_unused]] Params *params) >> + { >> + } >> + >> + virtual void process([[maybe_unused]] Context &context, >> + [[maybe_unused]] const Stats *stats) >> + { >> + } >> +}; >> + >> +} /* namespace ipa */ >> + >> +} /* namespace libcamera */ >> + >> +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */ >> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build >> index 4d073a03..161cc5a1 100644 >> --- a/src/ipa/libipa/meson.build >> +++ b/src/ipa/libipa/meson.build >> @@ -1,6 +1,7 @@ >> # SPDX-License-Identifier: CC0-1.0 >> >> libipa_headers = files([ >> + 'algorithm.h', >> 'camera_sensor_helper.h', >> 'histogram.h' >> ]) >> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h >> new file mode 100644 >> index 00000000..dfa58727 >> --- /dev/null >> +++ b/src/ipa/rkisp1/algorithms/algorithm.h >> @@ -0,0 +1,28 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> +/* >> + * Copyright (C) 2021, Ideas On Board >> + * >> + * algorithm.h - RkISP1 control algorithm interface >> + */ >> +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ >> +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ >> + >> +#include <linux/rkisp1-config.h> >> + >> +#include <libcamera/ipa/rkisp1_ipa_interface.h> >> + >> +#include <libipa/algorithm.h> >> + >> +#include "ipa_context.h" >> + >> +namespace libcamera { >> + >> +namespace ipa::rkisp1 { >> + >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; >> + >> +} /* namespace ipa::rkisp1 */ >> + >> +} /* namespace libcamera */ >> + >> +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */ >> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build >> new file mode 100644 >> index 00000000..1c6c59cf >> --- /dev/null >> +++ b/src/ipa/rkisp1/algorithms/meson.build >> @@ -0,0 +1,4 @@ >> +# SPDX-License-Identifier: CC0-1.0 >> + >> +rkisp1_ipa_algorithms = files([ >> +]) >> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build >> index 3683c922..8c822fbb 100644 >> --- a/src/ipa/rkisp1/meson.build >> +++ b/src/ipa/rkisp1/meson.build >> @@ -1,5 +1,7 @@ >> # SPDX-License-Identifier: CC0-1.0 >> >> +subdir('algorithms') >> + >> ipa_name = 'ipa_rkisp1' >> >> rkisp1_ipa_sources = files([ >> @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([ >> 'rkisp1.cpp', >> ]) >> >> +rkisp1_ipa_sources += rkisp1_ipa_algorithms >> + >> mod = shared_module(ipa_name, >> [rkisp1_ipa_sources, libcamera_generated_ipa_headers], >> name_prefix : '', >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp >> index 34c3f9a2..0c54d8ec 100644 >> --- a/src/ipa/rkisp1/rkisp1.cpp >> +++ b/src/ipa/rkisp1/rkisp1.cpp >> @@ -25,7 +25,7 @@ >> >> #include <libcamera/internal/mapped_framebuffer.h> >> >> -#include "ipa_context.h" >> +#include "algorithms/algorithm.h" >> #include "libipa/camera_sensor_helper.h" >> >> namespace libcamera { >> @@ -82,6 +82,9 @@ private: >> >> /* Local parameter storage */ >> struct IPAContext context_; >> + >> + /* Maintain the algorithms used by the IPA */ >> + std::list<std::unique_ptr<ipa::rkisp1::Algorithm>> algorithms_; >> }; >> >> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision) >
diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h index 43f5d8b0..3c0ce461 100644 --- a/src/ipa/ipu3/algorithms/algorithm.h +++ b/src/ipa/ipu3/algorithms/algorithm.h @@ -9,21 +9,15 @@ #include <libcamera/ipa/ipu3_ipa_interface.h> +#include <libipa/algorithm.h> + #include "ipa_context.h" namespace libcamera { namespace ipa::ipu3 { -class Algorithm -{ -public: - virtual ~Algorithm() {} - - virtual int configure(IPAContext &context, const IPAConfigInfo &configInfo); - virtual void prepare(IPAContext &context, ipu3_uapi_params *params); - virtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats); -}; +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build index 3ec42f72..4db6ae1d 100644 --- a/src/ipa/ipu3/algorithms/meson.build +++ b/src/ipa/ipu3/algorithms/meson.build @@ -2,7 +2,6 @@ ipu3_ipa_algorithms = files([ 'agc.cpp', - 'algorithm.cpp', 'awb.cpp', 'blc.cpp', 'tone_mapping.cpp', diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp similarity index 74% rename from src/ipa/ipu3/algorithms/algorithm.cpp rename to src/ipa/libipa/algorithm.cpp index 3e7e3018..888ffb57 100644 --- a/src/ipa/ipu3/algorithms/algorithm.cpp +++ b/src/ipa/libipa/algorithm.cpp @@ -2,7 +2,7 @@ /* * Copyright (C) 2021, Ideas On Board * - * algorithm.cpp - IPU3 control algorithm interface + * algorithm.cpp - IPA control algorithm interface */ #include "algorithm.h" @@ -14,11 +14,11 @@ namespace libcamera { -namespace ipa::ipu3 { +namespace ipa { /** * \class Algorithm - * \brief The base class for all IPU3 algorithms + * \brief The base class for all IPA algorithms * * The Algorithm class defines a standard interface for IPA algorithms. By * abstracting algorithms, it makes possible the implementation of generic code @@ -26,6 +26,7 @@ namespace ipa::ipu3 { */ /** + * \fn Algorithm::configure() * \brief Configure the Algorithm given an IPAConfigInfo * \param[in] context The shared IPA context * \param[in] configInfo The IPA configuration data, received from the pipeline @@ -39,37 +40,29 @@ namespace ipa::ipu3 { * * \return 0 if successful, an error code otherwise */ -int Algorithm::configure([[maybe_unused]] IPAContext &context, - [[maybe_unused]] const IPAConfigInfo &configInfo) -{ - return 0; -} /** + * \fn Algorithm::prepare() * \brief Fill the \a params buffer with ISP processing parameters for a frame * \param[in] context The shared IPA context - * \param[out] params The IPU3 specific parameters. + * \param[out] params The ISP specific parameters. * * This function is called for every frame when the camera is running before it - * is processed by the ImgU to prepare the ImgU processing parameters for that - * frame. + * is processed by the ISP. * * Algorithms shall fill in the parameter structure fields appropriately to - * configure the ImgU processing blocks that they are responsible for. This + * configure the ISP processing blocks that they are responsible for. This * includes setting fields and flags that enable those processing blocks. */ -void Algorithm::prepare([[maybe_unused]] IPAContext &context, - [[maybe_unused]] ipu3_uapi_params *params) -{ -} /** + * \fn Algorithm::process() * \brief Process ISP statistics, and run algorithm operations * \param[in] context The shared IPA context - * \param[in] stats The IPU3 statistics and ISP results + * \param[in] stats The IPA statistics and ISP results * * This function is called while camera is running for every frame processed by - * the ImgU, to process statistics generated from that frame by the ImgU. + * the ISP, to process statistics generated from that frame by the ISP. * Algorithms shall use this data to run calculations and update their state * accordingly. * @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context, * Care shall be taken to ensure the ordering of access to the information * such that the algorithms use up to date state as required. */ -void Algorithm::process([[maybe_unused]] IPAContext &context, - [[maybe_unused]] const ipu3_uapi_stats_3a *stats) -{ -} -} /* namespace ipa::ipu3 */ +} /* namespace ipa */ } /* namespace libcamera */ diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h new file mode 100644 index 00000000..74dc49c4 --- /dev/null +++ b/src/ipa/libipa/algorithm.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Ideas On Board + * + * algorithm.h - ISP control algorithm interface + */ +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ + +namespace libcamera { + +namespace ipa { + +template<typename Context, typename IPAConfigInfo, typename Params, typename Stats> +class Algorithm +{ +public: + virtual ~Algorithm() {} + + virtual int configure([[maybe_unused]] Context &context, + [[maybe_unused]] const IPAConfigInfo &configInfo) + { + return 0; + } + + virtual void prepare([[maybe_unused]] Context &context, + [[maybe_unused]] Params *params) + { + } + + virtual void process([[maybe_unused]] Context &context, + [[maybe_unused]] const Stats *stats) + { + } +}; + +} /* namespace ipa */ + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */ diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 4d073a03..161cc5a1 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -1,6 +1,7 @@ # SPDX-License-Identifier: CC0-1.0 libipa_headers = files([ + 'algorithm.h', 'camera_sensor_helper.h', 'histogram.h' ]) diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h new file mode 100644 index 00000000..dfa58727 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/algorithm.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Ideas On Board + * + * algorithm.h - RkISP1 control algorithm interface + */ +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ + +#include <linux/rkisp1-config.h> + +#include <libcamera/ipa/rkisp1_ipa_interface.h> + +#include <libipa/algorithm.h> + +#include "ipa_context.h" + +namespace libcamera { + +namespace ipa::rkisp1 { + +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; + +} /* namespace ipa::rkisp1 */ + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */ diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build new file mode 100644 index 00000000..1c6c59cf --- /dev/null +++ b/src/ipa/rkisp1/algorithms/meson.build @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: CC0-1.0 + +rkisp1_ipa_algorithms = files([ +]) diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build index 3683c922..8c822fbb 100644 --- a/src/ipa/rkisp1/meson.build +++ b/src/ipa/rkisp1/meson.build @@ -1,5 +1,7 @@ # SPDX-License-Identifier: CC0-1.0 +subdir('algorithms') + ipa_name = 'ipa_rkisp1' rkisp1_ipa_sources = files([ @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([ 'rkisp1.cpp', ]) +rkisp1_ipa_sources += rkisp1_ipa_algorithms + mod = shared_module(ipa_name, [rkisp1_ipa_sources, libcamera_generated_ipa_headers], name_prefix : '', diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 34c3f9a2..0c54d8ec 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -25,7 +25,7 @@ #include <libcamera/internal/mapped_framebuffer.h> -#include "ipa_context.h" +#include "algorithms/algorithm.h" #include "libipa/camera_sensor_helper.h" namespace libcamera { @@ -82,6 +82,9 @@ private: /* Local parameter storage */ struct IPAContext context_; + + /* Maintain the algorithms used by the IPA */ + std::list<std::unique_ptr<ipa::rkisp1::Algorithm>> algorithms_; }; int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)