[PATCH 2/2] libcamera: rpi: Draw sensor delays from CameraSensorProperties
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Dec 17 17:44:37 CET 2024
On Wed, Nov 27, 2024 at 01:32:33PM +0000, Daniel Scally wrote:
> Now that we have camera sensor control application delay values in
> the CameraSensorProperties class, remove the duplicated definitions
> in the RPi IPA's CameraSensorHelpers and update the pipeline handler
> to use the values from CameraSensorProperties.
>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/ipa/raspberrypi.mojom | 4 ----
> src/ipa/rpi/cam_helper/cam_helper.cpp | 13 -------------
> src/ipa/rpi/cam_helper/cam_helper.h | 7 -------
> src/ipa/rpi/cam_helper/cam_helper_imx283.cpp | 12 ------------
> src/ipa/rpi/cam_helper/cam_helper_imx290.cpp | 11 -----------
> src/ipa/rpi/cam_helper/cam_helper_imx296.cpp | 11 -----------
> src/ipa/rpi/cam_helper/cam_helper_imx477.cpp | 11 -----------
> src/ipa/rpi/cam_helper/cam_helper_imx519.cpp | 11 -----------
> src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 11 -----------
> src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp | 15 ---------------
> src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp | 12 ------------
> src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp | 12 ------------
> src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 12 ------------
> src/ipa/rpi/common/ipa_base.cpp | 14 ++------------
> .../pipeline/rpi/common/pipeline_base.cpp | 9 +++++----
> 15 files changed, 7 insertions(+), 158 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index 0b92587d..e30c70bd 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -12,10 +12,6 @@ import "include/libcamera/ipa/core.mojom";
> const uint32 MaxLsGridSize = 0x8000;
>
> struct SensorConfig {
> - uint32 gainDelay;
> - uint32 exposureDelay;
> - uint32 vblankDelay;
> - uint32 hblankDelay;
> uint32 sensorMetadata;
> };
>
> diff --git a/src/ipa/rpi/cam_helper/cam_helper.cpp b/src/ipa/rpi/cam_helper/cam_helper.cpp
> index 6493e882..8c720652 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper.cpp
> @@ -156,19 +156,6 @@ void CamHelper::setCameraMode(const CameraMode &mode)
> }
> }
>
> -void CamHelper::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /*
> - * These values are correct for many sensors. Other sensors will
> - * need to over-ride this function.
> - */
> - exposureDelay = 2;
> - gainDelay = 1;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> bool CamHelper::sensorEmbeddedDataPresent() const
> {
> return false;
> diff --git a/src/ipa/rpi/cam_helper/cam_helper.h b/src/ipa/rpi/cam_helper/cam_helper.h
> index 4a4ab5e6..29371bdb 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper.h
> +++ b/src/ipa/rpi/cam_helper/cam_helper.h
> @@ -36,11 +36,6 @@ namespace RPiController {
> * exposure time, and to convert between the sensor's gain codes and actual
> * gains.
> *
> - * A function to return the number of frames of delay between updating exposure,
> - * analogue gain and vblanking, and for the changes to take effect. For many
> - * sensors these take the values 2, 1 and 2 respectively, but sensors that are
> - * different will need to over-ride the default function provided.
> - *
> * A function to query if the sensor outputs embedded data that can be parsed.
> *
> * A function to return the sensitivity of a given camera mode.
> @@ -91,8 +86,6 @@ public:
> libcamera::utils::Duration lineLengthPckToDuration(uint32_t lineLengthPck) const;
> virtual uint32_t gainCode(double gain) const = 0;
> virtual double gain(uint32_t gainCode) const = 0;
> - virtual void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const;
> virtual bool sensorEmbeddedDataPresent() const;
> virtual double getModeSensitivity(const CameraMode &mode) const;
> virtual unsigned int hideFramesStartup() const;
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
> index cb0be72a..efc03193 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx283.cpp
> @@ -17,8 +17,6 @@ public:
> CamHelperImx283();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> unsigned int hideFramesModeSwitch() const override;
>
> private:
> @@ -49,16 +47,6 @@ double CamHelperImx283::gain(uint32_t gainCode) const
> return static_cast<double>(2048.0 / (2048 - gainCode));
> }
>
> -void CamHelperImx283::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /* The driver appears to behave as follows: */
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> unsigned int CamHelperImx283::hideFramesModeSwitch() const
> {
> /* After a mode switch, we seem to get 1 bad frame. */
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> index 3b87751e..c1aa8528 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> @@ -17,8 +17,6 @@ public:
> CamHelperImx290();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> unsigned int hideFramesStartup() const override;
> unsigned int hideFramesModeSwitch() const override;
>
> @@ -46,15 +44,6 @@ double CamHelperImx290::gain(uint32_t gainCode) const
> return std::pow(10, 0.015 * gainCode);
> }
>
> -void CamHelperImx290::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> unsigned int CamHelperImx290::hideFramesStartup() const
> {
> /* On startup, we seem to get 1 bad frame. */
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
> index d4a4fa79..ac7ee2ea 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx296.cpp
> @@ -23,8 +23,6 @@ public:
> double gain(uint32_t gainCode) const override;
> uint32_t exposureLines(const Duration exposure, const Duration lineLength) const override;
> Duration exposure(uint32_t exposureLines, const Duration lineLength) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
>
> private:
> static constexpr uint32_t minExposureLines = 1;
> @@ -66,15 +64,6 @@ Duration CamHelperImx296::exposure(uint32_t exposureLines,
> return std::max<uint32_t>(minExposureLines, exposureLines) * timePerLine + 14.26us;
> }
>
> -void CamHelperImx296::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> static CamHelper *create()
> {
> return new CamHelperImx296();
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
> index a53c40cd..a72ac67d 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx477.cpp
> @@ -51,8 +51,6 @@ public:
> void prepare(libcamera::Span<const uint8_t> buffer, Metadata &metadata) override;
> std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
> Duration maxFrameDuration) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> bool sensorEmbeddedDataPresent() const override;
>
> private:
> @@ -159,15 +157,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx477::getBlanking(Duration &exposure,
> return { frameLength - mode_.height, hblank };
> }
>
> -void CamHelperImx477::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 3;
> - hblankDelay = 3;
> -}
> -
> bool CamHelperImx477::sensorEmbeddedDataPresent() const
> {
> return true;
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> index 2ff08653..10cbea48 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx519.cpp
> @@ -51,8 +51,6 @@ public:
> void prepare(libcamera::Span<const uint8_t> buffer, Metadata &metadata) override;
> std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
> Duration maxFrameDuration) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> bool sensorEmbeddedDataPresent() const override;
>
> private:
> @@ -159,15 +157,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx519::getBlanking(Duration &exposure,
> return { frameLength - mode_.height, hblank };
> }
>
> -void CamHelperImx519::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 3;
> - hblankDelay = 3;
> -}
> -
> bool CamHelperImx519::sensorEmbeddedDataPresent() const
> {
> return true;
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> index ec83d9fd..24ffc846 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> @@ -54,8 +54,6 @@ public:
> void process(StatisticsPtr &stats, Metadata &metadata) override;
> std::pair<uint32_t, uint32_t> getBlanking(Duration &exposure, Duration minFrameDuration,
> Duration maxFrameDuration) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> bool sensorEmbeddedDataPresent() const override;
> double getModeSensitivity(const CameraMode &mode) const override;
> unsigned int hideFramesModeSwitch() const override;
> @@ -208,15 +206,6 @@ std::pair<uint32_t, uint32_t> CamHelperImx708::getBlanking(Duration &exposure,
> return { frameLength - mode_.height, hblank };
> }
>
> -void CamHelperImx708::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 3;
> - hblankDelay = 3;
> -}
> -
> bool CamHelperImx708::sensorEmbeddedDataPresent() const
> {
> return true;
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
> index c30b017c..40d6b6d7 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp
> @@ -17,8 +17,6 @@ public:
> CamHelperOv5647();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> unsigned int hideFramesStartup() const override;
> unsigned int hideFramesModeSwitch() const override;
> unsigned int mistrustFramesStartup() const override;
> @@ -52,19 +50,6 @@ double CamHelperOv5647::gain(uint32_t gainCode) const
> return static_cast<double>(gainCode) / 16.0;
> }
>
> -void CamHelperOv5647::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /*
> - * We run this sensor in a mode where the gain delay is bumped up to
> - * 2. It seems to be the only way to make the delays "predictable".
> - */
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> unsigned int CamHelperOv5647::hideFramesStartup() const
> {
> /*
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
> index a8efd389..980495a8 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_ov64a40.cpp
> @@ -18,8 +18,6 @@ public:
> CamHelperOv64a40();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
> double getModeSensitivity(const CameraMode &mode) const override;
>
> private:
> @@ -45,16 +43,6 @@ double CamHelperOv64a40::gain(uint32_t gainCode) const
> return static_cast<double>(gainCode) / 128.0;
> }
>
> -void CamHelperOv64a40::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /* The driver appears to behave as follows: */
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> double CamHelperOv64a40::getModeSensitivity(const CameraMode &mode) const
> {
> if (mode.binX >= 2 && mode.scaleX >= 4) {
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
> index 7b12c445..fc7b999f 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_ov7251.cpp
> @@ -17,8 +17,6 @@ public:
> CamHelperOv7251();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
>
> private:
> /*
> @@ -48,16 +46,6 @@ double CamHelperOv7251::gain(uint32_t gainCode) const
> return static_cast<double>(gainCode) / 16.0;
> }
>
> -void CamHelperOv7251::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /* The driver appears to behave as follows: */
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> static CamHelper *create()
> {
> return new CamHelperOv7251();
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
> index a65c8ac0..e56868e4 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp
> @@ -17,8 +17,6 @@ public:
> CamHelperOv9281();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const override;
>
> private:
> /*
> @@ -48,16 +46,6 @@ double CamHelperOv9281::gain(uint32_t gainCode) const
> return static_cast<double>(gainCode) / 16.0;
> }
>
> -void CamHelperOv9281::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay, int &hblankDelay) const
> -{
> - /* The driver appears to behave as follows: */
> - exposureDelay = 2;
> - gainDelay = 2;
> - vblankDelay = 2;
> - hblankDelay = 2;
> -}
> -
> static CamHelper *create()
> {
> return new CamHelperOv9281();
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 5fce17e6..45e2a1d7 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -134,18 +134,8 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams ¶ms, Ini
> return -EINVAL;
> }
>
> - /*
> - * Pass out the sensor config to the pipeline handler in order
> - * to setup the staggered writer class.
> - */
> - int gainDelay, exposureDelay, vblankDelay, hblankDelay, sensorMetadata;
> - helper_->getDelays(exposureDelay, gainDelay, vblankDelay, hblankDelay);
> - sensorMetadata = helper_->sensorEmbeddedDataPresent();
> -
> - result->sensorConfig.gainDelay = gainDelay;
> - result->sensorConfig.exposureDelay = exposureDelay;
> - result->sensorConfig.vblankDelay = vblankDelay;
> - result->sensorConfig.hblankDelay = hblankDelay;
> + /* Pass out the sensor metadata to the pipeline handler */
> + int sensorMetadata = helper_->sensorEmbeddedDataPresent();
> result->sensorConfig.sensorMetadata = sensorMetadata;
>
> /* Load the tuning file for this sensor. */
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 9e2d9d23..398a0280 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -816,11 +816,12 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
> * Setup our delayed control writer with the sensor default
> * gain and exposure delays. Mark VBLANK for priority write.
> */
> + const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();
> std::unordered_map<uint32_t, RPi::DelayedControls::ControlParams> params = {
> - { V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
> - { V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
> - { V4L2_CID_HBLANK, { result.sensorConfig.hblankDelay, false } },
> - { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
> + { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> + { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> + { V4L2_CID_HBLANK, { delays.hblankDelay, false } },
> + { V4L2_CID_VBLANK, { delays.vblankDelay, true } }
> };
> data->delayedCtrls_ = std::make_unique<RPi::DelayedControls>(data->sensor_->device(), params);
> data->sensorMetadata_ = result.sensorConfig.sensorMetadata;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list