[vlc-devel] [PATCH 4/6] demux: adaptive: reorder cleanup_push and cleanup_pop

[Hugo Beauzée-Luyssen]

On Wed, Sep 20, 2017, at 05:50 AM, Zhao Zhili wrote:
> ---
>   modules/demux/adaptive/PlaylistManager.cpp | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/modules/demux/adaptive/PlaylistManager.cpp 
> b/modules/demux/adaptive/PlaylistManager.cpp
> index 706ece0..96a0a57 100644
> --- a/modules/demux/adaptive/PlaylistManager.cpp
> +++ b/modules/demux/adaptive/PlaylistManager.cpp
> @@ -23,6 +23,9 @@
>   # include "config.h"
>   #endif
> 
> +#include <assert.h>
> +#include <vlc_common.h>
> +
>   #include "PlaylistManager.h"
>   #include "SegmentTracker.hpp"
>   #include "playlist/AbstractPlaylist.hpp"
> @@ -621,15 +624,14 @@ void PlaylistManager::setBufferingRunState(bool b)
>   void PlaylistManager::Run()
>   {
>       vlc_mutex_lock(&lock);
> +    mutex_cleanup_push(&lock);

This most likely improves the cancellation safety, but I'm afraid it
hurts the readability/self documentation more

>       const unsigned i_min_buffering = playlist->getMinBuffering();
>       const unsigned i_extra_buffering = playlist->getMaxBuffering() - 
> i_min_buffering;
>       while(1)
>       {
> -        mutex_cleanup_push(&lock);
>           while(!b_buffering)
>               vlc_cond_wait(&waitcond, &lock);
>           vlc_testcancel();
> -        vlc_cleanup_pop();
> 
>           if(needsUpdate())
>           {
> @@ -665,14 +667,14 @@ void PlaylistManager::Run()
>               vlc_cond_signal(&demux.cond);
>               vlc_mutex_unlock(&demux.lock);
> 
> -            mutex_cleanup_push(&lock);
>               while(b_buffering &&
>                       vlc_cond_timedwait(&waitcond, &lock, i_deadline) 
> == 0 &&
>                       i_deadline > mdate());
> -            vlc_cleanup_pop();
>           }
>       }
> +    vlc_cleanup_pop();
>       vlc_mutex_unlock(&lock);
> +    vlc_assert_unreachable();

I'd argue the unlock should be replaced by vlc_assert_unreachable
entirely

>   }
> 
>   void * PlaylistManager::managerThread(void *opaque)
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> Email had 1 attachment:
> + 0004-demux-adaptive-reorder-cleanup_push-and-cleanup_pop.patch
>   2k (text/x-patch)


-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr