* 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

@@ -1,3 +1,23 @@
+2008-09-11	Tobias Doerffel	<tobydox/at/users/dot/sourceforge/dot/net>
+
+	* 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 <drfaygo/at/gmail/dot/com>
 
 	* plugins/papu/gb_abu/Blip_Buffer.cpp:

include/audio_port.h

@@ -30,13 +30,14 @@
 #include <QtCore/QMutex>
 #include <QtCore/QMutexLocker>
 
-#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;

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;

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 )

include/play_handle.h

@@ -27,6 +27,7 @@
 #ifndef _PLAY_HANDLE_H
 #define _PLAY_HANDLE_H
 
+#include <QtCore/QThread>
 #include <QtCore/QVector>
 
 #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;
 
 } ;
 

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;

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 * );
 

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;
 }
 
 

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" ) )
 {
 }
 

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
 		{

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 );
 }
 
 

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<track>();
 	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();