| Message ID | 20260616-ipu3-libipa-rework-v1-9-d4448b54f1d8@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
2026. 06. 16. 8:41 keltezéssel, Daniel Scally írta: > Add an instance of IPACameraSensorInfo to the IPAContext and fill it > during IPAIPU3::init(). This will allow us to pass sensor information > to algorithms at init time. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/ipa/ipu3/ipa_context.cpp | 3 +++ > src/ipa/ipu3/ipa_context.h | 3 +++ > src/ipa/ipu3/ipu3.cpp | 1 + > 3 files changed, 7 insertions(+) > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index 7152d070d3ab1bc463fdaad437d5e1c1b87ce25c..7fcfd5e0e4ade92521cc2914dd07113235af8e45 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -54,6 +54,9 @@ namespace libcamera::ipa::ipu3 { > * > * \var IPAContext::ctrlMap > * \brief A ControlInfoMap::Map of controls populated by the algorithms > + * > + * \var IPAContext::sensorInfo > + * \brief An IPACameraSensorInfo instance holding information about the sensor > */ > > /** > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 1eaaac82da0e3ad5bed0749c39d9dad8c585cab0..f157f223cbb3119f108d768b14fca514ac5661ca 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -15,6 +15,8 @@ > #include <libcamera/controls.h> > #include <libcamera/geometry.h> > > +#include <libcamera/ipa/core_ipa_interface.h> > + > #include <libipa/awb.h> > #include <libipa/ccm.h> > #include <libipa/fc_queue.h> > @@ -93,6 +95,7 @@ struct IPAContext { > FCQueue<IPAFrameContext> frameContexts; > > ControlInfoMap::Map ctrlMap; > + IPACameraSensorInfo sensorInfo; > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 4bdc4b7677fe9703fb968ca91967c68742454514..0827ad207c9d89b8af4887f609fe08881612ab96 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -313,6 +313,7 @@ int IPAIPU3::init(const IPASettings &settings, > context_.configuration = {}; > context_.configuration.sensor.lineDuration = > sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate; > + context_.sensorInfo = sensorInfo; There is already a `sensorInfo_` member in `IPAIPU3`. Could we replace that with this one? > > /* Load the tuning data file. */ > File file(settings.configurationFile); > > -- > 2.43.0 >
Hi Barnabas On 16/06/2026 09:30, Barnabás Pőcze wrote: > 2026. 06. 16. 8:41 keltezéssel, Daniel Scally írta: >> Add an instance of IPACameraSensorInfo to the IPAContext and fill it >> during IPAIPU3::init(). This will allow us to pass sensor information >> to algorithms at init time. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> src/ipa/ipu3/ipa_context.cpp | 3 +++ >> src/ipa/ipu3/ipa_context.h | 3 +++ >> src/ipa/ipu3/ipu3.cpp | 1 + >> 3 files changed, 7 insertions(+) >> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp >> index 7152d070d3ab1bc463fdaad437d5e1c1b87ce25c..7fcfd5e0e4ade92521cc2914dd07113235af8e45 100644 >> --- a/src/ipa/ipu3/ipa_context.cpp >> +++ b/src/ipa/ipu3/ipa_context.cpp >> @@ -54,6 +54,9 @@ namespace libcamera::ipa::ipu3 { >> * >> * \var IPAContext::ctrlMap >> * \brief A ControlInfoMap::Map of controls populated by the algorithms >> + * >> + * \var IPAContext::sensorInfo >> + * \brief An IPACameraSensorInfo instance holding information about the sensor >> */ >> >> /** >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h >> index 1eaaac82da0e3ad5bed0749c39d9dad8c585cab0..f157f223cbb3119f108d768b14fca514ac5661ca 100644 >> --- a/src/ipa/ipu3/ipa_context.h >> +++ b/src/ipa/ipu3/ipa_context.h >> @@ -15,6 +15,8 @@ >> #include <libcamera/controls.h> >> #include <libcamera/geometry.h> >> >> +#include <libcamera/ipa/core_ipa_interface.h> >> + >> #include <libipa/awb.h> >> #include <libipa/ccm.h> >> #include <libipa/fc_queue.h> >> @@ -93,6 +95,7 @@ struct IPAContext { >> FCQueue<IPAFrameContext> frameContexts; >> >> ControlInfoMap::Map ctrlMap; >> + IPACameraSensorInfo sensorInfo; >> }; >> >> } /* namespace ipa::ipu3 */ >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >> index 4bdc4b7677fe9703fb968ca91967c68742454514..0827ad207c9d89b8af4887f609fe08881612ab96 100644 >> --- a/src/ipa/ipu3/ipu3.cpp >> +++ b/src/ipa/ipu3/ipu3.cpp >> @@ -313,6 +313,7 @@ int IPAIPU3::init(const IPASettings &settings, >> context_.configuration = {}; >> context_.configuration.sensor.lineDuration = >> sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate; >> + context_.sensorInfo = sensorInfo; > > There is already a `sensorInfo_` member in `IPAIPU3`. Could we replace > that with this one? Yes I think so...in fact when it's used in ::configure() it just repeats the operations here. I'll drop them from ::configure() and remove that member in the v2 of this patch > > >> >> /* Load the tuning data file. */ >> File file(settings.configurationFile); >> >> -- >> 2.43.0 >> >
Quoting Dan Scally (2026-06-19 10:38:25) > Hi Barnabas > > >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > >> index 4bdc4b7677fe9703fb968ca91967c68742454514..0827ad207c9d89b8af4887f609fe08881612ab96 100644 > >> --- a/src/ipa/ipu3/ipu3.cpp > >> +++ b/src/ipa/ipu3/ipu3.cpp > >> @@ -313,6 +313,7 @@ int IPAIPU3::init(const IPASettings &settings, > >> context_.configuration = {}; > >> context_.configuration.sensor.lineDuration = > >> sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate; > >> + context_.sensorInfo = sensorInfo; > > > > There is already a `sensorInfo_` member in `IPAIPU3`. Could we replace > > that with this one? > > Yes I think so...in fact when it's used in ::configure() it just repeats the operations here. I'll > drop them from ::configure() and remove that member in the v2 of this patch Check before dropping if they are updating based on an new sensor configuration! configure phase is when we find out what the user has configured the sensor to specifically... -- Kieran > > > > > >> > >> /* Load the tuning data file. */ > >> File file(settings.configurationFile); > >> > >> -- > >> 2.43.0 > >> > > >
Hi Kieran, Barnabas On 19/06/2026 13:04, Kieran Bingham wrote: > Quoting Dan Scally (2026-06-19 10:38:25) >> Hi Barnabas >> >>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >>>> index 4bdc4b7677fe9703fb968ca91967c68742454514..0827ad207c9d89b8af4887f609fe08881612ab96 100644 >>>> --- a/src/ipa/ipu3/ipu3.cpp >>>> +++ b/src/ipa/ipu3/ipu3.cpp >>>> @@ -313,6 +313,7 @@ int IPAIPU3::init(const IPASettings &settings, >>>> context_.configuration = {}; >>>> context_.configuration.sensor.lineDuration = >>>> sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate; >>>> + context_.sensorInfo = sensorInfo; >>> >>> There is already a `sensorInfo_` member in `IPAIPU3`. Could we replace >>> that with this one? >> >> Yes I think so...in fact when it's used in ::configure() it just repeats the operations here. I'll >> drop them from ::configure() and remove that member in the v2 of this patch > > Check before dropping if they are updating based on an new sensor > configuration! > > configure phase is when we find out what the user has configured the > sensor to specifically... Kieran is right; the sensorInfo_ member of the class is checked for the applied format size, whereas libcamera::ipa::LscAlgorithm::init() function wants to know the active array size. However, we can just update the sensorInfo in context at configure time, and that should be fine. > > -- > Kieran > >>> >>> >>>> >>>> /* Load the tuning data file. */ >>>> File file(settings.configurationFile); >>>> >>>> -- >>>> 2.43.0 >>>> >>> >>
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index 7152d070d3ab1bc463fdaad437d5e1c1b87ce25c..7fcfd5e0e4ade92521cc2914dd07113235af8e45 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -54,6 +54,9 @@ namespace libcamera::ipa::ipu3 { * * \var IPAContext::ctrlMap * \brief A ControlInfoMap::Map of controls populated by the algorithms + * + * \var IPAContext::sensorInfo + * \brief An IPACameraSensorInfo instance holding information about the sensor */ /** diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 1eaaac82da0e3ad5bed0749c39d9dad8c585cab0..f157f223cbb3119f108d768b14fca514ac5661ca 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -15,6 +15,8 @@ #include <libcamera/controls.h> #include <libcamera/geometry.h> +#include <libcamera/ipa/core_ipa_interface.h> + #include <libipa/awb.h> #include <libipa/ccm.h> #include <libipa/fc_queue.h> @@ -93,6 +95,7 @@ struct IPAContext { FCQueue<IPAFrameContext> frameContexts; ControlInfoMap::Map ctrlMap; + IPACameraSensorInfo sensorInfo; }; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 4bdc4b7677fe9703fb968ca91967c68742454514..0827ad207c9d89b8af4887f609fe08881612ab96 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -313,6 +313,7 @@ int IPAIPU3::init(const IPASettings &settings, context_.configuration = {}; context_.configuration.sensor.lineDuration = sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate; + context_.sensorInfo = sensorInfo; /* Load the tuning data file. */ File file(settings.configurationFile);
Add an instance of IPACameraSensorInfo to the IPAContext and fill it during IPAIPU3::init(). This will allow us to pass sensor information to algorithms at init time. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- src/ipa/ipu3/ipa_context.cpp | 3 +++ src/ipa/ipu3/ipa_context.h | 3 +++ src/ipa/ipu3/ipu3.cpp | 1 + 3 files changed, 7 insertions(+)