[{"id":17514,"web_url":"https://patchwork.libcamera.org/comment/17514/","msgid":"","date":"2021-06-11T10:53:29","subject":"Re: [libcamera-devel] [PATCH v3 1/2] ipa: raspberrypi: Add\n\tsensitivity field to camera mode","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your patch.\n\nOn Fri, 11 Jun 2021 at 11:07, David Plowman \nwrote:\n\n> We use the CamHelper class to initialise it to the usual value of 1.\n>\n> The CamHelper's GetModeSensitivity method can be redefined to\n> implement a different behaviour for sensors that require it.\n>\n> Signed-off-by: David Plowman \n>\n\nThis is a bit awkward with the whole chicken-and-egg thing, but I cannot\nsee what else\nto do.\n\nReviewed-by: Naushir Patuck \n\n\n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 11 +++++++++++\n> src/ipa/raspberrypi/cam_helper.hpp | 3 +++\n> src/ipa/raspberrypi/controller/camera_mode.h | 2 ++\n> src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++++\n> 4 files changed, 22 insertions(+)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4..d0104da7 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -130,6 +130,17 @@ bool CamHelper::SensorEmbeddedDataPresent() const\n> return false;\n> }\n>\n> +double CamHelper::GetModeSensitivity([[maybe_unused]] const CameraMode\n> &mode) const\n> +{\n> + /*\n> + * Most sensors have the same sensitivity in every mode, but this\n> + * method can be overridden for those that do not. Note that it is\n> + * called before mode_ is set, so it must return the sensitivity\n> + * of the mode that is passed in.\n> + */\n> + return 1.0;\n> +}\n> +\n> unsigned int CamHelper::HideFramesStartup() const\n> {\n> /*\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> b/src/ipa/raspberrypi/cam_helper.hpp\n> index f53f5c39..f524fe84 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -40,6 +40,8 @@ namespace RPiController {\n> //\n> // A method to query if the sensor outputs embedded data that can be\n> parsed.\n> //\n> +// A method to return the sensitivity of a given camera mode.\n> +//\n> // A parser to parse the embedded data buffers provided by some sensors\n> (for\n> // example, the imx219 does; the ov5647 doesn't). This allows us to know\n> for\n> // sure the exposure and gain of the frame we're looking at. CamHelper\n> @@ -83,6 +85,7 @@ public:\n> virtual void GetDelays(int &exposure_delay, int &gain_delay,\n> int &vblank_delay) const;\n> virtual bool SensorEmbeddedDataPresent() const;\n> + virtual double GetModeSensitivity(const CameraMode &mode) const;\n> virtual unsigned int HideFramesStartup() const;\n> virtual unsigned int HideFramesModeSwitch() const;\n> virtual unsigned int MistrustFramesStartup() const;\n> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h\n> b/src/ipa/raspberrypi/controller/camera_mode.h\n> index 2aa2335d..72c68dc6 100644\n> --- a/src/ipa/raspberrypi/controller/camera_mode.h\n> +++ b/src/ipa/raspberrypi/controller/camera_mode.h\n> @@ -41,6 +41,8 @@ struct CameraMode {\n> libcamera::Transform transform;\n> // minimum and maximum fame lengths in units of lines\n> uint32_t min_frame_length, max_frame_length;\n> + // sensitivity of this mode\n> + double sensitivity;\n> };\n>\n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 1c1e802a..9ce0db08 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -324,6 +324,12 @@ void IPARPi::setMode(const IPACameraSensorInfo\n> &sensorInfo)\n> */\n> mode_.min_frame_length = sensorInfo.minFrameLength;\n> mode_.max_frame_length = sensorInfo.maxFrameLength;\n> +\n> + /*\n> + * Some sensors may have different sensitivities in different\n> modes;\n> + * the CamHelper will know the correct value.\n> + */\n> + mode_.sensitivity = helper_->GetModeSensitivity(mode_);\n> }\n>\n> int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n> --\n> 2.20.1\n>\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 3CCE0BD78E\n\tfor ;\n\tFri, 11 Jun 2021 10:53:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA4796892F;\n\tFri, 11 Jun 2021 12:53:49 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3AFEF6029E\n\tfor ;\n\tFri, 11 Jun 2021 12:53:47 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id e2so9054378ljk.4\n\tfor ;\n\tFri, 11 Jun 2021 03:53:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"sNLtMhDL\"; 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=KpBh3YuczrANbXaY+rTjtZx5xM4VidkyFfySW3I3cug=;\n\tb=sNLtMhDLmbfBoZP1rZZuf+DQqBlG3ykVwUIrOhjo88Ra4BYicd89zVU6Ahim5s2hNX\n\tHd4hNiVgvIEf70T1WqhFlCa6lxs4/jeeS7Tf7zzPszQ+dAdebnhF8X5wLRKrgUejrUsa\n\t40oK9p+xwRlucA31BUl5P5CtTjbJbNnLRNTGTkAc+73XHmyGYHWFZF/O5n82A98Woc2a\n\tAQHiuKrj6hEgPkkyhTHw683kZwaIg09VkJlRtvE8AKLwbuMwr5f8qYsBjnj8EfgnYNyu\n\tmDHgCrKU6+LzW6GnLOUIniBD3e4Yjy1QtgW0hl83xkggVdgXXhbOI8tNP6dlZr4xmI8b\n\txo4g==","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=KpBh3YuczrANbXaY+rTjtZx5xM4VidkyFfySW3I3cug=;\n\tb=ZafqIfKKLPG527F/vNvK/oTfhNerPOXBE8erujJiT/tdaGwMBDI/8P22WrK+S+CcMS\n\tM5j4FKmH/Io49B+H9ZEjNNCiWN9DK08saXf7ZEU1o9G8wKT6kcGxG7OsR4P8o3wHjwwr\n\toKY/+/O9uPxKKdgAN4Pp2c3d2+RgZMv/0ynzP3DDilj11FKRg9m8C880oqeavS0BwUba\n\tuOvEVIErormdyvBpwbG47p8J6uK55WxLfZp5J1cOqWUl/D+hfr58Ew9yHF2ds2CxqNax\n\tJSaT8Ed6QxT+w9+DJ4fRnJQ04OA+TVUiy0C3Qz44w5pcwFGpeATxPgCcp9egoWEhnwyM\n\towhw==","X-Gm-Message-State":"AOAM5311o4/kCHesVfC1/y6GvO9to/QVqdo2W5c9Na3qrQ6NFAJsIBCB\n\t8Tu7YoeBel+SpdBAFPSCziufQ1Lm9zf6RdDM8gGKIA==","X-Google-Smtp-Source":"ABdhPJyI3g8JBBDy9iZeG/pQAe2KABLrBSNp41nurvJ9q/aZ+WYAh7ljBpNGULacoxTv69lEEh6J04j9fNflfV/Ctvg=","X-Received":"by 2002:a2e:90ca:: with SMTP id\n\to10mr2479574ljg.299.1623408826473; \n\tFri, 11 Jun 2021 03:53:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20210611100703.2285-1-david.plowman@raspberrypi.com>\n\t<20210611100703.2285-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20210611100703.2285-2-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Fri, 11 Jun 2021 11:53:29 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000006d7d7805c47b527e\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] ipa: raspberrypi: Add\n\tsensitivity field to camera mode","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18175,"web_url":"https://patchwork.libcamera.org/comment/18175/","msgid":"","date":"2021-07-14T13:57:52","subject":"Re: [libcamera-devel] [PATCH v3 1/2] ipa: raspberrypi: Add\n\tsensitivity field to camera mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nOn 11/06/2021 11:07, David Plowman wrote:\n> We use the CamHelper class to initialise it to the usual value of 1.\n> \n> The CamHelper's GetModeSensitivity method can be redefined to\n> implement a different behaviour for sensors that require it.\n> \n> Signed-off-by: David Plowman \n\nAcked-by: Kieran Bingham \n\n\n\n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 11 +++++++++++\n> src/ipa/raspberrypi/cam_helper.hpp | 3 +++\n> src/ipa/raspberrypi/controller/camera_mode.h | 2 ++\n> src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++++\n> 4 files changed, 22 insertions(+)\n> \n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 062e94c4..d0104da7 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -130,6 +130,17 @@ bool CamHelper::SensorEmbeddedDataPresent() const\n> \treturn false;\n> }\n> \n> +double CamHelper::GetModeSensitivity([[maybe_unused]] const CameraMode &mode) const\n> +{\n> +\t/*\n> +\t * Most sensors have the same sensitivity in every mode, but this\n> +\t * method can be overridden for those that do not. Note that it is\n> +\t * called before mode_ is set, so it must return the sensitivity\n> +\t * of the mode that is passed in.\n> +\t */\n> +\treturn 1.0;\n> +}\n> +\n> unsigned int CamHelper::HideFramesStartup() const\n> {\n> \t/*\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\n> index f53f5c39..f524fe84 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -40,6 +40,8 @@ namespace RPiController {\n> //\n> // A method to query if the sensor outputs embedded data that can be parsed.\n> //\n> +// A method to return the sensitivity of a given camera mode.\n> +//\n> // A parser to parse the embedded data buffers provided by some sensors (for\n> // example, the imx219 does; the ov5647 doesn't). This allows us to know for\n> // sure the exposure and gain of the frame we're looking at. CamHelper\n> @@ -83,6 +85,7 @@ public:\n> \tvirtual void GetDelays(int &exposure_delay, int &gain_delay,\n> \t\t\t int &vblank_delay) const;\n> \tvirtual bool SensorEmbeddedDataPresent() const;\n> +\tvirtual double GetModeSensitivity(const CameraMode &mode) const;\n> \tvirtual unsigned int HideFramesStartup() const;\n> \tvirtual unsigned int HideFramesModeSwitch() const;\n> \tvirtual unsigned int MistrustFramesStartup() const;\n> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h\n> index 2aa2335d..72c68dc6 100644\n> --- a/src/ipa/raspberrypi/controller/camera_mode.h\n> +++ b/src/ipa/raspberrypi/controller/camera_mode.h\n> @@ -41,6 +41,8 @@ struct CameraMode {\n> \tlibcamera::Transform transform;\n> \t// minimum and maximum fame lengths in units of lines\n> \tuint32_t min_frame_length, max_frame_length;\n> +\t// sensitivity of this mode\n> +\tdouble sensitivity;\n> };\n> \n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 1c1e802a..9ce0db08 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -324,6 +324,12 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)\n> \t */\n> \tmode_.min_frame_length = sensorInfo.minFrameLength;\n> \tmode_.max_frame_length = sensorInfo.maxFrameLength;\n> +\n> +\t/*\n> +\t * Some sensors may have different sensitivities in different modes;\n> +\t * the CamHelper will know the correct value.\n> +\t */\n> +\tmode_.sensitivity = helper_->GetModeSensitivity(mode_);\n> }\n> \n> int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\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 95E56C3226\n\tfor ;\n\tWed, 14 Jul 2021 13:57:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8E3668525;\n\tWed, 14 Jul 2021 15:57:56 +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 027B168503\n\tfor ;\n\tWed, 14 Jul 2021 15:57: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 5E26ACC;\n\tWed, 14 Jul 2021 15:57:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ie9RxzEc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626271075;\n\tbh=cEhqkG3JwwP90MBkYw1DLuTe/lR9SB2dWOj9cdQR4cc=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ie9RxzEcOew3VW1VEVbKX/uFershIRPvMycodtNy5jHey/C+ZNozcRUqQVZ6uUbaK\n\tOmg72LFidnvSHIz5d1S/UKDKfnvAlGWrtoOJR5y/7xW984m36dMbftss5PthTvamXx\n\tuXlAmbC3StaSOPhXA4V9MLvR/IGvJO8UFfw3npBI=","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210611100703.2285-1-david.plowman@raspberrypi.com>\n\t<20210611100703.2285-2-david.plowman@raspberrypi.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 14 Jul 2021 14:57:52 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210611100703.2285-2-david.plowman@raspberrypi.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] ipa: raspberrypi: Add\n\tsensitivity field to camera mode","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]