| Message ID | 20220803175517.175332-3-utkarsh02t@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Utkarsh Thank you for the patch. On 8/3/22 23:25, Utkarsh Tiwari via libcamera-devel wrote: > Currently if there is HotPlug event when the user is on the Camera > selection dialog, the QComboBox didn't update to reflect the change. > > If the cameraIdComboBox_ contains a valid QComboBox then update it to > reflect the HotPlug change. Check the validity of QComboBox pointer by > guarding it with a QPointer. > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > --- > src/qcam/main_window.cpp | 23 +++++++++++++++-------- > src/qcam/main_window.h | 4 ++++ > 2 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 7761a6c6..80a73b68 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -293,18 +293,15 @@ void MainWindow::switchCamera(int index) > > std::string MainWindow::chooseCamera() > { > - QStringList cameras; > std::string result; > > /* Present a dialog box to pick a camera. */ > QDialog *cameraSelectDialog = new QDialog(this); > > /* Setup a QComboBox to display camera Ids. */ > + cameraIdComboBox_ = new QComboBox; > for (const std::shared_ptr<Camera> &cam : cm_->cameras()) > - cameras.push_back(QString::fromStdString(cam->id())); > - > - QComboBox *cameraIdComboBox = new QComboBox; > - cameraIdComboBox->addItems(cameras); > + cameraIdComboBox_->addItem(QString::fromStdString(cam->id())); > > /* Setup QDialogButtonBox. */ > QDialogButtonBox *dialogButtonBox = new QDialogButtonBox; > @@ -313,7 +310,7 @@ std::string MainWindow::chooseCamera() > > connect(dialogButtonBox, &QDialogButtonBox::accepted, > this, [&]() { > - result = cameraIdComboBox->currentText().toStdString(); > + result = cameraIdComboBox_->currentText().toStdString(); > cameraSelectDialog->accept(); > }); > > @@ -325,7 +322,7 @@ std::string MainWindow::chooseCamera() > > /* Setup the layout for the dialog. */ > QFormLayout *cameraSelectLayout = new QFormLayout(cameraSelectDialog); > - cameraSelectLayout->addRow("Camera: ", cameraIdComboBox); > + cameraSelectLayout->addRow("Camera: ", cameraIdComboBox_); > cameraSelectLayout->addWidget(dialogButtonBox); > > cameraSelectDialog->exec(); > @@ -628,7 +625,12 @@ void MainWindow::processHotplug(HotplugEvent *e) > HotplugEvent::PlugEvent event = e->hotplugEvent(); > > if (event == HotplugEvent::HotPlug) { > - cameraCombo_->addItem(QString::fromStdString(camera->id())); > + QString cameraId = QString::fromStdString(camera->id()); > + cameraCombo_->addItem(cameraId); > + > + /* Update cameraIdCombox_ to include the new camera. */ > + if (cameraIdComboBox_) > + cameraIdComboBox_->addItem(cameraId); > } else if (event == HotplugEvent::HotUnplug) { > /* Check if the currently-streaming camera is removed. */ > if (camera == camera_.get()) { > @@ -638,6 +640,11 @@ void MainWindow::processHotplug(HotplugEvent *e) > cameraCombo_->setCurrentIndex(0); > } > > + if (cameraIdComboBox_) { > + int cameraIdIndex = cameraIdComboBox_->findText(QString::fromStdString(camera_->id())); > + cameraIdComboBox_->removeItem(cameraIdIndex); > + } > + > int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); > cameraCombo_->removeItem(camIndex); > } > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index fc70920f..b8122eb9 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -23,7 +23,9 @@ > #include <QMainWindow> > #include <QMutex> > #include <QObject> > +#include <QPointer> > #include <QQueue> > +#include <QStringList> > #include <QTimer> > > #include "../cam/stream_options.h" > @@ -99,6 +101,8 @@ private: > QString title_; > QTimer titleTimer_; > > + QPointer<QComboBox> cameraIdComboBox_; You can simply start with a private cameraIdComboBox_ member in 1/4, rather than defining it locally, then making it private here. > + > /* Options */ > const OptionsParser::Options &options_; >
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 7761a6c6..80a73b68 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -293,18 +293,15 @@ void MainWindow::switchCamera(int index) std::string MainWindow::chooseCamera() { - QStringList cameras; std::string result; /* Present a dialog box to pick a camera. */ QDialog *cameraSelectDialog = new QDialog(this); /* Setup a QComboBox to display camera Ids. */ + cameraIdComboBox_ = new QComboBox; for (const std::shared_ptr<Camera> &cam : cm_->cameras()) - cameras.push_back(QString::fromStdString(cam->id())); - - QComboBox *cameraIdComboBox = new QComboBox; - cameraIdComboBox->addItems(cameras); + cameraIdComboBox_->addItem(QString::fromStdString(cam->id())); /* Setup QDialogButtonBox. */ QDialogButtonBox *dialogButtonBox = new QDialogButtonBox; @@ -313,7 +310,7 @@ std::string MainWindow::chooseCamera() connect(dialogButtonBox, &QDialogButtonBox::accepted, this, [&]() { - result = cameraIdComboBox->currentText().toStdString(); + result = cameraIdComboBox_->currentText().toStdString(); cameraSelectDialog->accept(); }); @@ -325,7 +322,7 @@ std::string MainWindow::chooseCamera() /* Setup the layout for the dialog. */ QFormLayout *cameraSelectLayout = new QFormLayout(cameraSelectDialog); - cameraSelectLayout->addRow("Camera: ", cameraIdComboBox); + cameraSelectLayout->addRow("Camera: ", cameraIdComboBox_); cameraSelectLayout->addWidget(dialogButtonBox); cameraSelectDialog->exec(); @@ -628,7 +625,12 @@ void MainWindow::processHotplug(HotplugEvent *e) HotplugEvent::PlugEvent event = e->hotplugEvent(); if (event == HotplugEvent::HotPlug) { - cameraCombo_->addItem(QString::fromStdString(camera->id())); + QString cameraId = QString::fromStdString(camera->id()); + cameraCombo_->addItem(cameraId); + + /* Update cameraIdCombox_ to include the new camera. */ + if (cameraIdComboBox_) + cameraIdComboBox_->addItem(cameraId); } else if (event == HotplugEvent::HotUnplug) { /* Check if the currently-streaming camera is removed. */ if (camera == camera_.get()) { @@ -638,6 +640,11 @@ void MainWindow::processHotplug(HotplugEvent *e) cameraCombo_->setCurrentIndex(0); } + if (cameraIdComboBox_) { + int cameraIdIndex = cameraIdComboBox_->findText(QString::fromStdString(camera_->id())); + cameraIdComboBox_->removeItem(cameraIdIndex); + } + int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); cameraCombo_->removeItem(camIndex); } diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index fc70920f..b8122eb9 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -23,7 +23,9 @@ #include <QMainWindow> #include <QMutex> #include <QObject> +#include <QPointer> #include <QQueue> +#include <QStringList> #include <QTimer> #include "../cam/stream_options.h" @@ -99,6 +101,8 @@ private: QString title_; QTimer titleTimer_; + QPointer<QComboBox> cameraIdComboBox_; + /* Options */ const OptionsParser::Options &options_;
Currently if there is HotPlug event when the user is on the Camera selection dialog, the QComboBox didn't update to reflect the change. If the cameraIdComboBox_ contains a valid QComboBox then update it to reflect the HotPlug change. Check the validity of QComboBox pointer by guarding it with a QPointer. Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> --- src/qcam/main_window.cpp | 23 +++++++++++++++-------- src/qcam/main_window.h | 4 ++++ 2 files changed, 19 insertions(+), 8 deletions(-)