[libcamera-devel] [PATCH 4/4] ipa: rkisp1: add FrameDurationLimits control
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Nov 4 13:37:34 CET 2022
Quoting Jacopo Mondi via libcamera-devel (2022-11-04 10:50:04)
> From: Nicholas Roth <nicholas at rothemail.net>
>
> Currently, the Android HAL does not work on rkisp1-based devices because
> required FrameDurationLimits metadata is missing from the IPA
> implementation.
>
> This change sets FrameDurationLimits for rkisp1 based on the existing
> ipu3 implementation, using the sensor's reported range of vertical
> blanking intervals with the minimum reported horizontal blanking
> interval.
>
> Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
> ---
> include/libcamera/ipa/rkisp1.mojom | 6 ++-
> src/ipa/rkisp1/rkisp1.cpp | 57 +++++++++++++++++++++---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +++--
> 3 files changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index 36bf291e8a51..1009e970a1b5 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -15,14 +15,16 @@ struct IPAConfigInfo {
>
> interface IPARkISP1Interface {
> init(libcamera.IPASettings settings,
> - uint32 hwRevision)
> + uint32 hwRevision,
> + libcamera.IPACameraSensorInfo sensorInfo,
> + libcamera.ControlInfoMap sensorControls)
> => (int32 ret, libcamera.ControlInfoMap ipaControls);
> start() => (int32 ret);
> stop();
>
> configure(IPAConfigInfo configInfo,
> map<uint32, libcamera.IPAStream> streamConfig)
> - => (int32 ret);
> + => (int32 ret, libcamera.ControlInfoMap ipaControls);
>
> mapBuffers(array<libcamera.IPABuffer> buffers);
> unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index e1ede62ead3a..6d1238d54f57 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -49,12 +49,15 @@ public:
> IPARkISP1();
>
> int init(const IPASettings &settings, unsigned int hwRevision,
> + const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> ControlInfoMap *ipaControls) override;
> int start() override;
> void stop() override;
>
> int configure(const IPAConfigInfo &ipaConfig,
> - const std::map<uint32_t, IPAStream> &streamConfig) override;
> + const std::map<uint32_t, IPAStream> &streamConfig,
> + ControlInfoMap *ipaControls) override;
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
>
> @@ -67,6 +70,9 @@ protected:
> std::string logPrefix() const override;
>
> private:
> + void updateControls(const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> + ControlInfoMap *ipaControls);
> void setControls(unsigned int frame);
>
> std::map<unsigned int, FrameBuffer> buffers_;
> @@ -114,6 +120,8 @@ std::string IPARkISP1::logPrefix() const
> }
>
> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> + const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> ControlInfoMap *ipaControls)
> {
> /* \todo Add support for other revisions */
> @@ -179,9 +187,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> if (ret)
> return ret;
>
> - /* Return the controls handled by the IPA. */
> - ControlInfoMap::Map ctrlMap = rkisp1Controls;
> - *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> + /* Initialize controls. */
> + updateControls(sensorInfo, sensorControls, ipaControls);
>
> return 0;
> }
> @@ -199,7 +206,8 @@ void IPARkISP1::stop()
> }
>
> int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
> - [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig)
> + [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> + ControlInfoMap *ipaControls)
> {
> sensorControls_ = ipaConfig.sensorControls;
>
> @@ -229,6 +237,9 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
> context_.configuration.sensor.size = info.outputSize;
> context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;
>
> + /* Update the camera controls using the new sensor settings. */
> + updateControls(info, sensorControls_, ipaControls);
> +
> /*
> * When the AGC computes the new exposure values for a frame, it needs
> * to know the limits for shutter speed and analogue gain.
> @@ -329,6 +340,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
> metadataReady.emit(frame, metadata);
> }
>
> +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> + ControlInfoMap *ipaControls)
> +{
> + ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +
> + /*
> + * Compute the frame duration limits.
> + *
> + * The frame length is computed assuming a fixed line length combined
> + * with the vertical frame sizes.
> + */
Does the CameraSensor class have enough information to do all this in a
common location?
Yakety Yak - but it seems like it might be nicer to have this code in
CameraSensor class, and used by both IPU3 and RKISP1... (and therefore
any other platform using CameraSensor class).
Although - perhaps it's reliant upon the desired configuration too - but
maybe it's still something that could live in libipa ?
ControlInfo limits = libipa::frameDurationLimits(sensorControls,
sensorInfo.outputSise);
?
--
Kieran
> + const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
> + uint32_t hblank = v4l2HBlank.def().get<int32_t>();
> + uint32_t lineLength = sensorInfo.outputSize.width + hblank;
> +
> + const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
> + std::array<uint32_t, 3> frameHeights{
> + v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
> + v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
> + v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
> + };
> +
> + std::array<int64_t, 3> frameDurations;
> + for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> + uint64_t frameSize = lineLength * frameHeights[i];
> + frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
> + }
> +
> + ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
> + frameDurations[1],
> + frameDurations[2]);
Or
ctrlMap[&controls::FrameDurationLimits] =
libipa::frameDurationLimits(sensorControls,
sensorInfo.outputSize);
directly even?
> +
> + *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +}
> +
> void IPARkISP1::setControls(unsigned int frame)
> {
> /*
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 023d1fd4744f..8fbeed8ca800 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -347,8 +347,13 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> ipaTuningFile = std::string(configFromEnv);
> }
>
> - int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> - &controlInfo_);
> + IPACameraSensorInfo sensorInfo{};
> + int ret = sensor_->sensorInfo(&sensorInfo);
> + if (ret)
> + return ret;
> +
> + ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> + sensorInfo, sensor_->controls(), &controlInfo_);
> if (ret < 0) {
> LOG(RkISP1, Error) << "IPA initialization failure";
> return ret;
> @@ -724,7 +729,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>
> ipaConfig.sensorControls = data->sensor_->controls();
>
> - ret = data->ipa_->configure(ipaConfig, streamConfig);
> + ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);
> if (ret) {
> LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
> return ret;
> --
> 2.38.1
>
More information about the libcamera-devel
mailing list