[libcamera-devel] softISP for libcamera

[Laurent Pinchart]

On Tue, Dec 05, 2023 at 03:30:29PM +0300, Andrey Konovalov via libcamera-devel wrote:
> On 05.12.2023 15:03, Kieran Bingham wrote:
> > Quoting Hans de Goede via libcamera-devel (2023-12-05 11:38:57)
> >> On 12/5/23 00:04, Pavel Machek via libcamera-devel wrote:
> >>> Hi!
> >>>
> >>>> First, thanks for softisp efforts. I worked on something similar but
> >>>> did not get anywhere near mergeable state. (My hacks are at
> >>>> git at gitlab.com:tui/libcamera.git,  new-ae2 branch).
> >>>>
> >>>> I tried to test the code on PinePhone, so I started from
> >>>>
> >>>> commit 10c27dc4cf922fb79e9f3a3cf44602f472bbc102 (HEAD -> sisp, softisp/SoftwareISP-v02, origin/sisp)
> >>>> Author: Andrey Konovalov <andrey.konovalov at linaro.org>
> >>>> Date:   Fri Dec 1 17:01:30 2023 +0300
> >>>>
> >>>> But I'm getting build error:
> >>>>
> >>>> ../src/libcamera/pipeline/simple/simple.cpp:28:10: fatal error: libcamera/ipa/soft_ipa_interface.h: No such file or directory
> >>>>     28 | #include <libcamera/ipa/soft_ipa_interface.h>
> >>>>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> compilation terminated.
> >>>>
> >>>> ...and it sounds like libcamera/ipa/soft_ipa_interface.h is missing
> >>>> from the repository?
> >>>
> >>> It may be that I failed to enable simple/linaro pipeline in the
> >>> meson. It certainly does not list it. If I try with parent commit, it
> >>> builds okay, but I guess that means code is not enabled.
> >>
> >> Did you see Andrey's reply that you need to pass:
> >>
> >> "-Dpipelines=simple/linaro -Dipas=simple/linaro"
> > 
> > I assume this is a temporary naming thing? Will this work be added to
> > the simple pipeline handler? I or does it require a new pipeline handler
> > to be constructed?
> > 
> > I expect the IPA would be called 'softISP' (/softIPA?)
> 
> The current naming scheme is "simple/<SoftISP implementation name>".
> The idea is that the
>    "-Dpipelines=simple"
> would work as before (no softISP and softIPA instantiated). This isn't
> the case for the current patch set, as it always creates the linaro's
> implementation of softIPA, but I plan to fix that sooner than later.
> And
>    "-Dpipelines=simple/<SoftISP implementation name> -Dipas=simple/<SoftISP implementation name>"
> would use one of the softISP / softIPA implementations.
> Note that the <SoftISP implementation name> needs not to be "linaro".
> I haven't check myself if "-Dpipelines=simple/something-else -Dipas=simple/something-else"
> would work along with "simple/linaro" if the 2nd implementation were added,
> but the idea was that it should.
> 
> And the current scheme is rather a temporary solution.
> A more natural one could be
>    "-Dpipelines=simple -Dipas=softIPA/<SoftISP implementation name>"
> but this would require a serious rework of the meson files.

I think we'll need to rework them, but not right now. Let's focus on the
feature first.

> >> During the meson configure/setup phase? If you pass that
> >> then the libcamera/ipa/soft_ipa_interface.h file will
> >> get generated during the build.
> > 
> > Adding
> > 
> >      'simple/linaro':     ['any'],
> > 
> > to the pipelines_support table around line 192 in meson.build is
> > probably helpful for this series.
> 
> Indeed.
> 
> > But I think it should be handled a bit more generically otherwise.
> 
> I agree.
> 
> >> (I hit the exact same issue when trying Andrey's patches).

-- 
Regards,

Laurent Pinchart