Remove use of shared ownership for Sample

Sample does not need to be wrapped around a std::shared_ptr. This was to work with the audio thread, but the audio thread can instead have their own Sample separate from the UI's Sample, so changes to the UI's Sample would not leave the audio worker thread using freed data if it had pointed to it.
2023-08-31 10:24:57 -04:00
parent 9583fcfd48
commit 1d452331d1
10 changed files with 69 additions and 78 deletions
--- a/include/SampleClip.h
+++ b/include/SampleClip.h
@@ -63,10 +63,9 @@ public:
 		return "sampleclip";
 	}

-	std::shared_ptr<Sample> sample()
+	const Sample& sample()
 	{
-		// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-		return std::atomic_load(&m_sample);
+		return m_sample;
 	}

 	TimePos sampleLength() const;
@@ -88,7 +87,7 @@ public slots:


 private:
-	std::shared_ptr<Sample> m_sample = std::make_shared<Sample>();
+	Sample m_sample;
 	BoolModel m_recordModel;
 	bool m_isPlaying;

--- a/include/SamplePlayHandle.h
+++ b/include/SamplePlayHandle.h
@@ -44,7 +44,7 @@ class AudioPort;
 class LMMS_EXPORT SamplePlayHandle : public PlayHandle
 {
 public:
-	SamplePlayHandle(std::shared_ptr<const Sample> sampleBuffer , bool ownAudioPort = true);
+	SamplePlayHandle(const Sample& sample, bool ownAudioPort = true);
 	SamplePlayHandle( const QString& sampleFile );
 	SamplePlayHandle( SampleClip* clip );
 	~SamplePlayHandle() override;
@@ -82,7 +82,7 @@ public:


 private:
-	std::shared_ptr<const Sample> m_sample;
+	Sample m_sample;
 	bool m_doneMayReturnTrue;

 	f_cnt_t m_frame;
--- a/plugins/AudioFileProcessor/AudioFileProcessor.cpp
+++ b/plugins/AudioFileProcessor/AudioFileProcessor.cpp
@@ -77,7 +77,6 @@ Plugin::Descriptor PLUGIN_EXPORT audiofileprocessor_plugin_descriptor =

 AudioFileProcessor::AudioFileProcessor( InstrumentTrack * _instrument_track ) :
 	Instrument( _instrument_track, &audiofileprocessor_plugin_descriptor ),
-	m_sample(std::make_shared<Sample>()),
 	m_ampModel( 100, 0, 500, 1, this, tr( "Amplify" ) ),
 	m_startPointModel( 0, 0, 1, 0.0000001f, this, tr( "Start of sample" ) ),
 	m_endPointModel( 1, 0, 1, 0.0000001f, this, tr( "End of sample" ) ),
@@ -125,18 +124,18 @@ void AudioFileProcessor::playNote( NotePlayHandle * _n,
 	// played.
 	if( m_stutterModel.value() == true && _n->frequency() < 20.0 )
 	{
-		m_nextPlayStartPoint = m_sample->startFrame();
+		m_nextPlayStartPoint = m_sample.startFrame();
 		m_nextPlayBackwards = false;
 		return;
 	}

 	if( !_n->m_pluginData )
 	{
-		if (m_stutterModel.value() == true && m_nextPlayStartPoint >= m_sample->endFrame())
+		if (m_stutterModel.value() == true && m_nextPlayStartPoint >= m_sample.endFrame())
 		{
 			// Restart playing the note if in stutter mode, not in loop mode,
 			// and we're at the end of the sample.
-			m_nextPlayStartPoint = m_sample->startFrame();
+			m_nextPlayStartPoint = m_sample.startFrame();
 			m_nextPlayBackwards = false;
 		}
 		// set interpolation mode for libsamplerate
@@ -165,8 +164,7 @@ void AudioFileProcessor::playNote( NotePlayHandle * _n,

 	if( ! _n->isFinished() )
 	{
-		// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-		if (std::atomic_load(&m_sample)->play(_working_buffer + offset,
+		if (m_sample.play(_working_buffer + offset,
 						static_cast<Sample::PlaybackState*>(_n->m_pluginData),
 						frames, _n->frequency(),
 						static_cast<Sample::Loop>(m_loopModel.value())))
@@ -207,10 +205,10 @@ void AudioFileProcessor::deleteNotePluginData( NotePlayHandle * _n )

 void AudioFileProcessor::saveSettings(QDomDocument& doc, QDomElement& elem)
 {
-	elem.setAttribute("src", m_sample->sampleFile());
-	if (m_sample->sampleFile().isEmpty())
+	elem.setAttribute("src", m_sample.sampleFile());
+	if (m_sample.sampleFile().isEmpty())
 	{
-		elem.setAttribute("sampledata", m_sample->toBase64());
+		elem.setAttribute("sampledata", m_sample.toBase64());
 	}
 	m_reverseModel.saveSettings(doc, elem, "reversed");
 	m_loopModel.saveSettings(doc, elem, "looped");
@@ -231,18 +229,17 @@ void AudioFileProcessor::loadSettings(const QDomElement& elem)
 	{
 		setAudioFile(elem.attribute("src"), false);

-		QString absolutePath = PathUtil::toAbsolute(m_sample->sampleFile());
+		QString absolutePath = PathUtil::toAbsolute(m_sample.sampleFile());
 		if (!QFileInfo(absolutePath).exists())
 		{
-			QString message = tr("Sample not found: %1").arg(m_sample->sampleFile());
+			QString message = tr("Sample not found: %1").arg(m_sample.sampleFile());
 			Engine::getSong()->collectError(message);
 		}
 	}
 	else if (!elem.attribute("sampledata").isEmpty())
 	{
 		auto buffer = gui::SampleLoader::createBufferFromBase64(elem.attribute("srcdata"));
-		// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-		std::atomic_store(&m_sample, std::make_shared<Sample>(std::move(buffer)));
+		m_sample = Sample(std::move(buffer));
 	}

 	m_loopModel.loadSettings(elem, "looped");
@@ -301,7 +298,7 @@ int AudioFileProcessor::getBeatLen( NotePlayHandle * _n ) const
 	const float freq_factor = baseFreq / _n->frequency() *
 			Engine::audioEngine()->processingSampleRate() / Engine::audioEngine()->baseSampleRate();

-	return static_cast<int>(floorf((m_sample->endFrame() - m_sample->startFrame()) * freq_factor));
+	return static_cast<int>(floorf((m_sample.endFrame() - m_sample.startFrame()) * freq_factor));
 }


@@ -322,8 +319,8 @@ void AudioFileProcessor::setAudioFile( const QString & _audio_file,
 	// is current channel-name equal to previous-filename??
 	if( _rename &&
 		( instrumentTrack()->name() ==
-			QFileInfo(m_sample->sampleFile()).fileName() ||
-				m_sample->sampleFile().isEmpty()))
+			QFileInfo(m_sample.sampleFile()).fileName() ||
+				m_sample.sampleFile().isEmpty()))
 	{
 		// then set it to new one
 		instrumentTrack()->setName( PathUtil::cleanName( _audio_file ) );
@@ -331,8 +328,7 @@ void AudioFileProcessor::setAudioFile( const QString & _audio_file,
 	// else we don't touch the track-name, because the user named it self

 	auto buffer = gui::SampleLoader::createBufferFromFile(_audio_file);
-	// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-	std::atomic_store(&m_sample, std::make_shared<Sample>(std::move(buffer)));
+	m_sample = Sample(std::move(buffer));
 	
 	loopPointChanged();
 	emit sampleUpdated();
@@ -343,8 +339,8 @@ void AudioFileProcessor::setAudioFile( const QString & _audio_file,

 void AudioFileProcessor::reverseModelChanged()
 {
-	m_sample->setReversed(m_reverseModel.value());
-	m_nextPlayStartPoint = m_sample->startFrame();
+	m_sample.setReversed(m_reverseModel.value());
+	m_nextPlayStartPoint = m_sample.startFrame();
 	m_nextPlayBackwards = false;
 	emit sampleUpdated();
 }
@@ -354,14 +350,14 @@ void AudioFileProcessor::reverseModelChanged()

 void AudioFileProcessor::ampModelChanged()
 {
-	m_sample->setAmplification(m_ampModel.value() / 100.0f);
+	m_sample.setAmplification(m_ampModel.value() / 100.0f);
 	emit sampleUpdated();
 }


 void AudioFileProcessor::stutterModelChanged()
 {
-	m_nextPlayStartPoint = m_sample->startFrame();
+	m_nextPlayStartPoint = m_sample.startFrame();
 	m_nextPlayBackwards = false;
 }

@@ -430,14 +426,14 @@ void AudioFileProcessor::loopPointChanged()

 void AudioFileProcessor::pointChanged()
 {
-	const auto f_start = static_cast<f_cnt_t>(m_startPointModel.value() * m_sample->sampleSize());
-	const auto f_end = static_cast<f_cnt_t>(m_endPointModel.value() * m_sample->sampleSize());
-	const auto f_loop = static_cast<f_cnt_t>(m_loopPointModel.value() * m_sample->sampleSize());
+	const auto f_start = static_cast<f_cnt_t>(m_startPointModel.value() * m_sample.sampleSize());
+	const auto f_end = static_cast<f_cnt_t>(m_endPointModel.value() * m_sample.sampleSize());
+	const auto f_loop = static_cast<f_cnt_t>(m_loopPointModel.value() * m_sample.sampleSize());

 	m_nextPlayStartPoint = f_start;
 	m_nextPlayBackwards = false;

-	m_sample->setAllPointFrames(f_start, f_end, f_loop, f_end);
+	m_sample.setAllPointFrames(f_start, f_end, f_loop, f_end);
 	emit dataChanged();
 }

@@ -605,7 +601,7 @@ void AudioFileProcessorView::newWaveView()
 		delete m_waveView;
 		m_waveView = 0;
 	}
-	m_waveView = new AudioFileProcessorWaveView(this, 245, 75, castModel<AudioFileProcessor>()->m_sample.get());
+	m_waveView = new AudioFileProcessorWaveView(this, 245, 75, &castModel<AudioFileProcessor>()->m_sample);
 	m_waveView->move( 2, 172 );
 	m_waveView->setKnobs(
 		dynamic_cast<AudioFileProcessorWaveView::knob *>( m_startKnob ),
@@ -652,7 +648,7 @@ void AudioFileProcessorView::paintEvent( QPaintEvent * )

 	QString file_name = "";

-	int idx = a->m_sample->sampleFile().length();
+	int idx = a->m_sample.sampleFile().length();

 	p.setFont( pointSize<8>( font() ) );

@@ -663,7 +659,7 @@ void AudioFileProcessorView::paintEvent( QPaintEvent * )
 	while( idx > 0 &&
 		fm.size( Qt::TextSingleLine, file_name + "..." ).width() < 210 )
 	{
-		file_name = a->m_sample->sampleFile()[--idx] + file_name;
+		file_name = a->m_sample.sampleFile()[--idx] + file_name;
 	}

 	if( idx > 0 )
@@ -680,7 +676,6 @@ void AudioFileProcessorView::paintEvent( QPaintEvent * )

 void AudioFileProcessorView::sampleUpdated()
 {
-	m_waveView->m_sample = castModel<AudioFileProcessor>()->m_sample.get();
 	m_waveView->updateSampleRange();
 	m_waveView->update();
 	update();
--- a/plugins/AudioFileProcessor/AudioFileProcessor.h
+++ b/plugins/AudioFileProcessor/AudioFileProcessor.h
@@ -97,7 +97,7 @@ signals:
 	void sampleUpdated();

 private:
-	std::shared_ptr<Sample> m_sample;
+	Sample m_sample;

 	FloatModel m_ampModel;
 	FloatModel m_startPointModel;
--- a/plugins/Patman/Patman.cpp
+++ b/plugins/Patman/Patman.cpp
@@ -151,9 +151,9 @@ void PatmanInstrument::playNote( NotePlayHandle * _n,
 	auto hdata = (handle_data*)_n->m_pluginData;

 	float play_freq = hdata->tuned ? _n->frequency() :
-						hdata->sample->frequency();
+						hdata->sample.frequency();

-	if (hdata->sample->play(_working_buffer + offset, hdata->state, frames,
+	if (hdata->sample.play(_working_buffer + offset, hdata->state, frames,
 					play_freq, m_loopedModel.value() ? Sample::Loop::On : Sample::Loop::Off))
 	{
 		applyRelease( _working_buffer, _n );
@@ -357,13 +357,13 @@ PatmanInstrument::LoadError PatmanInstrument::loadPatch(
 			}
 		}

-		auto psample = std::make_shared<Sample>(data, frames, sample_rate);
-		psample->setFrequency(root_freq / 1000.0f);
+		auto psample = Sample(data, frames, sample_rate);
+		psample.setFrequency(root_freq / 1000.0f);

 		if( modes & MODES_LOOPING )
 		{
-			psample->setLoopStartFrame( loop_start );
-			psample->setLoopEndFrame( loop_end );
+			psample.setLoopStartFrame(loop_start);
+			psample.setLoopEndFrame(loop_end);
 		}

 		m_patchSamples.push_back(psample);
@@ -394,24 +394,24 @@ void PatmanInstrument::selectSample( NotePlayHandle * _n )
 	const float freq = _n->frequency();

 	float min_dist = HUGE_VALF;
-	std::shared_ptr<Sample> sample = nullptr;
+	const Sample* sample = nullptr;

 	for (const auto& patchSample : m_patchSamples)
 	{
-		float patch_freq = patchSample->frequency();
+		float patch_freq = patchSample.frequency();
 		float dist = freq >= patch_freq ? freq / patch_freq :
 							patch_freq / freq;

 		if( dist < min_dist )
 		{
 			min_dist = dist;
-			sample = patchSample;
+			sample = &patchSample;
 		}
 	}

 	auto hdata = new handle_data;
 	hdata->tuned = m_tunedModel.value();
-	hdata->sample = sample ? sample : std::make_shared<Sample>();
+	if (sample) { hdata->sample = *sample; }
 	hdata->state = new Sample::PlaybackState(_n->hasDetuningInfo());

 	_n->m_pluginData = hdata;
--- a/plugins/Patman/Patman.h
+++ b/plugins/Patman/Patman.h
@@ -90,11 +90,11 @@ private:
 		MM_OPERATORS
 		Sample::PlaybackState* state;
 		bool tuned;
-		std::shared_ptr<Sample> sample;
+		Sample sample;
 	};

 	QString m_patchFile;
-	QVector<std::shared_ptr<Sample>> m_patchSamples;
+	QVector<Sample> m_patchSamples;
 	BoolModel m_loopedModel;
 	BoolModel m_tunedModel;

--- a/src/core/SampleClip.cpp
+++ b/src/core/SampleClip.cpp
@@ -90,8 +90,7 @@ SampleClip::SampleClip( Track * _track ) :
 SampleClip::SampleClip(const SampleClip& orig) :
 	SampleClip(orig.getTrack())
 {
-	// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-	std::atomic_store(&m_sample, orig.m_sample);
+	m_sample = orig.m_sample;
 	m_isPlaying = orig.m_isPlaying;
 }

@@ -120,7 +119,7 @@ void SampleClip::changeLength( const TimePos & _length )

 QString SampleClip::sampleFile() const
 {
-	return m_sample->sampleFile();
+	return m_sample.sampleFile();
 }


@@ -129,7 +128,7 @@ void SampleClip::setSampleBuffer( SampleBuffer* sb )
 {
 	// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
 	auto buffer = std::shared_ptr<const SampleBuffer>(sb);
-	std::atomic_store(&m_sample, std::make_shared<Sample>(buffer));
+	m_sample = Sample(buffer);
 	updateLength();

 	emit sampleChanged();
@@ -149,8 +148,7 @@ void SampleClip::setSampleFile( const QString & _sf )
 	else
 	{	//Otherwise set it to the sample's length
 		auto buffer = gui::SampleLoader::createBufferFromFile(_sf);
-		// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-		std::atomic_store(&m_sample, std::make_shared<Sample>(std::move(buffer)));
+		m_sample = Sample(std::move(buffer));
 		length = sampleLength();
 	}
 	changeLength(length);
@@ -221,7 +219,7 @@ void SampleClip::updateLength()

 TimePos SampleClip::sampleLength() const
 {
-	return static_cast<int>(m_sample->playbackSize() / Engine::framesPerTick());
+	return static_cast<int>(m_sample.playbackSize() / Engine::framesPerTick());
 }


@@ -229,7 +227,7 @@ TimePos SampleClip::sampleLength() const

 void SampleClip::setSampleStartFrame(f_cnt_t startFrame)
 {
-	m_sample->setStartFrame(startFrame);
+	m_sample.setStartFrame(startFrame);
 }


@@ -237,7 +235,7 @@ void SampleClip::setSampleStartFrame(f_cnt_t startFrame)

 void SampleClip::setSamplePlayLength(f_cnt_t length)
 {
-	m_sample->setEndFrame(length);
+	m_sample.setEndFrame(length);
 }


@@ -260,15 +258,15 @@ void SampleClip::saveSettings( QDomDocument & _doc, QDomElement & _this )
 	if( sampleFile() == "" )
 	{
 		QString s;
-		_this.setAttribute("data", m_sample->toBase64());
+		_this.setAttribute("data", m_sample.toBase64());
 	}

-	_this.setAttribute("sample_rate", m_sample->sampleRate());
+	_this.setAttribute("sample_rate", m_sample.sampleRate());
 	if( usesCustomClipColor() )
 	{
 		_this.setAttribute( "color", color().name() );
 	}
-	if (m_sample->reversed())
+	if (m_sample.reversed())
 	{
 		_this.setAttribute("reversed", "true");
 	}
@@ -291,8 +289,7 @@ void SampleClip::loadSettings( const QDomElement & _this )
 			Engine::audioEngine()->processingSampleRate();

 		auto buffer = gui::SampleLoader::createBufferFromBase64(_this.attribute("data"), sampleRate);
-		// TODO C++20: Deprecated, use std::atomic<std::shared_ptr> instead
-		std::atomic_store(&m_sample, std::make_shared<Sample>(std::move(buffer)));
+		m_sample = Sample(std::move(buffer));
 	}
 	changeLength( _this.attribute( "len" ).toInt() );
 	setMuted( _this.attribute( "muted" ).toInt() );
@@ -310,7 +307,7 @@ void SampleClip::loadSettings( const QDomElement & _this )

 	if(_this.hasAttribute("reversed"))
 	{
-		m_sample->setReversed(true);
+		m_sample.setReversed(true);
 		emit wasReversed(); // tell SampleClipView to update the view
 	}
 }
--- a/src/core/SamplePlayHandle.cpp
+++ b/src/core/SamplePlayHandle.cpp
@@ -35,7 +35,7 @@ namespace lmms
 {


-SamplePlayHandle::SamplePlayHandle(std::shared_ptr<const Sample> sample, bool ownAudioPort) :
+SamplePlayHandle::SamplePlayHandle(const Sample& sample, bool ownAudioPort) :
 	PlayHandle( Type::SamplePlayHandle ),
 	m_sample(sample),
 	m_doneMayReturnTrue( true ),
@@ -56,7 +56,7 @@ SamplePlayHandle::SamplePlayHandle(std::shared_ptr<const Sample> sample, bool ow


 SamplePlayHandle::SamplePlayHandle( const QString& sampleFile ) :
-	SamplePlayHandle(std::make_shared<const Sample>(sampleFile), true)
+	SamplePlayHandle(Sample(sampleFile), true)
 {
 }

@@ -113,7 +113,7 @@ void SamplePlayHandle::play( sampleFrame * buffer )
 				m_volumeModel->value() / DefaultVolume } };*/
 		// SamplePlayHandle always plays the sample at its original pitch;
 		// it is used only for previews, SampleTracks and the metronome.
-		if (!m_sample->play(workingBuffer, &m_state, frames, DefaultBaseFreq))
+		if (!m_sample.play(workingBuffer, &m_state, frames, DefaultBaseFreq))
 		{
 			memset(workingBuffer, 0, frames * sizeof(sampleFrame));
 		}
@@ -143,8 +143,8 @@ bool SamplePlayHandle::isFromTrack( const Track * _track ) const

 f_cnt_t SamplePlayHandle::totalFrames() const
 {
-	return (m_sample->endFrame() - m_sample->startFrame()) *
-			(static_cast<float>(Engine::audioEngine()->processingSampleRate()) / m_sample->sampleRate());
+	return (m_sample.endFrame() - m_sample.startFrame()) *
+			(static_cast<float>(Engine::audioEngine()->processingSampleRate()) / m_sample.sampleRate());
 }


--- a/src/gui/clips/SampleClipView.cpp
+++ b/src/gui/clips/SampleClipView.cpp
@@ -60,8 +60,8 @@ void SampleClipView::updateSample()
 	update();
 	// set tooltip to filename so that user can see what sample this
 	// sample-clip contains
-	setToolTip(m_clip->m_sample->sampleFile() != "" ?
-					PathUtil::toAbsolute(m_clip->m_sample->sampleFile()) :
+	setToolTip(m_clip->m_sample.sampleFile() != "" ?
+					PathUtil::toAbsolute(m_clip->m_sample.sampleFile()) :
 					tr( "Double-click to open sample" ) );
 }

@@ -173,9 +173,9 @@ void SampleClipView::mouseDoubleClickEvent( QMouseEvent * )
 	QString af = gui::SampleLoader::openAudioFile();

 	if ( af.isEmpty() ) {} //Don't do anything if no file is loaded
-	else if (af == m_clip->m_sample->sampleFile())
+	else if (af == m_clip->m_sample.sampleFile())
 	{	//Instead of reloading the existing file, just reset the size
-		int length = static_cast<int>(m_clip->m_sample->sampleSize() / Engine::framesPerTick());
+		int length = static_cast<int>(m_clip->m_sample.sampleSize() / Engine::framesPerTick());
 		m_clip->changeLength(length);
 	}
 	else
@@ -262,9 +262,9 @@ void SampleClipView::paintEvent( QPaintEvent * pe )
 	float offset =  m_clip->startTimeOffset() / ticksPerBar * pixelsPerBar();
 	QRect r = QRect( offset, spacing,
 			qMax( static_cast<int>( m_clip->sampleLength() * ppb / ticksPerBar ), 1 ), rect().bottom() - 2 * spacing );
-	m_clip->m_sample->visualize(p, r);
+	m_clip->m_sample.visualize(p, r);

-	QString name = PathUtil::cleanName(m_clip->m_sample->sampleFile());
+	QString name = PathUtil::cleanName(m_clip->m_sample.sampleFile());
 	paintTextLabel(name, p);

 	// disable antialiasing for borders, since its not needed
@@ -317,7 +317,7 @@ void SampleClipView::paintEvent( QPaintEvent * pe )

 void SampleClipView::reverseSample()
 {
-	m_clip->sample()->setReversed(!m_clip->sample()->reversed());
+	m_clip->m_sample.setReversed(!m_clip->m_sample.reversed());
 	Engine::getSong()->setModified();
 	update();
 }
--- a/src/tracks/SampleTrack.cpp
+++ b/src/tracks/SampleTrack.cpp
@@ -108,10 +108,10 @@ bool SampleTrack::play( const TimePos & _start, const fpp_t _frames,
 			{
 				if( sClip->isPlaying() == false && _start >= (sClip->startPosition() + sClip->startTimeOffset()) )
 				{
-					auto bufferFramesPerTick = Engine::framesPerTick(sClip->sample()->sampleRate());
+					auto bufferFramesPerTick = Engine::framesPerTick(sClip->sample().sampleRate());
 					f_cnt_t sampleStart = bufferFramesPerTick * ( _start - sClip->startPosition() - sClip->startTimeOffset() );
 					f_cnt_t clipFrameLength = bufferFramesPerTick * ( sClip->endPosition() - sClip->startPosition() - sClip->startTimeOffset() );
-					f_cnt_t sampleBufferLength = sClip->sample()->sampleSize();
+					f_cnt_t sampleBufferLength = sClip->sample().sampleSize();
 					//if the Clip smaller than the sample length we play only until Clip end
 					//else we play the sample to the end but nothing more
 					f_cnt_t samplePlayLength = clipFrameLength > sampleBufferLength ? sampleBufferLength : clipFrameLength;