<div dir="ltr"><div dir="ltr"><img class="gmail-ajT" src="https://ssl.gstatic.com/ui/v1/icons/mail/images/cleardot.gif">Hi Kieran,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 21 Mar 2022 at 13:13, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Quoting Naushir Patuck via libcamera-devel (2022-03-21 12:48:28)<br>
> Add a timer to the pipeline handler that get set on start(), and subsequently<br>
> reset on each frame dequeue from the sensor. If the timeout expires, log an<br>
> error message indicating so. This is useful for debugging sensor failures were<br>
> the device just stops streaming frames to Unicam, or the pipeline handler has<br>
> failed to queue buffers to the Unicam device driver.<br>
> <br>
> The timeout is calculated as 2x the maximum frame length possible for a given<br>
> mode.<br>
> <br>
<br>
This is an interesting idea, and I think will prove useful for<br>
identifying stalls in streams.<br>
<br>
Would it make sense to make this a feature of the V4L2VideoDevice? Then<br>
it would (automatically?) cover ISP hangs as well as sensor hangs...<br></blockquote><div><br></div><div>It would be nice to add this to a level above, and allow all pipeline handlers</div><div>to gain this functionality! I can look to do that if we want to go this way.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Perhaps the hard part is in making sure the correct timeout is<br>
determined or set?<br></blockquote><div><br></div><div>Well, I've already done it for the sensor in this patch :-) Adding it for the</div><div>ISP is a bit simpler, as it is (normally) not mode dependent.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> include/libcamera/ipa/raspberrypi.mojom | 1 +<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 7 ++++++<br>
> .../pipeline/raspberrypi/raspberrypi.cpp | 22 +++++++++++++++++++<br>
> 3 files changed, 30 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom<br>
> index acd3cafe6c91..33d2a97ca916 100644<br>
> --- a/include/libcamera/ipa/raspberrypi.mojom<br>
> +++ b/include/libcamera/ipa/raspberrypi.mojom<br>
> @@ -41,6 +41,7 @@ struct IPAConfig {<br>
> struct StartConfig {<br>
> libcamera.ControlList controls;<br>
> int32 dropFrameCount;<br>
> + uint32 sensorTimeoutMs;<br>
> };<br>
> <br>
> interface IPARPiInterface {<br>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> index fd8fecb07f81..983d6e998b4c 100644<br>
> --- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> @@ -281,6 +281,13 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf<br>
> <br>
> startConfig->dropFrameCount = dropFrameCount_;<br>
> <br>
> + /*<br>
> + * Set the pipeline handler's sensor timeout to 2x the maximum possible<br>
> + * frame duration for this mode.<br>
> + */<br>
> + const Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length;<br>
> + startConfig->sensorTimeoutMs = 2 * maxSensorFrameDuration.get<std::milli>();<br>
> +<br>
> firstStart_ = false;<br>
> lastRunTimestamp_ = 0;<br>
> }<br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index c2230199fed7..86d952b52aed 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -15,6 +15,7 @@<br>
> #include <utility><br>
> <br>
> #include <libcamera/base/shared_fd.h><br>
> +#include <libcamera/base/timer.h><br>
> #include <libcamera/base/utils.h><br>
> <br>
> #include <libcamera/camera.h><br>
> @@ -202,6 +203,7 @@ public:<br>
> void setIspControls(const ControlList &controls);<br>
> void setDelayedControls(const ControlList &controls);<br>
> void setSensorControls(ControlList &controls);<br>
> + void sensorTimeout();<br>
> <br>
> /* bufferComplete signal handlers. */<br>
> void unicamBufferDequeue(FrameBuffer *buffer);<br>
> @@ -279,6 +281,10 @@ public:<br>
> */<br>
> std::optional<int32_t> notifyGainsUnity_;<br>
> <br>
> + /* Timer to ensure the sensor is producing frames for the pipeline handler. */<br>
> + Timer sensorTimeout_;<br>
> + std::chrono::milliseconds sensorTimeoutDuration_;<br>
> +<br>
> private:<br>
> void checkRequestCompleted();<br>
> void fillRequestMetadata(const ControlList &bufferControls,<br>
> @@ -1032,6 +1038,11 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
> }<br>
> }<br>
> <br>
> + LOG(RPI, Debug) << "Setting sensor timeout to " << startConfig.sensorTimeoutMs << " ms";<br>
> + data->sensorTimeoutDuration_ = std::chrono::milliseconds(startConfig.sensorTimeoutMs);<br>
> + data->sensorTimeout_.start(data->sensorTimeoutDuration_);<br>
> + data->sensorTimeout_.timeout.connect(data, &RPiCameraData::sensorTimeout);<br>
> +<br>
> return 0;<br>
> }<br>
> <br>
> @@ -1040,6 +1051,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)<br>
> RPiCameraData *data = cameraData(camera);<br>
> <br>
> data->state_ = RPiCameraData::State::Stopped;<br>
> + data->sensorTimeout_.timeout.disconnect();<br>
> + data->sensorTimeout_.stop();<br>
> <br>
> /* Disable SOF event generation. */<br>
> data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(false);<br>
> @@ -1757,6 +1770,12 @@ void RPiCameraData::setSensorControls(ControlList &controls)<br>
> sensor_->setControls(&controls);<br>
> }<br>
> <br>
> +void RPiCameraData::sensorTimeout()<br>
> +{<br>
> + LOG(RPI, Error) << "Sensor has timed out after "<br>
> + << sensorTimeoutDuration_.count() << " ms!";<br>
> +}<br>
> +<br>
> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)<br>
> {<br>
> RPi::Stream *stream = nullptr;<br>
> @@ -1792,6 +1811,9 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)<br>
> */<br>
> ctrl.set(controls::SensorTimestamp, buffer->metadata().timestamp);<br>
> bayerQueue_.push({ buffer, std::move(ctrl) });<br>
> +<br>
> + /* Restart the sensor timer. */<br>
> + sensorTimeout_.start(sensorTimeoutDuration_);<br>
> } else {<br>
> embeddedQueue_.push(buffer);<br>
> }<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>