[libcamera-devel] [PATCH v2 1/2] libcamera: camera: Add a mode sensitivity field

[David Plowman]

Hi Kieran

Thanks for the reply.

On Tue, 4 Jan 2022 at 10:58, Kieran Bingham
<kieran.bingham at 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 at 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 at ideasonboard.com>
>
>
>
> >  /**
> >   * \var CameraConfiguration::config_
> >   * \brief The vector of stream configurations
> > --
> > 2.20.1
> >