[libcamera-devel] [PATCH 0/2] Raspberrypi: support per-mode camera sensitivities

[David Plowman]

Another reply to myself... :)

On Thu, 27 May 2021 at 14:40, David Plowman
<david.plowman at raspberrypi.com> wrote:
>
> Replying to myself...
>
> On Thu, 27 May 2021 at 09:45, David Plowman
> <david.plowman at raspberrypi.com> wrote:
> >
> > Hi
> >
> > Here's an implementation of "per-mode camera sensitivities", following
> > on from a recent discussion email. This is taking the "don't lie, even
> > a little" approach, which I am inclined to prefer. There are two
> > patches as follows, though still some outstanding questions.
> >
> > 1. The first patch adds a "sensitivity" field to our camera mode
> > structure. The main question here is how to set this value correctly.
> >
> > - For now, I've just used a virtual function in the CamHelper. This is
> >   dead simple and it works. (Though the sensor that needs this
> >   function is still unreleased.)
> >
> > - But should I be using the sensor database? I'd have to add fields
> >   for "full resolution sensitivity", "2x2 binned sensitivity" as a
> >   minimum, though why stop there? There could be many different
> >   binning modes! Also, I'd have to get this through to the IPA. Would
> >   that mean writing a serialiser/deserialiser? Some clues on how to do
> >   that would be helpful, I looked at ipa_data_serializer.cpp but it
> >   was quite intimidating...
>
> Having stared at this a bit more, I see one could move structs into
> the core.mojom file (like IPACameraSensorInfo), where presumably
> things get taken care of for you. Or you could add a
> serialiser/deserialiser in ipa_data_serializer.cpp. I notice that
> DEFINE_POD_SERIALIZER only works with numeric types but presumably
> it's only a small tweak to make it work for anything that is "standard
> layout"? Would that work?
>
> Though I'm still not liking this solution too much because it's
> unclear how many "mode sensitivities" one would need to add, or indeed
> if mode sensitivities might differ for reasons other than binning...
> (and there's always "way out there" stuff like HDR modes)
>
> David
>
> >
> > - Or would we be better getting the value from the driver? That might
> >   need a new read-only V4L2 control. And how would we pass that to the
> >   IPA, in the IPACameraSensorInfo? It already has mode-dependent
> >   fields...
> >
> > 2. The second patch takes care of the changing sensitivity in the
> > AEC/AGC, when the SwitchMode method is called.
> >
> > Finally, I think there's still a problem with this approach. Suppose
> > you wanted to run preview, then do a capture with the same output
> > brightness, but selecting the exposure and gain explicitly for
> > yourself. You simply can't do this without knowing the sensitivity of
> > the camera modes. So how could we report this? Note that signalling it
> > in metadata with completed requests is too late - the typical pattern
> > would be:
> >
> > Stop camera -> Reconfigure camera -> Recalculate exposure/gain ->
> > Start camera

I'd also be interested in people's thoughts on this question - how to
tell the application what the mode sensitivity is. Could it be
returned in the camera configuration perhaps?

Thanks
David

> >
> > Opinions both sought and appreciated!
> >
> > Best regards
> > David
> >
> > David Plowman (2):
> >   ipa: raspberrypi: Add sensitivity field to camera mode
> >   ipa: raspberrypi: AGC: handle modes with different sensitivities
> >
> >  src/ipa/raspberrypi/cam_helper.cpp           |  5 ++++
> >  src/ipa/raspberrypi/cam_helper.hpp           |  3 +++
> >  src/ipa/raspberrypi/controller/camera_mode.h |  2 ++
> >  src/ipa/raspberrypi/controller/rpi/agc.cpp   | 25 ++++++++++++++++----
> >  src/ipa/raspberrypi/controller/rpi/agc.hpp   |  1 +
> >  src/ipa/raspberrypi/raspberrypi.cpp          | 12 ++++++++++
> >  6 files changed, 43 insertions(+), 5 deletions(-)
> >
> > --
> > 2.20.1
> >