[libcamera-devel] softISP for libcamera

[Andrey Konovalov]

Hi Kieran,

On 05.12.2023 15:03, Kieran Bingham wrote:
> Quoting Hans de Goede via libcamera-devel (2023-12-05 11:38:57)
>> Hi Pavel,
>>
>> 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.

>> 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.

Thanks,
Andrey

>>
>> (I hit the exact same issue when trying Andrey's patches).
>>
>> Regards,
>>
>> Hans
>>
>>