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

[Kieran Bingham]

Hi Umang,

On 30/09/2020 06:32, Umang Jain wrote:
> Hi Niklas,
> 
> On 9/29/20 8:16 PM, 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>
>> Reviewed-by: Umang Jain <email at uajain.com>
>> ---
>> * Changes since v5
>> - Rework camera name.
>>
>> * Changes since v4
>> - Make cameraName() member of 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 | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
>> index 244720b491f5c462..311df171a21f6152 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,30 @@ int CamApp::run()
>>       return 0;
>>   }
>>   +std::string CamApp::cameraName(const Camera *camera)
>> +{
>> +    const ControlList &props = camera->properties();
>> +    std::string name;
>> +
>> +    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";
>> +        if (props.contains(properties::Model))
>> +            name += " '" + props.get(properties::Model) + "'";
> Quick comment: Can we probably move this out of the switch block? I
> understand, as of now, only UVC will return the model property, but
> application's point-of-view, I don't think we need to carry that
> implementation detail in cam itself. As and when the reign the model
> property expands in libcamera, it shall auto-magically start to show up
> via `cam -l` :)

Wouldn't it be quite difficult to identify UVC cameras in the system
then? It's currently quite rare to have more than one front or back
camera (at least in systems we're developing with) but my laptop has 2
UVC devices internally. If they both simply say "External camera" it
would be quite hard to know which is the face-detect camera, and which
is the 'real' camera.

Or maybe it doesn't matter - perhaps 'face detect' is another feature to
express in the properties... (but we can only guess that if the only
format is R8/ greyscale, I think).

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

-- 
Regards
--
Kieran