[libcamera-devel] [RFC PATCH 03/12] libcamera: request: Add PFCError flag

[Umang Jain]

Hi Kieran,

On 7/21/22 17:43, Kieran Bingham via libcamera-devel wrote:
> Provide a new flag for the Request error state to indicate that a
> per-frame control error has occurred. One or more controls set for this
> request could not be guaranteed on this frame.

I would add a line here explaining the cases in which this flag is set 
and by which component.

It would help us to be explicit enough and correspond to a cohesive 
design decisions we make(or made).

>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>   include/libcamera/request.h | 1 +
>   src/libcamera/request.cpp   | 5 +++++
>   2 files changed, 6 insertions(+)
>
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 992629e11aa4..d6d9f0d214dc 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -47,6 +47,7 @@ public:
>   	enum ErrorFlag {
>   		NoError = 0,
>   		ControlError = (1 << 0),
> +		PFCError = (1 << 1),
>   	};
>   
>   	using ErrorFlags = Flags<ErrorFlag>;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 8b82757ea7e3..509ed4ca6ed5 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -339,10 +339,15 @@ void Request::Private::timeout()
>    * 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.
>    * The application should compare the metadata to the requested control values
>    * to check which controls weren't applied.
> + *
> + * \var Request::PFCError
> + * A per-frame-control error has occured. Controls that were expected to be set
> + * during the processing of this request were not processed in time.


Should we have any suggestion for the application here? I am stating 
because I see ::ControlError has a follow up suggestion for 
application's side.

Patch as such look fine to me.

>    */
>   
>   /**