[PATCH 3/7] mali-c55: implement support for ScalerCrop
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 17 07:11:33 CEST 2024
On Thu, Jun 13, 2024 at 08:33:11PM +0100, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-06-13 16:59:45)
> > From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> >
> > Implement support for the ScalerCrop control that allows to apply a
> > digital zoom to the captured streams.
> >
> > Initialize the camera controls at camera registration time and update
> > them at configure time as the sensor's analogue crop size might change
> > depending on the desired Camera configuration.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 138 ++++++++++++++++++-
> > 1 file changed, 137 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > index c4f1afbc..2c34f3e9 100644
> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > @@ -100,6 +100,8 @@ public:
> >
> > PixelFormat bestRawFormat() const;
> >
> > + void updateControls();
> > +
> > PixelFormat adjustRawFormat(const PixelFormat &pixFmt) const;
> > Size adjustRawSizes(const PixelFormat &pixFmt, const Size &rawSize) const;
> >
> > @@ -245,6 +247,27 @@ PixelFormat MaliC55CameraData::bestRawFormat() const
> > return rawFormat;
> > }
> >
> > +void MaliC55CameraData::updateControls()
> > +{
> > + if (!sensor_)
> > + return;
> > +
> > + IPACameraSensorInfo sensorInfo;
> > + int ret = sensor_->sensorInfo(&sensorInfo);
> > + if (ret) {
> > + LOG(MaliC55, Error) << "Failed to retrieve sensor info";
> > + return;
> > + }
> > +
> > + ControlInfoMap::Map controls;
> > + Rectangle ispMinCrop{ 0, 0, 640, 480 };
>
> This looks like it should be a constant somewhere outside of here?
> Perhaps class constant or just a constant at the top of the file.
>
> > + controls[&controls::ScalerCrop] =
> > + ControlInfo(ispMinCrop, sensorInfo.analogCrop,
> > + sensorInfo.analogCrop);
> > +
> > + controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);
> > +}
> > +
> > /*
> > * Make sure the provided raw pixel format is supported and adjust it to
> > * one of the supported ones if it's not.
> > @@ -515,6 +538,8 @@ private:
> > const StreamConfiguration &config,
> > V4L2SubdeviceFormat &subdevFormat);
> >
> > + void applyScalerCrop(Camera *camera, const ControlList &controls);
> > +
> > void registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
> > const std::string &name);
> > bool registerTPGCamera(MediaLink *link);
> > @@ -828,6 +853,8 @@ int PipelineHandlerMaliC55::configure(Camera *camera,
> > pipe->stream = stream;
> > }
> >
> > + data->updateControls();
> > +
> > return 0;
> > }
> >
> > @@ -875,6 +902,104 @@ void PipelineHandlerMaliC55::stopDevice([[maybe_unused]] Camera *camera)
> > }
> > }
> >
> > +void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,
> > + const ControlList &controls)
> > +{
> > + MaliC55CameraData *data = cameraData(camera);
> > +
> > + const auto &scalerCrop = controls.get<Rectangle>(controls::ScalerCrop);
> > + if (!scalerCrop)
> > + return;
> > +
> > + if (!data->sensor_) {
> > + LOG(MaliC55, Error) << "ScalerCrop not supported for TPG";
> > + return;
> > + }
> > +
> > + Rectangle nativeCrop = *scalerCrop;
> > +
> > + IPACameraSensorInfo sensorInfo;
> > + int ret = data->sensor_->sensorInfo(&sensorInfo);
Gathering the sensor info is a bit expensive, it should be cached.
> > + if (ret) {
> > + LOG(MaliC55, Error) << "Failed to retrieve sensor info";
> > + return;
> > + }
> > +
> > + /*
> > + * The ScalerCrop rectangle re-scaling in the ISP crop rectangle
> > + * comes straight from the RPi pipeline handler.
Does this mean we should have a helper somewhere ? And does this comment
belong here, or to the commit message ?
> > + *
> > + * Create a version of the crop rectangle aligned to the analogue crop
> > + * rectangle top-left coordinates and scaled in the [analogue crop to
> > + * output frame] ratio to take into account binning/skipping on the
> > + * sensor.
> > + */
> > + Rectangle ispCrop = nativeCrop.translatedBy(-sensorInfo.analogCrop
> > + .topLeft());
> > + ispCrop.scaleBy(sensorInfo.outputSize, sensorInfo.analogCrop.size());
> > +
> > + /*
> > + * The crop rectangle should be:
> > + * 1. At least as big as ispMinCropSize_, once that's been
> > + * enlarged to the same aspect ratio.
Does this mean the hardware doesn' support sensor resolutions smaller
than 640x480 ?
> > + * 2. With the same mid-point, if possible.
Why so ? Isn't the user in control of that ?
> > + * 3. But it can't go outside the sensor area.
> > + */
> > + Rectangle ispMinCrop{ 0, 0, 640, 480 };
>
> Especially if it's used multiple times.
>
> > + Size minSize = ispMinCrop.size().expandedToAspectRatio(nativeCrop.size());
> > + Size size = ispCrop.size().expandedTo(minSize);
> > + ispCrop = size.centeredTo(ispCrop.center())
> > + .enclosedIn(Rectangle(sensorInfo.outputSize));
> > +
> > + /*
> > + * As the resizer can't upscale, the crop rectangle has to be larger
> > + * than the larger stream output size.
> > + */
> > + Size maxYuvSize;
> > + for (MaliC55Pipe &pipe : pipes_) {
> > +
>
> Probably delete that blank line.
>
> > + if (!pipe.stream)
> > + continue;
> > +
> > + const StreamConfiguration &config = pipe.stream->configuration();
> > + if (isFormatRaw(config.pixelFormat)) {
> > + LOG(MaliC55, Debug) << "Cannot crop with a RAW stream";
> > + return;
> > + }
> > +
> > + Size streamSize = config.size;
> > + if (streamSize.width > maxYuvSize.width)
> > + maxYuvSize.width = streamSize.width;
> > + if (streamSize.height > maxYuvSize.height)
> > + maxYuvSize.height = streamSize.height;
> > + }
> > +
> > + ispCrop.size().expandTo(maxYuvSize);
> > +
> > + /*
> > + * Now apply the scaler crop to each enabled output. This overrides the
> > + * crop configuration performed at configure() time and can cause
> > + * square pixels if the crop rectangle and scaler output FOV ratio are
>
> Can cause 'non square' pixels perhaps?
>
> > + * different.
> > + */
> > + for (MaliC55Pipe &pipe : pipes_) {
> > +
> > + if (!pipe.stream)
> > + continue;
> > +
> > + /* Create a copy to avoid setSelection() to modify ispCrop. */
>
> 'to prevent setSelection() from modifying ispCrop'
>
> > + Rectangle pipeCrop = ispCrop;
> > + ret = pipe.resizer->setSelection(0, V4L2_SEL_TGT_CROP, &pipeCrop);
> > + if (ret) {
> > + LOG(MaliC55, Error)
> > + << "Failed to apply crop to "
> > + << (pipe.stream == &data->frStream_ ?
> > + "FR" : "DS") << " pipe";
> > + return;
> > + }
> > + }
The effective ScalerCrop value should be reported in metadata.
> > +}
> > +
> > int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)
> > {
> > int ret;
> > @@ -887,6 +1012,15 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)
> > return ret;
> > }
> >
> > + /*
> > + * Some controls need to be applied immediately, as in example,
> > + * the ScalerCrop one.
> > + *
> > + * \todo Move it buffer queue time (likely after the IPA has filled in
> > + * the parameters buffer) once we have plumbed the IPA loop in.
> > + */
> > + applyScalerCrop(camera, request->controls());
> > +
> > return 0;
> > }
> >
> > @@ -965,7 +1099,9 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)
> > if (data->init())
> > return false;
> >
> > - /* \todo: Init properties and controls. */
> > + /* \todo: Init properties. */
> > +
> > + data->updateControls();
> >
> > registerMaliCamera(std::move(data), sensor->name());
> > }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list