[libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add support for error flags

[Umang Jain]

Hi Kieran,

On 7/21/22 17:42, Kieran Bingham via libcamera-devel wrote:
> From: Paul Elder <paul.elder at ideasonboard.com>
>
> Add error flags to the Request to indicate non-fatal errors.
>
> Applications should check the error() state of the Request to determine
> if any fault occured while processing the request.
>
> Initially, a single error flag ControlError is added to allow a pipeline
> handler to report a failure to set controls on a hardware device while
> processing the request.
>
> ControlErrors occur when a Request was asked to set a control and the
> pipeline handler attempted to do so, but it was rejected by the kernel.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> v2:
>   - Add documentation for Request::Private::setErrorFlags
>
> ---
> v2
>   - Extend documentation
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>   include/libcamera/internal/request.h |  3 ++
>   include/libcamera/request.h          |  9 ++++++
>   src/libcamera/request.cpp            | 46 ++++++++++++++++++++++++++++
>   3 files changed, 58 insertions(+)
>
> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
> index 9dadd6c60361..8e592272cfae 100644
> --- a/include/libcamera/internal/request.h
> +++ b/include/libcamera/internal/request.h
> @@ -38,6 +38,7 @@ public:
>   	void complete();
>   	void cancel();
>   	void reuse();
> +	void setErrorFlags(ErrorFlags flags);
>   
>   	void prepare(std::chrono::milliseconds timeout = 0ms);
>   	Signal<> prepared;
> @@ -59,6 +60,8 @@ private:
>   	std::unordered_set<FrameBuffer *> pending_;
>   	std::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;
>   	std::unique_ptr<Timer> timer_;
> +
> +	ErrorFlags error_;
>   };
>   
>   } /* namespace libcamera */
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index dffde1536cad..992629e11aa4 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -15,6 +15,7 @@
>   #include <unordered_set>
>   
>   #include <libcamera/base/class.h>
> +#include <libcamera/base/flags.h>
>   #include <libcamera/base/signal.h>
>   
>   #include <libcamera/controls.h>
> @@ -43,6 +44,13 @@ public:
>   		ReuseBuffers = (1 << 0),
>   	};
>   
> +	enum ErrorFlag {
> +		NoError = 0,
> +		ControlError = (1 << 0),
> +	};
> +
> +	using ErrorFlags = Flags<ErrorFlag>;
> +
>   	using BufferMap = std::map<const Stream *, FrameBuffer *>;
>   
>   	Request(Camera *camera, uint64_t cookie = 0);
> @@ -60,6 +68,7 @@ public:
>   	uint32_t sequence() const;
>   	uint64_t cookie() const { return cookie_; }
>   	Status status() const { return status_; }
> +	ErrorFlags error() const;
>   
>   	bool hasPendingBuffers() const;
>   
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index d2af1d2212ad..8b82757ea7e3 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -162,6 +162,7 @@ void Request::Private::cancel()
>    */
>   void Request::Private::reuse()
>   {
> +	error_ = Request::NoError;
>   	sequence_ = 0;
>   	cancelled_ = false;
>   	prepared_ = false;
> @@ -284,6 +285,21 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)
>   	emitPrepareCompleted();
>   }
>   
> +/**
> + * \brief Update the error flags of the Request
> + * \param[in] flags Flags to apply on the Request
> + *
> + * Apply \a flags to the Request to report to the application when the Request
> + * completes.


s/to report to the application/which will get reported to the application/

> + *
> + * Setting an Error flag does not cause a Request to fail, and once set it can
> + * only be cleared by the application destroying the Request or calling reuse().
> + */
> +void Request::Private::setErrorFlags(ErrorFlags flags)
> +{
> +	error_ |= flags;
> +}
> +
>   void Request::Private::timeout()
>   {
>   	/* A timeout can only happen if there are fences not yet signalled. */
> @@ -318,6 +334,22 @@ void Request::Private::timeout()
>    * Reuse the buffers that were previously added by addBuffer()
>    */
>   
> +/**
> + * \enum Request::ErrorFlag
> + * Flags to report non-fatal errors
> + * \var Request::NoError
> + * No error
> + * \var Request::ControlError
> + * Control Error. At least on control was not able to be applied to the device.


s/on/one/ maybe?

> + * The application should compare the metadata to the requested control values
> + * to check which controls weren't applied.
> + */
> +
> +/**
> + * \typedef Request::ErrorFlags
> + * The error state of the request defined by \a Request::ErrorFlag
> + */
> +
>   /**
>    * \typedef Request::BufferMap
>    * \brief A map of Stream to FrameBuffer pointers
> @@ -560,6 +592,20 @@ uint32_t Request::sequence() const
>    * \return The request completion status
>    */
>   
> +/**
> + * \brief Retrieve the error flags
> + *
> + * The request could complete with non-fatal error. The completion status will
> + * indicate success. This function returns the non-fatal errors that the
> + * request completed with


s/that the request completed with/with which the request was completed.

> + *
> + * \return Flags of non-fatal errors that the request completed with
> + */
> +Request::ErrorFlags Request::error() const
> +{
> +	return _d()->error_;
> +}
> +
>   /**
>    * \brief Check if a request has buffers yet to be completed
>    *