| Message ID | 20211013154125.133419-3-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois (2021-10-13 16:41:14) Missing some context here. Is this correct? If not put something that's a bit more correct please ;-) """ The AGC frame context needs to be initialised correctly for the first iteration. Set the gain and exposure appropriately to the current values known to the IPA. """ Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/ipu3.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 6d9bbf39..e54fbab5 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -415,6 +415,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) > return ret; > } > > + context_.frameContext.agc.gain = camHelper_->gain(gain_); > + context_.frameContext.agc.exposure = exposure_; > + > return 0; > } > > -- > 2.30.2 >
On Thu, Oct 14, 2021 at 11:26:22AM +0100, Kieran Bingham wrote: > Quoting Jean-Michel Hautbois (2021-10-13 16:41:14) > > Missing some context here. Is this correct? If not put something that's > a bit more correct please ;-) > > """ > The AGC frame context needs to be initialised correctly for the first > iteration. > > Set the gain and exposure appropriately to the current values known to > the IPA. > """ > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/ipa/ipu3/ipu3.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 6d9bbf39..e54fbab5 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -415,6 +415,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) > > return ret; > > } > > > > + context_.frameContext.agc.gain = camHelper_->gain(gain_); > > + context_.frameContext.agc.exposure = exposure_; Why is this special, why don't we set the rest of the context here, or, possibly better, set this in the AGC algorithm ? The exposure_ and gain_ members of IPU3IPA seem a bit out of place. Maybe they're removed later in this series ? > > + > > return 0; > > } > >
Hi Kieran, Laurent, On 15/10/2021 00:00, Laurent Pinchart wrote: > On Thu, Oct 14, 2021 at 11:26:22AM +0100, Kieran Bingham wrote: >> Quoting Jean-Michel Hautbois (2021-10-13 16:41:14) >> >> Missing some context here. Is this correct? If not put something that's >> a bit more correct please ;-) >> >> """ >> The AGC frame context needs to be initialised correctly for the first >> iteration. >> >> Set the gain and exposure appropriately to the current values known to >> the IPA. >> """ >> Oh my, I missed this commit message, sorry :-(. >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>> --- >>> src/ipa/ipu3/ipu3.cpp | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >>> index 6d9bbf39..e54fbab5 100644 >>> --- a/src/ipa/ipu3/ipu3.cpp >>> +++ b/src/ipa/ipu3/ipu3.cpp >>> @@ -415,6 +415,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) >>> return ret; >>> } >>> >>> + context_.frameContext.agc.gain = camHelper_->gain(gain_); >>> + context_.frameContext.agc.exposure = exposure_; > > Why is this special, why don't we set the rest of the context here, or, > possibly better, set this in the AGC algorithm ? The exposure_ and gain_ > members of IPU3IPA seem a bit out of place. Maybe they're removed later > in this series ? > Indeed, they should move to AGC, and later we will not pass exposure and gain this way, but with a duration value and a double to represent the shutter speed and the analogue gain. I wanted it in the series but did not come to something satisfying yet. >>> + >>> return 0; >>> } >>> >
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 6d9bbf39..e54fbab5 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -415,6 +415,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) return ret; } + context_.frameContext.agc.gain = camHelper_->gain(gain_); + context_.frameContext.agc.exposure = exposure_; + return 0; }
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/ipu3.cpp | 3 +++ 1 file changed, 3 insertions(+)