[{"id":13409,"web_url":"https://patchwork.libcamera.org/comment/13409/","msgid":"<2ec2422a-3cef-33bb-00f9-992ce5339524@ideasonboard.com>","date":"2020-10-22T09:53:52","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: pipeline: simple:\n\treturn from match() on error","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Tomi,\n\nOn 22/10/2020 09:17, Tomi Valkeinen wrote:\n> If converter_->open() fails, the code deletes the converter_ but then\n> happily goes on, and at the very next lines will use converter_.\n> \n> I assume the intention is to return from the function with false.\n> \n> Signed-off-by: Tomi Valkeinen \n\nI think in this instance, the failure to open convertor should simply\ndisable conversion on the pipeline (as indicated by the warning message\nprinted in that code block).\n\nReturning false here would completely disable the whole pipeline ...\n\nPerhaps this should only connect the convertor bufferReady signal in an\nelse statement?\n\n--\nKieran\n\n\n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0d48e1b6..df93bc8d 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -774,6 +774,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t\t\t\t<< \"Failed to open converter, disabling format conversion\";\n> \t\t\tdelete converter_;\n> \t\t\tconverter_ = nullptr;\n> +\t\t\treturn false;\n> \t\t}\n> \n> \t\tconverter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);\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 AAB58C3D3C\n\tfor ;\n\tThu, 22 Oct 2020 09:53:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F9BA61059;\n\tThu, 22 Oct 2020 11:53:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBBD361059\n\tfor ;\n\tThu, 22 Oct 2020 11:53:55 +0200 (CEST)","from [192.168.0.20]\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 412C853;\n\tThu, 22 Oct 2020 11:53:55 +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=\"H0CuFtkM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603360435;\n\tbh=RX2FbmwMXRbSeqckQoBLOl23MKew7Iy7yprxT4cPFlE=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=H0CuFtkMP39JpEn2kcvicRmlpEouJ2s1oYGGn1RmnJ2vpsqZAAsP9Qn1AjbeBOjgb\n\tPmGTBrQAUrXii9ze7Pkf4t+8zqarEEhZeAEU/ceFoKkL6B7+bn68y0YpiwltWUQo5W\n\tmyuWMREmZ/RP4Gp8CpSRSzMPxmsGPEG1MdfcKJS4=","To":"Tomi Valkeinen ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201022081731.36217-1-tomi.valkeinen@iki.fi>\n\t<20201022081731.36217-3-tomi.valkeinen@iki.fi>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<2ec2422a-3cef-33bb-00f9-992ce5339524@ideasonboard.com>","Date":"Thu, 22 Oct 2020 10:53:52 +0100","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":"<20201022081731.36217-3-tomi.valkeinen@iki.fi>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: pipeline: simple:\n\treturn from match() on error","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13419,"web_url":"https://patchwork.libcamera.org/comment/13419/","msgid":"<20201022144425.GD3938@pendragon.ideasonboard.com>","date":"2020-10-22T14:44:25","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: pipeline: simple:\n\treturn from match() on error","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Oct 22, 2020 at 10:53:52AM +0100, Kieran Bingham wrote:\n> On 22/10/2020 09:17, Tomi Valkeinen wrote:\n> > If converter_->open() fails, the code deletes the converter_ but then\n> > happily goes on, and at the very next lines will use converter_.\n> > \n> > I assume the intention is to return from the function with false.\n> > \n> > Signed-off-by: Tomi Valkeinen \n> \n> I think in this instance, the failure to open convertor should simply\n> disable conversion on the pipeline (as indicated by the warning message\n> printed in that code block).\n> \n> Returning false here would completely disable the whole pipeline ...\n\nCorrect, that was the intent.\n\n> Perhaps this should only connect the convertor bufferReady signal in an\n> else statement?\n\nIndeed, that's a bug :-) Tomi, I assume you'll submit a v2 as you've\nspotted this, but if you'd like me to fix the bug instead, I'm happy to\ndo so.\n\n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 1 +\n> > 1 file changed, 1 insertion(+)\n> > \n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 0d48e1b6..df93bc8d 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -774,6 +774,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > \t\t\t\t<< \"Failed to open converter, disabling format conversion\";\n> > \t\t\tdelete converter_;\n> > \t\t\tconverter_ = nullptr;\n> > +\t\t\treturn false;\n> > \t\t}\n> > \n> > \t\tconverter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);","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 5D567BDB13\n\tfor ;\n\tThu, 22 Oct 2020 14:45:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D227F613E6;\n\tThu, 22 Oct 2020 16:45: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 E2F8560352\n\tfor ;\n\tThu, 22 Oct 2020 16:45:11 +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 52E0E53;\n\tThu, 22 Oct 2020 16:45:11 +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=\"qpT7X0+r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603377911;\n\tbh=gnOkDVwmmlYBPn4ORzwjqg5cgP12TCukLdcyDDTjfVM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qpT7X0+roV+uYsAvt4Hb21k7vW58VYGF1vDKnubM50XfVkfRKm5l1pa2sEcFjGvkD\n\t6/RdUkOMkt87pml+/iDcsU4TAxIhVSelw3NtI6zZlE3BQxKvbbCEuKWzrseTuYA6yp\n\toqyvJoljntHsLmK+uO2paL7K4wMWmgV+7A5a8U/o=","Date":"Thu, 22 Oct 2020 17:44:25 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201022144425.GD3938@pendragon.ideasonboard.com>","References":"<20201022081731.36217-1-tomi.valkeinen@iki.fi>\n\t<20201022081731.36217-3-tomi.valkeinen@iki.fi>\n\t<2ec2422a-3cef-33bb-00f9-992ce5339524@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<2ec2422a-3cef-33bb-00f9-992ce5339524@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: pipeline: simple:\n\treturn from match() on error","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,\n\tTomi Valkeinen ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]