[{"id":32470,"web_url":"https://patchwork.libcamera.org/comment/32470/","msgid":"<173291750600.4100433.14975771097664132266@ping.linuxembedded.co.uk>","date":"2024-11-29T21:58:26","subject":"Re: [PATCH v2] qcam: Use pointer when choosing camera","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stanislaw Gruszka (2024-10-31 11:02:56)\n> In order to remove redundant camera ID lookups and comparisons switch\n> to pointer-based checks when choosing and switching cameras.\n> \n> Co-developed-by: Laurent Pinchart \n> Signed-off-by: Laurent Pinchart \n> Signed-off-by: Stanislaw Gruszka \n\nJust ran this on my x13s,\n\nTested-by: Kieran Bingham \n\nLooks ok too so:\n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> v2:\n> - better error message when no cameras are present\n> - use camera_->id() is switch button, missed in previous patch\n> \n> src/apps/qcam/main_window.cpp | 43 +++++++++++++++--------------------\n> src/apps/qcam/main_window.h | 2 +-\n> 2 files changed, 19 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> index de487672..3880a846 100644\n> --- a/src/apps/qcam/main_window.cpp\n> +++ b/src/apps/qcam/main_window.cpp\n> @@ -251,16 +251,14 @@ void MainWindow::updateTitle()\n> void MainWindow::switchCamera()\n> {\n> /* Get and acquire the new camera. */\n> - std::string newCameraId = chooseCamera();\n> + std::shared_ptr cam = chooseCamera();\n> \n> - if (newCameraId.empty())\n> + if (!cam)\n> return;\n> \n> - if (camera_ && newCameraId == camera_->id())\n> + if (camera_ && cam == camera_)\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> return;\n> @@ -282,46 +280,41 @@ void MainWindow::switchCamera()\n> startStopAction_->setChecked(true);\n> \n> /* Display the current cameraId in the toolbar .*/\n> - cameraSelectButton_->setText(QString::fromStdString(newCameraId));\n> + cameraSelectButton_->setText(QString::fromStdString(cam->id()));\n> }\n> \n> -std::string MainWindow::chooseCamera()\n> +std::shared_ptr MainWindow::chooseCamera()\n> {\n> if (cameraSelectorDialog_->exec() != QDialog::Accepted)\n> - return std::string();\n> + return {};\n> \n> - return cameraSelectorDialog_->getCameraId();\n> + std::string id = cameraSelectorDialog_->getCameraId();\n> + return cm_->get(id);\n> }\n> \n> int MainWindow::openCamera()\n> {\n> - std::string cameraName;\n> -\n> /*\n> * If a camera is specified on the command line, get it. Otherwise, if\n> * only one camera is available, pick it automatically, else, display\n> * the selector dialog box.\n> */\n> if (options_.isSet(OptCamera)) {\n> - cameraName = static_cast(options_[OptCamera]);\n> + std::string cameraName = static_cast(options_[OptCamera]);\n> + camera_ = cm_->get(cameraName);\n> + if (!camera_)\n> + qInfo() << \"Camera\" << cameraName.c_str() << \"not found\";\n> } else {\n> std::vector> cameras = cm_->cameras();\n> - if (cameras.size() == 1)\n> - cameraName = cameras[0]->id();\n> - else\n> - cameraName = chooseCamera();\n> + camera_ = (cameras.size() == 1) ? cameras[0] : chooseCamera();\n> + if (!camera_)\n> + qInfo() << \"No camera detected\";\n> }\n> \n> - if (cameraName == \"\")\n> - return -EINVAL;\n> -\n> - /* Get and acquire the camera. */\n> - camera_ = cm_->get(cameraName);\n> - if (!camera_) {\n> - qInfo() << \"Camera\" << cameraName.c_str() << \"not found\";\n> + if (!camera_)\n> return -ENODEV;\n> - }\n> \n> + /* Acquire the camera. */\n> if (camera_->acquire()) {\n> qInfo() << \"Failed to acquire camera\";\n> camera_.reset();\n> @@ -329,7 +322,7 @@ int MainWindow::openCamera()\n> }\n> \n> /* Set the camera switch button with the currently selected Camera id. */\n> - cameraSelectButton_->setText(QString::fromStdString(cameraName));\n> + cameraSelectButton_->setText(QString::fromStdString(camera_->id()));\n> \n> return 0;\n> }\n> diff --git a/src/apps/qcam/main_window.h b/src/apps/qcam/main_window.h\n> index 4cead734..81fcf915 100644\n> --- a/src/apps/qcam/main_window.h\n> +++ b/src/apps/qcam/main_window.h\n> @@ -73,7 +73,7 @@ private Q_SLOTS:\n> private:\n> int createToolbars();\n> \n> - std::string chooseCamera();\n> + std::shared_ptr chooseCamera();\n> int openCamera();\n> \n> int startCapture();\n> -- \n> 2.43.0\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 8BCEDC31E9\n\tfor ;\n\tFri, 29 Nov 2024 21:58:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A61D36603F;\n\tFri, 29 Nov 2024 22:58:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BACE66025\n\tfor ;\n\tFri, 29 Nov 2024 22:58:29 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64D58A57;\n\tFri, 29 Nov 2024 22:58:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VXjz2Nqc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732917484;\n\tbh=vq/pRAC8wY/N3shmpMpQQG3PnkUeVoB0E41b5ZFz/Y0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=VXjz2Nqc0xmJhrsnuZsOoczwO89zhL7B91XZElq/i51F+gAIN/OSn00Q+zNffgBqC\n\tAnna90ybVdVEYWoK3agWBg4pWtSmdZrqkR/VUXu+GIJPutN9Fgy8WBRbFhUNs3lZbm\n\t1giozjbAQRriC7GFEc4VbgqWcVtaHkDqWaumEn+0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241031110256.415079-1-stanislaw.gruszka@linux.intel.com>","References":"<20241031110256.415079-1-stanislaw.gruszka@linux.intel.com>","Subject":"Re: [PATCH v2] qcam: Use pointer when choosing camera","From":"Kieran Bingham ","To":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 29 Nov 2024 21:58:26 +0000","Message-ID":"<173291750600.4100433.14975771097664132266@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]