Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect reads to SampleBuffer with read locks #2835

Merged
merged 1 commit into from
Jun 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions include/Mixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ class EXPORT Mixer : public QObject
// audio-port-stuff
inline void addAudioPort( AudioPort * _port )
{
lock();
requestChangeInModel();
m_audioPorts.push_back( _port );
unlock();
doneChangeInModel();
}

void removeAudioPort( AudioPort * _port );
Expand Down Expand Up @@ -274,16 +274,6 @@ class EXPORT Mixer : public QObject


// methods needed by other threads to alter knob values, waveforms, etc
void lock()
{
m_globalMutex.lock();
}

void unlock()
{
m_globalMutex.unlock();
}

void lockInputFrames()
{
m_inputFramesMutex.lock();
Expand Down Expand Up @@ -429,7 +419,6 @@ class EXPORT Mixer : public QObject
QString m_midiClientName;

// mutexes
QMutex m_globalMutex;
QMutex m_inputFramesMutex;
QMutex m_playHandleMutex; // mutex used only for adding playhandles
QMutex m_playHandleRemovalMutex;
Expand Down
41 changes: 21 additions & 20 deletions include/SampleBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,16 @@ class EXPORT SampleBuffer : public QObject, public sharedObject

void setLoopStartFrame( f_cnt_t _start )
{
QWriteLocker writeLocker(&m_varLock);
m_loopStartFrame = _start;
}

void setLoopEndFrame( f_cnt_t _end )
{
QWriteLocker writeLocker(&m_varLock);
m_loopEndFrame = _end;
}

void setAllPointFrames( f_cnt_t _start, f_cnt_t _end, f_cnt_t _loopstart, f_cnt_t _loopend )
{
QWriteLocker writeLocker(&m_varLock);
m_startFrame = _start;
m_endFrame = _end;
m_loopStartFrame = _loopstart;
Expand Down Expand Up @@ -202,13 +199,11 @@ class EXPORT SampleBuffer : public QObject, public sharedObject

inline void setFrequency( float _freq )
{
QWriteLocker writeLocker(&m_varLock);
m_frequency = _freq;
}

inline void setSampleRate( sample_rate_t _rate )
{
QWriteLocker writeLocker(&m_varLock);
m_sampleRate = _rate;
}

Expand All @@ -224,31 +219,37 @@ class EXPORT SampleBuffer : public QObject, public sharedObject
QString & toBase64( QString & _dst ) const;


static SampleBuffer * resample( sampleFrame * _data,
const f_cnt_t _frames,
const sample_rate_t _src_sr,
// protect calls from the GUI to this function with dataReadLock() and
// dataUnlock()
SampleBuffer * resample( const sample_rate_t _src_sr,
const sample_rate_t _dst_sr );

static inline SampleBuffer * resample( SampleBuffer * _buf,
const sample_rate_t _src_sr,
const sample_rate_t _dst_sr )
{
return resample( _buf->m_data, _buf->m_frames, _src_sr,
_dst_sr );
}

void normalizeSampleRate( const sample_rate_t _src_sr,
bool _keep_settings = false );

// protect calls from the GUI to this function with dataReadLock() and
// dataUnlock(), out of loops for efficiency
inline sample_t userWaveSample( const float _sample ) const
{
const float frame = _sample * m_frames;
f_cnt_t f1 = static_cast<f_cnt_t>( frame ) % m_frames;
f_cnt_t frames = m_frames;
sampleFrame * data = m_data;
const float frame = _sample * frames;
f_cnt_t f1 = static_cast<f_cnt_t>( frame ) % frames;
if( f1 < 0 )
{
f1 += m_frames;
f1 += frames;
}
return linearInterpolate( m_data[f1][0], m_data[ (f1 + 1) % m_frames ][0], fraction( frame ) );
return linearInterpolate( data[f1][0], data[ (f1 + 1) % frames ][0], fraction( frame ) );
}

void dataReadLock()
{
m_varLock.lockForRead();
}

void dataUnlock()
{
m_varLock.unlock();
}

static QString tryToMakeRelative( const QString & _file );
Expand Down
2 changes: 1 addition & 1 deletion include/SampleRecordHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@

#include "MidiTime.h"
#include "PlayHandle.h"
#include "SampleBuffer.h"

class BBTrack;
class SampleBuffer;
class SampleTCO;
class Track;

Expand Down
1 change: 1 addition & 0 deletions plugins/GigPlayer/GigPlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "Mixer.h"
#include "NotePlayHandle.h"
#include "Knob.h"
#include "SampleBuffer.h"
#include "Song.h"
#include "ConfigManager.h"
#include "endian_handling.h"
Expand Down
2 changes: 1 addition & 1 deletion plugins/GigPlayer/GigPlayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@
#include <QList>
#include <QMutex>
#include <QMutexLocker>
#include <samplerate.h>

#include "Instrument.h"
#include "PixmapButton.h"
#include "InstrumentView.h"
#include "Knob.h"
#include "LcdSpinBox.h"
#include "LedCheckbox.h"
#include "SampleBuffer.h"
#include "MemoryManager.h"
#include "gig.h"

Expand Down
1 change: 1 addition & 0 deletions plugins/sf2_player/sf2_player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "Mixer.h"
#include "NotePlayHandle.h"
#include "Knob.h"
#include "SampleBuffer.h"
#include "Song.h"

#include "patches_dialog.h"
Expand Down
2 changes: 1 addition & 1 deletion plugins/sf2_player/sf2_player.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#define SF2_PLAYER_H

#include <QMutex>
#include <samplerate.h>

#include "Instrument.h"
#include "PixmapButton.h"
Expand All @@ -36,7 +37,6 @@
#include "LcdSpinBox.h"
#include "LedCheckbox.h"
#include "fluidsynth.h"
#include "SampleBuffer.h"
#include "MemoryManager.h"

class sf2InstrumentView;
Expand Down
4 changes: 2 additions & 2 deletions plugins/vestige/vestige.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -644,15 +644,15 @@ void VestigeInstrumentView::openPlugin()
{
return;
}
Engine::mixer()->lock();
Engine::mixer()->requestChangeInModel();

if (m_vi->p_subWindow != NULL) {
delete m_vi->p_subWindow;
m_vi->p_subWindow = NULL;
}

m_vi->loadFile( ofd.selectedFiles()[0] );
Engine::mixer()->unlock();
Engine::mixer()->doneChangeInModel();
if( m_vi->m_plugin && m_vi->m_plugin->pluginWidget() )
{
m_vi->m_plugin->pluginWidget()->setWindowIcon(
Expand Down
19 changes: 8 additions & 11 deletions src/core/EffectChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ void EffectChain::loadSettings( const QDomElement & _this )

void EffectChain::appendEffect( Effect * _effect )
{
Engine::mixer()->lock();
Engine::mixer()->requestChangeInModel();
m_effects.append( _effect );
Engine::mixer()->unlock();
Engine::mixer()->doneChangeInModel();

emit dataChanged();
}
Expand All @@ -134,21 +134,18 @@ void EffectChain::appendEffect( Effect * _effect )

void EffectChain::removeEffect( Effect * _effect )
{
Engine::mixer()->lock();
Engine::mixer()->requestChangeInModel();

Effect ** found = qFind( m_effects.begin(), m_effects.end(), _effect );
if( found == m_effects.end() )
{
goto fail;
Engine::mixer()->doneChangeInModel();
return;
}
m_effects.erase( found );

Engine::mixer()->unlock();
Engine::mixer()->doneChangeInModel();
emit dataChanged();
return;

fail:
Engine::mixer()->unlock();
}


Expand Down Expand Up @@ -252,7 +249,7 @@ void EffectChain::clear()
{
emit aboutToClear();

Engine::mixer()->lock();
Engine::mixer()->requestChangeInModel();

m_enabledModel.setValue( false );
for( int i = 0; i < m_effects.count(); ++i )
Expand All @@ -261,5 +258,5 @@ void EffectChain::clear()
}
m_effects.clear();

Engine::mixer()->unlock();
Engine::mixer()->doneChangeInModel();
}
6 changes: 3 additions & 3 deletions src/core/FxMixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ void FxMixer::toggledSolo()

void FxMixer::deleteChannel( int index )
{
// lock the mixer so channel deletion is performed between mixer rounds
Engine::mixer()->lock();
// channel deletion is performed between mixer rounds
Engine::mixer()->requestChangeInModel();

FxChannel * ch = m_fxChannels[index];

Expand Down Expand Up @@ -347,7 +347,7 @@ void FxMixer::deleteChannel( int index )
}
}

Engine::mixer()->unlock();
Engine::mixer()->doneChangeInModel();
}


Expand Down
24 changes: 4 additions & 20 deletions src/core/Mixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ Mixer::Mixer( bool renderOnly ) :
m_audioDev( NULL ),
m_oldAudioDev( NULL ),
m_audioDevStartFailed( false ),
m_globalMutex( QMutex::Recursive ),
m_profiler(),
m_metronomeActive(false),
m_changesSignal( false ),
Expand Down Expand Up @@ -386,10 +385,6 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
}
unlockPlayHandleRemoval();

// now we have to make sure no other thread does anything bad
// while we're acting...
lock();

// rotate buffers
m_writeBuffer = ( m_writeBuffer + 1 ) % m_poolDepth;
m_readBuffer = ( m_readBuffer + 1 ) % m_poolDepth;
Expand Down Expand Up @@ -453,8 +448,6 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
// STAGE 3: do master mix in FX mixer
fxMixer->masterMix( m_writeBuf );

unlock();


emit nextAudioBuffer( m_readBuf );

Expand All @@ -481,7 +474,6 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
void Mixer::clear()
{
// TODO: m_midiClient->noteOffAll();
lock();
lockPlayHandleRemoval();
for( PlayHandleList::Iterator it = m_playHandles.begin(); it != m_playHandles.end(); ++it )
{
Expand All @@ -493,7 +485,6 @@ void Mixer::clear()
}
}
unlockPlayHandleRemoval();
unlock();
}


Expand Down Expand Up @@ -620,9 +611,7 @@ void Mixer::removeAudioPort( AudioPort * _port )
_port );
if( it != m_audioPorts.end() )
{
lock();
m_audioPorts.erase( it );
unlock();
}
}

Expand Down Expand Up @@ -650,7 +639,7 @@ bool Mixer::addPlayHandle( PlayHandle* handle )

void Mixer::removePlayHandle( PlayHandle * _ph )
{
lockPlayHandleRemoval();
requestChangeInModel();
// check thread affinity as we must not delete play-handles
// which were created in a thread different than mixer thread
if( _ph->affinityMatters() &&
Expand Down Expand Up @@ -694,15 +683,15 @@ void Mixer::removePlayHandle( PlayHandle * _ph )
{
m_playHandlesToRemove.push_back( _ph );
}
unlockPlayHandleRemoval();
doneChangeInModel();
}




void Mixer::removePlayHandlesOfTypes( Track * _track, const quint8 types )
{
lockPlayHandleRemoval();
requestChangeInModel();
PlayHandleList::Iterator it = m_playHandles.begin();
while( it != m_playHandles.end() )
{
Expand All @@ -721,26 +710,21 @@ void Mixer::removePlayHandlesOfTypes( Track * _track, const quint8 types )
++it;
}
}
unlockPlayHandleRemoval();
doneChangeInModel();
}




bool Mixer::hasNotePlayHandles()
{
lock();

for( PlayHandleList::Iterator it = m_playHandles.begin(); it != m_playHandles.end(); ++it )
{
if( (*it)->type() == PlayHandle::TypeNotePlayHandle )
{
unlock();
return true;
}
}

unlock();
return false;
}

Expand Down
Loading