From 9e1cdd0441bca0697f582be3b85d8e2d4c152c72 Mon Sep 17 00:00:00 2001 From: Vesa Date: Sun, 3 Aug 2014 14:49:45 +0300 Subject: [PATCH] Start work on replacing/removing global locks --- include/EnvelopeAndLfoParameters.h | 4 +-- include/InstrumentTrack.h | 2 ++ include/Mixer.h | 13 +++++++- include/PlayHandle.h | 15 ++++++++- include/track.h | 15 ++++++++- src/core/EnvelopeAndLfoParameters.cpp | 3 -- src/core/Mixer.cpp | 20 +++++++----- src/core/NotePlayHandle.cpp | 45 ++++++++++++++++----------- src/core/TrackContainer.cpp | 2 ++ src/core/song.cpp | 12 +++---- src/core/track.cpp | 15 ++++----- src/tracks/InstrumentTrack.cpp | 32 ++++++++++++------- src/tracks/pattern.cpp | 10 +++--- 13 files changed, 123 insertions(+), 65 deletions(-) diff --git a/include/EnvelopeAndLfoParameters.h b/include/EnvelopeAndLfoParameters.h index 42641c791..221b97608 100644 --- a/include/EnvelopeAndLfoParameters.h +++ b/include/EnvelopeAndLfoParameters.h @@ -22,8 +22,8 @@ * */ -#ifndef _ENVELOPE_AND_LFO_PARAMETERS_H -#define _ENVELOPE_AND_LFO_PARAMETERS_H +#ifndef ENVELOPE_AND_LFO_PARAMETERS_H +#define ENVELOPE_AND_LFO_PARAMETERS_H #include diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index fe8fbead2..07b5dc0ea 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -231,6 +231,8 @@ private: QMutex m_notesMutex; int m_runningMidiNotes[NumKeys]; + QMutex m_midiNotesMutex; + bool m_sustainPedalPressed; bool m_silentBuffersProcessed; diff --git a/include/Mixer.h b/include/Mixer.h index b52842ea3..4813fb0ea 100644 --- a/include/Mixer.h +++ b/include/Mixer.h @@ -308,6 +308,16 @@ public: { m_inputFramesMutex.unlock(); } + + void lockPlayHandleRemoval() + { + m_playHandleRemovalMutex.lock(); + } + + void unlockPlayHandleRemoval() + { + m_playHandleRemovalMutex.unlock(); + } // audio-buffer-mgm void bufferToPort( const sampleFrame * _buf, @@ -449,7 +459,8 @@ private: QMutex m_globalMutex; QMutex m_inputFramesMutex; - + + QMutex m_playHandleRemovalMutex; fifo * m_fifo; fifoWriter * m_fifoWriter; diff --git a/include/PlayHandle.h b/include/PlayHandle.h index 152f82989..bbcde7bcc 100644 --- a/include/PlayHandle.h +++ b/include/PlayHandle.h @@ -27,6 +27,7 @@ #include #include +#include #include "ThreadableJob.h" #include "lmms_basics.h" @@ -84,7 +85,18 @@ public: return !isFinished(); } - + void lock() + { + m_processingLock.lock(); + } + void unlock() + { + m_processingLock.unlock(); + } + bool tryLock() + { + return m_processingLock.tryLock(); + } virtual void play( sampleFrame* buffer ) = 0; virtual bool isFinished( void ) const = 0; @@ -108,6 +120,7 @@ private: Type m_type; f_cnt_t m_offset; const QThread* m_affinity; + QMutex m_processingLock; } ; diff --git a/include/track.h b/include/track.h index cdc09e335..6e54f3935 100644 --- a/include/track.h +++ b/include/track.h @@ -119,7 +119,7 @@ public: { return m_length; } - + virtual void movePosition( const MidiTime & _pos ); virtual void changeLength( const MidiTime & _length ); @@ -508,6 +508,18 @@ public: m_height = _height; } + void lock() + { + m_processingLock.lock(); + } + void unlock() + { + m_processingLock.unlock(); + } + bool tryLock() + { + return m_processingLock.tryLock(); + } public slots: virtual void setName( const QString & _new_name ) @@ -533,6 +545,7 @@ private: tcoVector m_trackContentObjects; + QMutex m_processingLock; friend class trackView; diff --git a/src/core/EnvelopeAndLfoParameters.cpp b/src/core/EnvelopeAndLfoParameters.cpp index 066c9132a..e1dcaeb3d 100644 --- a/src/core/EnvelopeAndLfoParameters.cpp +++ b/src/core/EnvelopeAndLfoParameters.cpp @@ -399,8 +399,6 @@ void EnvelopeAndLfoParameters::loadSettings( const QDomElement & _this ) void EnvelopeAndLfoParameters::updateSampleVars() { - engine::mixer()->lock(); - const float frames_per_env_seg = SECS_PER_ENV_SEGMENT * engine::mixer()->processingSampleRate(); // TODO: Remove the expKnobVals, time should be linear @@ -523,7 +521,6 @@ void EnvelopeAndLfoParameters::updateSampleVars() emit dataChanged(); - engine::mixer()->unlock(); } diff --git a/src/core/Mixer.cpp b/src/core/Mixer.cpp index b882b1b46..6bb3198c2 100644 --- a/src/core/Mixer.cpp +++ b/src/core/Mixer.cpp @@ -337,13 +337,10 @@ const surroundSampleFrame * Mixer::renderNextBuffer() m_inputBufferFrames[ m_inputBufferWrite ] = 0; unlockInputFrames(); - // now we have to make sure no other thread does anything bad - // while we're acting... - lock(); - // remove all play-handles that have to be deleted and delete // them if they still exist... // maybe this algorithm could be optimized... + lockPlayHandleRemoval(); ConstPlayHandleList::Iterator it_rem = m_playHandlesToRemove.begin(); while( it_rem != m_playHandlesToRemove.end() ) { @@ -357,6 +354,11 @@ const surroundSampleFrame * Mixer::renderNextBuffer() it_rem = m_playHandlesToRemove.erase( it_rem ); } + unlockPlayHandleRemoval(); + + // now we have to make sure no other thread does anything bad + // while we're acting... + lock(); // rotate buffers m_writeBuffer = ( m_writeBuffer + 1 ) % m_poolDepth; @@ -381,6 +383,7 @@ const surroundSampleFrame * Mixer::renderNextBuffer() m_playHandleMutex.unlock(); // STAGE 1: run and render all play handles + lockPlayHandleRemoval(); MixerWorkerThread::fillJobQueue( m_playHandles ); MixerWorkerThread::startAndWaitForJobs(); @@ -404,6 +407,7 @@ const surroundSampleFrame * Mixer::renderNextBuffer() ++it; } } + unlockPlayHandleRemoval(); // STAGE 2: process effects of all instrument- and sampletracks @@ -648,12 +652,12 @@ void Mixer::removeAudioPort( AudioPort * _port ) void Mixer::removePlayHandle( PlayHandle * _ph ) { - lock(); // check thread affinity as we must not delete play-handles // which were created in a thread different than mixer thread if( _ph->affinityMatters() && _ph->affinity() == QThread::currentThread() ) { + lockPlayHandleRemoval(); PlayHandleList::Iterator it = qFind( m_playHandles.begin(), m_playHandles.end(), _ph ); @@ -662,12 +666,12 @@ void Mixer::removePlayHandle( PlayHandle * _ph ) m_playHandles.erase( it ); delete _ph; } + unlockPlayHandleRemoval(); } else { m_playHandlesToRemove.push_back( _ph ); } - unlock(); } @@ -675,7 +679,7 @@ void Mixer::removePlayHandle( PlayHandle * _ph ) void Mixer::removePlayHandles( track * _track, bool removeIPHs ) { - lock(); + lockPlayHandleRemoval(); PlayHandleList::Iterator it = m_playHandles.begin(); while( it != m_playHandles.end() ) { @@ -689,7 +693,7 @@ void Mixer::removePlayHandles( track * _track, bool removeIPHs ) ++it; } } - unlock(); + unlockPlayHandleRemoval(); } diff --git a/src/core/NotePlayHandle.cpp b/src/core/NotePlayHandle.cpp index d43148862..d984abef0 100644 --- a/src/core/NotePlayHandle.cpp +++ b/src/core/NotePlayHandle.cpp @@ -94,23 +94,6 @@ NotePlayHandle::NotePlayHandle( InstrumentTrack* instrumentTrack, updateFrequency(); setFrames( _frames ); - - // inform attached components about new MIDI note (used for recording in Piano Roll) - if( m_origin == OriginMidiInput ) - { - m_instrumentTrack->midiNoteOn( *this ); - } - - if( hasParent() || ! m_instrumentTrack->isArpeggioEnabled() ) - { - const int baseVelocity = m_instrumentTrack->midiPort()->baseVelocity(); - - // send MidiNoteOn event - m_instrumentTrack->processOutEvent( - MidiEvent( MidiNoteOn, midiChannel(), midiKey(), midiVelocity( baseVelocity ) ), - MidiTime::fromFrames( offset(), engine::framesPerTick() ), - offset() ); - } } @@ -118,6 +101,7 @@ NotePlayHandle::NotePlayHandle( InstrumentTrack* instrumentTrack, NotePlayHandle::~NotePlayHandle() { + lock(); noteOff( 0 ); if( hasParent() == false ) @@ -143,6 +127,7 @@ NotePlayHandle::~NotePlayHandle() m_subNotes.clear(); delete m_filter; + unlock(); } @@ -195,6 +180,28 @@ void NotePlayHandle::play( sampleFrame * _working_buffer ) return; } + lock(); + + if( m_totalFramesPlayed == 0 ) + { + // inform attached components about new MIDI note (used for recording in Piano Roll) + if( m_origin == OriginMidiInput ) + { + m_instrumentTrack->midiNoteOn( *this ); + } + + if( hasParent() || ! m_instrumentTrack->isArpeggioEnabled() ) + { + const int baseVelocity = m_instrumentTrack->midiPort()->baseVelocity(); + + // send MidiNoteOn event + m_instrumentTrack->processOutEvent( + MidiEvent( MidiNoteOn, midiChannel(), midiKey(), midiVelocity( baseVelocity ) ), + MidiTime::fromFrames( offset(), engine::framesPerTick() ), + offset() ); + } + } + // number of frames that can be played this period f_cnt_t framesThisPeriod = m_totalFramesPlayed == 0 ? engine::mixer()->framesPerPeriod() - offset() @@ -295,6 +302,7 @@ void NotePlayHandle::play( sampleFrame * _working_buffer ) // update internal data m_totalFramesPlayed += framesThisPeriod; + unlock(); } @@ -346,6 +354,7 @@ void NotePlayHandle::noteOff( const f_cnt_t _s ) { return; } + m_released = true; // first note-off all sub-notes for( NotePlayHandleList::Iterator it = m_subNotes.begin(); it != m_subNotes.end(); ++it ) @@ -372,8 +381,6 @@ void NotePlayHandle::noteOff( const f_cnt_t _s ) setLength( MidiTime( static_cast( totalFramesPlayed() / engine::framesPerTick() ) ) ); m_instrumentTrack->midiNoteOff( *this ); } - - m_released = true; } diff --git a/src/core/TrackContainer.cpp b/src/core/TrackContainer.cpp index 744cac788..f67910637 100644 --- a/src/core/TrackContainer.cpp +++ b/src/core/TrackContainer.cpp @@ -160,9 +160,11 @@ void TrackContainer::addTrack( track * _track ) { if( _track->type() != track::HiddenAutomationTrack ) { + _track->lock(); m_tracksMutex.lockForWrite(); m_tracks.push_back( _track ); m_tracksMutex.unlock(); + _track->unlock(); emit trackAdded( _track ); } } diff --git a/src/core/song.cpp b/src/core/song.cpp index 961bcd9d7..5f1893cc8 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -138,8 +138,8 @@ void song::masterVolumeChanged() void song::setTempo() { + engine::mixer()->lockPlayHandleRemoval(); const bpm_t tempo = (bpm_t) m_tempoModel.value(); - engine::mixer()->lock(); PlayHandleList & playHandles = engine::mixer()->playHandles(); for( PlayHandleList::Iterator it = playHandles.begin(); it != playHandles.end(); ++it ) @@ -147,10 +147,12 @@ void song::setTempo() NotePlayHandle * nph = dynamic_cast( *it ); if( nph && !nph->isReleased() ) { + nph->lock(); nph->resize( tempo ); + nph->unlock(); } } - engine::mixer()->unlock(); + engine::mixer()->unlockPlayHandleRemoval(); engine::updateFramesPerTick(); @@ -670,10 +672,8 @@ void song::removeBar() void song::addBBTrack() { - engine::mixer()->lock(); track * t = track::create( track::BBTrack, this ); engine::getBBTrackContainer()->setCurrentBB( dynamic_cast( t )->index() ); - engine::mixer()->unlock(); } @@ -681,9 +681,7 @@ void song::addBBTrack() void song::addSampleTrack() { - engine::mixer()->lock(); (void) track::create( track::SampleTrack, this ); - engine::mixer()->unlock(); } @@ -691,9 +689,7 @@ void song::addSampleTrack() void song::addAutomationTrack() { - engine::mixer()->lock(); (void) track::create( track::AutomationTrack, this ); - engine::mixer()->unlock(); } diff --git a/src/core/track.cpp b/src/core/track.cpp index dffa1f683..be8a6e0c5 100644 --- a/src/core/track.cpp +++ b/src/core/track.cpp @@ -1697,18 +1697,17 @@ void trackOperationsWidget::paintEvent( QPaintEvent * _pe ) */ void trackOperationsWidget::cloneTrack() { - engine::mixer()->lock(); m_trackView->getTrack()->clone(); - engine::mixer()->unlock(); } /*! \brief Clear this track - clears all TCOs from the track */ void trackOperationsWidget::clearTrack() { - engine::mixer()->lock(); - m_trackView->getTrack()->deleteTCOs(); - engine::mixer()->unlock(); + track * t = m_trackView->getTrack(); + t->lock(); + t->deleteTCOs(); + t->unlock(); } @@ -1839,6 +1838,7 @@ track::track( TrackTypes _type, TrackContainer * _tc ) : */ track::~track() { + lock(); emit destroyedTrack(); while( !m_trackContentObjects.isEmpty() ) @@ -1847,6 +1847,7 @@ track::~track() } m_trackContainer->removeTrack( this ); + unlock(); } @@ -2534,9 +2535,9 @@ void trackView::dropEvent( QDropEvent * _de ) // value contains our XML-data so simply create a // DataFile which does the rest for us... DataFile dataFile( value.toUtf8() ); - engine::mixer()->lock(); + m_track->lock(); m_track->restoreState( dataFile.content().firstChild().toElement() ); - engine::mixer()->unlock(); + m_track->unlock(); _de->accept(); } } diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index cd456d567..c09c08256 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -157,7 +157,7 @@ InstrumentTrack::~InstrumentTrack() void InstrumentTrack::processAudioBuffer( sampleFrame* buf, const fpp_t frames, NotePlayHandle* n ) { // we must not play the sound if this InstrumentTrack is muted... - if( isMuted() || ( n && n->isBbTrackMuted() ) ) + if( isMuted() || ( n && n->isBbTrackMuted() ) || ! m_instrument ) { return; } @@ -392,6 +392,7 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t switch( event.type() ) { case MidiNoteOn: + m_midiNotesMutex.lock(); m_piano.setKeyState( event.key(), true ); // event.key() = original key if( key >= 0 && key < NumKeys ) @@ -402,19 +403,20 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t } ++m_runningMidiNotes[key]; m_instrument->handleMidiEvent( MidiEvent( MidiNoteOn, midiPort()->realOutputChannel(), key, event.velocity() ), time, offset ); - emit newNote(); } + m_midiNotesMutex.unlock(); break; case MidiNoteOff: + m_midiNotesMutex.lock(); m_piano.setKeyState( event.key(), false ); // event.key() = original key if( key >= 0 && key < NumKeys && --m_runningMidiNotes[key] <= 0 ) { - m_runningMidiNotes[key] = qMax( 0, m_runningMidiNotes[key] ); m_instrument->handleMidiEvent( MidiEvent( MidiNoteOff, midiPort()->realOutputChannel(), key, 0 ), time, offset ); } + m_midiNotesMutex.unlock(); break; default: @@ -432,18 +434,20 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t void InstrumentTrack::silenceAllNotes( bool removeIPH ) { m_notesMutex.lock(); + m_midiNotesMutex.lock(); for( int i = 0; i < NumKeys; ++i ) { m_notes[i] = NULL; m_runningMidiNotes[i] = 0; } m_notesMutex.unlock(); + m_midiNotesMutex.unlock(); - engine::mixer()->lock(); + lock(); // invalidate all NotePlayHandles linked to this track m_processHandles.clear(); engine::mixer()->removePlayHandles( this, removeIPH ); - engine::mixer()->unlock(); + unlock(); } @@ -532,13 +536,13 @@ void InstrumentTrack::setName( const QString & _new_name ) void InstrumentTrack::updateBaseNote() { - engine::mixer()->lock(); for( NotePlayHandleList::Iterator it = m_processHandles.begin(); it != m_processHandles.end(); ++it ) { + ( *it )->lock(); ( *it )->updateFrequency(); + ( *it )->unlock(); } - engine::mixer()->unlock(); } @@ -590,6 +594,10 @@ void InstrumentTrack::removeMidiPortNode( DataFile & _dataFile ) bool InstrumentTrack::play( const MidiTime & _start, const fpp_t _frames, const f_cnt_t _offset, int _tco_num ) { + if( ! tryLock() ) + { + return false; + } const float frames_per_tick = engine::framesPerTick(); tcoVector tcos; @@ -615,6 +623,7 @@ bool InstrumentTrack::play( const MidiTime & _start, const fpp_t _frames, if ( tcos.size() == 0 ) { + unlock(); return false; } @@ -678,6 +687,7 @@ bool InstrumentTrack::play( const MidiTime & _start, const fpp_t _frames, ++nit; } } + unlock(); return played_a_note; } @@ -731,7 +741,7 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement { silenceAllNotes( true ); - engine::mixer()->lock(); + lock(); m_volumeModel.loadSettings( thisElement, "vol" ); m_panningModel.loadSettings( thisElement, "pan" ); @@ -797,7 +807,7 @@ void InstrumentTrack::loadTrackSpecificSettings( const QDomElement & thisElement } node = node.nextSibling(); } - engine::mixer()->unlock(); + unlock(); } @@ -807,10 +817,10 @@ Instrument * InstrumentTrack::loadInstrument( const QString & _plugin_name ) { silenceAllNotes( true ); - engine::mixer()->lock(); + lock(); delete m_instrument; m_instrument = Instrument::instantiate( _plugin_name, this ); - engine::mixer()->unlock(); + unlock(); setName( m_instrument->displayName() ); emit instrumentChanged(); diff --git a/src/tracks/pattern.cpp b/src/tracks/pattern.cpp index 434379350..b88fef570 100644 --- a/src/tracks/pattern.cpp +++ b/src/tracks/pattern.cpp @@ -174,6 +174,7 @@ note * pattern::addNote( const note & _new_note, const bool _quant_pos ) new_note->quantizePos( engine::pianoRoll()->quantization() ); } + instrumentTrack()->lock(); if( m_notes.size() == 0 || m_notes.back()->pos() <= new_note->pos() ) { m_notes.push_back( new_note ); @@ -196,6 +197,7 @@ note * pattern::addNote( const note & _new_note, const bool _quant_pos ) m_notes.insert( it, new_note ); } + instrumentTrack()->unlock(); checkType(); changeLength( length() ); @@ -212,7 +214,7 @@ note * pattern::addNote( const note & _new_note, const bool _quant_pos ) void pattern::removeNote( const note * _note_to_del ) { - engine::mixer()->lock(); + instrumentTrack()->lock(); NoteVector::Iterator it = m_notes.begin(); while( it != m_notes.end() ) { @@ -224,7 +226,7 @@ void pattern::removeNote( const note * _note_to_del ) } ++it; } - engine::mixer()->unlock(); + instrumentTrack()->unlock(); checkType(); changeLength( length() ); @@ -274,14 +276,14 @@ void pattern::rearrangeAllNotes() void pattern::clearNotes() { - engine::mixer()->lock(); + instrumentTrack()->lock(); for( NoteVector::Iterator it = m_notes.begin(); it != m_notes.end(); ++it ) { delete *it; } m_notes.clear(); - engine::mixer()->unlock(); + instrumentTrack()->unlock(); checkType(); emit dataChanged();