[vlc-devel] [PATCH 5/7] win32/window: simplify using semaphore

[Thomas Guillem]

On Tue, Feb 4, 2020, at 09:22, Steve Lhomme wrote:
> On 2020-02-03 22:13, Rémi Denis-Courmont wrote:
> > ---
> >   modules/video_output/win32/window.c | 22 ++++++----------------
> >   1 file changed, 6 insertions(+), 16 deletions(-)
> > 
> > diff --git a/modules/video_output/win32/window.c b/modules/video_output/win32/window.c
> > index 3dc9cf32d2..00965f61fa 100644
> > --- a/modules/video_output/win32/window.c
> > +++ b/modules/video_output/win32/window.c
> > @@ -50,8 +50,7 @@ typedef struct vout_window_sys_t
> >   {
> >       vlc_thread_t thread;
> >       vlc_mutex_t  lock;
> > -    vlc_cond_t   wait;
> > -    bool         b_ready;
> > +    vlc_sem_t    ready;
> >   
> >       HWND hwnd;
> >   
> > @@ -486,8 +485,8 @@ static void Close(vout_window_t *wnd)
> >           PostMessage( sys->hwnd, WM_CLOSE, 0, 0 );
> >   
> >       vlc_join(sys->thread, NULL);
> > +    vlc_sem_destroy( &sys->ready );
> >       vlc_mutex_destroy( &sys->lock );
> > -    vlc_cond_destroy( &sys->wait );
> >   
> >       if (sys->hwnd)
> >           DestroyWindow( sys->hwnd );
> > @@ -637,10 +636,7 @@ static void *EventThread( void *p_this )
> >                       hInstance,            /* handle of this program instance */
> >                       wnd );                           /* send vd to WM_CREATE */
> >   
> > -    vlc_mutex_lock( &sys->lock );
> > -    sys->b_ready = true;
> > -    vlc_cond_signal( &sys->wait );
> > -    vlc_mutex_unlock( &sys->lock );
> > +    vlc_sem_post( &sys->ready );
> >   
> >       if (sys->hwnd == NULL)
> >       {
> > @@ -738,8 +734,7 @@ static int Open(vout_window_t *wnd)
> >           return VLC_EGENERIC;
> >       }
> >       vlc_mutex_init( &sys->lock );
> > -    vlc_cond_init( &sys->wait );
> > -    sys->b_ready = false;
> > +    vlc_sem_init( &sys->ready, 0 );
> >   
> >       wnd->sys = sys;
> >       if( vlc_clone( &sys->thread, EventThread, wnd, VLC_THREAD_PRIORITY_LOW ) )
> > @@ -748,18 +743,13 @@ static int Open(vout_window_t *wnd)
> >           return VLC_EGENERIC;
> >       }
> >   
> > -    vlc_mutex_lock( &sys->lock );
> > -    while( !sys->b_ready )
> > -    {
> > -        vlc_cond_wait( &sys->wait, &sys->lock );
> > -    }
> > +    vlc_sem_wait( &sys->ready );
> 
> Is this cancelable as it used to be ? If not we may end up in a state in 
> which we can never leave the Open code.

This part of code was not cancelable before. vlc_cancel() not used in this module. Small reminder, cancellation is inhibited when we enter a module.

> 
> Isn't a semaphore heavier to handle than a condition variable ?
> 
> > +
> >       if (sys->hwnd == NULL)
> >       {
> > -        vlc_mutex_unlock( &sys->lock );
> >           Close(wnd);
> >           return VLC_EGENERIC;
> >       }
> > -    vlc_mutex_unlock( &sys->lock );
> >   
> >       wnd->sys = sys;
> >       wnd->type = VOUT_WINDOW_TYPE_HWND;
> > -- 
> > 2.25.0

LGTM

> > 
> > _______________________________________________
> > 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