[vlc-devel] [PATCH 2/2] demux: adaptive: use C++ wait conditions and lock

[Thomas Guillem]

Not sure about this patch. cf. the Qt code. It is using VLC mutex and conds.

So, if you do that change, you should do it for every c++ code, for consistency. No ?

On Tue, Feb 11, 2020, at 10:01, Steve Lhomme wrote:
> ---
>  modules/demux/adaptive/PlaylistManager.cpp | 42 ++++++++++------------
>  modules/demux/adaptive/PlaylistManager.h   |  5 +--
>  2 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/modules/demux/adaptive/PlaylistManager.cpp 
> b/modules/demux/adaptive/PlaylistManager.cpp
> index 62c697e1432..13067f7589c 100644
> --- a/modules/demux/adaptive/PlaylistManager.cpp
> +++ b/modules/demux/adaptive/PlaylistManager.cpp
> @@ -70,8 +70,6 @@ PlaylistManager::PlaylistManager( demux_t *p_demux_,
>      demux.i_firstpcr = VLC_TICK_INVALID;
>      vlc_mutex_init(&demux.lock);
>      vlc_cond_init(&demux.cond);
> -    vlc_mutex_init(&lock);
> -    vlc_cond_init(&waitcond);
>      vlc_mutex_init(&cached.lock);
>      cached.b_live = false;
>      cached.f_position = 0.0;
> @@ -89,8 +87,6 @@ PlaylistManager::~PlaylistManager   ()
>      delete playlist;
>      delete logic;
>      delete resources;
> -    vlc_cond_destroy(&waitcond);
> -    vlc_mutex_destroy(&lock);
>      vlc_mutex_destroy(&demux.lock);
>      vlc_cond_destroy(&demux.cond);
>      vlc_mutex_destroy(&cached.lock);
> @@ -183,10 +179,11 @@ void PlaylistManager::stop()
>  {
>      if(thread != nullptr)
>      {
> -        vlc_mutex_lock(&lock);
> -        b_canceled = true;
> -        vlc_cond_signal(&waitcond);
> -        vlc_mutex_unlock(&lock);
> +        {
> +            std::lock_guard<std::mutex> lk(lock);
> +            b_canceled = true;
> +            waitcond.notify_one();
> +        }
>  
>          vlc_join(thread, NULL);
>          thread = nullptr;
> @@ -608,21 +605,20 @@ int PlaylistManager::doControl(int i_query, va_list args)
>  
>  void PlaylistManager::setBufferingRunState(bool b)
>  {
> -    vlc_mutex_lock(&lock);
> +    std::lock_guard<std::mutex> lk(lock);
>      b_buffering = b;
> -    vlc_cond_signal(&waitcond);
> -    vlc_mutex_unlock(&lock);
> +    waitcond.notify_one();
>  }
>  
>  void PlaylistManager::Run()
>  {
> -    vlc_mutex_lock(&lock);
> +    std::unique_lock<std::mutex> wait_lock(lock);
>      const vlc_tick_t i_min_buffering = playlist->getMinBuffering();
>      const vlc_tick_t i_extra_buffering = playlist->getMaxBuffering() - 
> i_min_buffering;
>      while(1)
>      {
>          while(!b_buffering && !b_canceled)
> -            vlc_cond_wait(&waitcond, &lock);
> +            waitcond.wait(wait_lock);
>          if (b_canceled)
>              break;
>  
> @@ -646,29 +642,29 @@ void PlaylistManager::Run()
>  
>          if(i_return != AbstractStream::buffering_lessthanmin)
>          {
> -            vlc_tick_t i_deadline = vlc_tick_now();
> +            auto deadline = std::chrono::system_clock::now();
> +
> +            using namespace std::chrono_literals;
>              if(i_return == AbstractStream::buffering_ongoing)
> -                i_deadline += VLC_TICK_FROM_MS(10);
> +                deadline += 10ms;
>              else if(i_return == AbstractStream::buffering_full)
> -                i_deadline += VLC_TICK_FROM_MS(100);
> +                deadline += 100ms;
>              else if(i_return == AbstractStream::buffering_end)
> -                i_deadline += VLC_TICK_FROM_SEC(1);
> +                deadline += 1000ms;
>              else /*if(i_return == AbstractStream::buffering_suspended)*/
> -                i_deadline += VLC_TICK_FROM_MS(250);
> +                deadline += 250ms;
>  
>              vlc_mutex_lock(&demux.lock);
>              vlc_cond_signal(&demux.cond);
>              vlc_mutex_unlock(&demux.lock);
>  
> -            while(b_buffering &&
> -                    vlc_cond_timedwait(&waitcond, &lock, i_deadline) == 0 &&
> -                    i_deadline > vlc_tick_now() &&
> -                    !b_canceled);
> +            waitcond.wait_until(wait_lock, deadline, [this](){
> +                      return !b_buffering || b_canceled;
> +                  });
>              if (b_canceled)
>                  break;
>          }
>      }
> -    vlc_mutex_unlock(&lock);
>  }
>  
>  void * PlaylistManager::managerThread(void *opaque)
> diff --git a/modules/demux/adaptive/PlaylistManager.h 
> b/modules/demux/adaptive/PlaylistManager.h
> index 501200f9b7a..3eece82aec5 100644
> --- a/modules/demux/adaptive/PlaylistManager.h
> +++ b/modules/demux/adaptive/PlaylistManager.h
> @@ -25,6 +25,7 @@
>  #include "logic/AbstractAdaptationLogic.h"
>  #include "Streams.hpp"
>  #include <vector>
> +#include <condition_variable>
>  
>  namespace adaptive
>  {
> @@ -130,9 +131,9 @@ namespace adaptive
>              void setBufferingRunState(bool);
>              void Run();
>              static void * managerThread(void *);
> -            vlc_mutex_t  lock;
>              vlc_thread_t thread;
> -            vlc_cond_t   waitcond;
> +            std::condition_variable waitcond;
> +            std::mutex              lock;
>              bool         b_buffering;
>              bool         b_canceled;
>      };
> -- 
> 2.17.1
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel