[{"id":13948,"web_url":"https://patchwork.libcamera.org/comment/13948/","msgid":"<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>","date":"2020-11-27T13:46:53","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello Naush,\n\nOn Fri, Nov 27, 2020 at 01:30:31PM +0000, Naushir Patuck wrote:\n> If the IPA fails during configuration, return an error flag to the\n> pipeline handler and fail the use case gracefully.\n>\n> At preset, the IPA configuration can fail for the following reasons:\n> - The sensor is not recognised, and fails to open a CamHelper object.\n> - The pipeline handler did not pass in controls for the ISP and sensor.\n>\n> Signed-off-by: Naushir Patuck \n\nLooks good to me one minor nit apart. See below.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/ipa/raspberrypi.h | 1 +\n> src/ipa/raspberrypi/raspberrypi.cpp | 16 ++++++++++++++--\n> .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++\n> 3 files changed, 20 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index 2b55dbfc..86c97ffa 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -21,6 +21,7 @@ enum ConfigParameters {\n> \tIPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> \tIPA_CONFIG_SENSOR = (1 << 2),\n> \tIPA_CONFIG_DROP_FRAMES = (1 << 3),\n> +\tIPA_CONFIG_FAILED = (1 << 4),\n> };\n>\n> enum Operations {\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 9853a343..7542dd26 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -197,8 +197,14 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\t const IPAOperationData &ipaConfig,\n> \t\t IPAOperationData *result)\n> {\n> -\tif (entityControls.empty())\n> +\tstd::string cameraName(sensorInfo.model);\n> +\n> +\tif (entityControls.size() != 2) {\n> +\t\tLOG(IPARPI, Error) << \"No ISP or sensor controls found for \"\n> +\t\t\t\t << cameraName;\n\nThis will print the sensor name while the error is global to the\ncamera, as the ISP controls might be missing as well.\n\n> +\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n> \t\treturn;\n> +\t}\n>\n> \tresult->operation = 0;\n>\n> @@ -213,10 +219,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t * that the kernel driver doesn't. We only do this the first time; we don't need\n> \t * to re-parse the metadata after a simple mode-switch for no reason.\n> \t */\n> -\tstd::string cameraName(sensorInfo.model);\n> \tif (!helper_) {\n> \t\thelper_ = std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n>\n> +\t\tif (!helper_) {\n> +\t\t\tLOG(IPARPI, Error) << \"Could not create camera helper for \"\n> +\t\t\t\t\t << cameraName;\n> +\t\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> \t\t/*\n> \t\t * Pass out the sensor config to the pipeline handler in order\n> \t\t * to setup the staggered writer class.\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 6fcdf557..76252806 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \tipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n> \t\t\t&result);\n>\n> +\tif (result.operation & RPi::IPA_CONFIG_FAILED) {\n> +\t\tLOG(RPI, Error) << \"IPA configuration failed!\";\n> +\t\treturn -EPIPE;\n> +\t}\n> +\n> \tunsigned int resultIdx = 0;\n> \tif (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {\n> \t\t/*\n> --\n> 2.25.1\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 C8478BE08A\n\tfor ;\n\tFri, 27 Nov 2020 13:46:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38EE56346F;\n\tFri, 27 Nov 2020 14:46:49 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E586632EE\n\tfor ;\n\tFri, 27 Nov 2020 14:46:48 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id F2F39C0007;\n\tFri, 27 Nov 2020 13:46:47 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Fri, 27 Nov 2020 14:46:53 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Message-ID":"<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>","References":"<20201127133031.1103686-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201127133031.1103686-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","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":13949,"web_url":"https://patchwork.libcamera.org/comment/13949/","msgid":"","date":"2020-11-27T13:51:56","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"HI Jacopo,\n\nThank you for the review.\n\nOn Fri, 27 Nov 2020 at 13:46, Jacopo Mondi wrote:\n\n> Hello Naush,\n>\n> On Fri, Nov 27, 2020 at 01:30:31PM +0000, Naushir Patuck wrote:\n> > If the IPA fails during configuration, return an error flag to the\n> > pipeline handler and fail the use case gracefully.\n> >\n> > At preset, the IPA configuration can fail for the following reasons:\n> > - The sensor is not recognised, and fails to open a CamHelper object.\n> > - The pipeline handler did not pass in controls for the ISP and sensor.\n> >\n> > Signed-off-by: Naushir Patuck \n>\n> Looks good to me one minor nit apart. See below.\n>\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n> > ---\n> > include/libcamera/ipa/raspberrypi.h | 1 +\n> > src/ipa/raspberrypi/raspberrypi.cpp | 16 ++++++++++++++--\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++\n> > 3 files changed, 20 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> > index 2b55dbfc..86c97ffa 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -21,6 +21,7 @@ enum ConfigParameters {\n> > IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> > IPA_CONFIG_SENSOR = (1 << 2),\n> > IPA_CONFIG_DROP_FRAMES = (1 << 3),\n> > + IPA_CONFIG_FAILED = (1 << 4),\n> > };\n> >\n> > enum Operations {\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 9853a343..7542dd26 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -197,8 +197,14 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> > const IPAOperationData &ipaConfig,\n> > IPAOperationData *result)\n> > {\n> > - if (entityControls.empty())\n> > + std::string cameraName(sensorInfo.model);\n> > +\n> > + if (entityControls.size() != 2) {\n> > + LOG(IPARPI, Error) << \"No ISP or sensor controls found for\n> \"\n> > + << cameraName;\n>\n> This will print the sensor name while the error is global to the\n> camera, as the ISP controls might be missing as well.\n>\n\nYou are right, it is a bit pointless here. I will remove and submit an\nupdate after waiting a bit for any more feedback.\n\nRegards,\nNaush\n\n\n\n>\n> > + result->operation = RPi::IPA_CONFIG_FAILED;\n> > return;\n> > + }\n> >\n> > result->operation = 0;\n> >\n> > @@ -213,10 +219,16 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> > * that the kernel driver doesn't. We only do this the first time;\n> we don't need\n> > * to re-parse the metadata after a simple mode-switch for no\n> reason.\n> > */\n> > - std::string cameraName(sensorInfo.model);\n> > if (!helper_) {\n> > helper_ =\n> std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n> >\n> > + if (!helper_) {\n> > + LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> > + << cameraName;\n> > + result->operation = RPi::IPA_CONFIG_FAILED;\n> > + return;\n> > + }\n> > +\n> > /*\n> > * Pass out the sensor config to the pipeline handler in\n> order\n> > * to setup the staggered writer class.\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 6fcdf557..76252806 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> > &result);\n> >\n> > + if (result.operation & RPi::IPA_CONFIG_FAILED) {\n> > + LOG(RPI, Error) << \"IPA configuration failed!\";\n> > + return -EPIPE;\n> > + }\n> > +\n> > unsigned int resultIdx = 0;\n> > if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {\n> > /*\n> > --\n> > 2.25.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\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 95824BE176\n\tfor ;\n\tFri, 27 Nov 2020 13:52:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18C2E6346F;\n\tFri, 27 Nov 2020 14:52:15 +0100 (CET)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB407632EE\n\tfor ;\n\tFri, 27 Nov 2020 14:52:12 +0100 (CET)","by mail-lf1-x12e.google.com with SMTP id l11so7175703lfg.0\n\tfor ;\n\tFri, 27 Nov 2020 05:52:12 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"AkI7Wenc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=KR1eKLMDzMKnZwD90ekDOEMvpF3QmLCvIO5WDgPRDbI=;\n\tb=AkI7WencxJrH+F5WboN5399tGo3GAQx26CWCk9AXdCNCYnEpMSE+cKKMBQlptI1RUM\n\tZNX9nvOQgiBfgucsP1UKDBzrf1+tSzqbm1jt0FUFG/V6poenz4VDdM/YX8ZAwF0rCxLs\n\tbwBghD5PDs9C75rCidbHMbmkrKkEqzP4mJVGSIvp1NlHwIdNLTyXQWvpOJ+AAlS/acWl\n\t2MZNQ/fnDHmbsy9Xr+zaa3eM5QCxuweMenX+kOelrqRNv47awceCiEHGM4/EckNXmwik\n\teNrOfbCBN7OfS2z0CGoKZk9KkbjJAAB1vcU309s8DKTOIFkJzCMLNpCTIY0dagoMGanS\n\tyG9Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=KR1eKLMDzMKnZwD90ekDOEMvpF3QmLCvIO5WDgPRDbI=;\n\tb=Wr1Hl41cI0J3yhIsvaDoOp5/33IR0m4CyDH4dKb2RKu4BgvNfigyj7n/eYCQpe8WeI\n\trRYuN+NBcw2iMcanLmqBRWJqqPl/sPXZVaqx3QeRQ1TEpKE/2oOFi3hnjKqeXG88g7I6\n\tI08xQN1sytmlEAnUodO3G2fSEMX8C4jB8BohBdKhAu99S2HFmw0uCkSJg0r3t5+PwIQG\n\tnhqR6keX+D7Nx9FCeNH7ObpTFYfUMa0HI4Vqk8bPxshf3y3PgbDRreGaO+/dJPM04Atn\n\t/ZM+JLg9qHI3QdiAF4M/YDzSz0caRx0o/AuivDRg6pf9Z8ZRC1CulkXJBEW0ZPwl+0D+\n\tNCeA==","X-Gm-Message-State":"AOAM532IuGRfs3FQzMX0XEDkxzibFxXX0Ei0KASzWK+Ckj8e65Swm2tE\n\t1ZRVgMeo2df1sW96EE5i3809qNL5ABptEWb60hqhW0v2hvI=","X-Google-Smtp-Source":"ABdhPJyGOBK1nSnPKUUxNM8D4o6Jeuh2Zum+RwAfQNjP+NvklWrmS6f4sIC1h/qUpwLjKSerAz478Sy8ReASZgd35f0=","X-Received":"by 2002:a19:b46:: with SMTP id 67mr3170959lfl.488.1606485132354; \n\tFri, 27 Nov 2020 05:52:12 -0800 (PST)","MIME-Version":"1.0","References":"<20201127133031.1103686-1-naush@raspberrypi.com>\n\t<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>","In-Reply-To":"<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>","From":"Naushir Patuck ","Date":"Fri, 27 Nov 2020 13:51:56 +0000","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============1442914035924117410==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13950,"web_url":"https://patchwork.libcamera.org/comment/13950/","msgid":"","date":"2020-11-27T14:21:12","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi Naush\n\nOn Fri, 27 Nov 2020 at 13:52, Naushir Patuck wrote:\n>\n> HI Jacopo,\n>\n> Thank you for the review.\n>\n> On Fri, 27 Nov 2020 at 13:46, Jacopo Mondi wrote:\n>>\n>> Hello Naush,\n>>\n>> On Fri, Nov 27, 2020 at 01:30:31PM +0000, Naushir Patuck wrote:\n>> > If the IPA fails during configuration, return an error flag to the\n>> > pipeline handler and fail the use case gracefully.\n>> >\n>> > At preset, the IPA configuration can fail for the following reasons:\n\ns/preset/present\n\n>> > - The sensor is not recognised, and fails to open a CamHelper object.\n>> > - The pipeline handler did not pass in controls for the ISP and sensor.\n>> >\n>> > Signed-off-by: Naushir Patuck \n>>\n>> Looks good to me one minor nit apart. See below.\n>>\n>> Reviewed-by: Jacopo Mondi \n>>\n>> Thanks\n>> j\n>>\n>> > ---\n>> > include/libcamera/ipa/raspberrypi.h | 1 +\n>> > src/ipa/raspberrypi/raspberrypi.cpp | 16 ++++++++++++++--\n>> > .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++\n>> > 3 files changed, 20 insertions(+), 2 deletions(-)\n>> >\n>> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n>> > index 2b55dbfc..86c97ffa 100644\n>> > --- a/include/libcamera/ipa/raspberrypi.h\n>> > +++ b/include/libcamera/ipa/raspberrypi.h\n>> > @@ -21,6 +21,7 @@ enum ConfigParameters {\n>> > IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n>> > IPA_CONFIG_SENSOR = (1 << 2),\n>> > IPA_CONFIG_DROP_FRAMES = (1 << 3),\n>> > + IPA_CONFIG_FAILED = (1 << 4),\n>> > };\n>> >\n>> > enum Operations {\n>> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > index 9853a343..7542dd26 100644\n>> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > @@ -197,8 +197,14 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>> > const IPAOperationData &ipaConfig,\n>> > IPAOperationData *result)\n>> > {\n>> > - if (entityControls.empty())\n>> > + std::string cameraName(sensorInfo.model);\n>> > +\n>> > + if (entityControls.size() != 2) {\n>> > + LOG(IPARPI, Error) << \"No ISP or sensor controls found for \"\n>> > + << cameraName;\n>>\n>> This will print the sensor name while the error is global to the\n>> camera, as the ISP controls might be missing as well.\n>\n>\n> You are right, it is a bit pointless here. I will remove and submit an update after waiting a bit for any more feedback.\n>\n> Regards,\n> Naush\n>\n>\n>>\n>>\n>> > + result->operation = RPi::IPA_CONFIG_FAILED;\n>> > return;\n>> > + }\n>> >\n>> > result->operation = 0;\n>> >\n>> > @@ -213,10 +219,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>> > * that the kernel driver doesn't. We only do this the first time; we don't need\n>> > * to re-parse the metadata after a simple mode-switch for no reason.\n>> > */\n>> > - std::string cameraName(sensorInfo.model);\n>> > if (!helper_) {\n>> > helper_ = std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n>> >\n>> > + if (!helper_) {\n>> > + LOG(IPARPI, Error) << \"Could not create camera helper for \"\n>> > + << cameraName;\n>> > + result->operation = RPi::IPA_CONFIG_FAILED;\n>> > + return;\n>> > + }\n>> > +\n>> > /*\n>> > * Pass out the sensor config to the pipeline handler in order\n>> > * to setup the staggered writer class.\n>> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > index 6fcdf557..76252806 100644\n>> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>> > ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n>> > &result);\n>> >\n>> > + if (result.operation & RPi::IPA_CONFIG_FAILED) {\n>> > + LOG(RPI, Error) << \"IPA configuration failed!\";\n>> > + return -EPIPE;\n>> > + }\n>> > +\n>> > unsigned int resultIdx = 0;\n>> > if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {\n>> > /*\n>> > --\n>> > 2.25.1\n>> >\n>> > _______________________________________________\n>> > libcamera-devel mailing list\n>> > libcamera-devel@lists.libcamera.org\n>> > https://lists.libcamera.org/listinfo/libcamera-devel\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 72C74BE176\n\tfor ;\n\tFri, 27 Nov 2020 14:21:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05F416346E;\n\tFri, 27 Nov 2020 15:21:29 +0100 (CET)","from mail-wm1-x331.google.com (mail-wm1-x331.google.com\n\t[IPv6:2a00:1450:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA159630BB\n\tfor ;\n\tFri, 27 Nov 2020 15:21:27 +0100 (CET)","by mail-wm1-x331.google.com with SMTP id x22so5253414wmc.5\n\tfor ;\n\tFri, 27 Nov 2020 06:21:27 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"EQ+wIwrA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=nKcJTOkrRAnpWB4OmXf6s+IcImZphyRHdtYyT0q9TWY=;\n\tb=EQ+wIwrAmnycsz2zBofR4IOzpv/LyfezZtET0ILd0W1PT9/lCOB/gkat5PBQSlFlKj\n\tdIxZ/YRR4hKB6ZhqQX5cNUFLmxN7P16PAz3D65/mCpXHZGvKOLHoW/+lUiPehnLf7pdB\n\tPUP0+S2LJo8NjCYmErNP99d/Ri+bGqccT50sinj7P6o1zatOe/6RJzClV17uNrruHsjy\n\tR7kBFd+Sw+n+ma9VBeCkVXDz87dwFC3VppIuFhBc8SYaQFQROpHAZkj20nCg4PObuXZI\n\tyjiA8FTOzaP8j8KJ/zyMSyf33UjIJ2sfY6c4unhft8IdQOctJEdEKSK87LoWxg4avPF5\n\tha8A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=nKcJTOkrRAnpWB4OmXf6s+IcImZphyRHdtYyT0q9TWY=;\n\tb=k9XOuPDa86TtSR40Sx5j5RDcTP8QIWZcedmBm2AINrSwF8ZKCVCEocoHeKsqMI0Zzp\n\twDymRRPHXkp0v2+Pcf2JBBSXJ3vTUlaH0LcGLS39QsNx/0JLb6Z6hlKU8/O0RrwCOpP/\n\tyny6WCcmp/M5P/HhK7w67ue1eX4hm/wB5II0iQ1CWWwPl3jY66UHcDElQsfJduz5tGIB\n\t/xz+ithzvVgnjfTbxknTsRKtIfta8B/uVu3ukqY4gPU2dovLfp0EClCUNndmafOfHtOP\n\tXAgJxrF+XYGmRH8O4dMRvkdPPRak/NKQq+NKB+Vw3OkTgPJqNdTylXeNsZDlU/CzPDaG\n\tOwBg==","X-Gm-Message-State":"AOAM532DaCBzRvH54Gitk2GEa3f+F9ns1i4uEYXLae0TZzd5qRcbp/cj\n\tpXmXDXnhuzEyB23xlmL7KtKu58svkyVLFEzy/DBaS5zHIx4=","X-Google-Smtp-Source":"ABdhPJyWu9AaiF81nyxkqPZ82KFMnKOrFnlPjOhp+zN1EHzDffTv/vbCQJ9eRWLquFwlBVcgEx9PWeamJ+TK56Ejiws=","X-Received":"by 2002:a1c:7d88:: with SMTP id\n\ty130mr9321314wmc.158.1606486887439; \n\tFri, 27 Nov 2020 06:21:27 -0800 (PST)","MIME-Version":"1.0","References":"<20201127133031.1103686-1-naush@raspberrypi.com>\n\t<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>\n\t","In-Reply-To":"","From":"Dave Stevenson ","Date":"Fri, 27 Nov 2020 14:21:12 +0000","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13953,"web_url":"https://patchwork.libcamera.org/comment/13953/","msgid":"","date":"2020-11-27T14:54:21","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Dave,\n\nThanks for the review.\n\nOn Fri, 27 Nov 2020 at 14:21, Dave Stevenson \nwrote:\n\n> Hi Naush\n>\n> On Fri, 27 Nov 2020 at 13:52, Naushir Patuck \n> wrote:\n> >\n> > HI Jacopo,\n> >\n> > Thank you for the review.\n> >\n> > On Fri, 27 Nov 2020 at 13:46, Jacopo Mondi wrote:\n> >>\n> >> Hello Naush,\n> >>\n> >> On Fri, Nov 27, 2020 at 01:30:31PM +0000, Naushir Patuck wrote:\n> >> > If the IPA fails during configuration, return an error flag to the\n> >> > pipeline handler and fail the use case gracefully.\n> >> >\n> >> > At preset, the IPA configuration can fail for the following reasons:\n>\n> s/preset/present\n>\n\nAck. Will post an update shortly.\n\nRegards,\nNaush\n\n\n>\n> >> > - The sensor is not recognised, and fails to open a CamHelper object.\n> >> > - The pipeline handler did not pass in controls for the ISP and\n> sensor.\n> >> >\n> >> > Signed-off-by: Naushir Patuck \n> >>\n> >> Looks good to me one minor nit apart. See below.\n> >>\n> >> Reviewed-by: Jacopo Mondi \n> >>\n> >> Thanks\n> >> j\n> >>\n> >> > ---\n> >> > include/libcamera/ipa/raspberrypi.h | 1 +\n> >> > src/ipa/raspberrypi/raspberrypi.cpp | 16\n> ++++++++++++++--\n> >> > .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++\n> >> > 3 files changed, 20 insertions(+), 2 deletions(-)\n> >> >\n> >> > diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> >> > index 2b55dbfc..86c97ffa 100644\n> >> > --- a/include/libcamera/ipa/raspberrypi.h\n> >> > +++ b/include/libcamera/ipa/raspberrypi.h\n> >> > @@ -21,6 +21,7 @@ enum ConfigParameters {\n> >> > IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> >> > IPA_CONFIG_SENSOR = (1 << 2),\n> >> > IPA_CONFIG_DROP_FRAMES = (1 << 3),\n> >> > + IPA_CONFIG_FAILED = (1 << 4),\n> >> > };\n> >> >\n> >> > enum Operations {\n> >> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > index 9853a343..7542dd26 100644\n> >> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > @@ -197,8 +197,14 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >> > const IPAOperationData &ipaConfig,\n> >> > IPAOperationData *result)\n> >> > {\n> >> > - if (entityControls.empty())\n> >> > + std::string cameraName(sensorInfo.model);\n> >> > +\n> >> > + if (entityControls.size() != 2) {\n> >> > + LOG(IPARPI, Error) << \"No ISP or sensor controls found\n> for \"\n> >> > + << cameraName;\n> >>\n> >> This will print the sensor name while the error is global to the\n> >> camera, as the ISP controls might be missing as well.\n> >\n> >\n> > You are right, it is a bit pointless here. I will remove and submit an\n> update after waiting a bit for any more feedback.\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> >>\n> >>\n> >> > + result->operation = RPi::IPA_CONFIG_FAILED;\n> >> > return;\n> >> > + }\n> >> >\n> >> > result->operation = 0;\n> >> >\n> >> > @@ -213,10 +219,16 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >> > * that the kernel driver doesn't. We only do this the first\n> time; we don't need\n> >> > * to re-parse the metadata after a simple mode-switch for no\n> reason.\n> >> > */\n> >> > - std::string cameraName(sensorInfo.model);\n> >> > if (!helper_) {\n> >> > helper_ =\n> std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n> >> >\n> >> > + if (!helper_) {\n> >> > + LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> >> > + << cameraName;\n> >> > + result->operation = RPi::IPA_CONFIG_FAILED;\n> >> > + return;\n> >> > + }\n> >> > +\n> >> > /*\n> >> > * Pass out the sensor config to the pipeline handler\n> in order\n> >> > * to setup the staggered writer class.\n> >> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > index 6fcdf557..76252806 100644\n> >> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> >> > ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> >> > &result);\n> >> >\n> >> > + if (result.operation & RPi::IPA_CONFIG_FAILED) {\n> >> > + LOG(RPI, Error) << \"IPA configuration failed!\";\n> >> > + return -EPIPE;\n> >> > + }\n> >> > +\n> >> > unsigned int resultIdx = 0;\n> >> > if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {\n> >> > /*\n> >> > --\n> >> > 2.25.1\n> >> >\n> >> > _______________________________________________\n> >> > libcamera-devel mailing list\n> >> > libcamera-devel@lists.libcamera.org\n> >> > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\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 2FDA9BE177\n\tfor ;\n\tFri, 27 Nov 2020 14:54:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B546A630BB;\n\tFri, 27 Nov 2020 15:54:39 +0100 (CET)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49905630BB\n\tfor ;\n\tFri, 27 Nov 2020 15:54:38 +0100 (CET)","by mail-lj1-x22b.google.com with SMTP id b17so6183821ljf.12\n\tfor ;\n\tFri, 27 Nov 2020 06:54:38 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"U/luiJlm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=dArHfiLj3IN5DInK3uyQ/QCRV76ratrgJat4SiS8gOI=;\n\tb=U/luiJlmcC66oUgw42tbmteuDYjiP3UqhbDIYyvb6OrRGYqQnW7+DpO+p7n+dnVBCX\n\tT1pRsl5HNAn1JULrF6Mon/ZgZmcOmq6bT7ZUZqKI5DjW1GcUtm+ZPszHZlPPU1so11w5\n\tcAqaPkCFj/PNQVkEEslHXfYNWVeiSk9wAh8ZoGBSlv8PgucXROh5FmYscvaz1Nt9JALE\n\ttUvF3RPvec4fo7euLhUfsm6SLUfIUgd9j1Lfcl0Q8Nzj3UmAUDVMnsyjUGB/rT9mzyrk\n\tuovQJ4IseY6nto/SAohgnDUn/AcWz6VuvWqgsf113bD1GxvGiLX8Eh2xw+F9HuVziDPj\n\tPMvg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=dArHfiLj3IN5DInK3uyQ/QCRV76ratrgJat4SiS8gOI=;\n\tb=PdAGP+mrHwO8NG9H1902avELdYWBuv1PlTJ5xGU3naGql9yg1Exz/+UhNS+gSgNwPT\n\tcSh/EHWcs6W0Zh/42d3xDAmZ4ixeVId7YBwyDjcWdcwWaMfGsy1D+3fZGF3ED7KsSk9X\n\t/k2TxMyPCZ6EBH6F2rYnRzc3b/8qb5Ffgdpx+W6hwmJaLO4jv3b8DRYdybcB27EMu9C/\n\tHI+M22B1DskXJIPU/uXhDO8dJJbD440z/a7nLGgZxJGm3eAln3EA9V9X1GJAnOVCW2Al\n\tgcxs3plC4TgVzhfLWWssebK+aI4b1MJORwK21RQS8PKDmvcaoHRXmUV/MGxhXjqU/k+v\n\tpBqg==","X-Gm-Message-State":"AOAM53164SARzeK+Lo/VKHLVPy2rgL/pcw9ZSS3tLFAgtiJJIXjke12j\n\t4gOuWqDjSrL/1Yv7FVJ4pjVcU2kONF+xsQ3DK34Dpg==","X-Google-Smtp-Source":"ABdhPJydEh5JIyI72yiYLvfaagDi8azR2otrT3IYu6jk7Fp65Cjydf/BYt0oNYIrEdFP9REAXjHdHtedmakVJXZR8uk=","X-Received":"by 2002:a2e:9197:: with SMTP id f23mr3599453ljg.83.1606488877626;\n\tFri, 27 Nov 2020 06:54:37 -0800 (PST)","MIME-Version":"1.0","References":"<20201127133031.1103686-1-naush@raspberrypi.com>\n\t<20201127134653.pkupwt4c2nhhnkcy@uno.localdomain>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 27 Nov 2020 14:54:21 +0000","Message-ID":"","To":"Dave Stevenson ","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Handle\n\tfailures during IPA configuration","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============7675688234798279401==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]