[libcamera-devel] [PATCH v2] libcamera: pipeline: raspberrypi: Fix scaler crop when sensor is configured
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Mar 3 15:20:25 CET 2022
Hi David,
Quoting David Plowman (2022-03-03 12:11:13)
> We must calculate the initial scaler crop when the camera is
> configured, otherwise the metadata will report this rectangle as being
> all zeroes.
>
> Because the calculation is identical to that performed later in handling
> the scaler crop control, we factor it into a small helper function,
> RPiCameraData::scaleIspCrop.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 20 ++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 29bff9d6..eede78e6 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -212,6 +212,7 @@ public:
> void handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream);
> void handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream);
> void handleState();
> + Rectangle scaleIspCrop(const Rectangle &ispCrop) const;
> void applyScalerCrop(const ControlList &controls);
>
> std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;
> @@ -887,6 +888,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> if (ret)
> LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
>
> + /* Set the scaler crop to the value we are using (scaled to native sensor coordinates). */
> + data->scalerCrop_ = data->scaleIspCrop(data->ispCrop_);
> +
> /*
> * Configure the Unicam embedded data output format only if the sensor
> * supports it.
> @@ -1974,6 +1978,18 @@ void RPiCameraData::checkRequestCompleted()
> }
> }
>
> +Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const
> +{
> + /*
> + * Scale a crop rectangle defined in the ISP's coordinates into native sensor
> + * coordinates.
> + */
> + Rectangle nativeCrop = ispCrop.scaledBy(sensorInfo_.analogCrop.size(),
> + sensorInfo_.outputSize);
> + nativeCrop.translateBy(sensorInfo_.analogCrop.topLeft());
I'm struggling to be sure, but this is what the code was before, so I'm
sure it's fine... should the amount we translate by also be scaled?
Or perhaps that's just to maintain the original cropping point.
> + return nativeCrop;
I know it was only two lines, but I certainly prefer this, as it's now
it's more clear (and more documented, and self-documenting) for what is
being returned.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> +}
> +
> void RPiCameraData::applyScalerCrop(const ControlList &controls)
> {
> if (controls.contains(controls::ScalerCrop)) {
> @@ -2006,9 +2022,7 @@ void RPiCameraData::applyScalerCrop(const ControlList &controls)
> * used. But we must first rescale that from ISP (camera mode) pixels
> * back into sensor native pixels.
> */
> - scalerCrop_ = ispCrop_.scaledBy(sensorInfo_.analogCrop.size(),
> - sensorInfo_.outputSize);
> - scalerCrop_.translateBy(sensorInfo_.analogCrop.topLeft());
> + scalerCrop_ = scaleIspCrop(ispCrop_);
> }
> }
> }
> --
> 2.30.2
>
More information about the libcamera-devel
mailing list