[libcamera-devel] [PATCH 2/2] ipa: ipu3: Consolidate querying exposure and gain limits
Umang Jain
umang.jain at ideasonboard.com
Mon Feb 28 14:52:19 CET 2022
Hi Kieran,
On 2/28/22 19:03, Kieran Bingham wrote:
> Hi Umang,
>
> Quoting Umang Jain (2022-02-28 13:22:50)
>> The exposure and gain limits are queried as part of
>> IPAIPU3::configure(). Consolidate it.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>> src/ipa/ipu3/ipu3.cpp | 44 +++++++++++++------------------------------
>> 1 file changed, 13 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 2fab4ee0..33fcf59b 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -196,12 +196,14 @@ private:
>> void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>> {
>> const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
>> - int32_t minExposure = v4l2Exposure.min().get<int32_t>();
>> - int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
>> + minExposure_ = v4l2Exposure.min().get<int32_t>();
>> + maxExposure_ = v4l2Exposure.max().get<int32_t>();
>> + exposure_ = minExposure_;
> Are the class members used elsewhere at all? (I.e. exposure_ etc?) or
> could they be removed leaving the state stored only in
> context_.configuration?
The class members dealing with min/max (i.e. static values) could be
brought under there, yes. I don't think the class members exposure_ and
gain_ can be brought under the umbrella of IPASessionConfiguration since
they reflect the latest values computed by IPA
>
> I don't think it's in yet, but I'm sure one of JM's patches adds state
> to a IPASessionConfiguration.sensor structure to keep sensor specfic
> state tied together in the context.
yes, I see [PATCH v4 3/4] ipa: ipu3: agc: Introduce lineDuration in
IPASessionConfiguration
We can have min/max exposure/gain under them I think
>
> These properties might really belong in there.
>
>> const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
>> - int32_t minGain = v4l2Gain.min().get<int32_t>();
>> - int32_t maxGain = v4l2Gain.max().get<int32_t>();
>> + minGain_ = v4l2Gain.min().get<int32_t>();
>> + maxGain_ = v4l2Gain.max().get<int32_t>();
>> + gain_ = minGain_;
>>
>> /*
>> * When the AGC computes the new exposure values for a frame, it needs
>> @@ -210,10 +212,10 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>> *
>> * \todo take VBLANK into account for maximum shutter speed
>> */
>> - context_.configuration.agc.minShutterSpeed = minExposure * lineDuration_;
>> - context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration_;
>> - context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>> - context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>> + context_.configuration.agc.minShutterSpeed = minExposure_ * lineDuration_;
>> + context_.configuration.agc.maxShutterSpeed = maxExposure_ * lineDuration_;
>> + context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
>> + context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
>> }
>>
>> /**
>> @@ -430,32 +432,12 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>> */
>> ctrls_ = configInfo.sensorControls;
>>
>> - const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
>> - if (itExp == ctrls_.end()) {
>> - LOG(IPAIPU3, Error) << "Can't find exposure control";
>> - return -EINVAL;
>> - }
>> -
>> - const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
>> - if (itGain == ctrls_.end()) {
>> - LOG(IPAIPU3, Error) << "Can't find gain control";
>> - return -EINVAL;
>> - }
>> -
>> const auto itVBlank = ctrls_.find(V4L2_CID_VBLANK);
>> if (itVBlank == ctrls_.end()) {
>> LOG(IPAIPU3, Error) << "Can't find VBLANK control";
>> return -EINVAL;
>> }
>>
>> - minExposure_ = itExp->second.min().get<int32_t>();
>> - maxExposure_ = itExp->second.max().get<int32_t>();
>> - exposure_ = minExposure_;
>> -
>> - minGain_ = itGain->second.min().get<int32_t>();
>> - maxGain_ = itGain->second.max().get<int32_t>();
>> - gain_ = minGain_;
>> -
>> defVBlank_ = itVBlank->second.def().get<int32_t>();
>>
>> /* Clean context at configuration */
>> @@ -465,12 +447,12 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>>
>> lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
>>
>> - /* Update the camera controls using the new sensor settings. */
>> - updateControls(sensorInfo_, ctrls_, ipaControls);
>> -
>> /* Update the IPASessionConfiguration using the sensor settings. */
>> updateSessionConfiguration(ctrls_);
>>
>> + /* Update the camera controls using the new sensor settings. */
>> + updateControls(sensorInfo_, ctrls_, ipaControls);
>> +
>> for (auto const &algo : algorithms_) {
>> int ret = algo->configure(context_, configInfo);
>> if (ret)
>> --
>> 2.31.1
>>
More information about the libcamera-devel
mailing list