[libcamera-devel,v9,4/7] qcam: CamSelectDialog: Display Location and Model propety of camera
diff mbox series

Message ID 20220831054938.21617-5-utkarsh02t@gmail.com
State Accepted
Headers show
Series
  • Introduce capture scripts to qcam
Related show

Commit Message

Utkarsh Tiwari Aug. 31, 2022, 5:49 a.m. UTC 
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 v9:
    1. Remove handleCameraChange and use updateCamInfo directly
    2. QLabel forward-declare
    3. Rename cameraProperties to just properties
 src/qcam/cam_select_dialog.cpp | 50 ++++++++++++++++++++++++++++++++++
 src/qcam/cam_select_dialog.h   |  8 ++++++
 2 files changed, 58 insertions(+)

Comments

Kieran Bingham Aug. 31, 2022, 9:05 a.m. UTC | #1 
Quoting Utkarsh Tiwari via libcamera-devel (2022-08-31 06:49:35)
> 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 v9:
>     1. Remove handleCameraChange and use updateCamInfo directly
>     2. QLabel forward-declare
>     3. Rename cameraProperties to just properties
>  src/qcam/cam_select_dialog.cpp | 50 ++++++++++++++++++++++++++++++++++
>  src/qcam/cam_select_dialog.h   |  8 ++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp
> index f82930cc..6543228a 100644
> --- a/src/qcam/cam_select_dialog.cpp
> +++ b/src/qcam/cam_select_dialog.cpp
> @@ -7,12 +7,15 @@
>  
>  #include "cam_select_dialog.h"
>  
> +#include <memory>
> +
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
>  
>  #include <QComboBox>
>  #include <QDialogButtonBox>
>  #include <QFormLayout>
> +#include <QLabel>
>  #include <QString>
>  
>  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> @@ -27,6 +30,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;
> +
> +       updateCamInfo(cameraIdComboBox_->currentText());
> +       connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
> +               this, &CameraSelectorDialog::updateCamInfo);
> +
>         /* Setup the QDialogButton Box */
>         QDialogButtonBox *buttonBox =
>                 new QDialogButtonBox(QDialogButtonBox::Ok |
> @@ -39,6 +50,8 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
>  
>         /* Set the layout. */
>         layout->addRow("Camera:", cameraIdComboBox_);
> +       layout->addRow("Location:", cameraLocation_);
> +       layout->addRow("Model:", cameraModel_);
>         layout->addWidget(buttonBox);
>  }
>  
> @@ -60,3 +73,40 @@ void CameraSelectorDialog::removeCamera(QString cameraId)
>         int cameraIndex = cameraIdComboBox_->findText(cameraId);
>         cameraIdComboBox_->removeItem(cameraIndex);
>  }
> +
> +/* Camera Information */
> +void CameraSelectorDialog::updateCamInfo(QString cameraId)
> +{
> +       const std::shared_ptr<libcamera::Camera> &camera =
> +               cm_->get(cameraId.toStdString());
> +
> +       if (!camera)
> +               return;
> +
> +       const libcamera::ControlList &properties = camera->properties();
> +
> +       const auto &location = properties.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");

I wondered if we could de-dup the "Unknown" string, but I'm not sure it
makes much of a difference here.


> +       }
> +
> +       const auto &model = properties
> +                                   .get(libcamera::properties::Model)

Minor here, The properties.get(..) fits on one line. So that's probably
how I'd format this.

	const auto &model = properties.get(libcamera::properties::Model)
				      .value_or("Unknown");

The 'floating' properties statement looks a bit odd otherwise.

Minor formatting aside, which could be updated on applying perhaps:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +                                   .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 bd2dbc1e..c91b7ebe 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 <QDialog>
>  #include <QString>
>  
>  class QComboBox;
> +class QLabel;
>  
>  class CameraSelectorDialog : public QDialog
>  {
> @@ -32,9 +35,14 @@ public:
>         void addCamera(QString cameraId);
>         void removeCamera(QString cameraId);
>  
> +       /* Camera Information */
> +       void updateCamInfo(QString cameraId);
> +
>  private:
>         libcamera::CameraManager *cm_;
>  
>         /* UI elements. */
>         QComboBox *cameraIdComboBox_;
> +       QLabel *cameraLocation_;
> +       QLabel *cameraModel_;
>  };
> -- 
> 2.34.1
>
Laurent Pinchart Aug. 31, 2022, 9:19 a.m. UTC | #2 
On Wed, Aug 31, 2022 at 10:05:37AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Utkarsh Tiwari via libcamera-devel (2022-08-31 06:49:35)
> > 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 v9:
> >     1. Remove handleCameraChange and use updateCamInfo directly
> >     2. QLabel forward-declare
> >     3. Rename cameraProperties to just properties
> >  src/qcam/cam_select_dialog.cpp | 50 ++++++++++++++++++++++++++++++++++
> >  src/qcam/cam_select_dialog.h   |  8 ++++++
> >  2 files changed, 58 insertions(+)
> > 
> > diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp
> > index f82930cc..6543228a 100644
> > --- a/src/qcam/cam_select_dialog.cpp
> > +++ b/src/qcam/cam_select_dialog.cpp
> > @@ -7,12 +7,15 @@
> >  
> >  #include "cam_select_dialog.h"
> >  
> > +#include <memory>
> > +
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> >  
> >  #include <QComboBox>
> >  #include <QDialogButtonBox>
> >  #include <QFormLayout>
> > +#include <QLabel>
> >  #include <QString>
> >  
> >  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> > @@ -27,6 +30,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;
> > +
> > +       updateCamInfo(cameraIdComboBox_->currentText());

I'd name this function updateCameraInfo() as you spell out camera in
full in the rest of the file. With this and Kieran's comments addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +       connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
> > +               this, &CameraSelectorDialog::updateCamInfo);
> > +
> >         /* Setup the QDialogButton Box */
> >         QDialogButtonBox *buttonBox =
> >                 new QDialogButtonBox(QDialogButtonBox::Ok |
> > @@ -39,6 +50,8 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
> >  
> >         /* Set the layout. */
> >         layout->addRow("Camera:", cameraIdComboBox_);
> > +       layout->addRow("Location:", cameraLocation_);
> > +       layout->addRow("Model:", cameraModel_);
> >         layout->addWidget(buttonBox);
> >  }
> >  
> > @@ -60,3 +73,40 @@ void CameraSelectorDialog::removeCamera(QString cameraId)
> >         int cameraIndex = cameraIdComboBox_->findText(cameraId);
> >         cameraIdComboBox_->removeItem(cameraIndex);
> >  }
> > +
> > +/* Camera Information */
> > +void CameraSelectorDialog::updateCamInfo(QString cameraId)
> > +{
> > +       const std::shared_ptr<libcamera::Camera> &camera =
> > +               cm_->get(cameraId.toStdString());
> > +
> > +       if (!camera)
> > +               return;
> > +
> > +       const libcamera::ControlList &properties = camera->properties();
> > +
> > +       const auto &location = properties.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");
> 
> I wondered if we could de-dup the "Unknown" string, but I'm not sure it
> makes much of a difference here.
> 
> 
> > +       }
> > +
> > +       const auto &model = properties
> > +                                   .get(libcamera::properties::Model)
> 
> Minor here, The properties.get(..) fits on one line. So that's probably
> how I'd format this.
> 
> 	const auto &model = properties.get(libcamera::properties::Model)
> 				      .value_or("Unknown");
> 
> The 'floating' properties statement looks a bit odd otherwise.
> 
> Minor formatting aside, which could be updated on applying perhaps:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +                                   .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 bd2dbc1e..c91b7ebe 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 <QDialog>
> >  #include <QString>
> >  
> >  class QComboBox;
> > +class QLabel;
> >  
> >  class CameraSelectorDialog : public QDialog
> >  {
> > @@ -32,9 +35,14 @@ public:
> >         void addCamera(QString cameraId);
> >         void removeCamera(QString cameraId);
> >  
> > +       /* Camera Information */
> > +       void updateCamInfo(QString cameraId);
> > +
> >  private:
> >         libcamera::CameraManager *cm_;
> >  
> >         /* UI elements. */
> >         QComboBox *cameraIdComboBox_;
> > +       QLabel *cameraLocation_;
> > +       QLabel *cameraModel_;
> >  };

Patch
diff mbox series 

diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp
index f82930cc..6543228a 100644
--- a/src/qcam/cam_select_dialog.cpp
+++ b/src/qcam/cam_select_dialog.cpp
@@ -7,12 +7,15 @@ 
 
 #include "cam_select_dialog.h"
 
+#include <memory>
+
 #include <libcamera/camera.h>
 #include <libcamera/camera_manager.h>
 
 #include <QComboBox>
 #include <QDialogButtonBox>
 #include <QFormLayout>
+#include <QLabel>
 #include <QString>
 
 CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
@@ -27,6 +30,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;
+
+	updateCamInfo(cameraIdComboBox_->currentText());
+	connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
+		this, &CameraSelectorDialog::updateCamInfo);
+
 	/* Setup the QDialogButton Box */
 	QDialogButtonBox *buttonBox =
 		new QDialogButtonBox(QDialogButtonBox::Ok |
@@ -39,6 +50,8 @@  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
 
 	/* Set the layout. */
 	layout->addRow("Camera:", cameraIdComboBox_);
+	layout->addRow("Location:", cameraLocation_);
+	layout->addRow("Model:", cameraModel_);
 	layout->addWidget(buttonBox);
 }
 
@@ -60,3 +73,40 @@  void CameraSelectorDialog::removeCamera(QString cameraId)
 	int cameraIndex = cameraIdComboBox_->findText(cameraId);
 	cameraIdComboBox_->removeItem(cameraIndex);
 }
+
+/* Camera Information */
+void CameraSelectorDialog::updateCamInfo(QString cameraId)
+{
+	const std::shared_ptr<libcamera::Camera> &camera =
+		cm_->get(cameraId.toStdString());
+
+	if (!camera)
+		return;
+
+	const libcamera::ControlList &properties = camera->properties();
+
+	const auto &location = properties.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 = properties
+				    .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 bd2dbc1e..c91b7ebe 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 <QDialog>
 #include <QString>
 
 class QComboBox;
+class QLabel;
 
 class CameraSelectorDialog : public QDialog
 {
@@ -32,9 +35,14 @@  public:
 	void addCamera(QString cameraId);
 	void removeCamera(QString cameraId);
 
+	/* Camera Information */
+	void updateCamInfo(QString cameraId);
+
 private:
 	libcamera::CameraManager *cm_;
 
 	/* UI elements. */
 	QComboBox *cameraIdComboBox_;
+	QLabel *cameraLocation_;
+	QLabel *cameraModel_;
 };