| Message ID | 20240627173305.1477718-3-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Milan Zamazal (2024-06-27 18:33:05) > SimplePipelineHandler::match may be called several times for different > pipeline configurations. Not all of these calls must succeed. For > example, for TI AM69 board with a single camera attached, the following > error is reported in the log even when libcamera works fine: > > ERROR SimplePipeline simple.cpp:1558 No sensor found > > This is because a sensor is found for /dev/media0 but not for > /dev/media1. The error is harmless in such a case and only confuses > users who may think no camera is detected at all. Let's change the > error to info and add the device node to the message to indicate the > error is specific to the given media only. It's up to the callers to > report a fatal error condition if libcamera cannot work due to no > matching pipeline configuration. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index eb36578e..be0bb677 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1548,9 +1548,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > /* Locate the sensors. */ > std::vector<MediaEntity *> sensors = locateSensors(); > if (sensors.empty()) { > - LOG(SimplePipeline, Error) << "No sensor found"; > + LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode(); > return false; > } > + LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode(); These I like. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Is logPrefix defined for simple pipeline handler yet? media_->deviceNode() could be part of that perhaps (unless it's too long) to make it clear /which/ media device is in use. > > /* > * Create one camera data instance for each sensor and gather all > -- > 2.44.1 >
Hi Kieran, thank you for review. Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > Quoting Milan Zamazal (2024-06-27 18:33:05) >> SimplePipelineHandler::match may be called several times for different >> pipeline configurations. Not all of these calls must succeed. For > >> example, for TI AM69 board with a single camera attached, the following >> error is reported in the log even when libcamera works fine: >> >> ERROR SimplePipeline simple.cpp:1558 No sensor found >> >> This is because a sensor is found for /dev/media0 but not for >> /dev/media1. The error is harmless in such a case and only confuses >> users who may think no camera is detected at all. Let's change the >> error to info and add the device node to the message to indicate the >> error is specific to the given media only. It's up to the callers to >> report a fatal error condition if libcamera cannot work due to no >> matching pipeline configuration. >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/pipeline/simple/simple.cpp | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index eb36578e..be0bb677 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -1548,9 +1548,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) >> /* Locate the sensors. */ >> std::vector<MediaEntity *> sensors = locateSensors(); >> if (sensors.empty()) { >> - LOG(SimplePipeline, Error) << "No sensor found"; >> + LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode(); >> return false; >> } >> + LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode(); > > These I like. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Is logPrefix defined for simple pipeline handler yet? No, it is defined only in the algorithm-based software ISP refactoring patches. > media_->deviceNode() could be part of that perhaps (unless it's too > long) to make it clear /which/ media device is in use. A good idea, I'll consider it in the next iteration of the refactoring patches. >> /* >> * Create one camera data instance for each sensor and gather all >> -- >> 2.44.1 >>
On Mon, Jul 01, 2024 at 11:37:37AM +0100, Kieran Bingham wrote: > Quoting Milan Zamazal (2024-06-27 18:33:05) > > SimplePipelineHandler::match may be called several times for different > > pipeline configurations. Not all of these calls must succeed. For > > example, for TI AM69 board with a single camera attached, the following > > error is reported in the log even when libcamera works fine: > > > > ERROR SimplePipeline simple.cpp:1558 No sensor found > > > > This is because a sensor is found for /dev/media0 but not for > > /dev/media1. The error is harmless in such a case and only confuses > > users who may think no camera is detected at all. Let's change the > > error to info and add the device node to the message to indicate the > > error is specific to the given media only. It's up to the callers to > > report a fatal error condition if libcamera cannot work due to no > > matching pipeline configuration. > > > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > > --- > > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index eb36578e..be0bb677 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -1548,9 +1548,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > > /* Locate the sensors. */ > > std::vector<MediaEntity *> sensors = locateSensors(); > > if (sensors.empty()) { > > - LOG(SimplePipeline, Error) << "No sensor found"; > > + LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode(); LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode(); > > return false; > > } Add a blank line. > > + LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode(); > > These I like. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Is logPrefix defined for simple pipeline handler yet? > media_->deviceNode() could be part of that perhaps (unless it's too > long) to make it clear /which/ media device is in use. We don't use logPrefix in any pipeline handler. > > > > /* > > * Create one camera data instance for each sensor and gather all
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index eb36578e..be0bb677 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1548,9 +1548,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) /* Locate the sensors. */ std::vector<MediaEntity *> sensors = locateSensors(); if (sensors.empty()) { - LOG(SimplePipeline, Error) << "No sensor found"; + LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode(); return false; } + LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode(); /* * Create one camera data instance for each sensor and gather all
SimplePipelineHandler::match may be called several times for different pipeline configurations. Not all of these calls must succeed. For example, for TI AM69 board with a single camera attached, the following error is reported in the log even when libcamera works fine: ERROR SimplePipeline simple.cpp:1558 No sensor found This is because a sensor is found for /dev/media0 but not for /dev/media1. The error is harmless in such a case and only confuses users who may think no camera is detected at all. Let's change the error to info and add the device node to the message to indicate the error is specific to the given media only. It's up to the callers to report a fatal error condition if libcamera cannot work due to no matching pipeline configuration. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)