[v2] libcamera: camera_manager: Log info message to report camera creation

22904 diff mbox series

Camera creation is one of the most important events generated by
libcamera, but we are completely silent about it. The lack of a log
message makes it more difficult to identify problems and provide
support. Fix it by adding an Info message that reports the camera id and
its pipeline handler when the camera is added.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Move message to CameraManager class
---
 src/libcamera/camera_manager.cpp | 4 ++++
 1 file changed, 4 insertions(+)


base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
--
Regards,

Laurent Pinchart

Quoting Laurent Pinchart (2025-03-03 13:45:18)
> Camera creation is one of the most important events generated by
> libcamera, but we are completely silent about it. The lack of a log
> message makes it more difficult to identify problems and provide
> support. Fix it by adding an Info message that reports the camera id and
> its pipeline handler when the camera is added.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I think you could have kept tags...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> Changes since v1:
> 
> - Move message to CameraManager class
> ---
>  src/libcamera/camera_manager.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 87e6717ece91..942a72dcfc96 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -217,6 +217,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
> 
>         unsigned int index = cameras_.size() - 1;
> 
> +       LOG(Camera, Info)
> +               << "Adding camera '" << camera->id() << "' for pipeline handler "
> +               << camera->_d()->pipe()->name();
> +
>         /* Report the addition to the public signal */
>         CameraManager *const o = LIBCAMERA_O_PTR();
>         o->cameraAdded.emit(cameras_[index]);
> 
> base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
> --
> Regards,
> 
> Laurent Pinchart
>

Hi


2025. 03. 03. 14:45 keltezéssel, Laurent Pinchart írta:
> Camera creation is one of the most important events generated by
> libcamera, but we are completely silent about it. The lack of a log
> message makes it more difficult to identify problems and provide
> support. Fix it by adding an Info message that reports the camera id and
> its pipeline handler when the camera is added.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Move message to CameraManager class
> ---
>   src/libcamera/camera_manager.cpp | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 87e6717ece91..942a72dcfc96 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -217,6 +217,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
> 
>   	unsigned int index = cameras_.size() - 1;
> 
> +	LOG(Camera, Info)
> +		<< "Adding camera '" << camera->id() << "' for pipeline handler "
> +		<< camera->_d()->pipe()->name();

I think the `camera` pointer is moved-from at this point, is it not nullptr?
Am I missing something? If it is nullptr, then please see https://patchwork.libcamera.org/patch/22905/
applying that first will remove the issue.


Regards,
Barnabás Pőcze


> +
>   	/* Report the addition to the public signal */
>   	CameraManager *const o = LIBCAMERA_O_PTR();
>   	o->cameraAdded.emit(cameras_[index]);
> 
> base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
> --
> Regards,
> 
> Laurent Pinchart
>

On Mon, Mar 03, 2025 at 03:39:09PM +0100, Barnabás Pőcze wrote:
> 2025. 03. 03. 14:45 keltezéssel, Laurent Pinchart írta:
> > Camera creation is one of the most important events generated by
> > libcamera, but we are completely silent about it. The lack of a log
> > message makes it more difficult to identify problems and provide
> > support. Fix it by adding an Info message that reports the camera id and
> > its pipeline handler when the camera is added.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Move message to CameraManager class
> > ---
> >   src/libcamera/camera_manager.cpp | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index 87e6717ece91..942a72dcfc96 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -217,6 +217,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
> > 
> >   	unsigned int index = cameras_.size() - 1;
> > 
> > +	LOG(Camera, Info)
> > +		<< "Adding camera '" << camera->id() << "' for pipeline handler "
> > +		<< camera->_d()->pipe()->name();
> 
> I think the `camera` pointer is moved-from at this point, is it not
> nullptr?

I completely messed up testing this patch due to a silly mistake in my
environment :-/ You're absolutely right.

> Am I missing something? If it is nullptr, then please see https://patchwork.libcamera.org/patch/22905/
> applying that first will remove the issue.

I've reviewed that patch. After you merge it, I'll send a v3 of this
one.

> > +
> >   	/* Report the addition to the public signal */
> >   	CameraManager *const o = LIBCAMERA_O_PTR();
> >   	o->cameraAdded.emit(cameras_[index]);
> > 
> > base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4