[{"id":24018,"web_url":"https://patchwork.libcamera.org/comment/24018/","msgid":"<165840215829.1228219.13882800628950639294@Monstersaurus>","date":"2022-07-21T11:15:58","subject":"Re: [libcamera-devel] [RFC PATCH] qcam: Fix IPU3 camera switching","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2022-07-06 10:22:18)\n> On the IPU3 we will disallow acquiring a second camera while one is\n> active.  This prevents us from doing a camera switch between the\n> available cameras.\n> \n> Re-instate this facility by ensuring we release the current camera\n> before attempting to acquire the new camera.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n> \n> QCam is broken on IPU3 in that once one camera is running, we can't swap\n> to the other without closing qcam (or swapping to a non IPU3 camera).\n> \n> This seems a bit of a pain - but here's a potential fix.\n> \n> Since writing this patch (some time ago) I know think that trying to\n> reaquire the previous camera if the switch fails might be wrong.\n> \n> I wonder if we should just leave the camera halted, with our 'no-image'\n> logo displayed, and wait. But the only way to 'fix' this would be for\n> the user to choose a new camera, there's no current option to retry\n> acquiring a camera...\n\nDoes anyone have any thoughts or insight on this topic?\n\n\n>  src/qcam/main_window.cpp | 26 ++++++++++++++++++++------\n>  1 file changed, 20 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 7433d647e8a0..487f522748ca 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -269,11 +269,6 @@ void MainWindow::switchCamera(int index)\n>  \n>         const std::shared_ptr<Camera> &cam = cameras[index];\n>  \n> -       if (cam->acquire()) {\n> -               qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> -               return;\n> -       }\n> -\n>         qInfo() << \"Switching to camera\" << cam->id().c_str();\n>  \n>         /*\n> @@ -283,7 +278,26 @@ void MainWindow::switchCamera(int index)\n>         startStopAction_->setChecked(false);\n>  \n>         camera_->release();\n> -       camera_ = cam;\n> +\n> +       /*\n> +        * Only attempt to acquire after releasing to ensure we support\n> +        * pipelines which can only stream from one device at a time.\n> +        *\n> +        * Todo: Consider how we could acquire more than one camera\n> +        *       but ensure only one could stream at a time.\n> +        */\n> +       if (cam->acquire()) {\n> +               qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> +               qInfo() << \"Switching back to \" << camera_->id().c_str();\n> +\n> +               if (camera_->acquire()) {\n> +                       qInfo() << \"Failed to reacquire \" << camera_->id().c_str();\n> +                       return;\n> +               }\n> +       } else {\n> +               /* Acquire was successful, Switch to this camera */\n> +               camera_ = cam;\n> +       }\n>  \n>         startStopAction_->setChecked(true);\n>  }\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 46DD8BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 21 Jul 2022 11:16:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8830C6330F;\n\tThu, 21 Jul 2022 13:16:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CE95601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jul 2022 13:16:01 +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 8B364496\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jul 2022 13:16:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658402162;\n\tbh=gDgvJ7dgzVulH4oxh/ypFhZWIaFqIIYcAweMTUXSTQU=;\n\th=In-Reply-To:References:Cc:To:Date:Subject:List-Id:\n\tList-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\tFrom:Reply-To:From;\n\tb=WJW2PC4hh/rO2/MeqE7Vwr/lhiqm090GUQgEVVwJbpE+8LXxOPmCzxdXu/2SHNUpe\n\t4znZP1VuM/ndPjCzF3f8nLX0ERh2fj2jfBhUqTuBPYJTSZjM7DeTazkItZ9N2OczbW\n\tPFCCl3r1FxqfesZh1uEhYyYsz47cUR4s6+tbt7Oe4vQSvn/SDnfWsCwSFPNmpHMubY\n\tLi6ta/xlXzuTom5oCaByfw+FqYidcacv5cPIFfM9h0GrAy13tLxmPMQnEz/G5PhNR5\n\tiIf3RDlgS2hdikhmymCC4Besm1HSMjA6en33sZb+6RI0DNopWOJ52UzluaaIg2Z9iC\n\tLJfu7/MZK8AOA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658402160;\n\tbh=gDgvJ7dgzVulH4oxh/ypFhZWIaFqIIYcAweMTUXSTQU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OccoIuf8eXbTyeEZ2h+6S8Qki7cKsq/D3O9pk04jRxMA3y1LDcsUvctRHqt1wJxBA\n\tSYZHw/7pq/vysiJk1xCWQmrWLGH14vjR5VKq3PsrMgJZ1QyrZNK/C4yyk6/As2qIHy\n\ta9bq5JYPPb6cIQq8vN5rh22k4ccA5ep/TyaXrMRg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OccoIuf8\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220706092218.2761216-1-kieran.bingham@ideasonboard.com>","References":"<20220706092218.2761216-1-kieran.bingham@ideasonboard.com>","Cc":"","To":"libcamera devel <libcamera-devel@lists.libcamera.org>","Date":"Thu, 21 Jul 2022 12:15:58 +0100","Message-ID":"<165840215829.1228219.13882800628950639294@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH] qcam: Fix IPU3 camera switching","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24041,"web_url":"https://patchwork.libcamera.org/comment/24041/","msgid":"<Ytm/ftp3XeBZLXpc@pendragon.ideasonboard.com>","date":"2022-07-21T21:05:02","subject":"Re: [libcamera-devel] [RFC PATCH] qcam: Fix IPU3 camera switching","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Wed, Jul 06, 2022 at 10:22:18AM +0100, Kieran Bingham via libcamera-devel wrote:\n> On the IPU3 we will disallow acquiring a second camera while one is\n> active.  This prevents us from doing a camera switch between the\n> available cameras.\n> \n> Re-instate this facility by ensuring we release the current camera\n> before attempting to acquire the new camera.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n> \n> QCam is broken on IPU3 in that once one camera is running, we can't swap\n> to the other without closing qcam (or swapping to a non IPU3 camera).\n> \n> This seems a bit of a pain - but here's a potential fix.\n> \n> Since writing this patch (some time ago) I know think that trying to\n> reaquire the previous camera if the switch fails might be wrong.\n> \n> I wonder if we should just leave the camera halted, with our 'no-image'\n> logo displayed, and wait. But the only way to 'fix' this would be for\n> the user to choose a new camera, there's no current option to retry\n> acquiring a camera...\n\nThis looks like a problem on the pipeline handler side, not in qcam. It\nturns out that I have a patch in a branch that has been sitting idle\nthere for way too long, I've posted it ([1]), let's move the discussion\nthere (and it would be nice if you could test it).\n\n[1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-July/032507.html\n\n>  src/qcam/main_window.cpp | 26 ++++++++++++++++++++------\n>  1 file changed, 20 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 7433d647e8a0..487f522748ca 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -269,11 +269,6 @@ void MainWindow::switchCamera(int index)\n>  \n>  \tconst std::shared_ptr<Camera> &cam = cameras[index];\n>  \n> -\tif (cam->acquire()) {\n> -\t\tqInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> -\t\treturn;\n> -\t}\n> -\n>  \tqInfo() << \"Switching to camera\" << cam->id().c_str();\n>  \n>  \t/*\n> @@ -283,7 +278,26 @@ void MainWindow::switchCamera(int index)\n>  \tstartStopAction_->setChecked(false);\n>  \n>  \tcamera_->release();\n> -\tcamera_ = cam;\n> +\n> +\t/*\n> +\t * Only attempt to acquire after releasing to ensure we support\n> +\t * pipelines which can only stream from one device at a time.\n> +\t *\n> +\t * Todo: Consider how we could acquire more than one camera\n> +\t *       but ensure only one could stream at a time.\n> +\t */\n> +\tif (cam->acquire()) {\n> +\t\tqInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> +\t\tqInfo() << \"Switching back to \" << camera_->id().c_str();\n> +\n> +\t\tif (camera_->acquire()) {\n> +\t\t\tqInfo() << \"Failed to reacquire \" << camera_->id().c_str();\n> +\t\t\treturn;\n> +\t\t}\n> +\t} else {\n> +\t\t/* Acquire was successful, Switch to this camera */\n> +\t\tcamera_ = cam;\n> +\t}\n>  \n>  \tstartStopAction_->setChecked(true);\n>  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 F1D14BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 21 Jul 2022 21:05:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2C846330F;\n\tThu, 21 Jul 2022 23:05:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 51874601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jul 2022 23:05:04 +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 E2D7D496;\n\tThu, 21 Jul 2022 23:05:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658437505;\n\tbh=kPdEOcijQOxROnj0H/T1SL9RVtF6PXczYcTSoYj3YGI=;\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=nEzi63QkTELif7gFxvUZMG9PA6zIm+IIqcsjRlwF2dn8dG6vFAtJsAlzubUWJVU4x\n\tGOtZPpUVtbIJm2YRJmgcQUizZVRF0cA2meBP9U+fCyvELgPFZCztPmRIdjZ1TanQin\n\tgenNZDFglna1RjuDAlAVgPEB1fN5YpKAA+qqJk/wuohjnTxZjFa20863Y8ALTnvbVO\n\tLcZa2m7sJRdhP3CwHn+eCgq1E5677sPuqJZDEahxfBasDd874lxkBh/7PvZOG+4vYS\n\t1mhpv2QDZoPMg1sjM7DwigRWKH4iUmw6jqIQ46C6Z2A3y6wS8O6Ge30hfQHZIxberG\n\tKPZtp4szSPI3A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658437504;\n\tbh=kPdEOcijQOxROnj0H/T1SL9RVtF6PXczYcTSoYj3YGI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=g57AX9R3ynt7UyJ2faii0uaA5aa5FNVuGPlnhbj4lETKAwZZN9KK7+nQx+K/ZGW3z\n\tZejONmjOeeGi5gzBf+ZUDGaPG+ghxJicqXC+wQ4uVjnzmm3m0o9anOZrGfBkpjAiVd\n\tWFbJRYxXPmj/NuIzg79lq/pwmPs0kp/O33XpHwt0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"g57AX9R3\"; dkim-atps=neutral","Date":"Fri, 22 Jul 2022 00:05:02 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<Ytm/ftp3XeBZLXpc@pendragon.ideasonboard.com>","References":"<20220706092218.2761216-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220706092218.2761216-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH] qcam: Fix IPU3 camera switching","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]