[libcamera-devel] [PATCH v2 14/14] libcamera: ipa: Merge controls and v4l2controls in IPAOperationData

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2019-10-12 21:44:07 +0300, Laurent Pinchart wrote:
> Now that the V4L2ControlList is merely a helper to construct a
> ControlList for V4L2 controls, without any data member, all controls can
> be transferred between pipeline handlers and IPAs using ControlList
> only. Remove the v4l2controls member for IPAOperationData and use the
> control member instead.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  include/ipa/ipa_interface.h              | 1 -
>  src/ipa/rkisp1/rkisp1.cpp                | 2 +-
>  src/libcamera/ipa_interface.cpp          | 8 --------
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++---
>  4 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index c393b64f6aa1..dfb1bcbee516 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -33,7 +33,6 @@ struct IPAOperationData {
>  	unsigned int operation;
>  	std::vector<uint32_t> data;
>  	std::vector<ControlList> controls;
> -	std::vector<V4L2ControlList> v4l2controls;
>  };
>  
>  class IPAInterface
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 69ced840585f..13059d99036c 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -215,7 +215,7 @@ void IPARkISP1::setControls(unsigned int frame)
>  	V4L2ControlList ctrls(ctrls_);
>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> -	op.v4l2controls.push_back(ctrls);
> +	op.controls.push_back(ctrls);
>  
>  	queueFrameAction.emit(frame, op);
>  }
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index 4fb178298f4c..0571b8e6bf8b 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -120,14 +120,6 @@ namespace libcamera {
>   * by the IPA protocol.
>   */
>  
> -/**
> - * \var IPAOperationData::v4l2controls
> - * \brief Operation V4L2 controls data
> - *
> - * The interpretation and position of different values in the array are defined
> - * by the IPA protocol.
> - */
> -
>  /**
>   * \class IPAInterface
>   * \brief Interface for IPA implementation
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 32b023730009..9b19bde8a274 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -324,7 +324,7 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)
>  class RkISP1ActionSetSensor : public FrameAction
>  {
>  public:
> -	RkISP1ActionSetSensor(unsigned int frame, CameraSensor *sensor, V4L2ControlList controls)
> +	RkISP1ActionSetSensor(unsigned int frame, CameraSensor *sensor, const ControlList &controls)
>  		: FrameAction(frame, SetSensor), sensor_(sensor), controls_(controls) {}
>  
>  protected:
> @@ -335,7 +335,7 @@ protected:
>  
>  private:
>  	CameraSensor *sensor_;
> -	V4L2ControlList controls_;
> +	ControlList controls_;
>  };
>  
>  class RkISP1ActionQueueBuffers : public FrameAction
> @@ -387,7 +387,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>  {
>  	switch (action.operation) {
>  	case RKISP1_IPA_ACTION_V4L2_SET: {
> -		V4L2ControlList controls = action.v4l2controls[0];
> +		const ControlList &controls = action.controls[0];
>  		timeline_.scheduleAction(utils::make_unique<RkISP1ActionSetSensor>(frame,
>  										   sensor_,
>  										   controls));
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund