[{"id":27777,"web_url":"https://patchwork.libcamera.org/comment/27777/","msgid":"<169473078929.2843013.13597067177055656550@ping.linuxembedded.co.uk>","date":"2023-09-14T22:33:09","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_sensor: Add\n\tfunction to apply a config","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"In $SUBJECT : \nI would probably do:\n\n-libcamera: camera_sensor: Add function to apply a config\n+libcamera: camera_sensor: Support a SensorConfiguration\n\n\nQuoting Jacopo Mondi via libcamera-devel (2023-07-31 12:31:14)\n> Add to the CameraSensor class a function to apply to the sensor\n> a full configuration.\n\nAdd a class function to the CameraSensor class to apply a full\nconfiguration to the sensor.\n\n\n\n> \n> The configuration shall be fully populated and shall apply without\n> modifications to the sensor.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Naushir Patuck \n> ---\n> include/libcamera/internal/camera_sensor.h | 5 ++\n> src/libcamera/camera_sensor.cpp | 86 ++++++++++++++++++++++\n> 2 files changed, 91 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 02c77ab037da..06791c3c6425 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -29,6 +29,7 @@ namespace libcamera {\n> class BayerFormat;\n> class CameraLens;\n> class MediaEntity;\n> +class SensorConfiguration;\n> \n> struct CameraSensorProperties;\n> \n> @@ -58,6 +59,10 @@ public:\n> Transform transform = Transform::Identity);\n> int tryFormat(V4L2SubdeviceFormat *format) const;\n> \n> + int applyConfiguration(const SensorConfiguration &config,\n> + Transform transform = Transform::Identity,\n> + V4L2SubdeviceFormat *sensorFormat = nullptr);\n> +\n> const ControlInfoMap &controls() const;\n> ControlList getControls(const std::vector &ids);\n> int setControls(ControlList *ctrls);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index f3a5aa37149f..96b618572a3d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \n> @@ -822,6 +823,91 @@ int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> V4L2Subdevice::Whence::TryFormat);\n> }\n> \n> +/**\n> + * \\brief Apply a sensor configuration to the camera sensor\n> + * \\param[in] config The sensor configuration\n> + * \\param[in] transform The transform to be applied on the sensor.\n> + * Defaults to Identity\n> + * \\param[out] sensorFormat Optional output parameter where the format\n> + * actually applied to the sensor is returned to the caller\n> + *\n> + * Apply to the camera sensor the configuration \\a config.\n> + *\n> + * \\todo The configuration shall be fully populated and if any of the fields\n> + * there specified cannot be applied as it is, an error code is returned.\n\nand if any of the fields specified cannot be applied exactly, an error\ncode is returned.\n\n> + *\n> + * \\return 0 if \\a config is applied to the camera sensor, a negative error code\n> + * otherwise\n> + */\n> +int CameraSensor::applyConfiguration(const SensorConfiguration &config,\n> + Transform transform,\n> + V4L2SubdeviceFormat *sensorFormat)\n> +{\n> + if (!config) {\n> + LOG(CameraSensor, Error) << \"Invalid sensor configuration\";\n> + return -EINVAL;\n> + }\n> +\n> + std::vector filteredCodes;\n> + std::copy_if(mbusCodes_.begin(), mbusCodes_.end(),\n> + std::back_inserter(filteredCodes),\n> + [&config](unsigned int mbusCode) {\n> + BayerFormat bayer = BayerFormat::fromMbusCode(mbusCode);\n> + if (bayer.bitDepth == config.bitDepth)\n> + return true;\n> + return false;\n> + });\n> + if (filteredCodes.empty()) {\n> + LOG(CameraSensor, Error)\n> + << \"Cannot find any format with bit depth \"\n> + << config.bitDepth;\n> + return -EINVAL;\n> + }\n> +\n> + /*\n> + * Compute the sensor's data frame size by applying the cropping\n> + * rectangle, subsampling and output crop to the sensor's pixel array\n> + * size.\n> + *\n> + * \\todo The actual size computation is for now ignored and only the\n> + * output size is considered. This implies that resolutions obtained\n> + * with two different cropping/subsampling will look identical and\n> + * only the first found one will be considered.\n> + */\n> + V4L2SubdeviceFormat subdevFormat = {};\n> + for (unsigned int code : filteredCodes) {\n> + for (const Size &size : sizes(code)) {\n> + if (size.width != config.outputSize.width ||\n> + size.height != config.outputSize.height)\n> + continue;\n> +\n> + subdevFormat.mbus_code = code;\n> + subdevFormat.size = size;\n> + break;\n> + }\n> + }\n> + if (!subdevFormat.mbus_code) {\n> + LOG(CameraSensor, Error) << \"Invalid output size in sensor configuration\";\n> + return -EINVAL;\n> + }\n> +\n> + int ret = setFormat(&subdevFormat, transform);\n> + if (ret)\n> + return ret;\n> +\n> + /*\n> + * Return to the caller the format actually applied to the sensor.\n> + * This is relevant if transform has changed the bayer pattern order.\n> + */\n> + if (sensorFormat)\n> + *sensorFormat = subdevFormat;\n> +\n> + /* \\todo Handle AnalogCrop. Most sensors do not support set_selection */\n> + /* \\todo Handle scaling in the digital domain. */\n\nShould these return an error if they are set as they are unhandled for\nnow? Or are we happy to just silently ignore here for now?\n\nExcept the known todos, nothing blocking here though so\n\n\nReviewed-by: Kieran Bingham \n\n\n> +\n> + return 0;\n> +}\n> +\n> /**\n> * \\brief Retrieve the supported V4L2 controls and their information\n> *\n> -- \n> 2.40.1\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 5DC20BE080\n\tfor ;\n\tThu, 14 Sep 2023 22:33:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9B7962917;\n\tFri, 15 Sep 2023 00:33:12 +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 08E8E628DA\n\tfor ;\n\tFri, 15 Sep 2023 00:33:12 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DEAD18A1;\n\tFri, 15 Sep 2023 00:31:39 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694730792;\n\tbh=MPJ+ks6Gm29VqBgfGjU+7y7kxNcwsvaoeaj3VvpkjAc=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fRTPTAtfpE1CqPQmKX9hkf4GRhzAlXS/5+WmgE+z9dwR+spREEuY50mSq4yka+Ot3\n\tD6McmRyYz+zReTqTzAbdcavHUFpUKVGBaVpBinwqqjkEwIf7rVZx+SrfD14mB87P+K\n\tcbeSEujg7Vx62v9TesOo5RAd7wn8euCTdn8No5nopyVHdvEjqA62suP3Vskj864Vwf\n\txRZ2yKSTZXKRFaUTNQQKSr848QyOOxyKcRGUFISGLh3zHMTy27uJMIm0ICvtHRt7N7\n\t2YZqb2mZKf9qEkgcFPWCqKNY+3y3MyAcoCuUa61gxPJDQzkdAfEoe4+Jyq/O95ODqD\n\ttT8PRFxu13B3g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694730699;\n\tbh=MPJ+ks6Gm29VqBgfGjU+7y7kxNcwsvaoeaj3VvpkjAc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GpuS7YMHkBfk5ZcoVUtZsxAIYNr32LSX7F3rVpzRHOKy4x7rxoQKf26Gsl0YLsMSI\n\tJ2PUBn4tpu8UHgSvcHvFfsDCwy4jv6Qk8viMDn7QFj4A8IbW8vMElY4aQcNApne+wj\n\ts6ZVjUPOmOM4Cl3VNj84u01wxyFYlAK90DbGRRTQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GpuS7YMH\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230731113115.5915-4-jacopo.mondi@ideasonboard.com>","References":"<20230731113115.5915-1-jacopo.mondi@ideasonboard.com>\n\t<20230731113115.5915-4-jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 14 Sep 2023 23:33:09 +0100","Message-ID":"<169473078929.2843013.13597067177055656550@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_sensor: Add\n\tfunction to apply a config","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27784,"web_url":"https://patchwork.libcamera.org/comment/27784/","msgid":"","date":"2023-09-15T08:23:47","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_sensor: Add\n\tfunction to apply a config","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Thu, Sep 14, 2023 at 11:33:09PM +0100, Kieran Bingham wrote:\n> In $SUBJECT :\n> I would probably do:\n>\n> -libcamera: camera_sensor: Add function to apply a config\n> +libcamera: camera_sensor: Support a SensorConfiguration\n>\n>\n> Quoting Jacopo Mondi via libcamera-devel (2023-07-31 12:31:14)\n> > Add to the CameraSensor class a function to apply to the sensor\n> > a full configuration.\n>\n> Add a class function to the CameraSensor class to apply a full\n> configuration to the sensor.\n>\n>\n>\n> >\n> > The configuration shall be fully populated and shall apply without\n> > modifications to the sensor.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Naushir Patuck \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 5 ++\n> > src/libcamera/camera_sensor.cpp | 86 ++++++++++++++++++++++\n> > 2 files changed, 91 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 02c77ab037da..06791c3c6425 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -29,6 +29,7 @@ namespace libcamera {\n> > class BayerFormat;\n> > class CameraLens;\n> > class MediaEntity;\n> > +class SensorConfiguration;\n> >\n> > struct CameraSensorProperties;\n> >\n> > @@ -58,6 +59,10 @@ public:\n> > Transform transform = Transform::Identity);\n> > int tryFormat(V4L2SubdeviceFormat *format) const;\n> >\n> > + int applyConfiguration(const SensorConfiguration &config,\n> > + Transform transform = Transform::Identity,\n> > + V4L2SubdeviceFormat *sensorFormat = nullptr);\n> > +\n> > const ControlInfoMap &controls() const;\n> > ControlList getControls(const std::vector &ids);\n> > int setControls(ControlList *ctrls);\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index f3a5aa37149f..96b618572a3d 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -15,6 +15,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -822,6 +823,91 @@ int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> > V4L2Subdevice::Whence::TryFormat);\n> > }\n> >\n> > +/**\n> > + * \\brief Apply a sensor configuration to the camera sensor\n> > + * \\param[in] config The sensor configuration\n> > + * \\param[in] transform The transform to be applied on the sensor.\n> > + * Defaults to Identity\n> > + * \\param[out] sensorFormat Optional output parameter where the format\n> > + * actually applied to the sensor is returned to the caller\n> > + *\n> > + * Apply to the camera sensor the configuration \\a config.\n> > + *\n> > + * \\todo The configuration shall be fully populated and if any of the fields\n> > + * there specified cannot be applied as it is, an error code is returned.\n>\n> and if any of the fields specified cannot be applied exactly, an error\n> code is returned.\n>\n> > + *\n> > + * \\return 0 if \\a config is applied to the camera sensor, a negative error code\n> > + * otherwise\n> > + */\n> > +int CameraSensor::applyConfiguration(const SensorConfiguration &config,\n> > + Transform transform,\n> > + V4L2SubdeviceFormat *sensorFormat)\n> > +{\n> > + if (!config) {\n> > + LOG(CameraSensor, Error) << \"Invalid sensor configuration\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + std::vector filteredCodes;\n> > + std::copy_if(mbusCodes_.begin(), mbusCodes_.end(),\n> > + std::back_inserter(filteredCodes),\n> > + [&config](unsigned int mbusCode) {\n> > + BayerFormat bayer = BayerFormat::fromMbusCode(mbusCode);\n> > + if (bayer.bitDepth == config.bitDepth)\n> > + return true;\n> > + return false;\n> > + });\n> > + if (filteredCodes.empty()) {\n> > + LOG(CameraSensor, Error)\n> > + << \"Cannot find any format with bit depth \"\n> > + << config.bitDepth;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + /*\n> > + * Compute the sensor's data frame size by applying the cropping\n> > + * rectangle, subsampling and output crop to the sensor's pixel array\n> > + * size.\n> > + *\n> > + * \\todo The actual size computation is for now ignored and only the\n> > + * output size is considered. This implies that resolutions obtained\n> > + * with two different cropping/subsampling will look identical and\n> > + * only the first found one will be considered.\n> > + */\n> > + V4L2SubdeviceFormat subdevFormat = {};\n> > + for (unsigned int code : filteredCodes) {\n> > + for (const Size &size : sizes(code)) {\n> > + if (size.width != config.outputSize.width ||\n> > + size.height != config.outputSize.height)\n> > + continue;\n> > +\n> > + subdevFormat.mbus_code = code;\n> > + subdevFormat.size = size;\n> > + break;\n> > + }\n> > + }\n> > + if (!subdevFormat.mbus_code) {\n> > + LOG(CameraSensor, Error) << \"Invalid output size in sensor configuration\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + int ret = setFormat(&subdevFormat, transform);\n> > + if (ret)\n> > + return ret;\n> > +\n> > + /*\n> > + * Return to the caller the format actually applied to the sensor.\n> > + * This is relevant if transform has changed the bayer pattern order.\n> > + */\n> > + if (sensorFormat)\n> > + *sensorFormat = subdevFormat;\n> > +\n> > + /* \\todo Handle AnalogCrop. Most sensors do not support set_selection */\n> > + /* \\todo Handle scaling in the digital domain. */\n>\n> Should these return an error if they are set as they are unhandled for\n> now? Or are we happy to just silently ignore here for now?\n\nAn error might be too harsh to be honest. Actually, right now most of\nthe sensor configuration fields are ignored, except for bitdepth and\noutput size. Should we maybe WARN ?\n\n>\n> Except the known todos, nothing blocking here though so\n>\n>\n> Reviewed-by: Kieran Bingham \n\nThanks\n j\n\n>\n>\n> > +\n> > + return 0;\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve the supported V4L2 controls and their information\n> > *\n> > --\n> > 2.40.1\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 38F7EBE080\n\tfor ;\n\tFri, 15 Sep 2023 08:23:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 684D862919;\n\tFri, 15 Sep 2023 10:23:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1543262911\n\tfor ;\n\tFri, 15 Sep 2023 10:23:50 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A507C12AE;\n\tFri, 15 Sep 2023 10:22:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694766231;\n\tbh=ceBLiAXM9zM4pCDHV4hM/EtamKRl6X0OgXz8xbNlqjs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1cSQ8T0rMahyvIdgg3HAYcfIIlz+h0nbL19co4Jabk2JL7G1VRud39gUszRKXasKM\n\tY3Fj+XCfHpatu42X+egF4u6X0eeReHUFGSmg4a8wCc/Qcr5sso5rUwiTL/6pR5aJKJ\n\td5HJC+VDceNujoaxEU9977HcANMtaFwtf3jp8rG+nK9C4f7Q4WkFlKUk7mMBhI9VPF\n\t7PMO1AacW22PNjGc8uhOCcZlSKgDTQdhvDhTJAbF2kf8L4sOOtbbtFEeVeO+qdBetA\n\tFj+FkRgNeSbVAKqVn7ZTMU/YXrR7o6qoOnRyqpvbzg8FDaJl6KH9adE8NaF2B/zJHP\n\tacslT1Jc/2BYA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694766136;\n\tbh=ceBLiAXM9zM4pCDHV4hM/EtamKRl6X0OgXz8xbNlqjs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NnL8vawTCPDjqFDm4tFj7ihXiALdG379ilfS9LPrcj9pW9CAlJsvR4tHOmT2eDcve\n\tcqucPg4AKp9djBF76cWtgIBd2eRZANmxUo0hgYPRMgJYohGoy5Do1GdM0YskGxzIw7\n\tKCAfFh4W37cgxQUFgoWtvE+BfBIHZQRmIknA9sGE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NnL8vawT\"; dkim-atps=neutral","Date":"Fri, 15 Sep 2023 10:23:47 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20230731113115.5915-1-jacopo.mondi@ideasonboard.com>\n\t<20230731113115.5915-4-jacopo.mondi@ideasonboard.com>\n\t<169473078929.2843013.13597067177055656550@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<169473078929.2843013.13597067177055656550@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_sensor: Add\n\tfunction to apply a config","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]