| Message ID | 20210222105222.393677-2-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@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.
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. + * + * 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.
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@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)