[libcamera-devel] [PATCH v2 07/12] libcamera: pipeline: raspberrypi: Don't call handleState() in stop state

[Jacopo Mondi]

Hi Laurent,

On Sat, Jul 04, 2020 at 03:52:22AM +0300, Laurent Pinchart wrote:
> The handleState() function is a no-op when the state is State::Stopped.
> Don't call it in that case, which simplifies the caller.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index a05fc68b98fa..74bee6895dcd 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1188,7 +1188,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
>  		ControlList controls = action.controls[0];
>  		if (!staggeredCtrl_.set(controls))
>  			LOG(RPI, Error) << "V4L2 staggered set failed";
> -		goto done;
> +		return;
>  	}
>
>  	case RPI_IPA_ACTION_SET_SENSOR_CONFIG: {
> @@ -1206,18 +1206,18 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
>  		/* Set the sensor orientation here as well. */
>  		ControlList controls = action.controls[0];
>  		unicam_[Unicam::Image].dev()->setControls(&controls);
> -		goto done;
> +		return;
>  	}
>
>  	case RPI_IPA_ACTION_V4L2_SET_ISP: {
>  		ControlList controls = action.controls[0];
>  		isp_[Isp::Input].dev()->setControls(&controls);
> -		goto done;
> +		return;

I'm sure I don't fully understand the IPA protocol for this device,
but the comment says this operation *can* be handled with the IPA in
stopped state, not that the stop state is required. Furthermore, the
next switch block says the there handled operation *must not* be handled
in stopped state, which makes me think these ones can but don't
require to.


>  	}
>  	}
>
>  	if (state_ == State::Stopped)
> -		goto done;
> +		return;

This, instead, is crystal clear :)

>
>  	/*
>  	 * The following actions must not be handled when the pipeline handler
> @@ -1261,7 +1261,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
>  		break;
>  	}
>
> -done:
>  	handleState();
>  }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel