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

[Laurent Pinchart]

Hi Niklas,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

On Fri, Aug 14, 2020 at 12:37:21AM +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 v1
> - Only print user-friendly names when listing cameras.
> - Update format of user-friendly names printed.
> - Update commit message.
> ---
>  src/cam/main.cpp | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index cc3facd5a5b22092..57f8c79b21090ece 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -21,6 +21,37 @@
>  
>  using namespace libcamera;
>  
> +std::string cameraName(const Camera *camera)
> +{
> +	const ControlList &props = camera->properties();
> +	std::string name;
> +
> +	/* Use camera model as fallback name if available. */
> +	if (props.contains(properties::Model))
> +		name = props.get(properties::Model);
> +	else
> +		name = "Unknown camera";

Do we need this, given that the Location property is supposed to be
mandatory ? With Umang's patch that hardcodes CameraLocationExternal in
the UVC pipeline handler, do we still have cases where Location isn't
reported ?

> +
> +	/* If camera location is available use it as highest priority name. */
> +	if (props.contains(properties::Location)) {
> +		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;
> +		}
> +	}
> +
> +	name += " (" + camera->id() + ")";
> +
> +	return name;
> +}

Can you make this a member of the CamApp class ?

> +
>  class CamApp
>  {
>  public:
> @@ -340,7 +371,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++;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart