[vlc-devel] [PATCH] win32: use the ThreadPoolTimer API to handle timers

[Rémi Denis-Courmont]

Le keskiviikkona 10. kesäkuuta 2020, 15.00.57 EEST Steve Lhomme a écrit :
> This is available since Windows 7 and is allowed in Winstore apps. So we
> only maintain one code for all Windows versions.
> 
> It should handle timers with an absolute time better as they had to be
> turned into relative times but the system sleep was delaying the timer. [1]
> 
> [1]
> https://docs.microsoft.com/en-us/windows/win32/api/threadpoollegacyapiset/n
> f-threadpoollegacyapiset-createtimerqueuetimer "The time that the system
> spends in sleep or hibernation does not count toward the expiration of the
> timer."
> ---
>  src/Makefile.am   |  5 +++--
>  src/win32/timer.c | 46 ++++++++++++++++++++++++----------------------
>  2 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9e088c1fd9e6..dc75776d7af7 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -406,11 +406,12 @@ libvlccore_la_SOURCES += \
>  	win32/rand.c \
>  	win32/specific.c \
>  	win32/thread.c \
> +	win32/timer.c \
>  	win32/winsock.c
>  if HAVE_WINSTORE
> -libvlccore_la_SOURCES += posix/timer.c win32/dirs-uap.c
> +libvlccore_la_SOURCES += win32/dirs-uap.c
>  else
> -libvlccore_la_SOURCES += win32/timer.c win32/dirs.c
> +libvlccore_la_SOURCES += win32/dirs.c
>  endif
>  endif
> 
> diff --git a/src/win32/timer.c b/src/win32/timer.c
> index 4fe3d2b067cb..4a2d68f996fb 100644
> --- a/src/win32/timer.c
> +++ b/src/win32/timer.c
> @@ -29,16 +29,16 @@
> 
>  struct vlc_timer
>  {
> -    HANDLE handle;
> +    PTP_TIMER ptimer;
>      void (*func) (void *);
>      void *data;
>  };
> 
> -static void CALLBACK vlc_timer_do (void *val, BOOLEAN timeout)
> +static void CALLBACK vlc_timer_do(PTP_CALLBACK_INSTANCE instance, void
> *val, PTP_TIMER ptimer) {
> +    (void) instance; (void) ptimer;
>      struct vlc_timer *timer = val;
> 
> -    assert (timeout);
>      timer->func (timer->data);
>  }
> 
> @@ -50,50 +50,52 @@ int vlc_timer_create (vlc_timer_t *id, void (*func)
> (void *), void *data) return ENOMEM;
>      timer->func = func;
>      timer->data = data;
> -    timer->handle = INVALID_HANDLE_VALUE;
> +    timer->ptimer = CreateThreadpoolTimer(vlc_timer_do, timer, NULL);
> +    if (unlikely(timer->ptimer == NULL))
> +    {
> +        free(timer);
> +        return EFAULT;

Is EFAULT really the intended error scenario? IMO, it's better to abort than 
return EFAULT, because it can only mean the calling code is buggy.

> +    }
>      *id = timer;
>      return 0;
>  }
> 
>  void vlc_timer_destroy (vlc_timer_t timer)
>  {
> -    if (timer->handle != INVALID_HANDLE_VALUE)
> -        DeleteTimerQueueTimer (NULL, timer->handle, INVALID_HANDLE_VALUE);
> +    SetThreadpoolTimer(timer->ptimer, NULL, 0, 0);
> +    WaitForThreadpoolTimerCallbacks(timer->ptimer, TRUE);
> +    CloseThreadpoolTimer(timer->ptimer);
>      free (timer);
>  }
> 
>  void vlc_timer_schedule (vlc_timer_t timer, bool absolute,
>                           vlc_tick_t value, vlc_tick_t interval)
>  {
> -    if (timer->handle != INVALID_HANDLE_VALUE)
> -    {
> -        DeleteTimerQueueTimer (NULL, timer->handle, INVALID_HANDLE_VALUE);
> -        timer->handle = INVALID_HANDLE_VALUE;
> -    }
> +    SetThreadpoolTimer(timer->ptimer, NULL, 0, 0);
> +    WaitForThreadpoolTimerCallbacks(timer->ptimer, TRUE);
>      if (value == VLC_TIMER_DISARM)
>          return; /* Disarm */
> 
> +    ULARGE_INTEGER s;
>      if (absolute)
> -    {
> -        value -= vlc_tick_now ();
> -        if (value < 0)
> -            value = 0;
> -    }
> +        s.QuadPart = MSFTIME_FROM_VLC_TICK(value);
> +    else
> +        s.QuadPart = -MSFTIME_FROM_VLC_TICK(value);
> +    FILETIME ts;
> +    ts.dwLowDateTime  = s.LowPart;
> +    ts.dwHighDateTime = s.HighPart;
> 
> -    DWORD val    = MS_FROM_VLC_TICK(value);
>      DWORD interv = MS_FROM_VLC_TICK(interval);
> -    if (val == 0 && value != 0)
> -        val = 1; /* rounding error */
>      if (interv == 0 && interval != 0)
>          interv = 1; /* rounding error */
> 
> -    if (!CreateTimerQueueTimer(&timer->handle, NULL, vlc_timer_do, timer,
> -                               val, interv, WT_EXECUTEDEFAULT))
> -        abort ();
> +    WaitForThreadpoolTimerCallbacks(timer->ptimer, FALSE);
> +    SetThreadpoolTimer(timer->ptimer, &ts, interv, 0 );
>  }
> 
>  unsigned vlc_timer_getoverrun (vlc_timer_t timer)
>  {
> +    // TODO return the amount of missed timers if any
>      (void)timer;
>      return 0;
>  }


-- 
Rémi Denis-Courmont
http://www.remlab.net/