[libcamera-devel] [PATCH 3/4] qcam: CameraSelectDialog: Display Location and Model propety of camera

[Umang Jain]

Hi Utkarsh,

Thank you for the patch.

On 8/3/22 23:25, Utkarsh Tiwari via libcamera-devel wrote:
> 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>
> ---
>   src/qcam/main_window.cpp | 69 ++++++++++++++++++++++++++++++++++++++++
>   src/qcam/main_window.h   |  7 ++++
>   2 files changed, 76 insertions(+)
>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 80a73b68..81fa3e60 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -12,6 +12,7 @@
>   #include <string>
>   
>   #include <libcamera/camera_manager.h>
> +#include <libcamera/property_ids.h>
>   #include <libcamera/version.h>
>   
>   #include <QComboBox>
> @@ -23,6 +24,7 @@
>   #include <QImage>
>   #include <QImageWriter>
>   #include <QInputDialog>
> +#include <QLabel>
>   #include <QMutexLocker>
>   #include <QStandardPaths>
>   #include <QStringList>
> @@ -291,6 +293,47 @@ void MainWindow::switchCamera(int index)
>   	startStopAction_->setChecked(true);
>   }
>   
> +QString MainWindow::getCameraLocation(const std::shared_ptr<Camera> &camera)
> +{
> +	if (camera == nullptr)
> +		return QString();
> +
> +	const ControlList &cameraProperties = camera->properties();
> +	const auto &location = cameraProperties.get(properties::Location);
> +
> +	if (location) {
> +		switch (*location) {
> +		case properties::CameraLocationFront:
> +			return "Internal front camera ";
> +		case properties::CameraLocationBack:
> +			return "Internal back camera ";
> +		case properties::CameraLocationExternal:
> +			return "External camera ";
> +		}
> +	}
> +	return QString();
> +}
> +
> +QString MainWindow::getCameraModel(const std::shared_ptr<Camera> &camera)
> +{
> +	if (camera == nullptr)
> +		return QString();
> +
> +	const ControlList &cameraProperties = camera->properties();
> +	const auto &model = cameraProperties.get(properties::Model);
> +
> +	if (model)
> +		return QString::fromStdString(*model);
> +
> +	return QString();
> +}
> +
> +void MainWindow::updateCameraInfo(const std::shared_ptr<libcamera::Camera> &camera)
> +{
> +	cameraLocation_->setText(getCameraLocation(camera));
> +	cameraModel_->setText(getCameraModel(camera));


The getCameramodel() and getCameraLocation are concise helpers which can 
be subsumed under updateCameraInfo() itself.

> +}
> +
>   std::string MainWindow::chooseCamera()
>   {
>   	std::string result;
> @@ -303,6 +346,29 @@ std::string MainWindow::chooseCamera()
>   	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
>   		cameraIdComboBox_->addItem(QString::fromStdString(cam->id()));
>   
> +	/* Display Camera Informtion in the Selection Dialog. */


	/* Display camera information in the selection dialog. */

> +	cameraLocation_ = new QLabel;
> +	cameraModel_ = new QLabel;
> +
> +	std::shared_ptr<Camera> currentCamera;
> +
> +	/*
> +	 * When the Qdialog starts, the QComboBox would have the first

     s/Qdialog/QDialog/

     s/would/should/

> +	 * camera's Id as its currentText.
> +	 */
> +	if (cm_->cameras().size())
> +		currentCamera = cm_->cameras()[0];
> +	else
> +		currentCamera = nullptr;


currentCamera is a shared_ptr above which doesn't yet manage an object 
which means it's nullptr unless it's assigned. Hence, you can drop the 
else block here.

Rest looks good to me!

> +
> +	if (currentCamera)
> +		updateCameraInfo(currentCamera);
> +
> +	connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
> +		this, [&]() {
> +			updateCameraInfo(cm_->get(cameraIdComboBox_->currentText().toStdString()));
> +		});
> +
>   	/* Setup QDialogButtonBox. */
>   	QDialogButtonBox *dialogButtonBox = new QDialogButtonBox;
>   	dialogButtonBox->addButton(QDialogButtonBox::Cancel);
> @@ -323,6 +389,9 @@ std::string MainWindow::chooseCamera()
>   	/* Setup the layout for the dialog. */
>   	QFormLayout *cameraSelectLayout = new QFormLayout(cameraSelectDialog);
>   	cameraSelectLayout->addRow("Camera: ", cameraIdComboBox_);
> +	cameraSelectLayout->addRow("Location: ", cameraLocation_);
> +	cameraSelectLayout->addRow("Model: ", cameraModel_);
> +
>   	cameraSelectLayout->addWidget(dialogButtonBox);
>   
>   	cameraSelectDialog->exec();
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index b8122eb9..3a1c6156 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -20,6 +20,7 @@
>   
>   #include <QElapsedTimer>
>   #include <QIcon>
> +#include <QLabel>
>   #include <QMainWindow>
>   #include <QMutex>
>   #include <QObject>
> @@ -88,6 +89,10 @@ private:
>   	void processHotplug(HotplugEvent *e);
>   	void processViewfinder(libcamera::FrameBuffer *buffer);
>   
> +	QString getCameraLocation(const std::shared_ptr<libcamera::Camera> &camera);
> +	QString getCameraModel(const std::shared_ptr<libcamera::Camera> &camera);
> +	void updateCameraInfo(const std::shared_ptr<libcamera::Camera> &camera);
> +
>   	/* UI elements */
>   	QToolBar *toolbar_;
>   	QAction *startStopAction_;
> @@ -102,6 +107,8 @@ private:
>   	QTimer titleTimer_;
>   
>   	QPointer<QComboBox> cameraIdComboBox_;
> +	QLabel *cameraLocation_;
> +	QLabel *cameraModel_;
>   
>   	/* Options */
>   	const OptionsParser::Options &options_;