[{"id":13321,"web_url":"https://patchwork.libcamera.org/comment/13321/","msgid":"<20201021102925.jlyve2qupyxjrvei@uno.localdomain>","date":"2020-10-21T10:29:25","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 21, 2020 at 03:24:16AM +0300, Laurent Pinchart wrote:\n> The rkisp1 and simple pipeline handlers can fail to register any camera,\n> if initialization of all the detected cameras fail. In that case, they\n> still return success from their match function. As no camera gets\n> registered, the pipeline handler is immediately destroyed, releasing the\n> acquired media devices, and the camera manager immediately tries to\n> match the same pipeline handler with the same media device, causing an\n> endless loop.\n>\n> Fix it by returning false from the match function if no camera gets\n> registered.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++---\n> src/libcamera/pipeline/simple/simple.cpp | 5 ++++-\n> 2 files changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 2352ab3b234a..c74a2e9bd548 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1107,10 +1107,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tif (!pad)\n> \t\treturn false;\n>\n> -\tfor (MediaLink *link : pad->links())\n> -\t\tcreateCamera(link->source()->entity());\n> +\tbool registered = false;\n> +\tfor (MediaLink *link : pad->links()) {\n> +\t\tif (!createCamera(link->source()->entity()))\n> +\t\t\tregistered = true;\n> +\t}\n>\n> -\treturn true;\n> +\treturn registered;\n> }\n>\n> /* -----------------------------------------------------------------------------\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8c00c0ffc121..8868a43beeb4 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -821,6 +821,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t}\n>\n> \t/* Initialize each pipeline and register a corresponding camera. */\n> +\tbool registered = false;\n> +\n\nIntentional empty line ?\n\n> \tfor (std::unique_ptr &data : pipelines) {\n> \t\tint ret = data->init();\n> \t\tif (ret < 0)\n> @@ -830,9 +832,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t\t\tCamera::create(this, data->sensor_->id(),\n> \t\t\t\t data->streams());\n> \t\tregisterCamera(std::move(camera), std::move(data));\n> +\t\tregistered = true;\n\nSo the actual camera matching happens in data->init() here ?\n\nAnyway, looks good\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \t}\n>\n> -\treturn true;\n> +\treturn registered;\n\n\n> }\n>\n> V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 D3823BDB13\n\tfor ;\n\tWed, 21 Oct 2020 10:29:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5719061D98;\n\tWed, 21 Oct 2020 12:29:29 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A86F60353\n\tfor ;\n\tWed, 21 Oct 2020 12:29:27 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id C7063E000A;\n\tWed, 21 Oct 2020 10:29:26 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 21 Oct 2020 12:29:25 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20201021102925.jlyve2qupyxjrvei@uno.localdomain>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13331,"web_url":"https://patchwork.libcamera.org/comment/13331/","msgid":"<20201021113704.GD2092600@oden.dyn.berto.se>","date":"2020-10-21T11:37:04","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nI assume this was intedned as a cover letter but ended up as a copy of \n'[PATCH] libcamera: pipeline: Fail match() when no camera is registered' \n:-)\n\nI can't see a diff between the two so if I missed something please let \nme know.\n\nOn 2020-10-21 03:24:16 +0300, Laurent Pinchart wrote:\n> The rkisp1 and simple pipeline handlers can fail to register any camera,\n> if initialization of all the detected cameras fail. In that case, they\n> still return success from their match function. As no camera gets\n> registered, the pipeline handler is immediately destroyed, releasing the\n> acquired media devices, and the camera manager immediately tries to\n> match the same pipeline handler with the same media device, causing an\n> endless loop.\n> \n> Fix it by returning false from the match function if no camera gets\n> registered.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++---\n> src/libcamera/pipeline/simple/simple.cpp | 5 ++++-\n> 2 files changed, 10 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 2352ab3b234a..c74a2e9bd548 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1107,10 +1107,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tif (!pad)\n> \t\treturn false;\n> \n> -\tfor (MediaLink *link : pad->links())\n> -\t\tcreateCamera(link->source()->entity());\n> +\tbool registered = false;\n> +\tfor (MediaLink *link : pad->links()) {\n> +\t\tif (!createCamera(link->source()->entity()))\n> +\t\t\tregistered = true;\n> +\t}\n> \n> -\treturn true;\n> +\treturn registered;\n> }\n> \n> /* -----------------------------------------------------------------------------\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8c00c0ffc121..8868a43beeb4 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -821,6 +821,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t}\n> \n> \t/* Initialize each pipeline and register a corresponding camera. */\n> +\tbool registered = false;\n> +\n> \tfor (std::unique_ptr &data : pipelines) {\n> \t\tint ret = data->init();\n> \t\tif (ret < 0)\n> @@ -830,9 +832,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t\t\tCamera::create(this, data->sensor_->id(),\n> \t\t\t\t data->streams());\n> \t\tregisterCamera(std::move(camera), std::move(data));\n> +\t\tregistered = true;\n> \t}\n> \n> -\treturn true;\n> +\treturn registered;\n> }\n> \n> V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 8CDEEBDB13\n\tfor ;\n\tWed, 21 Oct 2020 11:37:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19A1C60CDD;\n\tWed, 21 Oct 2020 13:37:08 +0200 (CEST)","from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B33560CDD\n\tfor ;\n\tWed, 21 Oct 2020 13:37:06 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id a9so2665933lfc.7\n\tfor ;\n\tWed, 21 Oct 2020 04:37:06 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tn19sm357527lfe.142.2020.10.21.04.37.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 21 Oct 2020 04:37:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"XMv/MK1X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=E14gdrxslqnV2Ao3k7zAt/9noq9vog14XpGDb3MJ+uE=;\n\tb=XMv/MK1XKerMsbu5T23fk0SETQxErQXH3RYBzrTLQZ1KUVhOPmzy/xqv4f/6u+J1+e\n\tjtvsEkVargq6y95rtCvWTONV1ot9AlpbUZp8y8YXwufesFBMbFMWdnmwwcjGzTM1Igw+\n\tpp3r2yihnJoXy4nppyPsjYfZXPjvdaXmjMxFywagIKJ8kn5ESnivTJIJ7LRMh0M/GONh\n\tp4drKUIogw38uKyoKF+fF0DhfNwyHyAc13ZSO4bUjq+SKY5yBWFhza1KqIuyyG9h5YhZ\n\toxZGfKQX9jpv1y/VIOxtXdaiw7oMUb6Y87MMvYxx6HcdwO/3XqC9dWneA5001T3AaoPX\n\trQhQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=E14gdrxslqnV2Ao3k7zAt/9noq9vog14XpGDb3MJ+uE=;\n\tb=YH5UH1iQSgLBJuNZHNbwLNESNPgLigekdaEaZ2FsBhbM2mQO+YcWyj4KTLNDhS9Mum\n\tKRU7cFXfECrZWVNC8OAtrsJFJaW9PC7XJgBi/Qo5V1i5mZIPtjJy2YI8FQpti4y5KGXm\n\tqx0WJDWoqkRJeNJy8yHc51PW5nvbNBPDuho4KxPJhcgZX1DYv8NB5/JZPWlfEVjud6G0\n\toIacG3iXVTM2ELS1h3V03ZLWuXiWK5ZeTpaOLZdxVQhaL2b0I9VMFOnJXg9BmlEPSzK3\n\tDF4GbPPG+lRgF2CMadkOjAumSmX22Nrl4KWNa7Ef/QtpA3NckgQQLXxoh72cnz20LSqE\n\thyrg==","X-Gm-Message-State":"AOAM532JEOdgZSold+Fh66g9BwPll7Ii/izri5EM4ShRFXUzN1hN0IIx\n\tSALAbVLkoiowoBcH0UITgmluOQ==","X-Google-Smtp-Source":"ABdhPJyeB2RgGgGWw+eyPOCewq/sNXVHcQXgUa/xj0/x0tXH6Y3+NsQHH/vUS2ah8w9S36MsxsNxeA==","X-Received":"by 2002:a19:7e53:: with SMTP id z80mr971308lfc.72.1603280225722; \n\tWed, 21 Oct 2020 04:37:05 -0700 (PDT)","Date":"Wed, 21 Oct 2020 13:37:04 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20201021113704.GD2092600@oden.dyn.berto.se>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13334,"web_url":"https://patchwork.libcamera.org/comment/13334/","msgid":"","date":"2020-10-21T12:39:52","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Laurent,\n\nThanks for looking into this!\n\nOn 21.10.2020 03:24, Laurent Pinchart wrote:\n> The rkisp1 and simple pipeline handlers can fail to register any camera,\n> if initialization of all the detected cameras fail. In that case, they\n> still return success from their match function. As no camera gets\n> registered, the pipeline handler is immediately destroyed, releasing the\n> acquired media devices, and the camera manager immediately tries to\n> match the same pipeline handler with the same media device, causing an\n> endless loop.\n> \n> Fix it by returning false from the match function if no camera gets\n> registered.\n\nI've faced this endless loop once when in the libcamera sources I commented out\nall the formats supported by the sensor - thus making data->init() to fail.\n\nYour patch does fix this issue - has tested it with simple pipeline handler.\n\nTested-by: Andrey Konovalov \n\nThanks,\nAndrey\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++---\n> src/libcamera/pipeline/simple/simple.cpp | 5 ++++-\n> 2 files changed, 10 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 2352ab3b234a..c74a2e9bd548 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1107,10 +1107,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tif (!pad)\n> \t\treturn false;\n> \n> -\tfor (MediaLink *link : pad->links())\n> -\t\tcreateCamera(link->source()->entity());\n> +\tbool registered = false;\n> +\tfor (MediaLink *link : pad->links()) {\n> +\t\tif (!createCamera(link->source()->entity()))\n> +\t\t\tregistered = true;\n> +\t}\n> \n> -\treturn true;\n> +\treturn registered;\n> }\n> \n> /* -----------------------------------------------------------------------------\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8c00c0ffc121..8868a43beeb4 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -821,6 +821,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t}\n> \n> \t/* Initialize each pipeline and register a corresponding camera. */\n> +\tbool registered = false;\n> +\n> \tfor (std::unique_ptr &data : pipelines) {\n> \t\tint ret = data->init();\n> \t\tif (ret < 0)\n> @@ -830,9 +832,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t\t\tCamera::create(this, data->sensor_->id(),\n> \t\t\t\t data->streams());\n> \t\tregisterCamera(std::move(camera), std::move(data));\n> +\t\tregistered = true;\n> \t}\n> \n> -\treturn true;\n> +\treturn registered;\n> }\n> \n> V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\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 287AFC3D3C\n\tfor ;\n\tWed, 21 Oct 2020 12:39:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A523861D7F;\n\tWed, 21 Oct 2020 14:39:57 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D531C60CDD\n\tfor ;\n\tWed, 21 Oct 2020 14:39:55 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id 184so2908725lfd.6\n\tfor ;\n\tWed, 21 Oct 2020 05:39:55 -0700 (PDT)","from [192.168.118.216] ([85.249.41.73])\n\tby smtp.gmail.com with ESMTPSA id\n\tz23sm400201ljg.89.2020.10.21.05.39.54\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 21 Oct 2020 05:39:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"n85XFr2t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=subject:to:references:from:cc:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=tnmHdaYoOFMtLaIRbC92VBKaHJWVxS7TMEG6bV0pl0M=;\n\tb=n85XFr2tsScwJ0NcnJgCRTpyXi8f+dExkona2FF4XqVTNYFEjWu+FqBoGFf214dFZW\n\tdac4/nZoAVyglEOzPk/vFHlgTSfuhGFqFjMjBAjob5donJrCri3LMDY5iu62h4847ZtC\n\tKF7V5CZe02PyCvOJ/S6wDcWlk4bEIFq44qIY23yQRpp+x0qoz2lq/X37nZg2TJHNsluo\n\tUwX5g20W0NmKLHrB+dm75zHuOMOU2+6KWKG/CZAtZsFqo4BZrgPSyTECasdHJvcl2rkl\n\tqPo2CUqT54fj30Lp5BBwT8clrQk6ukmeJFqS4ZJpMfXCokBMvJXaAuJSZXv6ixX6A3q8\n\tkLzg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:cc:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=tnmHdaYoOFMtLaIRbC92VBKaHJWVxS7TMEG6bV0pl0M=;\n\tb=PLoZdUMURAxUWS6oE7p/jYPiIrZG6TJi3oLf22kHu3yCOi9RKWsgRdmi0PCjSvonO+\n\t4DzqlvIZ0KCfK+9cDQoYqtQCJhN9gn6lwvrF4xjtK3KvgmhKUmXmjx9RzMP/Qoao0wQa\n\t+WT8ZwB3bgKJ3dOcsw5qJ3vm8rjlPXAIZRSGEZN/qrGzl3+PVrgjUu4pyVnQ+QpAZQ3u\n\tJ/yU9pjHHMMXXdTStKLPOtFY7IgMat5D5xosEHZfNpf+o6pupnU0VeI0seCKBnn8lvra\n\tCSRX7n9cbR8X9aprqa++pELJ+avqEY7zA4sqwH9TorCVuzHe0/KJvNAFobav8tQwlj2J\n\tiGJQ==","X-Gm-Message-State":"AOAM530TENw/+g8BcyetrLOTkHhRqcUr+qqCpGALwH6BDtbYCaqZHAgS\n\tWR3SdqJHlHXHIaC60Ynm3DWVQtVsUlJr5A==","X-Google-Smtp-Source":"ABdhPJw2agW3McUXcbwVo2j8AS/fDMABImVaa8SVtS6D5z6fm9JnF8ZEXHPIFG+Hs1jjkvX3OPOJrA==","X-Received":"by 2002:a19:7cf:: with SMTP id 198mr1078882lfh.67.1603283995181; \n\tWed, 21 Oct 2020 05:39:55 -0700 (PDT)","To":"Laurent Pinchart ","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","From":"Andrey Konovalov ","Message-ID":"","Date":"Wed, 21 Oct 2020 15:39:52 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13373,"web_url":"https://patchwork.libcamera.org/comment/13373/","msgid":"<20201021180751.GI3942@pendragon.ideasonboard.com>","date":"2020-10-21T18:07:51","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Oct 21, 2020 at 12:29:25PM +0200, Jacopo Mondi wrote:\n> On Wed, Oct 21, 2020 at 03:24:16AM +0300, Laurent Pinchart wrote:\n> > The rkisp1 and simple pipeline handlers can fail to register any camera,\n> > if initialization of all the detected cameras fail. In that case, they\n> > still return success from their match function. As no camera gets\n> > registered, the pipeline handler is immediately destroyed, releasing the\n> > acquired media devices, and the camera manager immediately tries to\n> > match the same pipeline handler with the same media device, causing an\n> > endless loop.\n> >\n> > Fix it by returning false from the match function if no camera gets\n> > registered.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++---\n> > src/libcamera/pipeline/simple/simple.cpp | 5 ++++-\n> > 2 files changed, 10 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 2352ab3b234a..c74a2e9bd548 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1107,10 +1107,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> > \tif (!pad)\n> > \t\treturn false;\n> >\n> > -\tfor (MediaLink *link : pad->links())\n> > -\t\tcreateCamera(link->source()->entity());\n> > +\tbool registered = false;\n> > +\tfor (MediaLink *link : pad->links()) {\n> > +\t\tif (!createCamera(link->source()->entity()))\n> > +\t\t\tregistered = true;\n> > +\t}\n> >\n> > -\treturn true;\n> > +\treturn registered;\n> > }\n> >\n> > /* -----------------------------------------------------------------------------\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 8c00c0ffc121..8868a43beeb4 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -821,6 +821,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > \t}\n> >\n> > \t/* Initialize each pipeline and register a corresponding camera. */\n> > +\tbool registered = false;\n> > +\n> \n> Intentional empty line ?\n\nYes, I like a bit of space sometimes :-) I think I tend to not add a\nblank line when the for statement itself operates on the variable that\nwas just declared.\n\n> > \tfor (std::unique_ptr &data : pipelines) {\n> > \t\tint ret = data->init();\n> > \t\tif (ret < 0)\n> > @@ -830,9 +832,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > \t\t\tCamera::create(this, data->sensor_->id(),\n> > \t\t\t\t data->streams());\n> > \t\tregisterCamera(std::move(camera), std::move(data));\n> > +\t\tregistered = true;\n> \n> So the actual camera matching happens in data->init() here ?\n\nNot so much the matching, but init() indeed analyzes the pipeline in\ndetails to make sure everything is right, so it can fail. It shouldn't,\nas we have an explicit match with the driver name table, but if\nsomething wrong happens, it's best to avoid infinite loops.\n\n> Anyway, looks good\n> Reviewed-by: Jacopo Mondi \n> \n> > \t}\n> >\n> > -\treturn true;\n> > +\treturn registered;\n> > }\n> >\n> > V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)","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 D5ADAC3D3C\n\tfor ;\n\tWed, 21 Oct 2020 18:08:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7177F603C1;\n\tWed, 21 Oct 2020 20:08:39 +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 8944C60361\n\tfor ;\n\tWed, 21 Oct 2020 20:08:37 +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 F0B1DBB5;\n\tWed, 21 Oct 2020 20:08:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZAUf4OY0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603303717;\n\tbh=IjHIJiYKLGh2Gg1qXSO/5UQrj1oBHvZZSK/Ko4pUG0k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZAUf4OY0vWjOCsCA6x/hQJax9ycSBEsMEm5FaAC13sUrnv5xrv+iyf0mKjeF5rEZB\n\tXzckw8y+nHrqVwtAlC4zGteHYbhN5d5zLEyYK/61xPm2yw0YNoS1QbxydUqpID8e8d\n\tDv76CiQ/9a1K4eOssNKSfdtWLOb5tNOqZ4lWaJQo=","Date":"Wed, 21 Oct 2020 21:07:51 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201021180751.GI3942@pendragon.ideasonboard.com>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>\n\t<20201021102925.jlyve2qupyxjrvei@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021102925.jlyve2qupyxjrvei@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Fail match()\n\twhen no camera is registered","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]