[PATCH v2 7/7] pipeline: rpi: Handler controls::rpi::ScalerCrops
Naushir Patuck
naush at raspberrypi.com
Tue Oct 1 13:20:17 CEST 2024
Hi Jacopo,
On Tue, 1 Oct 2024 at 08:37, Jacopo Mondi <jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Naush
>
> On Mon, Sep 30, 2024 at 03:14:15PM GMT, Naushir Patuck wrote:
> > Handle multiple scaler crops being set through the rpi::ScalerCrops
> > control. We now populate the cropParams_ map in the loop where we handle
> > the output stream configuration items. The key of this map is the index
> > of the stream configuration structure set by the application. This will
> > also be the same index used to specify the crop rectangles through the
> > ScalerCrops control.
> >
> > CameraData::applyScalerCrop() has been adapted to look at either
> > controls::ScalerCrop or controls::rpi::ScalerCrops. The former takes
> > priority over the latter, and if present, will apply the same scaler
> > crop to all output streams.
> >
> > Finally return all crops through the same ScalerCrops control via
> > request metadata. The first configure stream's crop rectangle is also
> > returned via the ScalerCrop control in the request metadata.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > .../pipeline/rpi/common/pipeline_base.cpp | 76 ++++++++++++++-----
> > 1 file changed, 59 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 267e6bd9cd70..9393e79ae4c7 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -181,12 +181,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >
> > rawStreams_.clear();
> > outStreams_.clear();
> > + unsigned int rawStreamIndex = 0;
> > + unsigned int outStreamIndex = 0;
> >
> > - for (const auto &[index, cfg] : utils::enumerate(config_)) {
> > + for (auto &cfg : config_) {
> > if (PipelineHandlerBase::isRaw(cfg.pixelFormat))
> > - rawStreams_.emplace_back(index, &cfg);
> > + rawStreams_.emplace_back(rawStreamIndex++, &cfg);
> > else
> > - outStreams_.emplace_back(index, &cfg);
> > + outStreams_.emplace_back(outStreamIndex++, &cfg);
> > }
> >
> > /* Sort the streams so the highest resolution is first. */
> > @@ -565,10 +567,24 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
> > const auto cropParamsIt = data->cropParams_.find(0);
> > if (cropParamsIt != data->cropParams_.end()) {
> > const CameraData::CropParams &cropParams = cropParamsIt->second;
> > - /* Add the ScalerCrop control limits based on the current mode. */
> > + /*
> > + * Add the ScalerCrop control limits based on the current mode and
> > + * the first configured stream.
> > + */
> > Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));
> > ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,
> > data->scaleIspCrop(cropParams.ispCrop));
> > + if (data->cropParams_.size() == 2) {
> > + /*
> > + * The control map for rpi::ScalerCrops has the min value
> > + * as the default crop for stream 0, max value as the default
> > + * value for stream 1.
> > + */
> > + ctrlMap[&controls::rpi::ScalerCrops] =
> > + ControlInfo(data->scaleIspCrop(data->cropParams_[0].ispCrop),
> > + data->scaleIspCrop(data->cropParams_[1].ispCrop),
> > + ctrlMap[&controls::ScalerCrop].def());
> > + }
> > }
> >
> > data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
> > @@ -1295,11 +1311,29 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const
> >
> > void CameraData::applyScalerCrop(const ControlList &controls)
> > {
> > - const auto &scalerCrop = controls.get<Rectangle>(controls::ScalerCrop);
> > - const auto cropParamsIt = cropParams_.find(0);
> > - if (scalerCrop && cropParamsIt != cropParams_.end()) {
> > - Rectangle nativeCrop = *scalerCrop;
> > - CropParams &cropParams = cropParamsIt->second;
> > + const auto &scalerCropRPi = controls.get<Span<const Rectangle>>(controls::rpi::ScalerCrops);
> > + const auto &scalerCropCore = controls.get<Rectangle>(controls::ScalerCrop);
> > + std::vector<Rectangle> scalerCrops;
> > +
> > + /*
> > + * First thing to do is create a vector of crops to apply to each ISP output
> > + * based on either controls::ScalerCrop or controls::rpi::ScalerCrops if
> > + * present.
> > + *
> > + * If controls::ScalerCrop is present, apply the same crop to all ISP output
> > + * streams. Otherwise if controls::rpi::ScalerCrops, apply the given crops
> > + * to the ISP output streams, indexed by the same order in which they had
> > + * been configured. This is not the same as the ISP output index.
> > + */
> > + for (unsigned int i = 0; i < cropParams_.size(); i++) {
> > + if (scalerCropRPi && i < scalerCropRPi->size())
> > + scalerCrops.push_back(scalerCropRPi->data()[i]);
> > + else if (scalerCropCore)
> > + scalerCrops.push_back(*scalerCropCore);
> > + }
>
> I think you want to be stricter here and better defines what happens
> if
> 1) ScalerCrops has less entries than the number of configured streams
>
> Your ScalerCrops documentation reports
>
> The order of rectangles passed into the control must match the order of
> streams configured by the application. The pipeline handler will only
> configure crop retangles up-to the number of output streams configured.
> All subsequent rectangles passed into this control are ignored by the
> pipeline handler.
>
> which addresses what happens if you have more ScalerCrops entries than
> Streams but not the other way around.
>
> 2) Both ScalerCrop and ScalerCrops have been set by the application
>
> The above code doesn't really handle the two above conditions if I got
> it right.
In the case of (2), we ignore ScalerCrop and use rpi::ScalerCrops. I
will explicitly mention this in the control documentation.
>
> Also, but that's up to you to decide, on vc4 you always crop at the
> ISP input, while on pisp you always crop on the outputs both for
> ScalerCrop and ScalerCrops. Not sure if you need (or can) differentiate,
> in example, ScalerCrop goes to the ISP input crop and ScalerCrops goes
> to the ISP outputs. Up to you!
I think there might be some confusion. On both platforms, the crop is
specified on the full resolution input coordinate space - and this is
the same for both ScalerCrop and rpi::ScalerCrops controls. So they
are pretty much interchanable if we only want a single crop. Where
the crop happens in the ISP HW pipeline is mostly irrelevant right?
Regards,
Naush
>
> > +
> > + for (auto const &[i, scalerCrop] : utils::enumerate(scalerCrops)) {
> > + Rectangle nativeCrop = scalerCrop;
> >
> > if (!nativeCrop.width || !nativeCrop.height)
> > nativeCrop = { 0, 0, 1, 1 };
> > @@ -1315,13 +1349,13 @@ void CameraData::applyScalerCrop(const ControlList &controls)
> > * 2. With the same mid-point, if possible.
> > * 3. But it can't go outside the sensor area.
> > */
> > - Size minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());
> > + Size minSize = cropParams_[i].ispMinCropSize.expandedToAspectRatio(nativeCrop.size());
> > Size size = ispCrop.size().expandedTo(minSize);
> > ispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));
> >
> > - if (ispCrop != cropParams.ispCrop) {
> > - cropParams.ispCrop = ispCrop;
> > - platformSetIspCrop(cropParams.ispIndex, ispCrop);
> > + if (ispCrop != cropParams_[i].ispCrop) {
> > + cropParams_[i].ispCrop = ispCrop;
> > + platformSetIspCrop(cropParams_[i].ispIndex, ispCrop);
> > }
> > }> }
> > @@ -1478,10 +1512,18 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
> > request->metadata().set(controls::SensorTimestamp,
> > bufferControls.get(controls::SensorTimestamp).value_or(0));
> >
> > - const auto cropParamsIt = cropParams_.find(0);
> > - if (cropParamsIt != cropParams_.end())
> > - request->metadata().set(controls::ScalerCrop,
> > - scaleIspCrop(cropParamsIt->second.ispCrop));
> > + if (cropParams_.size()) {
> > + std::vector<Rectangle> crops;
> > +
> > + for (auto const &[k, v] : cropParams_)
> > + crops.push_back(scaleIspCrop(v.ispCrop));
> > +
> > + request->metadata().set(controls::ScalerCrop, crops[0]);
> > + if (crops.size() > 1) {
> > + request->metadata().set(controls::rpi::ScalerCrops,
> > + Span<const Rectangle>(crops.data(), crops.size()));
> > + }
> > + }
> > }
> >
> > } /* namespace libcamera */
> > --
> > 2.34.1
> >
More information about the libcamera-devel
mailing list