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

[Steve Lhomme]

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 :
>>> The timer API is not supported in Winstore builds so switch to something
>>> that works for all.
>>> ---
>>>   modules/audio_output/wasapi.c | 41 ++++++++++++++++-------------------
>>>   1 file changed, 19 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c
>>> index ecfcc99ae1dd..3644d7cea5ae 100644
>>> --- a/modules/audio_output/wasapi.c
>>> +++ b/modules/audio_output/wasapi.c
>>> @@ -91,7 +91,8 @@ static msftime_t GetQPC(void)
>>>   typedef struct aout_stream_sys
>>>   {
>>>       IAudioClient *client;
>>> -    HANDLE hTimer;
>>> +    vlc_timer_t timer;
>>> +    atomic_bool has_timer;
>>>
>>>   #define STARTED_STATE_INIT 0
>>>   #define STARTED_STATE_OK 1
>>> @@ -112,11 +113,8 @@ typedef struct aout_stream_sys
>>>   static void ResetTimer(aout_stream_t *s)
>>>   {
>>>       aout_stream_sys_t *sys = s->sys;
>>> -    if (sys->hTimer != NULL)
>>> -    {
>>> -        DeleteTimerQueueTimer(NULL, sys->hTimer, INVALID_HANDLE_VALUE);
>>> -        sys->hTimer = NULL;
>>> -    }
>>> +    if (atomic_exchange(&sys->has_timer,false))
>>> +        vlc_timer_destroy(sys->timer);
>>>   }
>>>
>>>   /*** VLC audio output callbacks ***/
>>> @@ -160,7 +158,7 @@ static HRESULT TimeGet(aout_stream_t *s, vlc_tick_t
>>> *restrict delay) return hr;
>>>   }
>>>
>>> -static void DoStartDeferred(void *val)
>>> +static void StartDeferredCallback(void *val)
>>>   {
>>>       aout_stream_t *s = val;
>>>       aout_stream_sys_t *sys = s->sys;
>>> @@ -169,25 +167,24 @@ static void DoStartDeferred(void *val)
>>>       atomic_store(&sys->started_state,
>>>                    SUCCEEDED(hr) ? STARTED_STATE_OK : STARTED_STATE_ERROR);
>>>   }
>>> -static void CALLBACK StartDeferredCallback(void *val, BOOLEAN timeout)
>>> -{
>>> -    DoStartDeferred(val);
>>> -    (void)timeout;
>>> -}
>>>
>>>   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).

>>> +    {
>>> +        if (vlc_timer_create( &sys->timer, StartDeferredCallback, s ) == 0)
>>> +        {
>>> +            msg_Warn(s, "start THREAD!");
>>> +            atomic_store(&sys->has_timer, true);
>>> +        }
>>> +    }
>>> +    if (atomic_load(&sys->has_timer))
>>> +    {
>>> +        vlc_timer_schedule( sys->timer, false, start_delay, 0);
>>> +        timer_updated = true;
>>> +    }
>>>       return timer_updated;
>>>   }
>>>
>>> @@ -759,7 +756,7 @@ static HRESULT Start(aout_stream_t *s,
>>> audio_sample_format_t *restrict pfmt, if (unlikely(sys == NULL))
>>>           return E_OUTOFMEMORY;
>>>       sys->client = NULL;
>>> -    sys->hTimer = NULL;
>>> +    atomic_init(&sys->has_timer, false);
>>>       atomic_init(&sys->started_state, STARTED_STATE_INIT);
>>>
>>>       /* Configure audio stream */
>>
>>
>> -- 
>> レミ・デニ-クールモン
>> http://www.remlab.net/
>>
>>
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>