[libcamera-devel] [PATCH v2 01/10] camera_sensor: Add minimum and maximum line length to IPACameraSensorInfo
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Oct 8 00:28:47 CEST 2022
Hi Naush,
Thank you for the patch.
On Thu, Oct 06, 2022 at 02:17:35PM +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.
>
> Since the IPAs now use minLineLength for their calculations, set the starting
> value of the V4L2_CID_HBLANK control to its minimum in CameraSensor::init().
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Tested-by: Dave Stevenson <dave.stevenson at raspberrypi.com>
> ---
> include/libcamera/ipa/core.mojom | 21 +++++++++++++++------
> src/ipa/ipu3/ipu3.cpp | 6 ++++--
> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-
> src/ipa/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/camera_sensor.cpp | 15 +++++++++++++--
> 5 files changed, 34 insertions(+), 12 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 b93a09d40c39..7e26fc5639c2 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -336,7 +336,8 @@ 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;
The * should be aligned below the =. Same below.
>
> /* Load the tuning data file. */
> File file(settings.configurationFile);
> @@ -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
> 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..83f81d655395 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -176,6 +176,15 @@ int CameraSensor::init()
> if (ret)
> return ret;
>
> + /* Set HBLANK to the minimum as a starting value. */
I would capture the rationale here.
/*
* Set HBLANK to the minimum to start with a well-defined line length,
* allowing IPA modules that do not modify HBLANK to use the sensor
* minimum line length in their calculations.
*/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
No need to resubmit if only those small changes are required.
> + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
> + ControlList ctrl(subdev_->controls());
> +
> + ctrl.set(V4L2_CID_HBLANK, hblank.min().get<int32_t>());
> + ret = subdev_->setControls(&ctrl);
> + if (ret)
> + return ret;
> +
> return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> }
>
> @@ -883,10 +892,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>();
> +
> 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