From b217bbdfd86fe95b066310be0e5571dec6e6a7b3 Mon Sep 17 00:00:00 2001 From: Tobias Doerffel Date: Thu, 11 Sep 2008 13:43:43 +0000 Subject: [PATCH] * ensure correct thread affinity when deleting play handles - fixes crash when previewing samples and LMMS was linked against Qt 4.3.x * renamed destroyed()-signals for not conflicting with QObject::destroyed() in Qt 4.3 * made effectChain creation in audioPort optional * fixed various compiler warnings git-svn-id: https://lmms.svn.sf.net/svnroot/lmms/trunk/lmms@1602 0778d3d1-df1d-0410-868b-ea421aaaa00d --- ChangeLog | 20 ++++++++++++++++++++ include/audio_port.h | 9 +++++---- include/midi.h | 16 ++++++++-------- include/mixer.h | 27 +++++++++++++++++++++++---- include/play_handle.h | 15 ++++++++++++++- include/sample_play_handle.h | 6 ++++++ include/track.h | 4 ++-- src/core/audio/audio_port.cpp | 22 ++++++++++++---------- src/core/effect_chain.cpp | 2 +- src/core/mixer.cpp | 25 +++++++++++++++---------- src/core/sample_play_handle.cpp | 22 +++++++++++----------- src/core/track.cpp | 14 ++++++-------- 12 files changed, 123 insertions(+), 59 deletions(-) diff --git a/ChangeLog b/ChangeLog index cf108b194..94e4e5d4f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2008-09-11 Tobias Doerffel + + * include/audio_port.h: + * include/midi.h: + * include/mixer.h: + * include/play_handle.h: + * include/sample_play_handle.h: + * include/track.h: + * src/core/audio/audio_port.cpp: + * src/core/effect_chain.cpp: + * src/core/mixer.cpp: + * src/core/sample_play_handle.cpp: + * src/core/track.cpp: + - ensure correct thread affinity when deleting play handles - fixes + crash when previewing samples and LMMS was linked against Qt 4.3.x + - renamed destroyed()-signals for not conflicting with + QObject::destroyed() in Qt 4.3 + - made effectChain creation in audioPort optional + - fixed various compiler warnings + 2008-09-08 Paul Giblock * plugins/papu/gb_abu/Blip_Buffer.cpp: diff --git a/include/audio_port.h b/include/audio_port.h index 85e7e7f37..6ee29326d 100644 --- a/include/audio_port.h +++ b/include/audio_port.h @@ -30,13 +30,14 @@ #include #include -#include "effect_chain.h" +#include "mixer.h" +class effectChain; class audioPort { public: - audioPort( const QString & _name ); + audioPort( const QString & _name, bool _has_effect_chain = true ); ~audioPort(); inline sampleFrame * firstBuffer( void ) @@ -90,7 +91,7 @@ public: inline effectChain * getEffects( void ) { - return( &m_effects ); + return( m_effects ); } void setNextFxChannel( const fx_ch_t _chnl ) @@ -131,7 +132,7 @@ private: QString m_name; - effectChain m_effects; + effectChain * m_effects; friend class mixer; diff --git a/include/midi.h b/include/midi.h index 61e1736a9..cad10c60b 100644 --- a/include/midi.h +++ b/include/midi.h @@ -88,8 +88,8 @@ struct midiEvent { midiEvent( MidiEventTypes _type = MidiActiveSensing, Sint8 _channel = 0, - Uint16 _param1 = 0, - Uint16 _param2 = 0 ) : + Sint16 _param1 = 0, + Sint16 _param2 = 0 ) : m_type( _type ), m_channel( _channel ), m_sysExData( NULL ) @@ -119,22 +119,22 @@ struct midiEvent return m_channel; } - inline Uint16 key( void ) const + inline Sint16 key( void ) const { return( m_data.m_param[0] ); } - inline Uint16 & key( void ) + inline Sint16 & key( void ) { return( m_data.m_param[0] ); } - inline Uint16 velocity( void ) const + inline Sint16 velocity( void ) const { return( m_data.m_param[1] ); } - inline Uint16 & velocity( void ) + inline Sint16 & velocity( void ) { return( m_data.m_param[1] ); } @@ -149,9 +149,9 @@ struct midiEvent Sint8 m_channel; // MIDI channel union { - Uint16 m_param[2]; // first/second parameter (key/velocity) + Sint16 m_param[2]; // first/second parameter (key/velocity) Uint8 m_bytes[4]; // raw bytes - Uint32 m_sysExDataLen; // len of m_sysExData + Sint32 m_sysExDataLen; // len of m_sysExData } m_data; const char * m_sysExData; diff --git a/include/mixer.h b/include/mixer.h index 6792a5651..ffed5b9ae 100644 --- a/include/mixer.h +++ b/include/mixer.h @@ -243,11 +243,30 @@ public: return( FALSE ); } - inline void removePlayHandle( const playHandle * _ph ) + void removePlayHandle( playHandle * _ph ) { - lockPlayHandlesToRemove(); - m_playHandlesToRemove.push_back( _ph ); - unlockPlayHandlesToRemove(); + // 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() ) + { + lockPlayHandles(); + playHandleVector::iterator it = + qFind( m_playHandles.begin(), + m_playHandles.end(), _ph ); + if( it != m_playHandles.end() ) + { + m_playHandles.erase( it ); + } + unlockPlayHandles(); + delete _ph; + } + else + { + lockPlayHandlesToRemove(); + m_playHandlesToRemove.push_back( _ph ); + unlockPlayHandlesToRemove(); + } } inline playHandleVector & playHandles( void ) diff --git a/include/play_handle.h b/include/play_handle.h index 6b9df7d85..3b394b18c 100644 --- a/include/play_handle.h +++ b/include/play_handle.h @@ -27,6 +27,7 @@ #ifndef _PLAY_HANDLE_H #define _PLAY_HANDLE_H +#include #include #include "types.h" @@ -47,7 +48,8 @@ public: playHandle( const types _type, f_cnt_t _offset = 0 ) : m_type( _type ), - m_offset( _offset ) + m_offset( _offset ), + m_affinity( QThread::currentThread() ) { } @@ -55,6 +57,16 @@ public: { } + virtual inline bool affinityMatters( void ) const + { + return false; + } + + const QThread * affinity( void ) const + { + return( m_affinity ); + } + inline types type( void ) const { return( m_type ); @@ -92,6 +104,7 @@ public: private: types m_type; f_cnt_t m_offset; + QThread * m_affinity; } ; diff --git a/include/sample_play_handle.h b/include/sample_play_handle.h index 5bcea1d26..8ecec2b46 100644 --- a/include/sample_play_handle.h +++ b/include/sample_play_handle.h @@ -46,6 +46,12 @@ public: samplePlayHandle( pattern * _pattern ); virtual ~samplePlayHandle(); + virtual inline bool affinityMatters( void ) const + { + return true; + } + + virtual void play( bool _try_parallelizing, sampleFrame * _working_buffer ); virtual bool done( void ) const; diff --git a/include/track.h b/include/track.h index 7a5d02d77..bea4172e0 100644 --- a/include/track.h +++ b/include/track.h @@ -126,7 +126,7 @@ protected: signals: void lengthChanged( void ); void positionChanged( void ); - void destroyed( void ); + void destroyedTCO( void ); private: @@ -463,7 +463,7 @@ private: signals: - void destroyed( void ); + void destroyedTrack( void ); void nameChanged( void ); void trackContentObjectAdded( trackContentObject * ); diff --git a/src/core/audio/audio_port.cpp b/src/core/audio/audio_port.cpp index f2ec583ec..206a8b2f9 100644 --- a/src/core/audio/audio_port.cpp +++ b/src/core/audio/audio_port.cpp @@ -24,16 +24,13 @@ * */ -#include "mixer.h" -#include "audio_device.h" -#include "config_mgr.h" - #include "audio_port.h" #include "audio_device.h" +#include "effect_chain.h" #include "engine.h" -audioPort::audioPort( const QString & _name ) : +audioPort::audioPort( const QString & _name, bool _has_effect_chain ) : m_bufferUsage( NoUsage ), m_firstBuffer( new sampleFrame[engine::getMixer()->framesPerPeriod()] ), m_secondBuffer( new sampleFrame[ @@ -41,7 +38,7 @@ audioPort::audioPort( const QString & _name ) : m_extOutputEnabled( FALSE ), m_nextFxChannel( 0 ), m_name( "unnamed port" ), - m_effects( NULL ) + m_effects( _has_effect_chain ? new effectChain( NULL ) : NULL ) { engine::getMixer()->clearAudioBuffer( m_firstBuffer, engine::getMixer()->framesPerPeriod() ); @@ -60,6 +57,7 @@ audioPort::~audioPort() engine::getMixer()->removeAudioPort( this ); delete[] m_firstBuffer; delete[] m_secondBuffer; + delete m_effects; } @@ -113,11 +111,15 @@ void audioPort::setName( const QString & _name ) bool audioPort::processEffects( void ) { - lockFirstBuffer(); - bool more = m_effects.processAudioBuffer( m_firstBuffer, + if( m_effects ) + { + lockFirstBuffer(); + bool more = m_effects->processAudioBuffer( m_firstBuffer, engine::getMixer()->framesPerPeriod() ); - unlockFirstBuffer(); - return( more ); + unlockFirstBuffer(); + return( more ); + } + return false; } diff --git a/src/core/effect_chain.cpp b/src/core/effect_chain.cpp index 877ab7970..e95c8e1b7 100644 --- a/src/core/effect_chain.cpp +++ b/src/core/effect_chain.cpp @@ -38,7 +38,7 @@ effectChain::effectChain( model * _parent ) : model( _parent ), - m_enabledModel( FALSE, this, tr( "Effects enabled" ) ) + m_enabledModel( FALSE, NULL, tr( "Effects enabled" ) ) { } diff --git a/src/core/mixer.cpp b/src/core/mixer.cpp index 0d15e19c1..d5998873c 100644 --- a/src/core/mixer.cpp +++ b/src/core/mixer.cpp @@ -567,7 +567,7 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) m_playHandles.erase( it ); } - m_playHandlesToRemove.erase( it_rem ); + it_rem = m_playHandlesToRemove.erase( it_rem ); } unlockPlayHandlesToRemove(); unlockPlayHandles(); @@ -636,22 +636,27 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) } } } - idx = 0; - while( idx < m_playHandles.size() ) + for( playHandleVector::iterator it = m_playHandles.begin(); + it != m_playHandles.end(); ) { - playHandle * n = m_playHandles[idx]; - if( n->done() ) + if( ( *it )->affinityMatters() && + ( *it )->affinity() != QThread::currentThread() ) { - delete n; - m_playHandles.erase( - m_playHandles.begin() + idx ); + ++it; + continue; + } + if( ( *it )->done() ) + { + delete *it; + it = m_playHandles.erase( it ); } else { - ++idx; + ++it; } } unlockPlayHandles(); + if( m_multiThreaded ) { mixerWorkerThread::jobQueue jq; @@ -951,7 +956,7 @@ void mixer::removePlayHandles( track * _track ) if( ( *it )->isFromTrack( _track ) ) { delete *it; - m_playHandles.erase( it ); + it = m_playHandles.erase( it ); } else { diff --git a/src/core/sample_play_handle.cpp b/src/core/sample_play_handle.cpp index c55631fbb..d615ec0d2 100644 --- a/src/core/sample_play_handle.cpp +++ b/src/core/sample_play_handle.cpp @@ -39,10 +39,10 @@ samplePlayHandle::samplePlayHandle( const QString & _sample_file ) : playHandle( SamplePlayHandle ), m_sampleBuffer( new sampleBuffer( _sample_file ) ), - m_doneMayReturnTrue( TRUE ), + m_doneMayReturnTrue( true ), m_frame( 0 ), - m_audioPort( new audioPort( "samplePlayHandle" ) ), - m_ownAudioPort( TRUE ), + m_audioPort( new audioPort( "samplePlayHandle", false ) ), + m_ownAudioPort( true ), m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ), m_volumeModel( &m_defaultVolumeModel ), m_track( NULL ), @@ -56,10 +56,10 @@ samplePlayHandle::samplePlayHandle( const QString & _sample_file ) : samplePlayHandle::samplePlayHandle( sampleBuffer * _sample_buffer ) : playHandle( SamplePlayHandle ), m_sampleBuffer( sharedObject::ref( _sample_buffer ) ), - m_doneMayReturnTrue( TRUE ), + m_doneMayReturnTrue( true ), m_frame( 0 ), - m_audioPort( new audioPort( "samplePlayHandle" ) ), - m_ownAudioPort( TRUE ), + m_audioPort( new audioPort( "samplePlayHandle", false ) ), + m_ownAudioPort( true ), m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ), m_volumeModel( &m_defaultVolumeModel ), m_track( NULL ), @@ -73,10 +73,10 @@ samplePlayHandle::samplePlayHandle( sampleBuffer * _sample_buffer ) : samplePlayHandle::samplePlayHandle( sampleTCO * _tco ) : playHandle( SamplePlayHandle ), m_sampleBuffer( sharedObject::ref( _tco->getSampleBuffer() ) ), - m_doneMayReturnTrue( TRUE ), + m_doneMayReturnTrue( true ), m_frame( 0 ), m_audioPort( ( (sampleTrack *)_tco->getTrack() )->getAudioPort() ), - m_ownAudioPort( FALSE ), + m_ownAudioPort( false ), m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ), m_volumeModel( &m_defaultVolumeModel ), m_track( _tco->getTrack() ), @@ -90,10 +90,10 @@ samplePlayHandle::samplePlayHandle( sampleTCO * _tco ) : samplePlayHandle::samplePlayHandle( pattern * _pattern ) : playHandle( SamplePlayHandle ), m_sampleBuffer( sharedObject::ref( _pattern->getFrozenPattern() ) ), - m_doneMayReturnTrue( TRUE ), + m_doneMayReturnTrue( true ), m_frame( 0 ), m_audioPort( _pattern->getInstrumentTrack()->getAudioPort() ), - m_ownAudioPort( FALSE ), + m_ownAudioPort( false ), m_defaultVolumeModel( DefaultVolume, MinVolume, MaxVolume, 1 ), m_volumeModel( &m_defaultVolumeModel ), m_track( _pattern->getInstrumentTrack() ), @@ -146,7 +146,7 @@ void samplePlayHandle::play( bool /* _try_parallelizing */, bool samplePlayHandle::done( void ) const { - return( framesDone() >= totalFrames() && m_doneMayReturnTrue == TRUE ); + return( framesDone() >= totalFrames() && m_doneMayReturnTrue == true ); } diff --git a/src/core/track.cpp b/src/core/track.cpp index 4dc284035..4fff711e4 100644 --- a/src/core/track.cpp +++ b/src/core/track.cpp @@ -134,11 +134,7 @@ trackContentObject::trackContentObject( track * _track ) : */ trackContentObject::~trackContentObject() { -/*! \brief Start a drag event on this track View. - * - * \param _dee the DragEnterEvent to start. - */ - emit destroyed(); + emit destroyedTCO(); if( getTrack() ) { @@ -323,7 +319,7 @@ trackContentObjectView::trackContentObjectView( trackContentObject * _tco, this, SLOT( updateLength() ) ); connect( m_tco, SIGNAL( positionChanged() ), this, SLOT( updatePosition() ) ); - connect( m_tco, SIGNAL( destroyed() ), this, SLOT( close() ) ); + connect( m_tco, SIGNAL( destroyedTCO() ), this, SLOT( close() ) ); setModel( m_tco ); m_trackView->getTrackContentWidget()->addTCOView( this ); @@ -1572,6 +1568,8 @@ track::track( TrackTypes _type, trackContainer * _tc ) : */ track::~track() { + emit destroyedTrack(); + while( !m_trackContentObjects.isEmpty() ) { delete m_trackContentObjects.last(); @@ -2121,7 +2119,7 @@ trackView::trackView( track * _track, trackContainerView * _tcv ) : setAttribute( Qt::WA_DeleteOnClose ); - connect( m_track, SIGNAL( destroyed() ), this, SLOT( close() ) ); + connect( m_track, SIGNAL( destroyedTrack() ), this, SLOT( close() ) ); connect( m_track, SIGNAL( trackContentObjectAdded( trackContentObject * ) ), this, SLOT( createTCOView( trackContentObject * ) ), @@ -2206,7 +2204,7 @@ void trackView::modelChanged( void ) { m_track = castModel(); assert( m_track != NULL ); - connect( m_track, SIGNAL( destroyed() ), this, SLOT( close() ) ); + connect( m_track, SIGNAL( destroyedTrack() ), this, SLOT( close() ) ); m_trackOperationsWidget.m_muteBtn->setModel( &m_track->m_mutedModel ); m_trackOperationsWidget.m_soloBtn->setModel( &m_track->m_soloModel ); modelView::modelChanged();