[libcamera-devel] [PATCH v2 2/2] ipa: rpi: vc4: data: Update tuning files for HDR
David Plowman
david.plowman at raspberrypi.com
Mon Sep 18 11:06:28 CEST 2023
Hi Jacopo
Thanks for the review!
I think most of this relates again to the way we specify the different
AGC configs in the tuning file, and how we refer to the correct ones.
As per my reply to the previous patch, I think I will try a slightly
different scheme where we index the AGC configs with a name ("normal",
"short" or "long"). This is very similar to the way we handle this
elsewhere (e.g. AGC exposure modes and so on).
So it's probably not worth replying again here, instead let me post a
revised version and then we can revisit all this then!
Thanks
David
On Sat, 16 Sept 2023 at 14:08, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi David
>
> On Mon, Sep 11, 2023 at 04:35:13PM +0100, David Plowman via libcamera-devel wrote:
> > Hi Jacopo
> >
> > On Wed, 30 Aug 2023 at 10:40, Jacopo Mondi
> > <jacopo.mondi at ideasonboard.com> wrote:
> > >
> > > Hi David
> > >
> > > On Wed, Aug 23, 2023 at 03:49:25PM +0100, David Plowman via libcamera-devel wrote:
> > > > 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>
> > >
> > > Before reviewing the algorithm implementation itself let me ask a few
> > > question on the configuration file
> >
> > Of course...!
> >
> > >
> > > > ---
> > > > 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":
> > > > {
> > >
> > > [snip]
> > >
> > > > + "channels":
> > > > + [
> > > > + {
> > >
> > > an index won't hurt to easly connect it to entries in the hdr section
> >
> > I wasn't quite sure what you meant here. Are you suggesting including
> > an index number here that identifies it as libcamera's "long" (or
> > "short") channel? That's certainly possible, though I've tried to keep
> > HDR and AGC channels as separate concepts, the idea being that AGC
> > channels know nothing about HDR, but are merely something HDR will
> > use. Of course, in reality they are quite closely connected... though
> > I wouldn't want to predict exactly how in all cases. (Or maybe I've
> > misunderstood the question, in which case apologies!)
> >
>
> No no, that's what I meant..
>
> I see the HDR section using indexes to idntify channels and assign
> them names, in example
>
> "channel_map": { "short": 1, "long": 2 }
>
> Are the channels' indexes their definition order in the config file ?
> Isn't it slightly fragile ?
>
> > >
> > > > + "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
> > > > + ]
> > > > + },
> > > > + {
> > > > + "base_ev": 0.125,
> > >
> > > I guess this is what makes the channel "long exposure"
> >
> > Correct. (Actually "short exposure" in this case as it will be running
> > at 1/8 of normal exposure!)
> >
>
> Yeah, sorry
>
> > >
> > > > + "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
> > > > + ]
> > > > + },
> > > > + {
> > > > + "base_ev": 1.5,
> > > > + "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
> > > > + ]
> > > > + }
> > > > + ]
> > > > + }
> > > > },
> > > > {
> > > > "rpi.alsc":
> > > > @@ -463,6 +650,21 @@
> > > > },
> > > > {
> > > > "rpi.sharpen": { }
> > > > - }
> > > > + },
> > > > + {
> > > > + "rpi.hdr":
> > > > + {
> > >
> > >
> > > > + "MultiExposure":
> > > > + {
> > > > + "cadence": [ 1, 2 ],
> > > > + "channel_map": { "short": 1, "long": 2 }
> > > > + },
> > > > + "SingleExposure":
> > > > + {
> > > > + "cadence": [ 1 ],
> > > > + "channel_map": { "short": 1 }
> > > > + }
> > >
> > > Isn't the single exposure mode comparable to the non-hdr mode ? Do we
> > > need to specify a channel to use in this case, can't the AEGC
> > > algorithm be driven as usual ?
> >
> > Not necessarily. I could imagine a platform where the "single
> > exposure" method uses under-exposed images and is then followed by
> > some magic image accumulation and tone-mapping. But obviously we can't
> > do that in hardware on a Pi, you'd have to use some kind of software
> > post-processing.
>
> Ok, so this is a "shortcut" for selecting one of the above AGC
> channels and use that one only, right ?
>
> >
> > Thanks!
> > David
> >
> > >
> > > [snip]
> > >
More information about the libcamera-devel
mailing list