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

[Kieran Bingham]

Quoting Milan Zamazal (2024-07-01 13:55:07)
> 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?

I too have had to tell people "Oh ... yeah you can ignore that error" (I
can't remember if it was this or another case" ... and I think if
something can be ignored ... is it really an error?

The tricky part here is ... on RPi - it really is considered an error
and the camera will not be constructed without a tuning file I believe,
while on other platforms (at least for now) it is only a warning...

I still don't know the right answer yet I'm afraid :S

--
Kieran


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