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

[Thomas Guillem]

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.

> 
> > +    {
> > +        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