[libcamera-devel] [PATCH v7 4/8] qcam: CamSelectDialog: Display Location and Model propety of camera

[Utkarsh Tiwari]

Hi Kieran thanks for the review

On Wed, 10 Aug, 2022, 03:14 Kieran Bingham, <kieran.bingham at ideasonboard.com>
wrote:

> Quoting Utkarsh Tiwari (2022-08-09 21:50:38)
> > The camera selection dialog currently only displays the camera Id.
> > Display the camera location and camera model if available.
> >
> > Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > Difference :
> >         1. Now use value_or for optional
> >         2. Not use lambda and make a private member function
> >  src/qcam/cam_select_dialog.cpp | 52 ++++++++++++++++++++++++++++++++++
> >  src/qcam/cam_select_dialog.h   | 10 +++++++
> >  2 files changed, 62 insertions(+)
> >
> > diff --git a/src/qcam/cam_select_dialog.cpp
> b/src/qcam/cam_select_dialog.cpp
> > index d8982800..f97ad6eb 100644
> > --- a/src/qcam/cam_select_dialog.cpp
> > +++ b/src/qcam/cam_select_dialog.cpp
> > @@ -7,6 +7,7 @@
> >
> >  #include "cam_select_dialog.h"
> >
> > +#include <memory>
> >  #include <string>
> >
> >  #include <libcamera/camera.h>
> > @@ -30,6 +31,14 @@
> CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager
> *cameraManag
> >         for (const auto &cam : cm_->cameras())
> >
>  cameraIdComboBox_->addItem(QString::fromStdString(cam->id()));
> >
> > +       /* Set camera information labels. */
> > +       cameraLocation_ = new QLabel;
> > +       cameraModel_ = new QLabel;
> > +
> > +       handleCameraChange();
> > +       connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
> > +               this, &CameraSelectorDialog::handleCameraChange);
> > +
> >         /* Setup the QDialogButton Box */
> >         QDialogButtonBox *buttonBox =
> >                 new QDialogButtonBox(QDialogButtonBox::Ok |
> > @@ -41,7 +50,10 @@
> CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager
> *cameraManag
> >                 this, &QDialog::reject);
> >
> >         /* Set the layout. */
> > +
> >         layout->addRow("Camera:", cameraIdComboBox_);
> > +       layout->addRow("Location:", cameraLocation_);
> > +       layout->addRow("Model:", cameraModel_);
> >         layout->addWidget(buttonBox);
> >  }
> >
> > @@ -63,3 +75,43 @@ void
> CameraSelectorDialog::cameraRemoved(libcamera::Camera *camera)
> >
> >         cameraIdComboBox_->removeItem(cameraIndex);
> >  }
> > +
> > +/* Camera Information */
> > +void CameraSelectorDialog::handleCameraChange()
> > +{
> > +       updateCamInfo(cm_->get(getCameraId()));
> > +}
> > +
> > +void CameraSelectorDialog::updateCamInfo(const
> std::shared_ptr<libcamera::Camera> &camera)
> > +{
> > +       if (!camera)
> > +               return;
> > +
> > +       const libcamera::ControlList &cameraProperties =
> camera->properties();
> > +
> > +       const auto &location =
> > +               cameraProperties.get(libcamera::properties::Location);
> > +       if (location) {
> > +               switch (*location) {
> > +               case libcamera::properties::CameraLocationFront:
> > +                       cameraLocation_->setText("Internal front
> camera");
> > +                       break;
> > +               case libcamera::properties::CameraLocationBack:
> > +                       cameraLocation_->setText("Internal back camera");
> > +                       break;
> > +               case libcamera::properties::CameraLocationExternal:
> > +                       cameraLocation_->setText("External camera");
> > +                       break;
> > +               default:
> > +                       cameraLocation_->setText("Unknown");
> > +               }
> > +       } else {
> > +               cameraLocation_->setText("Unknown");
> > +       }
> > +
> > +       const auto &model = cameraProperties
> > +                                   .get(libcamera::properties::Model)
> > +                                   .value_or("Unknown");
> > +
> > +       cameraModel_->setText(QString::fromStdString(model));
> > +}
> > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h
> > index 567083ae..359df811 100644
> > --- a/src/qcam/cam_select_dialog.h
> > +++ b/src/qcam/cam_select_dialog.h
> > @@ -11,11 +11,14 @@
> >
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> > +#include <libcamera/controls.h>
> > +#include <libcamera/property_ids.h>
> >
> >  #include <QComboBox>
> >  #include <QDialog>
> >  #include <QDialogButtonBox>
> >  #include <QFormLayout>
> > +#include <QLabel>
> >  #include <QString>
> >
> >  class CameraSelectorDialog : public QDialog
> > @@ -33,9 +36,16 @@ public:
> >         void cameraAdded(libcamera::Camera *camera);
> >
> >         void cameraRemoved(libcamera::Camera *camera);
> > +
> > +       /* Camera Information */
> > +       void updateCamInfo(const std::shared_ptr<libcamera::Camera>
> &camera);
> > +       void handleCameraChange();
> > +
> >  private:
> >         libcamera::CameraManager *cm_;
> >
> >         /* UI elements. */
> >         QComboBox *cameraIdComboBox_;
> > +       QLabel *cameraLocation_;
> > +       QLabel *cameraModel_;
>
> Do these actually need to be pointers? What happens if they are
> allcoated as part of the CameraSelectorDialog ? - then we wouldn't need
> to call 'new' and they wouldn't be leaked (well, the lifetime would be
> directly associated with the CameraSelectorDialog at least)
>
Actually yes, the layout needs a pointer to the widget. Also the widget
to which the layout belongs takes the ownership of the widget. And when
it gets destroyed these are too.

> >  };
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220810/969c810e/attachment.htm>