[v1,12/12] ipa: rkisp1: Add LensShadingEnable control
diff mbox series

Message ID 20251014075252.2876485-13-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Add resampling support for polynomial LSC data
Related show

Commit Message

Stefan Klug Oct. 14, 2025, 7:52 a.m. UTC 
Implement the LensShadingEnable control for rkisp1.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/dpf.cpp   |  2 +-
 src/ipa/rkisp1/algorithms/lsc.cpp   | 41 ++++++++++++++++++++++++++---
 src/ipa/rkisp1/algorithms/lsc.h     |  3 +++
 src/ipa/rkisp1/ipa_context.h        | 12 ++++++---
 src/libcamera/control_ids_core.yaml |  3 +++
 5 files changed, 52 insertions(+), 9 deletions(-)

Comments

Barnabás Pőcze Oct. 24, 2025, 4:13 p.m. UTC | #1 
2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:
> Implement the LensShadingEnable control for rkisp1.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/ipa/rkisp1/algorithms/dpf.cpp   |  2 +-
>   src/ipa/rkisp1/algorithms/lsc.cpp   | 41 ++++++++++++++++++++++++++---
>   src/ipa/rkisp1/algorithms/lsc.h     |  3 +++
>   src/ipa/rkisp1/ipa_context.h        | 12 ++++++---
>   src/libcamera/control_ids_core.yaml |  3 +++
>   5 files changed, 52 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index cb6095daeeed..79cb4829fdbb 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -229,7 +229,7 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>   		*config = config_;
>   
>   		const auto &awb = context.configuration.awb;
> -		const auto &lsc = context.configuration.lsc;
> +		const auto &lsc = context.activeState.lsc;
>   
>   		auto &mode = config->gain.mode;
>   
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index 68cf35064182..ddb7154a0211 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -366,6 +366,8 @@ LensShadingCorrection::LensShadingCorrection()
>   int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>   				const YamlObject &tuningData)
>   {
> +	context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, true);

Maybe I'm missing something but it seems this control is only applicable for processed
streams, shouldn't it be hidden based on the configuration?


> +
>   	xSize_ = parseSizes(tuningData, "x-size");
>   	ySize_ = parseSizes(tuningData, "y-size");
>   
> @@ -449,7 +451,7 @@ int LensShadingCorrection::configure(IPAContext &context,
>   
>   	sets_.setData(std::move(shadingData));
>   
> -	context.configuration.lsc.enabled = true;
> +	context.activeState.lsc.enabled = true;
>   	return 0;
>   }
>   
> @@ -470,6 +472,27 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,
>   	std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);
>   }
>   
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void LensShadingCorrection::queueRequest(IPAContext &context,
> +					 [[maybe_unused]] const uint32_t frame,
> +					 IPAFrameContext &frameContext,
> +					 const ControlList &controls)
> +{
> +	auto &lsc = context.activeState.lsc;
> +
> +	const auto &lscEnable = controls.get(controls::LensShadingEnable);
> +	if (lscEnable && *lscEnable != lsc.enabled) {
> +		lsc.enabled = *lscEnable;
> +
> +		LOG(RkISP1Lsc, Debug)
> +			<< (*lscEnable ? "Enabling" : "Disabling") << " Lsc";
> +
> +		frameContext.lsc.update = true;
> +	}
> +}
> +
>   /**
>    * \copydoc libcamera::ipa::Algorithm::prepare
>    */
> @@ -479,16 +502,26 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,
>   				    RkISP1Params *params)
>   {
>   	uint32_t ct = frameContext.awb.temperatureK;
> -	if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
> +
> +	if (!frameContext.lsc.update && !context.activeState.lsc.enabled)
> +		return;
> +
> +	if (!frameContext.lsc.update &&
> +	    std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
>   	    kColourTemperatureChangeThreshhold)
>   		return;
> +
>   	unsigned int quantizedCt;
>   	const Components &set = sets_.getInterpolated(ct, &quantizedCt);
> -	if (lastAppliedQuantizedCt_ == quantizedCt)
> +	if (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt)
>   		return;

Maybe there is way to have just a single `frameContext.lsc.update` check,
which would be easier to understand in my opinion. At least it seems easily
doable for the first two `if`s.


>   
>   	auto config = params->block<BlockType::Lsc>();
> -	config.setEnabled(true);
> +	config.setEnabled(context.activeState.lsc.enabled);
> +
> +	if (!context.activeState.lsc.enabled)
> +		return;
> +
>   	setParameters(*config);
>   	copyTable(*config, set);
>   
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index 43fee337931f..a3d8042130c0 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -29,6 +29,9 @@ public:
>   	void prepare(IPAContext &context, const uint32_t frame,
>   		     IPAFrameContext &frameContext,
>   		     RkISP1Params *params) override;
> +	void queueRequest(IPAContext &context, const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>   
>   	struct Components {
>   		std::vector<uint16_t> r;
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index f85a130d9c23..5ec87ae4e5a3 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -55,10 +55,6 @@ struct IPASessionConfiguration {
>   		bool supported;
>   	} compress;
>   
> -	struct {
> -		bool enabled;
> -	} lsc;
> -
>   	struct {
>   		utils::Duration minExposureTime;
>   		utils::Duration maxExposureTime;
> @@ -139,6 +135,10 @@ struct IPAActiveState {
>   		double gain;
>   		double strength;
>   	} wdr;
> +
> +	struct {
> +		bool enabled;
> +	} lsc;
>   };
>   
>   struct IPAFrameContext : public FrameContext {
> @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext {
>   		double strength;
>   		double gain;
>   	} wdr;
> +
> +	struct {
> +		bool update;
> +	} lsc;
>   };
>   
>   struct IPAContext {
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index 9244b7eddb77..3b062df64bf5 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -1352,4 +1352,7 @@ controls:
>         description: |
>           Enable or disable the lens shading algorithm.
>   
> +        This control is only available when there are valid lens shading
> +        correction parameters available in the tuning file.
> +
>   ...
Rui Wang Oct. 24, 2025, 9:09 p.m. UTC | #2 
On 2025-10-14 03:52, Stefan Klug wrote:
> Implement the LensShadingEnable control for rkisp1. Signed-off-by: 
> Stefan Klug <stefan.klug@ideasonboard.com> --- 
> src/ipa/rkisp1/algorithms/dpf.cpp | 2 +- 
> src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++--- 
> src/ipa/rkisp1/algorithms/lsc.h | 3 +++ src/ipa/rkisp1/ipa_context.h | 
> 12 ++++++--- src/libcamera/control_ids_core.yaml | 3 +++ 5 files 
> changed, 52 insertions(+), 9 deletions(-) diff --git 
> a/src/ipa/rkisp1/algorithms/dpf.cpp 
> b/src/ipa/rkisp1/algorithms/dpf.cpp index cb6095daeeed..79cb4829fdbb 
> 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ 
> b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -229,7 +229,7 @@ void 
> Dpf::prepare(IPAContext &context, const uint32_t frame, *config = 
> config_; const auto &awb = context.configuration.awb; - const auto 
> &lsc = context.configuration.lsc; + const auto &lsc = 
> context.activeState.lsc; auto &mode = config->gain.mode; diff --git 
> a/src/ipa/rkisp1/algorithms/lsc.cpp 
> b/src/ipa/rkisp1/algorithms/lsc.cpp index 68cf35064182..ddb7154a0211 
> 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ 
> b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -366,6 +366,8 @@ 
> LensShadingCorrection::LensShadingCorrection() int 
> LensShadingCorrection::init([[maybe_unused]] IPAContext &context, 
> const YamlObject &tuningData) {

                 auto lscEnable = parseYaml(tuningData);  or default as true

                I think if Yaml has lsc enable toggle . it would be better .

> + context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, 
> true, true);

	  context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, lscEnable);

> + xSize_ = parseSizes(tuningData, "x-size"); ySize_ = 
> parseSizes(tuningData, "y-size"); @@ -449,7 +451,7 @@ int 
> LensShadingCorrection::configure(IPAContext &context, 
> sets_.setData(std::move(shadingData)); - 
> context.configuration.lsc.enabled = true; + 
> context.activeState.lsc.enabled = true;

context.activeState.lsc.enabled = lscEnable;

> return 0; } @@ -470,6 +472,27 @@ void 
> LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, 
> std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]); } 
> +/** + * \copydoc libcamera::ipa::Algorithm::queueRequest + */ +void 
> LensShadingCorrection::queueRequest(IPAContext &context, + 
> [[maybe_unused]] const uint32_t frame, + IPAFrameContext 
> &frameContext, + const ControlList &controls) +{ + auto &lsc = 
> context.activeState.lsc; + + const auto &lscEnable = 
> controls.get(controls::LensShadingEnable); + if (lscEnable && 
> *lscEnable != lsc.enabled) { + lsc.enabled = *lscEnable; + + 
> LOG(RkISP1Lsc, Debug) + << (*lscEnable ? "Enabling" : "Disabling") << 
> " Lsc"; + + frameContext.lsc.update = true; + } +} + /** * \copydoc 
> libcamera::ipa::Algorithm::prepare */ @@ -479,16 +502,26 @@ void 
> LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, 
> RkISP1Params *params) { uint32_t ct = frameContext.awb.temperatureK; - 
> if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) 
> < + + if (!frameContext.lsc.update && 
> !context.activeState.lsc.enabled) + return; + + if 
> (!frameContext.lsc.update && + std::abs(static_cast<int>(ct) - 
> static_cast<int>(lastAppliedCt_)) < 
> kColourTemperatureChangeThreshhold) return; + unsigned int 
> quantizedCt; const Components &set = sets_.getInterpolated(ct, 
> &quantizedCt); - if (lastAppliedQuantizedCt_ == quantizedCt) + if 
> (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt) 
> return; auto config = params->block<BlockType::Lsc>(); - 
> config.setEnabled(true); + 
> config.setEnabled(context.activeState.lsc.enabled); + + if 
> (!context.activeState.lsc.enabled) + return; + setParameters(*config); 
> copyTable(*config, set); diff --git a/src/ipa/rkisp1/algorithms/lsc.h 
> b/src/ipa/rkisp1/algorithms/lsc.h index 43fee337931f..a3d8042130c0 
> 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ 
> b/src/ipa/rkisp1/algorithms/lsc.h @@ -29,6 +29,9 @@ public: void 
> prepare(IPAContext &context, const uint32_t frame, IPAFrameContext 
> &frameContext, RkISP1Params *params) override; + void 
> queueRequest(IPAContext &context, const uint32_t frame, + 
> IPAFrameContext &frameContext, + const ControlList &controls) 
> override; struct Components { std::vector<uint16_t> r; diff --git 
> a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 
> f85a130d9c23..5ec87ae4e5a3 100644 --- a/src/ipa/rkisp1/ipa_context.h 
> +++ b/src/ipa/rkisp1/ipa_context.h @@ -55,10 +55,6 @@ struct 
> IPASessionConfiguration { bool supported; } compress; - struct { - 
> bool enabled; - } lsc; - struct { utils::Duration minExposureTime; 
> utils::Duration maxExposureTime; @@ -139,6 +135,10 @@ struct 
> IPAActiveState { double gain; double strength; } wdr; + + struct { + 
> bool enabled; + } lsc; }; struct IPAFrameContext : public FrameContext 
> { @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext { 
> double strength; double gain; } wdr; + + struct { + bool update; + } 
> lsc; }; struct IPAContext { diff --git 
> a/src/libcamera/control_ids_core.yaml 
> b/src/libcamera/control_ids_core.yaml index 9244b7eddb77..3b062df64bf5 
> 100644 --- a/src/libcamera/control_ids_core.yaml +++ 
> b/src/libcamera/control_ids_core.yaml @@ -1352,4 +1352,7 @@ controls: 
> description: | Enable or disable the lens shading algorithm. + This 
> control is only available when there are valid lens shading + 
> correction parameters available in the tuning file. + ...
Stefan Klug Jan. 8, 2026, 12:09 p.m. UTC | #3 
Hi Barnabás,

Thank you for the review.

Quoting Barnabás Pőcze (2025-10-24 18:13:16)
> 2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:
> > Implement the LensShadingEnable control for rkisp1.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >   src/ipa/rkisp1/algorithms/dpf.cpp   |  2 +-
> >   src/ipa/rkisp1/algorithms/lsc.cpp   | 41 ++++++++++++++++++++++++++---
> >   src/ipa/rkisp1/algorithms/lsc.h     |  3 +++
> >   src/ipa/rkisp1/ipa_context.h        | 12 ++++++---
> >   src/libcamera/control_ids_core.yaml |  3 +++
> >   5 files changed, 52 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > index cb6095daeeed..79cb4829fdbb 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -229,7 +229,7 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> >               *config = config_;
> >   
> >               const auto &awb = context.configuration.awb;
> > -             const auto &lsc = context.configuration.lsc;
> > +             const auto &lsc = context.activeState.lsc;
> >   
> >               auto &mode = config->gain.mode;
> >   
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > index 68cf35064182..ddb7154a0211 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > @@ -366,6 +366,8 @@ LensShadingCorrection::LensShadingCorrection()
> >   int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
> >                               const YamlObject &tuningData)
> >   {
> > +     context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, true);
> 
> Maybe I'm missing something but it seems this control is only applicable for processed
> streams, shouldn't it be hidden based on the configuration?

There is Algorithm::supportsRaw_ which defaults to false. But that only
guards the configure step. So the control is added to the info map
unconditionally. But that is the case for all algorithms already. As we
don't have a solution for thread safe modification of the control info
map and do not advertise to reload the map after camera configure time,
I think we should keep it that way for now. But the issue is there. 

> 
> 
> > +
> >       xSize_ = parseSizes(tuningData, "x-size");
> >       ySize_ = parseSizes(tuningData, "y-size");
> >   
> > @@ -449,7 +451,7 @@ int LensShadingCorrection::configure(IPAContext &context,
> >   
> >       sets_.setData(std::move(shadingData));
> >   
> > -     context.configuration.lsc.enabled = true;
> > +     context.activeState.lsc.enabled = true;
> >       return 0;
> >   }
> >   
> > @@ -470,6 +472,27 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,
> >       std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);
> >   }
> >   
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::queueRequest
> > + */
> > +void LensShadingCorrection::queueRequest(IPAContext &context,
> > +                                      [[maybe_unused]] const uint32_t frame,
> > +                                      IPAFrameContext &frameContext,
> > +                                      const ControlList &controls)
> > +{
> > +     auto &lsc = context.activeState.lsc;
> > +
> > +     const auto &lscEnable = controls.get(controls::LensShadingEnable);
> > +     if (lscEnable && *lscEnable != lsc.enabled) {
> > +             lsc.enabled = *lscEnable;
> > +
> > +             LOG(RkISP1Lsc, Debug)
> > +                     << (*lscEnable ? "Enabling" : "Disabling") << " Lsc";
> > +
> > +             frameContext.lsc.update = true;
> > +     }
> > +}
> > +
> >   /**
> >    * \copydoc libcamera::ipa::Algorithm::prepare
> >    */
> > @@ -479,16 +502,26 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,
> >                                   RkISP1Params *params)
> >   {
> >       uint32_t ct = frameContext.awb.temperatureK;
> > -     if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
> > +
> > +     if (!frameContext.lsc.update && !context.activeState.lsc.enabled)
> > +             return;
> > +
> > +     if (!frameContext.lsc.update &&
> > +         std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
> >           kColourTemperatureChangeThreshhold)
> >               return;
> > +
> >       unsigned int quantizedCt;
> >       const Components &set = sets_.getInterpolated(ct, &quantizedCt);
> > -     if (lastAppliedQuantizedCt_ == quantizedCt)
> > +     if (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt)
> >               return;
> 
> Maybe there is way to have just a single `frameContext.lsc.update` check,
> which would be easier to understand in my opinion. At least it seems easily
> doable for the first two `if`s.

Yes that wasn't nice. I think I found a nicer solution for v2.

Best regards,
Stefan

> 
> 
> >   
> >       auto config = params->block<BlockType::Lsc>();
> > -     config.setEnabled(true);
> > +     config.setEnabled(context.activeState.lsc.enabled);
> > +
> > +     if (!context.activeState.lsc.enabled)
> > +             return;
> > +
> >       setParameters(*config);
> >       copyTable(*config, set);
> >   
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> > index 43fee337931f..a3d8042130c0 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.h
> > +++ b/src/ipa/rkisp1/algorithms/lsc.h
> > @@ -29,6 +29,9 @@ public:
> >       void prepare(IPAContext &context, const uint32_t frame,
> >                    IPAFrameContext &frameContext,
> >                    RkISP1Params *params) override;
> > +     void queueRequest(IPAContext &context, const uint32_t frame,
> > +                       IPAFrameContext &frameContext,
> > +                       const ControlList &controls) override;
> >   
> >       struct Components {
> >               std::vector<uint16_t> r;
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index f85a130d9c23..5ec87ae4e5a3 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -55,10 +55,6 @@ struct IPASessionConfiguration {
> >               bool supported;
> >       } compress;
> >   
> > -     struct {
> > -             bool enabled;
> > -     } lsc;
> > -
> >       struct {
> >               utils::Duration minExposureTime;
> >               utils::Duration maxExposureTime;
> > @@ -139,6 +135,10 @@ struct IPAActiveState {
> >               double gain;
> >               double strength;
> >       } wdr;
> > +
> > +     struct {
> > +             bool enabled;
> > +     } lsc;
> >   };
> >   
> >   struct IPAFrameContext : public FrameContext {
> > @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext {
> >               double strength;
> >               double gain;
> >       } wdr;
> > +
> > +     struct {
> > +             bool update;
> > +     } lsc;
> >   };
> >   
> >   struct IPAContext {
> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > index 9244b7eddb77..3b062df64bf5 100644
> > --- a/src/libcamera/control_ids_core.yaml
> > +++ b/src/libcamera/control_ids_core.yaml
> > @@ -1352,4 +1352,7 @@ controls:
> >         description: |
> >           Enable or disable the lens shading algorithm.
> >   
> > +        This control is only available when there are valid lens shading
> > +        correction parameters available in the tuning file.
> > +
> >   ...
>
Stefan Klug Jan. 8, 2026, 12:13 p.m. UTC | #4 
Hi Rui,

Quoting rui wang (2025-10-24 23:09:02)
> 
> On 2025-10-14 03:52, Stefan Klug wrote:
> > Implement the LensShadingEnable control for rkisp1. Signed-off-by: 
> > Stefan Klug <stefan.klug@ideasonboard.com> --- 
> > src/ipa/rkisp1/algorithms/dpf.cpp | 2 +- 
> > src/ipa/rkisp1/algorithms/lsc.cpp | 41 ++++++++++++++++++++++++++--- 
> > src/ipa/rkisp1/algorithms/lsc.h | 3 +++ src/ipa/rkisp1/ipa_context.h | 
> > 12 ++++++--- src/libcamera/control_ids_core.yaml | 3 +++ 5 files 
> > changed, 52 insertions(+), 9 deletions(-) diff --git 
> > a/src/ipa/rkisp1/algorithms/dpf.cpp 
> > b/src/ipa/rkisp1/algorithms/dpf.cpp index cb6095daeeed..79cb4829fdbb 
> > 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ 
> > b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -229,7 +229,7 @@ void 
> > Dpf::prepare(IPAContext &context, const uint32_t frame, *config = 
> > config_; const auto &awb = context.configuration.awb; - const auto 
> > &lsc = context.configuration.lsc; + const auto &lsc = 
> > context.activeState.lsc; auto &mode = config->gain.mode; diff --git 
> > a/src/ipa/rkisp1/algorithms/lsc.cpp 
> > b/src/ipa/rkisp1/algorithms/lsc.cpp index 68cf35064182..ddb7154a0211 
> > 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ 
> > b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -366,6 +366,8 @@ 
> > LensShadingCorrection::LensShadingCorrection() int 
> > LensShadingCorrection::init([[maybe_unused]] IPAContext &context, 
> > const YamlObject &tuningData) {

The formatting is broken in your response mail. Please check your email
client.

> 
>                  auto lscEnable = parseYaml(tuningData);  or default as true
> 
>                 I think if Yaml has lsc enable toggle . it would be better .

We don't usually do that. If a algorithm is configured, it gets
enabled. I guess the only usecase for that would be during development
but is easy to workaround. And it somehow collides with the enabled
flag Isaac wants to integrate.

Best regards,
Stefan

> 
> > + context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, 
> > true, true);
> 
>           context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, lscEnable);
> 
> > + xSize_ = parseSizes(tuningData, "x-size"); ySize_ = 
> > parseSizes(tuningData, "y-size"); @@ -449,7 +451,7 @@ int 
> > LensShadingCorrection::configure(IPAContext &context, 
> > sets_.setData(std::move(shadingData)); - 
> > context.configuration.lsc.enabled = true; + 
> > context.activeState.lsc.enabled = true;
> 
> context.activeState.lsc.enabled = lscEnable;
> 
> > return 0; } @@ -470,6 +472,27 @@ void 
> > LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, 
> > std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]); } 
> > +/** + * \copydoc libcamera::ipa::Algorithm::queueRequest + */ +void 
> > LensShadingCorrection::queueRequest(IPAContext &context, + 
> > [[maybe_unused]] const uint32_t frame, + IPAFrameContext 
> > &frameContext, + const ControlList &controls) +{ + auto &lsc = 
> > context.activeState.lsc; + + const auto &lscEnable = 
> > controls.get(controls::LensShadingEnable); + if (lscEnable && 
> > *lscEnable != lsc.enabled) { + lsc.enabled = *lscEnable; + + 
> > LOG(RkISP1Lsc, Debug) + << (*lscEnable ? "Enabling" : "Disabling") << 
> > " Lsc"; + + frameContext.lsc.update = true; + } +} + /** * \copydoc 
> > libcamera::ipa::Algorithm::prepare */ @@ -479,16 +502,26 @@ void 
> > LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, 
> > RkISP1Params *params) { uint32_t ct = frameContext.awb.temperatureK; - 
> > if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) 
> > < + + if (!frameContext.lsc.update && 
> > !context.activeState.lsc.enabled) + return; + + if 
> > (!frameContext.lsc.update && + std::abs(static_cast<int>(ct) - 
> > static_cast<int>(lastAppliedCt_)) < 
> > kColourTemperatureChangeThreshhold) return; + unsigned int 
> > quantizedCt; const Components &set = sets_.getInterpolated(ct, 
> > &quantizedCt); - if (lastAppliedQuantizedCt_ == quantizedCt) + if 
> > (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt) 
> > return; auto config = params->block<BlockType::Lsc>(); - 
> > config.setEnabled(true); + 
> > config.setEnabled(context.activeState.lsc.enabled); + + if 
> > (!context.activeState.lsc.enabled) + return; + setParameters(*config); 
> > copyTable(*config, set); diff --git a/src/ipa/rkisp1/algorithms/lsc.h 
> > b/src/ipa/rkisp1/algorithms/lsc.h index 43fee337931f..a3d8042130c0 
> > 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ 
> > b/src/ipa/rkisp1/algorithms/lsc.h @@ -29,6 +29,9 @@ public: void 
> > prepare(IPAContext &context, const uint32_t frame, IPAFrameContext 
> > &frameContext, RkISP1Params *params) override; + void 
> > queueRequest(IPAContext &context, const uint32_t frame, + 
> > IPAFrameContext &frameContext, + const ControlList &controls) 
> > override; struct Components { std::vector<uint16_t> r; diff --git 
> > a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 
> > f85a130d9c23..5ec87ae4e5a3 100644 --- a/src/ipa/rkisp1/ipa_context.h 
> > +++ b/src/ipa/rkisp1/ipa_context.h @@ -55,10 +55,6 @@ struct 
> > IPASessionConfiguration { bool supported; } compress; - struct { - 
> > bool enabled; - } lsc; - struct { utils::Duration minExposureTime; 
> > utils::Duration maxExposureTime; @@ -139,6 +135,10 @@ struct 
> > IPAActiveState { double gain; double strength; } wdr; + + struct { + 
> > bool enabled; + } lsc; }; struct IPAFrameContext : public FrameContext 
> > { @@ -214,6 +214,10 @@ struct IPAFrameContext : public FrameContext { 
> > double strength; double gain; } wdr; + + struct { + bool update; + } 
> > lsc; }; struct IPAContext { diff --git 
> > a/src/libcamera/control_ids_core.yaml 
> > b/src/libcamera/control_ids_core.yaml index 9244b7eddb77..3b062df64bf5 
> > 100644 --- a/src/libcamera/control_ids_core.yaml +++ 
> > b/src/libcamera/control_ids_core.yaml @@ -1352,4 +1352,7 @@ controls: 
> > description: | Enable or disable the lens shading algorithm. + This 
> > control is only available when there are valid lens shading + 
> > correction parameters available in the tuning file. + ...

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index cb6095daeeed..79cb4829fdbb 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -229,7 +229,7 @@  void Dpf::prepare(IPAContext &context, const uint32_t frame,
 		*config = config_;
 
 		const auto &awb = context.configuration.awb;
-		const auto &lsc = context.configuration.lsc;
+		const auto &lsc = context.activeState.lsc;
 
 		auto &mode = config->gain.mode;
 
diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index 68cf35064182..ddb7154a0211 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -366,6 +366,8 @@  LensShadingCorrection::LensShadingCorrection()
 int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 				const YamlObject &tuningData)
 {
+	context.ctrlMap[&controls::LensShadingEnable] = ControlInfo(false, true, true);
+
 	xSize_ = parseSizes(tuningData, "x-size");
 	ySize_ = parseSizes(tuningData, "y-size");
 
@@ -449,7 +451,7 @@  int LensShadingCorrection::configure(IPAContext &context,
 
 	sets_.setData(std::move(shadingData));
 
-	context.configuration.lsc.enabled = true;
+	context.activeState.lsc.enabled = true;
 	return 0;
 }
 
@@ -470,6 +472,27 @@  void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,
 	std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);
 }
 
+/**
+ * \copydoc libcamera::ipa::Algorithm::queueRequest
+ */
+void LensShadingCorrection::queueRequest(IPAContext &context,
+					 [[maybe_unused]] const uint32_t frame,
+					 IPAFrameContext &frameContext,
+					 const ControlList &controls)
+{
+	auto &lsc = context.activeState.lsc;
+
+	const auto &lscEnable = controls.get(controls::LensShadingEnable);
+	if (lscEnable && *lscEnable != lsc.enabled) {
+		lsc.enabled = *lscEnable;
+
+		LOG(RkISP1Lsc, Debug)
+			<< (*lscEnable ? "Enabling" : "Disabling") << " Lsc";
+
+		frameContext.lsc.update = true;
+	}
+}
+
 /**
  * \copydoc libcamera::ipa::Algorithm::prepare
  */
@@ -479,16 +502,26 @@  void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,
 				    RkISP1Params *params)
 {
 	uint32_t ct = frameContext.awb.temperatureK;
-	if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
+
+	if (!frameContext.lsc.update && !context.activeState.lsc.enabled)
+		return;
+
+	if (!frameContext.lsc.update &&
+	    std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
 	    kColourTemperatureChangeThreshhold)
 		return;
+
 	unsigned int quantizedCt;
 	const Components &set = sets_.getInterpolated(ct, &quantizedCt);
-	if (lastAppliedQuantizedCt_ == quantizedCt)
+	if (!frameContext.lsc.update && lastAppliedQuantizedCt_ == quantizedCt)
 		return;
 
 	auto config = params->block<BlockType::Lsc>();
-	config.setEnabled(true);
+	config.setEnabled(context.activeState.lsc.enabled);
+
+	if (!context.activeState.lsc.enabled)
+		return;
+
 	setParameters(*config);
 	copyTable(*config, set);
 
diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
index 43fee337931f..a3d8042130c0 100644
--- a/src/ipa/rkisp1/algorithms/lsc.h
+++ b/src/ipa/rkisp1/algorithms/lsc.h
@@ -29,6 +29,9 @@  public:
 	void prepare(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext,
 		     RkISP1Params *params) override;
+	void queueRequest(IPAContext &context, const uint32_t frame,
+			  IPAFrameContext &frameContext,
+			  const ControlList &controls) override;
 
 	struct Components {
 		std::vector<uint16_t> r;
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index f85a130d9c23..5ec87ae4e5a3 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -55,10 +55,6 @@  struct IPASessionConfiguration {
 		bool supported;
 	} compress;
 
-	struct {
-		bool enabled;
-	} lsc;
-
 	struct {
 		utils::Duration minExposureTime;
 		utils::Duration maxExposureTime;
@@ -139,6 +135,10 @@  struct IPAActiveState {
 		double gain;
 		double strength;
 	} wdr;
+
+	struct {
+		bool enabled;
+	} lsc;
 };
 
 struct IPAFrameContext : public FrameContext {
@@ -214,6 +214,10 @@  struct IPAFrameContext : public FrameContext {
 		double strength;
 		double gain;
 	} wdr;
+
+	struct {
+		bool update;
+	} lsc;
 };
 
 struct IPAContext {
diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
index 9244b7eddb77..3b062df64bf5 100644
--- a/src/libcamera/control_ids_core.yaml
+++ b/src/libcamera/control_ids_core.yaml
@@ -1352,4 +1352,7 @@  controls:
       description: |
         Enable or disable the lens shading algorithm.
 
+        This control is only available when there are valid lens shading
+        correction parameters available in the tuning file.
+
 ...