[libcamera-devel,v3,5/6] libcamera: pipeline: raspberrypi: Implementation of digital zoom

9868 diff mbox series

During configure() we update the SensorOutputSize to the correct value
for this camera mode (which we also note internally) and work out the
minimum crop size allowed by the ISP.

Whenever a new IspCrop request is received we check it's valid and
apply it to the ISP V4L2 device. We also forward it to the IPA so that
the IPA can return the values used in the image metadata.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/ipa/raspberrypi.h           |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp           |  7 +++
 .../pipeline/raspberrypi/raspberrypi.cpp      | 47 +++++++++++++++++++
 3 files changed, 55 insertions(+)

Hi David,

On 29/09/2020 17:39, David Plowman wrote:
> During configure() we update the SensorOutputSize to the correct value
> for this camera mode (which we also note internally) and work out the
> minimum crop size allowed by the ISP.
> 
> Whenever a new IspCrop request is received we check it's valid and
> apply it to the ISP V4L2 device. We also forward it to the IPA so that
> the IPA can return the values used in the image metadata.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/ipa/raspberrypi.h           |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           |  7 +++
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 47 +++++++++++++++++++
>  3 files changed, 55 insertions(+)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index b3041591..9ef5f6fc 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -60,6 +60,7 @@ static const ControlInfoMap Controls = {
>  	{ &controls::Saturation, ControlInfo(0.0f, 32.0f) },
>  	{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>  	{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> +	{ &controls::IspCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>  };
>  
>  } /* namespace RPi */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index b0c7d1c1..dc73c20b 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -699,6 +699,13 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			break;
>  		}
>  
> +		case controls::ISP_CROP: {
> +			/* Just copy the information back. */
> +			Rectangle crop = ctrl.second.get<Rectangle>();
> +			libcameraMetadata_.set(controls::IspCrop, crop);
> +			break;
> +		}
> +
>  		default:
>  			LOG(IPARPI, Warning)
>  				<< "Ctrl " << controls::controls.at(ctrl.first)->name()
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index d2bee150..4a6d0bf4 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -193,6 +193,11 @@ public:
>  	bool flipsAlterBayerOrder_;
>  	BayerFormat::Order nativeBayerOrder_;
>  
> +	/* For handling digital zoom. */
> +	Size ispMinSize_;
> +	Size sensorOutputSize_;
> +	Rectangle lastIspCrop_;
> +
>  	unsigned int dropFrameCount_;
>  
>  private:
> @@ -587,6 +592,14 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  	 */
>  	ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);
>  
> +	/*
> +	 * Update the SensorOutputSize to the correct value for this camera mode.
> +	 *
> +	 * \todo Move this property to CameraConfiguration when that
> +	 * feature will be made available and set it at validate() time
> +	 */
> +	data->properties_.set(properties::SensorOutputSize, sensorFormat.size);
> +
>  	/*
>  	 * See which streams are requested, and route the user
>  	 * StreamConfiguration appropriately.
> @@ -677,6 +690,16 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  		return ret;
>  	}
>  
> +	/*
> +	 * Figure out the smallest selection the ISP will allow. We also store
> +	 * the output image size, in pixels, from the sensor. These will be
> +	 * used for digital zoom.
> +	 */
> +	Rectangle testCrop(0, 0, 1, 1);
> +	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);
> +	data->ispMinSize_ = testCrop.size();

I might have called testCrop minCrop, but I don't think that matters
much. testCrop is ok too.

Part of me is itching to say this should happen at camera creation time
or such, as I don't think it would change on any configure parameters?

But arguing against myself - that would then involve changing settings
on the device which we should avoid, and as this is used on the
per-frame controls, I think this is actually probably OK here. It's not
a big overhead for configure()


> +	data->sensorOutputSize_ = sensorFormat.size;
> +
>  	/* Adjust aspect ratio by providing crops on the input image. */
>  	Rectangle crop{ 0, 0, sensorFormat.size };
>  
> @@ -693,6 +716,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  	crop.y = (sensorFormat.size.height - crop.height) >> 1;
>  	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
>  
> +	data->lastIspCrop_ = crop;
> +
>  	ret = data->configureIPA(config);
>  	if (ret)
>  		LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
> @@ -1588,6 +1613,28 @@ void RPiCameraData::tryRunPipeline()
>  	/* Take the first request from the queue and action the IPA. */
>  	Request *request = requestQueue_.front();
>  
> +	if (request->controls().contains(controls::IspCrop)) {
> +		Rectangle crop = request->controls().get<Rectangle>(controls::IspCrop);
> +		if (crop.width && crop.height) {
> +			/*
> +			 * The crop that we set must be:
> +			 * 1. At least as big as ispMinSize_, once that's been
> +			 *    enlarged to the same aspect ratio.
> +			 * 2. With the same mid-point, if possible.
> +			 * 3. But it can't go outside the sensor area.
> +			 */
> +			Size minSize = ispMinSize_.alignedUpToAspectRatio(crop.size());

I like how the helpers make this expressive, over the underlying
calculations.

> +			Size size = crop.size().expandedTo(minSize);
> +			crop = size.centredTo(crop).boundedTo(Rectangle(sensorOutputSize_));
> +
> +			if (crop != lastIspCrop_)
> +				isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
> +			lastIspCrop_ = crop;
> +		}
> +
> +		request->controls().set(controls::IspCrop, lastIspCrop_);
> +	}
> +

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  	/*
>  	 * Process all the user controls by the IPA. Once this is complete, we
>  	 * queue the ISP output buffer listed in the request to start the HW
>