[libcamera-devel] [PATCH 5/7] libcamera: ipu3: Always report crop region

[Niklas Söderlund]

HI Jacopo,

Thanks for your work.

On 2021-02-03 17:25:58 +0100, Jacopo Mondi wrote:
> Report the crop region for every completed request.
> 
> The crop region is initialized as the sensor's analogue crop
> rectangle and updated when a Request with a new region completes.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

For my understanding. Before this patch the controls::ScalerCrop is 
currently only carried as is from the Request controls to the Request 
metadata. And after this change the rectangle is cached and reported in 
every Request's metadata while being updated if set in a Requests 
controls?

And before and after this patch this is just "informative" as the IPU3 
pipeline does not yet react and reflects the setting to the hardware?

I think this patch is a good step in the right direction, the question 
above is just to understand the bigger picture.

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index db0d6b91be70..a9c8e180d1c4 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -65,6 +65,7 @@ public:
>  	Stream rawStream_;
>  
>  	uint32_t exposureTime_;
> +	Rectangle cropRegion;
>  };
>  
>  class IPU3CameraConfiguration : public CameraConfiguration
> @@ -472,6 +473,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret)
>  		return ret;
>  
> +	CameraSensorInfo sensorInfo;
> +	cio2->sensor()->sensorInfo(&sensorInfo);
> +	data->cropRegion = sensorInfo.analogCrop;
> +
>  	/*
>  	 * If the ImgU gets configured, its driver seems to expect that
>  	 * buffers will be queued to its outputs, as otherwise the next
> @@ -967,10 +972,10 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  	/* \todo Move the ExposureTime control to the IPA. */
>  	request->metadata().set(controls::ExposureTime, exposureTime_);
>  	/* \todo Actually apply the scaler crop region to the ImgU. */
> -	if (request->controls().contains(controls::ScalerCrop)) {
> -		Rectangle cropRegion = request->controls().get(controls::ScalerCrop);
> -		request->metadata().set(controls::ScalerCrop, cropRegion);
> -	}
> +	if (request->controls().contains(controls::ScalerCrop))
> +		cropRegion = request->controls().get(controls::ScalerCrop);
> +	request->metadata().set(controls::ScalerCrop, cropRegion);
> +
>  	pipe_->completeRequest(request);
>  }
>  
> -- 
> 2.30.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund