[libcamera-devel] [PATCH v3 1/5] libcamera: ipu3: Initialize controls using sensor resolution
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 9 00:29:58 CET 2021
Hi Jacopo,
Thank you for the patch.
On Mon, Feb 22, 2021 at 11:52:18AM +0100, Jacopo Mondi wrote:
> The controls' limits initialized by the IPU3 pipeline handler depend
> on the sensor configuration. In order to compute controls using a known
> state apply to the sensor a configuration equal to its own resolution.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index b8a655ce0b68..f867b5913b27 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -805,10 +805,21 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> */
> int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> {
> + /*
> + * \todo The here intialized controls depend on sensor configuration.
s/intialized/initialized/
"here initialized" sounds weird to me, would "The controls initialized
herein depend on sensor configuration" be better ?
But more than that, the todo should describe what needs to be done, not
just the current state, otherwise we'll wonder in a few months from now
what was meant.
The rest of the patch looks good, so it's really a minor comment.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + *
> + * Initialize the sensor using its resolution and compute the control
> + * limits.
> + */
> CameraSensor *sensor = data->cio2_.sensor();
> - CameraSensorInfo sensorInfo{};
> + V4L2SubdeviceFormat sensorFormat = {};
> + sensorFormat.size = sensor->resolution();
> + int ret = sensor->setFormat(&sensorFormat);
> + if (ret)
> + return ret;
>
> - int ret = sensor->sensorInfo(&sensorInfo);
> + CameraSensorInfo sensorInfo{};
> + ret = sensor->sensorInfo(&sensorInfo);
> if (ret)
> return ret;
>
> @@ -851,17 +862,6 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> * sensor's full frame as ImgU input).
> */
>
> - /* Re-fetch the sensor info updated to use the largest resolution. */
> - V4L2SubdeviceFormat sensorFormat = {};
> - sensorFormat.size = sensor->resolution();
> - ret = sensor->setFormat(&sensorFormat);
> - if (ret)
> - return ret;
> -
> - ret = sensor->sensorInfo(&sensorInfo);
> - if (ret)
> - return ret;
> -
> /*
> * The maximum scaler crop rectangle is the analogue crop used to
> * produce the maximum frame size.
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list