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

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Sep 27, 2019 at 04:30:23AM +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.

Oops :-)

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/qcam/qt_event_dispatcher.cpp | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp
> index 6b332f6265d01fe0..fcde374c9936d1f9 100644
> --- a/src/qcam/qt_event_dispatcher.cpp
> +++ b/src/qcam/qt_event_dispatcher.cpp
> @@ -132,9 +132,19 @@ void QtEventDispatcher::unregisterTimer(Timer *timer)
>  void QtEventDispatcher::timerEvent(QTimerEvent *event)
>  {
>  	auto it = timers_.find(event->timerId());
> -	timerIds_.erase(it->second);
> +
> +	if (it == timers_.end()) {
> +		std::cout << "Can't find timer for event" << std::endl;
> +		return;
> +	}
> +
> +	Timer *timer = it->second;
> +
> +	timerIds_.erase(timer);
>  	killTimer(it->first);

Maybe

	killTimer(event->timerId());

to minimise usage of it->first and it->second, as they're a bit
confusing ?

>  	timers_.erase(it);
> +

I think you should call timer->stop() too. This will cause
unregisterTimer() to be called, which can handle the remove from the
maps, you don't have to handle it manually here.

	auto it = timers_.find(event->timerId());
	int id = it->first;
	Timer *timer = it->second;

	killTimer(id);
	timer->stop();
	timer->timeout.emit(timer);

> +	timer->timeout.emit(timer);
>  }
>  
>  void QtEventDispatcher::processEvents()

-- 
Regards,

Laurent Pinchart