[libcamera-devel] [PATCH 08/10] libcamera: pipelines: Align where to call base class queueRequest()

[Jacopo Mondi]

Hi Niklas,
    thanks for the patch

On Mon, Oct 28, 2019 at 03:22:22AM +0100, Niklas Söderlund wrote:
> The PipelineHandler base class queueRequest() where called at different
> points in different pipeline handlers. Align where they are called to
> make it easier to read different pipeline implementations.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 5 +++--
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +---
>  src/libcamera/pipeline/uvcvideo.cpp      | 4 +---
>  src/libcamera/pipeline/vimc.cpp          | 4 +---
>  4 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8d3ad568d16e9f8f..06ee6ccac641eb41 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -768,9 +768,10 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>  			error = ret;
>  	}
>
> -	PipelineHandler::queueRequest(camera, request);
> +	if (error)
> +		return error;

I wonder if, since we are now queueing the request if no buffer
queuing fails, isn't it better to return immediately as soon as

		int ret = stream->device_->dev->queueBuffer(buffer);

fails, in the for() loop ?


>
> -	return error;
> +	return PipelineHandler::queueRequest(camera, request);

Here and in all below parts: PipelineHandler::queueRequest() returns 0
unconditionally. As of now this patch does not change anything, but
maybe in future we'll made queueRequest() return a real value, so it
might be good to have it like this already ?

Thanks
   j

>  }
>
>  bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 7a28b03b8d38c8b9..a99df4f8b846d8b7 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -814,8 +814,6 @@ int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request)
>  	RkISP1CameraData *data = cameraData(camera);
>  	Stream *stream = &data->stream_;
>
> -	PipelineHandler::queueRequest(camera, request);
> -
>  	RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request,
>  							stream);
>  	if (!info)
> @@ -833,7 +831,7 @@ int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request)
>
>  	data->frame_++;
>
> -	return 0;
> +	return PipelineHandler::queueRequest(camera, request);
>  }
>
>  /* -----------------------------------------------------------------------------
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index fae0ffc4de30e1b7..dc17b456af6987e6 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -281,9 +281,7 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
>  	if (ret < 0)
>  		return ret;
>
> -	PipelineHandler::queueRequest(camera, request);
> -
> -	return 0;
> +	return PipelineHandler::queueRequest(camera, request);
>  }
>
>  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index c16ae4cb76b57e1c..5f83ae2b85997828 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -341,9 +341,7 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
>  	if (ret < 0)
>  		return ret;
>
> -	PipelineHandler::queueRequest(camera, request);
> -
> -	return 0;
> +	return PipelineHandler::queueRequest(camera, request);
>  }
>
>  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191106/93aa9fb3/attachment.sig>