| Message ID | 20230119084112.20564-1-dse@thaumatec.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
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 >
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 > >
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 >
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 > >