[vlc-devel] [PATCH] demux: adaptive: fix inverted logic and don't do wait in a loop

[Zhao Zhili]

On Wed, Jul 26, 2017 at 11:20 PM, Rémi Denis-Courmont <remi at remlab.net>
wrote:

> Le keskiviikkona 26. heinäkuuta 2017, 23.08.21 EEST Zhao Zhili a écrit :
> > On Wed, Jul 26, 2017 at 10:38 PM, Rémi Denis-Courmont <remi at remlab.net>
> >
> > wrote:
> > > Le keskiviikkona 26. heinäkuuta 2017, 19.43.34 EEST Zhao Zhili a écrit
> :
> > > > ---
> > > >
> > > >  modules/demux/adaptive/PlaylistManager.cpp | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/modules/demux/adaptive/PlaylistManager.cpp
> > > > b/modules/demux/adaptive/PlaylistManager.cpp
> > > > index f560d0f..f489a18f 100644
> > > > --- a/modules/demux/adaptive/PlaylistManager.cpp
> > > > +++ b/modules/demux/adaptive/PlaylistManager.cpp
> > > > @@ -664,8 +664,7 @@ void PlaylistManager::Run()
> > > >
> > > >              vlc_mutex_unlock(&demux.lock);
> > > >
> > > >              mutex_cleanup_push(&lock);
> > > >
> > > > -            while(vlc_cond_timedwait(&waitcond, &lock, i_deadline)
> == 0
> > > > -                 && i_deadline < mdate());
> > > > +            vlc_cond_timedwait(&waitcond, &lock, i_deadline);
> > > >
> > > >              vlc_cleanup_pop();
> > > >
> > > >          }
> > > >
> > > >      }
> > >
> > > The existing code is an obtuse and inefficient emulation of mwait(). It
> > > does
> > > not make much sense. But the patch is also wrong due to spurious
> wake-ups.
> >
> > I think it's more similar to ControlPop() than mwait().
>
> No. The second part of the loop predicate is a no-op since
> vlc_cond_timedwait() will only return non-zero after the deadline is
> reached.
> So the code is equivalent to:
>
>     mutex_cleanup_push(&lock);
>
>     while (vlc_cond_timedwait(&waitcond, &lock, i_deadline) == 0);
>
>     vlc_cleanup_pop();
>
> The predicate for the condition variable is always false. So that is
> obviously
> equivalent to a simple sleep:
>
>     mutex_cleanup_push(&lock);
>
>     vlc_mutex_unlock(&lock);
>     vlc_cleanup_push(vlc_mutex_lock, &lock);
>     mwait(i_deadline);
>     vlc_cleanup_pop();
>     vlc_mutex_lock(&lock);
>
>     vlc_cleanup_pop();
>
> The cleanup handlers obviously cancel out one another, so the code can
> trivially be further simplified:
>
>     vlc_mutex_unlock(&lock);
>     mwait(i_deadline);
>     vlc_mutex_lock(&lock);
>
>
The logic of the original code is like this, but it should wakeup
immediately in case
of setBufferingRunState(true). For example, it should start bufferize after
SET_TIME/SET_POSITION.



>
> --
> 雷米‧德尼-库尔蒙
> https://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170727/3631eb91/attachment.html>