| Message ID | 20240930141415.8857-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Mon, Sep 30, 2024 at 03:14:10PM GMT, Naushir Patuck wrote: > Do not cache the scalerCrop_ parameter. The cached value is used to > update the request metadata, but since this is not an expensive > operation (and can only occur once per frame), caching it is of limited > value. > > This will simplify logic in a future commit where we can specify a > crop per-output stream. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > .../pipeline/rpi/common/pipeline_base.cpp | 18 +++--------------- > .../pipeline/rpi/common/pipeline_base.h | 1 - > 2 files changed, 3 insertions(+), 16 deletions(-) > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index 3041fd1ed9fd..11f1bfd4a5da 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -544,12 +544,6 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) > return ret; > } > > - /* > - * Set the scaler crop to the value we are using (scaled to native sensor > - * coordinates). > - */ > - data->scalerCrop_ = data->scaleIspCrop(data->ispCrop_); > - > /* > * Update the ScalerCropMaximum to the correct value for this camera mode. > * For us, it's the same as the "analogue crop". > @@ -569,7 +563,8 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) > > /* Add the ScalerCrop control limits based on the current mode. */ > Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_)); > - ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop, data->scalerCrop_); > + ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop, > + data->scaleIspCrop(data->ispCrop_)); > > data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap()); > > @@ -1320,13 +1315,6 @@ void CameraData::applyScalerCrop(const ControlList &controls) > if (ispCrop != ispCrop_) { > ispCrop_ = ispCrop; > platformSetIspCrop(); > - > - /* > - * 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. > - */ > - scalerCrop_ = scaleIspCrop(ispCrop_); > } > } > } > @@ -1483,7 +1471,7 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request > request->metadata().set(controls::SensorTimestamp, > bufferControls.get(controls::SensorTimestamp).value_or(0)); > > - request->metadata().set(controls::ScalerCrop, scalerCrop_); > + request->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_)); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h > index f9cecf70f179..5161c16e518f 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h > @@ -134,7 +134,6 @@ public: > /* For handling digital zoom. */ > IPACameraSensorInfo sensorInfo_; > Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */ > - Rectangle scalerCrop_; /* crop in sensor native pixels */ > Size ispMinCropSize_; > > unsigned int dropFrameCount_; > -- > 2.34.1 >
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 3041fd1ed9fd..11f1bfd4a5da 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -544,12 +544,6 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) return ret; } - /* - * Set the scaler crop to the value we are using (scaled to native sensor - * coordinates). - */ - data->scalerCrop_ = data->scaleIspCrop(data->ispCrop_); - /* * Update the ScalerCropMaximum to the correct value for this camera mode. * For us, it's the same as the "analogue crop". @@ -569,7 +563,8 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) /* Add the ScalerCrop control limits based on the current mode. */ Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_)); - ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop, data->scalerCrop_); + ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop, + data->scaleIspCrop(data->ispCrop_)); data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap()); @@ -1320,13 +1315,6 @@ void CameraData::applyScalerCrop(const ControlList &controls) if (ispCrop != ispCrop_) { ispCrop_ = ispCrop; platformSetIspCrop(); - - /* - * 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. - */ - scalerCrop_ = scaleIspCrop(ispCrop_); } } } @@ -1483,7 +1471,7 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request request->metadata().set(controls::SensorTimestamp, bufferControls.get(controls::SensorTimestamp).value_or(0)); - request->metadata().set(controls::ScalerCrop, scalerCrop_); + request->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_)); } } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h index f9cecf70f179..5161c16e518f 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -134,7 +134,6 @@ public: /* For handling digital zoom. */ IPACameraSensorInfo sensorInfo_; Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */ - Rectangle scalerCrop_; /* crop in sensor native pixels */ Size ispMinCropSize_; unsigned int dropFrameCount_;