From 4eb486be1ee643e4fb92be8ec423a72142f5a330 Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 12:38:09 +0300 Subject: [PATCH 1/9] Attempt to remove mutex calls from instrumenttrack::processinevent --- include/InstrumentTrack.h | 5 ++++- src/tracks/InstrumentTrack.cpp | 20 ++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index ea10447c5..ec9c22d00 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -26,6 +26,9 @@ #ifndef INSTRUMENT_TRACK_H #define INSTRUMENT_TRACK_H +#include +#include + #include "AudioPort.h" #include "InstrumentFunctions.h" #include "InstrumentSoundShaping.h" @@ -227,7 +230,7 @@ private: AudioPort m_audioPort; MidiPort m_midiPort; - NotePlayHandle* m_notes[NumKeys]; + QAtomicPointer m_notes[NumKeys]; int m_runningMidiNotes[NumKeys]; bool m_sustainPedalPressed; diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index ea7f24315..2d6761167 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/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,17 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti case MidiNoteOn: if( event.velocity() > 0 ) { - if( m_notes[event.key()] == NULL ) - { - // create (timed) note-play-handle - NotePlayHandle* nph = new NotePlayHandle( this, offset, + NotePlayHandle* nph; + m_notes[event.key()].testAndSetOrdered( NULL, ( nph = new NotePlayHandle( this, offset, typeInfo::max() / 2, note( MidiTime(), MidiTime(), event.key(), event.volume( midiPort()->baseVelocity() ) ), NULL, event.channel(), - NotePlayHandle::OriginMidiInput ); - if( engine::mixer()->addPlayHandle( nph ) ) - { - m_notes[event.key()] = nph; - } + NotePlayHandle::OriginMidiInput ) ) ); + if( ! engine::mixer()->addPlayHandle( nph ) ) + { + m_notes[event.key()].testAndSetOrdered( nph, NULL ); } - + qDebug( "ok" ); eventHandled = true; break; } @@ -369,7 +365,6 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti qWarning( "InstrumentTrack: unhandled MIDI event %d", event.type() ); } - engine::mixer()->unlock(); } @@ -393,6 +388,7 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t if( key >= 0 && key < NumKeys ) { + if( m_runningMidiNotes[key] > 0 ) { m_instrument->handleMidiEvent( MidiEvent( MidiNoteOff, midiPort()->realOutputChannel(), key, 0 ), time, offset ); From f33d1f4972b08234d372130be2b6633bfdf85f8d Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 19:18:17 +0300 Subject: [PATCH 2/9] Instrument track, mixer... --- include/InstrumentTrack.h | 4 +++- include/Mixer.h | 8 +++++--- src/core/Mixer.cpp | 5 +++++ src/tracks/InstrumentTrack.cpp | 33 +++++++++++++++++++++++---------- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index ec9c22d00..cc1ef4ae5 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -230,7 +230,9 @@ private: AudioPort m_audioPort; MidiPort m_midiPort; - QAtomicPointer m_notes[NumKeys]; + NotePlayHandle* m_notes[NumKeys]; + QMutex m_notesMutex; + int m_runningMidiNotes[NumKeys]; bool m_sustainPedalPressed; diff --git a/include/Mixer.h b/include/Mixer.h index c10bbffbc..b52842ea3 100644 --- a/include/Mixer.h +++ b/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; diff --git a/src/core/Mixer.cpp b/src/core/Mixer.cpp index 928129271..4e53e8388 100644 --- a/src/core/Mixer.cpp +++ b/src/core/Mixer.cpp @@ -341,6 +341,11 @@ const surroundSampleFrame * Mixer::renderNextBuffer() // now we have to make sure no other thread does anything bad // while we're acting... lock(); + // add all play-handles that have to be added + m_playHandleMutex.lock(); + m_playHandles += m_newPlayHandles; + m_newPlayHandles.clear(); + m_playHandleMutex.unlock(); // remove all play-handles that have to be deleted and delete // them if they still exist... diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 2d6761167..01d337dae 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -264,6 +264,7 @@ MidiEvent InstrumentTrack::applyMasterKey( const MidiEvent& event ) void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& time, f_cnt_t offset ) { + qDebug( "pIE" ); bool eventHandled = false; switch( event.type() ) @@ -275,16 +276,23 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti if( event.velocity() > 0 ) { NotePlayHandle* nph; - m_notes[event.key()].testAndSetOrdered( NULL, ( nph = new NotePlayHandle( this, offset, - typeInfo::max() / 2, - note( MidiTime(), MidiTime(), event.key(), event.volume( midiPort()->baseVelocity() ) ), - NULL, event.channel(), - NotePlayHandle::OriginMidiInput ) ) ); - if( ! engine::mixer()->addPlayHandle( nph ) ) + if( m_notes[event.key()] == NULL ) { - m_notes[event.key()].testAndSetOrdered( nph, NULL ); + qDebug( "note on" ); + m_notesMutex.lock(); + nph = new NotePlayHandle( this, offset, + typeInfo::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()] = NULL; + delete nph; + } + m_notesMutex.unlock(); } - qDebug( "ok" ); eventHandled = true; break; } @@ -292,10 +300,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; @@ -372,6 +382,7 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& time, f_cnt_t offset ) { + qDebug( "pOE" ); // do nothing if we do not have an instrument instance (e.g. when loading settings) if( m_instrument == NULL ) { @@ -388,7 +399,7 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t if( key >= 0 && key < NumKeys ) { - + qDebug( "poe noteon" ); if( m_runningMidiNotes[key] > 0 ) { m_instrument->handleMidiEvent( MidiEvent( MidiNoteOff, midiPort()->realOutputChannel(), key, 0 ), time, offset ); @@ -424,13 +435,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 ); From ae5e0c32020fb07f2608701f0bff110a22f35920 Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 20:45:19 +0300 Subject: [PATCH 3/9] Rebase on master to fix older bugs --- src/tracks/InstrumentTrack.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 01d337dae..cd456d567 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/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 ) { - qDebug( "pIE" ); bool eventHandled = false; switch( event.type() ) @@ -278,7 +277,6 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti NotePlayHandle* nph; if( m_notes[event.key()] == NULL ) { - qDebug( "note on" ); m_notesMutex.lock(); nph = new NotePlayHandle( this, offset, typeInfo::max() / 2, @@ -382,7 +380,6 @@ void InstrumentTrack::processInEvent( const MidiEvent& event, const MidiTime& ti void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& time, f_cnt_t offset ) { - qDebug( "pOE" ); // do nothing if we do not have an instrument instance (e.g. when loading settings) if( m_instrument == NULL ) { @@ -399,7 +396,6 @@ void InstrumentTrack::processOutEvent( const MidiEvent& event, const MidiTime& t if( key >= 0 && key < NumKeys ) { - qDebug( "poe noteon" ); if( m_runningMidiNotes[key] > 0 ) { m_instrument->handleMidiEvent( MidiEvent( MidiNoteOff, midiPort()->realOutputChannel(), key, 0 ), time, offset ); From a4c4ea90dcdb841dd5048dc2ef2a059e485f375a Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:02:50 +0300 Subject: [PATCH 4/9] Pattern.cpp: Remove unnecessary global locks (improve rt safety) --- src/tracks/pattern.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tracks/pattern.cpp b/src/tracks/pattern.cpp index d11a7b980..c6a30f5a5 100644 --- a/src/tracks/pattern.cpp +++ b/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() ); From 4be118162f4bb54af3218bea53c3a01296cc9ede Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:14:47 +0300 Subject: [PATCH 5/9] TrackContainerView: remove unnecessary global locks (improve rt safety) --- src/gui/TrackContainerView.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/gui/TrackContainerView.cpp b/src/gui/TrackContainerView.cpp index 21441d247..c2d905e6d 100644 --- a/src/gui/TrackContainerView.cpp +++ b/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( @@ -371,7 +368,6 @@ void TrackContainerView::dropEvent( QDropEvent * _de ) track::create( dataFile.content().firstChild().toElement(), m_tc ); _de->accept(); } - engine::mixer()->unlock(); } From 4d321516e991ed5c4593a2fc63d0c57d3aac3bf1 Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:20:22 +0300 Subject: [PATCH 6/9] file_browser: remove unnecessary global locks --- src/gui/file_browser.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gui/file_browser.cpp b/src/gui/file_browser.cpp index 87714ea89..826d30199 100644 --- a/src/gui/file_browser.cpp +++ b/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( 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( track::create( track::InstrumentTrack, tc ) ); handleFile( m_contextMenuItem, it ); - engine::mixer()->unlock(); +// engine::mixer()->unlock(); } } From b8d4ee3047b750294110667f87e4e3c90227ecea Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:22:46 +0300 Subject: [PATCH 7/9] visualization_widget: remove unnecessary global locks --- src/gui/widgets/visualization_widget.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/gui/widgets/visualization_widget.cpp b/src/gui/widgets/visualization_widget.cpp index 2f6eebe95..fd0afbd84 100644 --- a/src/gui/widgets/visualization_widget.cpp +++ b/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(); } } From e6582fcd17812fc57f1bf4e818aa42ae7c1ca104 Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:24:10 +0300 Subject: [PATCH 8/9] InstrumentTrack.h: Remove unneeded includes --- include/InstrumentTrack.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/InstrumentTrack.h b/include/InstrumentTrack.h index cc1ef4ae5..fe8fbead2 100644 --- a/include/InstrumentTrack.h +++ b/include/InstrumentTrack.h @@ -26,9 +26,6 @@ #ifndef INSTRUMENT_TRACK_H #define INSTRUMENT_TRACK_H -#include -#include - #include "AudioPort.h" #include "InstrumentFunctions.h" #include "InstrumentSoundShaping.h" From cacb27d12f1b1d6f0f677b699032f31a8bb589f4 Mon Sep 17 00:00:00 2001 From: Vesa Date: Wed, 9 Jul 2014 21:39:04 +0300 Subject: [PATCH 9/9] Move playhandle-adding to the correct spot so playhandles won't get delayed --- src/core/Mixer.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/Mixer.cpp b/src/core/Mixer.cpp index 4e53e8388..bd97cf9a2 100644 --- a/src/core/Mixer.cpp +++ b/src/core/Mixer.cpp @@ -337,15 +337,9 @@ 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(); - // add all play-handles that have to be added - m_playHandleMutex.lock(); - m_playHandles += m_newPlayHandles; - m_newPlayHandles.clear(); - m_playHandleMutex.unlock(); // remove all play-handles that have to be deleted and delete // them if they still exist... @@ -380,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( m_playHandles );