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

[David Plowman]

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
>
> 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
>