[PATCH v2 10/19] libcamera: software_isp: Call Algorithm::configure
Milan Zamazal
mzamazal at redhat.com
Sat Jul 13 18:00:06 CEST 2024
Hi Umang,
thank you for review.
Umang Jain <umang.jain at ideasonboard.com> writes:
> Hi Milan,
>
> Thank you for the patch
>
> On 03/07/24 11:21 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 | 4 +++-
>> 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, 29 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
>> index 7365b49a..ff26b1d4 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 bd48ece9..5e124016 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);
>> [async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls);
>> diff --git a/src/ipa/simple/module.h b/src/ipa/simple/module.h
>> index 33a7d1db..8d4d53fb 100644
>> --- a/src/ipa/simple/module.h
>> +++ b/src/ipa/simple/module.h
>> @@ -9,6 +9,8 @@
>> #include <libcamera/controls.h>
>> +#include <libcamera/ipa/soft_ipa_interface.h>
>> +
>> #include "libcamera/internal/software_isp/debayer_params.h"
>> #include "libcamera/internal/software_isp/swisp_stats.h"
>> @@ -20,7 +22,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 dd300387..49fff312 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -72,7 +72,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;
>> @@ -206,9 +206,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;
>> @@ -247,6 +247,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 e96674ae..e0c9fe5c 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -1294,10 +1294,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);
>
> I guess {... } braces here as well. Other than that,
Right, I'll fix it. There are always more reasons to love the
simplicity of Lisp syntax. :-)
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>> + 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 0f4b23d3..2bbb86da 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -224,16 +224,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