| Message ID | 20260407-kbingham-awb-split-v1-6-a39af3f4dc20@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran On Tue, Apr 07, 2026 at 11:01:09PM +0100, Kieran Bingham wrote: > Move the ActiveState and FrameContext as used by the RKISP1 lux > to be exposed by the libipa implementation for re-use. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/libipa/lux.cpp | 16 ++++++++++++++++ > src/ipa/libipa/lux.h | 12 ++++++++++++ > src/ipa/rkisp1/ipa_context.h | 13 +++++-------- > 3 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/libipa/lux.cpp b/src/ipa/libipa/lux.cpp > index 899e88248f04fbe0d6986528c81b468384fe4a9f..012c6fb367142ce40f60f5e102e3addb142c4a6b 100644 > --- a/src/ipa/libipa/lux.cpp > +++ b/src/ipa/libipa/lux.cpp > @@ -33,6 +33,22 @@ LOG_DEFINE_CATEGORY(Lux) > > namespace ipa { > > +/** > + * \struct lux::ActiveState > + * \brief Active lux estimation state shared across frames nit: I would not reuse the "Active" and "state" words. Same below for "frame" and "context". \brief The most recent lux estimation > + * > + * \var lux::ActiveState::lux > + * \brief The most recently estimated lux value \brief The estimated lux value > + */ > + > +/** > + * \struct lux::FrameContext > + * \brief Per-frame lux estimation context \brief Per-frame lux estimation > + * > + * \var lux::FrameContext::lux > + * \brief The lux value estimation used for processing the frame \brief The estimated lux value Just ideas, up to you > + */ > + > /** > * \class Lux > * \brief Class that implements lux estimation > diff --git a/src/ipa/libipa/lux.h b/src/ipa/libipa/lux.h > index d95bcdafcfcdb44641ddb12306e4705157dc09ae..2aa630a1097e4bc672c792395d2d179dc084c7d8 100644 > --- a/src/ipa/libipa/lux.h > +++ b/src/ipa/libipa/lux.h > @@ -16,6 +16,18 @@ class YamlObject; > > namespace ipa { > > +namespace lux { > + > +struct ActiveState { > + double lux; > +}; > + > +struct FrameContext { > + double lux; > +}; > + > +} /* namespace lux */ > + > class Histogram; > > class Lux > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index e61391bb1510bb642dafc544ee7775390e6948d6..63a6f2b7d884bfff2553175fa32e45ae747fb4c7 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -28,6 +28,7 @@ > #include "libipa/camera_sensor_helper.h" > #include "libipa/fc_queue.h" > #include "libipa/fixedpoint.h" > +#include "libipa/lux.h" > > namespace libcamera { > > @@ -78,6 +79,8 @@ struct IPASessionConfiguration { > }; > > struct IPAActiveState { > + ipa::lux::ActiveState lux; > + It's a shame we didn't order the struct members > struct { > struct { > uint32_t exposure; > @@ -137,10 +140,6 @@ struct IPAActiveState { > double gamma; > } goc; > > - struct { > - double lux; > - } lux; > - it could also be kept here though > struct { > controls::WdrModeEnum mode; > AgcMeanLuminance::AgcConstraint constraint; > @@ -154,6 +153,8 @@ struct IPAActiveState { > }; > > struct IPAFrameContext : public FrameContext { > + ipa::lux::FrameContext lux; > + > struct { > uint32_t exposure; > double gain; > @@ -219,10 +220,6 @@ struct IPAFrameContext : public FrameContext { > Matrix<float, 3, 3> ccm; > } ccm; > > - struct { > - double lux; > - } lux; > - Same here. I like this! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > struct { > controls::WdrModeEnum mode; > double strength; > > -- > 2.53.0 >
Quoting Jacopo Mondi (2026-04-08 09:24:00) > Hi Kieran > > On Tue, Apr 07, 2026 at 11:01:09PM +0100, Kieran Bingham wrote: > > Move the ActiveState and FrameContext as used by the RKISP1 lux > > to be exposed by the libipa implementation for re-use. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/ipa/libipa/lux.cpp | 16 ++++++++++++++++ > > src/ipa/libipa/lux.h | 12 ++++++++++++ > > src/ipa/rkisp1/ipa_context.h | 13 +++++-------- > > 3 files changed, 33 insertions(+), 8 deletions(-) > > > > diff --git a/src/ipa/libipa/lux.cpp b/src/ipa/libipa/lux.cpp > > index 899e88248f04fbe0d6986528c81b468384fe4a9f..012c6fb367142ce40f60f5e102e3addb142c4a6b 100644 > > --- a/src/ipa/libipa/lux.cpp > > +++ b/src/ipa/libipa/lux.cpp > > @@ -33,6 +33,22 @@ LOG_DEFINE_CATEGORY(Lux) > > > > namespace ipa { > > > > +/** > > + * \struct lux::ActiveState > > + * \brief Active lux estimation state shared across frames > > nit: I would not reuse the "Active" and "state" words. Same below for > "frame" and "context". > > \brief The most recent lux estimation > > > + * > > + * \var lux::ActiveState::lux > > + * \brief The most recently estimated lux value > > \brief The estimated lux value > > > + */ > > + > > +/** > > + * \struct lux::FrameContext > > + * \brief Per-frame lux estimation context > > \brief Per-frame lux estimation > > > + * > > + * \var lux::FrameContext::lux > > + * \brief The lux value estimation used for processing the frame > > \brief The estimated lux value > > Just ideas, up to you > > > + */ > > + > > /** > > * \class Lux > > * \brief Class that implements lux estimation > > diff --git a/src/ipa/libipa/lux.h b/src/ipa/libipa/lux.h > > index d95bcdafcfcdb44641ddb12306e4705157dc09ae..2aa630a1097e4bc672c792395d2d179dc084c7d8 100644 > > --- a/src/ipa/libipa/lux.h > > +++ b/src/ipa/libipa/lux.h > > @@ -16,6 +16,18 @@ class YamlObject; > > > > namespace ipa { > > > > +namespace lux { > > + > > +struct ActiveState { > > + double lux; > > +}; > > + > > +struct FrameContext { > > + double lux; > > +}; > > + > > +} /* namespace lux */ > > + > > class Histogram; > > > > class Lux > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index e61391bb1510bb642dafc544ee7775390e6948d6..63a6f2b7d884bfff2553175fa32e45ae747fb4c7 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -28,6 +28,7 @@ > > #include "libipa/camera_sensor_helper.h" > > #include "libipa/fc_queue.h" > > #include "libipa/fixedpoint.h" > > +#include "libipa/lux.h" > > > > namespace libcamera { > > > > @@ -78,6 +79,8 @@ struct IPASessionConfiguration { > > }; > > > > struct IPAActiveState { > > + ipa::lux::ActiveState lux; > > + > > It's a shame we didn't order the struct members I've intentionally pulled the new additions up to the top to make it clearer, as I sort of hope/expect all of them to be convertable. But I guess I don't know yet. Would you order in processing order or alphabetical ? or other? > > > struct { > > struct { > > uint32_t exposure; > > @@ -137,10 +140,6 @@ struct IPAActiveState { > > double gamma; > > } goc; > > > > - struct { > > - double lux; > > - } lux; > > - > > it could also be kept here though > > > struct { > > controls::WdrModeEnum mode; > > AgcMeanLuminance::AgcConstraint constraint; > > @@ -154,6 +153,8 @@ struct IPAActiveState { > > }; > > > > struct IPAFrameContext : public FrameContext { > > + ipa::lux::FrameContext lux; > > + > > struct { > > uint32_t exposure; > > double gain; > > @@ -219,10 +220,6 @@ struct IPAFrameContext : public FrameContext { > > Matrix<float, 3, 3> ccm; > > } ccm; > > > > - struct { > > - double lux; > > - } lux; > > - > > Same here. > > I like this! > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Great, I've posted early to find out if I should continue so thanks - I shall! -- Kieran > > > struct { > > controls::WdrModeEnum mode; > > double strength; > > > > -- > > 2.53.0 > >
On Wed, Apr 08, 2026 at 06:30:12PM +0100, Kieran Bingham wrote: > Quoting Jacopo Mondi (2026-04-08 09:24:00) > > On Tue, Apr 07, 2026 at 11:01:09PM +0100, Kieran Bingham wrote: > > > Move the ActiveState and FrameContext as used by the RKISP1 lux > > > to be exposed by the libipa implementation for re-use. > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > src/ipa/libipa/lux.cpp | 16 ++++++++++++++++ > > > src/ipa/libipa/lux.h | 12 ++++++++++++ > > > src/ipa/rkisp1/ipa_context.h | 13 +++++-------- > > > 3 files changed, 33 insertions(+), 8 deletions(-) > > > > > > diff --git a/src/ipa/libipa/lux.cpp b/src/ipa/libipa/lux.cpp > > > index 899e88248f04fbe0d6986528c81b468384fe4a9f..012c6fb367142ce40f60f5e102e3addb142c4a6b 100644 > > > --- a/src/ipa/libipa/lux.cpp > > > +++ b/src/ipa/libipa/lux.cpp > > > @@ -33,6 +33,22 @@ LOG_DEFINE_CATEGORY(Lux) > > > > > > namespace ipa { > > > > > > +/** > > > + * \struct lux::ActiveState > > > + * \brief Active lux estimation state shared across frames > > > > nit: I would not reuse the "Active" and "state" words. Same below for > > "frame" and "context". > > > > \brief The most recent lux estimation > > > > > + * > > > + * \var lux::ActiveState::lux > > > + * \brief The most recently estimated lux value > > > > \brief The estimated lux value > > > > > + */ > > > + > > > +/** > > > + * \struct lux::FrameContext > > > + * \brief Per-frame lux estimation context > > > > \brief Per-frame lux estimation > > > > > + * > > > + * \var lux::FrameContext::lux > > > + * \brief The lux value estimation used for processing the frame > > > > \brief The estimated lux value > > > > Just ideas, up to you > > > > > + */ > > > + > > > /** > > > * \class Lux > > > * \brief Class that implements lux estimation > > > diff --git a/src/ipa/libipa/lux.h b/src/ipa/libipa/lux.h > > > index d95bcdafcfcdb44641ddb12306e4705157dc09ae..2aa630a1097e4bc672c792395d2d179dc084c7d8 100644 > > > --- a/src/ipa/libipa/lux.h > > > +++ b/src/ipa/libipa/lux.h > > > @@ -16,6 +16,18 @@ class YamlObject; > > > > > > namespace ipa { > > > > > > +namespace lux { > > > + > > > +struct ActiveState { > > > + double lux; > > > +}; > > > + > > > +struct FrameContext { > > > + double lux; > > > +}; > > > + > > > +} /* namespace lux */ > > > + > > > class Histogram; > > > > > > class Lux > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > > index e61391bb1510bb642dafc544ee7775390e6948d6..63a6f2b7d884bfff2553175fa32e45ae747fb4c7 100644 > > > --- a/src/ipa/rkisp1/ipa_context.h > > > +++ b/src/ipa/rkisp1/ipa_context.h > > > @@ -28,6 +28,7 @@ > > > #include "libipa/camera_sensor_helper.h" > > > #include "libipa/fc_queue.h" > > > #include "libipa/fixedpoint.h" > > > +#include "libipa/lux.h" > > > > > > namespace libcamera { > > > > > > @@ -78,6 +79,8 @@ struct IPASessionConfiguration { > > > }; > > > > > > struct IPAActiveState { > > > + ipa::lux::ActiveState lux; > > > + > > > > It's a shame we didn't order the struct members > > I've intentionally pulled the new additions up to the top to make it > clearer, as I sort of hope/expect all of them to be convertable. But I > guess I don't know yet. > > Would you order in processing order or alphabetical ? or other? I think processing order would be best, although the processing order currently depends on the tuning file. > > > struct { > > > struct { > > > uint32_t exposure; > > > @@ -137,10 +140,6 @@ struct IPAActiveState { > > > double gamma; > > > } goc; > > > > > > - struct { > > > - double lux; > > > - } lux; > > > - > > > > it could also be kept here though > > > > > struct { > > > controls::WdrModeEnum mode; > > > AgcMeanLuminance::AgcConstraint constraint; > > > @@ -154,6 +153,8 @@ struct IPAActiveState { > > > }; > > > > > > struct IPAFrameContext : public FrameContext { > > > + ipa::lux::FrameContext lux; > > > + > > > struct { > > > uint32_t exposure; > > > double gain; > > > @@ -219,10 +220,6 @@ struct IPAFrameContext : public FrameContext { > > > Matrix<float, 3, 3> ccm; > > > } ccm; > > > > > > - struct { > > > - double lux; > > > - } lux; > > > - > > > > Same here. > > > > I like this! > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Great, I've posted early to find out if I should continue so thanks - I > shall! It's promising, let's see what happens in the next patches :-) > > > struct { > > > controls::WdrModeEnum mode; > > > double strength; > > >
diff --git a/src/ipa/libipa/lux.cpp b/src/ipa/libipa/lux.cpp index 899e88248f04fbe0d6986528c81b468384fe4a9f..012c6fb367142ce40f60f5e102e3addb142c4a6b 100644 --- a/src/ipa/libipa/lux.cpp +++ b/src/ipa/libipa/lux.cpp @@ -33,6 +33,22 @@ LOG_DEFINE_CATEGORY(Lux) namespace ipa { +/** + * \struct lux::ActiveState + * \brief Active lux estimation state shared across frames + * + * \var lux::ActiveState::lux + * \brief The most recently estimated lux value + */ + +/** + * \struct lux::FrameContext + * \brief Per-frame lux estimation context + * + * \var lux::FrameContext::lux + * \brief The lux value estimation used for processing the frame + */ + /** * \class Lux * \brief Class that implements lux estimation diff --git a/src/ipa/libipa/lux.h b/src/ipa/libipa/lux.h index d95bcdafcfcdb44641ddb12306e4705157dc09ae..2aa630a1097e4bc672c792395d2d179dc084c7d8 100644 --- a/src/ipa/libipa/lux.h +++ b/src/ipa/libipa/lux.h @@ -16,6 +16,18 @@ class YamlObject; namespace ipa { +namespace lux { + +struct ActiveState { + double lux; +}; + +struct FrameContext { + double lux; +}; + +} /* namespace lux */ + class Histogram; class Lux diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index e61391bb1510bb642dafc544ee7775390e6948d6..63a6f2b7d884bfff2553175fa32e45ae747fb4c7 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -28,6 +28,7 @@ #include "libipa/camera_sensor_helper.h" #include "libipa/fc_queue.h" #include "libipa/fixedpoint.h" +#include "libipa/lux.h" namespace libcamera { @@ -78,6 +79,8 @@ struct IPASessionConfiguration { }; struct IPAActiveState { + ipa::lux::ActiveState lux; + struct { struct { uint32_t exposure; @@ -137,10 +140,6 @@ struct IPAActiveState { double gamma; } goc; - struct { - double lux; - } lux; - struct { controls::WdrModeEnum mode; AgcMeanLuminance::AgcConstraint constraint; @@ -154,6 +153,8 @@ struct IPAActiveState { }; struct IPAFrameContext : public FrameContext { + ipa::lux::FrameContext lux; + struct { uint32_t exposure; double gain; @@ -219,10 +220,6 @@ struct IPAFrameContext : public FrameContext { Matrix<float, 3, 3> ccm; } ccm; - struct { - double lux; - } lux; - struct { controls::WdrModeEnum mode; double strength;
Move the ActiveState and FrameContext as used by the RKISP1 lux to be exposed by the libipa implementation for re-use. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/ipa/libipa/lux.cpp | 16 ++++++++++++++++ src/ipa/libipa/lux.h | 12 ++++++++++++ src/ipa/rkisp1/ipa_context.h | 13 +++++-------- 3 files changed, 33 insertions(+), 8 deletions(-)