| Message ID | 20240616163910.5506-10-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | b53c6de03fce3b1ca8941292fdefd1f875add537 |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart (2024-06-16 17:39:07) > The effective exposure value for each frame is split into shutter time, > analog gain and digital gain based on the AGC constraint mode and > exposure mode. The algorithm uses the modes from the active state, which > tracks the latest queued request, instead of the frame context, which > tracks the value of the controls requested for that frame. Fix it by > using the correct modes. The state tracking at different points of time is going to be a recurring tricky part isn't it. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index a61201bb05c9..6a199b47c9ad 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > utils::Duration shutterTime; > double aGain, dGain; > std::tie(shutterTime, aGain, dGain) = > - calculateNewEv(context.activeState.agc.constraintMode, > - context.activeState.agc.exposureMode, > + calculateNewEv(frameContext.agc.constraintMode, > + frameContext.agc.exposureMode, > hist, effectiveExposureValue); > > LOG(RkISP1Agc, Debug) > -- > Regards, > > Laurent Pinchart >
On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2024-06-16 17:39:07) > > The effective exposure value for each frame is split into shutter time, > > analog gain and digital gain based on the AGC constraint mode and > > exposure mode. The algorithm uses the modes from the active state, which > > tracks the latest queued request, instead of the frame context, which > > tracks the value of the controls requested for that frame. Fix it by > > using the correct modes. > > The state tracking at different points of time is going to be a > recurring tricky part isn't it. It appears to be so. A recuring pain point, but at the same time, it makes me happy to see a pattern emerging. Patterns mean we can try to find structural solutions. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > index a61201bb05c9..6a199b47c9ad 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > utils::Duration shutterTime; > > double aGain, dGain; > > std::tie(shutterTime, aGain, dGain) = > > - calculateNewEv(context.activeState.agc.constraintMode, > > - context.activeState.agc.exposureMode, > > + calculateNewEv(frameContext.agc.constraintMode, > > + frameContext.agc.exposureMode, > > hist, effectiveExposureValue); > > > > LOG(RkISP1Agc, Debug)
On Mon, Jun 17, 2024 at 12:32:44PM +0300, Laurent Pinchart wrote: > On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote: > > Quoting Laurent Pinchart (2024-06-16 17:39:07) > > > The effective exposure value for each frame is split into shutter time, > > > analog gain and digital gain based on the AGC constraint mode and > > > exposure mode. The algorithm uses the modes from the active state, which > > > tracks the latest queued request, instead of the frame context, which > > > tracks the value of the controls requested for that frame. Fix it by > > > using the correct modes. > > > > The state tracking at different points of time is going to be a > > recurring tricky part isn't it. > > It appears to be so. A recuring pain point, but at the same time, it > makes me happy to see a pattern emerging. Patterns mean we can try to > find structural solutions. I can never remember which is the right one... Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > > index a61201bb05c9..6a199b47c9ad 100644 > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > > utils::Duration shutterTime; > > > double aGain, dGain; > > > std::tie(shutterTime, aGain, dGain) = > > > - calculateNewEv(context.activeState.agc.constraintMode, > > > - context.activeState.agc.exposureMode, > > > + calculateNewEv(frameContext.agc.constraintMode, > > > + frameContext.agc.exposureMode, > > > hist, effectiveExposureValue); > > > > > > LOG(RkISP1Agc, Debug)
On Mon, Jun 17, 2024 at 09:52:21PM +0900, Paul Elder wrote: > On Mon, Jun 17, 2024 at 12:32:44PM +0300, Laurent Pinchart wrote: > > On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote: > > > Quoting Laurent Pinchart (2024-06-16 17:39:07) > > > > The effective exposure value for each frame is split into shutter time, > > > > analog gain and digital gain based on the AGC constraint mode and > > > > exposure mode. The algorithm uses the modes from the active state, which > > > > tracks the latest queued request, instead of the frame context, which > > > > tracks the value of the controls requested for that frame. Fix it by > > > > using the correct modes. > > > > > > The state tracking at different points of time is going to be a > > > recurring tricky part isn't it. > > > > It appears to be so. A recuring pain point, but at the same time, it > > makes me happy to see a pattern emerging. Patterns mean we can try to > > find structural solutions. > > I can never remember which is the right one... I'll take that as a vote for a structural solution :-) > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > > > index a61201bb05c9..6a199b47c9ad 100644 > > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > > > utils::Duration shutterTime; > > > > double aGain, dGain; > > > > std::tie(shutterTime, aGain, dGain) = > > > > - calculateNewEv(context.activeState.agc.constraintMode, > > > > - context.activeState.agc.exposureMode, > > > > + calculateNewEv(frameContext.agc.constraintMode, > > > > + frameContext.agc.exposureMode, > > > > hist, effectiveExposureValue); > > > > > > > > LOG(RkISP1Agc, Debug)
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index a61201bb05c9..6a199b47c9ad 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, utils::Duration shutterTime; double aGain, dGain; std::tie(shutterTime, aGain, dGain) = - calculateNewEv(context.activeState.agc.constraintMode, - context.activeState.agc.exposureMode, + calculateNewEv(frameContext.agc.constraintMode, + frameContext.agc.exposureMode, hist, effectiveExposureValue); LOG(RkISP1Agc, Debug)
The effective exposure value for each frame is split into shutter time, analog gain and digital gain based on the AGC constraint mode and exposure mode. The algorithm uses the modes from the active state, which tracks the latest queued request, instead of the frame context, which tracks the value of the controls requested for that frame. Fix it by using the correct modes. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)