[{"id":24413,"web_url":"https://patchwork.libcamera.org/comment/24413/","msgid":"<165994813694.15821.13542708845061880792@Monstersaurus>","date":"2022-08-08T08:42:16","subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Utkarsh Tiwari via libcamera-devel (2022-08-06 20:04:31)\n> Currently if there is HotPlug event when the user is on the Camera\n> selection dialog, the QComboBox didn't update to reflect the change.\n> \n> If the QDialog exists then alert it for the Hotplug event. The check\n> for QDialog existance is done by QPointer.\n> \n> Signed-off-by: Utkarsh Tiwari \n> ---\n> src/qcam/cam_select_dialog.h | 14 ++++++++++++++\n> src/qcam/main_window.cpp | 6 ++++++\n> 2 files changed, 20 insertions(+)\n> \n> diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h\n> index c23bad59..ee65eb88 100644\n> --- a/src/qcam/cam_select_dialog.h\n> +++ b/src/qcam/cam_select_dialog.h\n> @@ -56,6 +56,20 @@ public:\n> return cameraIdComboBox_->currentText().toStdString();\n> }\n> \n> + /* Hotplug / Unplug Support. */\n> + void cameraAdded(libcamera::Camera *camera)\n> + {\n> + cameraIdComboBox_->addItem(QString::fromStdString(camera->id()));\n> + }\n> +\n> + void cameraRemoved(libcamera::Camera *camera)\n> + {\n> + int cameraIndex = cameraIdComboBox_->findText(\n> + QString::fromStdString(camera->id()));\n> +\n\nWill this always be guaranteed to find an item? Are there any races\nwhere we might try to remove a camera that hasn't been added?\n\nI expect not, but just curious to be sure what error case might occur.\n\nBut it's fine:\n\nhttps://doc.qt.io/qt-6/qcombobox.html#removeItem:\n void QComboBox::removeItem(int index)\n\n Removes the item at the given index from the combobox. This will\n update the current index if the index is removed.\n\n This function does nothing if index is out of range.\n\n\nSo even if the camera isn't found - it just won't do anything.\n\n\n> + cameraIdComboBox_->removeItem(cameraIndex);\n> + }\n> +\n> private:\n> libcamera::CameraManager *cm_;\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 758e2c94..dd30817d 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -594,6 +594,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> \n> if (event == HotplugEvent::HotPlug) {\n> cameraCombo_->addItem(QString::fromStdString(camera->id()));\n> +\n> + if (camSelectDialog_)\n> + camSelectDialog_->cameraAdded(camera);\n\nThis seems reasonable - but we're updating a camSelectDialog_ even if\nit's not visible - and I think it gets destroyed and remade when opened?\n(I'll have to go back and check) - but this is good for if we do re-use\nthe dialog anyway.\n\nAnyway, I think this is ok overall.\n\n\nReviewed-by: Kieran Bingham \n\n> } else if (event == HotplugEvent::HotUnplug) {\n> /* Check if the currently-streaming camera is removed. */\n> if (camera == camera_.get()) {\n> @@ -605,6 +608,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> \n> int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id()));\n> cameraCombo_->removeItem(camIndex);\n> +\n> + if (camSelectDialog_)\n> + camSelectDialog_->cameraRemoved(camera);\n> }\n> }\n> \n> -- \n> 2.25.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 92681C3272\n\tfor ;\n\tMon, 8 Aug 2022 08:42:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0954063327;\n\tMon, 8 Aug 2022 10:42:21 +0200 (CEST)","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 487C163326\n\tfor ;\n\tMon, 8 Aug 2022 10:42:19 +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 DBCAC481;\n\tMon, 8 Aug 2022 10:42:18 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659948141;\n\tbh=Vxi8nFc73sVU1zExIMc+rec6YO5CTOZ+B0Cf5E+MKWU=;\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=GOGZ7N6KCI8yOQ/RmsUdvKhkSeXQQBpjl31vfyAMWGA509zL2Ydm14uEDnQOdS17i\n\tMa0RNrByTtzdHevt7nyeaO2CZbEombL2bbk/x4hXlqFcBdSSu9V9GlBwaELmKnBuAT\n\togK6MEEh8BdqifOfX3VB0Px2U+rTJ7mXVGCcJiWEaJJQUE0UkUQJReeisdURbVaEQ3\n\t+bn8lfE8hpmQi9T/j4mDZEER3WzykxZ53Mj5Vh4qWK4eajgm2pdqUwBBGnvx1KMaB7\n\tUxm7sQIzxAxT5rsmIGbYTderdO/SQTVf02QC2/AeNE9/f6Dx3sV1AsZroDQt93M4XJ\n\tyF2p8aWqaQo4A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659948139;\n\tbh=Vxi8nFc73sVU1zExIMc+rec6YO5CTOZ+B0Cf5E+MKWU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=b+6WyrAieOnPfRAU/r21CKceyy6pVMBceX6G/Nge1nqsOSZWeb9r32ryQ+BgZzWFo\n\tnfQn0ZB4FdqlkfvoFl1ttKFbw85xONmCEt31S2XXjW2nlluSuA4/t3t83v+DFfnDQX\n\tlzLyTsDgX0xsfhzKolbPGm8WQ91xGj8Zjaz4jneM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b+6WyrAi\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220806190433.59128-3-utkarsh02t@gmail.com>","References":"<20220806190433.59128-1-utkarsh02t@gmail.com>\n\t<20220806190433.59128-3-utkarsh02t@gmail.com>","To":"Utkarsh Tiwari ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 08 Aug 2022 09:42:16 +0100","Message-ID":"<165994813694.15821.13542708845061880792@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","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":24435,"web_url":"https://patchwork.libcamera.org/comment/24435/","msgid":"","date":"2022-08-08T17:24:03","subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","submitter":{"id":114,"url":"https://patchwork.libcamera.org/api/people/114/","name":"Utkarsh Tiwari","email":"utkarsh02t@gmail.com"},"content":"Hi Kieran thanks for the review.\n\n\nOn Mon, Aug 8, 2022 at 2:12 PM Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Utkarsh Tiwari via libcamera-devel (2022-08-06 20:04:31)\n> > Currently if there is HotPlug event when the user is on the Camera\n> > selection dialog, the QComboBox didn't update to reflect the change.\n> >\n> > If the QDialog exists then alert it for the Hotplug event. The check\n> > for QDialog existance is done by QPointer.\n> >\n> > Signed-off-by: Utkarsh Tiwari \n> > ---\n> > src/qcam/cam_select_dialog.h | 14 ++++++++++++++\n> > src/qcam/main_window.cpp | 6 ++++++\n> > 2 files changed, 20 insertions(+)\n> >\n> > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h\n> > index c23bad59..ee65eb88 100644\n> > --- a/src/qcam/cam_select_dialog.h\n> > +++ b/src/qcam/cam_select_dialog.h\n> > @@ -56,6 +56,20 @@ public:\n> > return cameraIdComboBox_->currentText().toStdString();\n> > }\n> >\n> > + /* Hotplug / Unplug Support. */\n> > + void cameraAdded(libcamera::Camera *camera)\n> > + {\n> > +\n> cameraIdComboBox_->addItem(QString::fromStdString(camera->id()));\n> > + }\n> > +\n> > + void cameraRemoved(libcamera::Camera *camera)\n> > + {\n> > + int cameraIndex = cameraIdComboBox_->findText(\n> > + QString::fromStdString(camera->id()));\n> > +\n>\n> Will this always be guaranteed to find an item? Are there any races\n> where we might try to remove a camera that hasn't been added?\n>\n> I expect not, but just curious to be sure what error case might occur.\n>\n> But it's fine:\n>\n> https://doc.qt.io/qt-6/qcombobox.html#removeItem:\n> void QComboBox::removeItem(int index)\n>\n> Removes the item at the given index from the combobox. This will\n> update the current index if the index is removed.\n>\n> This function does nothing if index is out of range.\n>\n>\n> So even if the camera isn't found - it just won't do anything.\n>\n>\n> > + cameraIdComboBox_->removeItem(cameraIndex);\n> > + }\n> > +\n> > private:\n> > libcamera::CameraManager *cm_;\n> >\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 758e2c94..dd30817d 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -594,6 +594,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> >\n> > if (event == HotplugEvent::HotPlug) {\n> >\n> cameraCombo_->addItem(QString::fromStdString(camera->id()));\n> > +\n> > + if (camSelectDialog_)\n> > + camSelectDialog_->cameraAdded(camera);\n>\n> This seems reasonable - but we're updating a camSelectDialog_ even if\n> it's not visible - and I think it gets destroyed and remade when opened?\n> (I'll have to go back and check) - but this is good for if we do re-use\n> the dialog anyway.\n>\nYes, the remaking on open is not supposed to be the way. When we\ncreate just one instance of the CamSelectDialog we need to add the\ncamera even if it's not visible, because as the dialog is not destroyed on\nclosing the dialog we need to keep track of the camera's hotplug events.\n\n>\n> Anyway, I think this is ok overall.\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> > } else if (event == HotplugEvent::HotUnplug) {\n> > /* Check if the currently-streaming camera is removed. */\n> > if (camera == camera_.get()) {\n> > @@ -605,6 +608,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> >\n> > int camIndex =\n> cameraCombo_->findText(QString::fromStdString(camera->id()));\n> > cameraCombo_->removeItem(camIndex);\n> > +\n> > + if (camSelectDialog_)\n> > + camSelectDialog_->cameraRemoved(camera);\n> > }\n> > }\n> >\n> > --\n> > 2.25.1\n> >\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 BA67EBE173\n\tfor ;\n\tMon, 8 Aug 2022 17:24:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1515F6332B;\n\tMon, 8 Aug 2022 19:24:19 +0200 (CEST)","from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com\n\t[IPv6:2607:f8b0:4864:20::52e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E481A63315\n\tfor ;\n\tMon, 8 Aug 2022 19:24:17 +0200 (CEST)","by mail-pg1-x52e.google.com with SMTP id 73so9143200pgb.9\n\tfor ;\n\tMon, 08 Aug 2022 10:24:17 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659979459;\n\tbh=5sP4d9phCkmfJGoH85JWCiA3Jysy4ai8pe7IZaD2OWk=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=A1Hi/vNTQYf7CbX4xGpDfHj5PpSLPdlfBflA6V88B/mOMcOh1omyCinYmqaWN/Ggr\n\twzpMuM50lWFKHLfpk5tpdcSld3wC0v2MAh/yvZHS698O7lMmtIAq4FtHho6eGmK0hB\n\tUzcEt4duoAQ4Wo/VmKu8p2zQNeuho94L5aDblZ+xJXBDuenDDDmy2bmwd1HnVAWU7p\n\tVC2goWndBRhiZ6uY180Cnfr0ZpMHo418Nhz266epDvE+M8sn8Ofd8C2oGGuqo0AYe0\n\tUhnhiZwnWkkTljA0QQIW3VyIitEYDf6IB9l4mCVLraoOES0IH4yvnSmONci4RiLMcs\n\tOiLi1vmvxTHXw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc;\n\tbh=mxH2fnw1i/S7vKF8LYVoE/hGJeFWRLWyjKXxdH+LrQU=;\n\tb=gTqz/WIVW7ddaLgD/gPJAnH1lDh03mLzzUmqJCeHOXvhvkAxJQPuh9R7HTfz2LjH4Q\n\tlV1bPbZLSF9v3H+ERhInAFycfZgI7KZ+LHNGIwC12CFh7NAvvGU0KKxXAG0loW2JD22I\n\tAdvpLXEvTJmKSJ6UmWtrSiZwFTQa2RLXcje/2d3SWhIMOV7cPhLHF5rEAc/GihkQDA05\n\tC+3ThLvAVAhehZfvNcwauwBDyGBg1DJUKAPDo1K5r7E42WO2fu7X1jb3tnoFhlxUyD75\n\tTZiCPNctIrJLlmcxPhajzvLa/PS18+7FS8w9EA9rJILbmPf7KSMrXo1shO4hSfLOLjiX\n\tpdCw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gTqz/WIV\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc;\n\tbh=mxH2fnw1i/S7vKF8LYVoE/hGJeFWRLWyjKXxdH+LrQU=;\n\tb=ARzWXDec4grVUiEXi8xZ9g6kebPIkOctghAhf2Iar3yrpjc4JpiS3uTNIFHtt1u17t\n\teFBy2bhR19c2F6F2+7Rlfy4H61DqTUgucGMbGW6cCG159n0cUC04yxSEMLGBLfus1uqe\n\t6LY/5UrFqT8djs6otqeNrxX2BmcKfZmZb8J+b/kDl/peXg1PzVT7lAmd/VsU3WuiAPkg\n\tpkBaApZDZPjEkq0OVf6btmwoV6MgzViNVxmCvGVwmYFX8TqxiZvALIUA7YNCabgINBii\n\tZvFb0eDa12jbqeuEqL0jrm6V9HPd8RGLqjJwrqgogiHzLtawsk/Fsuxndrr0J6QgnMME\n\tCg6g==","X-Gm-Message-State":"ACgBeo3/KqAx6+HHY9SjUmFkQO0EVjbkoyXxcGfXQwnSscSvUgdf8ZEr\n\tPEd6GXgQkT4SZyXwiR+noYa5PuCWvZmBXfpcWq1eFD9F","X-Google-Smtp-Source":"AA6agR4183F+NIXh6wzhIBXG52fegscRtlfNlEBIEBH2O0BYdslPcGfk42I4FpnaNQbdjmf7SdOr43ejQnOYTu0vf3E=","X-Received":"by 2002:a05:6a00:1581:b0:52f:332d:9c98 with SMTP id\n\tu1-20020a056a00158100b0052f332d9c98mr6839692pfk.64.1659979456019;\n\tMon, 08 Aug 2022 10:24:16 -0700 (PDT)","MIME-Version":"1.0","References":"<20220806190433.59128-1-utkarsh02t@gmail.com>\n\t<20220806190433.59128-3-utkarsh02t@gmail.com>\n\t<165994813694.15821.13542708845061880792@Monstersaurus>","In-Reply-To":"<165994813694.15821.13542708845061880792@Monstersaurus>","Date":"Mon, 8 Aug 2022 22:54:03 +0530","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"000000000000cf815b05e5be151e\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","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":"Utkarsh Tiwari via libcamera-devel\n\t","Reply-To":"Utkarsh Tiwari ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24441,"web_url":"https://patchwork.libcamera.org/comment/24441/","msgid":"","date":"2022-08-08T20:45:45","subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Aug 08, 2022 at 10:54:03PM +0530, Utkarsh Tiwari via libcamera-devel wrote:\n> On Mon, Aug 8, 2022 at 2:12 PM Kieran Bingham wrote:\n> > Quoting Utkarsh Tiwari via libcamera-devel (2022-08-06 20:04:31)\n> > > Currently if there is HotPlug event when the user is on the Camera\n> > > selection dialog, the QComboBox didn't update to reflect the change.\n> > >\n> > > If the QDialog exists then alert it for the Hotplug event. The check\n> > > for QDialog existance is done by QPointer.\n\nAh that why you use a QPointer. After reworking patch 1/4 to reuse the\ndialog, I think you can drop the QPointer, as the dialog should always\nbe there.\n\n> > > Signed-off-by: Utkarsh Tiwari \n> > > ---\n> > > src/qcam/cam_select_dialog.h | 14 ++++++++++++++\n> > > src/qcam/main_window.cpp | 6 ++++++\n> > > 2 files changed, 20 insertions(+)\n> > >\n> > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h\n> > > index c23bad59..ee65eb88 100644\n> > > --- a/src/qcam/cam_select_dialog.h\n> > > +++ b/src/qcam/cam_select_dialog.h\n> > > @@ -56,6 +56,20 @@ public:\n> > > return cameraIdComboBox_->currentText().toStdString();\n> > > }\n> > >\n> > > + /* Hotplug / Unplug Support. */\n> > > + void cameraAdded(libcamera::Camera *camera)\n> > > + {\n> > > + cameraIdComboBox_->addItem(QString::fromStdString(camera->id()));\n> > > + }\n> > > +\n> > > + void cameraRemoved(libcamera::Camera *camera)\n> > > + {\n> > > + int cameraIndex = cameraIdComboBox_->findText(\n> > > + QString::fromStdString(camera->id()));\n> > > +\n> >\n> > Will this always be guaranteed to find an item? Are there any races\n> > where we might try to remove a camera that hasn't been added?\n> >\n> > I expect not, but just curious to be sure what error case might occur.\n> >\n> > But it's fine:\n> >\n> > https://doc.qt.io/qt-6/qcombobox.html#removeItem:\n\nSide note, you may want to read the Qt5 documentation instead, as that's\nwhat we're currently using. There could be behavioural differences.\n\n> > void QComboBox::removeItem(int index)\n> >\n> > Removes the item at the given index from the combobox. This will\n> > update the current index if the index is removed.\n> >\n> > This function does nothing if index is out of range.\n> >\n> >\n> > So even if the camera isn't found - it just won't do anything.\n> >\n> > > + cameraIdComboBox_->removeItem(cameraIndex);\n> > > + }\n> > > +\n> > > private:\n> > > libcamera::CameraManager *cm_;\n> > >\n> > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > > index 758e2c94..dd30817d 100644\n> > > --- a/src/qcam/main_window.cpp\n> > > +++ b/src/qcam/main_window.cpp\n> > > @@ -594,6 +594,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> > >\n> > > if (event == HotplugEvent::HotPlug) {\n> > > cameraCombo_->addItem(QString::fromStdString(camera->id()));\n> > > +\n> > > + if (camSelectDialog_)\n> > > + camSelectDialog_->cameraAdded(camera);\n> >\n> > This seems reasonable - but we're updating a camSelectDialog_ even if\n> > it's not visible - and I think it gets destroyed and remade when opened?\n> > (I'll have to go back and check) - but this is good for if we do re-use\n> > the dialog anyway.\n> \n> Yes, the remaking on open is not supposed to be the way. When we\n> create just one instance of the CamSelectDialog we need to add the\n> camera even if it's not visible, because as the dialog is not destroyed on\n> closing the dialog we need to keep track of the camera's hotplug events.\n> \n> > Anyway, I think this is ok overall.\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > > } else if (event == HotplugEvent::HotUnplug) {\n> > > /* Check if the currently-streaming camera is removed. */\n> > > if (camera == camera_.get()) {\n> > > @@ -605,6 +608,9 @@ void MainWindow::processHotplug(HotplugEvent *e)\n> > >\n> > > int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id()));\n> > > cameraCombo_->removeItem(camIndex);\n> > > +\n> > > + if (camSelectDialog_)\n> > > + camSelectDialog_->cameraRemoved(camera);\n> > > }\n> > > }\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 B72C6BE173\n\tfor ;\n\tMon, 8 Aug 2022 20:45:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C84E63327;\n\tMon, 8 Aug 2022 22:45:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C0A263315\n\tfor ;\n\tMon, 8 Aug 2022 22:45:56 +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 D3558481;\n\tMon, 8 Aug 2022 22:45:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659991558;\n\tbh=qkQk6mW+aa3wSgqb8AHDhg4Xy/k6neNO2+S9FpyyXl0=;\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=d0ZZeqUFJNaFgSJUZBEykvmnByB9R9iThuprtbRcnBhCdPJAemTMayZTYvhF+xc4T\n\twmTpuVPjVQHAoSWVLIZ/us7iVI4kOPPfN0NffcrPH9eKKHqBVc1pLWrWkRGhBpgRzf\n\tYEXHbjL+gRc1ycVuhEgxsrj9cFOyvkXtswqqCdMopeaphQOFM6CUD3WBwAbmJjmE5o\n\tpKuDVHKcwOTOyTCQ6isAC/YsJky45yEf9nlUd2+IVZmT3/358ZJVGD4+UN2mz+O0W7\n\tiHyBGj7gsc4fojDoeRj9xARfD8PijZiE7qZdcvd7fkdwfIvWdwc1Aak2oOg9CgbU/R\n\tQe4BGiYxKjgHA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659991556;\n\tbh=qkQk6mW+aa3wSgqb8AHDhg4Xy/k6neNO2+S9FpyyXl0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vi0VH36oyC4uT2Gf4VrEz7gRODIc0TTBuUOpCZSF+EzVKAEz+mVVpGff36y+9+VLg\n\tXZU5QYiLiBnDtCrYsv/VmCKeN6QdKsQpcLf9LYqUyLPGT1e5B8TnQy+4mP2KCRHy2L\n\t4+5wrWbXJwS9x0CNM2cFCO2xdlmax4R7uslE8yG4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Vi0VH36o\"; dkim-atps=neutral","Date":"Mon, 8 Aug 2022 23:45:45 +0300","To":"Utkarsh Tiwari ","Message-ID":"","References":"<20220806190433.59128-1-utkarsh02t@gmail.com>\n\t<20220806190433.59128-3-utkarsh02t@gmail.com>\n\t<165994813694.15821.13542708845061880792@Monstersaurus>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] qcam: Support Hotplug for\n\tCamera Selection Dialog","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\" "}}]