[libcamera-devel] [Simple-Cam: PATCH] simple-cam: Use friendly camera names

[Jacopo Mondi]

Hi Kieran,

On Wed, Oct 21, 2020 at 04:40:43PM +0100, Kieran Bingham wrote:
> Take the example code for generating a camera name from 'cam' and use
> it when reporting cameras within the simple-cam application.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>
> I wonder if this cameraName() might be something that
> should go into libcamera::utils as a public API helper?
>
>
> I know we want to allow applications to decide their own naming too, but
> I think we've discussed in the past about having a helper library on top
> to make things easier for application developers ?
>
>
>  simple-cam.cpp | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/simple-cam.cpp b/simple-cam.cpp
> index 727bb6d86480..1b6fd3939bf6 100644
> --- a/simple-cam.cpp
> +++ b/simple-cam.cpp
> @@ -60,6 +60,30 @@ static void requestComplete(Request *request)
>  	camera->queueRequest(request);
>  }
>
> +std::string cameraName(std::shared_ptr<libcamera::Camera> camera)

nit: camera can be a reference, or just a raw pointer. No need to
increase/decrease the usage count just for the scope of this function.

> +{
> +	const ControlList &props = camera->properties();
> +	std::string name;
> +
> +	switch (props.get(properties::Location)) {
> +	case properties::CameraLocationFront:
> +		name = "Internal front camera";
> +		break;

I wonder if defaulting to "Front" in CameraSensor is still a good idea
or we should let applications deals with that case...

> +	case properties::CameraLocationBack:
> +		name = "Internal back camera";
> +		break;
> +	case properties::CameraLocationExternal:
> +		name = "External camera";
> +		if (props.contains(properties::Model))
> +			name += " '" + props.get(properties::Model) + "'";

Why model is only considered for external cameras ?

With the last question clarified
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j


> +		break;
> +	}
> +
> +	name += " (" + camera->id() + ")";
> +
> +	return name;
> +}
> +
>  int main()
>  {
>  	/*
> @@ -77,11 +101,11 @@ int main()
>  	cm->start();
>
>  	/*
> -	 * Just as a test, list all id's of the Camera registered in the
> -	 * system. They are indexed by name by the CameraManager.
> +	 * Just as a test, generate names of the Cameras registered in the
> +	 * system, and list them.
>  	 */
>  	for (auto const &camera : cm->cameras())
> -		std::cout << camera->id() << std::endl;
> +		std::cout << " - " << cameraName(camera) << std::endl;
>
>  	/*
>  	 * --------------------------------------------------------------------
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel