| Message ID | 20220807154138.91960-1-utkarsh02t@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Utkarsh Tiwari via libcamera-devel (2022-08-07 16:41:38) > The camera selection dialog currently only displays the camera Id. > Display the camera location and camera model if available. > \o/ This bit is great! > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > --- > Difference from v2: > Use getCameraId() instead of getCurrentCamera() > src/qcam/cam_select_dialog.h | 54 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h > index ee65eb88..6ba61cff 100644 > --- a/src/qcam/cam_select_dialog.h > +++ b/src/qcam/cam_select_dialog.h > @@ -12,11 +12,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 CamSelectDialog : public QDialog > @@ -34,6 +37,17 @@ public: > for (const auto &cam : cm_->cameras()) > cameraIdComboBox_->addItem(QString::fromStdString(cam->id())); > > + /* Set camera information labels. */ > + cameraLocation_ = new QLabel; > + cameraModel_ = new QLabel; > + > + updateCamInfo(cm_->get(getCameraId())); > + connect(cameraIdComboBox_, &QComboBox::currentTextChanged, > + this, [&]() { > + updateCamInfo(cm_->get( > + cameraIdComboBox_->currentText().toStdString())); > + }); > + > /* Setup the QDialogButton Box */ > QDialogButtonBox *dialogButtonBox = > new QDialogButtonBox(QDialogButtonBox::Ok | > @@ -46,6 +60,8 @@ public: > > /* Set the layout. */ > camSelectDialogLayout->addRow("Camera: ", cameraIdComboBox_); > + camSelectDialogLayout->addRow("Location: ", cameraLocation_); > + camSelectDialogLayout->addRow("Model: ", cameraModel_); > camSelectDialogLayout->addWidget(dialogButtonBox); > } > > @@ -70,9 +86,47 @@ public: > cameraIdComboBox_->removeItem(cameraIndex); > } > > + /* Camera Information */ > + void updateCamInfo(const std::shared_ptr<libcamera::Camera> &camera) > + { > + if (camera == nullptr) > + 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 "); Do these need the space at the end? or could those be removed? > + break; > + case libcamera::properties::CameraLocationBack: > + cameraLocation_->setText("Internal back camera "); > + break; > + case libcamera::properties::CameraLocationExternal: > + cameraLocation_->setText("External camera "); > + break; > + default: > + cameraLocation_->setText(QString()); > + } > + } else { > + cameraLocation_->setText(QString()); > + } > + > + const auto &model = cameraProperties.get(libcamera::properties::Model); I think with the new 'optional' controls this could read something like (not tested/checked) const std::string model = cameraProperties .get(libcamera::properties::Model) .value_or(utils::defopt) To save having to be conditional on the if (model) below... But it all looks functional, and I've seen the output so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + > + if (model) > + cameraModel_->setText(QString::fromStdString(*model)); > + else > + cameraModel_->setText(QString()); > + } > + > private: > libcamera::CameraManager *cm_; > > /* UI elements. */ > QComboBox *cameraIdComboBox_; > + QLabel *cameraLocation_; > + QLabel *cameraModel_; > }; > -- > 2.25.1 >
On Mon, Aug 08, 2022 at 10:05:54AM +0100, Kieran Bingham via libcamera-devel wrote: > Quoting Utkarsh Tiwari via libcamera-devel (2022-08-07 16:41:38) > > The camera selection dialog currently only displays the camera Id. > > Display the camera location and camera model if available. > > > > \o/ This bit is great! > > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > > --- > > Difference from v2: > > Use getCameraId() instead of getCurrentCamera() > > src/qcam/cam_select_dialog.h | 54 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h > > index ee65eb88..6ba61cff 100644 > > --- a/src/qcam/cam_select_dialog.h > > +++ b/src/qcam/cam_select_dialog.h > > @@ -12,11 +12,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 CamSelectDialog : public QDialog > > @@ -34,6 +37,17 @@ public: > > for (const auto &cam : cm_->cameras()) > > cameraIdComboBox_->addItem(QString::fromStdString(cam->id())); > > > > + /* Set camera information labels. */ > > + cameraLocation_ = new QLabel; > > + cameraModel_ = new QLabel; > > + > > + updateCamInfo(cm_->get(getCameraId())); This would be best placed at the very end of the constructor, when the dialog layout is fully constructed. > > + connect(cameraIdComboBox_, &QComboBox::currentTextChanged, > > + this, [&]() { > > + updateCamInfo(cm_->get( > > + cameraIdComboBox_->currentText().toStdString())); > > + }); Can you add a private member function instead of using a lambda function ? That will be more efficient. > > + > > /* Setup the QDialogButton Box */ > > QDialogButtonBox *dialogButtonBox = > > new QDialogButtonBox(QDialogButtonBox::Ok | > > @@ -46,6 +60,8 @@ public: > > > > /* Set the layout. */ > > camSelectDialogLayout->addRow("Camera: ", cameraIdComboBox_); > > + camSelectDialogLayout->addRow("Location: ", cameraLocation_); > > + camSelectDialogLayout->addRow("Model: ", cameraModel_); > > camSelectDialogLayout->addWidget(dialogButtonBox); > > } > > > > @@ -70,9 +86,47 @@ public: > > cameraIdComboBox_->removeItem(cameraIndex); > > } > > > > + /* Camera Information */ > > + void updateCamInfo(const std::shared_ptr<libcamera::Camera> &camera) > > + { > > + if (camera == nullptr) 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 "); > > Do these need the space at the end? or could those be removed? > > > > + break; > > + case libcamera::properties::CameraLocationBack: > > + cameraLocation_->setText("Internal back camera "); > > + break; > > + case libcamera::properties::CameraLocationExternal: > > + cameraLocation_->setText("External camera "); > > + break; > > + default: > > + cameraLocation_->setText(QString()); "Unknown" may be better than an empty string. > > + } > > + } else { > > + cameraLocation_->setText(QString()); Same here. > > + } > > + > > + const auto &model = cameraProperties.get(libcamera::properties::Model); > > I think with the new 'optional' controls this could read something like > (not tested/checked) > const std::string model = cameraProperties > .get(libcamera::properties::Model) > .value_or(utils::defopt) > > To save having to be conditional on the if (model) below... That's nicer. It could also be writte .value_or("") as constructing an empty string is cheap. Similarly to the location, maybe "Unknown" would be better too ? > But it all looks functional, and I've seen the output so: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + > > + if (model) > > + cameraModel_->setText(QString::fromStdString(*model)); > > + else > > + cameraModel_->setText(QString()); > > + } > > + > > private: > > libcamera::CameraManager *cm_; > > > > /* UI elements. */ > > QComboBox *cameraIdComboBox_; > > + QLabel *cameraLocation_; > > + QLabel *cameraModel_; > > };
diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h index ee65eb88..6ba61cff 100644 --- a/src/qcam/cam_select_dialog.h +++ b/src/qcam/cam_select_dialog.h @@ -12,11 +12,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 CamSelectDialog : public QDialog @@ -34,6 +37,17 @@ public: for (const auto &cam : cm_->cameras()) cameraIdComboBox_->addItem(QString::fromStdString(cam->id())); + /* Set camera information labels. */ + cameraLocation_ = new QLabel; + cameraModel_ = new QLabel; + + updateCamInfo(cm_->get(getCameraId())); + connect(cameraIdComboBox_, &QComboBox::currentTextChanged, + this, [&]() { + updateCamInfo(cm_->get( + cameraIdComboBox_->currentText().toStdString())); + }); + /* Setup the QDialogButton Box */ QDialogButtonBox *dialogButtonBox = new QDialogButtonBox(QDialogButtonBox::Ok | @@ -46,6 +60,8 @@ public: /* Set the layout. */ camSelectDialogLayout->addRow("Camera: ", cameraIdComboBox_); + camSelectDialogLayout->addRow("Location: ", cameraLocation_); + camSelectDialogLayout->addRow("Model: ", cameraModel_); camSelectDialogLayout->addWidget(dialogButtonBox); } @@ -70,9 +86,47 @@ public: cameraIdComboBox_->removeItem(cameraIndex); } + /* Camera Information */ + void updateCamInfo(const std::shared_ptr<libcamera::Camera> &camera) + { + if (camera == nullptr) + 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(QString()); + } + } else { + cameraLocation_->setText(QString()); + } + + const auto &model = cameraProperties.get(libcamera::properties::Model); + + if (model) + cameraModel_->setText(QString::fromStdString(*model)); + else + cameraModel_->setText(QString()); + } + private: libcamera::CameraManager *cm_; /* UI elements. */ QComboBox *cameraIdComboBox_; + QLabel *cameraLocation_; + QLabel *cameraModel_; };
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@gmail.com> --- Difference from v2: Use getCameraId() instead of getCurrentCamera() src/qcam/cam_select_dialog.h | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)