[{"id":14512,"web_url":"https://patchwork.libcamera.org/comment/14512/","msgid":"","date":"2021-01-11T00:23:16","subject":"Re: [libcamera-devel] [PATCH 02/12] libcamera: camera_sensor: Make\n\tV4L2_CID_EXPOSURE mandatory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Jan 05, 2021 at 08:05:12PM +0100, Jacopo Mondi wrote:\n> Add the V4L2_CID_EXPOSURE control to the list of mandatory controls the\n> sensor driver has to report and document this new requirement.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> Documentation/sensor_driver_requirements.rst | 11 ++++++++++-\n> src/libcamera/camera_sensor.cpp | 1 +\n> 2 files changed, 11 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\n> index 0e658aaa03b5..97e98b9e12ef 100644\n> --- a/Documentation/sensor_driver_requirements.rst\n> +++ b/Documentation/sensor_driver_requirements.rst\n> @@ -26,11 +26,20 @@ The sensor driver shall support the following V4L2 controls:\n> \n> * `V4L2_CID_HBLANK`_\n> * `V4L2_CID_PIXEL_RATE`_\n> +* `V4L2_CID_EXPOSURE`_\n> \n> .. _V4L2_CID_HBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html\n> .. _V4L2_CID_PIXEL_RATE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-process.html\n> +.. _V4L2_CID_EXPOSURE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html\n\nCould you keep these alphabetically sorted, here, below and in the code\n?\n\n> +\n> +The `HBLANK` and `PIXEL_RATE` controls are used to compute the sensor\n> +output timings.\n> +\n> +The `EXPOSURE` control shall report the image integration time in number of\n> +lines. The V4L2 documentation does not specify a unit for this control, drivers\n> +compliant with the V4L2 specification might need to be changed to be used by\n> +libcamera.\n\nI'm not sure why I would have gone for the opposite order (V4L2 first,\nthen libcamera) so you can ignore the following if you wish.\n\nWhile V4L2 doesn't specify a unit for the `EXPOSURE` control, libcamera requires\nit to be expressed as a number of image lines. Camera sensor drivers that do not\ncomply with this requirement will need to be adapted or will produce incorrect\nresults.\n\nReviewed-by: Laurent Pinchart \n\n> \n> -Both controls are used to compute the sensor output timings.\n> \n> Optional Requirements\n> ---------------------\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 05a1d7c22e97..f7939b94d2f8 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -245,6 +245,7 @@ int CameraSensor::validateSensorDriver()\n> \tconst std::vector mandatoryControls{\n> \t\tV4L2_CID_PIXEL_RATE,\n> \t\tV4L2_CID_HBLANK,\n> +\t\tV4L2_CID_EXPOSURE,\n> \t};\n> \n> \tControlList ctrls = subdev_->getControls(mandatoryControls);","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 46FC2BD80C\n\tfor ;\n\tMon, 11 Jan 2021 00:23:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D35B16808E;\n\tMon, 11 Jan 2021 01:23:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A5EE60317\n\tfor ;\n\tMon, 11 Jan 2021 01:23:32 +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 1B507EC;\n\tMon, 11 Jan 2021 01:23:30 +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=\"rLmzKu9X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610324610;\n\tbh=AfM8N5EjhJIDudnLWCVED03CDVimIykUtWcJVe5B5us=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rLmzKu9XMuQFQuDQO4sFkS3Nd64k8rum999hSoRsUVK0pYEfvZg9PTQBgXNyY4PGh\n\tl/5/+n/ZFZl1AL5khR4VLU/zh02RzobdvXphsIE84pbbnQADGRGFt8WuCCIuXp0PL3\n\tM0hyQtAFIfEr07MogSzTx1dyUbdCqHg6CK+EabwA=","Date":"Mon, 11 Jan 2021 02:23:16 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105190522.682324-1-jacopo@jmondi.org>\n\t<20210105190522.682324-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210105190522.682324-3-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 02/12] libcamera: camera_sensor: Make\n\tV4L2_CID_EXPOSURE mandatory","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":14580,"web_url":"https://patchwork.libcamera.org/comment/14580/","msgid":"","date":"2021-01-18T15:07:25","subject":"Re: [libcamera-devel] [PATCH 02/12] libcamera: camera_sensor: Make\n\tV4L2_CID_EXPOSURE mandatory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2021-01-11 02:23:16 +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 05, 2021 at 08:05:12PM +0100, Jacopo Mondi wrote:\n> > Add the V4L2_CID_EXPOSURE control to the list of mandatory controls the\n> > sensor driver has to report and document this new requirement.\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > Documentation/sensor_driver_requirements.rst | 11 ++++++++++-\n> > src/libcamera/camera_sensor.cpp | 1 +\n> > 2 files changed, 11 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/Documentation/sensor_driver_requirements.rst b/Documentation/sensor_driver_requirements.rst\n> > index 0e658aaa03b5..97e98b9e12ef 100644\n> > --- a/Documentation/sensor_driver_requirements.rst\n> > +++ b/Documentation/sensor_driver_requirements.rst\n> > @@ -26,11 +26,20 @@ The sensor driver shall support the following V4L2 controls:\n> > \n> > * `V4L2_CID_HBLANK`_\n> > * `V4L2_CID_PIXEL_RATE`_\n> > +* `V4L2_CID_EXPOSURE`_\n> > \n> > .. _V4L2_CID_HBLANK: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-source.html\n> > .. _V4L2_CID_PIXEL_RATE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-image-process.html\n> > +.. _V4L2_CID_EXPOSURE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html\n> \n> Could you keep these alphabetically sorted, here, below and in the code\n> ?\n> \n> > +\n> > +The `HBLANK` and `PIXEL_RATE` controls are used to compute the sensor\n> > +output timings.\n> > +\n> > +The `EXPOSURE` control shall report the image integration time in number of\n> > +lines. The V4L2 documentation does not specify a unit for this control, drivers\n> > +compliant with the V4L2 specification might need to be changed to be used by\n> > +libcamera.\n> \n> I'm not sure why I would have gone for the opposite order (V4L2 first,\n> then libcamera) so you can ignore the following if you wish.\n> \n> While V4L2 doesn't specify a unit for the `EXPOSURE` control, libcamera requires\n> it to be expressed as a number of image lines. Camera sensor drivers that do not\n> comply with this requirement will need to be adapted or will produce incorrect\n> results.\n> \n> Reviewed-by: Laurent Pinchart \n\nI like Laurent's suggestion, but with or without it used,\n\nReviewed-by: Niklas Söderlund \n\n> \n> > \n> > -Both controls are used to compute the sensor output timings.\n> > \n> > Optional Requirements\n> > ---------------------\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 05a1d7c22e97..f7939b94d2f8 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -245,6 +245,7 @@ int CameraSensor::validateSensorDriver()\n> > \tconst std::vector mandatoryControls{\n> > \t\tV4L2_CID_PIXEL_RATE,\n> > \t\tV4L2_CID_HBLANK,\n> > +\t\tV4L2_CID_EXPOSURE,\n> > \t};\n> > \n> > \tControlList ctrls = subdev_->getControls(mandatoryControls);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\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 C5D8EC0F1C\n\tfor ;\n\tMon, 18 Jan 2021 15:07:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26DEB68129;\n\tMon, 18 Jan 2021 16:07:29 +0100 (CET)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D8E46010B\n\tfor ;\n\tMon, 18 Jan 2021 16:07:27 +0100 (CET)","by mail-lj1-x230.google.com with SMTP id f17so18498775ljg.12\n\tfor ;\n\tMon, 18 Jan 2021 07:07:27 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\th23sm1722000ljh.115.2021.01.18.07.07.25\n\tfor \n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 18 Jan 2021 07:07:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"aJ2Sdjw8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=GS3DIdB/JLzXbWmlpBZxWH9LBdzgRU/QYCUVJ0mcLK4=;\n\tb=aJ2Sdjw8SyCvXoh3duYhJuBDsE2o2z4mD/TNmYWFUvh39wNPCz2Txb25u2RFYjrpF3\n\tHwxdGHGEVqoZic3N4XQ0ZMFT8oec5wBN2uzHqxK02WTT5sydTMdYPBb6nO3ANtlmMHoS\n\tbRh5Zkwm2SDJX3Y2hJ6yu+hzCP/F3vDX/0Zp1F6pbHi8I816MUBwoFgqPRwRsp4Ym34x\n\t/8MmvI482oIm3mY6fb98T5uF6ruaC+jBpnCP7gopHp/Y+7dfQHBKHry+4aMTuEsz2mWF\n\tn/G8nFiBVZkzQmm6K2lPWhmekKZDVuY8aJrI3An7/ZXHBnOC/4/WyjJ/vcB42aDSrKSO\n\tvQ0A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=GS3DIdB/JLzXbWmlpBZxWH9LBdzgRU/QYCUVJ0mcLK4=;\n\tb=bteGl/o+/Uh4UL2b6SZ2SILrHMVoR9Kg3RFoMa0X7NLZ8/N7w0fsEextCg1x+Q3LVM\n\tFClAz8kxcod2Vl0Twec7hSAJ7B7sM19O8WtOMIomL4GXg/5nX5/qIHs4vCX5ApYjtQT3\n\tT/t1mKj/RHeVCv5zCYKHNEmETyfrN29X7CF5t9uw2Ks/QeRN1/4mVVG8Q8IpvBTgVyY4\n\tA9sT+3j8wOMsnVweVgjAuyTi2vda2DyyJGX4kIgeoL1BBRJmR6spn8c7tnAlZoCgA1eh\n\t+T8NvMW5L5CRCvUl7TW4bdhngU3SVcJK/S0OBYJO401dD+nEYlwvUO8SKLwXcfW/I736\n\tocJQ==","X-Gm-Message-State":"AOAM530FzYeK5VkUwhWEn0wKf5hU2LUVMkhStVEW3WEGuzlI1MZbCP4e\n\tvy4rXvvumqNN9KpVucYxd4a2lqpj2ps8KQ==","X-Google-Smtp-Source":"ABdhPJyG0c9F0BApzVIePj34fGAgdvZpfCYN4r1fUfth+H3Uj9rTNzedboh+WC1UCjBtJMep0u0/OA==","X-Received":"by 2002:a2e:98cc:: with SMTP id s12mr45775ljj.325.1610982446790; \n\tMon, 18 Jan 2021 07:07:26 -0800 (PST)","Date":"Mon, 18 Jan 2021 16:07:25 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"","References":"<20210105190522.682324-1-jacopo@jmondi.org>\n\t<20210105190522.682324-3-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 02/12] libcamera: camera_sensor: Make\n\tV4L2_CID_EXPOSURE mandatory","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]