[PATCH 2/2] libcamera: software_isp: Add contrast control
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Sep 10 21:50:10 CEST 2024
Quoting Milan Zamazal (2024-09-04 08:44:59)
> This patch introduces support for applying runtime controls to software
> ISP. It enables the contrast algorithm as the first control that can be
> used.
>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
> .../libcamera/internal/software_isp/software_isp.h | 3 ++-
> include/libcamera/ipa/soft.mojom | 2 +-
> src/ipa/simple/data/uncalibrated.yaml | 1 +
> src/ipa/simple/soft_simple.cpp | 13 +++++++++++--
> src/libcamera/pipeline/simple/simple.cpp | 2 +-
> src/libcamera/software_isp/software_isp.cpp | 8 ++++++--
> 6 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index a3e3a9da..d51b03fd 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -46,7 +46,8 @@ LOG_DECLARE_CATEGORY(SoftwareIsp)
> class SoftwareIsp
> {
> public:
> - SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor);
> + SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> + ControlInfoMap *ipaControls);
> ~SoftwareIsp();
>
> int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; }
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index bc70d4e4..3b8a72a2 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -17,7 +17,7 @@ interface IPASoftInterface {
> libcamera.SharedFD fdStats,
> libcamera.SharedFD fdParams,
> libcamera.ControlInfoMap sensorCtrlInfoMap)
> - => (int32 ret);
> + => (int32 ret, libcamera.ControlInfoMap ipaControls);
> start() => (int32 ret);
> stop();
> configure(IPAConfigInfo configInfo)
> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml
> index 324f98f4..2ed24e8c 100644
> --- a/src/ipa/simple/data/uncalibrated.yaml
> +++ b/src/ipa/simple/data/uncalibrated.yaml
> @@ -4,6 +4,7 @@
> version: 1
> algorithms:
> - BlackLevel:
> + - Contrast:
> - Gamma:
> - Awb:
> - Lut:
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index ac7a22b7..d6c7ff7f 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -34,6 +34,10 @@ LOG_DEFINE_CATEGORY(IPASoft)
>
> namespace ipa::soft {
>
> +const ControlInfoMap::Map swispControls{
> + { &controls::Contrast, ControlInfo(0.0f, 10.0f, 1.0f) },
> +};
I would prefer not to register all controls at the top level. Can the
control be registered by the algorithm that supports it?
I'm not sure if 'contrast' is a full algorithm if it's all being handled
through gamma ... but it's only a detail for now, I guess it's a way to
enable or disable features if it's separated...
> +
> /* Maximum number of frame contexts to be held */
> static constexpr uint32_t kMaxFrameContexts = 16;
>
> @@ -50,7 +54,8 @@ public:
> int init(const IPASettings &settings,
> const SharedFD &fdStats,
> const SharedFD &fdParams,
> - const ControlInfoMap &sensorInfoMap) override;
> + const ControlInfoMap &sensorInfoMap,
> + ControlInfoMap *ipaControls) override;
> int configure(const IPAConfigInfo &configInfo) override;
>
> int start() override;
> @@ -87,7 +92,8 @@ IPASoftSimple::~IPASoftSimple()
> int IPASoftSimple::init(const IPASettings &settings,
> const SharedFD &fdStats,
> const SharedFD &fdParams,
> - const ControlInfoMap &sensorInfoMap)
> + const ControlInfoMap &sensorInfoMap,
> + ControlInfoMap *ipaControls)
> {
> camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> if (!camHelper_) {
> @@ -158,6 +164,9 @@ int IPASoftSimple::init(const IPASettings &settings,
> stats_ = static_cast<SwIspStats *>(mem);
> }
>
> + ControlInfoMap::Map ctrlMap = swispControls;
> + *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +
> /*
> * Check if the sensor driver supports the controls required by the
> * Soft IPA.
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index fc80e665..86c4447e 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -531,7 +531,7 @@ int SimpleCameraData::init()
> * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.
> */
> if (!converter_ && pipe->swIspEnabled()) {
> - swIsp_ = std::make_unique<SoftwareIsp>(pipe, sensor_.get());
> + swIsp_ = std::make_unique<SoftwareIsp>(pipe, sensor_.get(), &controlInfo_);
> if (!swIsp_->isValid()) {
> LOG(SimplePipeline, Warning)
> << "Failed to create software ISP, disabling software debayering";
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index dbf27f31..78b78bab 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -13,6 +13,7 @@
> #include <sys/types.h>
> #include <unistd.h>
>
> +#include <libcamera/controls.h>
> #include <libcamera/formats.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
> @@ -61,9 +62,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
> * \brief Constructs SoftwareIsp object
> * \param[in] pipe The pipeline handler in use
> * \param[in] sensor Pointer to the CameraSensor instance owned by the pipeline
> + * \param[out] ipaControls The IPA controls to update
> * handler
> */
> -SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
> +SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> + ControlInfoMap *ipaControls)
> : dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> @@ -125,7 +128,8 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
> int ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
> debayer_->getStatsFD(),
> sharedParams_.fd(),
> - sensor->controls());
> + sensor->controls(),
> + ipaControls);
> if (ret) {
> LOG(SoftwareIsp, Error) << "IPA init failed";
> debayer_.reset();
> --
> 2.44.1
>
More information about the libcamera-devel
mailing list