[libcamera-devel] [PATCH v2 2/2] qcam: Fix timers not emitting timeout signal

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Sep 27, 2019 at 10:16:41PM +0200, Niklas Söderlund wrote:
> The timer signal was never emitted in QtEventDispatcher::timerEvent(),
> this results in timers not working as designed running under the Qt
> event loop. Fix this by emitting the signal on timeout and stopping the
> timer. By stopping the timer unregisterTimer() is called which handles
> the cleanup.

That's actually two changes in one patch. Did I hear you requesting
before that patches should be split with one change per patch ? ;-)

The move to timer->stop() isn't just cosmetics, it ensures that the
Timer::deadline_ field gets reset to 0, so it fixes potential bugs. You
should probably mention this in the commit message.

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/qcam/qt_event_dispatcher.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp
> index 98d2472c37856642..d7d1fed290851e6e 100644
> --- a/src/qcam/qt_event_dispatcher.cpp
> +++ b/src/qcam/qt_event_dispatcher.cpp
> @@ -131,9 +131,10 @@ void QtEventDispatcher::unregisterTimer(Timer *timer)
>  void QtEventDispatcher::timerEvent(QTimerEvent *event)
>  {
>  	auto it = timers_.find(event->timerId());
> -	timerIds_.erase(it->second);
> -	killTimer(it->first);
> -	timers_.erase(it);
> +	Timer *timer = it->second;
> +
> +	timer->stop();
> +	timer->timeout.emit(timer);
>  }
>  
>  void QtEventDispatcher::processEvents()

-- 
Regards,

Laurent Pinchart