[libcamera-devel] [PATCH v2 2/2] libcamera: pipelines: ipu3: Simplify error bail out path on start()

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Tue, Jan 12, 2021 at 10:21:40AM +0530, Umang Jain wrote:
> On the bail out path, always ensure that ImgU and CIO2 are stopped
> before freeing the buffers. V4L2VideoDevice class guarantees that
> calling stop() without having to call start() is harmless, hence use

s/having to call/having called/

> this guarantee to simplify error paths.
> 
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f1151733..73304ea7 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -617,15 +617,14 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
>  		goto error;
>  
>  	ret = imgu->start();
> -	if (ret) {
> -		imgu->stop();
> -		cio2->stop();
> +	if (ret)
>  		goto error;
> -	}
>  
>  	return 0;
>  
>  error:
> +	imgu->stop();
> +	cio2->stop();

cio2->stop() also calls V4L2VideoDevice::requestBuffers(), which calls
VIDIOC_REQBUFS(0). This shouldn't be an issue, but we could possibly
optimize it. It can be done on top if needed.

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

>  	freeBuffers(camera);
>  	LOG(IPU3, Error) << "Failed to start camera " << camera->id();
>  

-- 
Regards,

Laurent Pinchart