[libcamera-devel] [PATCH v3 1/3] pipeline: ipa: raspberrypi: Move ControlInfoMap to the IPA
David Plowman
david.plowman at raspberrypi.com
Tue Jun 28 10:39:22 CEST 2022
Hi Naush
Thanks for this patch! Very pleased that we'll be able to get accurate
framerate limits and so on for each mode.
On Wed, 22 Jun 2022 at 11:21, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Currently the pipeline handler advertises controls handled by the IPA from a
> static ControlInfoMap defined in the raspberrypi.h header. This change removes
> this header file, and instead the IPA returns the ControlInfoMap to the pipeline
> handler from the ipa::init() function. This is done to allow the IPA to adjust
> the limits of the controls based on the sensor mode in a subsequent change.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=83
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Thanks
David
> ---
> include/libcamera/ipa/raspberrypi.h | 55 -------------------
> include/libcamera/ipa/raspberrypi.mojom | 7 ++-
> src/ipa/raspberrypi/raspberrypi.cpp | 39 ++++++++++---
> .../pipeline/raspberrypi/raspberrypi.cpp | 30 +++++-----
> .../pipeline/raspberrypi/rpi_stream.h | 1 -
> 5 files changed, 53 insertions(+), 79 deletions(-)
> delete mode 100644 include/libcamera/ipa/raspberrypi.h
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> deleted file mode 100644
> index 6a56b0083b85..000000000000
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019-2020, Raspberry Pi Ltd.
> - *
> - * raspberrypi.h - Image Processing Algorithm interface for Raspberry Pi
> - */
> -
> -#pragma once
> -
> -#include <stdint.h>
> -
> -#include <libcamera/control_ids.h>
> -#include <libcamera/controls.h>
> -
> -#ifndef __DOXYGEN__
> -
> -namespace libcamera {
> -
> -namespace RPi {
> -
> -/*
> - * List of controls handled by the Raspberry Pi IPA
> - *
> - * \todo This list will need to be built dynamically from the control
> - * algorithms loaded by the json file, once this is supported. At that
> - * point applications should check first whether a control is supported,
> - * and the pipeline handler may be reverted so that it aborts when an
> - * unsupported control is encountered.
> - */
> -static const ControlInfoMap Controls({
> - { &controls::AeEnable, ControlInfo(false, true) },
> - { &controls::ExposureTime, ControlInfo(0, 999999) },
> - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> - { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> - { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> - { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> - { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },
> - { &controls::AwbEnable, ControlInfo(false, true) },
> - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> - { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> - { &controls::Brightness, ControlInfo(-1.0f, 1.0f, 0.0f) },
> - { &controls::Contrast, ControlInfo(0.0f, 32.0f, 1.0f) },
> - { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
> - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> - { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> - { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> - { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> - }, controls::controls);
> -
> -} /* namespace RPi */
> -
> -} /* namespace libcamera */
> -
> -#endif /* __DOXYGEN__ */
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index a60c3bb43d3c..77f52c282b0f 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -26,6 +26,11 @@ struct SensorConfig {
> uint32 sensorMetadata;
> };
>
> +struct IPAInitResult {
> + SensorConfig sensorConfig;
> + libcamera.ControlInfoMap controlInfo;
> +};
> +
> struct ISPConfig {
> uint32 embeddedBufferId;
> uint32 bayerBufferId;
> @@ -50,7 +55,7 @@ struct StartConfig {
>
> interface IPARPiInterface {
> init(libcamera.IPASettings settings)
> - => (int32 ret, SensorConfig sensorConfig);
> + => (int32 ret, IPAInitResult result);
> start(libcamera.ControlList controls) => (StartConfig startConfig);
> stop();
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 3b126bb5175e..089528f5e126 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -24,7 +24,6 @@
> #include <libcamera/framebuffer.h>
> #include <libcamera/ipa/ipa_interface.h>
> #include <libcamera/ipa/ipa_module_info.h>
> -#include <libcamera/ipa/raspberrypi.h>
> #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> #include <libcamera/request.h>
>
> @@ -72,6 +71,28 @@ constexpr Duration defaultMaxFrameDuration = 250.0s;
> */
> constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;
>
> +/* List of controls handled by the Raspberry Pi IPA */
> +static const ControlInfoMap::Map ipaControls{
> + { &controls::AeEnable, ControlInfo(false, true) },
> + { &controls::ExposureTime, ControlInfo(0, 999999) },
> + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> + { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> + { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> + { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> + { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },
> + { &controls::AwbEnable, ControlInfo(false, true) },
> + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> + { &controls::Brightness, ControlInfo(-1.0f, 1.0f, 0.0f) },
> + { &controls::Contrast, ControlInfo(0.0f, 32.0f, 1.0f) },
> + { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
> + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> + { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> + { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> + { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> +};
> +
> LOG_DEFINE_CATEGORY(IPARPI)
>
> namespace ipa::RPi {
> @@ -91,7 +112,7 @@ public:
> munmap(lsTable_, MaxLsGridSize);
> }
>
> - int init(const IPASettings &settings, SensorConfig *sensorConfig) override;
> + int init(const IPASettings &settings, IPAInitResult *result) override;
> void start(const ControlList &controls, StartConfig *startConfig) override;
> void stop() override {}
>
> @@ -180,7 +201,7 @@ private:
> uint32_t maxSensorGainCode_;
> };
>
> -int IPARPi::init(const IPASettings &settings, SensorConfig *sensorConfig)
> +int IPARPi::init(const IPASettings &settings, IPAInitResult *result)
> {
> /*
> * Load the "helper" for this sensor. This tells us all the device specific stuff
> @@ -202,15 +223,19 @@ int IPARPi::init(const IPASettings &settings, SensorConfig *sensorConfig)
> helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);
> sensorMetadata = helper_->SensorEmbeddedDataPresent();
>
> - sensorConfig->gainDelay = gainDelay;
> - sensorConfig->exposureDelay = exposureDelay;
> - sensorConfig->vblankDelay = vblankDelay;
> - sensorConfig->sensorMetadata = sensorMetadata;
> + result->sensorConfig.gainDelay = gainDelay;
> + result->sensorConfig.exposureDelay = exposureDelay;
> + result->sensorConfig.vblankDelay = vblankDelay;
> + result->sensorConfig.sensorMetadata = sensorMetadata;
>
> /* Load the tuning file for this sensor. */
> controller_.Read(settings.configurationFile.c_str());
> controller_.Initialise();
>
> + /* Return the controls handled by the IPA */
> + ControlInfoMap::Map ctrlMap = ipaControls;
> + result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +
> return 0;
> }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index adc397e8aabd..d980c1a71dd8 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -20,7 +20,6 @@
> #include <libcamera/camera.h>
> #include <libcamera/control_ids.h>
> #include <libcamera/formats.h>
> -#include <libcamera/ipa/raspberrypi.h>
> #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> #include <libcamera/ipa/raspberrypi_ipa_proxy.h>
> #include <libcamera/logging.h>
> @@ -199,7 +198,7 @@ public:
> void freeBuffers();
> void frameStarted(uint32_t sequence);
>
> - int loadIPA(ipa::RPi::SensorConfig *sensorConfig);
> + int loadIPA(ipa::RPi::IPAInitResult *result);
> int configureIPA(const CameraConfiguration *config, ipa::RPi::IPAConfigResult *result);
>
> void enumerateVideoDevices(MediaLink *link);
> @@ -1231,15 +1230,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
>
> data->sensorFormats_ = populateSensorFormats(data->sensor_);
>
> - ipa::RPi::SensorConfig sensorConfig;
> - if (data->loadIPA(&sensorConfig)) {
> + ipa::RPi::IPAInitResult result;
> + if (data->loadIPA(&result)) {
> LOG(RPI, Error) << "Failed to load a suitable IPA library";
> return -EINVAL;
> }
>
> - if (sensorConfig.sensorMetadata ^ !!unicamEmbedded) {
> + if (result.sensorConfig.sensorMetadata ^ !!unicamEmbedded) {
> LOG(RPI, Warning) << "Mismatch between Unicam and CamHelper for embedded data usage!";
> - sensorConfig.sensorMetadata = false;
> + result.sensorConfig.sensorMetadata = false;
> if (unicamEmbedded)
> data->unicam_[Unicam::Embedded].dev()->bufferReady.disconnect();
> }
> @@ -1253,7 +1252,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
> * iterate over all streams in one go.
> */
> data->streams_.push_back(&data->unicam_[Unicam::Image]);
> - if (sensorConfig.sensorMetadata)
> + if (result.sensorConfig.sensorMetadata)
> data->streams_.push_back(&data->unicam_[Unicam::Embedded]);
>
> for (auto &stream : data->isp_)
> @@ -1275,15 +1274,16 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
> * gain and exposure delays. Mark VBLANK for priority write.
> */
> std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> - { V4L2_CID_ANALOGUE_GAIN, { sensorConfig.gainDelay, false } },
> - { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },
> - { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }
> + { V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
> + { V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
> + { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
> };
> data->delayedCtrls_ = std::make_unique<DelayedControls>(data->sensor_->device(), params);
> - data->sensorMetadata_ = sensorConfig.sensorMetadata;
> + data->sensorMetadata_ = result.sensorConfig.sensorMetadata;
> +
> + /* Register initial controls that the Raspberry Pi IPA can handle. */
> + data->controlInfo_ = std::move(result.controlInfo);
>
> - /* Register the controls that the Raspberry Pi IPA can handle. */
> - data->controlInfo_ = RPi::Controls;
> /* Initialize the camera properties. */
> data->properties_ = data->sensor_->properties();
>
> @@ -1509,7 +1509,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)
> delayedCtrls_->applyControls(sequence);
> }
>
> -int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)
> +int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
> {
> ipa_ = IPAManager::createIPA<ipa::RPi::IPAProxyRPi>(pipe(), 1, 1);
>
> @@ -1535,7 +1535,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)
>
> IPASettings settings(configurationFile, sensor_->model());
>
> - return ipa_->init(settings, sensorConfig);
> + return ipa_->init(settings, result);
> }
>
> int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPAConfigResult *result)
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> index c37f7e82eef6..fe01110019b7 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -12,7 +12,6 @@
> #include <unordered_map>
> #include <vector>
>
> -#include <libcamera/ipa/raspberrypi.h>
> #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> #include <libcamera/stream.h>
>
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list