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

[Kieran Bingham]

Hi Niklas,


On 06/08/2020 14:46, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> On 2020-08-06 15:40:02 +0200, Jacopo Mondi wrote:
>> Hi Niklas,
>>
>> On Thu, Aug 06, 2020 at 03:32:49PM +0200, Niklas Söderlund wrote:
>>> Hi Jacopo,
>>>
>>> Thanks for your feedback.
>>>
>>> On 2020-08-06 15:28:04 +0200, Jacopo Mondi wrote:
>>>> Hi Niklas,
>>>>
>>>> On Thu, Aug 06, 2020 at 03:09:36PM +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 prating 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>
>>>>> ---
>>>>>  src/cam/main.cpp | 41 +++++++++++++++++++++++++++++++++++++----
>>>>>  1 file changed, 37 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
>>>>> index cc3facd5a5b22092..5af76f6965ef2387 100644
>>>>> --- a/src/cam/main.cpp
>>>>> +++ b/src/cam/main.cpp
>>>>> @@ -21,6 +21,39 @@
>>>>>
>>>>>  using namespace libcamera;
>>>>>
>>>>> +std::string cameraName(const Camera *camera)
>>>>> +{
>>>>> +	const ControlList &props = camera->properties();
>>>>> +	std::string name;
>>>>> +
>>>>> +	if (props.contains(properties::Model))
>>>>> +		name += props.get(properties::Model) + " ";
>>>>> +
>>>>> +	if (props.contains(properties::Location)) {
>>>>> +		switch (props.get(properties::Location)) {
>>>>> +		case properties::CameraLocationFront:
>>>>> +			name += "facing front ";
>>>>> +			break;
>>>>> +		case properties::CameraLocationBack:
>>>>> +			name += "facing back ";
>>>>> +			break;
>>>>> +		case properties::CameraLocationExternal:
>>>>> +			name += "external ";
>>>>> +			break;
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>> +	if (props.contains(properties::Rotation))
>>>>> +		name += "rotated " + std::to_string(props.get(properties::Rotation)) + " degrees ";
>>>>> +
>>>>> +	if (!name.empty())
>>>>> +		name += "with id ";
>>>>
>>>> Just a quick question while skimming through the series. cam can
>>>> printout camera properties, do we need to make 'friendly' names
>>>> 100 characters to repeat what's already available there ?
>>>
>>> I know it can print properties :-)
>>>
>>> I'm happy to change this to contain more or less information, my main
>>> goal of throwing in everything here is to showcase with cam how
>>> applications can create names.
>>>
>>> What properties would you like to see make up the user-friendly name?
>>
>> None if not useful to distinguish between cameras with the same name ?
>>
>> ie
>>         1- ov5670 (front)
>>         2- ov5670 (back)
>>
>> I know we could have
>>         1- ov5670 (external)
>>         2- ov5670 (external)
> 
> I like this and will use it for next version.
> 
> But I really think we need to print the id as well as it is one of two 
> possible augments to --camera to select which one is used. How about
> 
>     1: ov5695 (front) - /base/i2c at ff160000/camera at 36
>     2: ov2685 (back) - /base/i2c at ff160000/camera at 3c

I know we can reference cameras by index, but when referencing by name,
does this mean the user would have to type/paste the whole line? or will
it match on first unique match?

cam -C -c "ov2685 (back)"

is almost friendly enough to type ...

cam -C -c "ov2685 (back) - /base/i2c at ff160000/camera at 3c" is not
something anyone is going to be willing to type by hand.

Maybe that's not a problem if it's not a design consideration anyway.

--
Kieran

> 
> ?
> 
>>
>> but at that point, I think we've done the best we could
> 
>>
>>>
>>>>
>>>>
>>>>> +
>>>>> +	name += camera->id();
>>>>> +
>>>>> +	return name;
>>>>> +}
>>>>> +
>>>>>  class CamApp
>>>>>  {
>>>>>  public:
>>>>> @@ -117,7 +150,7 @@ int CamApp::init(int argc, char **argv)
>>>>>  			return -EINVAL;
>>>>>  		}
>>>>>
>>>>> -		std::cout << "Using camera " << camera_->id() << std::endl;
>>>>> +		std::cout << "Using camera " << cameraName(camera_.get()) << std::endl;
>>>>>
>>>>>  		ret = prepareConfig();
>>>>>  		if (ret) {
>>>>> @@ -323,12 +356,12 @@ int CamApp::infoConfiguration()
>>>>>
>>>>>  void CamApp::cameraAdded(std::shared_ptr<Camera> cam)
>>>>>  {
>>>>> -	std::cout << "Camera Added: " << cam->id() << std::endl;
>>>>> +	std::cout << "Camera Added: " << cameraName(cam.get()) << std::endl;
>>>>>  }
>>>>>
>>>>>  void CamApp::cameraRemoved(std::shared_ptr<Camera> cam)
>>>>>  {
>>>>> -	std::cout << "Camera Removed: " << cam->id() << std::endl;
>>>>> +	std::cout << "Camera Removed: " << cameraName(cam.get()) << std::endl;
>>>>>  }
>>>>>
>>>>>  int CamApp::run()
>>>>> @@ -340,7 +373,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++;
>>>>>  		}
>>>>>  	}
>>>>> --
>>>>> 2.28.0
>>>>>
>>>>> _______________________________________________
>>>>> libcamera-devel mailing list
>>>>> libcamera-devel at lists.libcamera.org
>>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>>
>>> --
>>> Regards,
>>> Niklas Söderlund
> 

-- 
Regards
--
Kieran