[libcamera-devel,v2,08/11] ipa: rkisp1: Report and use sensor controls

14692 diff mbox series

The pipeline handler populates a new sensorControls ControlList, to
have the effective exposure and gain values for the current frame. This
is done when a statistics buffer is received.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/ipa/rkisp1.mojom       | 2 ++
 src/ipa/rkisp1/rkisp1.cpp                | 2 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

Hi Jean-Michel,

Thank you for the patch.

On Tue, Nov 23, 2021 at 10:14:48AM +0100, Jean-Michel Hautbois wrote:
> The pipeline handler populates a new sensorControls ControlList, to
> have the effective exposure and gain values for the current frame. This
> is done when a statistics buffer is received.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

As Kieran mentioned on v1, it would be good to refactor both IPU3 and
RkISP1 to drop actions/events in favour of ad-hoc methods.

> ---
>  include/libcamera/ipa/rkisp1.mojom       | 2 ++
>  src/ipa/rkisp1/rkisp1.cpp                | 2 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index a6991d4f..c3a6d8e1 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -21,11 +21,13 @@ struct RkISP1Event {
>  	uint32 frame;
>  	uint32 bufferId;
>  	libcamera.ControlList controls;
> +	libcamera.ControlList sensorControls;
>  };
>  
>  struct RkISP1Action {
>  	RkISP1Operations op;
>  	libcamera.ControlList controls;
> +	libcamera.ControlList sensorControls;
>  };
>  
>  interface IPARkISP1Interface {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 0c54d8ec..b5aa93f8 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -312,7 +312,7 @@ void IPARkISP1::setControls(unsigned int frame)
>  	ControlList ctrls(ctrls_);
>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> -	op.controls = ctrls;
> +	op.sensorControls = ctrls;
>  
>  	queueFrameAction.emit(frame, op);
>  }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 6cdc4c03..36ef6a02 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -333,7 +333,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  {
>  	switch (action.op) {
>  	case ipa::rkisp1::ActionV4L2Set: {
> -		const ControlList &controls = action.controls;
> +		const ControlList &controls = action.sensorControls;
>  		delayedCtrls_->push(controls);
>  		break;
>  	}
> @@ -1124,6 +1124,7 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  	ev.op = ipa::rkisp1::EventSignalStatBuffer;
>  	ev.frame = info->frame;
>  	ev.bufferId = info->statBuffer->cookie();
> +	ev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);
>  	data->ipa_->processEvent(ev);
>  }
>