[libcamera-devel] [PATCH 3/4] ipa: ipu3: Copy IPACameraSensorInfo for future usage
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Tue Jun 8 07:31:54 CEST 2021
Hi Laurent, Umang,
On 08/06/2021 01:00, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Wed, Jun 02, 2021 at 03:53:25PM +0530, Umang Jain wrote:
>> IPACameraSensorInfo members will be needed at various places in the
>> IPAIPU3 class, in subsequent commits. Hence, it seems trivial to copy
>> this structure for wider availability throughout the class.
>>
>> This commit does not introduce any functional changes.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>> src/ipa/ipu3/ipu3.cpp | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 2496b0a0..97ddb863 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -63,6 +63,8 @@ private:
>>
>> ControlInfoMap ctrls_;
>>
>> + IPACameraSensorInfo sensorInfo_;
>
> This is OK for now, but in the not too distant future I think we should
> store fields individually as we should probably convert them. For
> instance, the line duration should be converted to a time unit when we
> receive it, and then used as such, instead of converting between pixels
> and time in multiple places.
>
> Jean-Michel, how about moving IPU3Agc to handling exposure time in time
> units, and doing the conversion in ipu3.cpp ? Is this something you have
> considered, or maybe you're already planning it ?
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I indeed have a local branch where exposure is a time unit and the line
duration is calculated in the agcAlgo_->initialise() call.
Those ctrls disappear from ipu3.cpp except for setting those in
setControls().
There is no need for a local variable in IPAIPU3 as IPU3Agc receives the
configInfo reference directly.
Would you prefer that new patch instead ? Or we can go with this one for
the moment and I will fix it when I will post the new agc algorithm series ?
>> +
>> /* Camera sensor controls. */
>> uint32_t exposure_;
>> uint32_t minExposure_;
>> @@ -144,6 +146,8 @@ void IPAIPU3::configure(const IPAConfigInfo &configInfo)
>> if (configInfo.entityControls.empty())
>> return;
>>
>> + sensorInfo_ = configInfo.sensorInfo;
>> +
>> ctrls_ = configInfo.entityControls.at(0);
>>
>> const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
>> @@ -174,7 +178,7 @@ void IPAIPU3::configure(const IPAConfigInfo &configInfo)
>> awbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);
>>
>> agcAlgo_ = std::make_unique<IPU3Agc>();
>> - agcAlgo_->initialise(bdsGrid_, configInfo.sensorInfo);
>> + agcAlgo_->initialise(bdsGrid_, sensorInfo_);
>> }
>>
>> void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
>
More information about the libcamera-devel
mailing list