| Message ID | 20220916100517.12446-3-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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 ? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > -- > 2.25.1 >
Hi Kieran, Thanks for the feedback. On Fri, 16 Sept 2022 at 14:34, Kieran Bingham < kieran.bingham@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@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@ideasonboard.com> > > > } > > > > void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > -- > > 2.25.1 > > >
Hi Kieran, On Fri, 16 Sept 2022 at 14:34, Kieran Bingham < kieran.bingham@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? > Sorry, I forgot to respond to this question. Right now, when a timeout occurs, we simply throw out a logging message, then the application stalls waiting for a request to complete - which never happens. By cancelling the requests, the application then has the choice of what to do. We will change libcamera-apps to do a full teardown and restart to try and continue the use case (in testing, this seems to work well for ov5647), but other applications might decide to just quit. Regards, Naush > > > > > 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@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 ? > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > } > > > > void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > -- > > 2.25.1 > > >
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 + * incomplete and cancelled. + */ + for (auto const stream : streams_) + stream->dev()->streamOff(); + + clearIncompleteRequests(); } void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
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. 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@raspberrypi.com> --- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+)