[libcamera-devel] [PATCH 2/8] libcamera: request: Add Request::cancel()

[Hirokazu Honda]

Hi Jacopo, thank you for the patch.

On Tue, May 11, 2021 at 5:25 AM Niklas Söderlund <
niklas.soderlund at ragnatech.se> wrote:

> Hi Jacopo,
>
> Thanks for your work.
>
> On 2021-05-10 12:52:29 +0200, Jacopo Mondi wrote:
> > Add a cancel() function to the Request class that allows to forcefully
> > complete the request and its associated buffers in error state.
> >
> > Only pending requests can be forcefully cancelled. Enforce that
> > by asserting the request state to be RequestPending.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  include/libcamera/request.h |  1 +
> >  src/libcamera/request.cpp   | 30 ++++++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 4cf5ff3f7d3b..5596901ddd8e 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -65,6 +65,7 @@ private:
> >       friend class PipelineHandler;
> >
> >       void complete();
> > +     void cancel();
> >
> >       bool completeBuffer(FrameBuffer *buffer);
> >
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index ce2dd7b17f10..fc5e25199112 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -292,6 +292,36 @@ void Request::complete()
> >       LIBCAMERA_TRACEPOINT(request_complete, this);
> >  }
> >
> > +/**
> > + * \brief Cancel a queued request
> > + *
> > + * Mark the request and its associated buffers as cancelled and
> complete it.
> > + *
> > + * Set the status of each buffer in the request to the frame cancelled
> state and
> > + * remove them from the pending buffer queue before completing the
> request with
> > + * error.
> > + */
> > +void Request::cancel()
> > +{
> > +     LIBCAMERA_TRACEPOINT(request_cancel, this);
> > +
> > +     ASSERT(status_ == RequestPending);
> > +
> > +     /*
> > +      * We can't simply loop and call completeBuffer() as erase()
> invalidates
> > +      * pointers and iterators, so we have to manually cancel the
> buffer and
> > +      * erase it from the pending buffers list.
> > +      */
> > +     for (auto buffer = pending_.begin(); buffer != pending_.end();) {
> > +             (*buffer)->cancel();
> > +             (*buffer)->setRequest(nullptr);
>

Shall we do (*buffer)->metadata().status = FrameMetadata::FrameCancelled?


> > +             buffer = pending_.erase(buffer);
> > +     }
>
> I wonder how this works if a buffer have been queued to hardware but not
> yet completed? Do we need a new Request status RequestProcessing(?) to
> deal with such a corner case or maybe it's not a problem?
>
>
I think Request::cancel() should be called when a request and its buffers
are not in the hardware.

-Hiro


> > +
> > +     cancelled_ = true;
> > +     complete();
> > +}
> > +
> >  /**
> >   * \brief Complete a buffer for the request
> >   * \param[in] buffer The buffer that has completed
> > --
> > 2.31.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210511/048879d0/attachment.htm>