From c7098d7b975e14015a1af9380963411bba38bbee Mon Sep 17 00:00:00 2001 From: Hyunin Song Date: Mon, 23 Oct 2017 23:09:11 +0900 Subject: [PATCH 1/4] Fix crash when closing while previewing preset --- src/core/PresetPreviewPlayHandle.cpp | 2 +- src/tracks/InstrumentTrack.cpp | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/core/PresetPreviewPlayHandle.cpp b/src/core/PresetPreviewPlayHandle.cpp index dd57e9f9c8e..452b4a319d7 100644 --- a/src/core/PresetPreviewPlayHandle.cpp +++ b/src/core/PresetPreviewPlayHandle.cpp @@ -228,7 +228,7 @@ bool PresetPreviewPlayHandle::isFinished() const bool PresetPreviewPlayHandle::isFromTrack( const Track * _track ) const { - return s_previewTC->previewInstrumentTrack() == _track; + return s_previewTC && s_previewTC->previewInstrumentTrack() == _track; } diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 6aaf3eb75e9..90dbf11a684 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -442,12 +442,15 @@ void InstrumentTrack::silenceAllNotes( bool removeIPH ) m_midiNotesMutex.unlock(); lock(); - // invalidate all NotePlayHandles linked to this track + // invalidate all NotePlayHandles and PresetPreviewHandles linked to this track m_processHandles.clear(); - Engine::mixer()->removePlayHandlesOfTypes( this, removeIPH - ? PlayHandle::TypeNotePlayHandle - | PlayHandle::TypeInstrumentPlayHandle - : PlayHandle::TypeNotePlayHandle ); + + quint8 flags = PlayHandle::TypeNotePlayHandle | PlayHandle::TypePresetPreviewHandle; + if( removeIPH ) + { + flags |= PlayHandle::TypeInstrumentPlayHandle; + } + Engine::mixer()->removePlayHandlesOfTypes( this, flags ); unlock(); } From 821b75f4ad43a88243ca327c76d0d9eb9b7bedc0 Mon Sep 17 00:00:00 2001 From: Hyunin Song Date: Wed, 25 Oct 2017 23:22:23 +0900 Subject: [PATCH 2/4] Fix deadlock on previewing presets while playing arpeggio --- src/core/PresetPreviewPlayHandle.cpp | 33 ++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/core/PresetPreviewPlayHandle.cpp b/src/core/PresetPreviewPlayHandle.cpp index 452b4a319d7..8a28ec9288a 100644 --- a/src/core/PresetPreviewPlayHandle.cpp +++ b/src/core/PresetPreviewPlayHandle.cpp @@ -84,6 +84,16 @@ class PreviewTrackContainer : public TrackContainer m_dataMutex.unlock(); } + void lockNote() + { + m_noteMutex.lock(); + } + + void unlockNote() + { + m_noteMutex.unlock(); + } + bool isPreviewing() { bool ret = !m_dataMutex.tryLock(); @@ -99,6 +109,7 @@ class PreviewTrackContainer : public TrackContainer InstrumentTrack* m_previewInstrumentTrack; NotePlayHandle* m_previewNote; QMutex m_dataMutex; + QMutex m_noteMutex; friend class PresetPreviewPlayHandle; @@ -113,15 +124,17 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, PlayHandle( TypePresetPreviewHandle ), m_previewNote( NULL ) { - s_previewTC->lockData(); - setUsesBuffer( false ); + s_previewTC->lockData(); + Engine::mixer()->requestChangeInModel(); + s_previewTC->lockNote(); if( s_previewTC->previewNote() != NULL ) { s_previewTC->previewNote()->mute(); } - + s_previewTC->unlockNote(); + Engine::mixer()->doneChangeInModel(); const bool j = Engine::projectJournal()->isJournalling(); Engine::projectJournal()->setJournalling( false ); @@ -174,6 +187,8 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, s_previewTC->previewInstrumentTrack()-> midiPort()->setMode( MidiPort::Disabled ); + Engine::mixer()->requestChangeInModel(); + s_previewTC->lockNote(); // create note-play-handle for it m_previewNote = NotePlayHandleManager::acquire( s_previewTC->previewInstrumentTrack(), 0, @@ -186,6 +201,8 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, Engine::mixer()->addPlayHandle( m_previewNote ); + s_previewTC->unlockNote(); + Engine::mixer()->doneChangeInModel(); s_previewTC->unlockData(); Engine::projectJournal()->setJournalling( j ); } @@ -195,7 +212,8 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, PresetPreviewPlayHandle::~PresetPreviewPlayHandle() { - s_previewTC->lockData(); + Engine::mixer()->requestChangeInModel(); + s_previewTC->lockNote(); // not muted by other preset-preview-handle? if( !m_previewNote->isMuted() ) { @@ -203,7 +221,8 @@ PresetPreviewPlayHandle::~PresetPreviewPlayHandle() s_previewTC->setPreviewNote( NULL ); } m_previewNote->noteOff(); - s_previewTC->unlockData(); + s_previewTC->unlockNote(); + Engine::mixer()->doneChangeInModel(); } @@ -258,13 +277,13 @@ ConstNotePlayHandleList PresetPreviewPlayHandle::nphsOfInstrumentTrack( const InstrumentTrack * _it ) { ConstNotePlayHandleList cnphv; - s_previewTC->lockData(); + s_previewTC->lockNote(); if( s_previewTC->previewNote() != NULL && s_previewTC->previewNote()->instrumentTrack() == _it ) { cnphv.push_back( s_previewTC->previewNote() ); } - s_previewTC->unlockData(); + s_previewTC->unlockNote(); return cnphv; } From bdf4884d22e1d8d68d6d573a5e49aa58e8ac1c18 Mon Sep 17 00:00:00 2001 From: Hyunin Song Date: Sat, 28 Oct 2017 12:02:48 +0900 Subject: [PATCH 3/4] Use QAtomicPointer instead of lock functions --- src/core/PresetPreviewPlayHandle.cpp | 44 +++++++++------------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/core/PresetPreviewPlayHandle.cpp b/src/core/PresetPreviewPlayHandle.cpp index 8a28ec9288a..2b8ec0045ed 100644 --- a/src/core/PresetPreviewPlayHandle.cpp +++ b/src/core/PresetPreviewPlayHandle.cpp @@ -22,6 +22,7 @@ * */ +#include #include #include "PresetPreviewPlayHandle.h" @@ -66,12 +67,17 @@ class PreviewTrackContainer : public TrackContainer NotePlayHandle* previewNote() { - return m_previewNote; + return m_previewNote.loadAcquire(); } void setPreviewNote( NotePlayHandle * _note ) { - m_previewNote = _note; + m_previewNote.storeRelease( _note ); + } + + bool testAndSetPreviewNote( NotePlayHandle * expectedVal, NotePlayHandle * newVal ) + { + return m_previewNote.testAndSetOrdered( expectedVal, newVal ); } void lockData() @@ -84,16 +90,6 @@ class PreviewTrackContainer : public TrackContainer m_dataMutex.unlock(); } - void lockNote() - { - m_noteMutex.lock(); - } - - void unlockNote() - { - m_noteMutex.unlock(); - } - bool isPreviewing() { bool ret = !m_dataMutex.tryLock(); @@ -107,9 +103,8 @@ class PreviewTrackContainer : public TrackContainer private: InstrumentTrack* m_previewInstrumentTrack; - NotePlayHandle* m_previewNote; + QAtomicPointer m_previewNote; QMutex m_dataMutex; - QMutex m_noteMutex; friend class PresetPreviewPlayHandle; @@ -127,13 +122,10 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, setUsesBuffer( false ); s_previewTC->lockData(); + Engine::mixer()->requestChangeInModel(); - s_previewTC->lockNote(); - if( s_previewTC->previewNote() != NULL ) - { - s_previewTC->previewNote()->mute(); - } - s_previewTC->unlockNote(); + s_previewTC->setPreviewNote( nullptr ); + s_previewTC->previewInstrumentTrack()->silenceAllNotes(); Engine::mixer()->doneChangeInModel(); const bool j = Engine::projectJournal()->isJournalling(); @@ -188,7 +180,6 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, midiPort()->setMode( MidiPort::Disabled ); Engine::mixer()->requestChangeInModel(); - s_previewTC->lockNote(); // create note-play-handle for it m_previewNote = NotePlayHandleManager::acquire( s_previewTC->previewInstrumentTrack(), 0, @@ -201,7 +192,6 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, Engine::mixer()->addPlayHandle( m_previewNote ); - s_previewTC->unlockNote(); Engine::mixer()->doneChangeInModel(); s_previewTC->unlockData(); Engine::projectJournal()->setJournalling( j ); @@ -213,15 +203,11 @@ PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, PresetPreviewPlayHandle::~PresetPreviewPlayHandle() { Engine::mixer()->requestChangeInModel(); - s_previewTC->lockNote(); // not muted by other preset-preview-handle? - if( !m_previewNote->isMuted() ) + if (s_previewTC->testAndSetPreviewNote(m_previewNote, nullptr)) { - // then set according state - s_previewTC->setPreviewNote( NULL ); + m_previewNote->noteOff(); } - m_previewNote->noteOff(); - s_previewTC->unlockNote(); Engine::mixer()->doneChangeInModel(); } @@ -277,13 +263,11 @@ ConstNotePlayHandleList PresetPreviewPlayHandle::nphsOfInstrumentTrack( const InstrumentTrack * _it ) { ConstNotePlayHandleList cnphv; - s_previewTC->lockNote(); if( s_previewTC->previewNote() != NULL && s_previewTC->previewNote()->instrumentTrack() == _it ) { cnphv.push_back( s_previewTC->previewNote() ); } - s_previewTC->unlockNote(); return cnphv; } From 2dadf418a2264305b5358f4ed4b0761b0426b156 Mon Sep 17 00:00:00 2001 From: Hyunin Song Date: Sat, 28 Oct 2017 12:19:37 +0900 Subject: [PATCH 4/4] Fix Qt4 compatibility --- src/core/PresetPreviewPlayHandle.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/PresetPreviewPlayHandle.cpp b/src/core/PresetPreviewPlayHandle.cpp index 2b8ec0045ed..b85d02474a3 100644 --- a/src/core/PresetPreviewPlayHandle.cpp +++ b/src/core/PresetPreviewPlayHandle.cpp @@ -67,12 +67,20 @@ class PreviewTrackContainer : public TrackContainer NotePlayHandle* previewNote() { + #if QT_VERSION >= 0x050000 return m_previewNote.loadAcquire(); + #else + return m_previewNote; + #endif } void setPreviewNote( NotePlayHandle * _note ) { + #if QT_VERSION >= 0x050000 m_previewNote.storeRelease( _note ); + #else + m_previewNote = _note; + #endif } bool testAndSetPreviewNote( NotePlayHandle * expectedVal, NotePlayHandle * newVal )