[PATCH v5 6/6] ipa: simple: Report exposure in metadata

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Mar 27 00:51:35 CET 2025


On Wed, Mar 26, 2025 at 10:56:51PM +0000, Kieran Bingham wrote:
> Quoting Milan Zamazal (2025-03-26 12:48:55)
> > Report exposure and gain in metadata.
> > 
> > This is more complicated than it could be expected because the exposure
> > value should be in microseconds but it's handled using V4L2_CID_EXPOSURE
> > control, which doesn't specify the unit, see
> > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/control.html.
> > So the unit conversion is done in the way rkisp1 IPA uses.
> > 
> > This requires getting and passing IPACameraSensorInfo around.  To avoid
> > naming confusion and to improve consistency with rkisp1 IPA,
> > sensorCtrlInfoMap parameter is renamed to sensorControls.
> 
> Getting the lineDuration helps for handling things like
> FrameDuration/Framerate control later too - so this is already helpful.
> 
> And it's working - I can see the results in camshark which is helpful
> already.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> > ---
> >  include/libcamera/ipa/soft.mojom            |  3 ++-
> >  src/ipa/simple/algorithms/agc.cpp           | 11 +++++++++--
> >  src/ipa/simple/ipa_context.h                |  6 +++++-
> >  src/ipa/simple/soft_simple.cpp              | 17 +++++++++++++----
> >  src/libcamera/software_isp/software_isp.cpp | 20 ++++++++++++++------
> >  5 files changed, 43 insertions(+), 14 deletions(-)
> > 
> > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> > index a8e6ecda..77328c5f 100644
> > --- a/include/libcamera/ipa/soft.mojom
> > +++ b/include/libcamera/ipa/soft.mojom
> > @@ -16,7 +16,8 @@ interface IPASoftInterface {
> >         init(libcamera.IPASettings settings,
> >              libcamera.SharedFD fdStats,
> >              libcamera.SharedFD fdParams,
> > -            libcamera.ControlInfoMap sensorCtrlInfoMap)
> > +            libcamera.IPACameraSensorInfo sensorInfo,
> > +            libcamera.ControlInfoMap sensorControls)
> >                 => (int32 ret, libcamera.ControlInfoMap ipaControls, bool ccmEnabled);
> >         start() => (int32 ret);
> >         stop();
> > diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp
> > index 72aade14..c46bb0eb 100644
> > --- a/src/ipa/simple/algorithms/agc.cpp
> > +++ b/src/ipa/simple/algorithms/agc.cpp
> > @@ -11,6 +11,8 @@
> >  
> >  #include <libcamera/base/log.h>
> >  
> > +#include "control_ids.h"
> > +
> >  namespace libcamera {
> >  
> >  LOG_DEFINE_CATEGORY(IPASoftExposure)
> > @@ -97,10 +99,15 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou
> >  
> >  void Agc::process(IPAContext &context,
> >                   [[maybe_unused]] const uint32_t frame,
> > -                 [[maybe_unused]] IPAFrameContext &frameContext,
> > +                 IPAFrameContext &frameContext,
> >                   const SwIspStats *stats,
> > -                 [[maybe_unused]] ControlList &metadata)
> > +                 ControlList &metadata)
> >  {
> > +       utils::Duration exposureTime =
> > +               context.configuration.agc.lineDuration * frameContext.sensor.exposure;
> > +       metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> > +       metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > +
> >         /*
> >          * Calculate Mean Sample Value (MSV) according to formula from:
> >          * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> > index 10d539f5..7dc2cd7a 100644
> > --- a/src/ipa/simple/ipa_context.h
> > +++ b/src/ipa/simple/ipa_context.h
> > @@ -1,6 +1,6 @@
> >  /* SPDX-License-Identifier: LGPL-2.1-or-later */
> >  /*
> > - * Copyright (C) 2024 Red Hat, Inc.
> > + * Copyright (C) 2024-2025 Red Hat, Inc.
> >   *
> >   * Simple pipeline IPA Context
> >   */
> > @@ -18,6 +18,8 @@
> >  
> >  #include <libipa/fc_queue.h>
> >  
> > +#include "core_ipa_interface.h"
> > +
> >  namespace libcamera {
> >  
> >  namespace ipa::soft {
> > @@ -27,6 +29,7 @@ struct IPASessionConfiguration {
> >         struct {
> >                 int32_t exposureMin, exposureMax;
> >                 double againMin, againMax, againMinStep;
> > +               utils::Duration lineDuration;
> >         } agc;
> >         struct {
> >                 std::optional<uint8_t> level;
> > @@ -83,6 +86,7 @@ struct IPAContext {
> >         {
> >         }
> >  
> > +       IPACameraSensorInfo sensorInfo;
> >         IPASessionConfiguration configuration;
> >         IPAActiveState activeState;
> >         FCQueue<IPAFrameContext> frameContexts;
> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> > index 4ef67c43..c94c4cd5 100644
> > --- a/src/ipa/simple/soft_simple.cpp
> > +++ b/src/ipa/simple/soft_simple.cpp
> > @@ -5,6 +5,7 @@
> >   * Simple Software Image Processing Algorithm module
> >   */
> >  
> > +#include <chrono>
> >  #include <stdint.h>
> >  #include <sys/mman.h>
> >  
> > @@ -32,6 +33,8 @@
> >  namespace libcamera {
> >  LOG_DEFINE_CATEGORY(IPASoft)
> >  
> > +using namespace std::literals::chrono_literals;
> > +
> >  namespace ipa::soft {
> >  
> >  /* Maximum number of frame contexts to be held */
> > @@ -50,7 +53,8 @@ public:
> >         int init(const IPASettings &settings,
> >                  const SharedFD &fdStats,
> >                  const SharedFD &fdParams,
> > -                const ControlInfoMap &sensorInfoMap,
> > +                const IPACameraSensorInfo &sensorInfo,
> > +                const ControlInfoMap &sensorControls,
> >                  ControlInfoMap *ipaControls,
> >                  bool *ccmEnabled) override;
> >         int configure(const IPAConfigInfo &configInfo) override;
> > @@ -89,7 +93,8 @@ IPASoftSimple::~IPASoftSimple()
> >  int IPASoftSimple::init(const IPASettings &settings,
> >                         const SharedFD &fdStats,
> >                         const SharedFD &fdParams,
> > -                       const ControlInfoMap &sensorInfoMap,
> > +                       const IPACameraSensorInfo &sensorInfo,
> > +                       const ControlInfoMap &sensorControls,
> >                         ControlInfoMap *ipaControls,
> >                         bool *ccmEnabled)
> >  {
> > @@ -100,6 +105,8 @@ int IPASoftSimple::init(const IPASettings &settings,
> >                         << settings.sensorModel;
> >         }
> >  
> > +       context_.sensorInfo = sensorInfo;
> > +
> >         /* Load the tuning data file */
> >         File file(settings.configurationFile);
> >         if (!file.open(File::OpenModeFlag::ReadOnly)) {
> > @@ -173,12 +180,12 @@ int IPASoftSimple::init(const IPASettings &settings,
> >          * Don't save the min and max control values yet, as e.g. the limits
> >          * for V4L2_CID_EXPOSURE depend on the configured sensor resolution.
> >          */
> > -       if (sensorInfoMap.find(V4L2_CID_EXPOSURE) == sensorInfoMap.end()) {
> > +       if (sensorControls.find(V4L2_CID_EXPOSURE) == sensorControls.end()) {
> >                 LOG(IPASoft, Error) << "Don't have exposure control";
> >                 return -EINVAL;
> >         }
> >  
> > -       if (sensorInfoMap.find(V4L2_CID_ANALOGUE_GAIN) == sensorInfoMap.end()) {
> > +       if (sensorControls.find(V4L2_CID_ANALOGUE_GAIN) == sensorControls.end()) {
> >                 LOG(IPASoft, Error) << "Don't have gain control";
> >                 return -EINVAL;
> >         }
> > @@ -198,6 +205,8 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
> >         context_.activeState = {};
> >         context_.frameContexts.clear();
> >  
> > +       context_.configuration.agc.lineDuration =
> > +               context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;
> >         context_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>();
> >         context_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>();
> >         if (!context_.configuration.agc.exposureMin) {
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index 6baa3fec..28e2a360 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -133,12 +133,20 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> >         std::string ipaTuningFile =
> >                 ipa_->configurationFile(sensor->model() + ".yaml", "uncalibrated.yaml");
> >  
> > -       int ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
> > -                            debayer_->getStatsFD(),
> > -                            sharedParams_.fd(),
> > -                            sensor->controls(),
> > -                            ipaControls,
> > -                            &ccmEnabled_);
> > +       IPACameraSensorInfo sensorInfo{};
> > +       int ret = sensor->sensorInfo(&sensorInfo);
> > +       if (ret) {
> > +               LOG(SoftwareIsp, Error) << "Camera sensor information not available";
> > +               return;
> > +       }
> > +
> > +       ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
> > +                        debayer_->getStatsFD(),
> > +                        sharedParams_.fd(),
> > +                        sensorInfo,
> > +                        sensor->controls(),
> > +                        ipaControls,
> > +                        &ccmEnabled_);
> >         if (ret) {
> >                 LOG(SoftwareIsp, Error) << "IPA init failed";
> >                 debayer_.reset();

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list