[libcamera-devel,v3,0/8] ipa: rkisp1: Add autofocus algorithm
mbox series

Message ID 20230119084112.20564-1-dse@thaumatec.com
Headers show
Series
  • ipa: rkisp1: Add autofocus algorithm
Related show

Message

Daniel Semkowicz Jan. 19, 2023, 8:41 a.m. UTC
Hi!

This is a patch series with a common AF algorithm implementation
basing on the IPU3 AF version. The common part was then used to
implement AF for the rkisp1 ISP.

Early version of this serie was uploaded in the July, but there were
some change in the IPA in the meantime. The current version was updated
to the new IPA code and has most of the comments from the v2 review
fixed.

Best regards
Daniel Semkowicz

Daniel Semkowicz (8):
  rkisp1: Add camera lens to PH and expose it to the IPA
  rkisp1: Control camera lens position from IPA
  ipa: Add base class defining AF algorithm interface
  ipa: Add class that implements base AF control algorithm
  ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class
  pipeline: rkisp1: Add basic AF controls to the supported controls list
  ipa: rkisp1: Add "Windows" Metering mode to auto focus algorithm
  ipa: rkisp1: Add OV5675 tuning file

 include/libcamera/ipa/rkisp1.mojom            |   2 +
 .../libipa/algorithms/af_hill_climbing.cpp    | 374 ++++++++++++++++++
 src/ipa/libipa/algorithms/af_hill_climbing.h  | 102 +++++
 src/ipa/libipa/algorithms/af_interface.cpp    |  92 +++++
 src/ipa/libipa/algorithms/af_interface.h      |  41 ++
 src/ipa/libipa/algorithms/meson.build         |  11 +
 src/ipa/libipa/meson.build                    |   6 +
 src/ipa/rkisp1/algorithms/af.cpp              | 182 +++++++++
 src/ipa/rkisp1/algorithms/af.h                |  51 +++
 src/ipa/rkisp1/algorithms/meson.build         |   1 +
 src/ipa/rkisp1/data/meson.build               |   1 +
 src/ipa/rkisp1/data/ov5675.yaml               |  20 +
 src/ipa/rkisp1/ipa_context.h                  |   5 +
 src/ipa/rkisp1/rkisp1.cpp                     |  23 ++
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  21 +
 15 files changed, 932 insertions(+)
 create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.cpp
 create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.h
 create mode 100644 src/ipa/libipa/algorithms/af_interface.cpp
 create mode 100644 src/ipa/libipa/algorithms/af_interface.h
 create mode 100644 src/ipa/libipa/algorithms/meson.build
 create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/af.h
 create mode 100644 src/ipa/rkisp1/data/ov5675.yaml

Comments

Quentin Schulz Feb. 3, 2023, 9:42 a.m. UTC | #1
Hi all,

Gentle ping here.

Cheers,
Quentin

On 1/19/23 09:41, Daniel Semkowicz via libcamera-devel wrote:
> Hi!
> 
> This is a patch series with a common AF algorithm implementation
> basing on the IPU3 AF version. The common part was then used to
> implement AF for the rkisp1 ISP.
> 
> Early version of this serie was uploaded in the July, but there were
> some change in the IPA in the meantime. The current version was updated
> to the new IPA code and has most of the comments from the v2 review
> fixed.
> 
> Best regards
> Daniel Semkowicz
> 
> Daniel Semkowicz (8):
>    rkisp1: Add camera lens to PH and expose it to the IPA
>    rkisp1: Control camera lens position from IPA
>    ipa: Add base class defining AF algorithm interface
>    ipa: Add class that implements base AF control algorithm
>    ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class
>    pipeline: rkisp1: Add basic AF controls to the supported controls list
>    ipa: rkisp1: Add "Windows" Metering mode to auto focus algorithm
>    ipa: rkisp1: Add OV5675 tuning file
> 
>   include/libcamera/ipa/rkisp1.mojom            |   2 +
>   .../libipa/algorithms/af_hill_climbing.cpp    | 374 ++++++++++++++++++
>   src/ipa/libipa/algorithms/af_hill_climbing.h  | 102 +++++
>   src/ipa/libipa/algorithms/af_interface.cpp    |  92 +++++
>   src/ipa/libipa/algorithms/af_interface.h      |  41 ++
>   src/ipa/libipa/algorithms/meson.build         |  11 +
>   src/ipa/libipa/meson.build                    |   6 +
>   src/ipa/rkisp1/algorithms/af.cpp              | 182 +++++++++
>   src/ipa/rkisp1/algorithms/af.h                |  51 +++
>   src/ipa/rkisp1/algorithms/meson.build         |   1 +
>   src/ipa/rkisp1/data/meson.build               |   1 +
>   src/ipa/rkisp1/data/ov5675.yaml               |  20 +
>   src/ipa/rkisp1/ipa_context.h                  |   5 +
>   src/ipa/rkisp1/rkisp1.cpp                     |  23 ++
>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  21 +
>   15 files changed, 932 insertions(+)
>   create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.cpp
>   create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.h
>   create mode 100644 src/ipa/libipa/algorithms/af_interface.cpp
>   create mode 100644 src/ipa/libipa/algorithms/af_interface.h
>   create mode 100644 src/ipa/libipa/algorithms/meson.build
>   create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
>   create mode 100644 src/ipa/rkisp1/algorithms/af.h
>   create mode 100644 src/ipa/rkisp1/data/ov5675.yaml
>
Jacopo Mondi Feb. 3, 2023, 10:04 a.m. UTC | #2
Hi Quentin

On Fri, Feb 03, 2023 at 10:42:03AM +0100, Quentin Schulz via libcamera-devel wrote:
> Date: Fri, 3 Feb 2023 10:42:03 +0100
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> To: Daniel Semkowicz <dse@thaumatec.com>,
>  libcamera-devel@lists.libcamera.org
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
>  Thunderbird/102.6.0
> Subject: Re: [libcamera-devel] [PATCH v3 0/8] ipa: rkisp1: Add autofocus
>  algorithm
>
> Hi all,
>
> Gentle ping here.
>

Sorry, I privately notified Daniel yesterday that his series has not
fallen into a (too deep) crack.

I'm in the process of setting up a [rockpi4 + imx519 + ak7375 lens] to
test it and will review it after.


> Cheers,
> Quentin
>
> On 1/19/23 09:41, Daniel Semkowicz via libcamera-devel wrote:
> > Hi!
> >
> > This is a patch series with a common AF algorithm implementation
> > basing on the IPU3 AF version. The common part was then used to
> > implement AF for the rkisp1 ISP.
> >
> > Early version of this serie was uploaded in the July, but there were
> > some change in the IPA in the meantime. The current version was updated
> > to the new IPA code and has most of the comments from the v2 review
> > fixed.
> >
> > Best regards
> > Daniel Semkowicz
> >
> > Daniel Semkowicz (8):
> >    rkisp1: Add camera lens to PH and expose it to the IPA
> >    rkisp1: Control camera lens position from IPA
> >    ipa: Add base class defining AF algorithm interface
> >    ipa: Add class that implements base AF control algorithm
> >    ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class
> >    pipeline: rkisp1: Add basic AF controls to the supported controls list
> >    ipa: rkisp1: Add "Windows" Metering mode to auto focus algorithm
> >    ipa: rkisp1: Add OV5675 tuning file
> >
> >   include/libcamera/ipa/rkisp1.mojom            |   2 +
> >   .../libipa/algorithms/af_hill_climbing.cpp    | 374 ++++++++++++++++++
> >   src/ipa/libipa/algorithms/af_hill_climbing.h  | 102 +++++
> >   src/ipa/libipa/algorithms/af_interface.cpp    |  92 +++++
> >   src/ipa/libipa/algorithms/af_interface.h      |  41 ++
> >   src/ipa/libipa/algorithms/meson.build         |  11 +
> >   src/ipa/libipa/meson.build                    |   6 +
> >   src/ipa/rkisp1/algorithms/af.cpp              | 182 +++++++++
> >   src/ipa/rkisp1/algorithms/af.h                |  51 +++
> >   src/ipa/rkisp1/algorithms/meson.build         |   1 +
> >   src/ipa/rkisp1/data/meson.build               |   1 +
> >   src/ipa/rkisp1/data/ov5675.yaml               |  20 +
> >   src/ipa/rkisp1/ipa_context.h                  |   5 +
> >   src/ipa/rkisp1/rkisp1.cpp                     |  23 ++
> >   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  21 +
> >   15 files changed, 932 insertions(+)
> >   create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.cpp
> >   create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.h
> >   create mode 100644 src/ipa/libipa/algorithms/af_interface.cpp
> >   create mode 100644 src/ipa/libipa/algorithms/af_interface.h
> >   create mode 100644 src/ipa/libipa/algorithms/meson.build
> >   create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
> >   create mode 100644 src/ipa/rkisp1/algorithms/af.h
> >   create mode 100644 src/ipa/rkisp1/data/ov5675.yaml
> >
Jacopo Mondi Feb. 7, 2023, 3:41 p.m. UTC | #3
Hi Daniel

On Thu, Jan 19, 2023 at 09:41:04AM +0100, Daniel Semkowicz via libcamera-devel wrote:
> Hi!
>
> This is a patch series with a common AF algorithm implementation
> basing on the IPU3 AF version. The common part was then used to
> implement AF for the rkisp1 ISP.
>
> Early version of this serie was uploaded in the July, but there were
> some change in the IPA in the meantime. The current version was updated
> to the new IPA code and has most of the comments from the v2 review
> fixed.

I managed to test your series with a different camera module, in CAF mode.

I'm very happy with the result! it's contrast based so it's not
comparable with the performances of a PDAF sensor, but it works
well and proves I was over-concerned about CAF mode with contrast
based algorithms!

I think the part that still need some work is the discussion about the
class hierarchy, but I'm in favour of moving forward with the series
and iterate on top with more fine-grained support for more complex
scenarios and integrate with the work Matthias has done for advanced
optics.

What do you think about the class hierachy design ? Is this something
you would like to consider discussing ?

Thanks!!
   j

>
> Best regards
> Daniel Semkowicz
>
> Daniel Semkowicz (8):
>   rkisp1: Add camera lens to PH and expose it to the IPA
>   rkisp1: Control camera lens position from IPA
>   ipa: Add base class defining AF algorithm interface
>   ipa: Add class that implements base AF control algorithm
>   ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class
>   pipeline: rkisp1: Add basic AF controls to the supported controls list
>   ipa: rkisp1: Add "Windows" Metering mode to auto focus algorithm
>   ipa: rkisp1: Add OV5675 tuning file
>
>  include/libcamera/ipa/rkisp1.mojom            |   2 +
>  .../libipa/algorithms/af_hill_climbing.cpp    | 374 ++++++++++++++++++
>  src/ipa/libipa/algorithms/af_hill_climbing.h  | 102 +++++
>  src/ipa/libipa/algorithms/af_interface.cpp    |  92 +++++
>  src/ipa/libipa/algorithms/af_interface.h      |  41 ++
>  src/ipa/libipa/algorithms/meson.build         |  11 +
>  src/ipa/libipa/meson.build                    |   6 +
>  src/ipa/rkisp1/algorithms/af.cpp              | 182 +++++++++
>  src/ipa/rkisp1/algorithms/af.h                |  51 +++
>  src/ipa/rkisp1/algorithms/meson.build         |   1 +
>  src/ipa/rkisp1/data/meson.build               |   1 +
>  src/ipa/rkisp1/data/ov5675.yaml               |  20 +
>  src/ipa/rkisp1/ipa_context.h                  |   5 +
>  src/ipa/rkisp1/rkisp1.cpp                     |  23 ++
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  21 +
>  15 files changed, 932 insertions(+)
>  create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.cpp
>  create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.h
>  create mode 100644 src/ipa/libipa/algorithms/af_interface.cpp
>  create mode 100644 src/ipa/libipa/algorithms/af_interface.h
>  create mode 100644 src/ipa/libipa/algorithms/meson.build
>  create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/af.h
>  create mode 100644 src/ipa/rkisp1/data/ov5675.yaml
>
> --
> 2.39.0
>
Daniel Semkowicz March 8, 2023, 6:31 a.m. UTC | #4
Hi Jacopo,

First, sorry for the late response...

On Tue, Feb 7, 2023 at 4:41 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Daniel
>
> On Thu, Jan 19, 2023 at 09:41:04AM +0100, Daniel Semkowicz via libcamera-devel wrote:
> > Hi!
> >
> > This is a patch series with a common AF algorithm implementation
> > basing on the IPU3 AF version. The common part was then used to
> > implement AF for the rkisp1 ISP.
> >
> > Early version of this serie was uploaded in the July, but there were
> > some change in the IPA in the meantime. The current version was updated
> > to the new IPA code and has most of the comments from the v2 review
> > fixed.
>
> I managed to test your series with a different camera module, in CAF mode.
>
> I'm very happy with the result! it's contrast based so it's not
> comparable with the performances of a PDAF sensor, but it works
> well and proves I was over-concerned about CAF mode with contrast
> based algorithms!

Good to hear that! CAF mode can be improved a bit further to make
the focusing more smooth. Currently, it just triggers focusing
from position 0 when it detects major change in contrast. We could
start the focusing from the current lens position instead of resetting
it to 0. However, this is something I would consider adding later
on top of these changes.

>
> I think the part that still need some work is the discussion about the
> class hierarchy, but I'm in favour of moving forward with the series
> and iterate on top with more fine-grained support for more complex
> scenarios and integrate with the work Matthias has done for advanced
> optics.

As you already mentioned, the performance comparing to the PDAF sensor
will be worse. The main problem I see with the contrast based AF,
is the reaction on changing lighting conditions. This will affect
the contrast value, and it can trigger the AF in continuous mode.
However, this will be hard to improve. At least, I do not have any
idea on that...

>
> What do you think about the class hierachy design ? Is this something
> you would like to consider discussing ?

Sure, I am open to discussion. will put additional comments in the related
patches.

Best regards
Daniel Semkowicz

>
> Thanks!!
>    j
>
> >
> > Best regards
> > Daniel Semkowicz
> >
> > Daniel Semkowicz (8):
> >   rkisp1: Add camera lens to PH and expose it to the IPA
> >   rkisp1: Control camera lens position from IPA
> >   ipa: Add base class defining AF algorithm interface
> >   ipa: Add class that implements base AF control algorithm
> >   ipa: rkisp1: Add AF algorithm basing on common AfHillClimbing class
> >   pipeline: rkisp1: Add basic AF controls to the supported controls list
> >   ipa: rkisp1: Add "Windows" Metering mode to auto focus algorithm
> >   ipa: rkisp1: Add OV5675 tuning file
> >
> >  include/libcamera/ipa/rkisp1.mojom            |   2 +
> >  .../libipa/algorithms/af_hill_climbing.cpp    | 374 ++++++++++++++++++
> >  src/ipa/libipa/algorithms/af_hill_climbing.h  | 102 +++++
> >  src/ipa/libipa/algorithms/af_interface.cpp    |  92 +++++
> >  src/ipa/libipa/algorithms/af_interface.h      |  41 ++
> >  src/ipa/libipa/algorithms/meson.build         |  11 +
> >  src/ipa/libipa/meson.build                    |   6 +
> >  src/ipa/rkisp1/algorithms/af.cpp              | 182 +++++++++
> >  src/ipa/rkisp1/algorithms/af.h                |  51 +++
> >  src/ipa/rkisp1/algorithms/meson.build         |   1 +
> >  src/ipa/rkisp1/data/meson.build               |   1 +
> >  src/ipa/rkisp1/data/ov5675.yaml               |  20 +
> >  src/ipa/rkisp1/ipa_context.h                  |   5 +
> >  src/ipa/rkisp1/rkisp1.cpp                     |  23 ++
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  21 +
> >  15 files changed, 932 insertions(+)
> >  create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.cpp
> >  create mode 100644 src/ipa/libipa/algorithms/af_hill_climbing.h
> >  create mode 100644 src/ipa/libipa/algorithms/af_interface.cpp
> >  create mode 100644 src/ipa/libipa/algorithms/af_interface.h
> >  create mode 100644 src/ipa/libipa/algorithms/meson.build
> >  create mode 100644 src/ipa/rkisp1/algorithms/af.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/af.h
> >  create mode 100644 src/ipa/rkisp1/data/ov5675.yaml
> >
> > --
> > 2.39.0
> >