[libcamera-devel] [PATCH v6 5/6] libcamera: pipeline: raspberrypi: Implementation of digital zoom
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 27 00:06:58 CET 2020
Hi David,
Thank you for the patch.
On Mon, Oct 26, 2020 at 05:19:07PM +0000, David Plowman wrote:
> During configure() we update the ScalerCropMaximum to the correct
> value for this camera mode and work out the minimum crop size allowed
> by the ISP.
>
> Whenever a new ScalerCrop request is received we check it's valid and
> apply it to the ISP V4L2 device. When the IPA returns its metadata to
> us we add the ScalerCrop information, rescaled to sensor native
> pixels.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> include/libcamera/ipa/raspberrypi.h | 1 +
> src/ipa/raspberrypi/raspberrypi.cpp | 5 +
> .../pipeline/raspberrypi/raspberrypi.cpp | 92 +++++++++++++++----
> 3 files changed, 81 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index b23baf2f..ff2faf86 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -62,6 +62,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::ScalerCrop, 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 1c255aa2..f338ff8b 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -699,6 +699,11 @@ void IPARPi::queueRequest(const ControlList &controls)
> break;
> }
>
> + case controls::SCALER_CROP: {
> + /* We do nothing with this, but should avoid the warning below. */
> + 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 763451a8..b9d74a81 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -135,7 +135,7 @@ public:
> RPiCameraData(PipelineHandler *pipe)
> : CameraData(pipe), sensor_(nullptr), state_(State::Stopped),
> supportsFlips_(false), flipsAlterBayerOrder_(false),
> - dropFrameCount_(0), ispOutputCount_(0)
> + updateScalerCrop_(true), dropFrameCount_(0), ispOutputCount_(0)
> {
> }
>
> @@ -193,6 +193,13 @@ public:
> bool flipsAlterBayerOrder_;
> BayerFormat::Order nativeBayerOrder_;
>
> + /* For handling digital zoom. */
> + CameraSensorInfo sensorInfo_;
> + Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> + Rectangle scalerCrop_; /* crop in sensor native pixels */
> + bool updateScalerCrop_;
> + Size ispMinCropSize_;
> +
> unsigned int dropFrameCount_;
>
> private:
> @@ -677,26 +684,31 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> return ret;
> }
>
> - /* Adjust aspect ratio by providing crops on the input image. */
> - Rectangle crop{ 0, 0, sensorFormat.size };
> -
> - int ar = maxSize.height * sensorFormat.size.width - maxSize.width * sensorFormat.size.height;
> - if (ar > 0)
> - crop.width = maxSize.width * sensorFormat.size.height / maxSize.height;
> - else if (ar < 0)
> - crop.height = maxSize.height * sensorFormat.size.width / maxSize.width;
> + /* Figure out the smallest selection the ISP will allow. */
> + Rectangle testCrop(0, 0, 1, 1);
> + data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);
> + data->ispMinCropSize_ = testCrop.size();
>
> - crop.width &= ~1;
> - crop.height &= ~1;
> + /* Adjust aspect ratio by providing crops on the input image. */
> + Size size = sensorFormat.size.boundedToAspectRatio(maxSize);
> + Rectangle crop = size.centeredTo(Rectangle(sensorFormat.size).center());
> + data->ispCrop_ = crop;
>
> - crop.x = (sensorFormat.size.width - crop.width) >> 1;
> - crop.y = (sensorFormat.size.height - crop.height) >> 1;
> data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
>
> ret = data->configureIPA(config);
> if (ret)
> LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
>
> + /*
> + * Update the ScalerCropMaximum to the correct value for this camera mode.
> + * For us, it's the same as the "analogue crop".
> + *
> + * \todo Make this property the ScalerCrop maximum value when dynamic
> + * controls are available and set it at validate() time
> + */
> + data->properties_.set(properties::ScalerCropMaximum, data->sensorInfo_.analogCrop);
> +
> return ret;
> }
>
> @@ -1154,8 +1166,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> ipaConfig.data.push_back(static_cast<unsigned int>(lsTable_.fd()));
> }
>
> - CameraSensorInfo sensorInfo = {};
> - int ret = sensor_->sensorInfo(&sensorInfo);
> + /* We store the CameraSensorInfo for digital zoom calculations. */
> + int ret = sensor_->sensorInfo(&sensorInfo_);
> if (ret) {
> LOG(RPI, Error) << "Failed to retrieve camera sensor info";
> return ret;
> @@ -1164,7 +1176,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> /* Ready the IPA - it must know about the sensor resolution. */
> IPAOperationData result;
>
> - ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
> + ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
> &result);
>
> unsigned int resultIdx = 0;
> @@ -1243,8 +1255,26 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame,
> FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
>
> handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> +
> /* Fill the Request metadata buffer with what the IPA has provided */
> - requestQueue_.front()->metadata() = std::move(action.controls[0]);
> + Request *request = requestQueue_.front();
> + request->metadata() = std::move(action.controls[0]);
> +
> + /*
> + * Also update the ScalerCrop in the metadata with what we actually
> + * used. But we must first rescale that from ISP (camera mode) pixels
> + * back into sensor native pixels.
> + *
> + * Sending this information on every frame may be helpful.
> + */
> + if (updateScalerCrop_) {
> + updateScalerCrop_ = false;
> + scalerCrop_ = ispCrop_.scaledBy(sensorInfo_.analogCrop.size(),
> + sensorInfo_.outputSize);
> + scalerCrop_.translateBy(sensorInfo_.analogCrop.topLeft());
I initially thought this would be done in
RPiCameraData::tryRunPipeline(), but here is fine too.
> + }
> + request->metadata().set(controls::ScalerCrop, scalerCrop_);
> +
> state_ = State::IpaComplete;
> break;
> }
> @@ -1595,6 +1625,34 @@ void RPiCameraData::tryRunPipeline()
> /* Take the first request from the queue and action the IPA. */
> Request *request = requestQueue_.front();
>
> + if (request->controls().contains(controls::ScalerCrop)) {
> + Rectangle crop = request->controls().get<Rectangle>(controls::ScalerCrop);
> +
> + if (crop.width && crop.height) {
This is the only part that I think needs a fix. If the crop rectangle is
invalid, I think it should be set to the closed valid value. How about
if (!crop.width || !crop.height)
crop = { 0, 0, 1, 1 };
and lowering the indentation of the next block of code ? Unless I'm
mistaken this will avoid a crash and produce a valid crop rectangle. We
can also try to make it a bit more fancy to use the same aspect ratio as
the analog crop rectangle, but I think that's overkill, if the crop
rectangle is invalid in the first place, it's the application's fault.
I can fix this when applying if you're fine with the change (it would be
nice if you could test it though).
With this addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + /* First scale the crop from sensor native to camera mode pixels. */
> + crop.translateBy(-sensorInfo_.analogCrop.topLeft());
> + crop.scaleBy(sensorInfo_.outputSize, sensorInfo_.analogCrop.size());
> +
> + /*
> + * The crop that we set must be:
> + * 1. At least as big as ispMinCropSize_, 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 = ispMinCropSize_.expandedToAspectRatio(crop.size());
> + Size size = crop.size().expandedTo(minSize);
> + crop = size.centeredTo(crop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));
> +
> + if (crop != ispCrop_) {
> + isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
> + ispCrop_ = crop;
> + /* queueFrameAction will have to update its scalerCrop_ */
> + updateScalerCrop_ = true;
> + }
> + }
> + }
> +
> /*
> * 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list