[libcamera-devel] [PATCH v4 8/9] ipa: rkisp1: Add enable field for LSC algorithm in IPA context

[Laurent Pinchart]

Hi Paul,

On Thu, Aug 18, 2022 at 10:18:03PM +0900, paul.elder at ideasonboard.com wrote:
> On Tue, Aug 16, 2022 at 04:54:13AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> > 
> > Add an enable variable in the lsc struct in IPASessionConfiguration which
> > indicates if the lsc algorithm has been configured. This will allow other
> > algorithms to retrieve this information.
> > 
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Improve documentation
> > ---
> >  src/ipa/rkisp1/algorithms/lsc.cpp | 10 ++++++++++
> >  src/ipa/rkisp1/algorithms/lsc.h   |  1 +
> >  src/ipa/rkisp1/ipa_context.cpp    |  8 ++++++++
> >  src/ipa/rkisp1/ipa_context.h      |  4 ++++
> >  4 files changed, 23 insertions(+)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > index 05c8c0dab5c8..da287ac7af75 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > @@ -119,6 +119,16 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
> >  	return 0;
> >  }
> >  
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::configure
> > + */
> > +int LensShadingCorrection::configure(IPAContext &context,
> > +				     [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> > +{
> > +	context.configuration.lsc.enabled = initialized_;
> > +	return 0;
> > +}
> > +
> >  /**
> >   * \copydoc libcamera::ipa::Algorithm::prepare
> >   */
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> > index fdb2ec1dd27d..f68602c005c4 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.h
> > +++ b/src/ipa/rkisp1/algorithms/lsc.h
> > @@ -20,6 +20,7 @@ public:
> >  	~LensShadingCorrection() = default;
> >  
> >  	int init(IPAContext &context, const YamlObject &tuningData) override;
> > +	int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> >  	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> >  
> >  private:
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 23a63f8c6e25..1a549c092d73 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -92,6 +92,14 @@ namespace libcamera::ipa::rkisp1 {
> >   * \brief Indicates if the AWB hardware is enabled to apply colour gains
> >   */
> >  
> > +/**
> > + * \var IPASessionConfiguration::lsc
> > + * \brief Lens Shading Correction configuration of the IPA
> > + *
> > + * \var IPASessionConfiguration::lsc.enabled
> > + * \brief Indicates if the LSC hardware is enabled
> 
> imo this doesn't match the intent that's described in the changelog:
> 
> "Add an enable variable <snip> which indicates if the lsc algorithm has been configured"

I'll update the commit message to "[...] indicates if the LSC hardware
module is enabled".

> > + */
> > +
> >  /**
> >   * \var IPASessionConfiguration::sensor
> >   * \brief Sensor-specific configuration of the IPA
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 7f7b3e4d88fa..0cd6aadb83ed 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -32,6 +32,10 @@ struct IPASessionConfiguration {
> >  		bool enabled;
> >  	} awb;
> >  
> > +	struct {
> > +		bool enabled;
> > +	} lsc;
> > +
> >  	struct {
> >  		utils::Duration lineDuration;
> >  		Size size;

-- 
Regards,

Laurent Pinchart