[vlc-devel] [PATCH 2/2] aout: wasapi: use a vlc_timer for the deferred start

[Steve Lhomme]

On 2020-06-10 9:33, Thomas Guillem wrote:
> 
> 
> On Wed, Jun 10, 2020, at 09:25, Steve Lhomme wrote:
>> On 2020-06-10 8:56, Thomas Guillem wrote:
>>>
>>> On Tue, Jun 9, 2020, at 17:33, Rémi Denis-Courmont wrote:
>>>> Le tiistaina 9. kesäkuuta 2020, 17.48.48 EEST Steve Lhomme a écrit :
>>>>>    static bool StartTimer(aout_stream_t *s, vlc_tick_t start_delay)
>>>>>    {
>>>>>        aout_stream_sys_t *sys = s->sys;
>>>>>        bool timer_updated = false;
>>>>> -    DWORD start_delay_ms = MS_FROM_VLC_TICK(start_delay);
>>>>> -    if (sys->hTimer == NULL)
>>>>> -        timer_updated =
>>>>> -            CreateTimerQueueTimer(&sys->hTimer, NULL,
>>>>> StartDeferredCallback, -                                    s,
>>>>> start_delay_ms, 0,
>>>>> -                                    WT_EXECUTEDEFAULT |
>>>>> WT_EXECUTEONLYONCE); -    else
>>>>> -        timer_updated =
>>>>> -            ChangeTimerQueueTimer(NULL, sys->hTimer, start_delay_ms, 0);
>>>>> +    if (!atomic_load(&sys->has_timer))
>>>>
>>>> I don't follow how the atomic works here. It looks like it's either
>>>> unnecessary or insufficient, as it would race with vlc_timer_create().
>>>
>>> Indeed, note that every audio callbacks are called from the same thread.
>>
>> I saw started_state, an atomic_char, is used more or less in the same
>> places, so went for an atomic. I will remove the atomic. But it seems
>> that variable doesn't one either. It's only accessed via callbacks as
>> well (TimeGet, Play, Pause, Flush, Stop).
>>
> 
> started_date is used from 2 threads. Audio callback thread and StartDeferredCallback().

Ah yes indeed.

I'm going to rework my patch to avoid timer create/destroy. There is 
vlc_timer_disarm() to do the ResetTimer more efficiently.