| Message ID | 20210119143711.153517-7-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Jan 19, 2021 at 03:37:06PM +0100, Jacopo Mondi wrote: > Register the ScalerCrop control in the ImgU pipeline handler computed > by using the Viewfinder [1280x720] pipeline output configuration and > the sensor resolution as parameters. > > The ScalerCrop control limits should be updated everytime a new s/everytime/every time/ > configuration is applied to the sensor. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index f928af4d92a2..fc5592f33032 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -40,6 +40,7 @@ static constexpr unsigned int IMGU_OUTPUT_WIDTH_ALIGN = 64; > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; > static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; > +static constexpr Size IPU3ViewfinderSize(1280, 720); > > static const ControlInfoMap::Map IPU3Controls = { > { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, > @@ -376,7 +377,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > * capped to the maximum sensor resolution and aligned > * to the ImgU output constraints. > */ > - size = sensorResolution.boundedTo({ 1280, 720 }) > + size = sensorResolution.boundedTo(IPU3ViewfinderSize) > .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN, > IMGU_OUTPUT_HEIGHT_ALIGN); > pixelFormat = formats::NV12; > @@ -745,7 +746,7 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > */ > int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > { > - const CameraSensor *sensor = data->cio2_.sensor(); > + CameraSensor *sensor = data->cio2_.sensor(); > CameraSensorInfo sensorInfo{}; > > int ret = sensor->sensorInfo(&sensorInfo); > @@ -780,6 +781,74 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > defExposure); > > + /* > + * Compute the scaler crop limits. > + * > + * \todo The scaler crop limits depend on the sensor configuration. It > + * should be updated when a new configuration is applied. To initialize > + * the control use the 'Viewfinder' configuration (1280x720) as the > + * pipeline output resolution and the full sensor size as input frame > + * (see the todo note in the validate() function about the usage of the > + * 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. > + */ > + const Rectangle &analogueCrop = sensorInfo.analogCrop; > + Rectangle maxCrop = analogueCrop; > + > + /* > + * As the ImgU cannot up-scale, the minimum selection rectangle has to > + * be as large as the pipeline output size. Use the default viewfinder > + * configuration as the desired output size and calculate the minimum > + * rectangle required to satisfy the ImgU processing margins, unless the > + * sensor resolution is smaller. > + * > + * \todo This implementation is based on the same assumptions about the > + * ImgU pipeline configuration described in then viewfinder and main > + * output sizes calculation in the validate() function. > + */ > + > + /* The strictly smaller size than the sensor resolution, aligned to margins. */ > + Size minSize = Size(sensor->resolution().width - 1, > + sensor->resolution().height - 1) > + .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN, > + IMGU_OUTPUT_HEIGHT_MARGIN); > + > + /* > + * Either the smallest margin-aligned size larger than the viewfinder > + * size or the adjusted sensor resolution. > + */ > + minSize = Size(IPU3ViewfinderSize.width + 1, > + IPU3ViewfinderSize.height + 1) > + .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN, > + IMGU_OUTPUT_HEIGHT_MARGIN) > + .boundedTo(minSize); > + > + /* > + * Re-scale in the sensor's native coordinates. Report (0,0) as > + * top-left corner as we allow application to feely pan the crop area. s/feely/freely/ > + */ > + Rectangle minCrop(minSize); > + minCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize); scaledBy() is a const function that returns a new rectangle. You probably want scaleBy() here (or see below for an alternative). > + minCrop.x = 0; > + minCrop.y = 0; (x,y) are set to 0 by the constructor, so you could write Rectangle minCrop = minSize.scaledBy(analogueCrop.size(), sensorInfo.outputSize); Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop); > + > data->controlInfo_ = std::move(controls); > > return 0;
Hi Laurent, On Mon, Jan 25, 2021 at 01:31:09PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > > + /* > > + * Re-scale in the sensor's native coordinates. Report (0,0) as > > + * top-left corner as we allow application to feely pan the crop area. > > s/feely/freely/ > > > + */ > > + Rectangle minCrop(minSize); > > + minCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize); > > scaledBy() is a const function that returns a new rectangle. You > probably want scaleBy() here (or see below for an alternative). > > > + minCrop.x = 0; > > + minCrop.y = 0; > > (x,y) are set to 0 by the constructor, so you could write > > Rectangle minCrop = minSize.scaledBy(analogueCrop.size(), > sensorInfo.outputSize); I went for the minCrop initialization as 'scaledBy' is not a Size class method. In facts, if preferred, I could work around it with Rectangle minCrop = Rectangle(minSize).scaledBy(analogueCrop.size(), sensorInfo.outputSize); Which I don't mind > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + > > + controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop); > > + > > data->controlInfo_ = std::move(controls); > > > > return 0; > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Mon, Jan 25, 2021 at 03:19:21PM +0100, Jacopo Mondi wrote: > On Mon, Jan 25, 2021 at 01:31:09PM +0200, Laurent Pinchart wrote: > > > + /* > > > + * Re-scale in the sensor's native coordinates. Report (0,0) as > > > + * top-left corner as we allow application to feely pan the crop area. > > > > s/feely/freely/ > > > > > + */ > > > + Rectangle minCrop(minSize); > > > + minCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize); > > > > scaledBy() is a const function that returns a new rectangle. You > > probably want scaleBy() here (or see below for an alternative). > > > > > + minCrop.x = 0; > > > + minCrop.y = 0; > > > > (x,y) are set to 0 by the constructor, so you could write > > > > Rectangle minCrop = minSize.scaledBy(analogueCrop.size(), > > sensorInfo.outputSize); > > I went for the minCrop initialization as 'scaledBy' is not a Size > class method. Oops, indeed. > In facts, if preferred, I could work around it with > Rectangle minCrop = Rectangle(minSize).scaledBy(analogueCrop.size(), > sensorInfo.outputSize); > > Which I don't mind Up to you. If you prefer keeping the existing code, just remember to s/scaledBy/scaleBy/ > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + > > > + controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop); > > > + > > > data->controlInfo_ = std::move(controls); > > > > > > return 0;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index f928af4d92a2..fc5592f33032 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -40,6 +40,7 @@ static constexpr unsigned int IMGU_OUTPUT_WIDTH_ALIGN = 64; static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; +static constexpr Size IPU3ViewfinderSize(1280, 720); static const ControlInfoMap::Map IPU3Controls = { { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, @@ -376,7 +377,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * capped to the maximum sensor resolution and aligned * to the ImgU output constraints. */ - size = sensorResolution.boundedTo({ 1280, 720 }) + size = sensorResolution.boundedTo(IPU3ViewfinderSize) .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN, IMGU_OUTPUT_HEIGHT_ALIGN); pixelFormat = formats::NV12; @@ -745,7 +746,7 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) */ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) { - const CameraSensor *sensor = data->cio2_.sensor(); + CameraSensor *sensor = data->cio2_.sensor(); CameraSensorInfo sensorInfo{}; int ret = sensor->sensorInfo(&sensorInfo); @@ -780,6 +781,74 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, defExposure); + /* + * Compute the scaler crop limits. + * + * \todo The scaler crop limits depend on the sensor configuration. It + * should be updated when a new configuration is applied. To initialize + * the control use the 'Viewfinder' configuration (1280x720) as the + * pipeline output resolution and the full sensor size as input frame + * (see the todo note in the validate() function about the usage of the + * 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. + */ + const Rectangle &analogueCrop = sensorInfo.analogCrop; + Rectangle maxCrop = analogueCrop; + + /* + * As the ImgU cannot up-scale, the minimum selection rectangle has to + * be as large as the pipeline output size. Use the default viewfinder + * configuration as the desired output size and calculate the minimum + * rectangle required to satisfy the ImgU processing margins, unless the + * sensor resolution is smaller. + * + * \todo This implementation is based on the same assumptions about the + * ImgU pipeline configuration described in then viewfinder and main + * output sizes calculation in the validate() function. + */ + + /* The strictly smaller size than the sensor resolution, aligned to margins. */ + Size minSize = Size(sensor->resolution().width - 1, + sensor->resolution().height - 1) + .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN, + IMGU_OUTPUT_HEIGHT_MARGIN); + + /* + * Either the smallest margin-aligned size larger than the viewfinder + * size or the adjusted sensor resolution. + */ + minSize = Size(IPU3ViewfinderSize.width + 1, + IPU3ViewfinderSize.height + 1) + .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN, + IMGU_OUTPUT_HEIGHT_MARGIN) + .boundedTo(minSize); + + /* + * Re-scale in the sensor's native coordinates. Report (0,0) as + * top-left corner as we allow application to feely pan the crop area. + */ + Rectangle minCrop(minSize); + minCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize); + minCrop.x = 0; + minCrop.y = 0; + + controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop); + data->controlInfo_ = std::move(controls); return 0;
Register the ScalerCrop control in the ImgU pipeline handler computed by using the Viewfinder [1280x720] pipeline output configuration and the sensor resolution as parameters. The ScalerCrop control limits should be updated everytime a new configuration is applied to the sensor. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-)