Merge pull request #961 from diizy/master-atomic

Improve RT-safety by eliminating some global locks

include/InstrumentTrack.h

@@ -228,6 +228,8 @@ private:
 	MidiPort m_midiPort;
 
 	NotePlayHandle* m_notes[NumKeys];
+	QMutex m_notesMutex;
+
 	int m_runningMidiNotes[NumKeys];
 	bool m_sustainPedalPressed;
 

include/Mixer.h

@@ -211,9 +211,9 @@ public:
 	{
 		if( criticalXRuns() == false )
 		{
-			lock();
-			m_playHandles.push_back( handle );
-			unlock();
+			m_playHandleMutex.lock();
+			m_newPlayHandles.append( handle );
+			m_playHandleMutex.unlock();
 			return true;
 		}
 
@@ -428,6 +428,8 @@ private:
 	int m_numWorkers;
 	QWaitCondition m_queueReadyWaitCond;
 
+	PlayHandleList m_newPlayHandles;	// place where new playhandles are added temporarily
+	QMutex m_playHandleMutex;			// mutex used only for adding playhandles
 
 	PlayHandleList m_playHandles;
 	ConstPlayHandleList m_playHandlesToRemove;

src/core/Mixer.cpp

@@ -337,7 +337,6 @@ 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();
@@ -375,6 +374,11 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
 	// create play-handles for new notes, samples etc.
 	engine::getSong()->processNextBuffer();
 
+	// add all play-handles that have to be added
+	m_playHandleMutex.lock();
+	m_playHandles += m_newPlayHandles;
+	m_newPlayHandles.clear();
+	m_playHandleMutex.unlock();
 
 	// STAGE 1: run and render all play handles
 	MixerWorkerThread::fillJobQueue<PlayHandleList>( m_playHandles );

src/gui/TrackContainerView.cpp

@@ -240,9 +240,7 @@ void TrackContainerView::deleteTrackView( trackView * _tv )
 	removeTrackView( _tv );
 	delete _tv;
 
-	engine::mixer()->lock();
 	delete t;
-	engine::mixer()->unlock();
 }
 
 
@@ -326,7 +324,6 @@ void TrackContainerView::dropEvent( QDropEvent * _de )
 {
 	QString type = stringPairDrag::decodeKey( _de );
 	QString value = stringPairDrag::decodeValue( _de );
-	engine::mixer()->lock();
 	if( type == "instrument" )
 	{
 		InstrumentTrack * it = dynamic_cast<InstrumentTrack *>(
@@ -371,7 +368,6 @@ void TrackContainerView::dropEvent( QDropEvent * _de )
 		track::create( dataFile.content().firstChild().toElement(), m_tc );
 		_de->accept();
 	}
-	engine::mixer()->unlock();
 }
 
 

src/gui/file_browser.cpp

@@ -615,12 +615,12 @@ void fileBrowserTreeWidget::activateListItem( QTreeWidgetItem * _item,
 	}
 	else if( f->handling() != fileItem::NotSupported )
 	{
-		engine::mixer()->lock();
+//		engine::mixer()->lock();
 		InstrumentTrack * it = dynamic_cast<InstrumentTrack *>(
 				track::create( track::InstrumentTrack,
 					engine::getBBTrackContainer() ) );
 		handleFile( f, it );
-		engine::mixer()->unlock();
+//		engine::mixer()->unlock();
 	}
 }
 
@@ -632,11 +632,11 @@ void fileBrowserTreeWidget::openInNewInstrumentTrack( TrackContainer* tc )
 	if( m_contextMenuItem->handling() == fileItem::LoadAsPreset ||
 		m_contextMenuItem->handling() == fileItem::LoadByPlugin )
 	{
-		engine::mixer()->lock();
+//		engine::mixer()->lock();
 		InstrumentTrack * it = dynamic_cast<InstrumentTrack *>(
 				track::create( track::InstrumentTrack, tc ) );
 		handleFile( m_contextMenuItem, it );
-		engine::mixer()->unlock();
+//		engine::mixer()->unlock();
 	}
 }
 

src/gui/widgets/visualization_widget.cpp

@@ -74,12 +74,10 @@ void visualizationWidget::updateAudioBuffer()
 {
 	if( !engine::getSong()->isExporting() )
 	{
-		engine::mixer()->lock();
 		const surroundSampleFrame * c = engine::mixer()->
 							currentReadBuffer();
 		const fpp_t fpp = engine::mixer()->framesPerPeriod();
 		memcpy( m_buffer, c, sizeof( surroundSampleFrame ) * fpp );
-		engine::mixer()->unlock();
 	}
 }
 

src/tracks/InstrumentTrack.cpp

@@ -264,7 +264,6 @@ MidiEvent InstrumentTrack::applyMasterKey( const MidiEvent& event )
 
 void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& time, f_cnt_t offset )
 {
-	engine::mixer()->lock();
 	bool eventHandled = false;
 
 	switch( event.type() )
@@ -275,20 +274,23 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti
 		case MidiNoteOn:
 			if( event.velocity() > 0 )
 			{
+				NotePlayHandle* nph;
 				if( m_notes[event.key()] == NULL )
 				{
-					// create (timed) note-play-handle
-					NotePlayHandle* nph = new NotePlayHandle( this, offset,
-																typeInfo<f_cnt_t>::max() / 2,
-																note( MidiTime(), MidiTime(), event.key(), event.volume( midiPort()->baseVelocity() ) ),
-																NULL, event.channel(),
-																NotePlayHandle::OriginMidiInput );
-					if( engine::mixer()->addPlayHandle( nph ) )
+					m_notesMutex.lock();
+					nph = new NotePlayHandle( this, offset,
+								typeInfo<f_cnt_t>::max() / 2,
+								note( MidiTime(), MidiTime(), event.key(), event.volume( midiPort()->baseVelocity() ) ),
+								NULL, event.channel(),
+								NotePlayHandle::OriginMidiInput );
+					m_notes[event.key()] = nph;
+					if( ! engine::mixer()->addPlayHandle( nph ) )
 					{
-						m_notes[event.key()] = nph;
+						m_notes[event.key()] = NULL;
+						delete nph;
 					}
+					m_notesMutex.unlock();
 				}
-
 				eventHandled = true;
 				break;
 			}
@@ -296,10 +298,12 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti
 		case MidiNoteOff:
 			if( m_notes[event.key()] != NULL )
 			{
+				m_notesMutex.lock();
 				// do actual note off and remove internal reference to NotePlayHandle (which itself will
 				// be deleted later automatically)
 				m_notes[event.key()]->noteOff( offset );
 				m_notes[event.key()] = NULL;
+				m_notesMutex.unlock();
 			}
 			eventHandled = true;
 			break;
@@ -369,7 +373,6 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti
 		qWarning( "InstrumentTrack: unhandled MIDI event %d", event.type() );
 	}
 
-	engine::mixer()->unlock();
 }
 
 
@@ -428,13 +431,15 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t
 
 void InstrumentTrack::silenceAllNotes( bool removeIPH )
 {
-	engine::mixer()->lock();
+	m_notesMutex.lock();
 	for( int i = 0; i < NumKeys; ++i )
 	{
 		m_notes[i] = NULL;
 		m_runningMidiNotes[i] = 0;
 	}
+	m_notesMutex.unlock();
 
+	engine::mixer()->lock();
 	// invalidate all NotePlayHandles linked to this track
 	m_processHandles.clear();
 	engine::mixer()->removePlayHandles( this, removeIPH );

src/tracks/pattern.cpp

@@ -174,7 +174,6 @@ note * pattern::addNote( const note & _new_note, const bool _quant_pos )
 		new_note->quantizePos( engine::pianoRoll()->quantization() );
 	}
 
-	engine::mixer()->lock();
 	if( m_notes.size() == 0 || m_notes.back()->pos() <= new_note->pos() )
 	{
 		m_notes.push_back( new_note );
@@ -197,7 +196,6 @@ note * pattern::addNote( const note & _new_note, const bool _quant_pos )
 
 		m_notes.insert( it, new_note );
 	}
-	engine::mixer()->unlock();
 
 	checkType();
 	changeLength( length() );