[PATCH 1/2] libcamera: ipa_proxy: Report a missing configuration as a warning

[Milan Zamazal]

Hi Kieran,

thank you for review.

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> Quoting Milan Zamazal (2024-06-27 18:33:04)
>> When the configuration file for an IPA module is missing, it is reported
>> as an error in the log, for example:
>
>> 
>>   ERROR IPAProxy ipa_proxy.cpp:149 Configuration file 'imx219.yaml' not found for IPA module
>> 'simple'
>> 
>> This is misleading because several pipelines use uncalibrated.yaml in
>> such a case and can continue working.  And in case of software ISP,
>> there is currently no other configuration file so the error is always
>> reported.
>
> I'm in two minds for this. At the moment, the SoftISP doesn't use a
> tuning file because it's not implemented.
>
> But I /would/ expect there to be one in the future.

Yes.

> I believe we report it as an error as without a tuning file, we expect
> there will always be a degraded image quality.
>
> But ... indeed - it still continues. So what's the difference between an
> error and a warning ?

I cannot know better than you what is a contingent libcamera policy, but
generally, I'd say error is something that prevents some important part
from working at all while warning is something that the user should pay
attention to but the action can still be performed, although perhaps in
a degraded or unintended way.

In this specific case I have experienced the following interaction with
a user:

- "Hmm, it doesn't work."
- "Sure, it reports an error!"
- "But this one is harmless."
- "Are you sure?  It clearly reports that it is an error."
- "Yes, it always prints this error, it's not the problem."
- "Really?  Hmm, OK."

I think that at least with software ISP, it shouldn't be reported as an
error.  But I don't insist on this fix, another obvious fix might be
implementing tuning files in software ISP, or perhaps some workaround.
What would you suggest?

>> Let's change the error to warning to not confuse users.
>> 
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/libcamera/ipa_proxy.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
>> index 6c17c456..494ed736 100644
>> --- a/src/libcamera/ipa_proxy.cpp
>> +++ b/src/libcamera/ipa_proxy.cpp
>> @@ -146,7 +146,7 @@ std::string IPAProxy::configurationFile(const std::string &name) const
>>                 }
>>         }
>>  
>> -       LOG(IPAProxy, Error)
>> +       LOG(IPAProxy, Warning)
>>                 << "Configuration file '" << name
>>                 << "' not found for IPA module '" << ipaName << "'";
>>  
>> -- 
>> 2.44.1
>>