[{"id":21938,"web_url":"https://patchwork.libcamera.org/comment/21938/","msgid":"<164129390422.3986460.993552112061270687@Monstersaurus>","date":"2022-01-04T10:58:24","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2021-09-22 14:29:14)\n> The modeSensitivity field is a number that describes how sensitive the\n> selected sensor mode is compared to other readout modes of the same\n> sensor. For example, a binned mode might have twice the sensitivity of\n> the full resolution mode, meaning you would get double the signal\n> level for the same exposure and gains.\n\nIs there a way to define which is the 'base' level?\n\nOr does it not matter, as it's all relative (but relative to what I\nguess is my question).\n\nI.e. Does it matter, or is it necessary to know that Mode A is 'half' as\nsensitive as Mode B?\n\nIn fact, I think that's already a given as whatever the pipeline handler\ndoes will be relative. I.e. it would be about comparing the value\nagainst other configurations not a standalone value.\n\n> \n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/camera.h | 2 ++\n> src/libcamera/camera.cpp | 16 +++++++++++++++-\n> 2 files changed, 17 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 601ee46e..3314c06b 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -66,6 +66,8 @@ public:\n> \n> Transform transform;\n> \n> + float modeSensitivity;\n> +\n> protected:\n> CameraConfiguration();\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 71809bcd..8d795dff 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> * \\brief Create an empty camera configuration\n> */\n> CameraConfiguration::CameraConfiguration()\n> - : transform(Transform::Identity), config_({})\n> + : transform(Transform::Identity), modeSensitivity(1.0), config_({})\n> {\n> }\n> \n> @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const\n> * may adjust this field at its discretion if the selection is not supported.\n> */\n> \n> +/**\n> + * \\var CameraConfiguration::modeSensitivity\n> + * \\brief The relative sensitivity of the chosen sensor mode\n> + *\n> + * Some sensors have readout modes with different sensitivities. For example,\n> + * a binned camera mode might, with the same exposure and gains, produce\n> + * twice the signal level of the full resolution readout. This would be\n> + * signalled by the binned mode, when it is chosen, indicating a value here\n> + * that is twice that of the full resolution mode.\n> + *\n> + * This value should only be read, and not set, by the user. It will be\n> + * valid after the configure method has reteurned successfully.\n> + */\n> +\n\nI bet we've discussed this in the past, but I can't recall. If there are\nmore of these properties, I would suspect they should be in a\nControlList somehow - but if there's not expected to be many, an\nexplicit field is fine to me, and I can see the importance of conveying\nthis information.\n\n\nI was also going to say perhaps we need some testing on this in\nlc-compliance, but given that the default is 1.0 for all camera\nconfigurations - I think that's fine. It wouldn't be something we could\neasily test anyway.\n\nReviewed-by: Kieran Bingham \n\n\n\n> /**\n> * \\var CameraConfiguration::config_\n> * \\brief The vector of stream configurations\n> -- \n> 2.20.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 30A5DBE080\n\tfor ;\n\tTue, 4 Jan 2022 10:58:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70E9B60925;\n\tTue, 4 Jan 2022 11:58:28 +0100 (CET)","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 35F3660219\n\tfor ;\n\tTue, 4 Jan 2022 11:58:27 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C0942501;\n\tTue, 4 Jan 2022 11:58:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BMPTIm++\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641293906;\n\tbh=wZ4S928k1rGffg/tLabPqjWLoYr2U6PBPqyYJdcQbrA=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=BMPTIm++62Xy0YbM8VjQTUW0JMzLw11T1czRLQH2bpAs7FKzqr8ZkvqSlXTiiSnnz\n\tEOmwVHdlLL4IcbMK6v0E8Mp08gLWR47IjfLJwFORtDHIUr6zropO1go7em8UhORj/c\n\tm+LKIpAaf77O4lA2tefRlBQ/kTacjE9m4LTnm6vk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20210922132915.14881-2-david.plowman@raspberrypi.com>","References":"<20210922132915.14881-1-david.plowman@raspberrypi.com>\n\t<20210922132915.14881-2-david.plowman@raspberrypi.com>","From":"Kieran Bingham ","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 04 Jan 2022 10:58:24 +0000","Message-ID":"<164129390422.3986460.993552112061270687@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21944,"web_url":"https://patchwork.libcamera.org/comment/21944/","msgid":"","date":"2022-01-04T13:30:03","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nThanks for the reply.\n\nOn Tue, 4 Jan 2022 at 10:58, Kieran Bingham\n wrote:\n>\n> Quoting David Plowman (2021-09-22 14:29:14)\n> > The modeSensitivity field is a number that describes how sensitive the\n> > selected sensor mode is compared to other readout modes of the same\n> > sensor. For example, a binned mode might have twice the sensitivity of\n> > the full resolution mode, meaning you would get double the signal\n> > level for the same exposure and gains.\n>\n> Is there a way to define which is the 'base' level?\n\nI suppose the \"base level\" would correspond to the mode that gives you\nthe lowest signal. But as per the discussion below, it's all relative\nso doesn't matter unless we want to ascribe a particular meaning.\n\n>\n> Or does it not matter, as it's all relative (but relative to what I\n> guess is my question).\n>\n> I.e. Does it matter, or is it necessary to know that Mode A is 'half' as\n> sensitive as Mode B?\n>\n> In fact, I think that's already a given as whatever the pipeline handler\n> does will be relative. I.e. it would be about comparing the value\n> against other configurations not a standalone value.\n>\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > include/libcamera/camera.h | 2 ++\n> > src/libcamera/camera.cpp | 16 +++++++++++++++-\n> > 2 files changed, 17 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 601ee46e..3314c06b 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -66,6 +66,8 @@ public:\n> >\n> > Transform transform;\n> >\n> > + float modeSensitivity;\n> > +\n> > protected:\n> > CameraConfiguration();\n> >\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 71809bcd..8d795dff 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > * \\brief Create an empty camera configuration\n> > */\n> > CameraConfiguration::CameraConfiguration()\n> > - : transform(Transform::Identity), config_({})\n> > + : transform(Transform::Identity), modeSensitivity(1.0), config_({})\n> > {\n> > }\n> >\n> > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const\n> > * may adjust this field at its discretion if the selection is not supported.\n> > */\n> >\n> > +/**\n> > + * \\var CameraConfiguration::modeSensitivity\n> > + * \\brief The relative sensitivity of the chosen sensor mode\n> > + *\n> > + * Some sensors have readout modes with different sensitivities. For example,\n> > + * a binned camera mode might, with the same exposure and gains, produce\n> > + * twice the signal level of the full resolution readout. This would be\n> > + * signalled by the binned mode, when it is chosen, indicating a value here\n> > + * that is twice that of the full resolution mode.\n> > + *\n> > + * This value should only be read, and not set, by the user. It will be\n> > + * valid after the configure method has reteurned successfully.\n> > + */\n> > +\n>\n> I bet we've discussed this in the past, but I can't recall. If there are\n> more of these properties, I would suspect they should be in a\n> ControlList somehow - but if there's not expected to be many, an\n> explicit field is fine to me, and I can see the importance of conveying\n> this information.\n\nYes, that's a good thought. One thing to note is that we need this\nvalue back during \"configure\", so you can calculate the right\nexposure/gain before calling \"start\". Could the configure method\nreturn a list of control values, I wonder?\n\n>\n>\n> I was also going to say perhaps we need some testing on this in\n> lc-compliance, but given that the default is 1.0 for all camera\n> configurations - I think that's fine. It wouldn't be something we could\n> easily test anyway.\n\nI have a sensor I'm working on that requires this feature - hopefully\nit won't be too long before others can try it!\n\nThanks\nDavid\n\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n>\n> > /**\n> > * \\var CameraConfiguration::config_\n> > * \\brief The vector of stream configurations\n> > --\n> > 2.20.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2AC4ABE080\n\tfor ;\n\tTue, 4 Jan 2022 13:30:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 64DE760921;\n\tTue, 4 Jan 2022 14:30:16 +0100 (CET)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20B91604F4\n\tfor ;\n\tTue, 4 Jan 2022 14:30:14 +0100 (CET)","by mail-wr1-x429.google.com with SMTP id e5so76191529wrc.5\n\tfor ;\n\tTue, 04 Jan 2022 05:30:14 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"HN3a53Ao\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=p28JCssML2qKoyPNU2XF4ssCxeIQLb3FoKxIhqv2GSo=;\n\tb=HN3a53AoqbkzXyS7wJpiwgF1vQsqDdNNVuvksfDgkcJyDcU+iR0ftwdWK1sBrVD5Hl\n\tAwfdAEzfk5Ko9DFhIweP4mtLhfQYUP4oG3xr/2e9RvDMBG+EwpiWZXq5ys6fvECCgtFT\n\tuEHlf2Dee47YY2nED1dnx0lFI4G7dGrlqzjn/iUCkrmySqNkF/DGNn7xc9NoDUofT6rg\n\tU6kI60PRMAGbAJ0NNYy/Vhl3cIlOSZOyGhjJxwhl5QhMzb8WfE+83U4HX5oEgj+fi1B+\n\tZGK0BumLcEavwPWiZQVRVF06Km50aw3UlIy0yfSyqR1n0nBJsz3r5k7cp/XNuToxPNYU\n\tQfVg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=p28JCssML2qKoyPNU2XF4ssCxeIQLb3FoKxIhqv2GSo=;\n\tb=OO8W1WtGPeMc4CEwGEX7vTeZ8gqz595MAjM7vbW34boZ2hVeX72yn7GEGdMFnRmd/T\n\tA6yg9j1rqwDcmareG2wevHFYS7EatYS0fmIPOiE5Z4MlTdDuvg2DpiT5gupqOcM4CukZ\n\tIMs6+KFoACZJJyYaUmxLCClFDAc30dUI0VL8OpQEOTjqg7F4dLVmqPHInXQi0fN+Opqa\n\tLntyrSv8JT/e9WmrNMfc8Rnm70IeJKrTIl+QeoEjMI6oEnQPshVQaURd0VnMjjSz6bRF\n\tbaNgMtsE4r2+yVxsQrb8eedj5oIdqh+6T7xGQA/3LLVWccIjQS7tQWY/P7QzgK1474Cx\n\tUAZQ==","X-Gm-Message-State":"AOAM531UIRfe6VRU0QI+GMwO98Xvu4IHrXxEJ0CVYCdQt5MMfTt8QBUe\n\tp8saefbOAXmSh7/RIldPg0b4+7ljqjT41cTqgqNZ5Gq8M8c=","X-Google-Smtp-Source":"ABdhPJzJUQ46rfFNgfTbKA7rCPkgmyyOVFwZZoJ9SwvDbOLuXQh6b4FJZZEritJdg5v20qGCiS4b/MCpAItX3bv7pqI=","X-Received":"by 2002:adf:f801:: with SMTP id s1mr41851614wrp.3.1641303013781; \n\tTue, 04 Jan 2022 05:30:13 -0800 (PST)","MIME-Version":"1.0","References":"<20210922132915.14881-1-david.plowman@raspberrypi.com>\n\t<20210922132915.14881-2-david.plowman@raspberrypi.com>\n\t<164129390422.3986460.993552112061270687@Monstersaurus>","In-Reply-To":"<164129390422.3986460.993552112061270687@Monstersaurus>","From":"David Plowman ","Date":"Tue, 4 Jan 2022 13:30:03 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22155,"web_url":"https://patchwork.libcamera.org/comment/22155/","msgid":"<164440872391.3354066.3555596882482877393@Monstersaurus>","date":"2022-02-09T12:12:03","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nQuoting David Plowman (2022-01-04 13:30:03)\n> Hi Kieran\n> \n> Thanks for the reply.\n> \n> On Tue, 4 Jan 2022 at 10:58, Kieran Bingham\n> wrote:\n> >\n> > Quoting David Plowman (2021-09-22 14:29:14)\n> > > The modeSensitivity field is a number that describes how sensitive the\n> > > selected sensor mode is compared to other readout modes of the same\n> > > sensor. For example, a binned mode might have twice the sensitivity of\n> > > the full resolution mode, meaning you would get double the signal\n> > > level for the same exposure and gains.\n> >\n> > Is there a way to define which is the 'base' level?\n> \n> I suppose the \"base level\" would correspond to the mode that gives you\n> the lowest signal. But as per the discussion below, it's all relative\n> so doesn't matter unless we want to ascribe a particular meaning.\n> \n> >\n> > Or does it not matter, as it's all relative (but relative to what I\n> > guess is my question).\n> >\n> > I.e. Does it matter, or is it necessary to know that Mode A is 'half' as\n> > sensitive as Mode B?\n> >\n> > In fact, I think that's already a given as whatever the pipeline handler\n> > does will be relative. I.e. it would be about comparing the value\n> > against other configurations not a standalone value.\n> >\n> > >\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > include/libcamera/camera.h | 2 ++\n> > > src/libcamera/camera.cpp | 16 +++++++++++++++-\n> > > 2 files changed, 17 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 601ee46e..3314c06b 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -66,6 +66,8 @@ public:\n> > >\n> > > Transform transform;\n> > >\n> > > + float modeSensitivity;\n> > > +\n> > > protected:\n> > > CameraConfiguration();\n> > >\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 71809bcd..8d795dff 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > * \\brief Create an empty camera configuration\n> > > */\n> > > CameraConfiguration::CameraConfiguration()\n> > > - : transform(Transform::Identity), config_({})\n> > > + : transform(Transform::Identity), modeSensitivity(1.0), config_({})\n> > > {\n> > > }\n> > >\n> > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const\n> > > * may adjust this field at its discretion if the selection is not supported.\n> > > */\n> > >\n> > > +/**\n> > > + * \\var CameraConfiguration::modeSensitivity\n> > > + * \\brief The relative sensitivity of the chosen sensor mode\n> > > + *\n> > > + * Some sensors have readout modes with different sensitivities. For example,\n> > > + * a binned camera mode might, with the same exposure and gains, produce\n> > > + * twice the signal level of the full resolution readout. This would be\n> > > + * signalled by the binned mode, when it is chosen, indicating a value here\n> > > + * that is twice that of the full resolution mode.\n> > > + *\n> > > + * This value should only be read, and not set, by the user. It will be\n> > > + * valid after the configure method has reteurned successfully.\n\ns/reteurned/returned/\n\n> > > + */\n> > > +\n> >\n> > I bet we've discussed this in the past, but I can't recall. If there are\n> > more of these properties, I would suspect they should be in a\n> > ControlList somehow - but if there's not expected to be many, an\n> > explicit field is fine to me, and I can see the importance of conveying\n> > this information.\n> \n> Yes, that's a good thought. One thing to note is that we need this\n> value back during \"configure\", so you can calculate the right\n> exposure/gain before calling \"start\". Could the configure method\n> return a list of control values, I wonder?\n\nI'm sure a ControlList could be returned somewhere, or it could be a\npart of the CameraConfiguration structure itself.\n\nThe difficulty is that if we extend the structure here, it's an ABI\nbreak / change, but if we have a ControlList to return these properties\n- it can be updated/added to without breaking API/ABI compatibility.\n\nLooking at CameraConfiguration now, I see we have:\n\n Transform transform;\n\nWhich could be considered a configuration property to put in a\nControlList too. And ... maybe there might be some ColourSpace property\nto declare on the whole camera, rather than per stream? (Only because I\nsee a flag ColorSpaceFlag about whether ColorSpaces are per-stream or not)\n\nWould you consider a Transform and a ModeSensitivity to be something\nthat could be grouped together as CameraConfiguration::properties() ?\n\nOr does that become more complicated?\n\n> > I was also going to say perhaps we need some testing on this in\n> > lc-compliance, but given that the default is 1.0 for all camera\n> > configurations - I think that's fine. It wouldn't be something we could\n> > easily test anyway.\n> \n> I have a sensor I'm working on that requires this feature - hopefully\n> it won't be too long before others can try it!\n> \n> Thanks\n> David\n> \n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >\n> >\n> > > /**\n> > > * \\var CameraConfiguration::config_\n> > > * \\brief The vector of stream configurations\n> > > --\n> > > 2.20.1\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0AE11BF415\n\tfor ;\n\tWed, 9 Feb 2022 12:12:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69788610EC;\n\tWed, 9 Feb 2022 13:12:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B086D6106D\n\tfor ;\n\tWed, 9 Feb 2022 13:12:06 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4C1CC93;\n\tWed, 9 Feb 2022 13:12:06 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tve5P+nt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644408726;\n\tbh=bSH7yKHwoPR1XpGvQTWpd5ojxgJHHRBgOQWmI+8gKKE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=tve5P+nt6VrFYh6fdAFi5MLxGT44QUbsEyIYz9BZ24yKpXfVFGK6tHU4A5ez+4v3B\n\tjOJRx4YM0zRXvTxyOqXSnS7AuZLMBNrI+CZCdcYRPNiurUM0KRGxL4ZBIvjGjQbpez\n\toxbIpe0zMYHHYeG/rgEJEuG564+kjXZ/2xrO1soE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20210922132915.14881-1-david.plowman@raspberrypi.com>\n\t<20210922132915.14881-2-david.plowman@raspberrypi.com>\n\t<164129390422.3986460.993552112061270687@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"David Plowman ","Date":"Wed, 09 Feb 2022 12:12:03 +0000","Message-ID":"<164440872391.3354066.3555596882482877393@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22157,"web_url":"https://patchwork.libcamera.org/comment/22157/","msgid":"","date":"2022-02-09T12:59:48","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\n\nOn Wed, 9 Feb 2022 at 12:12, Kieran Bingham\n wrote:\n>\n> Hi David,\n>\n> Quoting David Plowman (2022-01-04 13:30:03)\n> > Hi Kieran\n> >\n> > Thanks for the reply.\n> >\n> > On Tue, 4 Jan 2022 at 10:58, Kieran Bingham\n> > wrote:\n> > >\n> > > Quoting David Plowman (2021-09-22 14:29:14)\n> > > > The modeSensitivity field is a number that describes how sensitive the\n> > > > selected sensor mode is compared to other readout modes of the same\n> > > > sensor. For example, a binned mode might have twice the sensitivity of\n> > > > the full resolution mode, meaning you would get double the signal\n> > > > level for the same exposure and gains.\n> > >\n> > > Is there a way to define which is the 'base' level?\n> >\n> > I suppose the \"base level\" would correspond to the mode that gives you\n> > the lowest signal. But as per the discussion below, it's all relative\n> > so doesn't matter unless we want to ascribe a particular meaning.\n> >\n> > >\n> > > Or does it not matter, as it's all relative (but relative to what I\n> > > guess is my question).\n> > >\n> > > I.e. Does it matter, or is it necessary to know that Mode A is 'half' as\n> > > sensitive as Mode B?\n> > >\n> > > In fact, I think that's already a given as whatever the pipeline handler\n> > > does will be relative. I.e. it would be about comparing the value\n> > > against other configurations not a standalone value.\n> > >\n> > > >\n> > > > Signed-off-by: David Plowman \n> > > > ---\n> > > > include/libcamera/camera.h | 2 ++\n> > > > src/libcamera/camera.cpp | 16 +++++++++++++++-\n> > > > 2 files changed, 17 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > > index 601ee46e..3314c06b 100644\n> > > > --- a/include/libcamera/camera.h\n> > > > +++ b/include/libcamera/camera.h\n> > > > @@ -66,6 +66,8 @@ public:\n> > > >\n> > > > Transform transform;\n> > > >\n> > > > + float modeSensitivity;\n> > > > +\n> > > > protected:\n> > > > CameraConfiguration();\n> > > >\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 71809bcd..8d795dff 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > > * \\brief Create an empty camera configuration\n> > > > */\n> > > > CameraConfiguration::CameraConfiguration()\n> > > > - : transform(Transform::Identity), config_({})\n> > > > + : transform(Transform::Identity), modeSensitivity(1.0), config_({})\n> > > > {\n> > > > }\n> > > >\n> > > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const\n> > > > * may adjust this field at its discretion if the selection is not supported.\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\var CameraConfiguration::modeSensitivity\n> > > > + * \\brief The relative sensitivity of the chosen sensor mode\n> > > > + *\n> > > > + * Some sensors have readout modes with different sensitivities. For example,\n> > > > + * a binned camera mode might, with the same exposure and gains, produce\n> > > > + * twice the signal level of the full resolution readout. This would be\n> > > > + * signalled by the binned mode, when it is chosen, indicating a value here\n> > > > + * that is twice that of the full resolution mode.\n> > > > + *\n> > > > + * This value should only be read, and not set, by the user. It will be\n> > > > + * valid after the configure method has reteurned successfully.\n>\n> s/reteurned/returned/\n>\n> > > > + */\n> > > > +\n> > >\n> > > I bet we've discussed this in the past, but I can't recall. If there are\n> > > more of these properties, I would suspect they should be in a\n> > > ControlList somehow - but if there's not expected to be many, an\n> > > explicit field is fine to me, and I can see the importance of conveying\n> > > this information.\n> >\n> > Yes, that's a good thought. One thing to note is that we need this\n> > value back during \"configure\", so you can calculate the right\n> > exposure/gain before calling \"start\". Could the configure method\n> > return a list of control values, I wonder?\n>\n> I'm sure a ControlList could be returned somewhere, or it could be a\n> part of the CameraConfiguration structure itself.\n>\n> The difficulty is that if we extend the structure here, it's an ABI\n> break / change, but if we have a ControlList to return these properties\n> - it can be updated/added to without breaking API/ABI compatibility.\n>\n> Looking at CameraConfiguration now, I see we have:\n>\n> Transform transform;\n>\n> Which could be considered a configuration property to put in a\n> ControlList too. And ... maybe there might be some ColourSpace property\n> to declare on the whole camera, rather than per stream? (Only because I\n> see a flag ColorSpaceFlag about whether ColorSpaces are per-stream or not)\n>\n> Would you consider a Transform and a ModeSensitivity to be something\n> that could be grouped together as CameraConfiguration::properties() ?\n>\n> Or does that become more complicated?\n\nDon't really mind, something like this could be in the properties so\nlong as it gets updated correctly once you've set the camera mode. As\nlong as the application can find out the magic number before calling\nstart().\n\nDavid\n\n>\n> > > I was also going to say perhaps we need some testing on this in\n> > > lc-compliance, but given that the default is 1.0 for all camera\n> > > configurations - I think that's fine. It wouldn't be something we could\n> > > easily test anyway.\n> >\n> > I have a sensor I'm working on that requires this feature - hopefully\n> > it won't be too long before others can try it!\n> >\n> > Thanks\n> > David\n> >\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > >\n> > >\n> > > > /**\n> > > > * \\var CameraConfiguration::config_\n> > > > * \\brief The vector of stream configurations\n> > > > --\n> > > > 2.20.1\n> > > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 837A0BF415\n\tfor ;\n\tWed, 9 Feb 2022 13:00:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4628610F5;\n\tWed, 9 Feb 2022 14:00:00 +0100 (CET)","from mail-wr1-x433.google.com (mail-wr1-x433.google.com\n\t[IPv6:2a00:1450:4864:20::433])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7C4D6106D\n\tfor ;\n\tWed, 9 Feb 2022 13:59:59 +0100 (CET)","by mail-wr1-x433.google.com with SMTP id s18so3906584wrv.7\n\tfor ;\n\tWed, 09 Feb 2022 04:59:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"EW9v3CpQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=62BPAULo3LIImrbs4/5W+NLA1qsU3nBl8mFyMIdxkns=;\n\tb=EW9v3CpQf0Be1WtpS6lhqaFUx/fzZ8S9ZBJjiwpOcY+niM8LDadGi8xfkc+KlMwRju\n\tnb4FwiYQW5VqIbqf+n1c8Yge8+hxt8L0GvH9t32R1W5GyGQwnOm/EBUtmHfYT3Cwh1mA\n\tfgX+v6I8+BPFe4RQEUxB7evhSVJjkz46DEviBEEwVNtnAPV5VC4VnCp7fAj3sTMWSefO\n\tnlb+9xNU04N81ecRlPVCghIx8hKnl+7tnGA9v+jbL1bqW1PHD9jhjD32a3fcSvg6xA9+\n\tMrA/FXLDlQHSJofOI5hxwhxCH6xCD8UE7QQYX+imTtT4gqNFiyuS+eoOR28HYeexwXA3\n\tYrxQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=62BPAULo3LIImrbs4/5W+NLA1qsU3nBl8mFyMIdxkns=;\n\tb=N5zfEQBnW8BcSnlFJHFsiteuRUl9X5z4Byq8639mKLOAJ3P6ELhI8jr6KeP6MYEHsr\n\thPZL2kf4aOsEQabCjwvbScw6++a0zBt7YhTy3tpyMRckrbQXK615yXI99k/XkHvoZQAR\n\t1sKX9lpuT1IhiJ4Rs3wYQ+KMN+e9+SYClUULiFY/HOR701pknvqPMwQwb0CRK2c5hcYg\n\tS0hUdzISqm5pPosPYPK4Kz/ulipzR+lntZYbOKjcuYVcZG476HzegNRIVddweONT8l+u\n\t5375ao9rEtn60NHctOzGh2PcKxMlQ7cXfCPtr6G42eHM4vDIkaVVudMpeubT+F6tDkrF\n\tIGGA==","X-Gm-Message-State":"AOAM530KkiEfAElYspg7SE9U1R0DNY4S2ExxnDmhjs3XpERSPb+48z3o\n\tpID4HUmr9k2noa2ibBjaM3yi+aZJQShe9Gz48uCarzHjDMxgwA==","X-Google-Smtp-Source":"ABdhPJxMTwUeKA6Ywim1x5O2eirwUXYnjSt5rPTfQU2Vtf+RecMbEJFC/QAk6vjaC2kceiACUdF5WNkLUJ5WeCKNWd8=","X-Received":"by 2002:a05:6000:178d:: with SMTP id\n\te13mr1964345wrg.317.1644411599326; \n\tWed, 09 Feb 2022 04:59:59 -0800 (PST)","MIME-Version":"1.0","References":"<20210922132915.14881-1-david.plowman@raspberrypi.com>\n\t<20210922132915.14881-2-david.plowman@raspberrypi.com>\n\t<164129390422.3986460.993552112061270687@Monstersaurus>\n\t\n\t<164440872391.3354066.3555596882482877393@Monstersaurus>","In-Reply-To":"<164440872391.3354066.3555596882482877393@Monstersaurus>","From":"David Plowman ","Date":"Wed, 9 Feb 2022 12:59:48 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22690,"web_url":"https://patchwork.libcamera.org/comment/22690/","msgid":"","date":"2022-04-13T09:10:17","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your work.\n\nOn Wed, 22 Sept 2021 at 14:29, David Plowman \nwrote:\n\n> The modeSensitivity field is a number that describes how sensitive the\n> selected sensor mode is compared to other readout modes of the same\n> sensor. For example, a binned mode might have twice the sensitivity of\n> the full resolution mode, meaning you would get double the signal\n> level for the same exposure and gains.\n>\n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/camera.h | 2 ++\n> src/libcamera/camera.cpp | 16 +++++++++++++++-\n> 2 files changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 601ee46e..3314c06b 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -66,6 +66,8 @@ public:\n>\n> Transform transform;\n>\n> + float modeSensitivity;\n> +\n> protected:\n> CameraConfiguration();\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 71809bcd..8d795dff 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> * \\brief Create an empty camera configuration\n> */\n> CameraConfiguration::CameraConfiguration()\n> - : transform(Transform::Identity), config_({})\n> + : transform(Transform::Identity), modeSensitivity(1.0), config_({})\n> {\n> }\n>\n> @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const\n> * may adjust this field at its discretion if the selection is not\n> supported.\n> */\n>\n> +/**\n> + * \\var CameraConfiguration::modeSensitivity\n> + * \\brief The relative sensitivity of the chosen sensor mode\n> + *\n> + * Some sensors have readout modes with different sensitivities. For\n> example,\n> + * a binned camera mode might, with the same exposure and gains, produce\n> + * twice the signal level of the full resolution readout. This would be\n> + * signalled by the binned mode, when it is chosen, indicating a value\n> here\n> + * that is twice that of the full resolution mode.\n> + *\n> + * This value should only be read, and not set, by the user. It will be\n> + * valid after the configure method has reteurned successfully.\n>\n\ns/reteurned/returned/\n\nAparat from that:\n\nReviewed-by: Naushir Patuck \n\n\n> + */\n> +\n> /**\n> * \\var CameraConfiguration::config_\n> * \\brief The vector of stream configurations\n> --\n> 2.20.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0329AC0F1B\n\tfor ;\n\tWed, 13 Apr 2022 09:10:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB9C265644;\n\tWed, 13 Apr 2022 11:10:35 +0200 (CEST)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 026DC604B3\n\tfor ;\n\tWed, 13 Apr 2022 11:10:33 +0200 (CEST)","by mail-lj1-x236.google.com with SMTP id b21so1396925ljf.4\n\tfor ;\n\tWed, 13 Apr 2022 02:10:33 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649841036;\n\tbh=q1LYznDROt4AFV+T/V5aJh18N5PS61BCM3irgkTLt9Q=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=B/uwZSkd+VC+DBU6a9NzoJu+HuRoG2jft5NW8yPdrwPn7QPjSHFpjnEgJ0LCMeM6r\n\tHdxtZv7AaOcI3ecEURpYWRqyHhVwHibhQKQ9cAhACq+NGVGtWg11zgC9t6fyA4JFhU\n\tL8xu6rxIyWqnCocFYvdX3WvZA1SD0E57kOTnZ1lJAC+Ubog/lnRKfZ9TZWSSHvSgjr\n\t0hOphrgXNgl9puC4M8xzcZ8af93+WCW7tRLMmPAUb/SZYv3sCYwBNAnYYm1JRyefmH\n\tzTLqaDKipCcGjw9p/GqWobtCHpzeSigqglF2sb/2qy2qHUZlzxnbfQF7hjth565JV/\n\tuivINpS5LRwAA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=GExsLEpIo7HZZP7i5y4vB+6UY9DJ28IW9e4vpzOdFas=;\n\tb=X+eI7tJlzHaE6DUZqq3ceWEraeydEPfkErjDkdgbbXUsaJCShum7PMeLV19/bL5MeI\n\tGQpSJH1zHBAezrR5YKLbhfbmtqlXCnp97etVJD17fYcpx8jOnWvYJZl1GR8c21cDqL9t\n\tbddbqVrLZGTT2nRy7QRAwTaUTCb4Fqshrm7upnnN/nzGzA0VRZdD53HWe5HT2H/X5BAw\n\tPvZNy8PNCfPUE8s9wVdRMVavtJh5KkHuTHGcMYoIaVBHjOOpUudTFO11myglj00Cldc4\n\tTDrfG4qQX7otfXFciFMU8hiVbQwDO0b9DUoxo2GyDsgKaKkJEke4be6ETqqlm81eCHzd\n\to0kQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"X+eI7tJl\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=GExsLEpIo7HZZP7i5y4vB+6UY9DJ28IW9e4vpzOdFas=;\n\tb=wzXx2S1y/4YJ3fooPl/klVgI5aAjndC+DadjvIME7XYW4GNuFmB/FYKNIrb88zed6n\n\tggPORZHVSHOocaNE3zpF9fxK6wx1eo4syblCDwUnZycPH0ixTC/YdzaCGkuW3usNij2p\n\tPZy9hsYH4TuTQr7ATNtZjIXGeP6gz6zflMM1pn44zPBOE7b3BFoqcezQ+XI9j9D1b58B\n\tviSIy6slkjdp/hkT5ZJZGKTV+Va0ELzE2Tt9XqbCKT2WQAl9VIA80PZcut5CbNiIi0r0\n\tAYSxrPglLYo9CePFlakTXzyeD2MKVJw5rvZAwPiZeBo0UutvAADA6d44nbR6FEoZcK+X\n\t/mfQ==","X-Gm-Message-State":"AOAM533Hoeye9YphtdfgGGGFL5atHn4w4+9OHfRAgiPOj5nup+7/L+au\n\t6ooIP3WQbJQqI5ZO4CAvUzmgAXx4JyQVfRkcDKYDaPylCuC3GA==","X-Google-Smtp-Source":"ABdhPJyYV/6uepNRNybHK5wiLYKFWnIoDBZEcZqf2Ir4VeOL0zZwg1d6pSGrzazexUZcSaoIhDOD5tJ1Eun/jz8aCUs=","X-Received":"by 2002:a2e:bf01:0:b0:247:dfe7:62dc with SMTP id\n\tc1-20020a2ebf01000000b00247dfe762dcmr26037166ljr.365.1649841033251;\n\tWed, 13 Apr 2022 02:10:33 -0700 (PDT)","MIME-Version":"1.0","References":"<20210922132915.14881-1-david.plowman@raspberrypi.com>\n\t<20210922132915.14881-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20210922132915.14881-2-david.plowman@raspberrypi.com>","Date":"Wed, 13 Apr 2022 10:10:17 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000b9205705dc858c0c\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode\n\tsensitivity field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]