| Message ID | 20220319142545.40433-1-utkarsh02t@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Utkarsh Thank you for the patch. On 3/19/22 19:55, Utkarsh Tiwari via libcamera-devel wrote: > Showing Camera Id on the main window is quite non-intuitive for the user > > Implementation: A duplicate function exists in cam/main.cpp . There must be a better way to do this instead of duplication of code > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > --- > src/qcam/main_window.cpp | 61 +++++++++++++++++++++++++++++++++------- > src/qcam/main_window.h | 2 +- > 2 files changed, 52 insertions(+), 11 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index dd0e51f5..35e737c7 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -12,8 +12,10 @@ > #include <string> > > #include <libcamera/camera_manager.h> > +#include <libcamera/property_ids.h> > #include <libcamera/version.h> > > + spurious new line > #include <QComboBox> > #include <QCoreApplication> > #include <QFileDialog> > @@ -197,7 +199,7 @@ int MainWindow::createToolbars() > this, &MainWindow::switchCamera); > > for (const std::shared_ptr<Camera> &cam : cm_->cameras()) > - cameraCombo_->addItem(QString::fromStdString(cam->id())); > + cameraCombo_->addItem(QString::fromStdString(cameraName(cam.get()))); > > toolbar_->addWidget(cameraCombo_); > > @@ -287,6 +289,45 @@ void MainWindow::switchCamera(int index) > startStopAction_->setChecked(true); > } > > +std::string MainWindow::cameraName(const libcamera::Camera *camera) > +{ > + const ControlList &props = camera->properties(); > + bool addModel = true; > + std::string name; > + > + /* > + * Construct the name from the camera location, model and ID. The model > + * is only used if the location isn't present or is set to External. > + */ > + if (props.contains(properties::Location)) { > + switch (props.get(properties::Location)) { > + case properties::CameraLocationFront: > + addModel = false; > + name = "Internal front camera "; > + break; > + case properties::CameraLocationBack: > + addModel = false; > + name = "Internal back camera "; > + break; > + case properties::CameraLocationExternal: > + name = "External camera "; > + break; > + } > + } > + > + if (addModel && props.contains(properties::Model)) { > + /* > + * If the camera location is not availble use the camera model > + * to build the camera name. > + */ > + name = "'" + props.get(properties::Model) + "' "; > + } > + > + name += "(" + camera->id() + ")"; > + > + return name; > +} > + I get it that this is copied over from cam/main.cpp but since we are trying to keep the camera names construction in-sync across applications (cam and qcam), have you thought or discussed the possibility that this helper can go in libcamera::Camera class itself ? For e.g. as Camera::name() OR Camera::displayName() as a helper provided by libcamera. Applications not happy with this helper (given out by libcamera) can still construct/append their own quirks based on id(), location-property and so on.. as it is done right now anyway. I don't know, this might require some discussion but would simplify things a bit. > std::string MainWindow::chooseCamera() > { > QStringList cameras; > @@ -311,24 +352,24 @@ std::string MainWindow::chooseCamera() > > int MainWindow::openCamera() > { > - std::string cameraName; > + std::string cameraId; > > /* > * 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]); > + cameraId = static_cast<std::string>(options_[OptCamera]); > else > - cameraName = chooseCamera(); > + cameraId = chooseCamera(); > > - if (cameraName == "") > + if (cameraId == "") > return -EINVAL; > > /* Get and acquire the camera. */ > - camera_ = cm_->get(cameraName); > + camera_ = cm_->get(cameraId); > if (!camera_) { > - qInfo() << "Camera" << cameraName.c_str() << "not found"; > + qInfo() << "Camera" << cameraId.c_str() << "not found"; > return -ENODEV; > } This hunk is a variable rename so should be separated out in a different patch I think. > > @@ -339,7 +380,7 @@ int MainWindow::openCamera() > } > > /* Set the combo-box entry with the currently selected Camera. */ > - cameraCombo_->setCurrentText(QString::fromStdString(cameraName)); > + cameraCombo_->setCurrentText(QString::fromStdString(cameraName(camera_.get()))); > > return 0; > } > @@ -604,7 +645,7 @@ void MainWindow::processHotplug(HotplugEvent *e) > HotplugEvent::PlugEvent event = e->hotplugEvent(); > > if (event == HotplugEvent::HotPlug) { > - cameraCombo_->addItem(QString::fromStdString(camera->id())); > + cameraCombo_->addItem(QString::fromStdString(cameraName(camera))); > } else if (event == HotplugEvent::HotUnplug) { > /* Check if the currently-streaming camera is removed. */ > if (camera == camera_.get()) { > @@ -614,7 +655,7 @@ void MainWindow::processHotplug(HotplugEvent *e) > cameraCombo_->setCurrentIndex(0); > } > > - int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); > + int camIndex = cameraCombo_->findText(QString::fromStdString(cameraName(camera))); > cameraCombo_->removeItem(camIndex); > } > } > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > index 3fbe872c..b31c584f 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -70,7 +70,7 @@ private Q_SLOTS: > > private: > int createToolbars(); > - Spurious deletion of newline. It should be retained > + std::string cameraName(const libcamera::Camera *camera); > std::string chooseCamera(); > int openCamera(); >
Quoting Umang Jain via libcamera-devel (2022-03-21 06:33:47) > Hi Utkarsh > > Thank you for the patch. > > On 3/19/22 19:55, Utkarsh Tiwari via libcamera-devel wrote: > > Showing Camera Id on the main window is quite non-intuitive for the user > > > > Implementation: A duplicate function exists in cam/main.cpp . There must be a better way to do this instead of duplication of code A cleaner commit message could look something like: """ qcam: Display a human readable name in camera selection Showing the Camera ID on the main window and when selecting a camera is not very intuitive for the user. Camera IDs are unique references for the camera and include internal details that do not present an easy indicator for which camera it represents. Construct a Camera Name based on the camera properties by duplicating the existing naming scheme from cam to present more understandable device name to users of Qcam. """ We try to make sure the commit title clearly references which part of libcamera is being changed, (in this case 'qcam:'), and a commit message should try to clearly state the problem it's solving, and how it solves it. > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > > --- > > src/qcam/main_window.cpp | 61 +++++++++++++++++++++++++++++++++------- > > src/qcam/main_window.h | 2 +- > > 2 files changed, 52 insertions(+), 11 deletions(-) > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index dd0e51f5..35e737c7 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -12,8 +12,10 @@ > > #include <string> > > > > #include <libcamera/camera_manager.h> > > +#include <libcamera/property_ids.h> > > #include <libcamera/version.h> > > > > + > spurious new line > > #include <QComboBox> > > #include <QCoreApplication> > > #include <QFileDialog> > > @@ -197,7 +199,7 @@ int MainWindow::createToolbars() > > this, &MainWindow::switchCamera); > > > > for (const std::shared_ptr<Camera> &cam : cm_->cameras()) > > - cameraCombo_->addItem(QString::fromStdString(cam->id())); > > + cameraCombo_->addItem(QString::fromStdString(cameraName(cam.get()))); I think if MainWindow::cameraName() took a shared_ptr: std::string MainWindow::cameraName(const std::shared_ptr<Camera> &camera) then this doesn't have to '.get()' the ptr, it would just pass 'cam', and the cameraName function wouldn't have to otherwise change. > > > > toolbar_->addWidget(cameraCombo_); > > > > @@ -287,6 +289,45 @@ void MainWindow::switchCamera(int index) > > startStopAction_->setChecked(true); > > } > > > > +std::string MainWindow::cameraName(const libcamera::Camera *camera) > > +{ > > + const ControlList &props = camera->properties(); > > + bool addModel = true; > > + std::string name; > > + > > + /* > > + * Construct the name from the camera location, model and ID. The model > > + * is only used if the location isn't present or is set to External. > > + */ > > + if (props.contains(properties::Location)) { > > + switch (props.get(properties::Location)) { > > + case properties::CameraLocationFront: > > + addModel = false; > > + name = "Internal front camera "; > > + break; > > + case properties::CameraLocationBack: > > + addModel = false; > > + name = "Internal back camera "; > > + break; > > + case properties::CameraLocationExternal: > > + name = "External camera "; > > + break; > > + } > > + } > > + > > + if (addModel && props.contains(properties::Model)) { > > + /* > > + * If the camera location is not availble use the camera model > > + * to build the camera name. > > + */ > > + name = "'" + props.get(properties::Model) + "' "; > > + } > > + > > + name += "(" + camera->id() + ")"; > > + > > + return name; > > +} > > + > > > I get it that this is copied over from cam/main.cpp but since we are > trying to keep the camera names construction in-sync across applications > (cam and qcam), have you thought or discussed the possibility that this > helper can go in libcamera::Camera class itself ? For e.g. as > > Camera::name() > > OR > > Camera::displayName() > > as a helper provided by libcamera. Applications not happy with this > helper (given out by libcamera) can still construct/append their own > quirks based on id(), location-property and so on.. as it is done right > now anyway. > > I don't know, this might require some discussion but would simplify > things a bit. Defining how to name a camera is supposed to be up to the application, not libcamera, and there has indeed been some previous discussions about this. I wonder if providing a helper that does what we expect is useful though, and doesn't prevent applications from deciding to copy/or change that themselves as long as all the properties used are public anyway. But I think that means that duplicating the code is acceptable for now. > > std::string MainWindow::chooseCamera() > > { > > QStringList cameras; > > @@ -311,24 +352,24 @@ std::string MainWindow::chooseCamera() > > > > int MainWindow::openCamera() > > { > > - std::string cameraName; > > + std::string cameraId; > > > > /* > > * 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]); > > + cameraId = static_cast<std::string>(options_[OptCamera]); > > else > > - cameraName = chooseCamera(); > > + cameraId = chooseCamera(); > > > > - if (cameraName == "") > > + if (cameraId == "") > > return -EINVAL; > > > > /* Get and acquire the camera. */ > > - camera_ = cm_->get(cameraName); > > + camera_ = cm_->get(cameraId); > > if (!camera_) { > > - qInfo() << "Camera" << cameraName.c_str() << "not found"; > > + qInfo() << "Camera" << cameraId.c_str() << "not found"; > > return -ENODEV; > > } > > > This hunk is a variable rename so should be separated out in a different > patch I think. Maybe not essential, but certainly would be possible and cleaner I think to have this rename done as a first patch. > > > > > @@ -339,7 +380,7 @@ int MainWindow::openCamera() > > } > > > > /* Set the combo-box entry with the currently selected Camera. */ > > - cameraCombo_->setCurrentText(QString::fromStdString(cameraName)); > > + cameraCombo_->setCurrentText(QString::fromStdString(cameraName(camera_.get()))); > > > > return 0; > > } > > @@ -604,7 +645,7 @@ void MainWindow::processHotplug(HotplugEvent *e) > > HotplugEvent::PlugEvent event = e->hotplugEvent(); > > > > if (event == HotplugEvent::HotPlug) { > > - cameraCombo_->addItem(QString::fromStdString(camera->id())); > > + cameraCombo_->addItem(QString::fromStdString(cameraName(camera))); > > } else if (event == HotplugEvent::HotUnplug) { > > /* Check if the currently-streaming camera is removed. */ > > if (camera == camera_.get()) { > > @@ -614,7 +655,7 @@ void MainWindow::processHotplug(HotplugEvent *e) > > cameraCombo_->setCurrentIndex(0); > > } > > > > - int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); > > + int camIndex = cameraCombo_->findText(QString::fromStdString(cameraName(camera))); > > cameraCombo_->removeItem(camIndex); > > } > > } > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h > > index 3fbe872c..b31c584f 100644 > > --- a/src/qcam/main_window.h > > +++ b/src/qcam/main_window.h > > @@ -70,7 +70,7 @@ private Q_SLOTS: > > > > private: > > int createToolbars(); > > - > Spurious deletion of newline. It should be retained > > + std::string cameraName(const libcamera::Camera *camera); > > std::string chooseCamera(); > > int openCamera(); > >
Hi Kieran, On 3/21/22 18:34, Kieran Bingham wrote: > Quoting Umang Jain via libcamera-devel (2022-03-21 06:33:47) >> Hi Utkarsh >> >> Thank you for the patch. >> >> On 3/19/22 19:55, Utkarsh Tiwari via libcamera-devel wrote: >>> Showing Camera Id on the main window is quite non-intuitive for the user >>> >>> Implementation: A duplicate function exists in cam/main.cpp . There must be a better way to do this instead of duplication of code > A cleaner commit message could look something like: > > """ > qcam: Display a human readable name in camera selection > > Showing the Camera ID on the main window and when selecting a camera is > not very intuitive for the user. > > Camera IDs are unique references for the camera and include internal > details that do not present an easy indicator for which camera it > represents. > > Construct a Camera Name based on the camera properties by duplicating > the existing naming scheme from cam to present more understandable > device name to users of Qcam. > """ > > We try to make sure the commit title clearly references which part of > libcamera is being changed, (in this case 'qcam:'), and a commit message > should try to clearly state the problem it's solving, and how it solves > it. > > >>> Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> >>> --- >>> src/qcam/main_window.cpp | 61 +++++++++++++++++++++++++++++++++------- >>> src/qcam/main_window.h | 2 +- >>> 2 files changed, 52 insertions(+), 11 deletions(-) >>> >>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp >>> index dd0e51f5..35e737c7 100644 >>> --- a/src/qcam/main_window.cpp >>> +++ b/src/qcam/main_window.cpp >>> @@ -12,8 +12,10 @@ >>> #include <string> >>> >>> #include <libcamera/camera_manager.h> >>> +#include <libcamera/property_ids.h> >>> #include <libcamera/version.h> >>> >>> + >> spurious new line >>> #include <QComboBox> >>> #include <QCoreApplication> >>> #include <QFileDialog> >>> @@ -197,7 +199,7 @@ int MainWindow::createToolbars() >>> this, &MainWindow::switchCamera); >>> >>> for (const std::shared_ptr<Camera> &cam : cm_->cameras()) >>> - cameraCombo_->addItem(QString::fromStdString(cam->id())); >>> + cameraCombo_->addItem(QString::fromStdString(cameraName(cam.get()))); > I think if MainWindow::cameraName() took a shared_ptr: > > std::string MainWindow::cameraName(const std::shared_ptr<Camera> &camera) > > then this doesn't have to '.get()' the ptr, it would just pass 'cam', > and the cameraName function wouldn't have to otherwise change. > > >>> >>> toolbar_->addWidget(cameraCombo_); >>> >>> @@ -287,6 +289,45 @@ void MainWindow::switchCamera(int index) >>> startStopAction_->setChecked(true); >>> } >>> >>> +std::string MainWindow::cameraName(const libcamera::Camera *camera) >>> +{ >>> + const ControlList &props = camera->properties(); >>> + bool addModel = true; >>> + std::string name; >>> + >>> + /* >>> + * Construct the name from the camera location, model and ID. The model >>> + * is only used if the location isn't present or is set to External. >>> + */ >>> + if (props.contains(properties::Location)) { >>> + switch (props.get(properties::Location)) { >>> + case properties::CameraLocationFront: >>> + addModel = false; >>> + name = "Internal front camera "; >>> + break; >>> + case properties::CameraLocationBack: >>> + addModel = false; >>> + name = "Internal back camera "; >>> + break; >>> + case properties::CameraLocationExternal: >>> + name = "External camera "; >>> + break; >>> + } >>> + } >>> + >>> + if (addModel && props.contains(properties::Model)) { >>> + /* >>> + * If the camera location is not availble use the camera model >>> + * to build the camera name. >>> + */ >>> + name = "'" + props.get(properties::Model) + "' "; >>> + } >>> + >>> + name += "(" + camera->id() + ")"; >>> + >>> + return name; >>> +} >>> + >> >> I get it that this is copied over from cam/main.cpp but since we are >> trying to keep the camera names construction in-sync across applications >> (cam and qcam), have you thought or discussed the possibility that this >> helper can go in libcamera::Camera class itself ? For e.g. as >> >> Camera::name() >> >> OR >> >> Camera::displayName() >> >> as a helper provided by libcamera. Applications not happy with this >> helper (given out by libcamera) can still construct/append their own >> quirks based on id(), location-property and so on.. as it is done right >> now anyway. >> >> I don't know, this might require some discussion but would simplify >> things a bit. > Defining how to name a camera is supposed to be up to the application, > not libcamera, and there has indeed been some previous discussions about > this. The previous discussions were around a uniquely-defined camera-id instead of camera-name as far as I remember. > > I wonder if providing a helper that does what we expect is useful > though, and doesn't prevent applications from deciding to copy/or change > that themselves as long as all the properties used are public anyway. Precisely. Applications are free to append/structure the camera-name as their will - but a helper from libcamera might help as a base reference on what we might think as the best option. > > But I think that means that duplicating the code is acceptable for now. > > >>> std::string MainWindow::chooseCamera() >>> { >>> QStringList cameras; >>> @@ -311,24 +352,24 @@ std::string MainWindow::chooseCamera() >>> >>> int MainWindow::openCamera() >>> { >>> - std::string cameraName; >>> + std::string cameraId; >>> >>> /* >>> * 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]); >>> + cameraId = static_cast<std::string>(options_[OptCamera]); >>> else >>> - cameraName = chooseCamera(); >>> + cameraId = chooseCamera(); >>> >>> - if (cameraName == "") >>> + if (cameraId == "") >>> return -EINVAL; >>> >>> /* Get and acquire the camera. */ >>> - camera_ = cm_->get(cameraName); >>> + camera_ = cm_->get(cameraId); >>> if (!camera_) { >>> - qInfo() << "Camera" << cameraName.c_str() << "not found"; >>> + qInfo() << "Camera" << cameraId.c_str() << "not found"; >>> return -ENODEV; >>> } >> >> This hunk is a variable rename so should be separated out in a different >> patch I think. > Maybe not essential, but certainly would be possible and cleaner I think > to have this rename done as a first patch. > >>> >>> @@ -339,7 +380,7 @@ int MainWindow::openCamera() >>> } >>> >>> /* Set the combo-box entry with the currently selected Camera. */ >>> - cameraCombo_->setCurrentText(QString::fromStdString(cameraName)); >>> + cameraCombo_->setCurrentText(QString::fromStdString(cameraName(camera_.get()))); >>> >>> return 0; >>> } >>> @@ -604,7 +645,7 @@ void MainWindow::processHotplug(HotplugEvent *e) >>> HotplugEvent::PlugEvent event = e->hotplugEvent(); >>> >>> if (event == HotplugEvent::HotPlug) { >>> - cameraCombo_->addItem(QString::fromStdString(camera->id())); >>> + cameraCombo_->addItem(QString::fromStdString(cameraName(camera))); >>> } else if (event == HotplugEvent::HotUnplug) { >>> /* Check if the currently-streaming camera is removed. */ >>> if (camera == camera_.get()) { >>> @@ -614,7 +655,7 @@ void MainWindow::processHotplug(HotplugEvent *e) >>> cameraCombo_->setCurrentIndex(0); >>> } >>> >>> - int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); >>> + int camIndex = cameraCombo_->findText(QString::fromStdString(cameraName(camera))); >>> cameraCombo_->removeItem(camIndex); >>> } >>> } >>> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h >>> index 3fbe872c..b31c584f 100644 >>> --- a/src/qcam/main_window.h >>> +++ b/src/qcam/main_window.h >>> @@ -70,7 +70,7 @@ private Q_SLOTS: >>> >>> private: >>> int createToolbars(); >>> - >> Spurious deletion of newline. It should be retained >>> + std::string cameraName(const libcamera::Camera *camera); >>> std::string chooseCamera(); >>> int openCamera(); >>>
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index dd0e51f5..35e737c7 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -12,8 +12,10 @@ #include <string> #include <libcamera/camera_manager.h> +#include <libcamera/property_ids.h> #include <libcamera/version.h> + #include <QComboBox> #include <QCoreApplication> #include <QFileDialog> @@ -197,7 +199,7 @@ int MainWindow::createToolbars() this, &MainWindow::switchCamera); for (const std::shared_ptr<Camera> &cam : cm_->cameras()) - cameraCombo_->addItem(QString::fromStdString(cam->id())); + cameraCombo_->addItem(QString::fromStdString(cameraName(cam.get()))); toolbar_->addWidget(cameraCombo_); @@ -287,6 +289,45 @@ void MainWindow::switchCamera(int index) startStopAction_->setChecked(true); } +std::string MainWindow::cameraName(const libcamera::Camera *camera) +{ + const ControlList &props = camera->properties(); + bool addModel = true; + std::string name; + + /* + * Construct the name from the camera location, model and ID. The model + * is only used if the location isn't present or is set to External. + */ + if (props.contains(properties::Location)) { + switch (props.get(properties::Location)) { + case properties::CameraLocationFront: + addModel = false; + name = "Internal front camera "; + break; + case properties::CameraLocationBack: + addModel = false; + name = "Internal back camera "; + break; + case properties::CameraLocationExternal: + name = "External camera "; + break; + } + } + + if (addModel && props.contains(properties::Model)) { + /* + * If the camera location is not availble use the camera model + * to build the camera name. + */ + name = "'" + props.get(properties::Model) + "' "; + } + + name += "(" + camera->id() + ")"; + + return name; +} + std::string MainWindow::chooseCamera() { QStringList cameras; @@ -311,24 +352,24 @@ std::string MainWindow::chooseCamera() int MainWindow::openCamera() { - std::string cameraName; + std::string cameraId; /* * 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]); + cameraId = static_cast<std::string>(options_[OptCamera]); else - cameraName = chooseCamera(); + cameraId = chooseCamera(); - if (cameraName == "") + if (cameraId == "") return -EINVAL; /* Get and acquire the camera. */ - camera_ = cm_->get(cameraName); + camera_ = cm_->get(cameraId); if (!camera_) { - qInfo() << "Camera" << cameraName.c_str() << "not found"; + qInfo() << "Camera" << cameraId.c_str() << "not found"; return -ENODEV; } @@ -339,7 +380,7 @@ int MainWindow::openCamera() } /* Set the combo-box entry with the currently selected Camera. */ - cameraCombo_->setCurrentText(QString::fromStdString(cameraName)); + cameraCombo_->setCurrentText(QString::fromStdString(cameraName(camera_.get()))); return 0; } @@ -604,7 +645,7 @@ void MainWindow::processHotplug(HotplugEvent *e) HotplugEvent::PlugEvent event = e->hotplugEvent(); if (event == HotplugEvent::HotPlug) { - cameraCombo_->addItem(QString::fromStdString(camera->id())); + cameraCombo_->addItem(QString::fromStdString(cameraName(camera))); } else if (event == HotplugEvent::HotUnplug) { /* Check if the currently-streaming camera is removed. */ if (camera == camera_.get()) { @@ -614,7 +655,7 @@ void MainWindow::processHotplug(HotplugEvent *e) cameraCombo_->setCurrentIndex(0); } - int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id())); + int camIndex = cameraCombo_->findText(QString::fromStdString(cameraName(camera))); cameraCombo_->removeItem(camIndex); } } diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 3fbe872c..b31c584f 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -70,7 +70,7 @@ private Q_SLOTS: private: int createToolbars(); - + std::string cameraName(const libcamera::Camera *camera); std::string chooseCamera(); int openCamera();
Showing Camera Id on the main window is quite non-intuitive for the user Implementation: A duplicate function exists in cam/main.cpp . There must be a better way to do this instead of duplication of code Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> --- src/qcam/main_window.cpp | 61 +++++++++++++++++++++++++++++++++------- src/qcam/main_window.h | 2 +- 2 files changed, 52 insertions(+), 11 deletions(-)