[libcamera-devel] [PATCH v5 6/7] cam: Print user-friendly camera names

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Sep 25, 2020 at 05:07:42PM +0200, Niklas Söderlund wrote:
> Instead of only printing the camera ID which is not intended for humans
> to read and parse create a more user friendly string when printing
> camera names. The ID is still printed as it is one option used to select
> camera using the --camera option.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> * Changes since v4
> - Make cameraName() member ofr CamApp.
> 
> * Changes since v1
> - Only print user-friendly names when listing cameras.
> - Update format of user-friendly names printed.
> - Update commit message.
> ---
>  src/cam/main.cpp | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 244720b491f5c462..ed56d06c9d3386b9 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -45,6 +45,8 @@ private:
>  	int infoConfiguration();
>  	int run();
>  
> +	std::string cameraName(const Camera *camera);
> +
>  	static CamApp *app_;
>  	OptionsParser::Options options_;
>  	CameraManager *cm_;
> @@ -340,7 +342,7 @@ int CamApp::run()
>  
>  		unsigned int index = 1;
>  		for (const std::shared_ptr<Camera> &cam : cm_->cameras()) {
> -			std::cout << index << ": " << cam->id() << std::endl;
> +			std::cout << index << ": " << cameraName(cam.get()) << std::endl;
>  			index++;
>  		}
>  	}
> @@ -378,6 +380,33 @@ int CamApp::run()
>  	return 0;
>  }
>  
> +std::string CamApp::cameraName(const Camera *camera)
> +{
> +	const ControlList &props = camera->properties();
> +	std::string name;
> +
> +	if (props.contains(properties::Location)) {

As Location is mandatory, I would consider this to be a bug in
libcamera. Do we need to handle it explicitly ?

> +		switch (props.get(properties::Location)) {
> +		case properties::CameraLocationFront:
> +			name = "Internal front camera";
> +			break;
> +		case properties::CameraLocationBack:
> +			name = "Internal back camera";
> +			break;
> +		case properties::CameraLocationExternal:
> +			name = "External camera";
> +			break;
> +		}
> +	}
> +
> +	if (props.contains(properties::Model))
> +		name += " " + props.get(properties::Model);

For USB cameras, ideally I'd like to see something like

"External USB camera 'Logitech StreamCam' (on port 3)"

We don't have the infrastructure to know it's a USB camera yet, or to
know the port number, so we can leave that out of now. For internal
cameras, however, the sensor model should at best be printed under
parentheses (or in any other way that makes it apparent it's a detail),
if at all. As cam is more developer-oriented printing the camera sensor
model is probably useful, but if we consider it as an example
application, from an end-user point of view, the sensor model isn't very
relevant.

> +
> +	name += " (" + camera->id() + ")";
> +
> +	return name;
> +}
> +
>  void signalHandler([[maybe_unused]] int signal)
>  {
>  	std::cout << "Exiting" << std::endl;

-- 
Regards,

Laurent Pinchart