| Message ID | 20250910093539.3216782-3-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 09. 10. 11:35 keltezéssel, Paul Elder írta: > The ControlInfos of non-scalar controls that are reported in controls() > must have scalar min/max and non-scalar default values for controls that > have a defined size. Currently this is relevant to the following > controls: > - ColourGains > - ColourCorrectionMatrix > - FrameDurationLimits > > A mismatch of scalarness in these fields causes deserialization errors > in ControlSerializer which manifest when IPAs are run in isolation. > > Fix the scalarness of these controls where relevant. Related to https://bugs.libcamera.org/show_bug.cgi?id=101. Maybe the time has finally come to define a concrete policy about the default values of array-like controls. Regards, Barnabás Pőcze > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/ipu3/ipu3.cpp | 4 +++- > src/ipa/mali-c55/mali-c55.cpp | 5 ++++- > src/ipa/rkisp1/algorithms/awb.cpp | 5 ++++- > src/ipa/rkisp1/rkisp1.cpp | 4 +++- > src/ipa/rpi/common/ipa_base.cpp | 7 ++++++- > 5 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 1cae08bf255f..e71639a16522 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -20,6 +20,7 @@ > > #include <libcamera/base/file.h> > #include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > #include <libcamera/control_ids.h> > @@ -280,10 +281,11 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo, > uint64_t frameSize = lineLength * frameHeights[i]; > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); > } > + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; > > controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], > frameDurations[1], > - frameDurations[2]); > + Span<const int64_t, 2>{ defFrameDurations }); > > controls.merge(context_.ctrlMap); > *ipaControls = ControlInfoMap(std::move(controls), controls::controls); > diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp > index 7d45e7310aec..c63d3b2bb7be 100644 > --- a/src/ipa/mali-c55/mali-c55.cpp > +++ b/src/ipa/mali-c55/mali-c55.cpp > @@ -5,6 +5,7 @@ > * Mali-C55 ISP image processing algorithms > */ > > +#include <array> > #include <map> > #include <string.h> > #include <vector> > @@ -14,6 +15,7 @@ > > #include <libcamera/base/file.h> > #include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > > #include <libcamera/control_ids.h> > #include <libcamera/ipa/ipa_interface.h> > @@ -234,9 +236,10 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo, > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); > } > > + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; > ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], > frameDurations[1], > - frameDurations[2]); > + Span<const int64_t, 2>{ defFrameDurations }); > > /* > * Compute exposure time limits from the V4L2_CID_EXPOSURE control > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 399fb51be414..a664011a9f0d 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -91,7 +91,10 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData) > kMaxColourTemperature, > kDefaultColourTemperature); > cmap[&controls::AwbEnable] = ControlInfo(false, true); > - cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, 1.0f); > + > + std::array<float, 2> defColourGains = { 1.0f, 1.0f }; > + cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, > + Span<const float, 2>{ defColourGains }); > > if (!tuningData.contains("algorithm")) > LOG(RkISP1Awb, Info) << "No AWB algorithm specified." > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index cf66d5553dcd..6eb3ae9c6310 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -430,10 +430,12 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, > uint64_t frameSize = lineLength * frameHeights[i]; > frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); > } > + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; > > /* \todo Move this (and other agc-related controls) to agc */ > context_.ctrlMap[&controls::FrameDurationLimits] = > - ControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]); > + ControlInfo(frameDurations[0], frameDurations[1], > + ControlValue(Span<const int64_t, 2>{ defFrameDurations })); > > ctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end()); > *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 6448e6abd735..e34e890f0bdc 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -7,6 +7,7 @@ > > #include "ipa_base.h" > > +#include <array> > #include <cmath> > > #include <libcamera/base/log.h> > @@ -243,10 +244,14 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa > * based on the current sensor mode. > */ > ControlInfoMap::Map ctrlMap = ipaControls; > + std::array<int64_t, 2> defFrameDurations = { > + static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()), > + static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()) > + }; > ctrlMap[&controls::FrameDurationLimits] = > ControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()), > static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()), > - static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>())); > + Span<const int64_t, 2>{ defFrameDurations }); > > ctrlMap[&controls::AnalogueGain] = > ControlInfo(static_cast<float>(mode_.minAnalogueGain),
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 1cae08bf255f..e71639a16522 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -20,6 +20,7 @@ #include <libcamera/base/file.h> #include <libcamera/base/log.h> +#include <libcamera/base/span.h> #include <libcamera/base/utils.h> #include <libcamera/control_ids.h> @@ -280,10 +281,11 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo, uint64_t frameSize = lineLength * frameHeights[i]; frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], frameDurations[1], - frameDurations[2]); + Span<const int64_t, 2>{ defFrameDurations }); controls.merge(context_.ctrlMap); *ipaControls = ControlInfoMap(std::move(controls), controls::controls); diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp index 7d45e7310aec..c63d3b2bb7be 100644 --- a/src/ipa/mali-c55/mali-c55.cpp +++ b/src/ipa/mali-c55/mali-c55.cpp @@ -5,6 +5,7 @@ * Mali-C55 ISP image processing algorithms */ +#include <array> #include <map> #include <string.h> #include <vector> @@ -14,6 +15,7 @@ #include <libcamera/base/file.h> #include <libcamera/base/log.h> +#include <libcamera/base/span.h> #include <libcamera/control_ids.h> #include <libcamera/ipa/ipa_interface.h> @@ -234,9 +236,10 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo, frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0], frameDurations[1], - frameDurations[2]); + Span<const int64_t, 2>{ defFrameDurations }); /* * Compute exposure time limits from the V4L2_CID_EXPOSURE control diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 399fb51be414..a664011a9f0d 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -91,7 +91,10 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData) kMaxColourTemperature, kDefaultColourTemperature); cmap[&controls::AwbEnable] = ControlInfo(false, true); - cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, 1.0f); + + std::array<float, 2> defColourGains = { 1.0f, 1.0f }; + cmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f, + Span<const float, 2>{ defColourGains }); if (!tuningData.contains("algorithm")) LOG(RkISP1Awb, Info) << "No AWB algorithm specified." diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index cf66d5553dcd..6eb3ae9c6310 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -430,10 +430,12 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, uint64_t frameSize = lineLength * frameHeights[i]; frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U); } + std::array<int64_t, 2> defFrameDurations = { frameDurations[2], frameDurations[2] }; /* \todo Move this (and other agc-related controls) to agc */ context_.ctrlMap[&controls::FrameDurationLimits] = - ControlInfo(frameDurations[0], frameDurations[1], frameDurations[2]); + ControlInfo(frameDurations[0], frameDurations[1], + ControlValue(Span<const int64_t, 2>{ defFrameDurations })); ctrlMap.insert(context_.ctrlMap.begin(), context_.ctrlMap.end()); *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 6448e6abd735..e34e890f0bdc 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -7,6 +7,7 @@ #include "ipa_base.h" +#include <array> #include <cmath> #include <libcamera/base/log.h> @@ -243,10 +244,14 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa * based on the current sensor mode. */ ControlInfoMap::Map ctrlMap = ipaControls; + std::array<int64_t, 2> defFrameDurations = { + static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()), + static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()) + }; ctrlMap[&controls::FrameDurationLimits] = ControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()), static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()), - static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>())); + Span<const int64_t, 2>{ defFrameDurations }); ctrlMap[&controls::AnalogueGain] = ControlInfo(static_cast<float>(mode_.minAnalogueGain),
The ControlInfos of non-scalar controls that are reported in controls() must have scalar min/max and non-scalar default values for controls that have a defined size. Currently this is relevant to the following controls: - ColourGains - ColourCorrectionMatrix - FrameDurationLimits A mismatch of scalarness in these fields causes deserialization errors in ControlSerializer which manifest when IPAs are run in isolation. Fix the scalarness of these controls where relevant. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/ipa/ipu3/ipu3.cpp | 4 +++- src/ipa/mali-c55/mali-c55.cpp | 5 ++++- src/ipa/rkisp1/algorithms/awb.cpp | 5 ++++- src/ipa/rkisp1/rkisp1.cpp | 4 +++- src/ipa/rpi/common/ipa_base.cpp | 7 ++++++- 5 files changed, 20 insertions(+), 5 deletions(-)