Manage ownership of clips using std::unique_ptr's

include/AutomationTrack.h

@@ -48,8 +48,7 @@ class LMMS_EXPORT AutomationTrack : public Track
 	}
 
 	gui::TrackView * createView( gui::TrackContainerView* ) override;
-	Clip* createClip() override;
-	bool canAddClip(Clip* clip) override;
+	std::unique_ptr<Clip> createClip() override;
 
 	void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override;
 	void loadTrackSpecificSettings( const QDomElement & _this ) override;

include/InstrumentTrack.h

@@ -129,9 +129,7 @@ class LMMS_EXPORT InstrumentTrack : public Track, public MidiEventProcessor
 	gui::TrackView* createView( gui::TrackContainerView* tcv ) override;
 
 	// create new track-content-object = clip
-	Clip* createClip() override;
-
-	bool canAddClip(Clip* clip) override;
+	std::unique_ptr<Clip> createClip() override;
 
 	// called by track
 	void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override;

include/PatternTrack.h

@@ -55,8 +55,7 @@ class LMMS_EXPORT PatternTrack : public Track
 
 						const f_cnt_t _frame_base, int _clip_num = -1 ) override;
 	gui::TrackView * createView( gui::TrackContainerView* tcv ) override;
-	Clip* createClip() override;
-	bool canAddClip(Clip* clip) override;
+	std::unique_ptr<Clip> createClip() override;
 
 	void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override;
 	void loadTrackSpecificSettings( const QDomElement & _this ) override;

include/SampleTrack.h

@@ -51,8 +51,7 @@ class LMMS_EXPORT SampleTrack : public Track
 	bool play( const TimePos & _start, const fpp_t _frames,
 						const f_cnt_t _frame_base, int _clip_num = -1 ) override;
 	gui::TrackView * createView( gui::TrackContainerView* tcv ) override;
-	Clip* createClip() override;
-	bool canAddClip(Clip* clip) override;
+	std::unique_ptr<Clip> createClip() override;
 
 	void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) override;
 	void loadTrackSpecificSettings( const QDomElement & _this ) override;

include/Track.h

@@ -72,8 +72,6 @@ class LMMS_EXPORT Track : public Model, public JournallingObject
 	mapPropertyFromModel(bool,isMuted,setMuted,m_mutedModel);
 	mapPropertyFromModel(bool,isSolo,setSolo,m_soloModel);
 public:
-	using clipVector = std::vector<Clip*>;
-
 	enum class Type
 	{
 		Instrument,
@@ -100,12 +98,8 @@ class LMMS_EXPORT Track : public Model, public JournallingObject
 	virtual bool play( const TimePos & start, const fpp_t frames,
 						const f_cnt_t frameBase, int clipNum = -1 ) = 0;
 
-
-
 	virtual gui::TrackView * createView( gui::TrackContainerView * view ) = 0;
-	virtual Clip* createClip() = 0;
-	virtual bool canAddClip(Clip* clip) = 0;
-
+	virtual std::unique_ptr<Clip> createClip() = 0;
 	virtual void saveTrackSpecificSettings(QDomDocument& doc, QDomElement& parent, bool presetMode) = 0;
 	virtual void loadTrackSpecificSettings( const QDomElement & element ) = 0;
 
@@ -117,35 +111,16 @@ class LMMS_EXPORT Track : public Model, public JournallingObject
 	void saveSettings( QDomDocument & doc, QDomElement & element ) override;
 	void loadSettings( const QDomElement & element ) override;
 
-	template<typename T, typename... Args>
-	T* addClip(Args&&... args)
-	{
-		static_assert(std::is_base_of_v<Clip, T>, "T must be a kind of Clip");
-		const auto guard = Engine::audioEngine()->requestChangesGuard();
-
-		auto clip = new T(std::forward<Args>(args)...);
-		assert(canAddClip(clip) && "This clip cannot be added to this track (incompatible types?)");
-
-		m_clips.push_back(clip);
-		clip->setTrack(this);
-		clip->onAddedToTrack(this);
-		emit clipAdded(clip);
-		return clip;
-	}
-
+	Clip* addClip(std::unique_ptr<Clip> clip);
 	void removeClip(Clip* clip);
 	void deleteClips();
 
 	int numOfClips();
 	auto getClip(std::size_t clipNum) -> Clip*;
 	int getClipNum(const Clip* clip );
 
-	const clipVector & getClips() const
-	{
-		return m_clips;
-	}
-	void getClipsInRange( clipVector & clipV, const TimePos & start,
-							const TimePos & end );
+	std::vector<Clip*> getClips() const;
+	std::vector<Clip*> getClipsInRange(const TimePos& start, const TimePos& end);
 	void swapPositionOfClips( int clipNum1, int clipNum2 );
 
 	void createClipsForPattern(int pattern);
@@ -238,7 +213,7 @@ public slots:
 	BoolModel m_soloModel;
 	bool m_mutedBeforeSolo;
 
-	clipVector m_clips;
+	std::vector<std::unique_ptr<Clip>> m_clips;
 
 	QMutex m_processingLock;
 

plugins/MidiImport/MidiImport.cpp

@@ -187,7 +187,7 @@ class smfMidiCC
 		if( !ap || time > lastPos + DefaultTicksPerBar )
 		{
 			TimePos pPos = TimePos( time.getBar(), 0 );
-			ap = static_cast<AutomationClip*>(at->createClip());
+			ap = static_cast<AutomationClip*>(at->addClip(at->createClip()));
 			ap->movePosition(pPos);
 			ap->addObject(objModel);
 		}
@@ -254,7 +254,7 @@ class smfMidiChannel
 			it->pitchRangeModel()->setInitValue( 2 );
 
 			// Create a default pattern
-			p = static_cast<MidiClip*>(it->createClip());
+			p = static_cast<MidiClip*>(it->addClip(it->createClip()));
 		}
 		return this;
 	}
@@ -264,7 +264,7 @@ class smfMidiChannel
 	{
 		if (!p)
 		{
-			p = static_cast<MidiClip*>(it->createClip());
+			p = static_cast<MidiClip*>(it->addClip(it->createClip()));
 		}
 		p->addNote(n, false);
 		hasNotes = true;
@@ -281,7 +281,7 @@ class smfMidiChannel
 			if (!newMidiClip || n->pos() > lastEnd + DefaultTicksPerBar)
 			{
 				TimePos pPos = TimePos(n->pos().getBar(), 0);
-				newMidiClip = static_cast<MidiClip*>(it->createClip());
+				newMidiClip = static_cast<MidiClip*>(it->addClip(it->createClip()));
 				newMidiClip->movePosition(pPos);
 			}
 			lastEnd = n->pos() + n->length();
@@ -335,10 +335,10 @@ bool MidiImport::readSMF( TrackContainer* tc )
 	auto dt = Engine::getSong()->addTrack<AutomationTrack>();
 
 	dt->setName(tr("MIDI Time Signature Denominator"));
-	auto timeSigNumeratorPat = static_cast<AutomationClip*>(nt->createClip());
+	auto timeSigNumeratorPat = static_cast<AutomationClip*>(nt->addClip(nt->createClip()));
 	timeSigNumeratorPat->setDisplayName(tr("Numerator"));
 	timeSigNumeratorPat->addObject(&timeSigMM.numeratorModel());
-	auto timeSigDenominatorPat = static_cast<AutomationClip*>(dt->createClip());
+	auto timeSigDenominatorPat = static_cast<AutomationClip*>(dt->addClip(dt->createClip()));
 	timeSigDenominatorPat->setDisplayName(tr("Denominator"));
 	timeSigDenominatorPat->addObject(&timeSigMM.denominatorModel());
 
@@ -363,7 +363,7 @@ bool MidiImport::readSMF( TrackContainer* tc )
 	// Tempo stuff
 	auto tt = Engine::getSong()->addTrack<AutomationTrack>();
 	tt->setName(tr("Tempo"));
-	auto tap = static_cast<AutomationClip*>(tt->createClip());
+	auto tap = static_cast<AutomationClip*>(tt->addClip(tt->createClip()));
 	tap->setDisplayName(tr("Tempo"));
 	tap->addObject(&Engine::getSong()->tempoModel());
 	if( tap )

src/core/AutomationClip.cpp

@@ -1029,8 +1029,8 @@ AutomationClip * AutomationClip::globalAutomationClip(
 		}
 	}
 
-	auto a = static_cast<AutomationClip*>(t->createClip());
-	a->addObject( _m, false );
+	auto a = static_cast<AutomationClip*>(t->addClip(t->createClip()));
+	a->addObject(_m, false);
 	return a;
 }
 

src/core/Clip.cpp

@@ -60,11 +60,6 @@ Clip::Clip() :
 Clip::~Clip()
 {
 	emit destroyedClip();
-
-	if( getTrack() )
-	{
-		getTrack()->removeClip( this );
-	}
 }
 
 

src/core/Song.cpp

@@ -381,11 +381,11 @@ void Song::processAutomations(const TrackList &tracklist, TimePos timeStart, fpp
 	values = container->automatedValuesAt(timeStart, clipNum);
 	const TrackList& tracks = container->tracks();
 
-	Track::clipVector clips;
+	auto clips = std::vector<Clip*>{};
 	for (Track* track : tracks)
 	{
 		if (track->type() == Track::Type::Automation) {
-			track->getClipsInRange(clips, 0, timeStart);
+			clips = track->getClipsInRange(0, timeStart);
 		}
 	}
 

src/core/Track.cpp

@@ -79,12 +79,6 @@ Track::~Track()
 {
 	lock();
 	emit destroyedTrack();
-
-	while (!m_clips.empty())
-	{
-		delete m_clips.back();
-	}
-
 	m_trackContainer->removeTrack( this );
 	unlock();
 }
@@ -262,13 +256,23 @@ void Track::loadSettings(const QDomElement& element)
 	loadTrack(element, false);
 }
 
+Clip* Track::addClip(std::unique_ptr<Clip> clip)
+{
+	m_clips.emplace_back(std::move(clip));
+	m_clips.back()->setTrack(this);
+	m_clips.back()->onAddedToTrack(this);
+	emit clipAdded(m_clips.back().get());
+	return m_clips.back().get();
+}
+
 /*! \brief Remove a given Clip from this track
  *
  *  \param clip The Clip to remove from this track.
  */
 void Track::removeClip( Clip * clip )
 {
-	clipVector::iterator it = std::find( m_clips.begin(), m_clips.end(), clip );
+	const auto it = std::find_if(m_clips.begin(), m_clips.end(), [&](auto& x) { return x.get() == clip; });
+
 	if( it != m_clips.end() )
 	{
 		m_clips.erase( it );
@@ -284,10 +288,7 @@ void Track::removeClip( Clip * clip )
 /*! \brief Remove all Clips from this track */
 void Track::deleteClips()
 {
-	while (!m_clips.empty())
-	{
-		delete m_clips.front();
-	}
+	m_clips.clear();
 }
 
 
@@ -319,14 +320,14 @@ auto Track::getClip(std::size_t clipNum) -> Clip*
 {
 	if( clipNum < m_clips.size() )
 	{
-		return m_clips[clipNum];
+		return m_clips[clipNum].get();
 	}
 	printf( "called Track::getClip( %zu ), "
 			"but Clip %zu doesn't exist\n", clipNum, clipNum );
 
 	auto clip = createClip();
 	clip->movePosition(clipNum * TimePos::ticksPerBar());
-	return clip;
+	return addClip(std::move(clip));
 }
 
 
@@ -340,7 +341,7 @@ auto Track::getClip(std::size_t clipNum) -> Clip*
 int Track::getClipNum( const Clip * clip )
 {
 //	for( int i = 0; i < getTrackContentWidget()->numOfClips(); ++i )
-	clipVector::iterator it = std::find( m_clips.begin(), m_clips.end(), clip );
+	const auto it = std::find_if(m_clips.begin(), m_clips.end(), [&](auto& x) { return x.get() == clip; });
 	if( it != m_clips.end() )
 	{
 /*		if( getClip( i ) == _clip )
@@ -353,8 +354,12 @@ int Track::getClipNum( const Clip * clip )
 	return 0;
 }
 
-
-
+std::vector<Clip*> Track::getClips() const
+{
+	auto clips = std::vector<Clip*>(m_clips.size());
+	std::transform(m_clips.begin(), m_clips.end(), clips.begin(), [](auto& x) { return x.get(); });
+	return clips;
+}
 
 /*! \brief Retrieve a list of clips that fall within a period.
  *
@@ -367,21 +372,22 @@ int Track::getClipNum( const Clip * clip )
  *  \param start The MIDI start time of the range.
  *  \param end   The MIDI endi time of the range.
  */
-void Track::getClipsInRange( clipVector & clipV, const TimePos & start,
-							const TimePos & end )
+std::vector<Clip*> Track::getClipsInRange(const TimePos& start, const TimePos& end)
 {
-	for( Clip* clip : m_clips )
+	auto clips = std::vector<Clip*>();
+
+	for (auto& clip : m_clips)
 	{
 		int s = clip->startPosition();
 		int e = clip->endPosition();
 		if( ( s <= end ) && ( e >= start ) )
 		{
-			// Clip is within given range
-			// Insert sorted by Clip's position
-			clipV.insert(std::upper_bound(clipV.begin(), clipV.end(), clip, Clip::comparePosition),
-						clip);
+			clips.push_back(clip.get());
 		}
 	}
+
+	std::sort(clips.begin(), clips.end(), Clip::comparePosition);
+	return clips;
 }
 
 

src/core/TrackContainer.cpp

@@ -273,7 +273,7 @@ AutomatedValueMap TrackContainer::automatedValuesAt(TimePos time, int clipNum) c
 
 AutomatedValueMap TrackContainer::automatedValuesFromTracks(const TrackList &tracks, TimePos time, int clipNum)
 {
-	Track::clipVector clips;
+	auto clips = std::vector<Clip*>{};
 
 	for (Track* track: tracks)
 	{
@@ -287,7 +287,7 @@ AutomatedValueMap TrackContainer::automatedValuesFromTracks(const TrackList &tra
 		case Track::Type::HiddenAutomation:
 		case Track::Type::Pattern:
 			if (clipNum < 0) {
-				track->getClipsInRange(clips, 0, time);
+				clips = track->getClipsInRange(0, time);
 			} else {
 				Q_ASSERT(track->numOfClips() > clipNum);
 				clips.push_back(track->getClip(clipNum));

src/gui/FileBrowser.cpp

@@ -982,7 +982,7 @@ bool FileBrowserTreeWidget::openInNewSampleTrack(FileItem* item)
 
 	// Add the sample clip to the track
 	Engine::audioEngine()->requestChangeInModel();
-	auto clip = static_cast<SampleClip*>(sampleTrack->createClip());
+	auto clip = static_cast<SampleClip*>(sampleTrack->addClip(sampleTrack->createClip()));
 	clip->setSampleFile(item->fullName());
 	Engine::audioEngine()->doneChangeInModel();
 	return true;

src/gui/clips/ClipView.cpp

@@ -1279,7 +1279,7 @@ void ClipView::mergeClips(QVector<ClipView*> clipvs)
 	const TimePos earliestPos = (*earliestClipV)->getClip()->startPosition();
 
 	// Create a clip where all notes will be added
-	auto newMidiClip = static_cast<MidiClip*>(track->createClip());
+	auto newMidiClip = static_cast<MidiClip*>(track->addClip(track->createClip()));
 
 	if (!newMidiClip)
 	{

src/gui/clips/SampleClipView.cpp

@@ -362,7 +362,7 @@ bool SampleClipView::splitClip( const TimePos pos )
 		m_clip->getTrack()->addJournalCheckPoint();
 		m_clip->getTrack()->saveJournallingState( false );
 
-		auto rightClip = m_clip->getTrack()->addClip<SampleClip>(*m_clip);
+		auto rightClip = m_clip->getTrack()->addClip(m_clip->getTrack()->createClip());
 
 		m_clip->changeLength( splitPos - m_initialClipPos );
 

src/gui/tracks/AutomationTrackView.cpp

@@ -77,7 +77,7 @@ void AutomationTrackView::dropEvent( QDropEvent * _de )
 				pos.setTicks( 0 );
 			}
 
-			auto autoClip = static_cast<AutomationClip*>(getTrack()->createClip());
+			auto autoClip = static_cast<AutomationClip*>(getTrack()->addClip(getTrack()->createClip()));
 			autoClip->movePosition(pos);
 			autoClip->addObject( mod );
 		}

src/gui/tracks/SampleTrackView.cpp

@@ -217,7 +217,7 @@ void SampleTrackView::dropEvent(QDropEvent *de)
 							* TimePos::ticksPerBar()) + trackContainerView()->currentPosition()
 						).quantize(1.0);
 
-		auto sClip = static_cast<SampleClip*>(getTrack()->createClip());
+		auto sClip = static_cast<SampleClip*>(getTrack()->addClip(getTrack()->createClip()));
 		sClip->movePosition(clipPos);
 		if (sClip) { sClip->setSampleFile(value); }
 	}

src/gui/tracks/TrackView.cpp

@@ -109,7 +109,7 @@ TrackView::TrackView( Track * track, TrackContainerView * tcv ) :
 	connect(trackGrip, &TrackGrip::released, this, &TrackView::onTrackGripReleased);
 
 	// create views for already existing clips
-	for (const auto& clip : m_track->m_clips)
+	for (const auto& clip : m_track->getClips())
 	{
 		createClipView(clip);
 	}