| Message ID | 20210922132915.14881-2-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting David Plowman (2021-09-22 14:29:14) > The modeSensitivity field is a number that describes how sensitive the > selected sensor mode is compared to other readout modes of the same > sensor. For example, a binned mode might have twice the sensitivity of > the full resolution mode, meaning you would get double the signal > level for the same exposure and gains. Is there a way to define which is the 'base' level? Or does it not matter, as it's all relative (but relative to what I guess is my question). I.e. Does it matter, or is it necessary to know that Mode A is 'half' as sensitive as Mode B? In fact, I think that's already a given as whatever the pipeline handler does will be relative. I.e. it would be about comparing the value against other configurations not a standalone value. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > include/libcamera/camera.h | 2 ++ > src/libcamera/camera.cpp | 16 +++++++++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 601ee46e..3314c06b 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -66,6 +66,8 @@ public: > > Transform transform; > > + float modeSensitivity; > + > protected: > CameraConfiguration(); > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 71809bcd..8d795dff 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) > * \brief Create an empty camera configuration > */ > CameraConfiguration::CameraConfiguration() > - : transform(Transform::Identity), config_({}) > + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) > { > } > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const > * may adjust this field at its discretion if the selection is not supported. > */ > > +/** > + * \var CameraConfiguration::modeSensitivity > + * \brief The relative sensitivity of the chosen sensor mode > + * > + * Some sensors have readout modes with different sensitivities. For example, > + * a binned camera mode might, with the same exposure and gains, produce > + * twice the signal level of the full resolution readout. This would be > + * signalled by the binned mode, when it is chosen, indicating a value here > + * that is twice that of the full resolution mode. > + * > + * This value should only be read, and not set, by the user. It will be > + * valid after the configure method has reteurned successfully. > + */ > + I bet we've discussed this in the past, but I can't recall. If there are more of these properties, I would suspect they should be in a ControlList somehow - but if there's not expected to be many, an explicit field is fine to me, and I can see the importance of conveying this information. I was also going to say perhaps we need some testing on this in lc-compliance, but given that the default is 1.0 for all camera configurations - I think that's fine. It wouldn't be something we could easily test anyway. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > /** > * \var CameraConfiguration::config_ > * \brief The vector of stream configurations > -- > 2.20.1 >
Hi Kieran Thanks for the reply. On Tue, 4 Jan 2022 at 10:58, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting David Plowman (2021-09-22 14:29:14) > > The modeSensitivity field is a number that describes how sensitive the > > selected sensor mode is compared to other readout modes of the same > > sensor. For example, a binned mode might have twice the sensitivity of > > the full resolution mode, meaning you would get double the signal > > level for the same exposure and gains. > > Is there a way to define which is the 'base' level? I suppose the "base level" would correspond to the mode that gives you the lowest signal. But as per the discussion below, it's all relative so doesn't matter unless we want to ascribe a particular meaning. > > Or does it not matter, as it's all relative (but relative to what I > guess is my question). > > I.e. Does it matter, or is it necessary to know that Mode A is 'half' as > sensitive as Mode B? > > In fact, I think that's already a given as whatever the pipeline handler > does will be relative. I.e. it would be about comparing the value > against other configurations not a standalone value. > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > --- > > include/libcamera/camera.h | 2 ++ > > src/libcamera/camera.cpp | 16 +++++++++++++++- > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index 601ee46e..3314c06b 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -66,6 +66,8 @@ public: > > > > Transform transform; > > > > + float modeSensitivity; > > + > > protected: > > CameraConfiguration(); > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 71809bcd..8d795dff 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) > > * \brief Create an empty camera configuration > > */ > > CameraConfiguration::CameraConfiguration() > > - : transform(Transform::Identity), config_({}) > > + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) > > { > > } > > > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const > > * may adjust this field at its discretion if the selection is not supported. > > */ > > > > +/** > > + * \var CameraConfiguration::modeSensitivity > > + * \brief The relative sensitivity of the chosen sensor mode > > + * > > + * Some sensors have readout modes with different sensitivities. For example, > > + * a binned camera mode might, with the same exposure and gains, produce > > + * twice the signal level of the full resolution readout. This would be > > + * signalled by the binned mode, when it is chosen, indicating a value here > > + * that is twice that of the full resolution mode. > > + * > > + * This value should only be read, and not set, by the user. It will be > > + * valid after the configure method has reteurned successfully. > > + */ > > + > > I bet we've discussed this in the past, but I can't recall. If there are > more of these properties, I would suspect they should be in a > ControlList somehow - but if there's not expected to be many, an > explicit field is fine to me, and I can see the importance of conveying > this information. Yes, that's a good thought. One thing to note is that we need this value back during "configure", so you can calculate the right exposure/gain before calling "start". Could the configure method return a list of control values, I wonder? > > > I was also going to say perhaps we need some testing on this in > lc-compliance, but given that the default is 1.0 for all camera > configurations - I think that's fine. It wouldn't be something we could > easily test anyway. I have a sensor I'm working on that requires this feature - hopefully it won't be too long before others can try it! Thanks David > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > /** > > * \var CameraConfiguration::config_ > > * \brief The vector of stream configurations > > -- > > 2.20.1 > >
Hi David, Quoting David Plowman (2022-01-04 13:30:03) > Hi Kieran > > Thanks for the reply. > > On Tue, 4 Jan 2022 at 10:58, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: > > > > Quoting David Plowman (2021-09-22 14:29:14) > > > The modeSensitivity field is a number that describes how sensitive the > > > selected sensor mode is compared to other readout modes of the same > > > sensor. For example, a binned mode might have twice the sensitivity of > > > the full resolution mode, meaning you would get double the signal > > > level for the same exposure and gains. > > > > Is there a way to define which is the 'base' level? > > I suppose the "base level" would correspond to the mode that gives you > the lowest signal. But as per the discussion below, it's all relative > so doesn't matter unless we want to ascribe a particular meaning. > > > > > Or does it not matter, as it's all relative (but relative to what I > > guess is my question). > > > > I.e. Does it matter, or is it necessary to know that Mode A is 'half' as > > sensitive as Mode B? > > > > In fact, I think that's already a given as whatever the pipeline handler > > does will be relative. I.e. it would be about comparing the value > > against other configurations not a standalone value. > > > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > --- > > > include/libcamera/camera.h | 2 ++ > > > src/libcamera/camera.cpp | 16 +++++++++++++++- > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > > index 601ee46e..3314c06b 100644 > > > --- a/include/libcamera/camera.h > > > +++ b/include/libcamera/camera.h > > > @@ -66,6 +66,8 @@ public: > > > > > > Transform transform; > > > > > > + float modeSensitivity; > > > + > > > protected: > > > CameraConfiguration(); > > > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > > index 71809bcd..8d795dff 100644 > > > --- a/src/libcamera/camera.cpp > > > +++ b/src/libcamera/camera.cpp > > > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) > > > * \brief Create an empty camera configuration > > > */ > > > CameraConfiguration::CameraConfiguration() > > > - : transform(Transform::Identity), config_({}) > > > + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) > > > { > > > } > > > > > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const > > > * may adjust this field at its discretion if the selection is not supported. > > > */ > > > > > > +/** > > > + * \var CameraConfiguration::modeSensitivity > > > + * \brief The relative sensitivity of the chosen sensor mode > > > + * > > > + * Some sensors have readout modes with different sensitivities. For example, > > > + * a binned camera mode might, with the same exposure and gains, produce > > > + * twice the signal level of the full resolution readout. This would be > > > + * signalled by the binned mode, when it is chosen, indicating a value here > > > + * that is twice that of the full resolution mode. > > > + * > > > + * This value should only be read, and not set, by the user. It will be > > > + * valid after the configure method has reteurned successfully. s/reteurned/returned/ > > > + */ > > > + > > > > I bet we've discussed this in the past, but I can't recall. If there are > > more of these properties, I would suspect they should be in a > > ControlList somehow - but if there's not expected to be many, an > > explicit field is fine to me, and I can see the importance of conveying > > this information. > > Yes, that's a good thought. One thing to note is that we need this > value back during "configure", so you can calculate the right > exposure/gain before calling "start". Could the configure method > return a list of control values, I wonder? I'm sure a ControlList could be returned somewhere, or it could be a part of the CameraConfiguration structure itself. The difficulty is that if we extend the structure here, it's an ABI break / change, but if we have a ControlList to return these properties - it can be updated/added to without breaking API/ABI compatibility. Looking at CameraConfiguration now, I see we have: Transform transform; Which could be considered a configuration property to put in a ControlList too. And ... maybe there might be some ColourSpace property to declare on the whole camera, rather than per stream? (Only because I see a flag ColorSpaceFlag about whether ColorSpaces are per-stream or not) Would you consider a Transform and a ModeSensitivity to be something that could be grouped together as CameraConfiguration::properties() ? Or does that become more complicated? > > I was also going to say perhaps we need some testing on this in > > lc-compliance, but given that the default is 1.0 for all camera > > configurations - I think that's fine. It wouldn't be something we could > > easily test anyway. > > I have a sensor I'm working on that requires this feature - hopefully > it won't be too long before others can try it! > > Thanks > David > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > > > /** > > > * \var CameraConfiguration::config_ > > > * \brief The vector of stream configurations > > > -- > > > 2.20.1 > > >
Hi Kieran On Wed, 9 Feb 2022 at 12:12, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi David, > > Quoting David Plowman (2022-01-04 13:30:03) > > Hi Kieran > > > > Thanks for the reply. > > > > On Tue, 4 Jan 2022 at 10:58, Kieran Bingham > > <kieran.bingham@ideasonboard.com> wrote: > > > > > > Quoting David Plowman (2021-09-22 14:29:14) > > > > The modeSensitivity field is a number that describes how sensitive the > > > > selected sensor mode is compared to other readout modes of the same > > > > sensor. For example, a binned mode might have twice the sensitivity of > > > > the full resolution mode, meaning you would get double the signal > > > > level for the same exposure and gains. > > > > > > Is there a way to define which is the 'base' level? > > > > I suppose the "base level" would correspond to the mode that gives you > > the lowest signal. But as per the discussion below, it's all relative > > so doesn't matter unless we want to ascribe a particular meaning. > > > > > > > > Or does it not matter, as it's all relative (but relative to what I > > > guess is my question). > > > > > > I.e. Does it matter, or is it necessary to know that Mode A is 'half' as > > > sensitive as Mode B? > > > > > > In fact, I think that's already a given as whatever the pipeline handler > > > does will be relative. I.e. it would be about comparing the value > > > against other configurations not a standalone value. > > > > > > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > --- > > > > include/libcamera/camera.h | 2 ++ > > > > src/libcamera/camera.cpp | 16 +++++++++++++++- > > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > > > index 601ee46e..3314c06b 100644 > > > > --- a/include/libcamera/camera.h > > > > +++ b/include/libcamera/camera.h > > > > @@ -66,6 +66,8 @@ public: > > > > > > > > Transform transform; > > > > > > > > + float modeSensitivity; > > > > + > > > > protected: > > > > CameraConfiguration(); > > > > > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > > > index 71809bcd..8d795dff 100644 > > > > --- a/src/libcamera/camera.cpp > > > > +++ b/src/libcamera/camera.cpp > > > > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) > > > > * \brief Create an empty camera configuration > > > > */ > > > > CameraConfiguration::CameraConfiguration() > > > > - : transform(Transform::Identity), config_({}) > > > > + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) > > > > { > > > > } > > > > > > > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const > > > > * may adjust this field at its discretion if the selection is not supported. > > > > */ > > > > > > > > +/** > > > > + * \var CameraConfiguration::modeSensitivity > > > > + * \brief The relative sensitivity of the chosen sensor mode > > > > + * > > > > + * Some sensors have readout modes with different sensitivities. For example, > > > > + * a binned camera mode might, with the same exposure and gains, produce > > > > + * twice the signal level of the full resolution readout. This would be > > > > + * signalled by the binned mode, when it is chosen, indicating a value here > > > > + * that is twice that of the full resolution mode. > > > > + * > > > > + * This value should only be read, and not set, by the user. It will be > > > > + * valid after the configure method has reteurned successfully. > > s/reteurned/returned/ > > > > > + */ > > > > + > > > > > > I bet we've discussed this in the past, but I can't recall. If there are > > > more of these properties, I would suspect they should be in a > > > ControlList somehow - but if there's not expected to be many, an > > > explicit field is fine to me, and I can see the importance of conveying > > > this information. > > > > Yes, that's a good thought. One thing to note is that we need this > > value back during "configure", so you can calculate the right > > exposure/gain before calling "start". Could the configure method > > return a list of control values, I wonder? > > I'm sure a ControlList could be returned somewhere, or it could be a > part of the CameraConfiguration structure itself. > > The difficulty is that if we extend the structure here, it's an ABI > break / change, but if we have a ControlList to return these properties > - it can be updated/added to without breaking API/ABI compatibility. > > Looking at CameraConfiguration now, I see we have: > > Transform transform; > > Which could be considered a configuration property to put in a > ControlList too. And ... maybe there might be some ColourSpace property > to declare on the whole camera, rather than per stream? (Only because I > see a flag ColorSpaceFlag about whether ColorSpaces are per-stream or not) > > Would you consider a Transform and a ModeSensitivity to be something > that could be grouped together as CameraConfiguration::properties() ? > > Or does that become more complicated? Don't really mind, something like this could be in the properties so long as it gets updated correctly once you've set the camera mode. As long as the application can find out the magic number before calling start(). David > > > > I was also going to say perhaps we need some testing on this in > > > lc-compliance, but given that the default is 1.0 for all camera > > > configurations - I think that's fine. It wouldn't be something we could > > > easily test anyway. > > > > I have a sensor I'm working on that requires this feature - hopefully > > it won't be too long before others can try it! > > > > Thanks > > David > > > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > > > > > > > /** > > > > * \var CameraConfiguration::config_ > > > > * \brief The vector of stream configurations > > > > -- > > > > 2.20.1 > > > >
Hi David, Thank you for your work. On Wed, 22 Sept 2021 at 14:29, David Plowman <david.plowman@raspberrypi.com> wrote: > The modeSensitivity field is a number that describes how sensitive the > selected sensor mode is compared to other readout modes of the same > sensor. For example, a binned mode might have twice the sensitivity of > the full resolution mode, meaning you would get double the signal > level for the same exposure and gains. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > include/libcamera/camera.h | 2 ++ > src/libcamera/camera.cpp | 16 +++++++++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 601ee46e..3314c06b 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -66,6 +66,8 @@ public: > > Transform transform; > > + float modeSensitivity; > + > protected: > CameraConfiguration(); > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 71809bcd..8d795dff 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) > * \brief Create an empty camera configuration > */ > CameraConfiguration::CameraConfiguration() > - : transform(Transform::Identity), config_({}) > + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) > { > } > > @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const > * may adjust this field at its discretion if the selection is not > supported. > */ > > +/** > + * \var CameraConfiguration::modeSensitivity > + * \brief The relative sensitivity of the chosen sensor mode > + * > + * Some sensors have readout modes with different sensitivities. For > example, > + * a binned camera mode might, with the same exposure and gains, produce > + * twice the signal level of the full resolution readout. This would be > + * signalled by the binned mode, when it is chosen, indicating a value > here > + * that is twice that of the full resolution mode. > + * > + * This value should only be read, and not set, by the user. It will be > + * valid after the configure method has reteurned successfully. > s/reteurned/returned/ Aparat from that: Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > + */ > + > /** > * \var CameraConfiguration::config_ > * \brief The vector of stream configurations > -- > 2.20.1 > >
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 601ee46e..3314c06b 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -66,6 +66,8 @@ public: Transform transform; + float modeSensitivity; + protected: CameraConfiguration(); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 71809bcd..8d795dff 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -156,7 +156,7 @@ LOG_DECLARE_CATEGORY(Camera) * \brief Create an empty camera configuration */ CameraConfiguration::CameraConfiguration() - : transform(Transform::Identity), config_({}) + : transform(Transform::Identity), modeSensitivity(1.0), config_({}) { } @@ -327,6 +327,20 @@ std::size_t CameraConfiguration::size() const * may adjust this field at its discretion if the selection is not supported. */ +/** + * \var CameraConfiguration::modeSensitivity + * \brief The relative sensitivity of the chosen sensor mode + * + * Some sensors have readout modes with different sensitivities. For example, + * a binned camera mode might, with the same exposure and gains, produce + * twice the signal level of the full resolution readout. This would be + * signalled by the binned mode, when it is chosen, indicating a value here + * that is twice that of the full resolution mode. + * + * This value should only be read, and not set, by the user. It will be + * valid after the configure method has reteurned successfully. + */ + /** * \var CameraConfiguration::config_ * \brief The vector of stream configurations
The modeSensitivity field is a number that describes how sensitive the selected sensor mode is compared to other readout modes of the same sensor. For example, a binned mode might have twice the sensitivity of the full resolution mode, meaning you would get double the signal level for the same exposure and gains. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- include/libcamera/camera.h | 2 ++ src/libcamera/camera.cpp | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)