[PATCH 13/19] libcamera: software_isp: Call Algorithm::configure
Umang Jain
umang.jain at ideasonboard.com
Sat Jun 29 07:13:53 CEST 2024
Hi Milan
Thank you for the patch
On 26/06/24 12:50 pm, Milan Zamazal wrote:
> This patch adds Algorithm::configure call for the defined algorithms.
> This is preparation only since there are currently no Algorithm based
> algorithms defined.
>
> A part of this change is passing IPAConfigInfo instead of ControlInfoMap
> to configure() calls as this is what Algorithm::configure expects.
>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
> .../libcamera/internal/software_isp/software_isp.h | 2 +-
> include/libcamera/ipa/soft.mojom | 6 +++++-
> src/ipa/simple/module.h | 3 ++-
> src/ipa/simple/soft_simple.cpp | 12 +++++++++---
> src/libcamera/pipeline/simple/simple.cpp | 11 +++++++----
> src/libcamera/software_isp/software_isp.cpp | 7 ++++---
> 6 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 8753c2fd..1ba9eb62 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -61,7 +61,7 @@ public:
>
> int configure(const StreamConfiguration &inputCfg,
> const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
> - const ControlInfoMap &sensorControls);
> + const ipa::soft::IPAConfigInfo &configInfo);
>
> int exportBuffers(unsigned int output, unsigned int count,
> std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index dad352ba..65d90bfa 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -8,6 +8,10 @@ module ipa.soft;
>
> import "include/libcamera/ipa/core.mojom";
>
> +struct IPAConfigInfo {
> + libcamera.ControlInfoMap sensorControls;
> +};
> +
> interface IPASoftInterface {
> init(libcamera.IPASettings settings,
> libcamera.SharedFD fdStats,
> @@ -16,7 +20,7 @@ interface IPASoftInterface {
> => (int32 ret);
> start() => (int32 ret);
> stop();
> - configure(libcamera.ControlInfoMap sensorCtrlInfoMap)
> + configure(IPAConfigInfo configInfo)
> => (int32 ret);
>
> prepare(uint32 frame);
> diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h
> index 21328ecd..e518e742 100644
> --- a/src/ipa/simple/module.h
> +++ b/src/ipa/simple/module.h
> @@ -10,6 +10,7 @@
> #include <libcamera/controls.h>
>
> #include <libcamera/ipa/core_ipa_interface.h>
> +#include <libcamera/ipa/soft_ipa_interface.h>
I think the core_ipa_interface header can be (or should be dropped). It
is already included as part of template for module_ipa_interface via:
utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
Other IPAs (IPU3, RkISP1) also needs to be cleaned up, I'll try to
propose a patch for them.
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> #include "libcamera/internal/software_isp/debayer_params.h"
> #include "libcamera/internal/software_isp/swisp_stats.h"
> @@ -22,7 +23,7 @@ namespace libcamera {
>
> namespace ipa::soft {
>
> -using Module = ipa::Module<IPAContext, IPAFrameContext, ControlInfoMap,
> +using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo,
> DebayerParams, SwIspStats>;
>
> } /* namespace ipa::soft */
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index 514a9db5..7200abaf 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -73,7 +73,7 @@ public:
> const SharedFD &fdStats,
> const SharedFD &fdParams,
> const ControlInfoMap &sensorInfoMap) override;
> - int configure(const ControlInfoMap &sensorInfoMap) override;
> + int configure(const IPAConfigInfo &configInfo) override;
>
> int start() override;
> void stop() override;
> @@ -209,9 +209,9 @@ int IPASoftSimple::init(const IPASettings &settings,
> return 0;
> }
>
> -int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap)
> +int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
> {
> - sensorInfoMap_ = sensorInfoMap;
> + sensorInfoMap_ = configInfo.sensorControls;
>
> const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;
> const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;
> @@ -250,6 +250,12 @@ int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap)
> againMinStep_ = 1.0;
> }
>
> + for (auto const &algo : algorithms()) {
> + int ret = algo->configure(context_, configInfo);
> + if (ret)
> + return ret;
> + }
> +
> LOG(IPASoft, Info) << "Exposure " << exposureMin_ << "-" << exposureMax_
> << ", gain " << againMin_ << "-" << againMax_
> << " (" << againMinStep_ << ")";
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0fa9db38..a82d05f4 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1288,10 +1288,13 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> inputCfg.stride = captureFormat.planes[0].bpl;
> inputCfg.bufferCount = kNumInternalBuffers;
>
> - return data->converter_
> - ? data->converter_->configure(inputCfg, outputCfgs)
> - : data->swIsp_->configure(inputCfg, outputCfgs,
> - data->sensor_->controls());
> + if (data->converter_)
> + return data->converter_->configure(inputCfg, outputCfgs);
> + else {
> + ipa::soft::IPAConfigInfo configInfo;
> + configInfo.sensorControls = data->sensor_->controls();
> + return data->swIsp_->configure(inputCfg, outputCfgs, configInfo);
> + }
> }
>
> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index ba3f1bae..1f176214 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -223,16 +223,17 @@ SoftwareIsp::strideAndFrameSize(const PixelFormat &outputFormat, const Size &siz
> * \brief Configure the SoftwareIsp object according to the passed in parameters
> * \param[in] inputCfg The input configuration
> * \param[in] outputCfgs The output configurations
> - * \param[in] sensorControls ControlInfoMap of the controls supported by the sensor
> + * \param[in] configInfo The IPA configuration data, received from the pipeline
> + * handler
> * \return 0 on success, a negative errno on failure
> */
> int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
> const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
> - const ControlInfoMap &sensorControls)
> + const ipa::soft::IPAConfigInfo &configInfo)
> {
> ASSERT(ipa_ && debayer_);
>
> - int ret = ipa_->configure(sensorControls);
> + int ret = ipa_->configure(configInfo);
> if (ret < 0)
> return ret;
>
More information about the libcamera-devel
mailing list