libcamera: camera: Log info message to report camera creation
diff mbox series

Message ID 20250226005547.30237-1-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • libcamera: camera: Log info message to report camera creation
Related show

Commit Message

Laurent Pinchart Feb. 26, 2025, 12:55 a.m. UTC 
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 created.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/camera.cpp | 4 ++++
 1 file changed, 4 insertions(+)


base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
--
Regards,

Laurent Pinchart

Comments

Paul Elder Feb. 26, 2025, 6:04 a.m. UTC | #1 
On Wed, Feb 26, 2025 at 02:55:47AM +0200, Laurent Pinchart wrote:
> 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 created.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/camera.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 56c5851993c9..35097bfdafa2 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -862,6 +862,10 @@ std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
>  		}
>  	};
> 
> +	LOG(Camera, Info)
> +		<< "Creating camera '" << id << "' for pipeline handler "
> +		<< d->pipe_->name();
> +
>  	Camera *camera = new Camera(std::move(d), id, streams);
> 
>  	return std::shared_ptr<Camera>(camera, Deleter());
> 
> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
> --
> Regards,
> 
> Laurent Pinchart
>
adam@piggz.co.uk March 2, 2025, 8:35 p.m. UTC | #2 
I have tested this with the uvcvideo handler and it worked as expected.

Tested-by: Adam Pigg <adam@piggz.co.uk>


On Wednesday, 26 February 2025 00:55:47 Greenwich Mean Time Laurent Pinchart 
wrote:
> 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 created.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
> --
> Regards,
> 
> Laurent Pinchart
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 56c5851993c9..35097bfdafa2 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -862,6 +862,10 @@ std::shared_ptr<Camera>
> Camera::create(std::unique_ptr<Private> d, }
>  	};
> 
> +	LOG(Camera, Info)
> +		<< "Creating camera '" << id << "' for pipeline handler 
"
> +		<< d->pipe_->name();
> +
>  	Camera *camera = new Camera(std::move(d), id, streams);
> 
>  	return std::shared_ptr<Camera>(camera, Deleter());
Kieran Bingham March 3, 2025, 1:01 p.m. UTC | #3 
Quoting Laurent Pinchart (2025-02-26 00:55:47)
> 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 created.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 56c5851993c9..35097bfdafa2 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -862,6 +862,10 @@ std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
>                 }
>         };
> 
> +       LOG(Camera, Info)
> +               << "Creating camera '" << id << "' for pipeline handler "
> +               << d->pipe_->name();
> +

I'm fine with this indeed.

Later on I might send the patch I have that puts the pipeline handler
into the CameraID too ;-)


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

>         Camera *camera = new Camera(std::move(d), id, streams);
> 
>         return std::shared_ptr<Camera>(camera, Deleter());
> 
> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
> --
> Regards,
> 
> Laurent Pinchart
>
Barnabás Pőcze March 3, 2025, 1:10 p.m. UTC | #4 
Hi


2025. 02. 26. 1:55 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 created.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   src/libcamera/camera.cpp | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 56c5851993c9..35097bfdafa2 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -862,6 +862,10 @@ std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
>   		}
>   	};
> 
> +	LOG(Camera, Info)
> +		<< "Creating camera '" << id << "' for pipeline handler "
> +		<< d->pipe_->name();
> +

I have a similar change locally but it adds log messages to `CameraManager::{add,remove}Camera()`,
not sure which is the better place, but I think the removal also deserves at
least a "Debug" message. I am also wondering if the "Info" level is not too high?


Regards,
Barnabás Pőcze


>   	Camera *camera = new Camera(std::move(d), id, streams);
> 
>   	return std::shared_ptr<Camera>(camera, Deleter());
> 
> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
> --
> Regards,
> 
> Laurent Pinchart
>
Barnabás Pőcze March 3, 2025, 1:18 p.m. UTC | #5 
2025. 03. 03. 14:10 keltezéssel, Barnabás Pőcze írta:
> Hi
> 
> 
> 2025. 02. 26. 1:55 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 created.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>   src/libcamera/camera.cpp | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
>> index 56c5851993c9..35097bfdafa2 100644
>> --- a/src/libcamera/camera.cpp
>> +++ b/src/libcamera/camera.cpp
>> @@ -862,6 +862,10 @@ std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
>>           }
>>       };
>>
>> +    LOG(Camera, Info)
>> +        << "Creating camera '" << id << "' for pipeline handler "
>> +        << d->pipe_->name();
>> +
> 
> I have a similar change locally but it adds log messages to `CameraManager::{add,remove}Camera()`,
> not sure which is the better place, but I think the removal also deserves at
> least a "Debug" message. I am also wondering if the "Info" level is not too high?

Sorry, there is already a message in `removeCamera()`... I don't know what I was hallucinating...


> 
> 
> Regards,
> Barnabás Pőcze
> 
> 
>>       Camera *camera = new Camera(std::move(d), id, streams);
>>
>>       return std::shared_ptr<Camera>(camera, Deleter());
>>
>> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>
Laurent Pinchart March 3, 2025, 1:34 p.m. UTC | #6 
On Mon, Mar 03, 2025 at 02:18:20PM +0100, Barnabás Pőcze wrote:
> 2025. 03. 03. 14:10 keltezéssel, Barnabás Pőcze írta:
> > 2025. 02. 26. 1:55 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 created.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >>   src/libcamera/camera.cpp | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> >> index 56c5851993c9..35097bfdafa2 100644
> >> --- a/src/libcamera/camera.cpp
> >> +++ b/src/libcamera/camera.cpp
> >> @@ -862,6 +862,10 @@ std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
> >>           }
> >>       };
> >>
> >> +    LOG(Camera, Info)
> >> +        << "Creating camera '" << id << "' for pipeline handler "
> >> +        << d->pipe_->name();
> >> +
> > 
> > I have a similar change locally but it adds log messages to `CameraManager::{add,remove}Camera()`,
> > not sure which is the better place, but I think the removal also deserves at
> > least a "Debug" message. I am also wondering if the "Info" level is not too high?
> 
> Sorry, there is already a message in `removeCamera()`... I don't know
> what I was hallucinating...

The message could go to CameraManager::Private::addCamera() indeed, to
match CameraManager::Private::removeCamera(). I've picked Info on
purpose, I think this is an important event that deserves it.

I'll send a v2 that moves the message to the CameraManager class.

> >>       Camera *camera = new Camera(std::move(d), id, streams);
> >>
> >>       return std::shared_ptr<Camera>(camera, Deleter());
> >>
> >> base-commit: 33ce463a46c44f874fdbc3e484bee730e7b251a3

Patch
diff mbox series 

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 56c5851993c9..35097bfdafa2 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -862,6 +862,10 @@  std::shared_ptr<Camera> Camera::create(std::unique_ptr<Private> d,
 		}
 	};

+	LOG(Camera, Info)
+		<< "Creating camera '" << id << "' for pipeline handler "
+		<< d->pipe_->name();
+
 	Camera *camera = new Camera(std::move(d), id, streams);

 	return std::shared_ptr<Camera>(camera, Deleter());