[libcamera-devel] [PATCH v1 4/8] libcamera: camera: Validate MandatoryStream in queueRequest()

[Jacopo Mondi]

Hi Naush

On Fri, Feb 03, 2023 at 09:44:20AM +0000, Naushir Patuck via libcamera-devel wrote:
> Add some validation in queueRequest() to ensure that a frame buffer is
> provided in a Request if the MandatoryStream flag has been set in the
> StreamConfiguration for every active stream.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/camera.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 0da167a7bc51..c0a368c1e66c 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -1146,6 +1146,18 @@ int Camera::queueRequest(Request *request)
>  		}
>  	}
>
> +	for (auto const stream : d->activeStreams_) {
> +		const StreamConfiguration &config = stream->configuration();
> +		FrameBuffer *buffer = request->findBuffer(stream);
> +
> +		if (!buffer && config.hints & StreamConfiguration::Hint::MandatoryStream) {
> +			LOG(Camera, Error)
> +				<< "No buffer provided for mandatory stream";
> +			request->_d()->reset();
> +			return -ENOMEM;

ENOMEM is documented as
 * \retval -ENOMEM No buffer memory was available to handle the request

I would rather use EINVAL
 * \retval -EINVAL The request is invalid

We don't document the expectations on the Request being reset by the
Camera if it fails. Why do you need to do it here (not saying it's
wrong, just wondering if we should document that)


> +		}
> +	}
> +
>  	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
>  			       ConnectionTypeQueued, request);
>
> --
> 2.25.1
>