[libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: add FrameDurationLimits control
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Nov 23 15:39:02 CET 2022
Hello Nicholas and Jacopo,
Thank you for the patch.
On Wed, Nov 23, 2022 at 02:43:46PM +0100, Jacopo Mondi via libcamera-devel wrote:
> 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>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/ipa/rkisp1.mojom | 6 ++-
> src/ipa/rkisp1/rkisp1.cpp | 57 +++++++++++++++++++++---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 ++++---
> 3 files changed, 67 insertions(+), 13 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 a9e144303072..9265d3c9f53c 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 */
> @@ -180,9 +188,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;
> }
> @@ -200,7 +207,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;
>
> @@ -230,6 +238,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.
> @@ -332,6 +343,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.
> + */
> + 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]);
> +
> + *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 e946ccc4c930..231b16eca110 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -347,8 +347,15 @@ 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) {
> + LOG(RkISP1, Error) << "Camera sensor information not available";
> + return ret;
> + }
> +
> + ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> + sensorInfo, sensor_->controls(), &controlInfo_);
> if (ret < 0) {
> LOG(RkISP1, Error) << "IPA initialization failure";
> return ret;
> @@ -718,14 +725,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> ipa::rkisp1::IPAConfigInfo ipaConfig{};
>
> ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo);
> - if (ret) {
> - LOG(RkISP1, Error) << "Camera sensor information not available";
> + if (ret)
> return ret;
> - }
>
> 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;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list