[{"id":14873,"web_url":"https://patchwork.libcamera.org/comment/14873/","msgid":"<20210201095212.5bceparnuwgzu54s@uno.localdomain>","date":"2021-02-01T09:52:12","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Sun, Jan 31, 2021 at 08:17:21PM +0200, Laurent Pinchart wrote:\n> YUV sensors don't provide the necessary information to fill\n> CameraSensorInfo, as they include an ISP and provide a higher-level API\n> that doesn't always expose low-level information. The CameraSensor class\n> makes low-level V4L2 controls mandatory for all sensors, which prevents\n> usage of YUV sensors with the simple pipeline handler.\n>\n> Make CameraSensor::sensorInfo() available for raw sensors only. This\n> won't introduce any regression in pipeline handlers that currently use\n> the sensorInfo() function as they all operate with raw sensors, and\n> won't be a limitation for the simple pipeline handler as well as it\n> doesn't use sensor info. If part of the sensor info (such as the active\n> pixel array size for instance) becomes useful to expose for YUV sensors\n> in the future, the sensorInfo() function can be extended to report that\n> information only and skip data that is only available for raw sensors.\n>\n\nIt makes sense, and I see the reasons why you won't this.\nBut I'm a little concerned that with the sensorInfo() function\nreturning -EINVAL we're limiting pipeline handlers that use\nsensorInfo to work with raw sensor only, while I assume it's totally\nplausible that someone wants to connect a YUV sensor to the pi, in\nexample.\n\nAlternatively, sensorInfo can return only information that are\navailable in YUV sensor, but then the pipeline-IPA would break anyway.\n\nThis is less trivial than it seems and I'm afraid it will require a\ndiffernet handling of RAW/YUV sensor in the pipeline handlers. Or do\nyou think a YUV sensor connected to a platform with a ISP it's not\nthat likely as a combination and we should not care too much ?\n\nThanks\n j\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/sensor_driver_requirements.rst | 2 +-\n> src/libcamera/camera_sensor.cpp | 59 ++++++++++++--------\n> 2 files changed, 36 insertions(+), 25 deletions(-)\n>\n> diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\n> index 6dcd4e68d64d..64e00fd5fc7c 100644\n> --- a/Documentation/sensor_driver_requirements.rst\n> +++ b/Documentation/sensor_driver_requirements.rst\n> @@ -22,7 +22,7 @@ Mandatory Requirements\n>\n> The sensor driver is assumed to be fully compliant with the V4L2 specification.\n>\n> -The sensor driver shall support the following V4L2 controls:\n> +For RAW sensors, the sensor driver shall support the following V4L2 controls:\n>\n> * `V4L2_CID_EXPOSURE`_\n> * `V4L2_CID_HBLANK`_\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 35312857ff90..10713d3a0c29 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -246,27 +246,6 @@ int CameraSensor::init()\n>\n> int CameraSensor::validateSensorDriver()\n> {\n> -\t/*\n> -\t * Make sure the sensor driver supports the mandatory controls\n> -\t * required by the CameraSensor class.\n> -\t */\n> -\tconst std::vector mandatoryControls{\n> -\t\tV4L2_CID_EXPOSURE,\n> -\t\tV4L2_CID_HBLANK,\n> -\t\tV4L2_CID_PIXEL_RATE,\n> -\t};\n> -\n> -\tControlList ctrls = subdev_->getControls(mandatoryControls);\n> -\tif (ctrls.empty()) {\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"Mandatory V4L2 controls not available\";\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> -\t\tLOG(CameraSensor, Error)\n> -\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> \t/*\n> \t * Optional controls are used to register optional sensor properties. If\n> \t * not present, some values will be defaulted.\n> @@ -276,7 +255,7 @@ int CameraSensor::validateSensorDriver()\n> \t\tV4L2_CID_CAMERA_SENSOR_ROTATION,\n> \t};\n>\n> -\tctrls = subdev_->getControls(optionalControls);\n> +\tControlList ctrls = subdev_->getControls(optionalControls);\n> \tif (ctrls.empty())\n> \t\tLOG(CameraSensor, Debug) << \"Optional V4L2 controls not supported\";\n>\n> @@ -326,6 +305,30 @@ int CameraSensor::validateSensorDriver()\n> \t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> \t}\n>\n> +\tif (!bayerFormat_)\n> +\t\treturn 0;\n> +\n> +\t/*\n> +\t * For raw sensors, make sure the sensor driver supports the controls\n> +\t * required by the CameraSensor class.\n> +\t */\n> +\tconst std::vector mandatoryControls{\n> +\t\tV4L2_CID_EXPOSURE,\n> +\t\tV4L2_CID_HBLANK,\n> +\t\tV4L2_CID_PIXEL_RATE,\n> +\t};\n> +\n> +\tctrls = subdev_->getControls(mandatoryControls);\n> +\tif (ctrls.empty()) {\n> +\t\tLOG(CameraSensor, Error)\n> +\t\t\t<< \"Mandatory V4L2 controls not available\";\n> +\t\tLOG(CameraSensor, Error)\n> +\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> +\t\tLOG(CameraSensor, Error)\n> +\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \treturn 0;\n> }\n>\n> @@ -662,13 +665,21 @@ int CameraSensor::setControls(ControlList *ctrls)\n> * \\brief Assemble and return the camera sensor info\n> * \\param[out] info The camera sensor info\n> *\n> - * The CameraSensorInfo content is assembled by inspecting the currently\n> - * applied sensor configuration and the sensor static properties.\n> + * This function fills \\a info with information that describes the camera sensor\n> + * and its current configuration. The information combines static data (such as\n> + * the the sensor model or active pixel array size) and data specific to the\n> + * current sensor configuration (such as the line length and pixel rate).\n> + *\n> + * Sensor information is only available for raw sensors. When called for a YUV\n> + * sensor, this function returns -EINVAL.\n> *\n> * \\return 0 on success, a negative error code otherwise\n> */\n> int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> {\n> +\tif (!bayerFormat_)\n> +\t\treturn -EINVAL;\n> +\n> \tinfo->model = model();\n>\n> \t/*\n> --\n> Regards,\n>\n> Laurent Pinchart\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 E540ABD808\n\tfor ;\n\tMon, 1 Feb 2021 09:51:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7417D60307;\n\tMon, 1 Feb 2021 10:51:51 +0100 (CET)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D6F6160307\n\tfor ;\n\tMon, 1 Feb 2021 10:51:50 +0100 (CET)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 31A6940010;\n\tMon, 1 Feb 2021 09:51:49 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Mon, 1 Feb 2021 10:52:12 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210201095212.5bceparnuwgzu54s@uno.localdomain>","References":"<20210131181722.5410-1-laurent.pinchart@ideasonboard.com>\n\t<20210131181722.5410-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210131181722.5410-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","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":14889,"web_url":"https://patchwork.libcamera.org/comment/14889/","msgid":"","date":"2021-02-01T20:41:30","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Feb 01, 2021 at 10:52:12AM +0100, Jacopo Mondi wrote:\n> On Sun, Jan 31, 2021 at 08:17:21PM +0200, Laurent Pinchart wrote:\n> > YUV sensors don't provide the necessary information to fill\n> > CameraSensorInfo, as they include an ISP and provide a higher-level API\n> > that doesn't always expose low-level information. The CameraSensor class\n> > makes low-level V4L2 controls mandatory for all sensors, which prevents\n> > usage of YUV sensors with the simple pipeline handler.\n> >\n> > Make CameraSensor::sensorInfo() available for raw sensors only. This\n> > won't introduce any regression in pipeline handlers that currently use\n> > the sensorInfo() function as they all operate with raw sensors, and\n> > won't be a limitation for the simple pipeline handler as well as it\n> > doesn't use sensor info. If part of the sensor info (such as the active\n> > pixel array size for instance) becomes useful to expose for YUV sensors\n> > in the future, the sensorInfo() function can be extended to report that\n> > information only and skip data that is only available for raw sensors.\n> \n> It makes sense, and I see the reasons why you won't this.\n> But I'm a little concerned that with the sensorInfo() function\n> returning -EINVAL we're limiting pipeline handlers that use\n> sensorInfo to work with raw sensor only, while I assume it's totally\n> plausible that someone wants to connect a YUV sensor to the pi, in\n> example.\n\nThat's absolutely correct, but in that case, the YUV sensor will include\nan ISP and will produce processed images. The RPi ISP and the IPA should\nbe bypassed completely. I think we should support this at some point,\nand it will require changes in the RPi pipeline handler, which will\ninclude not use sensorInfo() in that case (as the sensor info is meant\nfor the IPA).\n\n> Alternatively, sensorInfo can return only information that are\n> available in YUV sensor, but then the pipeline-IPA would break anyway.\n\nI think it would make sense if some of the information is still useful\nfor pipeline that don't use an ISP and IPA. As I can't tell yet what\ninformation that would be, I've decided not to implement this yet until\nwe get a use case.\n\n> This is less trivial than it seems and I'm afraid it will require a\n> differnet handling of RAW/YUV sensor in the pipeline handlers. Or do\n> you think a YUV sensor connected to a platform with a ISP it's not\n> that likely as a combination and we should not care too much ?\n\nYou're right in my opinion that we should support this, but I believe\nonly when bypassing the ISP and IPA.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/sensor_driver_requirements.rst | 2 +-\n> > src/libcamera/camera_sensor.cpp | 59 ++++++++++++--------\n> > 2 files changed, 36 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\n> > index 6dcd4e68d64d..64e00fd5fc7c 100644\n> > --- a/Documentation/sensor_driver_requirements.rst\n> > +++ b/Documentation/sensor_driver_requirements.rst\n> > @@ -22,7 +22,7 @@ Mandatory Requirements\n> >\n> > The sensor driver is assumed to be fully compliant with the V4L2 specification.\n> >\n> > -The sensor driver shall support the following V4L2 controls:\n> > +For RAW sensors, the sensor driver shall support the following V4L2 controls:\n> >\n> > * `V4L2_CID_EXPOSURE`_\n> > * `V4L2_CID_HBLANK`_\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 35312857ff90..10713d3a0c29 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -246,27 +246,6 @@ int CameraSensor::init()\n> >\n> > int CameraSensor::validateSensorDriver()\n> > {\n> > -\t/*\n> > -\t * Make sure the sensor driver supports the mandatory controls\n> > -\t * required by the CameraSensor class.\n> > -\t */\n> > -\tconst std::vector mandatoryControls{\n> > -\t\tV4L2_CID_EXPOSURE,\n> > -\t\tV4L2_CID_HBLANK,\n> > -\t\tV4L2_CID_PIXEL_RATE,\n> > -\t};\n> > -\n> > -\tControlList ctrls = subdev_->getControls(mandatoryControls);\n> > -\tif (ctrls.empty()) {\n> > -\t\tLOG(CameraSensor, Error)\n> > -\t\t\t<< \"Mandatory V4L2 controls not available\";\n> > -\t\tLOG(CameraSensor, Error)\n> > -\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> > -\t\tLOG(CameraSensor, Error)\n> > -\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > -\t\treturn -EINVAL;\n> > -\t}\n> > -\n> > \t/*\n> > \t * Optional controls are used to register optional sensor properties. If\n> > \t * not present, some values will be defaulted.\n> > @@ -276,7 +255,7 @@ int CameraSensor::validateSensorDriver()\n> > \t\tV4L2_CID_CAMERA_SENSOR_ROTATION,\n> > \t};\n> >\n> > -\tctrls = subdev_->getControls(optionalControls);\n> > +\tControlList ctrls = subdev_->getControls(optionalControls);\n> > \tif (ctrls.empty())\n> > \t\tLOG(CameraSensor, Debug) << \"Optional V4L2 controls not supported\";\n> >\n> > @@ -326,6 +305,30 @@ int CameraSensor::validateSensorDriver()\n> > \t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > \t}\n> >\n> > +\tif (!bayerFormat_)\n> > +\t\treturn 0;\n> > +\n> > +\t/*\n> > +\t * For raw sensors, make sure the sensor driver supports the controls\n> > +\t * required by the CameraSensor class.\n> > +\t */\n> > +\tconst std::vector mandatoryControls{\n> > +\t\tV4L2_CID_EXPOSURE,\n> > +\t\tV4L2_CID_HBLANK,\n> > +\t\tV4L2_CID_PIXEL_RATE,\n> > +\t};\n> > +\n> > +\tctrls = subdev_->getControls(mandatoryControls);\n> > +\tif (ctrls.empty()) {\n> > +\t\tLOG(CameraSensor, Error)\n> > +\t\t\t<< \"Mandatory V4L2 controls not available\";\n> > +\t\tLOG(CameraSensor, Error)\n> > +\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> > +\t\tLOG(CameraSensor, Error)\n> > +\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > \treturn 0;\n> > }\n> >\n> > @@ -662,13 +665,21 @@ int CameraSensor::setControls(ControlList *ctrls)\n> > * \\brief Assemble and return the camera sensor info\n> > * \\param[out] info The camera sensor info\n> > *\n> > - * The CameraSensorInfo content is assembled by inspecting the currently\n> > - * applied sensor configuration and the sensor static properties.\n> > + * This function fills \\a info with information that describes the camera sensor\n> > + * and its current configuration. The information combines static data (such as\n> > + * the the sensor model or active pixel array size) and data specific to the\n> > + * current sensor configuration (such as the line length and pixel rate).\n> > + *\n> > + * Sensor information is only available for raw sensors. When called for a YUV\n> > + * sensor, this function returns -EINVAL.\n> > *\n> > * \\return 0 on success, a negative error code otherwise\n> > */\n> > int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > {\n> > +\tif (!bayerFormat_)\n> > +\t\treturn -EINVAL;\n> > +\n> > \tinfo->model = model();\n> >\n> > \t/*","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 21D03BD808\n\tfor ;\n\tMon, 1 Feb 2021 20:41:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9453A68412;\n\tMon, 1 Feb 2021 21:41:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C665B683FF\n\tfor ;\n\tMon, 1 Feb 2021 21:41:51 +0100 (CET)","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 3BAC9556;\n\tMon, 1 Feb 2021 21:41:51 +0100 (CET)"],"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=\"RuJujYEt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612212111;\n\tbh=jLBeWiWUAHr7g3kfcJbuDwU5vMCkJ2l86+aCSs0zUKA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RuJujYEt74vGgkFfxg0TUCVy/2qMVuu9DY++2Dckp3MA4K8YfixaXJYtigslYlS6Y\n\tHZqIRPOdi0twsH2vj8hUQnlWQvpxMZVVDAd4KGuKkXvU9brrcyDmMsynO1gQ+9TPoJ\n\t5yyWsTmysoJzu7j1LiNCi4WtahKtlFquSh+GhEh0=","Date":"Mon, 1 Feb 2021 22:41:30 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210131181722.5410-1-laurent.pinchart@ideasonboard.com>\n\t<20210131181722.5410-3-laurent.pinchart@ideasonboard.com>\n\t<20210201095212.5bceparnuwgzu54s@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210201095212.5bceparnuwgzu54s@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","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":14963,"web_url":"https://patchwork.libcamera.org/comment/14963/","msgid":"<20210204121345.ezpdbuia7nkgk2sn@uno.localdomain>","date":"2021-02-04T12:13:45","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Feb 01, 2021 at 10:41:30PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Feb 01, 2021 at 10:52:12AM +0100, Jacopo Mondi wrote:\n> > On Sun, Jan 31, 2021 at 08:17:21PM +0200, Laurent Pinchart wrote:\n> > > YUV sensors don't provide the necessary information to fill\n> > > CameraSensorInfo, as they include an ISP and provide a higher-level API\n> > > that doesn't always expose low-level information. The CameraSensor class\n> > > makes low-level V4L2 controls mandatory for all sensors, which prevents\n> > > usage of YUV sensors with the simple pipeline handler.\n> > >\n> > > Make CameraSensor::sensorInfo() available for raw sensors only. This\n> > > won't introduce any regression in pipeline handlers that currently use\n> > > the sensorInfo() function as they all operate with raw sensors, and\n> > > won't be a limitation for the simple pipeline handler as well as it\n> > > doesn't use sensor info. If part of the sensor info (such as the active\n> > > pixel array size for instance) becomes useful to expose for YUV sensors\n> > > in the future, the sensorInfo() function can be extended to report that\n> > > information only and skip data that is only available for raw sensors.\n> >\n> > It makes sense, and I see the reasons why you won't this.\n> > But I'm a little concerned that with the sensorInfo() function\n> > returning -EINVAL we're limiting pipeline handlers that use\n> > sensorInfo to work with raw sensor only, while I assume it's totally\n> > plausible that someone wants to connect a YUV sensor to the pi, in\n> > example.\n>\n> That's absolutely correct, but in that case, the YUV sensor will include\n> an ISP and will produce processed images. The RPi ISP and the IPA should\n> be bypassed completely. I think we should support this at some point,\n> and it will require changes in the RPi pipeline handler, which will\n> include not use sensorInfo() in that case (as the sensor info is meant\n> for the IPA).\n>\n> > Alternatively, sensorInfo can return only information that are\n> > available in YUV sensor, but then the pipeline-IPA would break anyway.\n>\n> I think it would make sense if some of the information is still useful\n> for pipeline that don't use an ISP and IPA. As I can't tell yet what\n> information that would be, I've decided not to implement this yet until\n> we get a use case.\n>\n> > This is less trivial than it seems and I'm afraid it will require a\n> > differnet handling of RAW/YUV sensor in the pipeline handlers. Or do\n> > you think a YUV sensor connected to a platform with a ISP it's not\n> > that likely as a combination and we should not care too much ?\n>\n> You're right in my opinion that we should support this, but I believe\n> only when bypassing the ISP and IPA.\n\nIt all makes sense. I'm a bit afraid than with this change someone\ntrying an YUV sensor with a pipeline that requires SensorInfo will\nhave an hard failure.\n\nAnyway, I have no better options, so please add\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > Documentation/sensor_driver_requirements.rst | 2 +-\n> > > src/libcamera/camera_sensor.cpp | 59 ++++++++++++--------\n> > > 2 files changed, 36 insertions(+), 25 deletions(-)\n> > >\n> > > diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\n> > > index 6dcd4e68d64d..64e00fd5fc7c 100644\n> > > --- a/Documentation/sensor_driver_requirements.rst\n> > > +++ b/Documentation/sensor_driver_requirements.rst\n> > > @@ -22,7 +22,7 @@ Mandatory Requirements\n> > >\n> > > The sensor driver is assumed to be fully compliant with the V4L2 specification.\n> > >\n> > > -The sensor driver shall support the following V4L2 controls:\n> > > +For RAW sensors, the sensor driver shall support the following V4L2 controls:\n> > >\n> > > * `V4L2_CID_EXPOSURE`_\n> > > * `V4L2_CID_HBLANK`_\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 35312857ff90..10713d3a0c29 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -246,27 +246,6 @@ int CameraSensor::init()\n> > >\n> > > int CameraSensor::validateSensorDriver()\n> > > {\n> > > -\t/*\n> > > -\t * Make sure the sensor driver supports the mandatory controls\n> > > -\t * required by the CameraSensor class.\n> > > -\t */\n> > > -\tconst std::vector mandatoryControls{\n> > > -\t\tV4L2_CID_EXPOSURE,\n> > > -\t\tV4L2_CID_HBLANK,\n> > > -\t\tV4L2_CID_PIXEL_RATE,\n> > > -\t};\n> > > -\n> > > -\tControlList ctrls = subdev_->getControls(mandatoryControls);\n> > > -\tif (ctrls.empty()) {\n> > > -\t\tLOG(CameraSensor, Error)\n> > > -\t\t\t<< \"Mandatory V4L2 controls not available\";\n> > > -\t\tLOG(CameraSensor, Error)\n> > > -\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> > > -\t\tLOG(CameraSensor, Error)\n> > > -\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > > -\t\treturn -EINVAL;\n> > > -\t}\n> > > -\n> > > \t/*\n> > > \t * Optional controls are used to register optional sensor properties. If\n> > > \t * not present, some values will be defaulted.\n> > > @@ -276,7 +255,7 @@ int CameraSensor::validateSensorDriver()\n> > > \t\tV4L2_CID_CAMERA_SENSOR_ROTATION,\n> > > \t};\n> > >\n> > > -\tctrls = subdev_->getControls(optionalControls);\n> > > +\tControlList ctrls = subdev_->getControls(optionalControls);\n> > > \tif (ctrls.empty())\n> > > \t\tLOG(CameraSensor, Debug) << \"Optional V4L2 controls not supported\";\n> > >\n> > > @@ -326,6 +305,30 @@ int CameraSensor::validateSensorDriver()\n> > > \t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > > \t}\n> > >\n> > > +\tif (!bayerFormat_)\n> > > +\t\treturn 0;\n> > > +\n> > > +\t/*\n> > > +\t * For raw sensors, make sure the sensor driver supports the controls\n> > > +\t * required by the CameraSensor class.\n> > > +\t */\n> > > +\tconst std::vector mandatoryControls{\n> > > +\t\tV4L2_CID_EXPOSURE,\n> > > +\t\tV4L2_CID_HBLANK,\n> > > +\t\tV4L2_CID_PIXEL_RATE,\n> > > +\t};\n> > > +\n> > > +\tctrls = subdev_->getControls(mandatoryControls);\n> > > +\tif (ctrls.empty()) {\n> > > +\t\tLOG(CameraSensor, Error)\n> > > +\t\t\t<< \"Mandatory V4L2 controls not available\";\n> > > +\t\tLOG(CameraSensor, Error)\n> > > +\t\t\t<< \"The sensor kernel driver needs to be fixed\";\n> > > +\t\tLOG(CameraSensor, Error)\n> > > +\t\t\t<< \"See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information\";\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > > +\n> > > \treturn 0;\n> > > }\n> > >\n> > > @@ -662,13 +665,21 @@ int CameraSensor::setControls(ControlList *ctrls)\n> > > * \\brief Assemble and return the camera sensor info\n> > > * \\param[out] info The camera sensor info\n> > > *\n> > > - * The CameraSensorInfo content is assembled by inspecting the currently\n> > > - * applied sensor configuration and the sensor static properties.\n> > > + * This function fills \\a info with information that describes the camera sensor\n> > > + * and its current configuration. The information combines static data (such as\n> > > + * the the sensor model or active pixel array size) and data specific to the\n> > > + * current sensor configuration (such as the line length and pixel rate).\n> > > + *\n> > > + * Sensor information is only available for raw sensors. When called for a YUV\n> > > + * sensor, this function returns -EINVAL.\n> > > *\n> > > * \\return 0 on success, a negative error code otherwise\n> > > */\n> > > int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > > {\n> > > +\tif (!bayerFormat_)\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > \tinfo->model = model();\n> > >\n> > > \t/*\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 9EE6BBD160\n\tfor ;\n\tThu, 4 Feb 2021 12:13:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 25B2561431;\n\tThu, 4 Feb 2021 13:13:25 +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 8DAA06140F\n\tfor ;\n\tThu, 4 Feb 2021 13:13:23 +0100 (CET)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 08E3DC0004;\n\tThu, 4 Feb 2021 12:13:22 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Thu, 4 Feb 2021 13:13:45 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210204121345.ezpdbuia7nkgk2sn@uno.localdomain>","References":"<20210131181722.5410-1-laurent.pinchart@ideasonboard.com>\n\t<20210131181722.5410-3-laurent.pinchart@ideasonboard.com>\n\t<20210201095212.5bceparnuwgzu54s@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor:\n\tRestrict sensor info to raw sensors","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\" "}}]