[libcamera-devel] [PATCH v1 1/9] camera_sensor: Add minimum and maximum line length to IPACameraSensorInfo
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 4 18:40:20 CEST 2022
Hi Naush,
Thank you for the patch.
On Mon, Oct 03, 2022 at 09:39:27AM +0100, Naushir Patuck via libcamera-devel wrote:
> Add fields for minimum and maximum line length (in units of pixels) to the
> IPACameraSensorInfo structure. This replaces the existing lineLength field.
>
> Update the ipu3, raspberrypi and rkisp1 IPAs to use IPACameraSensorInfo::minLineLength
> instead of IPACameraSensorInfo::lineLength, as logically we will always want to
> use the fastest sensor readout by default.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/ipa/core.mojom | 21 +++++++++++++++------
> src/ipa/ipu3/ipu3.cpp | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-
> src/ipa/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/camera_sensor.cpp | 6 ++++--
This causes a compilation failure in src/ipa/ipu3/ipu3.cpp due to a
change that has been merged recently. It's easy to fix with
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index d66965e7d94e..c89f76c56ee3 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -499,7 +500,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
context_.frameContexts.clear();
/* Initialise the sensor configuration. */
- context_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
+ context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength
+ * 1.0s / sensorInfo_.pixelRate;
/*
* Compute the sensor V4L2 controls to be used by the algorithms and
If that's the only change needed in this series, I'll fix it myself,
otherwise you can include this in v2.
> 5 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> index 74f3339e56f2..2bc3028c22d6 100644
> --- a/include/libcamera/ipa/core.mojom
> +++ b/include/libcamera/ipa/core.mojom
> @@ -172,10 +172,17 @@ module libcamera;
> */
>
> /**
> - * \var IPACameraSensorInfo::lineLength
> - * \brief Total line length in pixels
> + * \var IPACameraSensorInfo::minLineLength
> + * \brief The minimum line length in pixels
> *
> - * The total line length in pixel clock periods, including blanking.
> + * The minimum allowable line length in pixel clock periods, including blanking.
> + */
> +
> +/**
> + * \var IPACameraSensorInfo::maxLineLength
> + * \brief The maximum line length in pixels
> + *
> + * The maximum allowable line length in pixel clock periods, including blanking.
> */
>
> /**
> @@ -189,7 +196,7 @@ module libcamera;
> * To obtain the minimum frame duration:
> *
> * \verbatim
> - frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)
> + frameDuration(s) = minFrameLength(lines) * minLineLength(pixels) / pixelRate(pixels per second)
> \endverbatim
> */
>
> @@ -204,7 +211,7 @@ module libcamera;
> * To obtain the maximum frame duration:
> *
> * \verbatim
> - frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)
> + frameDuration(s) = maxFrameLength(lines) * maxLineLength(pixels) / pixelRate(pixels per second)
> \endverbatim
> */
> struct IPACameraSensorInfo {
> @@ -217,7 +224,9 @@ struct IPACameraSensorInfo {
> Size outputSize;
>
> uint64 pixelRate;
> - uint32 lineLength;
> +
> + uint32 minLineLength;
> + uint32 maxLineLength;
>
> uint32 minFrameLength;
> uint32 maxFrameLength;
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index d1ea081d595d..da029571ba55 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -341,7 +341,7 @@ int IPAIPU3::init(const IPASettings &settings,
>
> /* Clean context */
> context_.configuration = {};
> - context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
> + context_.configuration.sensor.lineDuration = sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;
This is getting long, let's wrap it.
context_.configuration.sensor.lineDuration = sensorInfo.minLineLength
* 1.0s / sensorInfo.pixelRate;
(I can also apply this change myself if no v2 is otherwise needed.)
>
> /* Load the tuning data file. */
> File file(settings.configurationFile);
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 8d731435764e..358a119da222 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -356,7 +356,7 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)
> * Calculate the line length as the ratio between the line length in
> * pixels and the pixel rate.
> */
> - mode_.lineLength = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate);
> + mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate);
>
> /*
> * Set the frame length limits for the mode to ensure exposure and
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 32feb1682749..ddb22d98eb41 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
> context_.configuration.hw.revision = hwRevision_;
>
> context_.configuration.sensor.size = info.outputSize;
> - context_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate;
> + context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;
>
> /*
> * When the AGC computes the new exposure values for a frame, it needs
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 911fd0beae4e..c2c8d3d44e26 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -883,10 +883,12 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
> return -EINVAL;
> }
>
> - int32_t hblank = ctrls.get(V4L2_CID_HBLANK).get<int32_t>();
> - info->lineLength = info->outputSize.width + hblank;
> info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
>
> + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
> + info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();
> + info->maxLineLength = info->outputSize.width + hblank.max().get<int32_t>();
Hmmm... If the current value of V4L2_CID_HBLANK is different than the
minimum, won't all the IPA modules compute an incorrect line length ?
I'm sure this is fixed for the Raspberry Pi IPA module in subsequent
patches, but the IPU3 and RkISP1 will likely be affected.
> +
> const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);
> info->minFrameLength = info->outputSize.height + vblank.min().get<int32_t>();
> info->maxFrameLength = info->outputSize.height + vblank.max().get<int32_t>();
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list