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

[Niklas Söderlund]

Hi Kieran,

Thanks for your feedback.

On 2020-08-13 16:49:26 +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 13/08/2020 10:57, 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>
> > ---
> > * 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";
> > +
> > +	/* 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;
> > +		}
> > +	}
> 
> 
> Err, aren't we going to make UVC cameras all marked as
> CameraLocationExternal?
> 
> I'm not sure I'd use this as the 'highest' priority, more as the lowest
> priority if the model property wasn't known but the location was...

Laurent made a good case for having location front and back as highest 
priority as it makes most sens for users, and I would like to keep this.  
Model really is only useful for us in this case.

I agree with the proposed change to mark all UVC cameras as external vs 
not giving them a location maybe properties::Model should be used 
instead of "External camera"?

As I commented on the series which changes this for the UVC pipeline I'm 
not sure location is the best property to introduce to UVC as what that 
series really want is to know if the camera is hot-pluggable nor not.  
How about we keep this as it is for now and adopt it once we know how 
the other series turns out? I feel chancing this patch now for something 
we don't know might be busy work.

> 
> Though I don't think we'll ever not have a model would we ?

We will always have a model, but it's not enforced (yet).

> 
> 
> 
> 
> > +
> > +	name += " (" + camera->id() + ")";
> > +
> > +	return name;
> > +}
> > +
> >  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
> --
> Kieran

-- 
Regards,
Niklas Söderlund