<html><head></head><body>Memory fault should abort or reliably crash the process ASAP. Nothing new or VLC-specific here.<br><br>I'm not saying that we should proactively check for those errors since they are by definition UB and not supposed to happen. But to the extent that we detect them anyway, we must abort.<br><br><div class="gmail_quote">Le 12 juin 2020 08:31:30 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2020-06-11 17:25, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Le keskiviikkona 10. kesäkuuta 2020, 15.00.57 EEST Steve Lhomme a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> This is available since Windows 7 and is allowed in Winstore apps. So we<br> only maintain one code for all Windows versions.<br><br> It should handle timers with an absolute time better as they had to be<br> turned into relative times but the system sleep was delaying the timer. [1]<br><br> [1]<br> <a href="https://docs.microsoft.com/en-us/windows/win32/api/threadpoollegacyapiset/n">https://docs.microsoft.com/en-us/windows/win32/api/threadpoollegacyapiset/n</a><br> f-threadpoollegacyapiset-createtimerqueuetimer "The time that the system<br> spends in sleep or hibernation does not count toward the expiration of the<br> timer."<hr> src/Makefile.am | 5 +++--<br> src/win32/timer.c | 46 ++++++++++++++++++++++++----------------------<br> 2 files changed, 27 insertions(+), 24 deletions(-)<br><br> diff --git a/src/Makefile.am b/src/Makefile.am<br> index 9e088c1fd9e6..dc75776d7af7 100644<br> --- a/src/Makefile.am<br> +++ b/src/Makefile.am<br> @@ -406,11 +406,12 @@ libvlccore_la_SOURCES += \<br> win32/rand.c \<br> win32/specific.c \<br> win32/thread.c \<br> + win32/timer.c \<br> win32/winsock.c<br> if HAVE_WINSTORE<br> -libvlccore_la_SOURCES += posix/timer.c win32/dirs-uap.c<br> +libvlccore_la_SOURCES += win32/dirs-uap.c<br> else<br> -libvlccore_la_SOURCES += win32/timer.c win32/dirs.c<br> +libvlccore_la_SOURCES += win32/dirs.c<br> endif<br> endif<br><br> diff --git a/src/win32/timer.c b/src/win32/timer.c<br> index 4fe3d2b067cb..4a2d68f996fb 100644<br> --- a/src/win32/timer.c<br> +++ b/src/win32/timer.c<br> @@ -29,16 +29,16 @@<br><br> struct vlc_timer<br> {<br> - HANDLE handle;<br> + PTP_TIMER ptimer;<br> void (*func) (void *);<br> void *data;<br> };<br><br> -static void CALLBACK vlc_timer_do (void *val, BOOLEAN timeout)<br> +static void CALLBACK vlc_timer_do(PTP_CALLBACK_INSTANCE instance, void<br> *val, PTP_TIMER ptimer) {<br> + (void) instance; (void) ptimer;<br> struct vlc_timer *timer = val;<br><br> - assert (timeout);<br> timer->func (timer->data);<br> }<br><br> @@ -50,50 +50,52 @@ int vlc_timer_create (vlc_timer_t *id, void (*func)<br> (void *), void *data) return ENOMEM;<br> timer->func = func;<br> timer->data = data;<br> - timer->handle = INVALID_HANDLE_VALUE;<br> + timer->ptimer = CreateThreadpoolTimer(vlc_timer_do, timer, NULL);<br> + if (unlikely(timer->ptimer == NULL))<br> + {<br> + free(timer);<br> + return EFAULT;<br></blockquote>Is EFAULT really the intended error scenario? IMO, it's better to abort than<br>return EFAULT, because it can only mean the calling code is buggy.<br></blockquote><br>I would never add an abort in the core (or in modules for that matter). <br>If you mean asserting, maybe. But do have a return value here that <br>callers should handle (and they do) so I don't see why we would want to <br>add extra checks.<br><br>I don't see how this is a bug in our code if the OS cannot create a <br>timer pool at that moment. The documentation doesn't say in which case <br>it can happen, but it can.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> + }<br> *id = timer;<br> return 0;<br> }<br><br> void vlc_timer_destroy (vlc_timer_t timer)<br> {<br> - if (timer->handle != INVALID_HANDLE_VALUE)<br> - DeleteTimerQueueTimer (NULL, timer->handle, INVALID_HANDLE_VALUE);<br> + SetThreadpoolTimer(timer->ptimer, NULL, 0, 0);<br> + WaitForThreadpoolTimerCallbacks(timer->ptimer, TRUE);<br> + CloseThreadpoolTimer(timer->ptimer);<br> free (timer);<br> }<br><br> void vlc_timer_schedule (vlc_timer_t timer, bool absolute,<br> vlc_tick_t value, vlc_tick_t interval)<br> {<br> - if (timer->handle != INVALID_HANDLE_VALUE)<br> - {<br> - DeleteTimerQueueTimer (NULL, timer->handle, INVALID_HANDLE_VALUE);<br> - timer->handle = INVALID_HANDLE_VALUE;<br> - }<br> + SetThreadpoolTimer(timer->ptimer, NULL, 0, 0);<br> + WaitForThreadpoolTimerCallbacks(timer->ptimer, TRUE);<br> if (value == VLC_TIMER_DISARM)<br> return; /* Disarm */<br><br> + ULARGE_INTEGER s;<br> if (absolute)<br> - {<br> - value -= vlc_tick_now ();<br> - if (value < 0)<br> - value = 0;<br> - }<br> + s.QuadPart = MSFTIME_FROM_VLC_TICK(value);<br> + else<br> + s.QuadPart = -MSFTIME_FROM_VLC_TICK(value);<br> + FILETIME ts;<br> + ts.dwLowDateTime = s.LowPart;<br> + ts.dwHighDateTime = s.HighPart;<br><br> - DWORD val = MS_FROM_VLC_TICK(value);<br> DWORD interv = MS_FROM_VLC_TICK(interval);<br> - if (val == 0 && value != 0)<br> - val = 1; /* rounding error */<br> if (interv == 0 && interval != 0)<br> interv = 1; /* rounding error */<br><br> - if (!CreateTimerQueueTimer(&timer->handle, NULL, vlc_timer_do, timer,<br> - val, interv, WT_EXECUTEDEFAULT))<br> - abort ();<br> + WaitForThreadpoolTimerCallbacks(timer->ptimer, FALSE);<br> + SetThreadpoolTimer(timer->ptimer, &ts, interv, 0 );<br> }<br><br> unsigned vlc_timer_getoverrun (vlc_timer_t timer)<br> {<br> + // TODO return the amount of missed timers if any<br> (void)timer;<br> return 0;<br> }<br></blockquote><br>-- <br>Rémi Denis-Courmont<br><a href="http://www.remlab.net/">http://www.remlab.net/</a><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>