[libcamera-devel] [PATCH 5/6] libcamera: pipeline: raspberrypi: Implementation of digital zoom
Jacopo Mondi
jacopo at jmondi.org
Wed Sep 23 11:51:34 CEST 2020
Hi David,
On Tue, Sep 22, 2020 at 11:03:59AM +0100, 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 at raspberrypi.com>
> ---
> 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 dd6ebea..91a1892 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -58,6 +58,7 @@ static const ControlInfoMap RPiControls = {
> { &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(0, 0, 0, 0), Rectangle(65535, 65535, 65535, 65535), Rectangle(0, 0, 0, 0)) },
If you feel it's more convenient, you can
s/Rectangle(0, 0, 0, 0)/{}
whatever you like the most
> };
>
> } /* namespace libcamera */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 0555cc4..64f0e35 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -687,6 +687,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;
> + }
> +
I wonder if this needs to go to the IPA at all.. I like the symmetry with
other controls but the IPA is actually not involved at all in the process...
Do you expect this to change ? Otherwise the metadata can be updated here
with the value of lastIspCrop_ ?
> 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 50f0718..1674f8f 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -182,6 +182,11 @@ public:
> std::queue<FrameBuffer *> embeddedQueue_;
> std::deque<Request *> requestQueue_;
>
> + /* For handling digital zoom. */
> + Size ispMinSize_;
> + Size sensorOutputSize_;
> + Rectangle lastIspCrop_;
> +
> unsigned int dropFrameCount_;
>
> private:
> @@ -496,6 +501,14 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> LOG(RPI, Info) << "Sensor: " << camera->id()
> << " - Selected mode: " << sensorFormat.toString();
>
> + /*
> + * 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);
The next instruction is to apply 'sensorFormat' to the ISP input node.
I don't know if this operation is expected to change the format, but I
would however update the property value after that.
> +
> /*
> * This format may be reset on start() if the bayer order has changed
> * because of flips in the sensor.
> @@ -592,6 +605,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();
> + data->sensorOutputSize_ = sensorFormat.size;
> +
> /* Adjust aspect ratio by providing crops on the input image. */
> Rectangle crop{ 0, 0, sensorFormat.size };
>
> @@ -608,6 +631,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();
> if (ret)
> LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
> @@ -1449,6 +1474,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());
> + Size size = crop.size().expandedTo(minSize);
> + crop = size.centredTo(crop).boundedTo(Rectangle(sensorOutputSize_));
I might again be missing something, but isn't this equal to:
crop.size().expandTo(minSize).boundTo({sensorOutputSize_});
I'm missing the need to centering, 'crop' has already it's own offsets
set, so we just need to make sure it's larger that the minSize and
smaller than the sensor output size.
General question: how do we expect applications to reset crop ? By
setting it to the sensorOutputSize (or at any larger value). Do we
want to predispose a special case, like a {0, 0, 0, 0} crop rectangle
resets the crop ?
> +
> + if (crop != lastIspCrop_)
> + isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
> + lastIspCrop_ = crop;
> + }
> +
> + request->controls().set(controls::IspCrop, lastIspCrop_);
> + }
> +
Very nice, I think as soon we can test this easily with qcam we'll see
interesting results!
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> /*
> * 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
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list