| Message ID | 20220413074353.13147-1-hpa@redhat.com |
|---|---|
| State | Accepted |
| Commit | 0111feb04b4e2442519d3dbdcd572b573e750316 |
| Headers | show |
| Series |
|
| Related | show |
Quoting Kate Hsuan via libcamera-devel (2022-04-13 08:43:53) > A not initialized frame ignore counter (ignoreCounter_) makes the AF > function not work since the ignore counter may start from a random > negative number. The counter was set to kIgnoreFrame when AF is in > prepare stage. > > Signed-off-by: Kate Hsuan<hpa@redhat.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index addf98af..f700b01f 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > /* Initial max focus step */ > maxStep_ = kMaxFocusSteps; > > + /* Initial frame ignore counter */ > + afIgnoreFrameReset(); > + Should this be handled by the constructor at all? (I suspect it doesn't need to if the data is not accessed before the algorithm is running) Or is there any requirement to reset the counter when a stream off, stream on cycle occurs (without a reconfigure in between?) Assuming those are fine... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > /* Initial focus value */ > context.frameContext.af.focus = 0; > /* Maximum variance of the AF statistics */ > -- > 2.35.1 >
Hello, On 4/13/22 13:13, Kate Hsuan via libcamera-devel wrote: > A not initialized frame ignore counter (ignoreCounter_) makes the AF > function not work since the ignore counter may start from a random > negative number. The counter was set to kIgnoreFrame when AF is in > prepare stage. > > Signed-off-by: Kate Hsuan<hpa@redhat.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index addf98af..f700b01f 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > /* Initial max focus step */ > maxStep_ = kMaxFocusSteps; > > + /* Initial frame ignore counter */ > + afIgnoreFrameReset(); > + ... wondering if afReset() makes more sense here? > /* Initial focus value */ > context.frameContext.af.focus = 0; > /* Maximum variance of the AF statistics */
Hi Umang, On Wed, Apr 13, 2022 at 4:20 PM Umang Jain <umang.jain@ideasonboard.com> wrote: > > Hello, > > On 4/13/22 13:13, Kate Hsuan via libcamera-devel wrote: > > A not initialized frame ignore counter (ignoreCounter_) makes the AF > > function not work since the ignore counter may start from a random > > negative number. The counter was set to kIgnoreFrame when AF is in > > prepare stage. > > > > Signed-off-by: Kate Hsuan<hpa@redhat.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > index addf98af..f700b01f 100644 > > --- a/src/ipa/ipu3/algorithms/af.cpp > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /* Initial max focus step */ > > maxStep_ = kMaxFocusSteps; > > > > + /* Initial frame ignore counter */ > > + afIgnoreFrameReset(); > > + > > > ... wondering if afReset() makes more sense here? afReset first checks the ignore counter and then resets all the variables. I think it can't be used here. :) > > > /* Initial focus value */ > > context.frameContext.af.focus = 0; > > /* Maximum variance of the AF statistics */ >
Hi Kieran, On Wed, Apr 13, 2022 at 4:10 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Kate Hsuan via libcamera-devel (2022-04-13 08:43:53) > > A not initialized frame ignore counter (ignoreCounter_) makes the AF > > function not work since the ignore counter may start from a random > > negative number. The counter was set to kIgnoreFrame when AF is in > > prepare stage. > > > > Signed-off-by: Kate Hsuan<hpa@redhat.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > index addf98af..f700b01f 100644 > > --- a/src/ipa/ipu3/algorithms/af.cpp > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /* Initial max focus step */ > > maxStep_ = kMaxFocusSteps; > > > > + /* Initial frame ignore counter */ > > + afIgnoreFrameReset(); > > + > > Should this be handled by the constructor at all? (I suspect it doesn't > need to if the data is not accessed before the algorithm is running) Or > is there any requirement to reset the counter when a stream off, stream > on cycle occurs (without a reconfigure in between?) Those places are good to initialize the counter. We only have to make sure the counter will not be a negative number. For stream on, the counter could be reset since it makes more time to stable the image status, such as AE and AWB. Thank you :) > > Assuming those are fine... > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > /* Initial focus value */ > > context.frameContext.af.focus = 0; > > /* Maximum variance of the AF statistics */ > > -- > > 2.35.1 > > >
Hi, On 4/13/22 14:01, Kate Hsuan wrote: > Hi Umang, > > On Wed, Apr 13, 2022 at 4:20 PM Umang Jain <umang.jain@ideasonboard.com> wrote: >> Hello, >> >> On 4/13/22 13:13, Kate Hsuan via libcamera-devel wrote: >>> A not initialized frame ignore counter (ignoreCounter_) makes the AF >>> function not work since the ignore counter may start from a random >>> negative number. The counter was set to kIgnoreFrame when AF is in >>> prepare stage. >>> >>> Signed-off-by: Kate Hsuan<hpa@redhat.com> >>> --- >>> src/ipa/ipu3/algorithms/af.cpp | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp >>> index addf98af..f700b01f 100644 >>> --- a/src/ipa/ipu3/algorithms/af.cpp >>> +++ b/src/ipa/ipu3/algorithms/af.cpp >>> @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) >>> /* Initial max focus step */ >>> maxStep_ = kMaxFocusSteps; >>> >>> + /* Initial frame ignore counter */ >>> + afIgnoreFrameReset(); >>> + >> >> ... wondering if afReset() makes more sense here? > afReset first checks the ignore counter and then resets all the > variables. I think it can't be used here. :) Ah yes! I missed to notice that part - for what it's worth Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > >>> /* Initial focus value */ >>> context.frameContext.af.focus = 0; >>> /* Maximum variance of the AF statistics */ >
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index addf98af..f700b01f 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -181,6 +181,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) /* Initial max focus step */ maxStep_ = kMaxFocusSteps; + /* Initial frame ignore counter */ + afIgnoreFrameReset(); + /* Initial focus value */ context.frameContext.af.focus = 0; /* Maximum variance of the AF statistics */
A not initialized frame ignore counter (ignoreCounter_) makes the AF function not work since the ignore counter may start from a random negative number. The counter was set to kIgnoreFrame when AF is in prepare stage. Signed-off-by: Kate Hsuan<hpa@redhat.com> --- src/ipa/ipu3/algorithms/af.cpp | 3 +++ 1 file changed, 3 insertions(+)