[libcamera-devel] [PATCH 6/7] cam: Print user-friendly camera names
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Aug 6 16:46:08 CEST 2020
Hi Kieran,
On 2020-08-06 15:35:34 +0100, Kieran Bingham wrote:
> 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.
No the two ways we can specify cameras today using the --camera option
are not changed in by this series, so the options are by index or id. So
to select the "ov2685 (back)" from the example above there are two
options,
$ cam --camera 2 ...
$ cam --camera /base/i2c at ff160000/camera at 3c ...
Where doing it by index is not stable as if we plug in a third camera
the ordering may change while using the id is guaranteed to be stable,
even between reboots of the system. One of the ides of the id is that it
could be save to persistent storage and reread later and expect to get
the same camera.
In my automated tests for example I use the id to make sure I always
test the camera I expect to test.
>
> --
> 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
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list