[libcamera-devel] [PATCH v1 2/2] pipeline: raspberrypi: Improve Unicam timeout handling

[Naushir Patuck]

Hi Kieran,

Thanks for the feedback.

On Fri, 16 Sept 2022 at 14:34, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Naushir Patuck via libcamera-devel (2022-09-16 11:05:17)
> > Currently, if a Unicam timeout is signalled, the pipeline handler only
> raises
> > an error message. Update the error handling to put the pipeline handler
> in an
> > internal error state, disable all device streams, and return all
> outstanding
> > requests as cancelled. Any subsequent requests that come into the
> pipeline
> > handler will also be returned as cancelled.
>
> Does this mean that currently the application will stop and exit?
>
> >
> > Any further error handling (e.g. a reset with camera stop()/start()) is
> up to
> > the application to perform as it requires.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index d429cb444d58..4464d4d07f15 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1819,6 +1819,17 @@ void RPiCameraData::unicamTimeout()
> >         LOG(RPI, Error) << "Unicam has timed out!";
> >         LOG(RPI, Error) << "Please check that your camera sensor
> connector is attached securely.";
> >         LOG(RPI, Error) << "Alternatively, try another cable and/or
> sensor.";
> > +
> > +       state_ = RPiCameraData::State::Error;
> > +       /*
> > +        * To allow the application to attempt a recovery from this
> timeout,
> > +        * stop all device streaming, and return any outstanding
> requests as
>
> s/device/devices/
>
> > +        * incomplete and cancelled.
> > +        */
> > +       for (auto const stream : streams_)
> > +               stream->dev()->streamOff();
> > +
> > +       clearIncompleteRequests();
>
> Again, I think this is fine for RPi, but I wonder if this is something
> we should also (instead of?/as well as) handle this in the Camera object
> directly ?
>

It might be possible to put this in a layer above - perhaps the base
pipeline handler
class?  But then we might not be able to make it entirely generic for all
pipeline
handlers, unless we have a defined abort() member function that each
pipeline handler
needs to implement? Perhaps this is something for the future :)

Regards,
Naush


>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> >  }
> >
> >  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220916/e05eba7a/attachment.htm>