Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New digital gain handling #98

Draft
wants to merge 28 commits into
base: next
Choose a base branch
from
Draft

New digital gain handling #98

wants to merge 28 commits into from

Conversation

davidplowman
Copy link
Collaborator

@davidplowman davidplowman commented Dec 19, 2023

This certainly isn't ready for merging yet, but it might be worth starting to have a look at it.

The digital gain calculation is moved out of prepare() and into process().

The IPA classes are updated to fetch the digital gain from the delayed status, so we can no longer have the problem where prepare() gets skipped (at higher framerates) causing the image to "wink" with an old digital gain value.

There are still some things to think about.

For instance, the updating of the "actual" exposure/gain values when we write them to the sensor means we have to write them back to the metadata so that the correctly adjusted digital gain will be in the delayed status.

The split of platformPrepareIsp into platformPrepareIsp and platformPrepareAgc (the latter always being called, even when platformPrepareIsp is skipped).

Also the way I'm storing the AgcStatus from the switchMode so that I can use its values until the delayed status reappears, and probably other stuff too.

naushir and others added 28 commits November 30, 2023 17:03
Add a new subpoject wrap file for the libpisp library located at
https://github.com/raspberrypi/libpisp

The libpisp library is used to configure the Raspberry Pi 5 Frontend
and Backend ISP components.

Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
Add the relevant definitions for a 16-bit mono pixel and media-bus
format.

Signed-off-by: Naushir Patuck <[email protected]>
Add support for 16-bps (48-bpp) RGB output formats. These new formats
are define for the RGB and BGR ordering.

Signed-off-by: Naushir Patuck <[email protected]>
Add the Raspberry Pi 5 PiSP specific verification format:
- V4L2_PIX_FMT_RPI_BE

Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:
- V4L2_PIX_FMT_PISP_COMP1_xxx
- V4L2_PIX_FMT_PISP_COMP2_xxx

Add the Raspberry Pi 5 PiSP Frontend and Backend config formats:
- V4L2_META_FMT_RPI_FE_CFG
- V4L2_META_FMT_RPI_BE_CFG

Add the Raspberry Pi 5 PiSP Frontend statistics format:
- V4L2_META_FMT_RPI_FE_STATS

Add 16-bit Bayer formats:
- MEDIA_BUS_FMT_Sxxx16_1X16

Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
Add support for the following HDR modes in the Raspberry Pi IPA:
- Night mode
- Single exposure mode
- Multi-exposure (merged and unmerged)

The algorithm is updated to expect the HDR short channel to meter
explicitly for highlights. This means that it will not in general
under-expose the short channel more than is actually necessary.

When images don't have much saturation, it's good to detect this so
that some of the boost we want to apply to the dark areas can be
implemented as regular gain. This means we can then adjust the tone
curve less, leading to less flat looking images.

The impact on the HDR algorithm is then that this determines how we
build tonemaps dynamically. The highlights are more-or-less correct
now, so we have to build a power-type curve that gives us the
appropriately configured targets in the lower part of the histogram.

We allow the tuning file to supply the maximum spatial gain value,
rather than the whole curve (though it can supply this if it
wants). Some parameter defaults are tweaked to be generally better
across the range of our cameras.

Signed-off-by: David Plowman <[email protected]>
Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
The following member variable will be used by the derived IPAs in an
upcoming commit, so make them protected:

lensPresent_
monoSensor_
libcameraMetadata_

Signed-off-by: Naushir Patuck <[email protected]>
Add the Raspberry Pi 5 ISP (PiSP) pipeline handler to libcamera. To
include this pipeline handler in the build, set the following meson
option:

meson configure -Dpipelines=rpi/pisp

Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
Add the Raspberry Pi 5 ISP (PiSP) IPA to libcamera. To include this IPA
in the build, set the following meson option:

meson configure -Dipas=rpi/pisp

Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
The old ctt.py and alsc_only.py scripts are removed.

Instead of ctt.py use ctt_vc4.py or ctt_pisp.py, depending on your
target platform.

Instead of alsc_only.py use alsc_vc4.py or alsc_pisp.py, again
according to your platform.

Signed-off-by: David Plowman <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Added the ability to tune the chromatic aberration correction
within the ctt. There are options for cac_only or to tune as part
of a larger tuning process. CTT will now recognise any files that
begin with "cac" as being chromatic aberration tuning files.

Signed-off-by: Ben Benson <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Changed how users select which platform to tune for. Now users
specify a command line argument, '-t', to specify which target
platform.

Signed-off-by: Ben Benson <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
The various boilerplate parts of the tuning file are extended to
include the necessary extra bits for HDR, specifically:

* rpi.denoise has different configurations for HDR modes
* rpi.agc now has extra channels for HDR
* rpi.hdr parameters are added.

Signed-off-by: David Plowman <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
The IMX708 sensor driver advertises its module variants (narrow/wide angle lens,
IR block/pass) by modifying the media entity name string. So add duplicate
entries for each variant.

Signed-off-by: Nick Hollinghurst <[email protected]>
Signed-off-by: Naushir Patuck <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Reviewed-by: David Plowman <[email protected]>
Look for the RAW mandatory stream flag in the pipeline handler config
file. If this flag is set, it guarantees that the application will
provide buffers for Unicam Image, so override the minUnicamBuffers and
minTotalUnicamBuffers config parameters in the following way:

- If startup drop frames are required, allocate at least 1 internal buffer.
- If no startup drop frames are required, do not allocate any internal buffers.

Look for the Output 0 mandatory stream flag in in the pipeline handler
config file. If this flag is set, it guarantees that the application
will provide buffers for the ISP, do not allocate any internal buffers
for the device.

Add a new rpi_apps.yaml pipeline handler config file that enables both
these flags.  To use the file, set the following env variable for a
custom build:

export LIBCAMERA_RPI_CONFIG_FILE=/usr/local/share/libcamera/pipeline/rpi/vc4/rpi_apps.yaml

or for a packaged install:

export LIBCAMERA_RPI_CONFIG_FILE=/usr/share/libcamera/pipeline/rpi/vc4/rpi_apps.yaml

Signed-off-by: Naushir Patuck <[email protected]>
Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
vendor controls. This contains a single control PispConfigDumpFile that
will be used in the Pi 5 pipeline handler as a trigger to dump the
Backend configuration as a JSON file.

Signed-off-by: Naushir Patuck <[email protected]>
This allows the IPA to discover the correct black level values even
before any frames have been processed. This is important on the PiSP
platform where the front end black level blocks must be programmed in
advance.

Signed-off-by: David Plowman <[email protected]>
This allows the IPA to get reasonable default colour gains before AWB
has run. This is particularly important on the PiSP platform where
these numbers are helpful in programming the Front End statistics
block in advance.

Signed-off-by: David Plowman <[email protected]>
On the PiSP platform we have to program the Front End statistics
before we've have a chance to run any of the algorithms. So instead we
ask them for reasonable default values which we program in for the
first few frames.

Signed-off-by: David Plowman <[email protected]>
Previously these were handled in the AGC/AEC exposure update
calculations by explicitly driving a higher digital gain to "cancel
out" any colour gains that were less than 1.

Now we're ignoring this in the AGC and leaving it to the IPA code to
normalise all the gains so that the smallest is 1. We don't regard
this as a "real" increase because one of the colour channels (just not
necessarily the green one) still gets the minimum gain possible.

We do, however, update the statistics calculations so that they
reflect any such digital gain increase, so that images are driven to
the correct level.

Signed-off-by: David Plowman <[email protected]>
The delay context counter must be advanced even when we decide not to
run prepare/process. Otherwise, when we skip them at higher
framerates, the current IPA context counter will catch up and
overwrite the delay context.

It's safe to advance the counter because the metadata is always copied
forward a slot when we decide not to run the IPAs.

Signed-off-by: David Plowman <[email protected]>
The maximum allowed digital gain is hard-coded to 4. Make it a
configurable parameter.

Signed-off-by: David Plowman <[email protected]>
Much of the time we use the term "analogue gain" where we really mean
the combined analogue and digital gain (because the digital gain will
make up whatever the analogue gain can't deliver).

This commit replaces the use of "analogue gain" by just "gain" in
places where we really mean the combined gain. There are a couple of
principle areas:

1. Where we previously talked about the "fixedAnalaogueGain"
(including setting it "manually") this is now just the "fixedGain"
(because it always encompassed both analogue and digital gain).

Along with this, the setFixedShutter/Gain functions no longer update
the output status directly. Applications should wait in the usual way
for AGC/AEC changes to take effect, and this "shortcut" actually
doesn't fit well with the gain being the combined gain.

2. The divideUpExposure method is adjusted to be clearer that it's
setting the combined gain, and it's prepare() that will discover later
what the analogue gain actually delivered.

Signed-off-by: David Plowman <[email protected]>
Previously we let prepare() do the work by comparing the desired total
exposure against the shutter time and analogue gain. This can cause
the image to "wink" at high framerates because we may skip running
prepare() to get the new digital gain even when the delayed AGC status
(which came out of an earlier call to process()) shows that a change
was required.

Now we're taking explicit control of the digital gain by calculating
it ourselves so that we can output it in the standard AgcStatus
object. This means that whenever the delayed AGC status changes, we
have the correct digital gain to go with it.

Signed-off-by: David Plowman <[email protected]>
Here we update the digital gain handling to use the value computed by
process() in the AgcStatus, not the version that was previously in the
AgcPrepareStatus.

Because we apply this digital gain directly with no further
modification, we have to update it to reflect any exposure/gain
quantisation that happens (in IpaBase::applyAGC).

We must also run the new platformPrepareAgc() even when we're skipping
platformPrepareIsp(), which has been split out of the previous
platformPrepareIsp() implementation.

Signed-off-by: David Plowman <[email protected]>
The digital gain now comes from the AgcStatus, not from the
AgcPrepareStatus.

Signed-off-by: David Plowman <[email protected]>
All platforms are now using the digital gain from the AgcStatus, so
the AgcPrepareStatus and prepare() methods can be tidied up.

Signed-off-by: David Plowman <[email protected]>
@davidplowman
Copy link
Collaborator Author

One little thing to note is that the digital gain reported in metadata doesn't get updated according to the minimum colour gain. That is, if the red colour gain was 0.8, the digital gain would previously have been reported as 1.25 (= 1 / 0.8). It was, in fact, always reporting the green gain. Now, however, it will report 1.0 (actually the lowest of the gains applied to any channel, in this case it will be the red rather than the green one). How do we feel about that?

@naushir naushir force-pushed the next branch 5 times, most recently from 4a130f9 to 075b54d Compare February 15, 2024 14:54
@naushir naushir force-pushed the next branch 2 times, most recently from ebda3b7 to eb00c13 Compare April 18, 2024 13:17
@naushir naushir force-pushed the next branch 3 times, most recently from 37f2d98 to 6ddd79b Compare June 17, 2024 09:45
@naushir naushir force-pushed the next branch 2 times, most recently from eb29254 to 48fe316 Compare August 29, 2024 15:21
@naushir naushir force-pushed the next branch 2 times, most recently from 5da5394 to 75fe515 Compare November 12, 2024 11:25
@naushir naushir force-pushed the next branch 2 times, most recently from 53cf84d to b7ba311 Compare December 12, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants