[libcamera-devel] [PATCH v7 6/8] libcamera: camera: Validate Request before queueing it

[Jacopo Mondi]

On Wed, Apr 17, 2019 at 03:58:56PM +0200, Jacopo Mondi wrote:
> Extend the Request::prepare() operation to validate the request before
> preparing it. Return an error if the request is invalid, which for now
> is limited to ensuring that the request contains at least one buffer.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera.cpp  | 11 ++++++++---
>  src/libcamera/request.cpp | 12 +++++++++++-
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 2d0a80664214..75a21008070b 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -713,9 +713,14 @@ Request *Camera::createRequest()
>   * \brief Queue a request to the camera
>   * \param[in] request The request to queue to the camera
>   *
> - * This method queues a \a request allocated with createRequest() to the camera
> - * for capture. Once the request has been queued, the camera will notify its
> - * completion through the \ref requestCompleted signal.
> + * This method queues a \a request to the camera for capture.
> + *
> + * After allocating the request with createRequest(), the application shall
> + * fill it with at least one capture buffer before queuing it. Requests that
> + * contain no buffers are invalid and are rejected without being queued.
> + *
> + * Once the request has been queued, the camera will notify its completion
> + * through the \ref requestCompleted signal.
>   *
>   * Ownership of the request is transferred to the camera. It will be deleted
>   * automatically after it completes.
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index e5c25d2c6988..9fc8c6845578 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -115,10 +115,20 @@ Buffer *Request::findBuffer(Stream *stream) const
>   */
>
>  /**
> - * \brief Prepare the resources for the completion handler
> + * \brief Validate the request and prepare it for the completion handler
> + *
> + * Requests that contain no buffers are invalid and are rejected.
> + *
> + * \return 0 on success or a negative error code otherwise
> + * \retval -EINVAL The request is invalid
>   */
>  int Request::prepare()
>  {
> +	if (!hasPendingBuffers()) {
> +		LOG(Request, Error) << "Invalid request due to missing buffers";
> +		return -EINVAL;
> +	}
> +

Oh, this is so stupid!
hasPendingBuffers() checks for pending_ to be empty, and pending_ is
here below populated :(

Sorry about this, I moved it up to skip the below loop if no buffers
were available, not a great idea it seems!

>  	for (auto const &pair : bufferMap_) {
>  		Buffer *buffer = pair.second;
>  		pending_.insert(buffer);
> --
> 2.21.0
>
-------------- 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/20190417/713c37a3/attachment.sig>