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

[Steve Lhomme]

On 2020-02-11 10:15, Thomas Guillem wrote:
> 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 ?

Ultimately if we agree on that, yes. We would only need to use our own 
implementation if we ever share condition variables, locks, mutex, etc 
between C++ and C code. But I doubt we do that anywhere nor it is 
something we should ever do.

This file also has other conditional variables that could be replaced.

> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>