[vlc-devel] [PATCH 1/4] demux:adaptive: use C++ vlc_mutex_locker
Zhao Zhili
quinkblack at foxmail.com
Mon Jan 8 13:30:46 CET 2018
On 2018年01月08日 20:25, Steve Lhomme wrote:
> Le 08/01/2018 à 13:20, Zhao Zhili a écrit :
>>
>>
>> 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.
>
> Which code block ?
> Anyway, using vlc_mutex_locker doesn't mean one doesn't need to check
> for lock boundaries. It just makes the code a little cleaner.
>
I mean add a pair of curly braces since the lock only work in a small
region:
- 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;
}
>>> 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)
>>
>>
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>
>
More information about the vlc-devel
mailing list