[libcamera-devel] [PATCH v4 2/2] ipa: rpi: vc4: data: Update tuning files for HDR

[David Plowman]

Hi Kieran

On Tue, 9 Jan 2024 at 10:43, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting David Plowman via libcamera-devel (2024-01-09 10:25:47)
> > All the Raspberry Pi official camera tuning files are updated for
> > HDR. As stated previously, there is no mechanism in the hardware for
> > combining images so all this does is enable multi-channel AGC to
> > produce short and long exposure frames. It will be up to the
> > application to deal with them.
> >
> > The changes are identical in every tuning file.
> >
> > 1. The existing AGC tuning is duplicated twice so that we have 3 AGC
> > channels.
> >
> > 2. The first is left alone (the default AGC channel), the second is
> > tweaked to under-expose significantly (ev -3) and the final one is
> > tweaked to over-exposure slightly (ev +0.5)
> >
> > 3. Control parameters are provided to the "rpi.hdr" algorithm to
> > associate these AGC channels correctly with the HDR modes.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/rpi/vc4/data/imx219.json           | 384 +++++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx219_noir.json      | 384 +++++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx477.json           | 376 +++++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx477_noir.json      | 384 +++++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx708.json           | 346 +++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx708_noir.json      | 346 +++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx708_wide.json      | 346 +++++++++++++-----
> >  src/ipa/rpi/vc4/data/imx708_wide_noir.json | 346 +++++++++++++-----
> >  src/ipa/rpi/vc4/data/ov5647.json           | 388 ++++++++++++++++-----
> >  9 files changed, 2524 insertions(+), 776 deletions(-)
> >
> > diff --git a/src/ipa/rpi/vc4/data/imx219.json b/src/ipa/rpi/vc4/data/imx219.json
> > index e8fce164..54defc0b 100644
> > --- a/src/ipa/rpi/vc4/data/imx219.json
> > +++ b/src/ipa/rpi/vc4/data/imx219.json
> > @@ -131,95 +131,282 @@
> >          {
> >              "rpi.agc":
> >              {
> > -                "metering_modes":
> > -                {
> > -                    "centre-weighted":
> > -                    {
> > -                        "weights": [ 3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0 ]
> > -                    },
> > -                    "spot":
> > -                    {
> > -                        "weights": [ 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]
> > -                    },
> > -                    "matrix":
> > -                    {
> > -                        "weights": [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ]
> > -                    }
> > -                },
> > -                "exposure_modes":
> > -                {
> > -                    "normal":
> > -                    {
> > -                        "shutter": [ 100, 10000, 30000, 60000, 66666 ],
> > -                        "gain": [ 1.0, 2.0, 4.0, 6.0, 8.0 ]
> > -                    },
> > -                    "short":
> > -                    {
> > -                        "shutter": [ 100, 5000, 10000, 20000, 33333 ],
> > -                        "gain": [ 1.0, 2.0, 4.0, 6.0, 8.0 ]
> > -                    },
> > -                    "long":
> > -                    {
> > -                        "shutter": [ 100, 10000, 30000, 60000, 120000 ],
> > -                        "gain": [ 1.0, 2.0, 4.0, 6.0, 12.0 ]
> > -                    }
> > -                },
> > -                "constraint_modes":
> > -                {
> > -                    "normal": [
> > -                        {
> > -                            "bound": "LOWER",
> > -                            "q_lo": 0.98,
> > -                            "q_hi": 1.0,
> > -                            "y_target":
> > -                            [
> > -                                0, 0.5,
> > -                                1000, 0.5
> > -                            ]
> > -                        }
> > -                    ],
> > -                    "highlight": [
> > -                        {
> > -                            "bound": "LOWER",
> > -                            "q_lo": 0.98,
> > -                            "q_hi": 1.0,
> > -                            "y_target":
> > -                            [
> > -                                0, 0.5,
> > -                                1000, 0.5
> > -                            ]
> > -                        },
> > -                        {
> > -                            "bound": "UPPER",
> > -                            "q_lo": 0.98,
> > -                            "q_hi": 1.0,
> > -                            "y_target":
> > -                            [
> > -                                0, 0.8,
> > -                                1000, 0.8
> > -                            ]
> > -                        }
> > -                    ],
> > -                    "shadows": [
> > -                        {
> > -                            "bound": "LOWER",
> > -                            "q_lo": 0.0,
> > -                            "q_hi": 0.5,
> > -                            "y_target":
> > -                            [
> > -                                0, 0.17,
> > -                                1000, 0.17
> > -                            ]
> > -                        }
> > -                    ]
> > -                },
> > -                "y_target":
> > -                [
> > -                    0, 0.16,
> > -                    1000, 0.165,
> > -                    10000, 0.17
> > -                ]
> > -            }
> > +               "channels":
> > +               [
> > +                   {
> > +                       "metering_modes":
>
> It looks like this 'breaks' ABI of the tuning files.
> Should there be a corresponding bump of the tuning file version number
> with a check? (2.1?) or does the rpi.agc implementation fall back to the
> 'unchanneled' layout if there are no channels?
>
> As long as it falls back then:

Yes, good to check this. But I think we're OK because Agc::read
(src/ipa/rpi/controller/rpi/agc.cpp line 34) still reads the
"channel-less" version as just being a single channel.

David

>
> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> --
> Kieran