[libcamera-devel] [PATCH 6/7] cam: Print user-friendly camera names
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Aug 8 22:13:20 CEST 2020
Hi Niklas,
On Sat, Aug 08, 2020 at 02:22:35PM +0200, Niklas Söderlund wrote:
> On 2020-08-08 15:10:34 +0300, Laurent Pinchart wrote:
> > On Thu, Aug 06, 2020 at 05:06:34PM +0200, Jacopo Mondi wrote:
> >> On Thu, Aug 06, 2020 at 03:46:01PM +0200, Niklas Söderlund wrote:
> >>> On 2020-08-06 15:40:02 +0200, Jacopo Mondi wrote:
> >>>> On Thu, Aug 06, 2020 at 03:32:49PM +0200, Niklas Söderlund wrote:
> >>>>> On 2020-08-06 15:28:04 +0200, Jacopo Mondi wrote:
> >>>>>> 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 now wonder if an application like cam should expose machine-friendly
> >> id at all. But I guess it's fine, I like the above proposal
> >
> > I'm happy to join the bikeshedding :-) Jokes aside, I think this is
> > important.
> >
> > Ideally, for internal cameras, I'd like to see something along the lines
> > of
> >
> > 1: Internal front camera (/base/i2c at ff160000/camera at 36)
> > 2. Internal back camera (/base/i2c at ff160000/camera at 3c)
> >
> > reported by cam, or possibly
> >
> > 1: Internal front 5MP camera (/base/i2c at ff160000/camera at 36)
> > 2. Internal back 12MP camera (/base/i2c at ff160000/camera at 3c)
> >
> > if we want to make this more marketing-friendly :-) I know that
> > currently we'll get "Internal front camera" twice, and I think that's
> > OK. I believe the ID is important to print in cam, less so in qcam.
> >
> > For external camera, I've been dreaming of
> >
> > 3: Logitech Webcam C930e on USB port 2.2 (\_SB_.PCI0.RP05.PXSX-2.2:1.0-046d:0843)
> > 4: Logitech Webcam C930e on USB port 2.4 (\_SB_.PCI0.RP05.PXSX-2.4:1.0-046d:0843)
> >
> > but getting a sensible port number may be difficult (in theory we could
> > get the physical location from ACPI tables, but that's really the
> > theory). I think reporting extended physical location through properties
> > would be useful, but it doesn't have to be part of this series.
>
> I like the above but to be able to implement all of it more properties
> needs to be added to lib camera core. As a first step to make cam
> user-friendly again would the following output be sufficient for this
> series in your opinion?
>
> 1: Internal front camera (/base/i2c at ff160000/camera at 36)
> 2. Internal back camera (/base/i2c at ff160000/camera at 3c)
> 3: Logitech Webcam C930e (\_SB_.PCI0.RP05.PXSX-2.2:1.0-046d:0843)
Absolutely. I tried to convey that in my previous e-mail, we'll need to
expose more information about camera location, and it's fine to start
with what we have.
> As camera selection is only possible by `index` or `id` modifying the
> name as we gain more information will not break any scripts.
>
> >>>> 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++;
> >>>>>>> }
> >>>>>>> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list