[libcamera-devel] [PATCH v8 1/8] qcam: Use QDialog for selection of cameras at startup
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 23 03:38:48 CEST 2022
Hi Utkrash,
Thank you for the patch.
On Wed, Aug 10, 2022 at 08:33:42PM +0530, Utkarsh Tiwari via libcamera-devel 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 by
> the console.
s/by the console/on the command line/
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Difference from v7:
> 1. Updated the commit message to inform of the behavioural change in
> selecting the first camera.
> src/qcam/cam_select_dialog.cpp | 51 ++++++++++++++++++++++++++++++++++
> src/qcam/cam_select_dialog.h | 34 +++++++++++++++++++++++
> src/qcam/main_window.cpp | 44 +++++++++++------------------
> src/qcam/main_window.h | 3 ++
> src/qcam/meson.build | 2 ++
> 5 files changed, 106 insertions(+), 28 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..dceaa590
> --- /dev/null
> +++ b/src/qcam/cam_select_dialog.cpp
> @@ -0,0 +1,51 @@
> +/* 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 <string>
You don't need to include this header.
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> +#include <QComboBox>
> +#include <QDialog>
You can drop QDialog, as CameraSelectorDialog inherits from it, it's
guaranteed to be included in cam_select_dialog.h.
> +#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);
> +}
> +
> +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..5544f49a
> --- /dev/null
> +++ b/src/qcam/cam_select_dialog.h
> @@ -0,0 +1,34 @@
> +/* 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 <QComboBox>
QComboxBox can be forward-declared (see main_window.h).
> +#include <QDialog>
> +
> +class CameraSelectorDialog : public QDialog
> +{
> + Q_OBJECT
> +public:
> + CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> + QWidget *parent);
> +
> + ~CameraSelectorDialog() = default;
Defining a function in the class definition, regardless of whether you
define it with a manual implementation or with = default, results in the
function being inlined. This can increase the binary size, and should
thus only be done when needed. Here, you should write
CameraSelectorDialog(libcamera::CameraManager *cameraManager,
QWidget *parent);
~CameraSelectorDialog();
and in the .cpp file add
CameraSelectorDialog::~CameraSelectorDialog() = default;
> +
> + 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..e794221a 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"
> @@ -97,8 +97,8 @@ private:
> };
>
> MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
> - : saveRaw_(nullptr), options_(options), cm_(cm), allocator_(nullptr),
> - isCapturing_(false), captureRaw_(false)
> + : saveRaw_(nullptr), cameraSelectorDialog_(nullptr), options_(options),
> + cm_(cm), allocator_(nullptr), isCapturing_(false), captureRaw_(false)
> {
> int ret;
>
> @@ -290,38 +290,26 @@ 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)
> - return std::string();
> -
> - return id.toStdString();
> -}
> -
> -int MainWindow::openCamera()
> -{
> - std::string cameraName;
> + /* Construct the selection dialog, only the first time. */
> + if (!cameraSelectorDialog_)
> + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, this);
>
> /*
> * Use the camera specified on the command line, if any, or display the
> * camera selection dialog box otherwise.
> */
> if (options_.isSet(OptCamera))
> - cameraName = static_cast<std::string>(options_[OptCamera]);
> + return static_cast<std::string>(options_[OptCamera]);
> +
> + if (cameraSelectorDialog_->exec() == QDialog::Accepted)
> + return cameraSelectorDialog_->getCameraId();
> else
> - cameraName = chooseCamera();
> + return std::string();
> +}
> +
> +int MainWindow::openCamera()
> +{
> + std::string cameraName = chooseCamera();
>
> if (cameraName == "")
> return -EINVAL;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index fc70920f..4ad5e6e9 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -28,6 +28,7 @@
>
> #include "../cam/stream_options.h"
>
> +#include "cam_select_dialog.h"
> #include "viewfinder.h"
>
> class QAction;
> @@ -99,6 +100,8 @@ private:
> QString title_;
> QTimer titleTimer_;
>
> + CameraSelectorDialog *cameraSelectorDialog_;
You can forward-declare CameraSelectorDialog the same way we do for
Image and HotplugEvent.
With those small issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> /* 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