| Message ID | 20260829094747.192641-1-robert.mader@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Robert Mader (2026-08-29 10:47:47) > On some platforms such as the PinePhone these logs clutter the output of > tools and the journal a lot. Let's silence them for normal usage. > > Signed-off-by: Robert Mader <robert.mader@collabora.com> Ack, If someone finds they need to add a new input format to the debayer, I'm sure they'll already be able to increase the log level. This will silence formats that are simply never expected to be debayered. We /could/ go further and always silently ignore RGB/YUV formats, but report more vocally if an unsupported bayer format finds it's way in here. But ... I don't think we need that yak right now. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- > src/libcamera/software_isp/debayer_egl.cpp | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp > index c6d5d1e1813b..6787473163b0 100644 > --- a/src/libcamera/software_isp/debayer_cpu.cpp > +++ b/src/libcamera/software_isp/debayer_cpu.cpp > @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf > return 0; > } > > - LOG(Debayer, Info) > - << "Unsupported input format " << inputFormat.toString(); > + LOG(Debayer, Debug) > + << "Unsupported input format " << inputFormat; > return -EINVAL; > } > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 97aa03793574..d02a3c53ec7b 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf > return 0; > } > > - LOG(Debayer, Info) > + LOG(Debayer, Debug) > << "Unsupported input format " << inputFormat; > > return -EINVAL; > -- > 2.55.0 >
Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > Quoting Robert Mader (2026-08-29 10:47:47) >> On some platforms such as the PinePhone these logs clutter the output of >> tools and the journal a lot. Let's silence them for normal usage. >> >> Signed-off-by: Robert Mader <robert.mader@collabora.com> > > Ack, > > If someone finds they need to add a new input format to the debayer, I'm > sure they'll already be able to increase the log level. > > This will silence formats that are simply never expected to be > debayered. > > We /could/ go further and always silently ignore RGB/YUV formats, but > report more vocally if an unsupported bayer format finds it's way in > here. Indeed, it looks to me like this message should be error rather than debug level. How is it that software ISP is attempted to start with an unsupported format regularly? > But ... I don't think we need that yak right now. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> --- >> src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- >> src/libcamera/software_isp/debayer_egl.cpp | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >> index c6d5d1e1813b..6787473163b0 100644 >> --- a/src/libcamera/software_isp/debayer_cpu.cpp >> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >> @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >> return 0; >> } >> >> - LOG(Debayer, Info) >> - << "Unsupported input format " << inputFormat.toString(); >> + LOG(Debayer, Debug) >> + << "Unsupported input format " << inputFormat; >> return -EINVAL; >> } >> >> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >> index 97aa03793574..d02a3c53ec7b 100644 >> --- a/src/libcamera/software_isp/debayer_egl.cpp >> +++ b/src/libcamera/software_isp/debayer_egl.cpp >> @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >> return 0; >> } >> >> - LOG(Debayer, Info) >> + LOG(Debayer, Debug) >> << "Unsupported input format " << inputFormat; >> >> return -EINVAL; >> -- >> 2.55.0 >>
Hi, On 31.08.26 13:13, Milan Zamazal wrote: > Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > >> Quoting Robert Mader (2026-08-29 10:47:47) >>> On some platforms such as the PinePhone these logs clutter the output of >>> tools and the journal a lot. Let's silence them for normal usage. >>> >>> Signed-off-by: Robert Mader <robert.mader@collabora.com> >> Ack, >> >> If someone finds they need to add a new input format to the debayer, I'm >> sure they'll already be able to increase the log level. >> >> This will silence formats that are simply never expected to be >> debayered. >> >> We /could/ go further and always silently ignore RGB/YUV formats, but >> report more vocally if an unsupported bayer format finds it's way in >> here. > Indeed, it looks to me like this message should be error rather than > debug level. For DebayerEGL that used to be the case until recently - it got demoted to Info in commit 2985ca6ac024f Quoting it here: libcamera: software_isp: debayer_egl: Demote unsupported format log to Info DebayerEGL::getInputConfig() logs at Error level when it encounters an unsupported input format. This function is called during format enumeration from the simple pipeline handler's tryPipeline(), which iterates over all pixel formats reported by the video capture device, including non-Bayer formats such as UYVY and YUYV. The caller already handles the failure gracefully by checking the return value. Similarly, DebayerEGL::getOutputConfig() logs at Error level for unsupported output formats, but is called from strideAndFrameSize() which also handles failures via the return value. Demote both log messages from Error to Info for consistency with DebayerCpu, which uses Info level in getInputConfig() and getOutputConfig() for the same situation. > How is it that software ISP is attempted to start with an > unsupported format regularly? For me it's the case on the PinePhone when enabling the softISP for sun6i-csi. The reason is that the platform has some (really bad) in-build hardware ISP, so additionally to bayer-formats some YUV formats get passed into the debayer class. > >> But ... I don't think we need that yak right now. >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> --- >>> src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- >>> src/libcamera/software_isp/debayer_egl.cpp | 2 +- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >>> index c6d5d1e1813b..6787473163b0 100644 >>> --- a/src/libcamera/software_isp/debayer_cpu.cpp >>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >>> @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >>> return 0; >>> } >>> >>> - LOG(Debayer, Info) >>> - << "Unsupported input format " << inputFormat.toString(); >>> + LOG(Debayer, Debug) >>> + << "Unsupported input format " << inputFormat; >>> return -EINVAL; >>> } >>> >>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >>> index 97aa03793574..d02a3c53ec7b 100644 >>> --- a/src/libcamera/software_isp/debayer_egl.cpp >>> +++ b/src/libcamera/software_isp/debayer_egl.cpp >>> @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >>> return 0; >>> } >>> >>> - LOG(Debayer, Info) >>> + LOG(Debayer, Debug) >>> << "Unsupported input format " << inputFormat; >>> >>> return -EINVAL; >>> -- >>> 2.55.0 >>>
Hi Robert, Robert Mader <robert.mader@collabora.com> writes: > Hi, > > On 31.08.26 13:13, Milan Zamazal wrote: >> Kieran Bingham <kieran.bingham@ideasonboard.com> writes: >> >>> Quoting Robert Mader (2026-08-29 10:47:47) >>>> On some platforms such as the PinePhone these logs clutter the output of >>>> tools and the journal a lot. Let's silence them for normal usage. >>>> >>>> Signed-off-by: Robert Mader <robert.mader@collabora.com> >>> Ack, >>> >>> If someone finds they need to add a new input format to the debayer, I'm >>> sure they'll already be able to increase the log level. >>> >>> This will silence formats that are simply never expected to be >>> debayered. >>> >>> We /could/ go further and always silently ignore RGB/YUV formats, but >>> report more vocally if an unsupported bayer format finds it's way in >>> here. >> Indeed, it looks to me like this message should be error rather than >> debug level. > > For DebayerEGL that used to be the case until recently - it got demoted to Info in commit 2985ca6ac024f > > Quoting it here: > > libcamera: software_isp: debayer_egl: Demote unsupported format log to Info > > DebayerEGL::getInputConfig() logs at Error level when it encounters an > unsupported input format. This function is called during format > enumeration from the simple pipeline handler's tryPipeline(), which > iterates over all pixel formats reported by the video capture device, > including non-Bayer formats such as UYVY and YUYV. The caller already > handles the failure gracefully by checking the return value. Similarly, > DebayerEGL::getOutputConfig() logs at Error level for unsupported > output formats, but is called from strideAndFrameSize() which also > handles failures via the return value. > > Demote both log messages from Error to Info for consistency with > DebayerCpu, which uses Info level in getInputConfig() and > getOutputConfig() for the same situation. > Thank you for the pointer. >> How is it that software ISP is attempted to start with an >> unsupported format regularly? > > For me it's the case on the PinePhone when enabling the softISP for sun6i-csi. The reason is that the > platform has some (really bad) in-build hardware ISP, so additionally to bayer-formats some YUV formats > get passed into the debayer class. Looking again, the problem seems to me to be that getInputConfig is used for multiple purposes. When called from DebayerEGL::formats, it's a question and completely appropriate to silence the message, but when called from DebayerEGL::configure, it's an assertion and the message shouldn't be hidden, right? Would a new argument to getInputConfig distinguishing those usages be a good solution? >> >>> But ... I don't think we need that yak right now. >>> >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >>> >>>> --- >>>> src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- >>>> src/libcamera/software_isp/debayer_egl.cpp | 2 +- >>>> 2 files changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >>>> index c6d5d1e1813b..6787473163b0 100644 >>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp >>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >>>> @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >>>> return 0; >>>> } >>>> - LOG(Debayer, Info) >>>> - << "Unsupported input format " << inputFormat.toString(); >>>> + LOG(Debayer, Debug) >>>> + << "Unsupported input format " << inputFormat; >>>> return -EINVAL; >>>> } >>>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >>>> index 97aa03793574..d02a3c53ec7b 100644 >>>> --- a/src/libcamera/software_isp/debayer_egl.cpp >>>> +++ b/src/libcamera/software_isp/debayer_egl.cpp >>>> @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >>>> return 0; >>>> } >>>> - LOG(Debayer, Info) >>>> + LOG(Debayer, Debug) >>>> << "Unsupported input format " << inputFormat; >>>> return -EINVAL; >>>> -- 2.55.0 >>>>
On Tue, Sep 01, 2026 at 03:52:53PM +0200, Milan Zamazal wrote: > Robert Mader writes: > > On 31.08.26 13:13, Milan Zamazal wrote: > >> Kieran Bingham writes: > >>> Quoting Robert Mader (2026-08-29 10:47:47) > >>>> On some platforms such as the PinePhone these logs clutter the output of > >>>> tools and the journal a lot. Let's silence them for normal usage. > >>>> > >>>> Signed-off-by: Robert Mader <robert.mader@collabora.com> > >>> Ack, > >>> > >>> If someone finds they need to add a new input format to the debayer, I'm > >>> sure they'll already be able to increase the log level. > >>> > >>> This will silence formats that are simply never expected to be > >>> debayered. > >>> > >>> We /could/ go further and always silently ignore RGB/YUV formats, but > >>> report more vocally if an unsupported bayer format finds it's way in > >>> here. > >> Indeed, it looks to me like this message should be error rather than > >> debug level. > > > > For DebayerEGL that used to be the case until recently - it got demoted to Info in commit 2985ca6ac024f > > > > Quoting it here: > > > > libcamera: software_isp: debayer_egl: Demote unsupported format log to Info > > > > DebayerEGL::getInputConfig() logs at Error level when it encounters an > > unsupported input format. This function is called during format > > enumeration from the simple pipeline handler's tryPipeline(), which > > iterates over all pixel formats reported by the video capture device, > > including non-Bayer formats such as UYVY and YUYV. The caller already > > handles the failure gracefully by checking the return value. Similarly, > > DebayerEGL::getOutputConfig() logs at Error level for unsupported > > output formats, but is called from strideAndFrameSize() which also > > handles failures via the return value. > > > > Demote both log messages from Error to Info for consistency with > > DebayerCpu, which uses Info level in getInputConfig() and > > getOutputConfig() for the same situation. > > > > Thank you for the pointer. > > >> How is it that software ISP is attempted to start with an > >> unsupported format regularly? > > > > For me it's the case on the PinePhone when enabling the softISP for sun6i-csi. The reason is that the > > platform has some (really bad) in-build hardware ISP, so additionally to bayer-formats some YUV formats > > get passed into the debayer class. > > Looking again, the problem seems to me to be that getInputConfig is used > for multiple purposes. When called from DebayerEGL::formats, it's a > question and completely appropriate to silence the message, but when > called from DebayerEGL::configure, it's an assertion and the message > shouldn't be hidden, right? Would a new argument to getInputConfig > distinguishing those usages be a good solution? A parameter to control logging often indicates a bad design. > >>> But ... I don't think we need that yak right now. > >>> > >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >>> > >>>> --- > >>>> src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- > >>>> src/libcamera/software_isp/debayer_egl.cpp | 2 +- > >>>> 2 files changed, 3 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp > >>>> index c6d5d1e1813b..6787473163b0 100644 > >>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp > >>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp > >>>> @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf > >>>> return 0; > >>>> } > >>>> - LOG(Debayer, Info) > >>>> - << "Unsupported input format " << inputFormat.toString(); > >>>> + LOG(Debayer, Debug) > >>>> + << "Unsupported input format " << inputFormat; > >>>> return -EINVAL; > >>>> } > >>>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > >>>> index 97aa03793574..d02a3c53ec7b 100644 > >>>> --- a/src/libcamera/software_isp/debayer_egl.cpp > >>>> +++ b/src/libcamera/software_isp/debayer_egl.cpp > >>>> @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf > >>>> return 0; > >>>> } > >>>> - LOG(Debayer, Info) > >>>> + LOG(Debayer, Debug) > >>>> << "Unsupported input format " << inputFormat; > >>>> return -EINVAL;
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Tue, Sep 01, 2026 at 03:52:53PM +0200, Milan Zamazal wrote: >> Robert Mader writes: >> > On 31.08.26 13:13, Milan Zamazal wrote: > >> >> Kieran Bingham writes: >> >>> Quoting Robert Mader (2026-08-29 10:47:47) >> >>>> On some platforms such as the PinePhone these logs clutter the output of >> >>>> tools and the journal a lot. Let's silence them for normal usage. >> >>>> >> >>>> Signed-off-by: Robert Mader <robert.mader@collabora.com> >> >>> Ack, >> >>> >> >>> If someone finds they need to add a new input format to the debayer, I'm >> >>> sure they'll already be able to increase the log level. >> >>> >> >>> This will silence formats that are simply never expected to be >> >>> debayered. >> >>> >> >>> We /could/ go further and always silently ignore RGB/YUV formats, but >> >>> report more vocally if an unsupported bayer format finds it's way in >> >>> here. >> >> Indeed, it looks to me like this message should be error rather than >> >> debug level. >> > >> > For DebayerEGL that used to be the case until recently - it got demoted to Info in commit 2985ca6ac024f >> > >> > Quoting it here: >> > >> > libcamera: software_isp: debayer_egl: Demote unsupported format log to Info >> > >> > DebayerEGL::getInputConfig() logs at Error level when it encounters an >> > unsupported input format. This function is called during format >> > enumeration from the simple pipeline handler's tryPipeline(), which >> > iterates over all pixel formats reported by the video capture device, >> > including non-Bayer formats such as UYVY and YUYV. The caller already >> > handles the failure gracefully by checking the return value. Similarly, >> > DebayerEGL::getOutputConfig() logs at Error level for unsupported >> > output formats, but is called from strideAndFrameSize() which also >> > handles failures via the return value. >> > >> > Demote both log messages from Error to Info for consistency with >> > DebayerCpu, which uses Info level in getInputConfig() and >> > getOutputConfig() for the same situation. >> > >> >> Thank you for the pointer. >> >> >> How is it that software ISP is attempted to start with an >> >> unsupported format regularly? >> > >> > For me it's the case on the PinePhone when enabling the softISP for sun6i-csi. The reason is that the >> > platform has some (really bad) in-build hardware ISP, so additionally to bayer-formats some YUV formats >> > get passed into the debayer class. >> >> Looking again, the problem seems to me to be that getInputConfig is used >> for multiple purposes. When called from DebayerEGL::formats, it's a >> question and completely appropriate to silence the message, but when >> called from DebayerEGL::configure, it's an assertion and the message >> shouldn't be hidden, right? Would a new argument to getInputConfig >> distinguishing those usages be a good solution? > > A parameter to control logging often indicates a bad design. Right; since getInputConfig returns an error only on unsupported input formats, an error can be logged by the caller when needed. >> >>> But ... I don't think we need that yak right now. >> >>> >> >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> >> >>>> --- >> >>>> src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- >> >>>> src/libcamera/software_isp/debayer_egl.cpp | 2 +- >> >>>> 2 files changed, 3 insertions(+), 3 deletions(-) >> >>>> >> >>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp >> >>>> index c6d5d1e1813b..6787473163b0 100644 >> >>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp >> >>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp >> >>>> @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >> >>>> return 0; >> >>>> } >> >>>> - LOG(Debayer, Info) >> >>>> - << "Unsupported input format " << inputFormat.toString(); >> >>>> + LOG(Debayer, Debug) >> >>>> + << "Unsupported input format " << inputFormat; >> >>>> return -EINVAL; >> >>>> } >> >>>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >> >>>> index 97aa03793574..d02a3c53ec7b 100644 >> >>>> --- a/src/libcamera/software_isp/debayer_egl.cpp >> >>>> +++ b/src/libcamera/software_isp/debayer_egl.cpp >> >>>> @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf >> >>>> return 0; >> >>>> } >> >>>> - LOG(Debayer, Info) >> >>>> + LOG(Debayer, Debug) >> >>>> << "Unsupported input format " << inputFormat; >> >>>> return -EINVAL;
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index c6d5d1e1813b..6787473163b0 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -470,8 +470,8 @@ int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf return 0; } - LOG(Debayer, Info) - << "Unsupported input format " << inputFormat.toString(); + LOG(Debayer, Debug) + << "Unsupported input format " << inputFormat; return -EINVAL; } diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp index 97aa03793574..d02a3c53ec7b 100644 --- a/src/libcamera/software_isp/debayer_egl.cpp +++ b/src/libcamera/software_isp/debayer_egl.cpp @@ -91,7 +91,7 @@ int DebayerEGL::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &conf return 0; } - LOG(Debayer, Info) + LOG(Debayer, Debug) << "Unsupported input format " << inputFormat; return -EINVAL;
On some platforms such as the PinePhone these logs clutter the output of tools and the journal a lot. Let's silence them for normal usage. Signed-off-by: Robert Mader <robert.mader@collabora.com> --- src/libcamera/software_isp/debayer_cpu.cpp | 4 ++-- src/libcamera/software_isp/debayer_egl.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)