| Message ID | 20240311141524.27192-19-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hans de Goede <hdegoede@redhat.com> writes: > From: Andrei Konovalov <andrey.konovalov.ynk@gmail.com> > > Use CameraSensorHelper to convert the analogue gain code read from the > camera sensor into real analogue gain value. In the future this makes > it possible to use faster AE/AGC algorithm. For now the same AE/AGC > algorithm is used, but even then the CameraSensorHelper lets us use the > full range of analogue gain values. > > If there is no CameraSensorHelper for the camera sensor in use, a > warning log message is printed, and the AE/AGC works exactly as before > this change. > > Signed-off-by: Andrei Konovalov <andrey.konovalov.ynk@gmail.com> > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > .../internal/software_isp/software_isp.h | 3 +- > src/ipa/simple/soft_simple.cpp | 77 ++++++++++++------- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > src/libcamera/software_isp/software_isp.cpp | 8 +- > 4 files changed, 57 insertions(+), 33 deletions(-) > > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h > index 8d25e979..2a6db7ba 100644 > --- a/include/libcamera/internal/software_isp/software_isp.h > +++ b/include/libcamera/internal/software_isp/software_isp.h > @@ -26,6 +26,7 @@ > #include <libcamera/ipa/soft_ipa_interface.h> > #include <libcamera/ipa/soft_ipa_proxy.h> > > +#include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/dma_heaps.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/shared_mem_object.h" > @@ -43,7 +44,7 @@ LOG_DECLARE_CATEGORY(SoftwareIsp) > class SoftwareIsp > { > public: > - SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls); > + SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor); > ~SoftwareIsp(); > > int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index ac027568..e4d64762 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -22,6 +22,8 @@ > #include "libcamera/internal/software_isp/debayer_params.h" > #include "libcamera/internal/software_isp/swisp_stats.h" > > +#include "libipa/camera_sensor_helper.h" > + > #include "black_level.h" > > namespace libcamera { > @@ -67,18 +69,27 @@ private: > DebayerParams *params_; > SwIspStats *stats_; > BlackLevel blackLevel_; > + std::unique_ptr<CameraSensorHelper> camHelper_; > > int32_t exposure_min_, exposure_max_; > - int32_t again_min_, again_max_; > - int32_t again_, exposure_; > + int32_t exposure_; > + double again_min_, again_max_, againMinStep_; > + double again_; > unsigned int ignore_updates_; > }; > > -int IPASoftSimple::init([[maybe_unused]] const IPASettings &settings, > +int IPASoftSimple::init(const IPASettings &settings, > const SharedFD &fdStats, > const SharedFD &fdParams, > const ControlInfoMap &sensorInfoMap) > { > + camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel); > + if (camHelper_ == nullptr) { > + LOG(IPASoft, Warning) > + << "Failed to create camera sensor helper for " > + << settings.sensorModel; > + } > + > fdStats_ = fdStats; > if (!fdStats_.isValid()) { > LOG(IPASoft, Error) << "Invalid Statistics handle"; > @@ -132,25 +143,35 @@ int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap) > exposure_min_ = 1; > } > > - again_min_ = gain_info.min().get<int32_t>(); > - again_max_ = gain_info.max().get<int32_t>(); > - /* > - * The camera sensor gain (g) is usually not equal to the value written > - * into the gain register (x). But the way how the AGC algorithm changes > - * the gain value to make the total exposure closer to the optimum assumes > - * that g(x) is not too far from linear function. If the minimal gain is 0, > - * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). > - * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near > - * one edge, and very small near the other) we limit the range of the gain > - * values used. > - */ > - if (!again_min_) { > - LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; > - again_min_ = std::min(100, again_min_ / 2 + again_max_ / 2); > + int32_t again_min = gain_info.min().get<int32_t>(); > + int32_t again_max = gain_info.max().get<int32_t>(); > + > + if (camHelper_) { > + again_min_ = camHelper_->gain(again_min); > + again_max_ = camHelper_->gain(again_max); > + againMinStep_ = (again_max_ - again_min_) / 100.0; > + } else { > + /* > + * The camera sensor gain (g) is usually not equal to the value written > + * into the gain register (x). But the way how the AGC algorithm changes > + * the gain value to make the total exposure closer to the optimum assumes > + * that g(x) is not too far from linear function. If the minimal gain is 0, > + * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). > + * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near > + * one edge, and very small near the other) we limit the range of the gain > + * values used. > + */ > + again_max_ = again_max; > + if (!again_min) { > + LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; > + again_min_ = std::min(100, again_min / 2 + again_max / 2); > + } > + againMinStep_ = 1.0; > } > > LOG(IPASoft, Info) << "Exposure " << exposure_min_ << "-" << exposure_max_ > - << ", gain " << again_min_ << "-" << again_max_; > + << ", gain " << again_min_ << "-" << again_max_ > + << " (" << againMinStep_ << ")"; > > return 0; > } > @@ -252,12 +273,14 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) > ControlList ctrls(sensorControls); > > exposure_ = ctrls.get(V4L2_CID_EXPOSURE).get<int32_t>(); > - again_ = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>(); > + int32_t again = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>(); > + again_ = camHelper_ ? camHelper_->gain(again) : again; > > updateExposure(exposureMSV); > > ctrls.set(V4L2_CID_EXPOSURE, exposure_); > - ctrls.set(V4L2_CID_ANALOGUE_GAIN, again_); > + ctrls.set(V4L2_CID_ANALOGUE_GAIN, > + static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_)); > > ignore_updates_ = 2; > > @@ -276,7 +299,7 @@ void IPASoftSimple::updateExposure(double exposureMSV) > static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; > static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; > > - int next; > + double next; > > if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { > next = exposure_ * kExpNumeratorUp / kExpDenominator; > @@ -286,18 +309,18 @@ void IPASoftSimple::updateExposure(double exposureMSV) > exposure_ = next; > if (exposure_ >= exposure_max_) { > next = again_ * kExpNumeratorUp / kExpDenominator; > - if (next - again_ < 1) > - again_ += 1; > + if (next - again_ < againMinStep_) > + again_ += againMinStep_; > else > again_ = next; > } > } > > if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { > - if (exposure_ == exposure_max_ && again_ != again_min_) { > + if (exposure_ == exposure_max_ && again_ > again_min_) { > next = again_ * kExpNumeratorDown / kExpDenominator; > - if (again_ - next < 1) > - again_ -= 1; > + if (again_ - next < againMinStep_) > + again_ -= againMinStep_; > else > again_ = next; > } else { > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index e491ab62..32b56c57 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -526,7 +526,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_->controls()); > + swIsp_ = std::make_unique<SoftwareIsp>(pipe, sensor_.get()); > 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 9b49be41..ea4d96e4 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -60,9 +60,9 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) > /** > * \brief Constructs SoftwareIsp object > * \param[in] pipe The pipeline handler in use > - * \param[in] sensorControls ControlInfoMap describing the controls supported by the sensor > + * \param[in] sensor Pointer to the CameraSensor instance owned by the pipeline handler > */ > -SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls) > +SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) > : debayer_(nullptr), > debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, DebayerParams::kGain10, 0.5f, 0 }, > dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) > @@ -97,10 +97,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorCont > return; > } > > - int ret = ipa_->init(IPASettings{ "No cfg file", "No sensor model" }, > + int ret = ipa_->init(IPASettings{ "No cfg file", sensor->model() }, > debayer_->getStatsFD(), > sharedParams_.fd(), > - sensorControls); > + sensor->controls()); > if (ret) { > LOG(SoftwareIsp, Error) << "IPA init failed"; > debayer_.reset();
diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h index 8d25e979..2a6db7ba 100644 --- a/include/libcamera/internal/software_isp/software_isp.h +++ b/include/libcamera/internal/software_isp/software_isp.h @@ -26,6 +26,7 @@ #include <libcamera/ipa/soft_ipa_interface.h> #include <libcamera/ipa/soft_ipa_proxy.h> +#include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/dma_heaps.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/shared_mem_object.h" @@ -43,7 +44,7 @@ LOG_DECLARE_CATEGORY(SoftwareIsp) class SoftwareIsp { public: - SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls); + SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor); ~SoftwareIsp(); int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index ac027568..e4d64762 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -22,6 +22,8 @@ #include "libcamera/internal/software_isp/debayer_params.h" #include "libcamera/internal/software_isp/swisp_stats.h" +#include "libipa/camera_sensor_helper.h" + #include "black_level.h" namespace libcamera { @@ -67,18 +69,27 @@ private: DebayerParams *params_; SwIspStats *stats_; BlackLevel blackLevel_; + std::unique_ptr<CameraSensorHelper> camHelper_; int32_t exposure_min_, exposure_max_; - int32_t again_min_, again_max_; - int32_t again_, exposure_; + int32_t exposure_; + double again_min_, again_max_, againMinStep_; + double again_; unsigned int ignore_updates_; }; -int IPASoftSimple::init([[maybe_unused]] const IPASettings &settings, +int IPASoftSimple::init(const IPASettings &settings, const SharedFD &fdStats, const SharedFD &fdParams, const ControlInfoMap &sensorInfoMap) { + camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel); + if (camHelper_ == nullptr) { + LOG(IPASoft, Warning) + << "Failed to create camera sensor helper for " + << settings.sensorModel; + } + fdStats_ = fdStats; if (!fdStats_.isValid()) { LOG(IPASoft, Error) << "Invalid Statistics handle"; @@ -132,25 +143,35 @@ int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap) exposure_min_ = 1; } - again_min_ = gain_info.min().get<int32_t>(); - again_max_ = gain_info.max().get<int32_t>(); - /* - * The camera sensor gain (g) is usually not equal to the value written - * into the gain register (x). But the way how the AGC algorithm changes - * the gain value to make the total exposure closer to the optimum assumes - * that g(x) is not too far from linear function. If the minimal gain is 0, - * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). - * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near - * one edge, and very small near the other) we limit the range of the gain - * values used. - */ - if (!again_min_) { - LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; - again_min_ = std::min(100, again_min_ / 2 + again_max_ / 2); + int32_t again_min = gain_info.min().get<int32_t>(); + int32_t again_max = gain_info.max().get<int32_t>(); + + if (camHelper_) { + again_min_ = camHelper_->gain(again_min); + again_max_ = camHelper_->gain(again_max); + againMinStep_ = (again_max_ - again_min_) / 100.0; + } else { + /* + * The camera sensor gain (g) is usually not equal to the value written + * into the gain register (x). But the way how the AGC algorithm changes + * the gain value to make the total exposure closer to the optimum assumes + * that g(x) is not too far from linear function. If the minimal gain is 0, + * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). + * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near + * one edge, and very small near the other) we limit the range of the gain + * values used. + */ + again_max_ = again_max; + if (!again_min) { + LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; + again_min_ = std::min(100, again_min / 2 + again_max / 2); + } + againMinStep_ = 1.0; } LOG(IPASoft, Info) << "Exposure " << exposure_min_ << "-" << exposure_max_ - << ", gain " << again_min_ << "-" << again_max_; + << ", gain " << again_min_ << "-" << again_max_ + << " (" << againMinStep_ << ")"; return 0; } @@ -252,12 +273,14 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) ControlList ctrls(sensorControls); exposure_ = ctrls.get(V4L2_CID_EXPOSURE).get<int32_t>(); - again_ = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>(); + int32_t again = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>(); + again_ = camHelper_ ? camHelper_->gain(again) : again; updateExposure(exposureMSV); ctrls.set(V4L2_CID_EXPOSURE, exposure_); - ctrls.set(V4L2_CID_ANALOGUE_GAIN, again_); + ctrls.set(V4L2_CID_ANALOGUE_GAIN, + static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_)); ignore_updates_ = 2; @@ -276,7 +299,7 @@ void IPASoftSimple::updateExposure(double exposureMSV) static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; - int next; + double next; if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { next = exposure_ * kExpNumeratorUp / kExpDenominator; @@ -286,18 +309,18 @@ void IPASoftSimple::updateExposure(double exposureMSV) exposure_ = next; if (exposure_ >= exposure_max_) { next = again_ * kExpNumeratorUp / kExpDenominator; - if (next - again_ < 1) - again_ += 1; + if (next - again_ < againMinStep_) + again_ += againMinStep_; else again_ = next; } } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { - if (exposure_ == exposure_max_ && again_ != again_min_) { + if (exposure_ == exposure_max_ && again_ > again_min_) { next = again_ * kExpNumeratorDown / kExpDenominator; - if (again_ - next < 1) - again_ -= 1; + if (again_ - next < againMinStep_) + again_ -= againMinStep_; else again_ = next; } else { diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index e491ab62..32b56c57 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -526,7 +526,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_->controls()); + swIsp_ = std::make_unique<SoftwareIsp>(pipe, sensor_.get()); 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 9b49be41..ea4d96e4 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -60,9 +60,9 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) /** * \brief Constructs SoftwareIsp object * \param[in] pipe The pipeline handler in use - * \param[in] sensorControls ControlInfoMap describing the controls supported by the sensor + * \param[in] sensor Pointer to the CameraSensor instance owned by the pipeline handler */ -SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls) +SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) : debayer_(nullptr), debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, DebayerParams::kGain10, 0.5f, 0 }, dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) @@ -97,10 +97,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorCont return; } - int ret = ipa_->init(IPASettings{ "No cfg file", "No sensor model" }, + int ret = ipa_->init(IPASettings{ "No cfg file", sensor->model() }, debayer_->getStatsFD(), sharedParams_.fd(), - sensorControls); + sensor->controls()); if (ret) { LOG(SoftwareIsp, Error) << "IPA init failed"; debayer_.reset();