[libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add support for error flags
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jul 25 14:38:44 CEST 2022
Quoting Umang Jain (2022-07-22 16:15:05)
> 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/
Ack.
>
> > + *
> > + * 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?
Ack.
>
> > + * 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.
Ack, but I think Jacopo has comments here too.
>
> > + *
> > + * \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
> > *
More information about the libcamera-devel
mailing list