[libcamera-devel] [PATCH v9 1/7] qcam: Use QDialog for selection of cameras at startup

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Aug 31 11:05:50 CEST 2022


On Wed, Aug 31, 2022 at 11:19:32AM +0530, Utkarsh Tiwari wrote:
> Currently we use QInputDialog convenience dialogs to allow the user to
> select a camera. This doesn't allow adding of more information (such as
> camera location, model etc).
> 
> Create a QDialog with a QFormLayout that shows a QComboBox with camera
> Ids. Use a QDialogButtonBox to provide buttons for accepting and
> cancelling the action.
> 
> The CameraSelectorDialog is only initialized the first time when the
> MainWindow is created.
> 
> From this commit we cease to auto select the camera if only a single
> camera is available to libcamera. We would always display the selection
> dialog with the exception being that being if the camera is supplied on
> the command line.
> 
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Difference from v8:
>     1. s/by the console/on the command line in the commit msg
>     2. Removed <string> header from cam_select_dialog.cpp
>     3. Drop <QDialog> from cam_select_dialog.cpp
>     4. QComboBox is forward-declared in cam_select_dialog.h
>     5. ~CameraSelectorDialog() implementation now resides in .cpp
>     6. CameraSelectorDialog is forward-declared in main_window.h
>  src/qcam/cam_select_dialog.cpp | 50 ++++++++++++++++++++++++++++++++++
>  src/qcam/cam_select_dialog.h   | 35 ++++++++++++++++++++++++
>  src/qcam/main_window.cpp       | 27 ++++--------------
>  src/qcam/main_window.h         |  3 ++
>  src/qcam/meson.build           |  2 ++
>  5 files changed, 96 insertions(+), 21 deletions(-)
>  create mode 100644 src/qcam/cam_select_dialog.cpp
>  create mode 100644 src/qcam/cam_select_dialog.h
> 
> diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp
> new file mode 100644
> index 00000000..a49d822b
> --- /dev/null
> +++ b/src/qcam/cam_select_dialog.cpp
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2022, Utkarsh Tiwari <utkarsh02t at gmail.com>
> + *
> + * cam_select_dialog.cpp - qcam - Camera Selection dialog
> + */
> +
> +#include "cam_select_dialog.h"
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> +#include <QComboBox>
> +#include <QDialogButtonBox>
> +#include <QFormLayout>
> +#include <QString>
> +
> +CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> +					   QWidget *parent)
> +	: QDialog(parent), cm_(cameraManager)
> +{
> +	/* Use a QFormLayout for the dialog. */
> +	QFormLayout *layout = new QFormLayout(this);
> +
> +	/* Setup the camera id combo-box. */
> +	cameraIdComboBox_ = new QComboBox;
> +	for (const auto &cam : cm_->cameras())
> +		cameraIdComboBox_->addItem(QString::fromStdString(cam->id()));
> +
> +	/* Setup the QDialogButton Box */
> +	QDialogButtonBox *buttonBox =
> +		new QDialogButtonBox(QDialogButtonBox::Ok |
> +				     QDialogButtonBox::Cancel);
> +
> +	connect(buttonBox, &QDialogButtonBox::accepted,
> +		this, &QDialog::accept);
> +	connect(buttonBox, &QDialogButtonBox::rejected,
> +		this, &QDialog::reject);
> +
> +	/* Set the layout. */
> +	layout->addRow("Camera:", cameraIdComboBox_);
> +	layout->addWidget(buttonBox);
> +}
> +
> +CameraSelectorDialog::~CameraSelectorDialog() = default;
> +
> +std::string CameraSelectorDialog::getCameraId()
> +{
> +	return cameraIdComboBox_->currentText().toStdString();
> +}
> diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h
> new file mode 100644
> index 00000000..c31f4f82
> --- /dev/null
> +++ b/src/qcam/cam_select_dialog.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2022, Utkarsh Tiwari <utkarsh02t at gmail.com>
> + *
> + * cam_select_dialog.h - qcam - Camera Selection dialog
> + */
> +
> +#pragma once
> +
> +#include <string>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> +#include <QDialog>
> +
> +class QComboBox;
> +
> +class CameraSelectorDialog : public QDialog
> +{
> +	Q_OBJECT
> +public:
> +	CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> +			     QWidget *parent);
> +

You can drop this blank line.

> +	~CameraSelectorDialog();
> +
> +	std::string getCameraId();
> +
> +private:
> +	libcamera::CameraManager *cm_;
> +
> +	/* UI elements. */
> +	QComboBox *cameraIdComboBox_;
> +};
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 7433d647..14bcf03e 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -19,7 +19,6 @@
>  #include <QFileDialog>
>  #include <QImage>
>  #include <QImageWriter>
> -#include <QInputDialog>
>  #include <QMutexLocker>
>  #include <QStandardPaths>
>  #include <QStringList>
> @@ -30,6 +29,7 @@
>  
>  #include "../cam/image.h"
>  
> +#include "cam_select_dialog.h"
>  #include "dng_writer.h"
>  #ifndef QT_NO_OPENGL
>  #include "viewfinder_gl.h"
> @@ -144,6 +144,8 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	cm_->cameraAdded.connect(this, &MainWindow::addCamera);
>  	cm_->cameraRemoved.connect(this, &MainWindow::removeCamera);
>  
> +	cameraSelectorDialog_ = new CameraSelectorDialog(cm_, this);
> +
>  	/* Open the camera and start capture. */
>  	ret = openCamera();
>  	if (ret < 0) {
> @@ -290,34 +292,17 @@ void MainWindow::switchCamera(int index)
>  
>  std::string MainWindow::chooseCamera()
>  {
> -	QStringList cameras;
> -	bool result;
> -
> -	/* If only one camera is available, use it automatically. */
> -	if (cm_->cameras().size() == 1)
> -		return cm_->cameras()[0]->id();
> -
> -	/* Present a dialog box to pick a camera. */
> -	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> -		cameras.append(QString::fromStdString(cam->id()));
> -
> -	QString id = QInputDialog::getItem(this, "Select Camera",
> -					   "Camera:", cameras, 0,
> -					   false, &result);
> -	if (!result)
> +	if (cameraSelectorDialog_->exec() != QDialog::Accepted)
>  		return std::string();
>  
> -	return id.toStdString();
> +	return cameraSelectorDialog_->getCameraId();
>  }
>  
>  int MainWindow::openCamera()
>  {
>  	std::string cameraName;
>  
> -	/*
> -	 * Use the camera specified on the command line, if any, or display the
> -	 * camera selection dialog box otherwise.
> -	 */
> +	/* Use camera provided on the command line else prompt for selection.*/

Anything wrong with the current comment ?

>  	if (options_.isSet(OptCamera))
>  		cameraName = static_cast<std::string>(options_[OptCamera]);
>  	else
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index fc70920f..def44605 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -35,6 +35,7 @@ class QComboBox;
>  
>  class Image;
>  class HotplugEvent;
> +class CameraSelectorDialog;

Alphabetical order please.

>  
>  enum {
>  	OptCamera = 'c',
> @@ -99,6 +100,8 @@ private:
>  	QString title_;
>  	QTimer titleTimer_;
>  
> +	CameraSelectorDialog *cameraSelectorDialog_;
> +
>  	/* Options */
>  	const OptionsParser::Options &options_;
>  
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index c46f4631..61861ea6 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -18,6 +18,7 @@ qcam_sources = files([
>      '../cam/image.cpp',
>      '../cam/options.cpp',
>      '../cam/stream_options.cpp',
> +    'cam_select_dialog.cpp',
>      'format_converter.cpp',
>      'main.cpp',
>      'main_window.cpp',
> @@ -26,6 +27,7 @@ qcam_sources = files([
>  ])
>  
>  qcam_moc_headers = files([
> +    'cam_select_dialog.h',
>      'main_window.h',
>      'viewfinder_qt.h',
>  ])

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list