[v2,3/3] libcamera: software_isp: Fix LIBCAMERA_SOFTISP_MODE log print
diff mbox series

Message ID 20260204-kbingham-fixups-v2-3-c341cced1fad@ideasonboard.com
State Accepted
Headers show
Series
  • ipa: awb: Fix and cleanup RKISP1 and Simple Awb
Related show

Commit Message

Kieran Bingham Feb. 4, 2026, 10:42 p.m. UTC 
When an invalid parameter is specified to LIBCAMERA_SOFTISP_MODE, the
error log has a typo. Fix the typo and reflow the line while here.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/software_isp/software_isp.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Feb. 5, 2026, 7:49 a.m. UTC | #1 
Hi Kieran

On Wed, Feb 04, 2026 at 10:42:58PM +0000, Kieran Bingham wrote:
> When an invalid parameter is specified to LIBCAMERA_SOFTISP_MODE, the
> error log has a typo. Fix the typo and reflow the line while here.

Took me a while to spot the "SOFISP" misspelling

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/software_isp/software_isp.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index a83986b787b75ac1cd9073b32f2b937e9efb6cf0..3eb9cf97dea66cb68f2277a8274003e28418d830 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -108,8 +108,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>  	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
>  	if (softISPMode) {
>  		if (softISPMode != "gpu" && softISPMode != "cpu") {
> -			LOG(SoftwareIsp, Error) << "LIBCAMERA_SOFISP_MODE " << softISPMode.value() << " invalid "
> -						<< "must be \"cpu\" or \"gpu\"";
> +			LOG(SoftwareIsp, Error)
> +				<< "LIBCAMERA_SOFTISP_MODE "
> +				<< softISPMode.value() << " invalid "
> +				<< "must be \"cpu\" or \"gpu\"";
>  			return;
>  		}
>  	}
>
> --
> 2.52.0
>
Kieran Bingham Feb. 5, 2026, 10:03 a.m. UTC | #2 
Quoting Jacopo Mondi (2026-02-05 07:49:34)
> Hi Kieran
> 
> On Wed, Feb 04, 2026 at 10:42:58PM +0000, Kieran Bingham wrote:
> > When an invalid parameter is specified to LIBCAMERA_SOFTISP_MODE, the
> > error log has a typo. Fix the typo and reflow the line while here.
> 
> Took me a while to spot the "SOFISP" misspelling

Guess which string I used to try to 'set' the ISP to CPU mode ...

--
Kieran

> 
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Thanks
>   j
> 
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/software_isp/software_isp.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index a83986b787b75ac1cd9073b32f2b937e9efb6cf0..3eb9cf97dea66cb68f2277a8274003e28418d830 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -108,8 +108,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> >       std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
> >       if (softISPMode) {
> >               if (softISPMode != "gpu" && softISPMode != "cpu") {
> > -                     LOG(SoftwareIsp, Error) << "LIBCAMERA_SOFISP_MODE " << softISPMode.value() << " invalid "
> > -                                             << "must be \"cpu\" or \"gpu\"";
> > +                     LOG(SoftwareIsp, Error)
> > +                             << "LIBCAMERA_SOFTISP_MODE "
> > +                             << softISPMode.value() << " invalid "
> > +                             << "must be \"cpu\" or \"gpu\"";
> >                       return;
> >               }
> >       }
> >
> > --
> > 2.52.0
> >
Barnabás Pőcze Feb. 5, 2026, 10:26 a.m. UTC | #3 
2026. 02. 04. 23:42 keltezéssel, Kieran Bingham írta:
> When an invalid parameter is specified to LIBCAMERA_SOFTISP_MODE, the
> error log has a typo. Fix the typo and reflow the line while here.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>   src/libcamera/software_isp/software_isp.cpp | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index a83986b787b75ac1cd9073b32f2b937e9efb6cf0..3eb9cf97dea66cb68f2277a8274003e28418d830 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -108,8 +108,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>   	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
>   	if (softISPMode) {
>   		if (softISPMode != "gpu" && softISPMode != "cpu") {
> -			LOG(SoftwareIsp, Error) << "LIBCAMERA_SOFISP_MODE " << softISPMode.value() << " invalid "
> -						<< "must be \"cpu\" or \"gpu\"";
> +			LOG(SoftwareIsp, Error)
> +				<< "LIBCAMERA_SOFTISP_MODE "
> +				<< softISPMode.value() << " invalid "
> +				<< "must be \"cpu\" or \"gpu\"";

I think this message could be improved, at least to me something like

   LIBCAMERA_SOFTISP_MODE must be "cpu" or "gpu", got "XYZ"

is easier to understand.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   			return;
>   		}
>   	}
>
Laurent Pinchart Feb. 5, 2026, 10:51 p.m. UTC | #4 
On Thu, Feb 05, 2026 at 11:26:34AM +0100, Barnabás Pőcze wrote:
> 2026. 02. 04. 23:42 keltezéssel, Kieran Bingham írta:
> > When an invalid parameter is specified to LIBCAMERA_SOFTISP_MODE, the
> > error log has a typo. Fix the typo and reflow the line while here.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >   src/libcamera/software_isp/software_isp.cpp | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index a83986b787b75ac1cd9073b32f2b937e9efb6cf0..3eb9cf97dea66cb68f2277a8274003e28418d830 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -108,8 +108,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> >   	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
> >   	if (softISPMode) {
> >   		if (softISPMode != "gpu" && softISPMode != "cpu") {
> > -			LOG(SoftwareIsp, Error) << "LIBCAMERA_SOFISP_MODE " << softISPMode.value() << " invalid "
> > -						<< "must be \"cpu\" or \"gpu\"";
> > +			LOG(SoftwareIsp, Error)
> > +				<< "LIBCAMERA_SOFTISP_MODE "
> > +				<< softISPMode.value() << " invalid "
> > +				<< "must be \"cpu\" or \"gpu\"";
> 
> I think this message could be improved, at least to me something like
> 
>    LIBCAMERA_SOFTISP_MODE must be "cpu" or "gpu", got "XYZ"

Or
	Invalid LIBCAMERA_SOFTISP_MODE "xyz", must be "cpu" or "gpu"

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> is easier to understand.
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> >   			return;
> >   		}
> >   	}
> >

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index a83986b787b75ac1cd9073b32f2b937e9efb6cf0..3eb9cf97dea66cb68f2277a8274003e28418d830 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -108,8 +108,10 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
 	if (softISPMode) {
 		if (softISPMode != "gpu" && softISPMode != "cpu") {
-			LOG(SoftwareIsp, Error) << "LIBCAMERA_SOFISP_MODE " << softISPMode.value() << " invalid "
-						<< "must be \"cpu\" or \"gpu\"";
+			LOG(SoftwareIsp, Error)
+				<< "LIBCAMERA_SOFTISP_MODE "
+				<< softISPMode.value() << " invalid "
+				<< "must be \"cpu\" or \"gpu\"";
 			return;
 		}
 	}