libcamera: Adding LensShadingCorrection maps and ToneCurve to controls metadata

28219 diff mbox series

From: Michael Kunz <mkunz@articimaging.eu>

Having the LensShadingCorrection maps and ToneCurves available in frame
metadata allows creating a DNG file with all necessary information so that the
DNG matches the JPEG image without colour casts.

I opened a PR in raspberrypi/rpicam-apps
(https://github.com/raspberrypi/rpicam-apps/pull/928) improving the colour
accuracy of said DNG files created by these apps. The information needed is
currently not provided by libcamera in the frame metadata.

This patch intends to add LensShadingCorrection maps and ToneCurve to the
controls metadata in libcamera, so that DNG files can be written with correct
colours.

Signed-off-by: Michael Kunz <mkunz@articimaging.eu>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
This patch is forwarded on behalf of Michael Kunz. It appears there is
some issue on the mail server which is rejecting his posting. A double
signed DKIM is being rejected by the OpenDKIM instance on the libcamera
mail server.

While we try to investigate this further, which might involve an upgrade
to the mail server instance, I've applied the patch locally, compiled it
and sending via git-send-email from me instead.

Applying this patch had a conflict which required me to manually apply
the hunks to src/ipa/rpi/common/ipa_base.cpp but I think everything here
is enough to still continue to review and discussion. But I'll leave
that to the patch!
---
 src/ipa/rpi/common/ipa_base.cpp     | 50 ++++++++++++++++++++++++++++++++++---
 src/ipa/rpi/common/ipa_base.h       |  1 +
 src/ipa/rpi/common/meson.build      |  2 +-
 src/ipa/rpi/controller/rpi/alsc.cpp |  3 +++
 src/libcamera/control_ids_core.yaml | 38 ++++++++++++++++++++++++++++
 5 files changed, 90 insertions(+), 4 deletions(-)


---
base-commit: c08caf6672fca8ea613ccb3a36e5f8e926b84903
change-id: 20260910-articimaging-lsc-2e3f25916831

Best regards,

Hi Michael

Thanks for the patch. (And also to Kieran for forwarding it!)

On Thu, 10 Sept 2026 at 15:50, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> From: Michael Kunz <mkunz@articimaging.eu>
>
> Having the LensShadingCorrection maps and ToneCurves available in frame
> metadata allows creating a DNG file with all necessary information so that the
> DNG matches the JPEG image without colour casts.
>
> I opened a PR in raspberrypi/rpicam-apps
> (https://github.com/raspberrypi/rpicam-apps/pull/928) improving the colour
> accuracy of said DNG files created by these apps. The information needed is
> currently not provided by libcamera in the frame metadata.
>
> This patch intends to add LensShadingCorrection maps and ToneCurve to the
> controls metadata in libcamera, so that DNG files can be written with correct
> colours.
>
> Signed-off-by: Michael Kunz <mkunz@articimaging.eu>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> This patch is forwarded on behalf of Michael Kunz. It appears there is
> some issue on the mail server which is rejecting his posting. A double
> signed DKIM is being rejected by the OpenDKIM instance on the libcamera
> mail server.
>
> While we try to investigate this further, which might involve an upgrade
> to the mail server instance, I've applied the patch locally, compiled it
> and sending via git-send-email from me instead.
>
> Applying this patch had a conflict which required me to manually apply
> the hunks to src/ipa/rpi/common/ipa_base.cpp but I think everything here
> is enough to still continue to review and discussion. But I'll leave
> that to the patch!
> ---
>  src/ipa/rpi/common/ipa_base.cpp     | 50 ++++++++++++++++++++++++++++++++++---
>  src/ipa/rpi/common/ipa_base.h       |  1 +
>  src/ipa/rpi/common/meson.build      |  2 +-
>  src/ipa/rpi/controller/rpi/alsc.cpp |  3 +++
>  src/libcamera/control_ids_core.yaml | 38 ++++++++++++++++++++++++++++
>  5 files changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index bc8e65cb0811..988c1f5b2ad2 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -18,15 +18,18 @@
>  #include "controller/af_algorithm.h"
>  #include "controller/af_status.h"
>  #include "controller/agc_algorithm.h"
> +#include "controller/alsc_status.h"
>  #include "controller/awb_algorithm.h"
>  #include "controller/awb_status.h"
>  #include "controller/black_level_status.h"
>  #include "controller/ccm_algorithm.h"
>  #include "controller/ccm_status.h"
>  #include "controller/contrast_algorithm.h"
> +#include "controller/contrast_status.h"
>  #include "controller/denoise_algorithm.h"
>  #include "controller/hdr_algorithm.h"
>  #include "controller/lux_status.h"
> +#include "controller/noise_status.h"
>  #include "controller/sharpen_algorithm.h"
>  #include "controller/statistics.h"
>
> @@ -80,6 +83,7 @@ const ControlInfoMap::Map ipaControls{
>                       static_cast<int64_t>(defaultMaxFrameDuration.get<std::micro>()),
>                       std::span<const int64_t, 2>{ { static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),
>                                                      static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()) } }) },
> +       { &controls::EnableLensShadingCorrectionMapOutput, ControlInfo(false, true, false) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>         { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },
>  };
> @@ -121,7 +125,7 @@ LOG_DEFINE_CATEGORY(IPARPI)
>  namespace ipa::RPi {
>
>  IpaBase::IpaBase()
> -       : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),
> +       : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), lscMapsOutput_(false), statsMetadataOutput_(false),
>           stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),
>           firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)
>  {
> @@ -237,7 +241,6 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa
>                 agcStatus.exposureTime = defaultExposureTime;
>                 agcStatus.analogueGain = defaultAnalogueGain;
>                 applyAGC(&agcStatus, ctrls);
> -
>         }
>
>         result->sensorControls = std::move(ctrls);
> @@ -772,8 +775,8 @@ static const std::map<int32_t, std::string> HdrModeTable = {
>
>  void IpaBase::applyControls(const ControlList &controls)
>  {
> -       using RPiController::AgcAlgorithm;
>         using RPiController::AfAlgorithm;
> +       using RPiController::AgcAlgorithm;
>         using RPiController::ContrastAlgorithm;
>         using RPiController::DenoiseAlgorithm;
>         using RPiController::HdrAlgorithm;
> @@ -1454,6 +1457,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                         break;
>                 }
>
> +               case controls::ENABLE_LENS_SHADING_CORRECTION_MAP_OUTPUT:
> +                       lscMapsOutput_ = ctrl.second.get<bool>();
> +                       break;
> +
>                 case controls::rpi::STATS_OUTPUT_ENABLE:
>                         statsMetadataOutput_ = ctrl.second.get<bool>();
>                         break;
> @@ -1647,6 +1654,43 @@ void IpaBase::reportMetadata(unsigned int ipaContext)
>                         libcameraMetadata_.set(controls::HdrChannel, controls::HdrChannelNone);
>         }
>
> +       NoiseStatus *noiseStatus = rpiMetadata.getLocked<NoiseStatus>("noise.status");
> +       if (noiseStatus) {
> +               float noiseProfile[] = { static_cast<float>(noiseStatus->noiseSlope),
> +                                        static_cast<float>(noiseStatus->noiseConstant) };
> +
> +               libcameraMetadata_.set(controls::NoiseProfile, noiseProfile);
> +       }
> +
> +       ContrastStatus *contrastStatus = rpiMetadata.getLocked<ContrastStatus>("contrast.status");
> +       if (contrastStatus && contrastStatus->gammaCurve.size() > 0) {
> +               std::vector<float> contrast;
> +               contrast.reserve(contrastStatus->gammaCurve.size() * 2);
> +
> +               contrastStatus->gammaCurve.map([&](double x, double y) {
> +                       contrast.emplace_back(static_cast<float>(x));
> +                       contrast.emplace_back(static_cast<float>(y));
> +               });
> +               libcameraMetadata_.set(controls::ToneCurve, contrast);
> +       }
> +
> +       if (lscMapsOutput_) {
> +               AlscStatus *alscStatus = rpiMetadata.getLocked<AlscStatus>("alsc.status");
> +               if (alscStatus) {
> +                       uint32_t elements = alscStatus->cols * alscStatus->rows;
> +                       std::vector<float> map(3 * elements);
> +
> +                       std::copy(alscStatus->r.begin(), alscStatus->r.end(), map.begin() + 0 * elements);
> +                       std::copy(alscStatus->g.begin(), alscStatus->g.end(), map.begin() + 1 * elements);
> +                       std::copy(alscStatus->b.begin(), alscStatus->b.end(), map.begin() + 2 * elements);
> +
> +                       uint32_t sizeTable[] = { 3, alscStatus->cols, alscStatus->rows };
> +                       libcameraMetadata_.set(controls::LensShadingCorrectionMaps, map);
> +                       libcameraMetadata_.set(controls::LensShadingCorrectionMapSize, sizeTable);
> +                       libcameraMetadata_.set(controls::EnableLensShadingCorrectionMapOutput, true);
> +               }
> +       }
> +
>         metadataReady.emit(libcameraMetadata_);
>  }
>
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
> index 0d7842f1f742..dbd6a8e5056f 100644
> --- a/src/ipa/rpi/common/ipa_base.h
> +++ b/src/ipa/rpi/common/ipa_base.h
> @@ -68,6 +68,7 @@ protected:
>         std::deque<utils::Duration> frameLengths_;
>         utils::Duration lastTimeout_;
>         ControlList libcameraMetadata_;
> +       bool lscMapsOutput_;
>         bool statsMetadataOutput_;
>
>         /* Remember the HDR status after a mode switch. */
> diff --git a/src/ipa/rpi/common/meson.build b/src/ipa/rpi/common/meson.build
> index 73d2ee732339..cb3555a802c0 100644
> --- a/src/ipa/rpi/common/meson.build
> +++ b/src/ipa/rpi/common/meson.build
> @@ -5,7 +5,7 @@ rpi_ipa_common_sources = files([
>  ])
>
>  rpi_ipa_common_includes = [
> -    include_directories('..'),
> +    include_directories('..','../..'),
>  ]
>
>  rpi_ipa_common_deps = [
> diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp
> index e13d6c6c2832..c0d39d1fe41b 100644
> --- a/src/ipa/rpi/controller/rpi/alsc.cpp
> +++ b/src/ipa/rpi/controller/rpi/alsc.cpp
> @@ -409,6 +409,9 @@ void Alsc::prepare(Metadata *imageMetadata)
>         status.r = prevSyncResults_[0].data();
>         status.g = prevSyncResults_[1].data();
>         status.b = prevSyncResults_[2].data();
> +       status.cols = config_.tableSize.width;
> +       status.rows = config_.tableSize.height;
> +
>         imageMetadata->set("alsc.status", status);
>         /*
>          * Put the results in the global metadata as well. This will be used by
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index 89991d03d79a..608681f3e395 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -1376,4 +1376,42 @@ controls:
>          The nominal range is [-180, 180], where 0° leaves hues unchanged and the
>          range wraps around continuously, with 180° == -180°.
>
> +  - EnableLensShadingCorrectionMapOutput:

Apologies for the very minor nit-pick, but I would slightly prefer
LensShadingCorrectionMapOutputEnable, so that the related controls
come out together if you sort them alphabetically. I think it's a bit
more in keeping with other enables, like AwbEnable etc.

> +      type: bool
> +      direction: inout
> +      description: |
> +        Indicates if the lens shading correction maps should be set in frame-
> +        metadata for each frame. Returns true if tables are successfully set.
> +
> +  - LensShadingCorrectionMaps:
> +      type: float
> +      direction: out
> +      description: |
> +        A map giving the lens shading correction factors. Dimesnions of the

s/Dimesnions/Dimensions/

> +        returned table is [number of channels, sizeX, sizeY] (planar tables)

I'm guessing someone might ask for a full stop at the end... (also the
one below). Sorry!

> +      size: [n]
> +
> +  - LensShadingCorrectionMapSize:
> +      type: uint32_t
> +      direction: out
> +      description: |
> +        The number of channels/maps and the size of the lens shading correction
> +        maps in pixels. E.g. [3, 32, 32]
> +      size: [3]
> +
> +  - ToneCurve:
> +      type: float
> +      direction: out
> +      description: |
> +        A profile tone curve to apply on linear RGB - as it seems it has the sRGB
> +        gamma curve baked in.

Again very minor, but I'd prefer a slightly more authoritative
wording, maybe like

A profile tone curve to apply to linear RGB to produce the requested
output colour space.

or

A profile tone curve to apply to linear RGB that should also
incorporate the output colour space's transfer function.

Or something. Please take your pick!!

I suppose the only other thing that springs to mind is what a platform
might report if it doesn't support tables, but maybe polynomials or
something. We could add LensShadingCorrectionPolynomial at a later
date? (But no reason to do it now, I feel.)

Anyway, it seems fine to me with those one or two little nit-picks, so:

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!

David

> +      size: [n]
> +
> +  - NoiseProfile:
> +      type: float
> +      direction: out
> +      description: |
> +        The noise profile [Scale, Offset].
> +      size: [2]
> +
>  ...
>
> ---
> base-commit: c08caf6672fca8ea613ccb3a36e5f8e926b84903
> change-id: 20260910-articimaging-lsc-2e3f25916831
>
> Best regards,
> --
> --
> Kieran
>