[{"id":24848,"web_url":"https://patchwork.libcamera.org/comment/24848/","msgid":"<166193652828.15972.9623002895178195397@Monstersaurus>","date":"2022-08-31T09:02:08","subject":"Re: [libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace\n\tcameraCombo_ with CameraSelectorDialog","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Utkarsh Tiwari (2022-08-31 06:49:34)\n> Replace the cameraCombo_ on the toolbar with a QPushButton which\n> displays the CameraSelectorDialog. This would allow the user to view\n> information about the camera when switching.\n> \n> The QPushButton text is set to the camera Id currently in use.\n> \n> Signed-off-by: Utkarsh Tiwari \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n> ---\n> Differences from v9:\n> 1. Checking for OptCamera now happens in 1/7 in openCamera dropping\n> the firstCameraSelect_\n> 2. Improve curly brace structure\n> 3. Drop QComboBox\n> src/qcam/main_window.cpp | 33 ++++++++++++++++-----------------\n> src/qcam/main_window.h | 5 +++--\n> 2 files changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index e8e22d49..505bdc56 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -14,7 +14,6 @@\n> #include \n> #include \n> \n> -#include \n> #include \n> #include \n> #include \n> @@ -195,14 +194,11 @@ int MainWindow::createToolbars()\n> connect(action, &QAction::triggered, this, &MainWindow::quit);\n> \n> /* Camera selector. */\n> - cameraCombo_ = new QComboBox();\n> - connect(cameraCombo_, QOverload::of(&QComboBox::activated),\n> + cameraSelectButton_ = new QPushButton;\n> + connect(cameraSelectButton_, &QPushButton::clicked,\n> this, &MainWindow::switchCamera);\n> \n> - for (const std::shared_ptr &cam : cm_->cameras())\n> - cameraCombo_->addItem(QString::fromStdString(cam->id()));\n> -\n> - toolbar_->addWidget(cameraCombo_);\n> + toolbar_->addWidget(cameraSelectButton_);\n> \n> toolbar_->addSeparator();\n> \n> @@ -262,14 +258,18 @@ void MainWindow::updateTitle()\n> * Camera Selection\n> */\n> \n> -void MainWindow::switchCamera(int index)\n> +void MainWindow::switchCamera()\n> {\n> /* Get and acquire the new camera. */\n> - const auto &cameras = cm_->cameras();\n> - if (static_cast(index) >= cameras.size())\n> + std::string newCameraId = chooseCamera();\n> +\n> + if (newCameraId.empty())\n> return;\n> \n> - const std::shared_ptr &cam = cameras[index];\n> + if (camera_ && newCameraId == camera_->id())\n> + return;\n> +\n> + const std::shared_ptr &cam = cm_->get(newCameraId);\n> \n> if (cam->acquire()) {\n> qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> @@ -288,6 +288,9 @@ void MainWindow::switchCamera(int index)\n> camera_ = cam;\n> \n> startStopAction_->setChecked(true);\n> +\n> + /* Display the current cameraId in the toolbar .*/\n> + cameraSelectButton_->setText(QString::fromStdString(newCameraId));\n\nReally minor, and doesn't deserve a new version/iteration (could be done\nwhile applying) - but I'd set the text before calling the\nstartStopAction(true).\n\nLogically - it makes sense to update the context before starting it.\nThat's all - functionally here, I don't think it will have a significant\ndifference.\n\nThis already has tags and is good to go though I think.\n\n\n> }\n> \n> std::string MainWindow::chooseCamera()\n> @@ -324,8 +327,8 @@ int MainWindow::openCamera()\n> return -EBUSY;\n> }\n> \n> - /* Set the combo-box entry with the currently selected Camera. */\n> - cameraCombo_->setCurrentText(QString::fromStdString(cameraName));\n> + /* Set the camera switch button with the currently selected Camera id. */\n> + cameraSelectButton_->setText(QString::fromStdString(cameraName));\n> \n> return 0;\n> }\n> @@ -591,7 +594,6 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> HotplugEvent::PlugEvent event = e->hotplugEvent();\n> \n> if (event == HotplugEvent::HotPlug) {\n> - cameraCombo_->addItem(cameraId);\n> cameraSelectorDialog_->addCamera(cameraId);\n> } else if (event == HotplugEvent::HotUnplug) {\n> /* Check if the currently-streaming camera is removed. */\n> @@ -599,11 +601,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> toggleCapture(false);\n> camera_->release();\n> camera_.reset();\n> - cameraCombo_->setCurrentIndex(0);\n> }\n> \n> - int camIndex = cameraCombo_->findText(cameraId);\n> - cameraCombo_->removeItem(camIndex);\n> cameraSelectorDialog_->removeCamera(cameraId);\n> }\n> }\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index def44605..79723256 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -23,6 +23,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -60,7 +61,7 @@ private Q_SLOTS:\n> void quit();\n> void updateTitle();\n> \n> - void switchCamera(int index);\n> + void switchCamera();\n> void toggleCapture(bool start);\n> \n> void saveImageAs();\n> @@ -90,7 +91,7 @@ private:\n> /* UI elements */\n> QToolBar *toolbar_;\n> QAction *startStopAction_;\n> - QComboBox *cameraCombo_;\n> + QPushButton *cameraSelectButton_;\n> QAction *saveRaw_;\n> ViewFinder *viewfinder_;\n> \n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A2EF0C3272\n\tfor ;\n\tWed, 31 Aug 2022 09:02:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5A8D61FBE;\n\tWed, 31 Aug 2022 11:02:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A1CA603E1\n\tfor ;\n\tWed, 31 Aug 2022 11:02:11 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 01F29481;\n\tWed, 31 Aug 2022 11:02:10 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661936533;\n\tbh=2CTYeH5RwRzICZDO/FjeIvJWi2E0N7SZwZj8Mlyug88=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=CIElx7Pj7+v2NZS+QFnT7B6mfxeuBtcFV2qGzz8J3mX2KmlRkRKPHQDyEnJA1UPTj\n\tuKwmUGh9ZXLZpjRfV9M82XzoXObKUT7tB8x8bDQ7q3HkYg+Dw/RDkCcxJMv9ZHW5tA\n\tveZoidJx/wJT4BQl8pcDm6x+E8pI40g2TW+TLDVsS67JwXuHIYoIc7LfiAyDi0tZw1\n\tzcXeK9AvlS1Q9vI/TuTJWaRdXqF/zgqM7hnfX4i5zg/+nVIPhxSE6krcbVySyhvR/F\n\texx96Dc8QYKVFflpxvruEYcswgnB9Z6REKinrv2T30ki5WNbH/NdMc7iHx6ppr2ClG\n\tU5Hyu39up83Kw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661936531;\n\tbh=2CTYeH5RwRzICZDO/FjeIvJWi2E0N7SZwZj8Mlyug88=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gqR+t0isUhmfMuZDCGozh6WRjPR0NSrLN8mzguUga0o1GtPxPRvejaeRZO7ovGjVg\n\tJ69JFkxxC+O/9KXPt3nO0Yh80xTe0vUqXrU0/MCF+OLEjy66F+6AqXKt+w8+MuQPJ3\n\tdIspQW1bx6BDxSr7R7WLYJSjFtjM+JArcqhelAZg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gqR+t0is\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220831054938.21617-4-utkarsh02t@gmail.com>","References":"<20220831054938.21617-1-utkarsh02t@gmail.com>\n\t<20220831054938.21617-4-utkarsh02t@gmail.com>","To":"Utkarsh Tiwari ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 31 Aug 2022 10:02:08 +0100","Message-ID":"<166193652828.15972.9623002895178195397@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace\n\tcameraCombo_ with CameraSelectorDialog","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24853,"web_url":"https://patchwork.libcamera.org/comment/24853/","msgid":"","date":"2022-08-31T09:14:53","subject":"Re: [libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace\n\tcameraCombo_ with CameraSelectorDialog","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Utkarsh,\n\nThank you for the patch.\n\nOn Wed, Aug 31, 2022 at 11:19:34AM +0530, Utkarsh Tiwari wrote:\n> Replace the cameraCombo_ on the toolbar with a QPushButton which\n> displays the CameraSelectorDialog. This would allow the user to view\n> information about the camera when switching.\n> \n> The QPushButton text is set to the camera Id currently in use.\n> \n> Signed-off-by: Utkarsh Tiwari \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n> ---\n> Differences from v9:\n> 1. Checking for OptCamera now happens in 1/7 in openCamera dropping\n> the firstCameraSelect_\n> 2. Improve curly brace structure\n> 3. Drop QComboBox\n> src/qcam/main_window.cpp | 33 ++++++++++++++++-----------------\n> src/qcam/main_window.h | 5 +++--\n> 2 files changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index e8e22d49..505bdc56 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -14,7 +14,6 @@\n> #include \n> #include \n> \n> -#include \n> #include \n> #include \n> #include \n> @@ -195,14 +194,11 @@ int MainWindow::createToolbars()\n> \tconnect(action, &QAction::triggered, this, &MainWindow::quit);\n> \n> \t/* Camera selector. */\n> -\tcameraCombo_ = new QComboBox();\n> -\tconnect(cameraCombo_, QOverload::of(&QComboBox::activated),\n> +\tcameraSelectButton_ = new QPushButton;\n> +\tconnect(cameraSelectButton_, &QPushButton::clicked,\n> \t\tthis, &MainWindow::switchCamera);\n> \n> -\tfor (const std::shared_ptr &cam : cm_->cameras())\n> -\t\tcameraCombo_->addItem(QString::fromStdString(cam->id()));\n> -\n> -\ttoolbar_->addWidget(cameraCombo_);\n> +\ttoolbar_->addWidget(cameraSelectButton_);\n> \n> \ttoolbar_->addSeparator();\n> \n> @@ -262,14 +258,18 @@ void MainWindow::updateTitle()\n> * Camera Selection\n> */\n> \n> -void MainWindow::switchCamera(int index)\n> +void MainWindow::switchCamera()\n> {\n> \t/* Get and acquire the new camera. */\n> -\tconst auto &cameras = cm_->cameras();\n> -\tif (static_cast(index) >= cameras.size())\n> +\tstd::string newCameraId = chooseCamera();\n> +\n> +\tif (newCameraId.empty())\n> \t\treturn;\n> \n> -\tconst std::shared_ptr &cam = cameras[index];\n> +\tif (camera_ && newCameraId == camera_->id())\n> +\t\treturn;\n> +\n> +\tconst std::shared_ptr &cam = cm_->get(newCameraId);\n> \n> \tif (cam->acquire()) {\n> \t\tqInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> @@ -288,6 +288,9 @@ void MainWindow::switchCamera(int index)\n> \tcamera_ = cam;\n> \n> \tstartStopAction_->setChecked(true);\n> +\n> +\t/* Display the current cameraId in the toolbar .*/\n> +\tcameraSelectButton_->setText(QString::fromStdString(newCameraId));\n> }\n> \n> std::string MainWindow::chooseCamera()\n> @@ -324,8 +327,8 @@ int MainWindow::openCamera()\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\t/* Set the combo-box entry with the currently selected Camera. */\n> -\tcameraCombo_->setCurrentText(QString::fromStdString(cameraName));\n> +\t/* Set the camera switch button with the currently selected Camera id. */\n> +\tcameraSelectButton_->setText(QString::fromStdString(cameraName));\n> \n> \treturn 0;\n> }\n> @@ -591,7 +594,6 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> \tHotplugEvent::PlugEvent event = e->hotplugEvent();\n> \n> \tif (event == HotplugEvent::HotPlug) {\n> -\t\tcameraCombo_->addItem(cameraId);\n> \t\tcameraSelectorDialog_->addCamera(cameraId);\n> \t} else if (event == HotplugEvent::HotUnplug) {\n> \t\t/* Check if the currently-streaming camera is removed. */\n> @@ -599,11 +601,8 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> \t\t\ttoggleCapture(false);\n> \t\t\tcamera_->release();\n> \t\t\tcamera_.reset();\n> -\t\t\tcameraCombo_->setCurrentIndex(0);\n> \t\t}\n> \n> -\t\tint camIndex = cameraCombo_->findText(cameraId);\n> -\t\tcameraCombo_->removeItem(camIndex);\n> \t\tcameraSelectorDialog_->removeCamera(cameraId);\n> \t}\n> }\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index def44605..79723256 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -23,6 +23,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -60,7 +61,7 @@ private Q_SLOTS:\n> \tvoid quit();\n> \tvoid updateTitle();\n> \n> -\tvoid switchCamera(int index);\n> +\tvoid switchCamera();\n> \tvoid toggleCapture(bool start);\n> \n> \tvoid saveImageAs();\n> @@ -90,7 +91,7 @@ private:\n> \t/* UI elements */\n> \tQToolBar *toolbar_;\n> \tQAction *startStopAction_;\n> -\tQComboBox *cameraCombo_;\n\nYou can drop the forward declaration of QComboBox above in this file.\n\n> +\tQPushButton *cameraSelectButton_;\n> \tQAction *saveRaw_;\n> \tViewFinder *viewfinder_;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2E7B7C3272\n\tfor ;\n\tWed, 31 Aug 2022 09:15:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5392C61FBE;\n\tWed, 31 Aug 2022 11:15:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CA41603E1\n\tfor ;\n\tWed, 31 Aug 2022 11:15:05 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 37D49481;\n\tWed, 31 Aug 2022 11:15:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661937306;\n\tbh=VkiA3w+2nvhj0yq4UmDC5R/VdpUuNpkOswaYcoFkRHU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=kWT7yYj6BY5r/H6dVkm/5q0qNzIiDFQ8unSwKNAzii9OKa6jo8MVwK2KhUaugzo7Z\n\t7pRsa3Q7N6p1SKBr/6zQqI6BD/PtKColNTEbT8ojguswx4nS/4Jk7hj4bvwXGMHq6F\n\tyq9a4UmMe9IW70l1kP0hIWL6npVApvmCLM46O0r4FmeDkQkDtofHsBlBWzfF4GV6G7\n\tNbNRQQx0+UhzpdX5GkCb2XvXbnLwfwG7rXNToLI8+Iv9mbVB9X8KJEPBHKegdgzeq2\n\t5TT12eaUfgD6GJZwQ5fBEYEXswxlCehGlYS5MNTBDBhVLKT59Xcz6KB98j7Ji9FSnG\n\tOGZpjgyn3PDAA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661937304;\n\tbh=VkiA3w+2nvhj0yq4UmDC5R/VdpUuNpkOswaYcoFkRHU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Mvo3lcBjjBjJA7T4X6vDs9JACrtNKjg2htUmlk1CYhZBsgMKcMexXGBntXT+fmsnD\n\tt4lMNsDsHNTq9tRXehq5MhqtRfP/taSIqwzQJGJKFeOcgKWKzRzj95vj0mZu9ujjQI\n\t1dcJAwfK8NcpcQJ/2CFbuF+Hmg9yVr1uO2xyEoC0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Mvo3lcBj\"; dkim-atps=neutral","Date":"Wed, 31 Aug 2022 12:14:53 +0300","To":"Utkarsh Tiwari ","Message-ID":"","References":"<20220831054938.21617-1-utkarsh02t@gmail.com>\n\t<20220831054938.21617-4-utkarsh02t@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220831054938.21617-4-utkarsh02t@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace\n\tcameraCombo_ with CameraSelectorDialog","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]