[{"id":15552,"web_url":"https://patchwork.libcamera.org/comment/15552/","msgid":"","date":"2021-03-09T07:28:36","subject":"Re: [libcamera-devel] [PATCH v2.1] ipa: raspberrypi: Use direct\n\treturn value for configure()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Paul,\n\nThank you for your work.\n\nOn Tue, 9 Mar 2021 at 02:39, Paul Elder wrote:\n\n> Now that we support returning int directly in addition to other output\n> parameters, improve the configure() function in the raspberrypi IPA\n> interface.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n>\n> ---\n> Changes in v2.1:\n> - remove init() changes, keep configure changes\n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 34 ++++++++-----------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 5 ++-\n> 3 files changed, 18 insertions(+), 23 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> index c3d614b8..eb427697 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -77,7 +77,7 @@ interface IPARPiInterface {\n> map streamConfig,\n> map entityControls,\n> ConfigInput ipaConfig)\n> - => (ConfigOutput results, int32 ret);\n> + => (int32 ret, ConfigOutput results);\n>\n> /**\n> * \\fn mapBuffers()\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 85a2b846..7904225a 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -84,11 +84,11 @@ public:\n> ipa::RPi::StartControls *result) override;\n> void stop() override {}\n>\n> - void configure(const CameraSensorInfo &sensorInfo,\n> - const std::map\n> &streamConfig,\n> - const std::map\n> &entityControls,\n> - const ipa::RPi::ConfigInput &data,\n> - ipa::RPi::ConfigOutput *response, int32_t *ret)\n> override;\n> + int configure(const CameraSensorInfo &sensorInfo,\n> + const std::map\n> &streamConfig,\n> + const std::map\n> &entityControls,\n> + const ipa::RPi::ConfigInput &data,\n> + ipa::RPi::ConfigOutput *response) override;\n>\n\nShould the return value here be an int32_t type to be consistent with the\nmojom definition?\nApart from that,\n\nReviewed-by: Naushir Patuck \n\n\n> void mapBuffers(const std::vector &buffers) override;\n> void unmapBuffers(const std::vector &ids) override;\n> void signalStatReady(const uint32_t bufferId) override;\n> @@ -290,16 +290,15 @@ void IPARPi::setMode(const CameraSensorInfo\n> &sensorInfo)\n> mode_.max_frame_length = sensorInfo.maxFrameLength;\n> }\n>\n> -void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> - [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> - const std::map\n> &entityControls,\n> - const ipa::RPi::ConfigInput &ipaConfig,\n> - ipa::RPi::ConfigOutput *result, int32_t *ret)\n> +int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> + [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> + const std::map\n> &entityControls,\n> + const ipa::RPi::ConfigInput &ipaConfig,\n> + ipa::RPi::ConfigOutput *result)\n> {\n> if (entityControls.size() != 2) {\n> LOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n> - *ret = -1;\n> - return;\n> + return -1;\n> }\n>\n> result->params = 0;\n> @@ -309,14 +308,12 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n>\n> if (!validateSensorControls()) {\n> LOG(IPARPI, Error) << \"Sensor control validation failed.\";\n> - *ret = -1;\n> - return;\n> + return -1;\n> }\n>\n> if (!validateIspControls()) {\n> LOG(IPARPI, Error) << \"ISP control validation failed.\";\n> - *ret = -1;\n> - return;\n> + return -1;\n> }\n>\n> /* Setup a metadata ControlList to output metadata. */\n> @@ -334,8 +331,7 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> if (!helper_) {\n> LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> << cameraName;\n> - *ret = -1;\n> - return;\n> + return -1;\n> }\n>\n> /*\n> @@ -403,7 +399,7 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n>\n> lastMode_ = mode_;\n>\n> - *ret = 0;\n> + return 0;\n> }\n>\n> void IPARPi::mapBuffers(const std::vector &buffers)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 6387fae5..0f01ce8f 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1250,9 +1250,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> /* Ready the IPA - it must know about the sensor resolution. */\n> ipa::RPi::ConfigOutput result;\n>\n> - ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> - &result, &ret);\n> -\n> + ret = ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> + &result);\n> if (ret < 0) {\n> LOG(RPI, Error) << \"IPA configuration failed!\";\n> return -EPIPE;\n> --\n> 2.27.0\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 A8497BD1F1\n\tfor ;\n\tTue, 9 Mar 2021 07:28:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1BA3868AA3;\n\tTue, 9 Mar 2021 08:28:54 +0100 (CET)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0472D602E4\n\tfor ;\n\tTue, 9 Mar 2021 08:28:53 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id d3so25359894lfg.10\n\tfor ;\n\tMon, 08 Mar 2021 23:28:52 -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=\"VKANhxsI\"; 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=Xil8RoLTwbnjZEFz/jA48eOqISvO0kzciyVqygTC/Fs=;\n\tb=VKANhxsI3tQgWaS3QkP1rtYjKF2juKralcPryk/emY6K5oT074NQlUdV3dyOIm7R10\n\tWlgE/Wd4714zeYtmTsVisYhJ9vOaWtaaBi7i/lHYGkfCT+OtZBBff8svoARYsX6yQlG/\n\tHle3atXrbZL/c9sl0pdiP5K5YKD9Fy0jl4jb8/QSdBDofRP+iRK9lnSZ8SGp1W9HeFJl\n\tX5Hitz+YEKiO4VHVp09U9GPlpZwK2SMnMmoNFmk8/GJqm8ic+MSX7C9lE/zG9xn4Lfs4\n\tMHkwF5lg1k0Txq4iJHxeY+Ob28utxPGM4VRToeTTOq+amxtgsffvXKAse750pggpwH4i\n\t9NHw==","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=Xil8RoLTwbnjZEFz/jA48eOqISvO0kzciyVqygTC/Fs=;\n\tb=fWMBZleqaVJsJ9gfTlpyN35gz/k3IKR4d4isNoUbDByuNTS19woVYEg/RJViTD3lmR\n\teK9/h6y4V8hQP7xOPVAS7Dp4nxlBVzNBjWeVEXLdZLZ3UTYgqpYTUYXGenvBOkCF3UMw\n\tV8/ATaYKzKk+m945+8KofWcYjU3mbrxII4owNU0JDkZ1Y1PlY64kYeYJ6ktpsW+g/atH\n\tx/fYr8fJqyW2KJsO2UfW4+myjD5SWV9nDYPKXER/BUSZ6s+JcJufTaUXsK65YZ2SZ6VG\n\tSiHoDwmDZbBQAthEfla/pOXOXWQZZO7Q27fLVpN+tPdcHZsIGHiXJUAsjdLLOxxpmVUj\n\thy8g==","X-Gm-Message-State":"AOAM5337YbPKBCvcwilT1euwegbLuycmiliN5Fm9/FZBJSJ8FCX65PU/\n\ttTFPdjbYvL5s1F6WCLK7CDwt95v3u0dnGJzEJLxbvFkJrBVj3Q==","X-Google-Smtp-Source":"ABdhPJzTLsFcdE2zqN2UxqKKZIaYTfjI2dWOpvqvt1tv4nqFJprdFP8PetPq3DpgX6MGEthSCHuR0AvqaLQStOrP4nQ=","X-Received":"by 2002:ac2:4205:: with SMTP id\n\ty5mr10670327lfh.375.1615274932320; \n\tMon, 08 Mar 2021 23:28:52 -0800 (PST)","MIME-Version":"1.0","References":"<20210308074828.13228-4-paul.elder@ideasonboard.com>\n\t<20210309023907.83798-1-paul.elder@ideasonboard.com>","In-Reply-To":"<20210309023907.83798-1-paul.elder@ideasonboard.com>","From":"Naushir Patuck ","Date":"Tue, 9 Mar 2021 07:28:36 +0000","Message-ID":"","To":"Paul Elder ","Subject":"Re: [libcamera-devel] [PATCH v2.1] ipa: raspberrypi: Use direct\n\treturn value for configure()","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=\"===============6043047601281814201==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]