| Message ID | 20251011063333.2169364-3-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 10. 11. 8:33 keltezéssel, Paul Elder írta: > The ControlInfos of non-scalar controls that are reported in controls() > must have non-scalar default values for controls that have a defined > size. This is because applications should be able to directly set the > default value from a ControlInfo to the control. Now I am wondering if the same should be true for dynamically size arrays, of which there is only one: `AfWindows`. But currently its default value is set to a scalar value. > > Currently this is relevant to the following controls: > - ColourGains > - ColourCorrectionMatrix > - FrameDurationLimits > > Fix the scalarness of these controls where relevant. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > Changes in v4: > - improve commit message > > No change in v3 > > No change in v2 > --- > 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 }); Small thing, here and everywhere else, the array can created "on the fly": Span<const int64_t>{{ frameDurations[2], frameDurations[2] }} Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # rkisp1 >>> c.controls[lc.controls.FrameDurationLimits] libcamera.ControlInfo([21066..1241564]) >>> c.controls[lc.controls.FrameDurationLimits].default (33400, 33400) >>> c.controls[lc.controls.ColourCorrectionMatrix] libcamera.ControlInfo([-8.000000..7.993000]) >>> c.controls[lc.controls.ColourCorrectionMatrix].default (1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0) Regards, Barnabás Pőcze > > controls.merge(context_.ctrlMap); > *ipaControls = ControlInfoMap(std::move(controls), controls::controls); > [...]
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 fa22bfc34904..3f098610a06a 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -436,10 +436,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 8dfe35cc3267..67dab6cd970c 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 non-scalar default values for controls that have a defined size. This is because applications should be able to directly set the default value from a ControlInfo to the control. Currently this is relevant to the following controls: - ColourGains - ColourCorrectionMatrix - FrameDurationLimits Fix the scalarness of these controls where relevant. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v4: - improve commit message No change in v3 No change in v2 --- 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(-)