[v2,8/8] pipeline: rkisp1: Add ScalerMaximumCrop property

22075 diff mbox series

The ScalerMaximumCrop property holds the biggest allowed ScalerCrop
value. Add it to the rkisp1.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2:
- Moved one hunk to the correct patch 6/7
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++
 1 file changed, 2 insertions(+)

Hi Stefan

On Mon, Nov 25, 2024 at 04:14:17PM +0100, Stefan Klug wrote:
> The ScalerMaximumCrop property holds the biggest allowed ScalerCrop
> value. Add it to the rkisp1.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---
> Changes in v2:
> - Moved one hunk to the correct patch 6/7
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 0a044b08bc87..993515c258ef 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -24,6 +24,7 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/property_ids.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>  #include <libcamera/transform.h>
> @@ -1247,6 +1248,7 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
>  								     dewarperSensorCrop_);
>
>  		controls[&controls::ScalerCrop] = ControlInfo(min, max, max);
> +		data->properties_.set(properties::ScalerCropMaximum, max);

Ah, here you go :)

Does 'max', computed as

		std::pair<Rectangle, Rectangle> cropLimits =
			dewarper_->inputCropBounds(&data->mainPathStream_);
		Rectangle max = cropLimits.second.transformedBetween(cropLimits.second,
								     dewarperSensorCrop_);

contain the maximum limits of the dewarper, or does it take into
account the current configuration ? As suggested in the
previous patch, would using dewarperSensorCrop_ would give the limits taking
into account the currently applied crop-to-aspect-ratio ?

>  		activeCrop_ = max;
>  	}
>
> --
> 2.43.0
>

Hi Jacopo,

On Mon, Nov 25, 2024 at 08:35:01PM +0100, Jacopo Mondi wrote:
> Hi Stefan
> 
> On Mon, Nov 25, 2024 at 04:14:17PM +0100, Stefan Klug wrote:
> > The ScalerMaximumCrop property holds the biggest allowed ScalerCrop
> > value. Add it to the rkisp1.
> >
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - Moved one hunk to the correct patch 6/7
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 0a044b08bc87..993515c258ef 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -24,6 +24,7 @@
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/formats.h>
> >  #include <libcamera/framebuffer.h>
> > +#include <libcamera/property_ids.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/stream.h>
> >  #include <libcamera/transform.h>
> > @@ -1247,6 +1248,7 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
> >  								     dewarperSensorCrop_);
> >
> >  		controls[&controls::ScalerCrop] = ControlInfo(min, max, max);
> > +		data->properties_.set(properties::ScalerCropMaximum, max);
> 
> Ah, here you go :)
> 
> Does 'max', computed as
> 
> 		std::pair<Rectangle, Rectangle> cropLimits =
> 			dewarper_->inputCropBounds(&data->mainPathStream_);
> 		Rectangle max = cropLimits.second.transformedBetween(cropLimits.second,
> 								     dewarperSensorCrop_);
> 
> contain the maximum limits of the dewarper, or does it take into
> account the current configuration ? As suggested in the
> previous patch, would using dewarperSensorCrop_ would give the limits taking
> into account the currently applied crop-to-aspect-ratio ?

I replaced the max = ... line with scalerMaxCrop_ in v3. This will also
change in the next series.

Cheers,
Stefan

> 
> >  		activeCrop_ = max;
> >  	}
> >
> > --
> > 2.43.0
> >