[vlc-devel] [PATCH 1/4] demux:adaptive: use C++ vlc_mutex_locker

Zhao Zhili quinkblack at foxmail.com
Mon Jan 8 13:20:29 CET 2018



On 2018年01月08日 15:37, Steve Lhomme wrote:
> It can simplify the code in some cases.
> ---
>   modules/demux/adaptive/PlaylistManager.cpp         |  3 +-
>   modules/demux/adaptive/Streams.cpp                 |  3 +-
>   .../adaptive/logic/NearOptimalAdaptationLogic.cpp  | 12 ++---
>   modules/demux/adaptive/plumbing/CommandsQueue.cpp  | 40 ++++++-----------
>   modules/demux/adaptive/plumbing/FakeESOut.cpp      | 51 +++++++---------------
>   5 files changed, 35 insertions(+), 74 deletions(-)
>
> diff --git a/modules/demux/adaptive/PlaylistManager.cpp b/modules/demux/adaptive/PlaylistManager.cpp
> index 700fa3a8d3..ec446582c1 100644
> --- a/modules/demux/adaptive/PlaylistManager.cpp
> +++ b/modules/demux/adaptive/PlaylistManager.cpp
> @@ -247,13 +247,12 @@ AbstractStream::buffering_status PlaylistManager::bufferize(mtime_t i_nzdeadline
>               break;
>       }
>   
> -    vlc_mutex_lock(&demux.lock);
> +    vlc_mutex_locker locker(&demux.lock);
>       if(demux.i_nzpcr == VLC_TS_INVALID &&
>          i_return != AbstractStream::buffering_lessthanmin /* prevents starting before buffering is reached */ )
>       {
>           demux.i_nzpcr = getFirstDTS();
>       }
> -    vlc_mutex_unlock(&demux.lock);
>   
>       return i_return;
>   }

I'm afraid it won't work if we add more code after the vlc_mutex_unlock().
If we really want auto lock and unlock, put it in a code block.

> diff --git a/modules/demux/adaptive/Streams.cpp b/modules/demux/adaptive/Streams.cpp
> index ecb3efc68b..461d05a3d2 100644
> --- a/modules/demux/adaptive/Streams.cpp
> +++ b/modules/demux/adaptive/Streams.cpp
> @@ -162,7 +162,7 @@ mtime_t AbstractStream::getMinAheadTime() const
>   mtime_t AbstractStream::getFirstDTS() const
>   {
>       mtime_t dts;
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t *>(&lock));
>       if(isDisabled())
>       {
>           dts = VLC_TS_INVALID;
> @@ -173,7 +173,6 @@ mtime_t AbstractStream::getFirstDTS() const
>           if(dts == VLC_TS_INVALID)
>               dts = commandsqueue->getPCR();
>       }
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
>       return dts;
>   }
>   
> diff --git a/modules/demux/adaptive/logic/NearOptimalAdaptationLogic.cpp b/modules/demux/adaptive/logic/NearOptimalAdaptationLogic.cpp
> index ed54d15f0d..c9c40a9365 100644
> --- a/modules/demux/adaptive/logic/NearOptimalAdaptationLogic.cpp
> +++ b/modules/demux/adaptive/logic/NearOptimalAdaptationLogic.cpp
> @@ -180,7 +180,7 @@ unsigned NearOptimalAdaptationLogic::getMaxCurrentBw() const
>   
>   void NearOptimalAdaptationLogic::updateDownloadRate(const ID &id, size_t dlsize, mtime_t time)
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       std::map<ID, NearOptimalContext>::iterator it = streams.find(id);
>       if(it != streams.end())
>       {
> @@ -188,7 +188,6 @@ void NearOptimalAdaptationLogic::updateDownloadRate(const ID &id, size_t dlsize,
>           ctx.last_download_rate = ctx.average.push(CLOCK_FREQ * dlsize * 8 / time);
>       }
>       currentBps = getMaxCurrentBw();
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void NearOptimalAdaptationLogic::trackerEvent(const SegmentTrackerEvent &event)
> @@ -197,20 +196,19 @@ void NearOptimalAdaptationLogic::trackerEvent(const SegmentTrackerEvent &event)
>       {
>       case SegmentTrackerEvent::SWITCHING:
>           {
> -            vlc_mutex_lock(&lock);
> +            vlc_mutex_locker locker(&lock);
>               if(event.u.switching.prev)
>                   usedBps -= event.u.switching.prev->getBandwidth();
>               if(event.u.switching.next)
>                   usedBps += event.u.switching.next->getBandwidth();
>                    BwDebug(msg_Info(p_obj, "New total bandwidth usage %zu kBps", (usedBps / 8000)));
> -            vlc_mutex_unlock(&lock);
>           }
>           break;
>   
>       case SegmentTrackerEvent::BUFFERING_STATE:
>           {
>               const ID &id = *event.u.buffering.id;
> -            vlc_mutex_lock(&lock);
> +            vlc_mutex_locker locker(&lock);
>               if(event.u.buffering.enabled)
>               {
>                   if(streams.find(id) == streams.end())
> @@ -225,7 +223,6 @@ void NearOptimalAdaptationLogic::trackerEvent(const SegmentTrackerEvent &event)
>                   if(it != streams.end())
>                       streams.erase(it);
>               }
> -            vlc_mutex_unlock(&lock);
>               BwDebug(msg_Info(p_obj, "Stream %s is now known %sactive", id.str().c_str(),
>                            (event.u.buffering.enabled) ? "" : "in"));
>           }
> @@ -234,11 +231,10 @@ void NearOptimalAdaptationLogic::trackerEvent(const SegmentTrackerEvent &event)
>       case SegmentTrackerEvent::BUFFERING_LEVEL_CHANGE:
>           {
>               const ID &id = *event.u.buffering.id;
> -            vlc_mutex_lock(&lock);
> +            vlc_mutex_locker locker(&lock);
>               NearOptimalContext &ctx = streams[id];
>               ctx.buffering_level = event.u.buffering_level.current;
>               ctx.buffering_target = event.u.buffering_level.target;
> -            vlc_mutex_unlock(&lock);
>           }
>           break;
>   
> diff --git a/modules/demux/adaptive/plumbing/CommandsQueue.cpp b/modules/demux/adaptive/plumbing/CommandsQueue.cpp
> index 850c94b60e..722c01b218 100644
> --- a/modules/demux/adaptive/plumbing/CommandsQueue.cpp
> +++ b/modules/demux/adaptive/plumbing/CommandsQueue.cpp
> @@ -254,7 +254,7 @@ static bool compareCommands( AbstractCommand *a, AbstractCommand *b )
>   
>   void CommandsQueue::Schedule( AbstractCommand *command )
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       if( b_drop )
>       {
>           delete command;
> @@ -269,7 +269,6 @@ void CommandsQueue::Schedule( AbstractCommand *command )
>       {
>           incoming.push_back( command );
>       }
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   const CommandsFactory * CommandsQueue::factory() const
> @@ -293,7 +292,7 @@ mtime_t CommandsQueue::Process( es_out_t *out, mtime_t barrier )
>       std::list<AbstractCommand *> output;
>       std::list<AbstractCommand *> in;
>   
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>   
>       in.splice( in.end(), commands );
>   
> @@ -362,8 +361,6 @@ mtime_t CommandsQueue::Process( es_out_t *out, mtime_t barrier )
>       }
>       pcr = lastdts; /* Warn! no PCR update/lock release until execution */
>   
> -    vlc_mutex_unlock(&lock);
> -
>       return lastdts;
>   }
>   
> @@ -376,14 +373,13 @@ void CommandsQueue::LockedCommit()
>   
>   void CommandsQueue::Commit()
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       LockedCommit();
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void CommandsQueue::Abort( bool b_reset )
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       commands.splice( commands.end(), incoming );
>       while( !commands.empty() )
>       {
> @@ -398,37 +394,30 @@ void CommandsQueue::Abort( bool b_reset )
>           b_draining = false;
>           b_eof = false;
>       }
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   bool CommandsQueue::isEmpty() const
>   {
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> -    bool b_empty = commands.empty() && incoming.empty();
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
> -    return b_empty;
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t *>(&lock));
> +    return commands.empty() && incoming.empty();
>   }
>   
>   void CommandsQueue::setDrop( bool b )
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       b_drop = b;
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void CommandsQueue::setDraining()
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       LockedSetDraining();
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   bool CommandsQueue::isDraining() const
>   {
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> -    bool b = b_draining;
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
> -    return b;
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t *>(&lock));
> +    return b_draining;
>   }
>   
>   void CommandsQueue::setEOF( bool b )
> @@ -467,7 +456,7 @@ mtime_t CommandsQueue::getBufferingLevel() const
>   mtime_t CommandsQueue::getFirstDTS() const
>   {
>       std::list<AbstractCommand *>::const_iterator it;
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t *>(&lock));
>       mtime_t i_firstdts = pcr;
>       for( it = commands.begin(); it != commands.end(); ++it )
>       {
> @@ -479,7 +468,6 @@ mtime_t CommandsQueue::getFirstDTS() const
>               break;
>           }
>       }
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
>       return i_firstdts;
>   }
>   
> @@ -491,8 +479,6 @@ void CommandsQueue::LockedSetDraining()
>   
>   mtime_t CommandsQueue::getPCR() const
>   {
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> -    mtime_t i_pcr = pcr;
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
> -    return i_pcr;
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t *>(&lock));
> +    return pcr;
>   }
> diff --git a/modules/demux/adaptive/plumbing/FakeESOut.cpp b/modules/demux/adaptive/plumbing/FakeESOut.cpp
> index bc1517ebc7..3f3bb1f749 100644
> --- a/modules/demux/adaptive/plumbing/FakeESOut.cpp
> +++ b/modules/demux/adaptive/plumbing/FakeESOut.cpp
> @@ -66,26 +66,23 @@ FakeESOut::~FakeESOut()
>   
>   void FakeESOut::setExpectedTimestampOffset(mtime_t offset)
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       timestamps_offset = 0;
>       timestamps_expected = offset;
>       timestamps_check_done = false;
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void FakeESOut::setTimestampOffset(mtime_t offset)
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       timestamps_offset = offset;
>       timestamps_check_done = true;
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void FakeESOut::setExtraInfoProvider( ExtraFMTInfoInterface *extra )
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       extrainfo = extra;
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   FakeESOutID * FakeESOut::createNewID( const es_format_t *p_fmt )
> @@ -96,7 +93,7 @@ FakeESOutID * FakeESOut::createNewID( const es_format_t *p_fmt )
>       fmtcopy.i_group = 0; /* Always ignore group for adaptive */
>       fmtcopy.i_id = -1;
>   
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>   
>       if( extrainfo )
>           extrainfo->fillExtraFMTInfo( &fmtcopy );
> @@ -105,8 +102,6 @@ FakeESOutID * FakeESOut::createNewID( const es_format_t *p_fmt )
>       if(likely(es_id))
>           fakeesidlist.push_back( es_id );
>   
> -    vlc_mutex_unlock(&lock);
> -
>       es_format_Clean( &fmtcopy );
>   
>       return es_id;
> @@ -117,7 +112,7 @@ void FakeESOut::createOrRecycleRealEsID( FakeESOutID *es_id )
>       std::list<FakeESOutID *>::iterator it;
>       es_out_id_t *realid = NULL;
>   
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>   
>       bool b_select = false;
>       for( it=recycle_candidates.begin(); it!=recycle_candidates.end(); ++it )
> @@ -149,27 +144,22 @@ void FakeESOut::createOrRecycleRealEsID( FakeESOutID *es_id )
>       }
>   
>       es_id->setRealESID( realid );
> -
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   mtime_t FakeESOut::getTimestampOffset() const
>   {
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> -    mtime_t time = timestamps_offset;
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
> -    return time;
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t*>(&lock));
> +    return timestamps_offset;
>   }
>   
>   size_t FakeESOut::esCount() const
>   {
>       size_t i_count = 0;
>       std::list<FakeESOutID *>::const_iterator it;
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t*>(&lock));
>       for( it=fakeesidlist.begin(); it!=fakeesidlist.end(); ++it )
>           if( (*it)->realESID() )
>               i_count++;
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
>       return i_count;
>   }
>   
> @@ -183,7 +173,7 @@ void FakeESOut::schedulePCRReset()
>   void FakeESOut::scheduleAllForDeletion()
>   {
>       std::list<FakeESOutID *>::const_iterator it;
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       for( it=fakeesidlist.begin(); it!=fakeesidlist.end(); ++it )
>       {
>           FakeESOutID *es_id = *it;
> @@ -197,7 +187,6 @@ void FakeESOut::scheduleAllForDeletion()
>               }
>           }
>       }
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void FakeESOut::recycleAll()
> @@ -205,17 +194,15 @@ void FakeESOut::recycleAll()
>       /* Only used when demux is killed and commands queue is cancelled */
>       commandsqueue->Abort( true );
>       assert(commandsqueue->isEmpty());
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       recycle_candidates.splice( recycle_candidates.end(), fakeesidlist );
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   void FakeESOut::gc()
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       if( recycle_candidates.empty() )
>       {
> -        vlc_mutex_unlock(&lock);
>           return;
>       }
>   
> @@ -230,21 +217,19 @@ void FakeESOut::gc()
>           delete *it;
>       }
>       recycle_candidates.clear();
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   bool FakeESOut::hasSelectedEs() const
>   {
>       bool b_selected = false;
>       std::list<FakeESOutID *>::const_iterator it;
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t*>(&lock));
>       for( it=fakeesidlist.begin(); it!=fakeesidlist.end() && !b_selected; ++it )
>       {
>           FakeESOutID *esID = *it;
>           if( esID->realESID() )
>               es_out_Control( real_es_out, ES_OUT_GET_ES_STATE, esID->realESID(), &b_selected );
>       }
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
>       return b_selected;
>   }
>   
> @@ -257,18 +242,15 @@ bool FakeESOut::decodersDrained()
>   
>   bool FakeESOut::restarting() const
>   {
> -    vlc_mutex_lock(const_cast<vlc_mutex_t *>(&lock));
> -    bool b = !recycle_candidates.empty();
> -    vlc_mutex_unlock(const_cast<vlc_mutex_t *>(&lock));
> -    return b;
> +    vlc_mutex_locker locker(const_cast<vlc_mutex_t*>(&lock));
> +    return !recycle_candidates.empty();
>   }
>   
>   void FakeESOut::recycle( FakeESOutID *id )
>   {
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       fakeesidlist.remove( id );
>       recycle_candidates.push_back( id );
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   /* Static callbacks */
> @@ -305,14 +287,13 @@ void FakeESOut::checkTimestampsStart(mtime_t i_start)
>       if( i_start == VLC_TS_INVALID )
>           return;
>   
> -    vlc_mutex_lock(&lock);
> +    vlc_mutex_locker locker(&lock);
>       if( !timestamps_check_done )
>       {
>           if( i_start < CLOCK_FREQ ) /* Starts 0 */
>               timestamps_offset = timestamps_expected;
>           timestamps_check_done = true;
>       }
> -    vlc_mutex_unlock(&lock);
>   }
>   
>   int FakeESOut::esOutSend_Callback(es_out_t *fakees, es_out_id_t *p_es, block_t *p_block)





More information about the vlc-devel mailing list