[libcamera-devel,v1,4/8] libcamera: camera: Validate MandatoryStream in queueRequest()
diff mbox series

Message ID 20230203094424.25243-5-naush@raspberrypi.com
State New
Headers show
Series
  • Stream hints
Related show

Commit Message

Naushir Patuck Feb. 3, 2023, 9:44 a.m. UTC
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@raspberrypi.com>
---
 src/libcamera/camera.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jacopo Mondi March 2, 2023, 10:03 a.m. UTC | #1
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@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
>

Patch
diff mbox series

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;
+		}
+	}
+
 	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
 			       ConnectionTypeQueued, request);