[{"id":11907,"web_url":"https://patchwork.libcamera.org/comment/11907/","msgid":"<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","date":"2020-08-06T08:45:17","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":64,"url":"https://patchwork.libcamera.org/api/people/64/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 05/08/2020 12:57, Jacopo Mondi wrote:\n> Provide a table to describe how the V4L2 selection targets can be used\n> to access an image sensor pixel array properties.\n> \n> Reference the table in the sub-device documentation.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> 2 files changed, 53 insertions(+)\n> \n> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> index c47861dff9b9b..2f7da3832f458 100644\n> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> can set the analog crop rectangle to select which portion of the pixel array\n> to read out.\n> \n> +A description of each of the above mentioned targets when used to access the\n> +image sensor pixel array properties is provided by\n> +:ref:`v4l2-selection-targets-image-sensor-table`\n> +\n> \n> Types of selection targets\n> --------------------------\n> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> index 69f500093aa2a..632e6448b784e 100644\n> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> \tmodified by hardware.\n> - Yes\n> - No\n> +\n> +\n> +.. _v4l2-selection-targets-image-sensor-table:\n> +\n> +********************************************\n> +Selection Targets For Pixel Array Properties\n> +********************************************\n> +\n> +The V4L2 selection API can be used to retrieve the size and disposition of the\n> +pixel units that compose and image sensor pixel matrix when applied to a video\n> +sub-device that represents an image sensor.\n> +\n> +A description of the properties associated with each of the sensor pixel array\n> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> +\n> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> +\n> +.. flat-table:: Selection target definitions\n> + :header-rows: 1\n> + :stub-columns: 0\n> +\n> + * - Target name\n> + - id\n> + - Definition\n> + - Read/Write\n> + * - ``V4L2_SEL_TGT_CROP``\n> + - 0x0000\n> + - The analog crop rectangle. Represents the portion of the active pixel\n> + array which is processed to produce images.\n> + - RW\n> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> + - 0x0001\n> + - The active pixel array rectangle. It includes only active pixels and\n> + excludes other ones such as optical black pixels. Its width and height\n> + represent the maximum image resolution an image sensor can produce.\n> + - RO\n> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> + - 0x0002\n> + - The readable portion of the physical pixel array matrix. It includes\n> + pixels that contains valid image data and calibration pixels such as the\n> + optical black ones.\n> + - RO\n> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> + - 0x0003\n> + - The physical pixel array size, including readable and not readable\n> + pixels. As pixels that cannot be read from application processor are not\n> + relevant for calibration purposes, this rectangle is useful to calculate\n> + the physical properties of the image sensor.\n> + - RO\n> \n\nHmm, this basically just duplicates the previous patch.\n\nI think you are documenting things at the wrong place. What you documented in the\nprevious patch really belongs here since it is shared between the subdev API and the\nregular V4L2 API. And in dev-subdev.rst you then refer to here.\n\nFrankly, the selection API documentation is a mess. It's spread out over various sections:\nThe VIDIOC_G/S_SELECTION and VIDIOC_SUBDEV_G/S_SELECTION descriptions in the Function Reference,\nsection 8 (\"Common definitions for V4L2 and V4L2 subdev interfaces\"), 1.25 \"Cropping, composing\nand scaling – the SELECTION API\" and 4.13.3.2-4.13.3.3 \"Selections: cropping, scaling and composition\".\n\nIn my view section 8 should be moved to section 1.25.2. Ideally 1.25 should be rewritten for both\nthe V4L2 and V4L2 subdev APIs, but that's a lot of work.\n\nI would suggest that you add a first patch that moves section 8 to 1.25.2. Note that I don't like\nthe tables for the selection targets and flags: the 'Valid for V4L2/V4L2 subdevs' columns should\nbe removed and it should either be mentioned in the definition if a target/flag is invalid for\nan API, or it should be put in a separate table.\n\nAnd in 1.25.2 perhaps a new picture can be created for the specific sensor use-case that includes\nthe NATIVE_SIZE target.\n\nAnother pet peeve of mine is that section 8 splits the selection targets and flags into separate\nsubsections. I'd just keep it in one section.\n\nRegards,\n\n\tHans","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 334C9BD87A\n\tfor ;\n\tThu, 6 Aug 2020 08:45:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC5E26073B;\n\tThu, 6 Aug 2020 10:45:19 +0200 (CEST)","from lb3-smtp-cloud9.xs4all.net (lb3-smtp-cloud9.xs4all.net\n\t[194.109.24.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CBCD60390\n\tfor ;\n\tThu, 6 Aug 2020 10:45:18 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid 3bWXk7SPcuuXO3bWYknW9h; Thu, 06 Aug 2020 10:45:18 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"sQeqY0wl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1596703518; bh=S6mVY9Ie7xZPkb4dHa8bBC9Ou42RNWhyhX7m+9bBbPE=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=sQeqY0wlxAxbnhZeUzlIvL8CE05m+6q3KE3I3BSDbtPjD3hXbPajjtYqlsB7ivJUx\n\t0G+sq7bHYyt7yVWCdYxlZaGhq+pfEEyMKXY2rgV9Z7WDloXH7cgu7KzG5LAARtHg7e\n\tZMLQmGQWS/afPgRZxN2RrqDas50bS6TI820fAE6YyXQz05lo2vATOD0akYy2mOI6Oc\n\tf8sdflD+pJkjDR+TS9wJOmW96In9vLKTZF5OA9/zBWZnoNpmt6eIHjXrXMwzo+JS63\n\tqIDiK5pKhgif88uAQ8TtYb0tiiA7SoXHYEYpS2gVXEytK1KMDl3iWj00dNxUV3fCtz\n\tVGbVqk6dX+eRg==","To":"Jacopo Mondi ,\n\tSakari Ailus ,\n\tLaurent Pinchart ","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>","From":"Hans Verkuil ","Message-ID":"<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","Date":"Thu, 6 Aug 2020 10:45:17 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200805105721.15445-3-jacopo@jmondi.org>","Content-Language":"en-US","X-CMAE-Envelope":"MS4wfAYPW7M0u30eSlSWBanj1unuI0nDBAysDbAvnhGTsEL8oBy20GtlLUvh3fEUJ4NRNn3KZgFgEJEUpiAKBgvPKncr2QWTaRE6ERS763psWJ3JEcsMawLY\n\t8N4FNPgeWoL8Yxc7WsdrBxMKgspPFcgKlgmodgsy6kOe6v1FKi5e9m+ILQwgvSBUJFpNc/U66Q0SY3w0/WMQ4rG8c7rokzfpNPiNcglSqr4H4L0l+rNidRmx\n\t1PqiQwFqaK3wHThnksYuNBRmZXohZwzUvz5Tq61X6SZc0RcB00ctPFdI9AN9im0Y0qbp+T2kNn9+oK+kwAzNzH7D8E4iT9K4EAdg/UBvjMChvvWS1QrMN3rP\n\tbQ7gkjKMKzfF5/emXp4rGjcpVzo9IWlWuLFk0hqcemig/bouBfiOnlACD0NaX+IDPhwd/z2EJ2l81ClG8ajRC2gSibg8UsdYQM9AvAn6nWTafoqopLV5qCTs\n\tQi1XpJBNRDFE0Q16YrG3ljM84IgoK4z+KSqDyuAIqPD+d1Kv4QA4grlFwZo=","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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":"Ricardo Ribalda Delgado ,\n\tlibcamera-devel@lists.libcamera.org,\n\tSowjanya Komatineni ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11911,"web_url":"https://patchwork.libcamera.org/comment/11911/","msgid":"<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>","date":"2020-08-06T10:08:22","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n\nOn Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> > Provide a table to describe how the V4L2 selection targets can be used\n> > to access an image sensor pixel array properties.\n> >\n> > Reference the table in the sub-device documentation.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> > .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> > 2 files changed, 53 insertions(+)\n> >\n> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > index c47861dff9b9b..2f7da3832f458 100644\n> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> > can set the analog crop rectangle to select which portion of the pixel array\n> > to read out.\n> >\n> > +A description of each of the above mentioned targets when used to access the\n> > +image sensor pixel array properties is provided by\n> > +:ref:`v4l2-selection-targets-image-sensor-table`\n> > +\n> >\n> > Types of selection targets\n> > --------------------------\n> > diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > index 69f500093aa2a..632e6448b784e 100644\n> > --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> > \tmodified by hardware.\n> > - Yes\n> > - No\n> > +\n> > +\n> > +.. _v4l2-selection-targets-image-sensor-table:\n> > +\n> > +********************************************\n> > +Selection Targets For Pixel Array Properties\n> > +********************************************\n> > +\n> > +The V4L2 selection API can be used to retrieve the size and disposition of the\n> > +pixel units that compose and image sensor pixel matrix when applied to a video\n> > +sub-device that represents an image sensor.\n> > +\n> > +A description of the properties associated with each of the sensor pixel array\n> > +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> > +\n> > +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> > +\n> > +.. flat-table:: Selection target definitions\n> > + :header-rows: 1\n> > + :stub-columns: 0\n> > +\n> > + * - Target name\n> > + - id\n> > + - Definition\n> > + - Read/Write\n> > + * - ``V4L2_SEL_TGT_CROP``\n> > + - 0x0000\n> > + - The analog crop rectangle. Represents the portion of the active pixel\n> > + array which is processed to produce images.\n> > + - RW\n> > + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> > + - 0x0001\n> > + - The active pixel array rectangle. It includes only active pixels and\n> > + excludes other ones such as optical black pixels. Its width and height\n> > + represent the maximum image resolution an image sensor can produce.\n> > + - RO\n> > + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> > + - 0x0002\n> > + - The readable portion of the physical pixel array matrix. It includes\n> > + pixels that contains valid image data and calibration pixels such as the\n> > + optical black ones.\n> > + - RO\n> > + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> > + - 0x0003\n> > + - The physical pixel array size, including readable and not readable\n> > + pixels. As pixels that cannot be read from application processor are not\n> > + relevant for calibration purposes, this rectangle is useful to calculate\n> > + the physical properties of the image sensor.\n> > + - RO\n> >\n>\n> Hmm, this basically just duplicates the previous patch.\n>\n> I think you are documenting things at the wrong place. What you documented in the\n> previous patch really belongs here since it is shared between the subdev API and the\n> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n\nI originally had it here, but then I moved to dev-subdev as an image\nsensor will always be represented as a video sub-device, doen't it ?\n\n>\n> Frankly, the selection API documentation is a mess. It's spread out over various sections:\n\nI know :(\n\n> The VIDIOC_G/S_SELECTION and VIDIOC_SUBDEV_G/S_SELECTION descriptions in the Function Reference,\n> section 8 (\"Common definitions for V4L2 and V4L2 subdev interfaces\"), 1.25 \"Cropping, composing\n> and scaling – the SELECTION API\" and 4.13.3.2-4.13.3.3 \"Selections: cropping, scaling and composition\".\n>\n> In my view section 8 should be moved to section 1.25.2. Ideally 1.25 should be rewritten for both\n> the V4L2 and V4L2 subdev APIs, but that's a lot of work.\n\nThat's a lot of work indeed. But it could be split as\n\n1) Augment the 1.25.1 section with a mention of subdev, maybe using\n pieces of 4.13.3.2\n2) Move what's in section 8 to be 1.25.x\n Actually merging 1.25.2 and 8.1.1, splitting 1.25.2 in a device and\n sub-device section\n>\n> I would suggest that you add a first patch that moves section 8 to 1.25.2. Note that I don't like\n> the tables for the selection targets and flags: the 'Valid for V4L2/V4L2 subdevs' columns should\n> be removed and it should either be mentioned in the definition if a target/flag is invalid for\n> an API, or it should be put in a separate table.\n\nTwo tables seems about right\n\n>\n> And in 1.25.2 perhaps a new picture can be created for the specific sensor use-case that includes\n> the NATIVE_SIZE target.\n\nI know 'image sensor' is not an API concept like device and subdevice,\nbut if we split 1.25.2 in 'video device' and 'subdevice' parts, a\nsection for 'image sensors' with what I've now put in section 4.13.3.3\n(the description of the sensor array areas) could fit there ?\n\nSeems like a long re-work. Can the imx219 patch be broke out and\nfast-tracked ?\n\nThanks\n j\n\n>\n> Another pet peeve of mine is that section 8 splits the selection targets and flags into separate\n> subsections. I'd just keep it in one section.\n>\n> Regards,\n>\n> \tHans","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 A697EBD87A\n\tfor ;\n\tThu, 6 Aug 2020 10:04:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18E9660730;\n\tThu, 6 Aug 2020 12:04:47 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D1E060554\n\tfor ;\n\tThu, 6 Aug 2020 12:04:45 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id BA2412000F;\n\tThu, 6 Aug 2020 10:04:42 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 6 Aug 2020 12:08:22 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Message-ID":"<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11913,"web_url":"https://patchwork.libcamera.org/comment/11913/","msgid":"<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>","date":"2020-08-06T10:15:49","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":64,"url":"https://patchwork.libcamera.org/api/people/64/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 06/08/2020 12:08, Jacopo Mondi wrote:\n> Hi Hans,\n> \n> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n>>> Provide a table to describe how the V4L2 selection targets can be used\n>>> to access an image sensor pixel array properties.\n>>>\n>>> Reference the table in the sub-device documentation.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n>>> 2 files changed, 53 insertions(+)\n>>>\n>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>> index c47861dff9b9b..2f7da3832f458 100644\n>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n>>> can set the analog crop rectangle to select which portion of the pixel array\n>>> to read out.\n>>>\n>>> +A description of each of the above mentioned targets when used to access the\n>>> +image sensor pixel array properties is provided by\n>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n>>> +\n>>>\n>>> Types of selection targets\n>>> --------------------------\n>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>> index 69f500093aa2a..632e6448b784e 100644\n>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n>>> \tmodified by hardware.\n>>> - Yes\n>>> - No\n>>> +\n>>> +\n>>> +.. _v4l2-selection-targets-image-sensor-table:\n>>> +\n>>> +********************************************\n>>> +Selection Targets For Pixel Array Properties\n>>> +********************************************\n>>> +\n>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n>>> +sub-device that represents an image sensor.\n>>> +\n>>> +A description of the properties associated with each of the sensor pixel array\n>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n>>> +\n>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n>>> +\n>>> +.. flat-table:: Selection target definitions\n>>> + :header-rows: 1\n>>> + :stub-columns: 0\n>>> +\n>>> + * - Target name\n>>> + - id\n>>> + - Definition\n>>> + - Read/Write\n>>> + * - ``V4L2_SEL_TGT_CROP``\n>>> + - 0x0000\n>>> + - The analog crop rectangle. Represents the portion of the active pixel\n>>> + array which is processed to produce images.\n>>> + - RW\n>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n>>> + - 0x0001\n>>> + - The active pixel array rectangle. It includes only active pixels and\n>>> + excludes other ones such as optical black pixels. Its width and height\n>>> + represent the maximum image resolution an image sensor can produce.\n>>> + - RO\n>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n>>> + - 0x0002\n>>> + - The readable portion of the physical pixel array matrix. It includes\n>>> + pixels that contains valid image data and calibration pixels such as the\n>>> + optical black ones.\n>>> + - RO\n>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n>>> + - 0x0003\n>>> + - The physical pixel array size, including readable and not readable\n>>> + pixels. As pixels that cannot be read from application processor are not\n>>> + relevant for calibration purposes, this rectangle is useful to calculate\n>>> + the physical properties of the image sensor.\n>>> + - RO\n>>>\n>>\n>> Hmm, this basically just duplicates the previous patch.\n>>\n>> I think you are documenting things at the wrong place. What you documented in the\n>> previous patch really belongs here since it is shared between the subdev API and the\n>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> \n> I originally had it here, but then I moved to dev-subdev as an image\n> sensor will always be represented as a video sub-device, doen't it ?\n\nNo. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\nplatform drivers that don't use the subdev API.\n\n> \n>>\n>> Frankly, the selection API documentation is a mess. It's spread out over various sections:\n> \n> I know :(\n> \n>> The VIDIOC_G/S_SELECTION and VIDIOC_SUBDEV_G/S_SELECTION descriptions in the Function Reference,\n>> section 8 (\"Common definitions for V4L2 and V4L2 subdev interfaces\"), 1.25 \"Cropping, composing\n>> and scaling – the SELECTION API\" and 4.13.3.2-4.13.3.3 \"Selections: cropping, scaling and composition\".\n>>\n>> In my view section 8 should be moved to section 1.25.2. Ideally 1.25 should be rewritten for both\n>> the V4L2 and V4L2 subdev APIs, but that's a lot of work.\n> \n> That's a lot of work indeed. But it could be split as\n> \n> 1) Augment the 1.25.1 section with a mention of subdev, maybe using\n> pieces of 4.13.3.2\n> 2) Move what's in section 8 to be 1.25.x\n> Actually merging 1.25.2 and 8.1.1, splitting 1.25.2 in a device and\n> sub-device section\n>>\n>> I would suggest that you add a first patch that moves section 8 to 1.25.2. Note that I don't like\n>> the tables for the selection targets and flags: the 'Valid for V4L2/V4L2 subdevs' columns should\n>> be removed and it should either be mentioned in the definition if a target/flag is invalid for\n>> an API, or it should be put in a separate table.\n> \n> Two tables seems about right\n> \n>>\n>> And in 1.25.2 perhaps a new picture can be created for the specific sensor use-case that includes\n>> the NATIVE_SIZE target.\n> \n> I know 'image sensor' is not an API concept like device and subdevice,\n> but if we split 1.25.2 in 'video device' and 'subdevice' parts, a\n> section for 'image sensors' with what I've now put in section 4.13.3.3\n> (the description of the sensor array areas) could fit there ?\n\nI think so.\n\n> \n> Seems like a long re-work. Can the imx219 patch be broke out and\n> fast-tracked ?\n\nYes.\n\nRegards,\n\n\tHans\n\n> \n> Thanks\n> j\n> \n>>\n>> Another pet peeve of mine is that section 8 splits the selection targets and flags into separate\n>> subsections. I'd just keep it in one section.\n>>\n>> Regards,\n>>\n>> \tHans","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 06873BD87A\n\tfor ;\n\tThu, 6 Aug 2020 10:15:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91BA360730;\n\tThu, 6 Aug 2020 12:15:52 +0200 (CEST)","from lb1-smtp-cloud7.xs4all.net (lb1-smtp-cloud7.xs4all.net\n\t[194.109.24.24])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B7B060554\n\tfor ;\n\tThu, 6 Aug 2020 12:15:51 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid 3cw9kgel6ywL53cwAk44Pu; Thu, 06 Aug 2020 12:15:50 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"MUbGgnP6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1596708950; bh=gNLyASUNByD3HO6tVw8Df9yaWFza/lOZVkhsrcazWZE=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=MUbGgnP6qOK0DUanWJ6mS+wyqcw+m2uyg2/UoQ/rDWqe2k/yyQmkLIFQR5YVe/FQm\n\tdiN/h+Y2+cLpYNqyIxjGJcb2AwNqh6y8ji3rpgtsIr66T7ndI07ar+6pYSjTuXaqjy\n\tBFWSDegel3y5VywdSXNQXY3coWmPXWLP515hBlzs8LbEzhNOqFbot0jjDFIA7ro0tJ\n\tMStLT1BDTTrmH+83lEcexMcpNyU0yvw5JsS1v0t+IAkR/5E9/PILdoOHAimMSoYpZ4\n\tdahtm5agbZXlePIzH2tKYx3A1WbJIDVMKH7xbc3vxE1KkE+7wHM7xFZcP1/EiQ2kjG\n\t57q0rNA7yYFYw==","To":"Jacopo Mondi ","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>","From":"Hans Verkuil ","Message-ID":"<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>","Date":"Thu, 6 Aug 2020 12:15:49 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>","Content-Language":"en-US","X-CMAE-Envelope":"MS4wfFDgt0kq2D3IT0EjvVdOm5VtmDAHmvuKxFs/+PvvqSYR/7rj+4dTyGp8H4Dl7i/pAJOFEJ/Ni+id5+l9MtFC5Gykn+vAit86U10GRBdzbzl+D7eQjgeB\n\tQk8rKiNTtrvRxXvAKteJmLVwBGuPxNqecwGCEg6LuyKOpragdloUWQdymQlpi3UocqfcXdYxoTBKSHKZsogIUdH9EgVZd2LPuJvOuqQgY+4cdxtlTUXMCPe+\n\tnwVXdJTlbBR2RYHHhyN2KQ3JA0ylVxO2oPuqMkVzHb8vtbxehuhCsabSgi5dwLmqszUl1jXG1jBkWfTG7oWGB5qCX/CdFK2SLy0HLFa2APhdkZKH+KJjrRS3\n\taR2QT99c5go6Yaqqzpzj02Lzl2+2kqcN99/bZJTssP57QZ5KEiEAoOqM3iL1YMeYLo7Bz6zWAJIYAic7pP5FQOwMhg0bSLBUh1QdhbjzDqp++E2u4Q7kw5h0\n\twlsLvebNFMP8XvGfQbFCIQTz9H75soP8JeumVZro2gch1BL0KItkLqZIwWY=","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11919,"web_url":"https://patchwork.libcamera.org/comment/11919/","msgid":"<20200806124523.llccxvcz65ohqqwq@uno.localdomain>","date":"2020-08-06T12:45:23","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n\nOn Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n> On 06/08/2020 12:08, Jacopo Mondi wrote:\n> > Hi Hans,\n> >\n> > On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> >> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> >>> Provide a table to describe how the V4L2 selection targets can be used\n> >>> to access an image sensor pixel array properties.\n> >>>\n> >>> Reference the table in the sub-device documentation.\n> >>>\n> >>> Signed-off-by: Jacopo Mondi \n> >>> ---\n> >>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> >>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> >>> 2 files changed, 53 insertions(+)\n> >>>\n> >>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>> index c47861dff9b9b..2f7da3832f458 100644\n> >>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> >>> can set the analog crop rectangle to select which portion of the pixel array\n> >>> to read out.\n> >>>\n> >>> +A description of each of the above mentioned targets when used to access the\n> >>> +image sensor pixel array properties is provided by\n> >>> +:ref:`v4l2-selection-targets-image-sensor-table`\n> >>> +\n> >>>\n> >>> Types of selection targets\n> >>> --------------------------\n> >>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>> index 69f500093aa2a..632e6448b784e 100644\n> >>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> >>> \tmodified by hardware.\n> >>> - Yes\n> >>> - No\n> >>> +\n> >>> +\n> >>> +.. _v4l2-selection-targets-image-sensor-table:\n> >>> +\n> >>> +********************************************\n> >>> +Selection Targets For Pixel Array Properties\n> >>> +********************************************\n> >>> +\n> >>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n> >>> +pixel units that compose and image sensor pixel matrix when applied to a video\n> >>> +sub-device that represents an image sensor.\n> >>> +\n> >>> +A description of the properties associated with each of the sensor pixel array\n> >>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> >>> +\n> >>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> >>> +\n> >>> +.. flat-table:: Selection target definitions\n> >>> + :header-rows: 1\n> >>> + :stub-columns: 0\n> >>> +\n> >>> + * - Target name\n> >>> + - id\n> >>> + - Definition\n> >>> + - Read/Write\n> >>> + * - ``V4L2_SEL_TGT_CROP``\n> >>> + - 0x0000\n> >>> + - The analog crop rectangle. Represents the portion of the active pixel\n> >>> + array which is processed to produce images.\n> >>> + - RW\n> >>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> >>> + - 0x0001\n> >>> + - The active pixel array rectangle. It includes only active pixels and\n> >>> + excludes other ones such as optical black pixels. Its width and height\n> >>> + represent the maximum image resolution an image sensor can produce.\n> >>> + - RO\n> >>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> >>> + - 0x0002\n> >>> + - The readable portion of the physical pixel array matrix. It includes\n> >>> + pixels that contains valid image data and calibration pixels such as the\n> >>> + optical black ones.\n> >>> + - RO\n> >>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> >>> + - 0x0003\n> >>> + - The physical pixel array size, including readable and not readable\n> >>> + pixels. As pixels that cannot be read from application processor are not\n> >>> + relevant for calibration purposes, this rectangle is useful to calculate\n> >>> + the physical properties of the image sensor.\n> >>> + - RO\n> >>>\n> >>\n> >> Hmm, this basically just duplicates the previous patch.\n> >>\n> >> I think you are documenting things at the wrong place. What you documented in the\n> >> previous patch really belongs here since it is shared between the subdev API and the\n> >> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> >\n> > I originally had it here, but then I moved to dev-subdev as an image\n> > sensor will always be represented as a video sub-device, doen't it ?\n>\n> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n> platform drivers that don't use the subdev API.\n\nDo we expect to be able to retrieve sensor array properties from video\ndevice nodes which represents, in my understanding a DMA engine that\nwrites data to memory ? As I see it, not subdev for the image sensor,\nno pixel array properties. How can these be exposed by a video device\nwhich abstracts the full capture pipeline ?","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 670DCBD86F\n\tfor ;\n\tThu, 6 Aug 2020 12:41:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF65960730;\n\tThu, 6 Aug 2020 14:41:46 +0200 (CEST)","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 9160560554\n\tfor ;\n\tThu, 6 Aug 2020 14:41:45 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id CB257C0003;\n\tThu, 6 Aug 2020 12:41:43 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 6 Aug 2020 14:45:23 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Message-ID":"<20200806124523.llccxvcz65ohqqwq@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11923,"web_url":"https://patchwork.libcamera.org/comment/11923/","msgid":"","date":"2020-08-06T13:15:54","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":64,"url":"https://patchwork.libcamera.org/api/people/64/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 06/08/2020 14:45, Jacopo Mondi wrote:\n> Hi Hans,\n> \n> On Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n>> On 06/08/2020 12:08, Jacopo Mondi wrote:\n>>> Hi Hans,\n>>>\n>>> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n>>>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n>>>>> Provide a table to describe how the V4L2 selection targets can be used\n>>>>> to access an image sensor pixel array properties.\n>>>>>\n>>>>> Reference the table in the sub-device documentation.\n>>>>>\n>>>>> Signed-off-by: Jacopo Mondi \n>>>>> ---\n>>>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n>>>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n>>>>> 2 files changed, 53 insertions(+)\n>>>>>\n>>>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>> index c47861dff9b9b..2f7da3832f458 100644\n>>>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n>>>>> can set the analog crop rectangle to select which portion of the pixel array\n>>>>> to read out.\n>>>>>\n>>>>> +A description of each of the above mentioned targets when used to access the\n>>>>> +image sensor pixel array properties is provided by\n>>>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n>>>>> +\n>>>>>\n>>>>> Types of selection targets\n>>>>> --------------------------\n>>>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>> index 69f500093aa2a..632e6448b784e 100644\n>>>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n>>>>> \tmodified by hardware.\n>>>>> - Yes\n>>>>> - No\n>>>>> +\n>>>>> +\n>>>>> +.. _v4l2-selection-targets-image-sensor-table:\n>>>>> +\n>>>>> +********************************************\n>>>>> +Selection Targets For Pixel Array Properties\n>>>>> +********************************************\n>>>>> +\n>>>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n>>>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n>>>>> +sub-device that represents an image sensor.\n>>>>> +\n>>>>> +A description of the properties associated with each of the sensor pixel array\n>>>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n>>>>> +\n>>>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n>>>>> +\n>>>>> +.. flat-table:: Selection target definitions\n>>>>> + :header-rows: 1\n>>>>> + :stub-columns: 0\n>>>>> +\n>>>>> + * - Target name\n>>>>> + - id\n>>>>> + - Definition\n>>>>> + - Read/Write\n>>>>> + * - ``V4L2_SEL_TGT_CROP``\n>>>>> + - 0x0000\n>>>>> + - The analog crop rectangle. Represents the portion of the active pixel\n>>>>> + array which is processed to produce images.\n>>>>> + - RW\n>>>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n>>>>> + - 0x0001\n>>>>> + - The active pixel array rectangle. It includes only active pixels and\n>>>>> + excludes other ones such as optical black pixels. Its width and height\n>>>>> + represent the maximum image resolution an image sensor can produce.\n>>>>> + - RO\n>>>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n>>>>> + - 0x0002\n>>>>> + - The readable portion of the physical pixel array matrix. It includes\n>>>>> + pixels that contains valid image data and calibration pixels such as the\n>>>>> + optical black ones.\n>>>>> + - RO\n>>>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n>>>>> + - 0x0003\n>>>>> + - The physical pixel array size, including readable and not readable\n>>>>> + pixels. As pixels that cannot be read from application processor are not\n>>>>> + relevant for calibration purposes, this rectangle is useful to calculate\n>>>>> + the physical properties of the image sensor.\n>>>>> + - RO\n>>>>>\n>>>>\n>>>> Hmm, this basically just duplicates the previous patch.\n>>>>\n>>>> I think you are documenting things at the wrong place. What you documented in the\n>>>> previous patch really belongs here since it is shared between the subdev API and the\n>>>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n>>>\n>>> I originally had it here, but then I moved to dev-subdev as an image\n>>> sensor will always be represented as a video sub-device, doen't it ?\n>>\n>> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n>> platform drivers that don't use the subdev API.\n> \n> Do we expect to be able to retrieve sensor array properties from video\n> device nodes which represents, in my understanding a DMA engine that\n> writes data to memory ? As I see it, not subdev for the image sensor,\n> no pixel array properties. How can these be exposed by a video device\n> which abstracts the full capture pipeline ?\n\nThey will typically ask the subdev driver. The vidioc_g_selection op\nimplementation will in turn call the get_selection op of the sensor subdev\ndriver and pass that information back to userspace.\n\nThere is nothing subdev specific to this API.\n\nRegards,\n\n\tHans","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 5C4B7BD86F\n\tfor ;\n\tThu, 6 Aug 2020 13:15:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C18FE60836;\n\tThu, 6 Aug 2020 15:15:57 +0200 (CEST)","from lb3-smtp-cloud7.xs4all.net (lb3-smtp-cloud7.xs4all.net\n\t[194.109.24.31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CEDC86038F\n\tfor ;\n\tThu, 6 Aug 2020 15:15:55 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid 3fkQkhdO1ywL53fkRk4Xfb; Thu, 06 Aug 2020 15:15:55 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"gKZZqyor\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1596719755; bh=wGB2xCe1fjPvlGuz/wKVKuCa5euw35OniJ/tqt+nDxw=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=gKZZqyorAgaoRODpsGKov1BYoRENjf9zTfCcsCqbx04HLMWvcOc7b0uQmW8u8aAN8\n\t01NaThFY72VwUtEdh3zoi0BD/uHMH1lL4bm54e77eDqq4QrTxrOtoHEg8SSp9b6DPY\n\tk938e8faicytENHmYVMEi4051cEuVwz8iSX4hWudxn32sZZCqZ5f+6B0Adm3kAhWOE\n\tRKBImdsiG/Xu/mFpCJoDVuMyNQFSMZM8fUIvDfMYZwgCOjI1FJ3Xz4eYqxil9BbOdU\n\tcMLM4Bm674OPKGi+c03m8B+FLFZ7LKnbO9ZFLrtD76Tj/qRD/pDz0jgw70nNFktV+D\n\tuYWjI4jb1tz9w==","To":"Jacopo Mondi ","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>\n\t<20200806124523.llccxvcz65ohqqwq@uno.localdomain>","From":"Hans Verkuil ","Message-ID":"","Date":"Thu, 6 Aug 2020 15:15:54 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200806124523.llccxvcz65ohqqwq@uno.localdomain>","Content-Language":"en-US","X-CMAE-Envelope":"MS4wfGzgtmk1JexUg2VEwGo5asrsXI0A7wQlNuAQks8si48bY8g40GvwMvFjPBSsRh15Q575WuvBfqdV0C37WShNg1Ss0nl+aKIxy6fyRmh70nTbFlFqOSA3\n\tRnHJpHT//SQ9OUUKFnOZcN5JzOrkSWpKhWqRQquLA+THMWGGrinDPMn0iOust2FBTBK4fuKV5CUpn1IAcrdHfo5VSUcUSaXYzFevW2qR8WOSZ17Pz47YkHh2\n\tiljJkX2S81HT2WNwvZnWfrymcim3P34wSbCd7p7xXUBTgEailvpSX2786S6frNQtkkbPBpbcm860mZRVWS47ff9bb211p4gAGZgxQtIJZHqZKPp125R4OsTe\n\tIQLy86T1V/5tjlIaERg/so1P17AG7pHdAAxZCG1nksKCLwkycbL+ANJ8+wVJuYVC/FAjjS8E3VYtNxcF5rQ8TMsiNkSm1dzozw1I+AXqBDvSvp0A7T/i4OQy\n\tL2r1gq/futoW36BexD5XHL6yf5Yi5gCL23/ZkxRdxnYvt5pkXVM6QB7X37I=","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11927,"web_url":"https://patchwork.libcamera.org/comment/11927/","msgid":"<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>","date":"2020-08-06T13:36:17","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n I'm sorry, I don't want to insist but..\n\nOn Thu, Aug 06, 2020 at 03:15:54PM +0200, Hans Verkuil wrote:\n> On 06/08/2020 14:45, Jacopo Mondi wrote:\n> > Hi Hans,\n> >\n> > On Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n> >> On 06/08/2020 12:08, Jacopo Mondi wrote:\n> >>> Hi Hans,\n> >>>\n> >>> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> >>>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> >>>>> Provide a table to describe how the V4L2 selection targets can be used\n> >>>>> to access an image sensor pixel array properties.\n> >>>>>\n> >>>>> Reference the table in the sub-device documentation.\n> >>>>>\n> >>>>> Signed-off-by: Jacopo Mondi \n> >>>>> ---\n> >>>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> >>>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> >>>>> 2 files changed, 53 insertions(+)\n> >>>>>\n> >>>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>> index c47861dff9b9b..2f7da3832f458 100644\n> >>>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> >>>>> can set the analog crop rectangle to select which portion of the pixel array\n> >>>>> to read out.\n> >>>>>\n> >>>>> +A description of each of the above mentioned targets when used to access the\n> >>>>> +image sensor pixel array properties is provided by\n> >>>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n> >>>>> +\n> >>>>>\n> >>>>> Types of selection targets\n> >>>>> --------------------------\n> >>>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>> index 69f500093aa2a..632e6448b784e 100644\n> >>>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> >>>>> \tmodified by hardware.\n> >>>>> - Yes\n> >>>>> - No\n> >>>>> +\n> >>>>> +\n> >>>>> +.. _v4l2-selection-targets-image-sensor-table:\n> >>>>> +\n> >>>>> +********************************************\n> >>>>> +Selection Targets For Pixel Array Properties\n> >>>>> +********************************************\n> >>>>> +\n> >>>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n> >>>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n> >>>>> +sub-device that represents an image sensor.\n> >>>>> +\n> >>>>> +A description of the properties associated with each of the sensor pixel array\n> >>>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> >>>>> +\n> >>>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> >>>>> +\n> >>>>> +.. flat-table:: Selection target definitions\n> >>>>> + :header-rows: 1\n> >>>>> + :stub-columns: 0\n> >>>>> +\n> >>>>> + * - Target name\n> >>>>> + - id\n> >>>>> + - Definition\n> >>>>> + - Read/Write\n> >>>>> + * - ``V4L2_SEL_TGT_CROP``\n> >>>>> + - 0x0000\n> >>>>> + - The analog crop rectangle. Represents the portion of the active pixel\n> >>>>> + array which is processed to produce images.\n> >>>>> + - RW\n> >>>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> >>>>> + - 0x0001\n> >>>>> + - The active pixel array rectangle. It includes only active pixels and\n> >>>>> + excludes other ones such as optical black pixels. Its width and height\n> >>>>> + represent the maximum image resolution an image sensor can produce.\n> >>>>> + - RO\n> >>>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> >>>>> + - 0x0002\n> >>>>> + - The readable portion of the physical pixel array matrix. It includes\n> >>>>> + pixels that contains valid image data and calibration pixels such as the\n> >>>>> + optical black ones.\n> >>>>> + - RO\n> >>>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> >>>>> + - 0x0003\n> >>>>> + - The physical pixel array size, including readable and not readable\n> >>>>> + pixels. As pixels that cannot be read from application processor are not\n> >>>>> + relevant for calibration purposes, this rectangle is useful to calculate\n> >>>>> + the physical properties of the image sensor.\n> >>>>> + - RO\n> >>>>>\n> >>>>\n> >>>> Hmm, this basically just duplicates the previous patch.\n> >>>>\n> >>>> I think you are documenting things at the wrong place. What you documented in the\n> >>>> previous patch really belongs here since it is shared between the subdev API and the\n> >>>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> >>>\n> >>> I originally had it here, but then I moved to dev-subdev as an image\n> >>> sensor will always be represented as a video sub-device, doen't it ?\n> >>\n> >> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n> >> platform drivers that don't use the subdev API.\n> >\n> > Do we expect to be able to retrieve sensor array properties from video\n> > device nodes which represents, in my understanding a DMA engine that\n> > writes data to memory ? As I see it, not subdev for the image sensor,\n> > no pixel array properties. How can these be exposed by a video device\n> > which abstracts the full capture pipeline ?\n>\n> They will typically ask the subdev driver. The vidioc_g_selection op\n> implementation will in turn call the get_selection op of the sensor subdev\n> driver and pass that information back to userspace.\n\nHow do we know that the any target reports the actual sensor\nproperties and not some other limitation introduced by processing\ncomponents down the pipeline, as everything sits behind a single video\ndevice node ? In example, the default crop rectangle might very well depend\non the receiver's side decision to crop the frames received from the\nsensor to maximize the FOV or whatnot. How do we know which 'cropping\npoint' is reported ?\n\nI hardly see a vidio_g_selection() being ideal to report properties of\nthe camera sensor such as the pixel array ones. I still feel we're\nhijacking that API for that purpose and I would be glad to have\ndedicated targets for image sensors. This would promote 'image\nsensors' as first class citizens of the API like devices and\nsub-devices, and I'm not sure this is desirable.\n\n>\n> There is nothing subdev specific to this API.\n>\n> Regards,\n>\n> \tHans","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 E8798BD86F\n\tfor ;\n\tThu, 6 Aug 2020 13:32:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 759C36071B;\n\tThu, 6 Aug 2020 15:32:40 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E84E60552\n\tfor ;\n\tThu, 6 Aug 2020 15:32:39 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 2AA7460009;\n\tThu, 6 Aug 2020 13:32:36 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 6 Aug 2020 15:36:17 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Message-ID":"<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>\n\t<20200806124523.llccxvcz65ohqqwq@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11938,"web_url":"https://patchwork.libcamera.org/comment/11938/","msgid":"<6a349665-8342-4f3b-d9b4-8d49a1da7d95@xs4all.nl>","date":"2020-08-06T15:32:50","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":64,"url":"https://patchwork.libcamera.org/api/people/64/","name":"Hans Verkuil","email":"hverkuil@xs4all.nl"},"content":"On 06/08/2020 15:36, Jacopo Mondi wrote:\n> Hi Hans,\n> I'm sorry, I don't want to insist but..\n> \n> On Thu, Aug 06, 2020 at 03:15:54PM +0200, Hans Verkuil wrote:\n>> On 06/08/2020 14:45, Jacopo Mondi wrote:\n>>> Hi Hans,\n>>>\n>>> On Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n>>>> On 06/08/2020 12:08, Jacopo Mondi wrote:\n>>>>> Hi Hans,\n>>>>>\n>>>>> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n>>>>>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n>>>>>>> Provide a table to describe how the V4L2 selection targets can be used\n>>>>>>> to access an image sensor pixel array properties.\n>>>>>>>\n>>>>>>> Reference the table in the sub-device documentation.\n>>>>>>>\n>>>>>>> Signed-off-by: Jacopo Mondi \n>>>>>>> ---\n>>>>>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n>>>>>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n>>>>>>> 2 files changed, 53 insertions(+)\n>>>>>>>\n>>>>>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>>>> index c47861dff9b9b..2f7da3832f458 100644\n>>>>>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n>>>>>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n>>>>>>> can set the analog crop rectangle to select which portion of the pixel array\n>>>>>>> to read out.\n>>>>>>>\n>>>>>>> +A description of each of the above mentioned targets when used to access the\n>>>>>>> +image sensor pixel array properties is provided by\n>>>>>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n>>>>>>> +\n>>>>>>>\n>>>>>>> Types of selection targets\n>>>>>>> --------------------------\n>>>>>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>>>> index 69f500093aa2a..632e6448b784e 100644\n>>>>>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n>>>>>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n>>>>>>> \tmodified by hardware.\n>>>>>>> - Yes\n>>>>>>> - No\n>>>>>>> +\n>>>>>>> +\n>>>>>>> +.. _v4l2-selection-targets-image-sensor-table:\n>>>>>>> +\n>>>>>>> +********************************************\n>>>>>>> +Selection Targets For Pixel Array Properties\n>>>>>>> +********************************************\n>>>>>>> +\n>>>>>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n>>>>>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n>>>>>>> +sub-device that represents an image sensor.\n>>>>>>> +\n>>>>>>> +A description of the properties associated with each of the sensor pixel array\n>>>>>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n>>>>>>> +\n>>>>>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n>>>>>>> +\n>>>>>>> +.. flat-table:: Selection target definitions\n>>>>>>> + :header-rows: 1\n>>>>>>> + :stub-columns: 0\n>>>>>>> +\n>>>>>>> + * - Target name\n>>>>>>> + - id\n>>>>>>> + - Definition\n>>>>>>> + - Read/Write\n>>>>>>> + * - ``V4L2_SEL_TGT_CROP``\n>>>>>>> + - 0x0000\n>>>>>>> + - The analog crop rectangle. Represents the portion of the active pixel\n>>>>>>> + array which is processed to produce images.\n>>>>>>> + - RW\n>>>>>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n>>>>>>> + - 0x0001\n>>>>>>> + - The active pixel array rectangle. It includes only active pixels and\n>>>>>>> + excludes other ones such as optical black pixels. Its width and height\n>>>>>>> + represent the maximum image resolution an image sensor can produce.\n>>>>>>> + - RO\n>>>>>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n>>>>>>> + - 0x0002\n>>>>>>> + - The readable portion of the physical pixel array matrix. It includes\n>>>>>>> + pixels that contains valid image data and calibration pixels such as the\n>>>>>>> + optical black ones.\n>>>>>>> + - RO\n>>>>>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n>>>>>>> + - 0x0003\n>>>>>>> + - The physical pixel array size, including readable and not readable\n>>>>>>> + pixels. As pixels that cannot be read from application processor are not\n>>>>>>> + relevant for calibration purposes, this rectangle is useful to calculate\n>>>>>>> + the physical properties of the image sensor.\n>>>>>>> + - RO\n>>>>>>>\n>>>>>>\n>>>>>> Hmm, this basically just duplicates the previous patch.\n>>>>>>\n>>>>>> I think you are documenting things at the wrong place. What you documented in the\n>>>>>> previous patch really belongs here since it is shared between the subdev API and the\n>>>>>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n>>>>>\n>>>>> I originally had it here, but then I moved to dev-subdev as an image\n>>>>> sensor will always be represented as a video sub-device, doen't it ?\n>>>>\n>>>> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n>>>> platform drivers that don't use the subdev API.\n>>>\n>>> Do we expect to be able to retrieve sensor array properties from video\n>>> device nodes which represents, in my understanding a DMA engine that\n>>> writes data to memory ? As I see it, not subdev for the image sensor,\n>>> no pixel array properties. How can these be exposed by a video device\n>>> which abstracts the full capture pipeline ?\n>>\n>> They will typically ask the subdev driver. The vidioc_g_selection op\n>> implementation will in turn call the get_selection op of the sensor subdev\n>> driver and pass that information back to userspace.\n> \n> How do we know that the any target reports the actual sensor\n> properties and not some other limitation introduced by processing\n> components down the pipeline, as everything sits behind a single video\n> device node ? In example, the default crop rectangle might very well depend\n> on the receiver's side decision to crop the frames received from the\n> sensor to maximize the FOV or whatnot. How do we know which 'cropping\n> point' is reported ?\n\nWhy would you care? Would you do anything different in userspace if a driver\nwould modify these rectangles before passing it on to userspace?\n\n> I hardly see a vidio_g_selection() being ideal to report properties of\n> the camera sensor such as the pixel array ones. I still feel we're\n> hijacking that API for that purpose and I would be glad to have\n> dedicated targets for image sensors. This would promote 'image\n> sensors' as first class citizens of the API like devices and\n> sub-devices, and I'm not sure this is desirable.\n\nSorry, but the selection API (and its CROP predecessor) has been in use\nfor sensors and webcam drivers since pretty much the beginning of V4L1.\n\nI'm not sure what it is that you think is problematical with the current\nAPI.\n\nRegards,\n\n\tHans\n\n> \n>>\n>> There is nothing subdev specific to this API.\n>>\n>> Regards,\n>>\n>> \tHans","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 76FFABD86F\n\tfor ;\n\tThu, 6 Aug 2020 15:32:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E0D76071B;\n\tThu, 6 Aug 2020 17:32:54 +0200 (CEST)","from lb3-smtp-cloud9.xs4all.net (lb3-smtp-cloud9.xs4all.net\n\t[194.109.24.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 335B860495\n\tfor ;\n\tThu, 6 Aug 2020 17:32:52 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid 3hswk9uqMuuXO3hsxkpCmO; Thu, 06 Aug 2020 17:32:51 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"lbYmm7Uw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1596727971; bh=nxBqDfxzCtGhzcIHuoaXNQt8AGe56sInEZvA4tZYmcs=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=lbYmm7UwEiLe6DuYxcW7WjcX6m7K+mz3gaZoIyeow99KtrkwsgSJphtTQ3EUa6ehZ\n\t5Bn7XL+bPX/mMsYhnbTStaCWuuYoGHQPuGrRk/iYGHIgU5PBAB1fETJIqi6Sr/cIm7\n\turH2aNXeG/G0twM3+no0fvp0j5mJQajTmExjVPK+EwWyEq/G1xw+ZghClgvl3N7Rh8\n\tdrmG8hqOpVtA+Wi8s8MDW2oIDktpYRy5KR4pQ2kRrb1axvPj60h3RF+iOsftJzkOZd\n\tUgw4JMi0JGIuIrriVcNRmcqHOdvKtvsGTHsbkGa+Iv4u/eL6wCyvP4AMNpfwDE31Dk\n\tGBtHl2bwXnOmw==","To":"Jacopo Mondi ","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>\n\t<20200806124523.llccxvcz65ohqqwq@uno.localdomain>\n\t\n\t<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>","From":"Hans Verkuil ","Message-ID":"<6a349665-8342-4f3b-d9b4-8d49a1da7d95@xs4all.nl>","Date":"Thu, 6 Aug 2020 17:32:50 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>","Content-Language":"en-US","X-CMAE-Envelope":"MS4wfB1u6GbqagLLlXamMgPkgFaUUfW4XQ03TsYm3+uN5B6gCi0W2e3f5xEs9HZ7zF+2U0a/SwuMvCSD+7uQ1WOXZTx7UtMjtjlTSPli6NslH+EUMvEW+Bm3\n\tIAYsHI49ywJtPGlu/i35LddddJkx0AdtNE2GZxw5dOxssaIiNWSxjJ5ALMEHCnLx7t408sPOhAQqbspWy7dFvHchic8NEJ48Gwd8/vUxHEXDBLoYRYJhem4d\n\t/Z30iouMfRkDz3nJqf8ljJNJ1loKiMH4wUQfx+DH6K5ItTWvE8IcADl7pQT1vhJJtVpbVJOnwv/4Fg3N+LraUKL65/cGhY6OggFNHPjcoPhaXiibxB5D/tjs\n\t3Yqm05Ojn2lTm4IB49AAgUxuY7C+QH8FFr+SiNDUZlRQqOMRsGyBUdD2XXtxc2xwliirfc7fe5Rr+cGroNPHO4bgAykWUEr8MWB+gCgp7f1CocKSudNS4apF\n\t8sTT5HSMlCQjxcabFXQJjxwtqW5QrkkdBRVcXiEfGy7WaYQBn6mzBBJkk5A=","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11939,"web_url":"https://patchwork.libcamera.org/comment/11939/","msgid":"<20200806161156.umcsxduyta3vfnvg@uno.localdomain>","date":"2020-08-06T16:11:56","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n\nOn Thu, Aug 06, 2020 at 05:32:50PM +0200, Hans Verkuil wrote:\n> On 06/08/2020 15:36, Jacopo Mondi wrote:\n> > Hi Hans,\n> > I'm sorry, I don't want to insist but..\n> >\n> > On Thu, Aug 06, 2020 at 03:15:54PM +0200, Hans Verkuil wrote:\n> >> On 06/08/2020 14:45, Jacopo Mondi wrote:\n> >>> Hi Hans,\n> >>>\n> >>> On Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n> >>>> On 06/08/2020 12:08, Jacopo Mondi wrote:\n> >>>>> Hi Hans,\n> >>>>>\n> >>>>> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> >>>>>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> >>>>>>> Provide a table to describe how the V4L2 selection targets can be used\n> >>>>>>> to access an image sensor pixel array properties.\n> >>>>>>>\n> >>>>>>> Reference the table in the sub-device documentation.\n> >>>>>>>\n> >>>>>>> Signed-off-by: Jacopo Mondi \n> >>>>>>> ---\n> >>>>>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> >>>>>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> >>>>>>> 2 files changed, 53 insertions(+)\n> >>>>>>>\n> >>>>>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>> index c47861dff9b9b..2f7da3832f458 100644\n> >>>>>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> >>>>>>> can set the analog crop rectangle to select which portion of the pixel array\n> >>>>>>> to read out.\n> >>>>>>>\n> >>>>>>> +A description of each of the above mentioned targets when used to access the\n> >>>>>>> +image sensor pixel array properties is provided by\n> >>>>>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n> >>>>>>> +\n> >>>>>>>\n> >>>>>>> Types of selection targets\n> >>>>>>> --------------------------\n> >>>>>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>> index 69f500093aa2a..632e6448b784e 100644\n> >>>>>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> >>>>>>> \tmodified by hardware.\n> >>>>>>> - Yes\n> >>>>>>> - No\n> >>>>>>> +\n> >>>>>>> +\n> >>>>>>> +.. _v4l2-selection-targets-image-sensor-table:\n> >>>>>>> +\n> >>>>>>> +********************************************\n> >>>>>>> +Selection Targets For Pixel Array Properties\n> >>>>>>> +********************************************\n> >>>>>>> +\n> >>>>>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n> >>>>>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n> >>>>>>> +sub-device that represents an image sensor.\n> >>>>>>> +\n> >>>>>>> +A description of the properties associated with each of the sensor pixel array\n> >>>>>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> >>>>>>> +\n> >>>>>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> >>>>>>> +\n> >>>>>>> +.. flat-table:: Selection target definitions\n> >>>>>>> + :header-rows: 1\n> >>>>>>> + :stub-columns: 0\n> >>>>>>> +\n> >>>>>>> + * - Target name\n> >>>>>>> + - id\n> >>>>>>> + - Definition\n> >>>>>>> + - Read/Write\n> >>>>>>> + * - ``V4L2_SEL_TGT_CROP``\n> >>>>>>> + - 0x0000\n> >>>>>>> + - The analog crop rectangle. Represents the portion of the active pixel\n> >>>>>>> + array which is processed to produce images.\n> >>>>>>> + - RW\n> >>>>>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> >>>>>>> + - 0x0001\n> >>>>>>> + - The active pixel array rectangle. It includes only active pixels and\n> >>>>>>> + excludes other ones such as optical black pixels. Its width and height\n> >>>>>>> + represent the maximum image resolution an image sensor can produce.\n> >>>>>>> + - RO\n> >>>>>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> >>>>>>> + - 0x0002\n> >>>>>>> + - The readable portion of the physical pixel array matrix. It includes\n> >>>>>>> + pixels that contains valid image data and calibration pixels such as the\n> >>>>>>> + optical black ones.\n> >>>>>>> + - RO\n> >>>>>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> >>>>>>> + - 0x0003\n> >>>>>>> + - The physical pixel array size, including readable and not readable\n> >>>>>>> + pixels. As pixels that cannot be read from application processor are not\n> >>>>>>> + relevant for calibration purposes, this rectangle is useful to calculate\n> >>>>>>> + the physical properties of the image sensor.\n> >>>>>>> + - RO\n> >>>>>>>\n> >>>>>>\n> >>>>>> Hmm, this basically just duplicates the previous patch.\n> >>>>>>\n> >>>>>> I think you are documenting things at the wrong place. What you documented in the\n> >>>>>> previous patch really belongs here since it is shared between the subdev API and the\n> >>>>>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> >>>>>\n> >>>>> I originally had it here, but then I moved to dev-subdev as an image\n> >>>>> sensor will always be represented as a video sub-device, doen't it ?\n> >>>>\n> >>>> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n> >>>> platform drivers that don't use the subdev API.\n> >>>\n> >>> Do we expect to be able to retrieve sensor array properties from video\n> >>> device nodes which represents, in my understanding a DMA engine that\n> >>> writes data to memory ? As I see it, not subdev for the image sensor,\n> >>> no pixel array properties. How can these be exposed by a video device\n> >>> which abstracts the full capture pipeline ?\n> >>\n> >> They will typically ask the subdev driver. The vidioc_g_selection op\n> >> implementation will in turn call the get_selection op of the sensor subdev\n> >> driver and pass that information back to userspace.\n> >\n> > How do we know that the any target reports the actual sensor\n> > properties and not some other limitation introduced by processing\n> > components down the pipeline, as everything sits behind a single video\n> > device node ? In example, the default crop rectangle might very well depend\n> > on the receiver's side decision to crop the frames received from the\n> > sensor to maximize the FOV or whatnot. How do we know which 'cropping\n> > point' is reported ?\n>\n> Why would you care? Would you do anything different in userspace if a driver\n> would modify these rectangles before passing it on to userspace?\n>\n\nYes it indeed makes a difference, particularly for application dealing\nwith RAW sensors and for IPS tuning algorithms that need to access the\nsensor pixel matrix sizes to calculate, in example, the FOV ratio, or\nmight want to access black pixels for calibration purposes.\n\nNow, I'm not an expert on that part, but in example I see the\nRaspberryPi 3A algorthms in libcamera using the ratio between the\nactive pixel array size and what I referred to as 'analogCrop' to\ncalculate the lens shading correction maps\nhttps://git.linuxtv.org/libcamera.git/tree/src/ipa/raspberrypi/controller/rpi/alsc.cpp#n185\n\nI can imagine there are other relevant image tuning algorithms that\nneeds to access the sensor characteristics precisely. Knowing that\nwhat I'm getting describes the sensor properties is relevant. That's\nwhy I hardly see this happening going through a single device node.\n\n> > I hardly see a vidio_g_selection() being ideal to report properties of\n> > the camera sensor such as the pixel array ones. I still feel we're\n> > hijacking that API for that purpose and I would be glad to have\n> > dedicated targets for image sensors. This would promote 'image\n> > sensors' as first class citizens of the API like devices and\n> > sub-devices, and I'm not sure this is desirable.\n>\n> Sorry, but the selection API (and its CROP predecessor) has been in use\n> for sensors and webcam drivers since pretty much the beginning of V4L1.\n>\n> I'm not sure what it is that you think is problematical with the current\n> API.\n>\n\nI'm not proposing to kill that API, I just think the existing targets\nfall short or are under-specified when applied to RAW image sensor,\nthat's it :)\n\nThanks\n j\n\n> Regards,\n>\n> \tHans\n>\n> >\n> >>\n> >> There is nothing subdev specific to this API.\n> >>\n> >> Regards,\n> >>\n> >> \tHans\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 4F2E7BD87A\n\tfor ;\n\tThu, 6 Aug 2020 16:08:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D61D060836;\n\tThu, 6 Aug 2020 18:08:19 +0200 (CEST)","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 6AE6060392\n\tfor ;\n\tThu, 6 Aug 2020 18:08:18 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 935C940002;\n\tThu, 6 Aug 2020 16:08:16 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 6 Aug 2020 18:11:56 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Message-ID":"<20200806161156.umcsxduyta3vfnvg@uno.localdomain>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>\n\t<20200806124523.llccxvcz65ohqqwq@uno.localdomain>\n\t\n\t<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>\n\t<6a349665-8342-4f3b-d9b4-8d49a1da7d95@xs4all.nl>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<6a349665-8342-4f3b-d9b4-8d49a1da7d95@xs4all.nl>","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11956,"web_url":"https://patchwork.libcamera.org/comment/11956/","msgid":"<20200809173206.GD5981@pendragon.ideasonboard.com>","date":"2020-08-09T17:32:06","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo and Hans,\n\nOn Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> > Provide a table to describe how the V4L2 selection targets can be used\n> > to access an image sensor pixel array properties.\n> > \n> > Reference the table in the sub-device documentation.\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> > .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> > 2 files changed, 53 insertions(+)\n> > \n> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > index c47861dff9b9b..2f7da3832f458 100644\n> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> > @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> > can set the analog crop rectangle to select which portion of the pixel array\n> > to read out.\n> > \n> > +A description of each of the above mentioned targets when used to access the\n> > +image sensor pixel array properties is provided by\n> > +:ref:`v4l2-selection-targets-image-sensor-table`\n> > +\n> > \n> > Types of selection targets\n> > --------------------------\n> > diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > index 69f500093aa2a..632e6448b784e 100644\n> > --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> > @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> > \tmodified by hardware.\n> > - Yes\n> > - No\n> > +\n> > +\n> > +.. _v4l2-selection-targets-image-sensor-table:\n> > +\n> > +********************************************\n> > +Selection Targets For Pixel Array Properties\n> > +********************************************\n> > +\n> > +The V4L2 selection API can be used to retrieve the size and disposition of the\n> > +pixel units that compose and image sensor pixel matrix when applied to a video\n> > +sub-device that represents an image sensor.\n> > +\n> > +A description of the properties associated with each of the sensor pixel array\n> > +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> > +\n> > +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> > +\n> > +.. flat-table:: Selection target definitions\n> > + :header-rows: 1\n> > + :stub-columns: 0\n> > +\n> > + * - Target name\n> > + - id\n> > + - Definition\n> > + - Read/Write\n> > + * - ``V4L2_SEL_TGT_CROP``\n> > + - 0x0000\n> > + - The analog crop rectangle. Represents the portion of the active pixel\n> > + array which is processed to produce images.\n> > + - RW\n> > + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> > + - 0x0001\n> > + - The active pixel array rectangle. It includes only active pixels and\n> > + excludes other ones such as optical black pixels. Its width and height\n> > + represent the maximum image resolution an image sensor can produce.\n> > + - RO\n> > + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> > + - 0x0002\n> > + - The readable portion of the physical pixel array matrix. It includes\n> > + pixels that contains valid image data and calibration pixels such as the\n> > + optical black ones.\n> > + - RO\n> > + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> > + - 0x0003\n> > + - The physical pixel array size, including readable and not readable\n> > + pixels. As pixels that cannot be read from application processor are not\n> > + relevant for calibration purposes, this rectangle is useful to calculate\n> > + the physical properties of the image sensor.\n> > + - RO\n> > \n> \n> Hmm, this basically just duplicates the previous patch.\n> \n> I think you are documenting things at the wrong place. What you documented in the\n> previous patch really belongs here since it is shared between the subdev API and the\n> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> \n> Frankly, the selection API documentation is a mess. It's spread out over various sections:\n> The VIDIOC_G/S_SELECTION and VIDIOC_SUBDEV_G/S_SELECTION descriptions in the Function Reference,\n> section 8 (\"Common definitions for V4L2 and V4L2 subdev interfaces\"), 1.25 \"Cropping, composing\n> and scaling – the SELECTION API\" and 4.13.3.2-4.13.3.3 \"Selections: cropping, scaling and composition\".\n> \n> In my view section 8 should be moved to section 1.25.2.\n\nAgreed.\n\n> Ideally 1.25 should be rewritten for both\n> the V4L2 and V4L2 subdev APIs, but that's a lot of work.\n\nAgreed too.\n\n> I would suggest that you add a first patch that moves section 8 to 1.25.2. Note that I don't like\n> the tables for the selection targets and flags: the 'Valid for V4L2/V4L2 subdevs' columns should\n> be removed and it should either be mentioned in the definition if a target/flag is invalid for\n> an API, or it should be put in a separate table.\n\nShould \"1.25.3.1. Configuration of video capture\" be moved to \"4.1.\nVideo Capture Interface\", and \"1.25.3.2. Configuration of video output\"\nto \"4.3. Video Output Interface\" ?\n\nI'm not sure where patch 1/4 should be moved to though. As I mentioned\nin the review, I think it should create a section specific to camera\nsensors. \"4.13. Sub-device Interface\" isn't a very good candidate as it\ndescribes the interface, it shouldn't document how particular classes of\nsubdevs are to be handled. Maybe a new top-level section in \"Part I -\nVideo for Linux API\" to describe different types of sub-devices ?\n\n> And in 1.25.2 perhaps a new picture can be created for the specific sensor use-case that includes\n> the NATIVE_SIZE target.\n> \n> Another pet peeve of mine is that section 8 splits the selection targets and flags into separate\n> subsections. I'd just keep it in one section.","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 E1323BD87C\n\tfor ;\n\tSun, 9 Aug 2020 17:32:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69B1760E4D;\n\tSun, 9 Aug 2020 19:32:20 +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 DF47D60D5B\n\tfor ;\n\tSun, 9 Aug 2020 19:32:18 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EB6FF9;\n\tSun, 9 Aug 2020 19:32:18 +0200 (CEST)"],"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=\"XRJutgNP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596994338;\n\tbh=mJHOhEwbmsrfhw/PQIKMshYf/7jqPTV2GgKbdarle1s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XRJutgNPokcv+w4adKU8gSvuRzOVhb/b4x7YWVah08C+B3Rm+sFrldYBZltH5Mgpv\n\tScKJ5KGGL2WQnl4oeWPcP0tiR3oxvjGlpz27cqSddoPjuAQ42nDSayaqZY3ypV4iPJ\n\t5IoXmKjcV878ksvjY7G6apbIYnPIatTQbvBg3fxs=","Date":"Sun, 9 Aug 2020 20:32:06 +0300","From":"Laurent Pinchart ","To":"Hans Verkuil ","Message-ID":"<20200809173206.GD5981@pendragon.ideasonboard.com>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11957,"web_url":"https://patchwork.libcamera.org/comment/11957/","msgid":"<20200809175444.GE5981@pendragon.ideasonboard.com>","date":"2020-08-09T17:54:44","subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo and Hans,\n\nOn Thu, Aug 06, 2020 at 06:11:56PM +0200, Jacopo Mondi wrote:\n> On Thu, Aug 06, 2020 at 05:32:50PM +0200, Hans Verkuil wrote:\n> > On 06/08/2020 15:36, Jacopo Mondi wrote:\n> >> On Thu, Aug 06, 2020 at 03:15:54PM +0200, Hans Verkuil wrote:\n> >>> On 06/08/2020 14:45, Jacopo Mondi wrote:\n> >>>> On Thu, Aug 06, 2020 at 12:15:49PM +0200, Hans Verkuil wrote:\n> >>>>> On 06/08/2020 12:08, Jacopo Mondi wrote:\n> >>>>>> On Thu, Aug 06, 2020 at 10:45:17AM +0200, Hans Verkuil wrote:\n> >>>>>>> On 05/08/2020 12:57, Jacopo Mondi wrote:\n> >>>>>>>> Provide a table to describe how the V4L2 selection targets can be used\n> >>>>>>>> to access an image sensor pixel array properties.\n> >>>>>>>>\n> >>>>>>>> Reference the table in the sub-device documentation.\n> >>>>>>>>\n> >>>>>>>> Signed-off-by: Jacopo Mondi \n> >>>>>>>> ---\n> >>>>>>>> .../userspace-api/media/v4l/dev-subdev.rst | 4 ++\n> >>>>>>>> .../media/v4l/v4l2-selection-targets.rst | 49 +++++++++++++++++++\n> >>>>>>>> 2 files changed, 53 insertions(+)\n> >>>>>>>>\n> >>>>>>>> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>>> index c47861dff9b9b..2f7da3832f458 100644\n> >>>>>>>> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>>> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst\n> >>>>>>>> @@ -467,6 +467,10 @@ desired image resolution. If the sub-device driver supports that, userspace\n> >>>>>>>> can set the analog crop rectangle to select which portion of the pixel array\n> >>>>>>>> to read out.\n> >>>>>>>>\n> >>>>>>>> +A description of each of the above mentioned targets when used to access the\n> >>>>>>>> +image sensor pixel array properties is provided by\n> >>>>>>>> +:ref:`v4l2-selection-targets-image-sensor-table`\n> >>>>>>>> +\n> >>>>>>>>\n> >>>>>>>> Types of selection targets\n> >>>>>>>> --------------------------\n> >>>>>>>> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>>> index 69f500093aa2a..632e6448b784e 100644\n> >>>>>>>> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>>> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst\n> >>>>>>>> @@ -76,3 +76,52 @@ of the two interfaces they are used.\n> >>>>>>>> \tmodified by hardware.\n> >>>>>>>> - Yes\n> >>>>>>>> - No\n> >>>>>>>> +\n> >>>>>>>> +\n> >>>>>>>> +.. _v4l2-selection-targets-image-sensor-table:\n> >>>>>>>> +\n> >>>>>>>> +********************************************\n> >>>>>>>> +Selection Targets For Pixel Array Properties\n> >>>>>>>> +********************************************\n> >>>>>>>> +\n> >>>>>>>> +The V4L2 selection API can be used to retrieve the size and disposition of the\n> >>>>>>>> +pixel units that compose and image sensor pixel matrix when applied to a video\n> >>>>>>>> +sub-device that represents an image sensor.\n> >>>>>>>> +\n> >>>>>>>> +A description of the properties associated with each of the sensor pixel array\n> >>>>>>>> +areas is provided by the :ref:`v4l2-subdev-pixel-array-properties` section.\n> >>>>>>>> +\n> >>>>>>>> +.. tabularcolumns:: |p{6.0cm}|p{1.4cm}|p{7.4cm}|p(1.4cm)|\n> >>>>>>>> +\n> >>>>>>>> +.. flat-table:: Selection target definitions\n> >>>>>>>> + :header-rows: 1\n> >>>>>>>> + :stub-columns: 0\n> >>>>>>>> +\n> >>>>>>>> + * - Target name\n> >>>>>>>> + - id\n> >>>>>>>> + - Definition\n> >>>>>>>> + - Read/Write\n> >>>>>>>> + * - ``V4L2_SEL_TGT_CROP``\n> >>>>>>>> + - 0x0000\n> >>>>>>>> + - The analog crop rectangle. Represents the portion of the active pixel\n> >>>>>>>> + array which is processed to produce images.\n> >>>>>>>> + - RW\n> >>>>>>>> + * - ``V4L2_SEL_TGT_CROP_DEFAULT``\n> >>>>>>>> + - 0x0001\n> >>>>>>>> + - The active pixel array rectangle. It includes only active pixels and\n> >>>>>>>> + excludes other ones such as optical black pixels. Its width and height\n> >>>>>>>> + represent the maximum image resolution an image sensor can produce.\n> >>>>>>>> + - RO\n> >>>>>>>> + * - ``V4L2_SEL_TGT_CROP_BOUNDS``\n> >>>>>>>> + - 0x0002\n> >>>>>>>> + - The readable portion of the physical pixel array matrix. It includes\n> >>>>>>>> + pixels that contains valid image data and calibration pixels such as the\n> >>>>>>>> + optical black ones.\n> >>>>>>>> + - RO\n> >>>>>>>> + * - ``V4L2_SEL_TGT_NATIVE_SIZE``\n> >>>>>>>> + - 0x0003\n> >>>>>>>> + - The physical pixel array size, including readable and not readable\n> >>>>>>>> + pixels. As pixels that cannot be read from application processor are not\n> >>>>>>>> + relevant for calibration purposes, this rectangle is useful to calculate\n> >>>>>>>> + the physical properties of the image sensor.\n> >>>>>>>> + - RO\n> >>>>>>>>\n> >>>>>>>\n> >>>>>>> Hmm, this basically just duplicates the previous patch.\n> >>>>>>>\n> >>>>>>> I think you are documenting things at the wrong place. What you documented in the\n> >>>>>>> previous patch really belongs here since it is shared between the subdev API and the\n> >>>>>>> regular V4L2 API. And in dev-subdev.rst you then refer to here.\n> >>>>>>\n> >>>>>> I originally had it here, but then I moved to dev-subdev as an image\n> >>>>>> sensor will always be represented as a video sub-device, doen't it ?\n> >>>>>\n> >>>>> No. Some camera drivers are V4L2 only, most notably uvc. Also there are several simple\n> >>>>> platform drivers that don't use the subdev API.\n> >>>>\n> >>>> Do we expect to be able to retrieve sensor array properties from video\n> >>>> device nodes which represents, in my understanding a DMA engine that\n> >>>> writes data to memory ? As I see it, not subdev for the image sensor,\n> >>>> no pixel array properties. How can these be exposed by a video device\n> >>>> which abstracts the full capture pipeline ?\n> >>>\n> >>> They will typically ask the subdev driver. The vidioc_g_selection op\n> >>> implementation will in turn call the get_selection op of the sensor subdev\n> >>> driver and pass that information back to userspace.\n> >>\n> >> How do we know that the any target reports the actual sensor\n> >> properties and not some other limitation introduced by processing\n> >> components down the pipeline, as everything sits behind a single video\n> >> device node ? In example, the default crop rectangle might very well depend\n> >> on the receiver's side decision to crop the frames received from the\n> >> sensor to maximize the FOV or whatnot. How do we know which 'cropping\n> >> point' is reported ?\n> >\n> > Why would you care? Would you do anything different in userspace if a driver\n> > would modify these rectangles before passing it on to userspace?\n>\n> Yes it indeed makes a difference, particularly for application dealing\n> with RAW sensors and for IPS tuning algorithms that need to access the\n> sensor pixel matrix sizes to calculate, in example, the FOV ratio, or\n> might want to access black pixels for calibration purposes.\n> \n> Now, I'm not an expert on that part, but in example I see the\n> RaspberryPi 3A algorthms in libcamera using the ratio between the\n> active pixel array size and what I referred to as 'analogCrop' to\n> calculate the lens shading correction maps\n> https://git.linuxtv.org/libcamera.git/tree/src/ipa/raspberrypi/controller/rpi/alsc.cpp#n185\n> \n> I can imagine there are other relevant image tuning algorithms that\n> needs to access the sensor characteristics precisely. Knowing that\n> what I'm getting describes the sensor properties is relevant. That's\n> why I hardly see this happening going through a single device node.\n> \n> >> I hardly see a vidio_g_selection() being ideal to report properties of\n> >> the camera sensor such as the pixel array ones. I still feel we're\n> >> hijacking that API for that purpose and I would be glad to have\n> >> dedicated targets for image sensors. This would promote 'image\n> >> sensors' as first class citizens of the API like devices and\n> >> sub-devices, and I'm not sure this is desirable.\n> >\n> > Sorry, but the selection API (and its CROP predecessor) has been in use\n> > for sensors and webcam drivers since pretty much the beginning of V4L1.\n> >\n> > I'm not sure what it is that you think is problematical with the current\n> > API.\n> \n> I'm not proposing to kill that API, I just think the existing targets\n> fall short or are under-specified when applied to RAW image sensor,\n> that's it :)\n\nThere's a bit difference between what is exposed by a V4L2 video capture\ndevice in video-node-centric designs, and by a camera sensor subdev in\nMC-centric designs.\n\nIn the first case, the video node exposes a logical view of the device,\nwith the ability to scale and crop if supported, but without any control\non how and where those operations are performed. When userspace wants\nto crop the image, a USB webcam, an embedded system camera (for lack of\na better term, I mean here a camera sensor and a receiver, both being\ncontrollable by Linux) or an analog capture device are exposed\nsimilarly, without applications having to care much about the device\ntype (there are of course device-specific details exposed, such as the\nanalog or digital TV standards, but that's separate).\n\nNote that in the USB case, it's common for UVC webcams to expose a set\nof resolutions, and use a combination of scaling and cropping to achieve\nthem, without reporting to the host how much cropping is applied. The\nUVC standard allows webcams to crop internally, and doesn't expose any\ncropping control, so the UVC driver doesn't implement control of the\ncrop rectangle. This is an extreme case that shows how userspace has\nlittle control of the internal configuration of the device. Reading\noptical black pixels is clearly out of the picture with this API, the\nvideo-node-centric device is really a simple, high-level view of a\nlogical device. It's good enough for many devices.\n\nIn the MC-centric case, applications need full control of all processing\nsteps, and that's where getting detailed information about the crop\nrectangles, and the ability to control them all (including up to 3\ndifferent stages of cropping in SMIA++ devices for instance) is\nimportant. As Jacopo explained above, the lens shading correction map is\ncalculated based on how much cropping has been applied. Here we're\ntalking about specialized userspace, with the main user being\nlibcamera. libcamera abstracts all this, and exposes a simpler, logical\nview of the device to applications. In effect, it turns an MC-centric\nkernel device into a simpler logical device (but with extra features\ncompared to video-node-centric devices, such as a request-based API, and\nthe ability to have some more control over the ISP).\n\nFor this kind of userspace, the V4L2 video API was not enough, and we\nhave introduced the MC API and the V4L2 subdev API. The lack of a\nuserspace stack on top of that, a long term issue that we're fixing now,\nhas prevented us to see the still numerous shortcomings of our kernel\nAPIs. Vendors who have been using MC-centric drivers to implement\nproprietary camera stacks have thus resorted to either out-of-tree\nAPI extentions or abuses of the existing interfaces. For instance, I've\nseen out-of-tree drivers using the v4l2_streamparm.capture.capturemode\nfield returned by VIDIOC_G_PARM to retrieve the resolution and crop\nconfiguration of the camera sensor by indexing in a table of modes in\nuserspace. There are lots of examples of horrible code, which shows the\nneed not only for API extensions, but for standardizing through\ndocumentation how the existing APIS are used.\n\nThis is what Jacopo is trying to address with this series. I however\ndon't think we need to define extra selection targets (but I'm open to\nconsidering that if there are good reasons I'm not aware of), but we\nvery much need to describe in details how the selection targets apply to\ncamera sensor subdevs. This is separate from the V4L2 video API, we may\nalso need better descriptions there, but that should be handled\nseparately. The base concepts and the targets are shared between the two\ncases, but that's where it stops.\n\n> >>> There is nothing subdev specific to this API.","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 384BEBD87D\n\tfor ;\n\tSun, 9 Aug 2020 17:55:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E14460E95;\n\tSun, 9 Aug 2020 19:55:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1DF9C60D5B\n\tfor ;\n\tSun, 9 Aug 2020 19:54:59 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3887FF9;\n\tSun, 9 Aug 2020 19:54:57 +0200 (CEST)"],"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=\"qeFCU+n0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596995698;\n\tbh=5CSZllsvdkxoYj4Cydv0rPSP7KQv8tLYjWYW/RJ0uXA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qeFCU+n0IclVo1MhvuyLvlZKyyyqywVRX8geH4YFw7f5MUHWCsPLC4IRL5NQ/fjcw\n\tTqH+nm94lDArnfrVgfN9F2aBzDDyfG3jVsifzXscYk1hYDfdbYN1ZuVmCcTVQbrpov\n\tab7fFEtV+zkOLCZhi82MH8HbEEnbm6C0h9K0ORvc=","Date":"Sun, 9 Aug 2020 20:54:44 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200809175444.GE5981@pendragon.ideasonboard.com>","References":"<20200805105721.15445-1-jacopo@jmondi.org>\n\t<20200805105721.15445-3-jacopo@jmondi.org>\n\t<1896673c-ae91-84c3-9573-5da91fb00f41@xs4all.nl>\n\t<20200806100822.kvlzivbtnqzni3xw@uno.localdomain>\n\t<851192ea-6498-4f9e-a6b4-bf5164f3c5a7@xs4all.nl>\n\t<20200806124523.llccxvcz65ohqqwq@uno.localdomain>\n\t\n\t<20200806133617.q2g5a63a3qzvzkaa@uno.localdomain>\n\t<6a349665-8342-4f3b-d9b4-8d49a1da7d95@xs4all.nl>\n\t<20200806161156.umcsxduyta3vfnvg@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200806161156.umcsxduyta3vfnvg@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/4] media: docs: Describe targets for\n\tsensor properties","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,\n\tSowjanya Komatineni ,\n\tSakari Ailus ,\n\tRicardo Ribalda Delgado ,\n\tLinux Media Mailing List ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]