<div dir="ltr"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div dir="ltr"><div style="font-size:12.8px"><div style="color:rgb(136,136,136);font-size:12.8px"><div style="font-size:small"><span style="color:rgb(34,34,34)">>  cp utils/hooks/</span><span class="gmail-il" style="color:rgb(34,34,34)">post</span><span style="color:rgb(34,34,34)">-</span><span class="gmail-il" style="color:rgb(34,34,34)">commit</span><span style="color:rgb(34,34,34)"> .git/hooks/</span><span class="gmail-il" style="color:rgb(34,34,34)">post</span><span style="color:rgb(34,34,34)">-</span><span class="gmail-il" style="color:rgb(34,34,34)">commit</span><font face="arial, helvetica, sans-serif" color="#000000"><br></font></div><div style="font-size:small"><span class="gmail-il" style="color:rgb(34,34,34)">The post-commits seem to want me to replace my tabs with spaces. I use tabs to be consistent with the surrounding codebase. Is it OK to ignore these errors, or should I do something else?</span></div><div style="font-size:small"><span class="gmail-il" style="color:rgb(34,34,34)"><br></span></div><div style="font-size:small"><span class="gmail-il" style="color:rgb(34,34,34)">Example:</span></div></div></div></div></div></div></div>--- src/android/camera_hal_config.cpp<br>+++ src/android/camera_hal_config.cpp<br>@@ -164,7 +164,7 @@<br>        File file(filePath);<br>        if (!file.exist()) {<br>                LOG(HALConfig, Debug)<br>-                                       << "Configuration file: \"" << filePath << "\" not found";<br>+                       << "Configuration file: \"" << filePath << "\" not found";<br>                return -ENOENT;<br>        }<br><br>---<br>1 potential issue detected, please review<br><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 25, 2022 at 6:14 AM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Nicholas Roth via libcamera-devel (2022-10-25 04:15:36)<br>
> > Could I ask you to read <a href="https://cbea.ms/git-commit/" rel="noreferrer" target="_blank">https://cbea.ms/git-commit/</a> please?<br>
> Read. I thought that was quite informative. I noticed some differences<br>
> between the style suggested in the blog post and the commit messages you<br>
> suggest. Indeed, I have generally foregone good history in favor of<br>
<br>
Style differences are fine. Content is King. We won't merge patches that<br>
say "Latest code" or "fixes foo". I like to break things down to cover<br>
three points, but it doesn't always apply. Capturing rationale is the<br>
important part.<br>
<br>
<br>
prefix: of: components: Clear subject<br>
<br>
 - What is wrong<br>
 - What this patch does<br>
 - Why it does it (the way it does)<br>
<br>
Signed-off-by: <author><br>
<br>
<br>
<br>
 Subject: ships: argo: Increase sailers on the Argo<br>
<br>
 The Argo is insuffienctly staffed and incapable of slaying any<br>
 dragons, (or managing any other adventures accordingly).<br>
<br>
 To avoid mortal peril beyond expectations, staff the Argo with at<br>
 least 50 of the finest Heroes available.<br>
<br>
 The target of 50 has been reached according to the size of the vessel,<br>
 and the number of beds available for resting during the journey, as<br>
 well as examining the food supplies that are expected to last over the<br>
 extended travel period.<br>
<br>
 The return Journey may have a reduced staffing level, but this is not<br>
 desired. Staffing levels should be kept as close to the maximum level<br>
 as possible at all times.<br>
<br>
 Signed-off-by: Jason of Aeson <<a href="mailto:jason@argonauts.org" target="_blank">jason@argonauts.org</a>><br>
 ---<br>
 diff --git a/ships/argo b/ships/argo<br>
 index c9a40e8788e7..e257b2e257b4 100644<br>
 --- a/ships/argo<br>
 +++ b/ships/argo<br>
 @@ -1 +1 @@<br>
 -staff-level: 0<br>
 +staff-level: 50<br>
<br>
<br>
<br>
Why do I have a sudden urge to write an anthology of mythological<br>
stories in patch form ?<br>
<br>
<br>
> convenience in git projects, and I hope it will be interesting to<br>
> experience the advantages of clear history and educational to learn how<br>
> this looks in a Git project.<br>
<br>
Please feel free to look through the commit history of libcamera. We're<br>
over 4000 commits now, and I would really hope that a vast majority<br>
would be considered as good examples of commits. (we do allow some slack<br>
at times... - justifying a typo is usually quite short :D )<br>
<br>
 - <a href="https://git.libcamera.org/libcamera/libcamera.git/log/" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/log/</a><br>
<br>
We mostly come with a background (and continuation) in Linux Kernel<br>
development, where keeping this history is cruicial due to the scale of<br>
the kernel. It could be argued that it's not crucial to the same level<br>
at libcamera, but I believe it's good development practice to maintain<br>
clear commits throughout.<br>
<br>
The commit messages usually come into their own when bisecting failures,<br>
or looking through git-blame to see /why/ a line of code is the way it<br>
is.<br>
<br>
Ah yes, also we expect bisectability - so each commit should compile and<br>
be functional as an independent task, while aiming to do one thing per<br>
patch.<br>
<br>
<br>
> This might also be a good place to ask about a style guide for your code.<br>
> I've done my best to be consistent with surrounding code, but it would be<br>
> helpful to have something more concrete, maybe even someone's .vimrc if<br>
> that's available.<br>
<br>
We have tooling in place to help already.<br>
 - <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/utils/hooks/post-commit" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/utils/hooks/post-commit</a><br>
<br>
To utilise this hook, install this file with:<br>
  cp utils/hooks/post-commit .git/hooks/post-commit<br>
<br>
That will run our commit style checks on every commit.<br>
I prefer doing it as a post-commit so you can still commit with<br>
failures. pre-commit is usually too 'harsh'.<br>
<br>
Beyond that, we have some documentation on the subject at:<br>
 - <a href="https://libcamera.org/contributing.html" rel="noreferrer" target="_blank">https://libcamera.org/contributing.html</a><br>
and<br>
 - <a href="https://libcamera.org/coding-style.html" rel="noreferrer" target="_blank">https://libcamera.org/coding-style.html</a><br>
<br>
But I'm sure this is always something that needs more work on<br>
documenting, or making it easier to read/find.<br>
<br>
<br>
<br>
> > I wonder if it would be possible to overload "operator /" for the<br>
> utils::Duration type<br>
> Gives an error: multiple definitions.<br>
> > Or, if that can't be done for the Duration type only, somehow override<br>
> the std::chrono::duration::operator/() globally<br>
> In global namespace: compiler error<br>
<br>
What's the compiler error here ?<br>
<br>
<br>
> In the class itself: read on<br>
> <br>
> My power is out but I’ll follow up with the actual compiler errors sometime<br>
> tomorrow.<br>
> <br>
> I’ve come up with a centralized solution that might work, but it’s not<br>
> ideal either. I’d like to get your thoughts here before going further:<br>
> * Make the chrono baseclass private to resolve ambiguity<br>
> * Implement all of the usual operators i.e. +, -, /, * inside the class<br>
> (even friend functions can’t see private base classes, oddly enough)<br>
> <br>
> This might also involve some template metaprogramming but I’m trying hard<br>
> to do without it.<br>
<br>
Ayee, this sounds like more work ... which is painful. I'd be interested<br>
to see what the compiler error is for the operator/() is to see if<br>
that's a shorter solution.<br>
<br>
--<br>
Kieran<br>
<br>
<br>
> <br>
> -Nicholas<br>
> <br>
> <br>
> On Mon, Oct 24, 2022 at 8:30 AM Laurent Pinchart <<br>
> <a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br>
> <br>
> > On Mon, Oct 24, 2022 at 11:50:53AM +0100, Naushir Patuck via<br>
> > libcamera-devel wrote:<br>
> > > Hi Nicholas,<br>
> > ><br>
> > > Thank you for your patch.<br>
> > ><br>
> > > As you've already noted, this removes much of the niceness of using<br>
> > > std::chrono<br>
> > > types, and impacts code readability.<br>
> > ><br>
> > > I wonder if it would be possible to overload "operator /" for the<br>
> > utils::Duration type<br>
> > > to work-around this bug instead? That way majority of the code need not<br>
> > change<br>
> > > and the fix only lives in one place in the codebase making it easier to<br>
> > revert when<br>
> > > the time comes.<br>
> ><br>
> > Or, if that can't be done for the Duration type only, somehow override<br>
> > the std::chrono::duration::operator/() globally, with a version that<br>
> > fixes the bug. I think this change is fairly intrusive, I'd like a more<br>
> > centralized solution that will not require patching every division.<br>
> ><br>
> > > On Mon, 24 Oct 2022 at 06:55, Nicholas Roth via libcamera-devel wrote:<br>
> > ><br>
> > > > From: Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
> > > ><br>
> > > > ---<br>
> > > >  src/ipa/ipu3/algorithms/agc.cpp            | 18 ++++++++++++++----<br>
> > > >  src/ipa/raspberrypi/cam_helper.cpp         |  9 ++++++---<br>
> > > >  src/ipa/raspberrypi/cam_helper_imx296.cpp  |  5 ++++-<br>
> > > >  src/ipa/raspberrypi/controller/rpi/agc.cpp | 18 ++++++++++++++----<br>
> > > >  src/ipa/raspberrypi/controller/rpi/lux.cpp |  5 ++++-<br>
> > > >  src/ipa/rkisp1/algorithms/agc.cpp          | 22 ++++++++++++++++++----<br>
> > > >  6 files changed, 60 insertions(+), 17 deletions(-)<br>
> > > ><br>
> > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp<br>
> > b/src/ipa/ipu3/algorithms/agc.cpp<br>
> > > > index a1a3c38f..80c551bb 100644<br>
> > > > --- a/src/ipa/ipu3/algorithms/agc.cpp<br>
> > > > +++ b/src/ipa/ipu3/algorithms/agc.cpp<br>
> > > > @@ -100,7 +100,10 @@ int Agc::configure(IPAContext &context,<br>
> > > ><br>
> > > >         /* Configure the default exposure and gain. */<br>
> > > >         activeState.agc.gain = std::max(minAnalogueGain_,<br>
> > kMinAnalogueGain);<br>
> > > > -       activeState.agc.exposure = 10ms /<br>
> > configuration.sensor.lineDuration;<br>
> > > > +       /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +       double ten_millis = utils::Duration(10ms).get<std::nano>();<br>
> > > > +       activeState.agc.exposure = ten_millis /<br>
> > > > +               configuration.sensor.lineDuration.get<std::nano>();<br>
> > > ><br>
> > > >         frameCount_ = 0;<br>
> > > >         return 0;<br>
> > > > @@ -238,11 +241,16 @@ void Agc::computeExposure(IPAContext &context,<br>
> > IPAFrameContext &frameContext,<br>
> > > >          *<br>
> > > >          * Push the shutter time up to the maximum first, and only then<br>
> > > >          * increase the gain.<br>
> > > > +        *<br>
> > > > +        * TODO(Bug 156): Workaround for LLVM bug.<br>
> > > >          */<br>
> > > > +       double exposureValueDouble = exposureValue.get<std::nano>();<br>
> > > > +       utils::Duration shutterTimeRaw(exposureValueDouble /<br>
> > minAnalogueGain_);<br>
> > > >         utils::Duration shutterTime =<br>
> > > > -               std::clamp<utils::Duration>(exposureValue /<br>
> > minAnalogueGain_,<br>
> > > > +               std::clamp<utils::Duration>(shutterTimeRaw,<br>
> > > >                                             minShutterSpeed_,<br>
> > maxShutterSpeed_);<br>
> > > > -       double stepGain = std::clamp(exposureValue / shutterTime,<br>
> > > > +       double shutterTimeDouble = shutterTime.get<std::nano>();<br>
> > > > +       double stepGain = std::clamp(exposureValueDouble /<br>
> > shutterTimeDouble,<br>
> > > >                                      minAnalogueGain_,<br>
> > maxAnalogueGain_);<br>
> > > >         LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "<br>
> > > >                             << shutterTime << " and "<br>
> > > > @@ -250,7 +258,9 @@ void Agc::computeExposure(IPAContext &context,<br>
> > IPAFrameContext &frameContext,<br>
> > > ><br>
> > > >         IPAActiveState &activeState = context.activeState;<br>
> > > >         /* Update the estimated exposure and gain. */<br>
> > > > -       activeState.agc.exposure = shutterTime /<br>
> > configuration.sensor.lineDuration;<br>
> > > > +       /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +       double lineDurationDouble =<br>
> > configuration.sensor.lineDuration.get<std::nano>();<br>
> > > > +       activeState.agc.exposure = shutterTimeDouble /<br>
> > lineDurationDouble;<br>
> > > >         activeState.agc.gain = stepGain;<br>
> > > >  }<br>
> > > ><br>
> > > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp<br>
> > b/src/ipa/raspberrypi/cam_helper.cpp<br>
> > > > index d90ac1de..31a9a1ef 100644<br>
> > > > --- a/src/ipa/raspberrypi/cam_helper.cpp<br>
> > > > +++ b/src/ipa/raspberrypi/cam_helper.cpp<br>
> > > > @@ -63,7 +63,8 @@ void CamHelper::process([[maybe_unused]]<br>
> > StatisticsPtr<br>
> > > > &stats,<br>
> > > ><br>
> > > >  uint32_t CamHelper::exposureLines(const Duration exposure, const<br>
> > Duration<br>
> > > > lineLength) const<br>
> > > >  {<br>
> > > > -       return exposure / lineLength;<br>
> > > > +       /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +       return exposure.get<std::nano>() / lineLength.get<std::nano>();<br>
> > > >  }<br>
> > > ><br>
> > > >  Duration CamHelper::exposure(uint32_t exposureLines, const Duration<br>
> > lineLength) const<br>
> > > > @@ -84,9 +85,11 @@ std::pair<uint32_t, uint32_t><br>
> > CamHelper::getBlanking(Duration &exposure,<br>
> > > >          *<br>
> > > >          * frameLengthMax gets calculated on the smallest line length<br>
> > as we do<br>
> > > >          * not want to extend that unless absolutely necessary.<br>
> > > > +        *<br>
> > > > +        * TODO(Bug 156): Workaround for LLVM bug.<br>
> > > >          */<br>
> > > > -       frameLengthMin = minFrameDuration / mode_.minLineLength;<br>
> > > > -       frameLengthMax = maxFrameDuration / mode_.minLineLength;<br>
> > > > +       frameLengthMin = minFrameDuration.get<std::nano>() /<br>
> > mode_.minLineLength.get<std::nano>();<br>
> > > > +       frameLengthMax = maxFrameDuration.get<std::nano>() /<br>
> > mode_.minLineLength.get<std::nano>();<br>
> > > ><br>
> > > >         /*<br>
> > > >          * Watch out for (exposureLines + frameIntegrationDiff_)<br>
> > overflowing a<br>
> > > > diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp<br>
> > b/src/ipa/raspberrypi/cam_helper_imx296.cpp<br>
> > > > index ecb845e7..e48f5cf2 100644<br>
> > > > --- a/src/ipa/raspberrypi/cam_helper_imx296.cpp<br>
> > > > +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp<br>
> > > > @@ -57,7 +57,10 @@ double CamHelperImx296::gain(uint32_t gainCode)<br>
> > const<br>
> > > >  uint32_t CamHelperImx296::exposureLines(const Duration exposure,<br>
> > > >                                         [[maybe_unused]] const<br>
> > Duration lineLength) const<br>
> > > >  {<br>
> > > > -       return std::max<uint32_t>(minExposureLines, (exposure -<br>
> > 14.26us) / timePerLine);<br>
> > > > +       /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +       double exposureTime = Duration(exposure -<br>
> > 14.26us).get<std::nano>();<br>
> > > > +       double timePerLineNano = timePerLine.get<std::nano>();<br>
> > > > +       return std::max<uint32_t>(minExposureLines, exposureTime /<br>
> > timePerLineNano);<br>
> > > >  }<br>
> > > ><br>
> > > >  Duration CamHelperImx296::exposure(uint32_t exposureLines,<br>
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> > b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> > > > index bd54a639..720ba788 100644<br>
> > > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> > > > @@ -418,7 +418,10 @@ void Agc::prepare(Metadata *imageMetadata)<br>
> > > >                         Duration actualExposure =<br>
> > deviceStatus.shutterSpeed *<br>
> > > ><br>
> >  deviceStatus.analogueGain;<br>
> > > >                         if (actualExposure) {<br>
> > > > -                               status_.digitalGain =<br>
> > status_.totalExposureValue / actualExposure;<br>
> > > > +                               /* TODO(Bug 156): Workaround for LLVM<br>
> > bug.<br>
> > > > */<br>
> > > > +                               double totalExposureDouble =<br>
> > status_.totalExposureValue.get<std::nano>();<br>
> > > > +                               double actualExposureDouble =<br>
> > actualExposure.get<std::nano>();<br>
> > > > +                               status_.digitalGain =<br>
> > totalExposureDouble / actualExposureDouble;<br>
> > > >                                 LOG(RPiAgc, Debug) << "Want total<br>
> > exposure " << status_.totalExposureValue;<br>
> > > >                                 /*<br>
> > > >                                  * Never ask for a gain < 1.0, and<br>
> > also impose<br>
> > > > @@ -823,7 +826,10 @@ void Agc::divideUpExposure()<br>
> > > >                         }<br>
> > > >                         if (status_.fixedAnalogueGain == 0.0) {<br>
> > > >                                 if (exposureMode_->gain[stage] *<br>
> > shutterTime >= exposureValue) {<br>
> > > > -                                       analogueGain = exposureValue /<br>
> > shutterTime;<br>
> > > > +                                       /* TODO(Bug 156): Workaround<br>
> > for LLVM bug. */<br>
> > > > +                                       double exposureDouble =<br>
> > exposureValue.get<std::nano>();<br>
> > > > +                                       double shutterTimeDouble =<br>
> > shutterTime.get<std::nano>();<br>
> > > > +                                       analogueGain = exposureDouble<br>
> > / shutterTimeDouble;<br>
> > > >                                         break;<br>
> > > >                                 }<br>
> > > >                                 analogueGain =<br>
> > exposureMode_->gain[stage];<br>
> > > > @@ -838,10 +844,14 @@ void Agc::divideUpExposure()<br>
> > > >          */<br>
> > > >         if (!status_.fixedShutter && !status_.fixedAnalogueGain &&<br>
> > > >             status_.flickerPeriod) {<br>
> > > > -               int flickerPeriods = shutterTime /<br>
> > status_.flickerPeriod;<br>
> > > > +               /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +               double shutterTimeDouble =<br>
> > shutterTime.get<std::nano>();<br>
> > > > +               double flickerPeriod =<br>
> > status_.flickerPeriod.get<std::nano>();<br>
> > > > +               int flickerPeriods = shutterTimeDouble / flickerPeriod;<br>
> > > >                 if (flickerPeriods) {<br>
> > > >                         Duration newShutterTime = flickerPeriods *<br>
> > status_.flickerPeriod;<br>
> > > > -                       analogueGain *= shutterTime / newShutterTime;<br>
> > > > +                       double newShutterTimeDouble =<br>
> > newShutterTime.get<std::nano>();<br>
> > > > +                       analogueGain *= shutterTimeDouble /<br>
> > newShutterTimeDouble;<br>
> > > >                         /*<br>
> > > >                          * We should still not allow the ag to go over<br>
> > the<br>
> > > >                          * largest value in the exposure mode. Note<br>
> > that this<br>
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> > b/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> > > > index 9759186a..49303409 100644<br>
> > > > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> > > > @@ -93,8 +93,11 @@ void Lux::process(StatisticsPtr &stats, Metadata<br>
> > *imageMetadata)<br>
> > > >                 /* add .5 to reflect the mid-points of bins */<br>
> > > >                 double currentY = sum / (double)num + .5;<br>
> > > >                 double gainRatio = referenceGain_ / currentGain;<br>
> > > > +               /* TODO(Bug 156): Workaround for LLVM bug. */<br>
> > > > +               double referenceShutterSpeedDouble =<br>
> > referenceShutterSpeed_.get<std::nano>();<br>
> > > > +               double deviceShutterSpeed =<br>
> > deviceStatus.shutterSpeed.get<std::nano>();<br>
> > > >                 double shutterSpeedRatio =<br>
> > > > -                       referenceShutterSpeed_ /<br>
> > deviceStatus.shutterSpeed;<br>
> > > > +                       referenceShutterSpeedDouble /<br>
> > deviceShutterSpeed;<br>
> > > >                 double apertureRatio = referenceAperture_ /<br>
> > currentAperture;<br>
> > > >                 double yRatio = currentY * (65536 / numBins) /<br>
> > referenceY_;<br>
> > > >                 double estimatedLux = shutterSpeedRatio * gainRatio *<br>
> > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp<br>
> > b/src/ipa/rkisp1/algorithms/agc.cpp<br>
> > > > index 04062a36..3ea0b732 100644<br>
> > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp<br>
> > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp<br>
> > > > @@ -74,7 +74,13 @@ int Agc::configure(IPAContext &context, const<br>
> > IPACameraSensorInfo &configInfo)<br>
> > > >  {<br>
> > > >         /* Configure the default exposure and gain. */<br>
> > > >         context.activeState.agc.gain =<br>
> > std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);<br>
> > > > -       context.activeState.agc.exposure = 10ms /<br>
> > context.configuration.sensor.lineDuration;<br>
> > > > +       /* TODO(Bug 156): Explicit division of ticks (e.g.,<br>
> > `x.get<std::nano>() /<br>
> > > > +        * y.get<std::nano>()` as opposed to `x / y`) is a workaround<br>
> > for<br>
> > > > +        * LLVM bug 41130 and should be reverted once we no longer<br>
> > target<br>
> > > > +        * Android 11 / sdk30 since it compromises unit safety and<br>
> > readability. */<br>
> > > > +       constexpr libcamera::utils::Duration ten_millis(10ms);<br>
> > > > +       long double exposure = ten_millis.get<std::nano>() /<br>
> > context.configuration.sensor.lineDuration.get<std::nano>();<br>
> > > > +       context.activeState.agc.exposure = uint32_t(exposure);<br>
> > > ><br>
> > > >         /*<br>
> > > >          * According to the RkISP1 documentation:<br>
> > > > @@ -211,17 +217,25 @@ void Agc::computeExposure(IPAContext &context,<br>
> > IPAFrameContext &frameContext,<br>
> > > >         /*<br>
> > > >          * Push the shutter time up to the maximum first, and only then<br>
> > > >          * increase the gain.<br>
> > > > +        *<br>
> > > > +        * TODO(Bug 156): Explicit division of ticks (e.g.,<br>
> > `x.get<std::nano>() /<br>
> > > > +        * y.get<std::nano>()` as opposed to `x / y`) is a workaround<br>
> > for<br>
> > > > +        * LLVM bug 41130 and should be reverted once we no longer<br>
> > target<br>
> > > > +        * Android 11 / sdk30 since it compromises unit safety and<br>
> > readability.<br>
> > > >          */<br>
> > > > -       utils::Duration shutterTime =<br>
> > std::clamp<utils::Duration>(exposureValue / minAnalogueGain,<br>
> > > > +       utils::Duration<br>
> > shutterTimeUnclamped(exposureValue.get<std::nano>() / minAnalogueGain);<br>
> > > > +       utils::Duration shutterTime =<br>
> > std::clamp<utils::Duration>(shutterTimeUnclamped,<br>
> > > ><br>
> >  minShutterSpeed, maxShutterSpeed);<br>
> > > > -       double stepGain = std::clamp(exposureValue / shutterTime,<br>
> > > > +       double stepGainUnclamped = exposureValue.get<std::nano>() /<br>
> > shutterTime.get<std::nano>();<br>
> > > > +       double stepGain = std::clamp(stepGainUnclamped,<br>
> > > >                                      minAnalogueGain, maxAnalogueGain);<br>
> > > >         LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are "<br>
> > > >                               << shutterTime << " and "<br>
> > > >                               << stepGain;<br>
> > > ><br>
> > > >         /* Update the estimated exposure and gain. */<br>
> > > > -       activeState.agc.exposure = shutterTime /<br>
> > configuration.sensor.lineDuration;<br>
> > > > +       activeState.agc.exposure =<br>
> > uint32_t(shutterTime.get<std::nano>() /<br>
> > > > +               configuration.sensor.lineDuration.get<std::nano>());<br>
> > > >         activeState.agc.gain = stepGain;<br>
> > > >  }<br>
> > > ><br>
> ><br>
> > --<br>
> > Regards,<br>
> ><br>
> > Laurent Pinchart<br>
> ><br>
> -- <br>
> *Nicholas Roth*<br>
> *Software Engineer, Machine Learning*<br>
> *Google <<a href="https://www.google.com/" rel="noreferrer" target="_blank">https://www.google.com/</a>> *<br>
> C: (512) 944-0747<br>
> <br>
> *This footer provides context about my professional background. I am not<br>
> acting on behalf of Google. My words and opinions are my own, and not those<br>
> of Google.*<br>
</blockquote></div>