| Message ID | 20240904074500.106019-3-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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 >
Hi Kieran, thank you for review. Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > 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@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. Well, I followed what the other pipelines do. > Can the control be registered by the algorithm that supports it? I don't think Algorithm class API allows that. But I can look again at the other pipelines for possibly more inspiration. > 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... I don't have strong opinion about this. On one hand it's better to not put many things on a single pile, on the other hand the separation may not make much sense here. Another option is to modify the gamma table, or at least provide the S-curve, in the contrast algorithm. >> + >> /* 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 >>
Milan Zamazal <mzamazal@redhat.com> writes: > Hi Kieran, > > thank you for review. > > Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > >> 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@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. > > Well, I followed what the other pipelines do. > >> Can the control be registered by the algorithm that supports it? > > I don't think Algorithm class API allows that. But I can look again at > the other pipelines for possibly more inspiration. I can see there has been a similar change posted for rkisp1, I'll try to follow 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... > > I don't have strong opinion about this. On one hand it's better to not > put many things on a single pile, on the other hand the separation may > not make much sense here. Another option is to modify the gamma table, > or at least provide the S-curve, in the contrast algorithm. > >>> + >>> /* 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 >>>
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) }, +}; + /* 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();
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@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(-)