Use arrays of smart pointers to WaveClip

src/Snap.cpp

@@ -137,7 +137,7 @@ void SnapManager::Reinit()
                for (size_t j = 0, cnt = mClipExclusions->size(); j < cnt; ++j)
                {
                   if ((*mClipExclusions)[j].track == waveTrack &&
-                      (*mClipExclusions)[j].clip == clip)
+                      (*mClipExclusions)[j].clip == clip.get())
                   {
                      skip = true;
                      break;

src/Snap.h

@@ -43,6 +43,7 @@ public:
    Track *origTrack;
    Track *dstTrack;
    WaveClip *clip;
+   movable_ptr<WaveClip> holder;
 };
 
 class TrackClipArray : public std::vector < TrackClip > {};

src/TrackArtist.cpp

@@ -1472,7 +1472,7 @@ void TrackArtist::DrawWaveform(const WaveTrack *track,
          selectedRegion, zoomInfo);
 
    for (const auto &clip: track->GetClips())
-      DrawClipWaveform(track, clip, dc, rect, selectedRegion, zoomInfo,
+      DrawClipWaveform(track, clip.get(), dc, rect, selectedRegion, zoomInfo,
                        drawEnvelope, bigPoints,
                        dB, muted);
 
@@ -2011,7 +2011,7 @@ void TrackArtist::DrawSpectrum(const WaveTrack *track,
 
    WaveTrackCache cache(track);
    for (const auto &clip: track->GetClips()) {
-      DrawClipSpectrum(cache, clip, dc, rect, selectedRegion, zoomInfo);
+      DrawClipSpectrum(cache, clip.get(), dc, rect, selectedRegion, zoomInfo);
    }
 }
 

src/TrackPanel.cpp

@@ -3429,14 +3429,14 @@ void TrackPanel::AddClipsToCaptured(Track *t, double t0, double t1)
             // Avoid getting clips that were already captured
             bool newClip = true;
             for (unsigned int i = 0; i < mCapturedClipArray.size(); ++i) {
-               if (mCapturedClipArray[i].clip == clip) {
+               if (mCapturedClipArray[i].clip == clip.get()) {
                   newClip = false;
                   break;
                }
             }
 
             if (newClip)
-               mCapturedClipArray.push_back(TrackClip(t, clip));
+               mCapturedClipArray.push_back(TrackClip(t, clip.get()));
          }
       }
    }
@@ -3634,7 +3634,8 @@ void TrackPanel::DoSlide(wxMouseEvent & event)
          TrackClip &trackClip = mCapturedClipArray[ii];
          WaveClip *const pSrcClip = trackClip.clip;
          if (pSrcClip)
-            static_cast<WaveTrack*>(trackClip.track)->MoveClipToTrack(pSrcClip, NULL);
+            trackClip.holder =
+            static_cast<WaveTrack*>(trackClip.track)->RemoveAndReturnClip(pSrcClip);
       }
 
       // Now check that the move is possible
@@ -3652,7 +3653,7 @@ void TrackPanel::DoSlide(wxMouseEvent & event)
             TrackClip &trackClip = mCapturedClipArray[ii];
             WaveClip *const pSrcClip = trackClip.clip;
             if (pSrcClip) {
-               static_cast<WaveTrack*>(trackClip.track)->AddClip(pSrcClip);
+               static_cast<WaveTrack*>(trackClip.track)->AddClip(std::move(trackClip.holder));
             }
          }
          return;
@@ -3664,7 +3665,7 @@ void TrackPanel::DoSlide(wxMouseEvent & event)
             WaveClip *const pSrcClip = trackClip.clip;
             if (pSrcClip) {
                Track *const dstTrack = trackClip.dstTrack;
-               static_cast<WaveTrack*>(dstTrack)->AddClip(pSrcClip);
+               static_cast<WaveTrack*>(dstTrack)->AddClip(std::move(trackClip.holder));
                trackClip.track = dstTrack;
             }
          }

src/WaveClip.cpp

@@ -320,7 +320,7 @@ WaveClip::WaveClip(const WaveClip& orig, DirManager *projDirManager)
    mSpecPxCache = std::make_unique<SpecPxCache>(1);
 
    for (const auto &clip: orig.mCutLines)
-      mCutLines.push_back(new WaveClip(*clip, projDirManager));
+      mCutLines.push_back(make_movable<WaveClip>(*clip, projDirManager));
 
    mAppendBufferLen = 0;
    mDirty = 0;
@@ -329,9 +329,6 @@ WaveClip::WaveClip(const WaveClip& orig, DirManager *projDirManager)
 
 WaveClip::~WaveClip()
 {
-   for (const auto &clip : mCutLines)
-      delete clip;
-   mCutLines.clear();
 }
 
 void WaveClip::SetOffset(double offset)
@@ -1362,11 +1359,12 @@ XMLTagHandler *WaveClip::HandleXMLChild(const wxChar *tag)
    else if (!wxStrcmp(tag, wxT("waveclip")))
    {
       // Nested wave clips are cut lines
-      WaveClip *newCutLine = new WaveClip(mSequence->GetDirManager(),
+      mCutLines.push_back(
-                                mSequence->GetSampleFormat(), mRate);
+         make_movable<WaveClip>(mSequence->GetDirManager(),
-      mCutLines.push_back(newCutLine);
+            mSequence->GetSampleFormat(), mRate));
-      return newCutLine;
+      return mCutLines.back().get();
-   } else
+   }
+   else
       return NULL;
 }
 
@@ -1445,10 +1443,9 @@ bool WaveClip::Paste(double t0, const WaveClip* other)
       // Paste cut lines contained in pasted clip
       for (const auto &cutline: pastedClip->mCutLines)
       {
-         WaveClip* newCutLine = new WaveClip(*cutline,
+         mCutLines.push_back(
-                                             mSequence->GetDirManager());
+            make_movable<WaveClip>(*cutline, mSequence->GetDirManager()));
-         newCutLine->Offset(t0 - mOffset);
+         mCutLines.back()->Offset(t0 - mOffset);
-         mCutLines.push_back(newCutLine);
       }
 
       result = true;
@@ -1506,12 +1503,11 @@ bool WaveClip::Clear(double t0, double t1)
       // May delete as we iterate, so don't use range-for
       for (auto it = mCutLines.begin(); it != mCutLines.end();)
       {
-         WaveClip* clip = *it;
+         WaveClip* clip = it->get();
          double cutlinePosition = mOffset + clip->GetOffset();
          if (cutlinePosition >= t0 && cutlinePosition <= t1)
          {
             // This cutline is within the area, DELETE it
-            delete clip;
             it = mCutLines.erase(it);
          }
          else
@@ -1541,9 +1537,8 @@ bool WaveClip::ClearAndAddCutLine(double t0, double t1)
    if (t0 > GetEndTime() || t1 < GetStartTime())
       return true; // time out of bounds
 
-   WaveClip *newClip = new WaveClip(mSequence->GetDirManager(),
+   auto newClip = make_movable<WaveClip>
-                                    mSequence->GetSampleFormat(),
+      (mSequence->GetDirManager(), mSequence->GetSampleFormat(), mRate);
-                                    mRate);
    double clip_t0 = t0;
    double clip_t1 = t1;
    if (clip_t0 < GetStartTime())
@@ -1559,12 +1554,12 @@ bool WaveClip::ClearAndAddCutLine(double t0, double t1)
    // May delete as we iterate, so don't use range-for
    for (auto it = mCutLines.begin(); it != mCutLines.end();)
    {
-      WaveClip* clip = *it;
+      WaveClip* clip = it->get();
       double cutlinePosition = mOffset + clip->GetOffset();
       if (cutlinePosition >= t0 && cutlinePosition <= t1)
       {
          clip->SetOffset(cutlinePosition - newClip->GetOffset() - mOffset);
-         newClip->mCutLines.push_back(clip);
+         newClip->mCutLines.push_back(std::move(*it)); // transfer ownership!!
          it = mCutLines.erase(it);
       }
       else
@@ -1592,14 +1587,11 @@ bool WaveClip::ClearAndAddCutLine(double t0, double t1)
 
       MarkChanged();
 
-      mCutLines.push_back(newClip);
+      mCutLines.push_back(std::move(newClip));
       return true;
    }
    else
-   {
-      delete newClip;
       return false;
-   }
 }
 
 bool WaveClip::FindCutLine(double cutLinePosition,
@@ -1628,10 +1620,9 @@ bool WaveClip::ExpandCutLine(double cutLinePosition)
       const auto &cutline = *it;
       if (fabs(mOffset + cutline->GetOffset() - cutLinePosition) < 0.0001)
       {
-         if (!Paste(mOffset+cutline->GetOffset(), cutline))
+         if (!Paste(mOffset+cutline->GetOffset(), cutline.get()))
             return false;
-         delete cutline;
+         mCutLines.erase(it); // deletes cutline!
-         mCutLines.erase(it);
          return true;
       }
    }
@@ -1646,8 +1637,7 @@ bool WaveClip::RemoveCutLine(double cutLinePosition)
       const auto &cutline = *it;
       if (fabs(mOffset + cutline->GetOffset() - cutLinePosition) < 0.0001)
       {
-         delete cutline;
+         mCutLines.erase(it); // deletes cutline!
-         mCutLines.erase(it);
          return true;
       }
    }
@@ -1657,8 +1647,6 @@ bool WaveClip::RemoveCutLine(double cutLinePosition)
 
 void WaveClip::RemoveAllCutLines()
 {
-   for (const auto &cutLine : mCutLines)
-      delete cutLine;
    mCutLines.clear();
 }
 

src/WaveClip.h

@@ -149,8 +149,9 @@ public:
 
 class WaveClip;
 
-using WaveClipHolders = std::vector < WaveClip* > ;
+// Array of pointers that assume ownership
-using WaveClipConstHolders = std::vector < const WaveClip* > ;
+using WaveClipHolders = std::vector < movable_ptr< WaveClip > >;
+using WaveClipConstHolders = std::vector < movable_ptr< const WaveClip > >;
 
 // Temporary arrays of mere pointers
 using WaveClipPointers = std::vector < WaveClip* >;
@@ -207,6 +208,7 @@ class AUDACITY_DLL_API WaveClip final : public XMLTagHandler
 {
 private:
    // It is an error to copy a WaveClip without specifying the DirManager.
+
    WaveClip(const WaveClip&) PROHIBITED;
    WaveClip& operator= (const WaveClip&) PROHIBITED;
 

src/WaveTrack.cpp

@@ -132,7 +132,7 @@ WaveTrack::WaveTrack(const WaveTrack &orig):
    Init(orig);
 
    for (const auto &clip : orig.mClips)
-      mClips.push_back(new WaveClip(*clip, mDirManager));
+      mClips.push_back(make_movable<WaveClip>(*clip, mDirManager));
 }
 
 // Copy the track metadata but not the contents.
@@ -177,10 +177,6 @@ WaveTrack::~WaveTrack()
    //Deschedules tasks associated with this track.
    if(ODManager::IsInstanceCreated())
       ODManager::Instance()->RemoveWaveTrack(this);
-
-   for (const auto &clip : mClips)
-      delete clip;
-   mClips.clear();
 }
 
 double WaveTrack::GetOffset() const
@@ -646,17 +642,19 @@ Track::Holder WaveTrack::Copy(double t0, double t1) const
          // Whole clip is in copy region
          //printf("copy: clip %i is in copy region\n", (int)clip);
 
-         WaveClip *newClip = new WaveClip(*clip, mDirManager);
+         newTrack->mClips.push_back
+            (make_movable<WaveClip>(*clip, mDirManager));
+         WaveClip *const newClip = newTrack->mClips.back().get();
          newClip->RemoveAllCutLines();
          newClip->Offset(-t0);
-         newTrack->mClips.push_back(newClip);
+      }
-      } else
+      else
       if (t1 > clip->GetStartTime() && t0 < clip->GetEndTime())
       {
          // Clip is affected by command
          //printf("copy: clip %i is affected by command\n", (int)clip);
 
-         WaveClip *newClip = new WaveClip(*clip, mDirManager);
+         auto newClip = make_movable<WaveClip>(*clip, mDirManager);
          newClip->RemoveAllCutLines();
          double clip_t0 = t0;
          double clip_t1 = t1;
@@ -673,7 +671,7 @@ Track::Holder WaveTrack::Copy(double t0, double t1) const
 
          //printf("copy: clip offset is now %f\n", newClip->GetOffset());
 
-         if (!newClip->CreateFromCopy(clip_t0, clip_t1, clip))
+         if (!newClip->CreateFromCopy(clip_t0, clip_t1, clip.get()))
          {
             //printf("paste: CreateFromCopy(%f, %f, %i) returns false, quitting\n",
             //   clip_t0, clip_t1, (int)clip);
@@ -681,11 +679,10 @@ Track::Holder WaveTrack::Copy(double t0, double t1) const
             // could leave *dest undefined.
             // I think this is dealing with clips that don't have any sequence content
             // i.e. we don't copy cut lines and such - anyone like to explain more?
-            delete newClip;
          }
          else
          {
-            newTrack->mClips.push_back(newClip);
+            newTrack->mClips.push_back(std::move(newClip)); // transfer ownership
          }
       }
    }
@@ -694,18 +691,18 @@ Track::Holder WaveTrack::Copy(double t0, double t1) const
    // clip representing that whitespace
    if (newTrack->GetEndTime() + 1.0 / newTrack->GetRate() < t1 - t0)
    {
-      WaveClip *placeholder = new WaveClip(mDirManager,
+      auto placeholder = make_movable<WaveClip>(mDirManager,
-            newTrack->GetSampleFormat(), newTrack->GetRate());
+            newTrack->GetSampleFormat(),
+            static_cast<int>(newTrack->GetRate()));
       placeholder->SetIsPlaceholder(true);
       if ( ! placeholder->InsertSilence(
                0, (t1 - t0) - newTrack->GetEndTime()) )
       {
-         delete placeholder;
       }
       else
       {
          placeholder->Offset(newTrack->GetEndTime());
-         newTrack->mClips.push_back(placeholder);
+         newTrack->mClips.push_back(std::move(placeholder)); // transfer ownership
       }
    }
 
@@ -853,19 +850,17 @@ bool WaveTrack::ClearAndPaste(double t0, // Start of time to clear
       auto &cutlines = clip->GetCutLines();
       // May erase from cutlines, so don't use range-for
       for (auto it = cutlines.begin(); it != cutlines.end(); ) {
-         WaveClip *cut = *it;
+         WaveClip *cut = it->get();
          double cs = LongSamplesToTime(TimeToLongSamples(clip->GetOffset() +
                                                          cut->GetOffset()));
 
          // Remember cut point
          if (cs >= t0 && cs <= t1) {
-            // Remove cut point from this clips cutlines array, otherwise
-            // it will not be deleted when HandleClear() is called.
-            it = cutlines.erase(it);
 
             // Remember the absolute offset and add to our cuts array.
             cut->SetOffset(cs);
-            cuts.push_back(cut);
+            cuts.push_back(std::move(*it)); // transfer ownership!
+            it = cutlines.erase(it);
          }
          else
             ++it;
@@ -947,14 +942,14 @@ bool WaveTrack::ClearAndPaste(double t0, // Start of time to clear
 
                // Scan the cuts for any that live within this clip
                for (auto it = cuts.begin(); it != cuts.end();) {
-                  WaveClip *cut = *it;
+                  WaveClip *cut = it->get();
                   double cs = cut->GetOffset();
 
                   // Offset the cut from the start of the clip and add it to
                   // this clips cutlines.
                   if (cs >= st && cs <= et) {
                      cut->SetOffset(warper->Warp(cs) - st);
-                     clip->GetCutLines().push_back(cut);
+                     clip->GetCutLines().push_back( std::move(*it) ); // transfer ownership!
                      it = cuts.erase(it);
                   }
                   else
@@ -965,10 +960,6 @@ bool WaveTrack::ClearAndPaste(double t0, // Start of time to clear
       }
    }
 
-   // Delete cutlines that fell outside of resulting clips
-   for (const auto cut: cuts)
-      delete cut;
-
    return true;
 }
 
@@ -989,7 +980,7 @@ namespace
       auto it = list.begin();
       for (const auto end = list.end(); it != end; ++it)
       {
-         if (*it == clip)
+         if (it->get() == clip)
             break;
          if (distance)
             ++*distance;
@@ -1005,7 +996,7 @@ namespace
       auto it = list.begin();
       for (const auto end = list.end(); it != end; ++it)
       {
-         if (*it == clip)
+         if (it->get() == clip)
             break;
          if (distance)
             ++*distance;
@@ -1014,19 +1005,24 @@ namespace
    }
 }
 
-WaveClip* WaveTrack::RemoveAndReturnClip(WaveClip* clip)
+movable_ptr<WaveClip> WaveTrack::RemoveAndReturnClip(WaveClip* clip)
 {
+   // Be clear about who owns the clip!!
    auto it = FindClip(mClips, clip);
-   if (it != mClips.end())
+   if (it != mClips.end()) {
+      auto result = std::move(*it); // Array stops owning the clip, before we shrink it
       mClips.erase(it);
-   return clip;
+      return result;
+   }
+   else
+      return {};
 }
 
-void WaveTrack::AddClip(WaveClip* clip)
+void WaveTrack::AddClip(movable_ptr<WaveClip> &&clip)
 {
    // Uncomment the following line after we correct the problem of zero-length clips
    //if (CanInsertClip(clip))
-      mClips.push_back(clip);
+      mClips.push_back(std::move(clip)); // transfer ownership
 }
 
 bool WaveTrack::HandleClear(double t0, double t1,
@@ -1061,9 +1057,9 @@ bool WaveTrack::HandleClear(double t0, double t1,
       if (clip->BeforeClip(t0) && clip->AfterClip(t1))
       {
          // Whole clip must be deleted - remember this
-         clipsToDelete.push_back(clip);
+         clipsToDelete.push_back(clip.get());
-      } else
+      }
-      if (!clip->BeforeClip(t1) && !clip->AfterClip(t0))
+      else if (!clip->BeforeClip(t1) && !clip->AfterClip(t0))
       {
          // Clip data is affected by command
          if (addCutLines)
@@ -1088,16 +1084,17 @@ bool WaveTrack::HandleClear(double t0, double t1,
                   // Delete in the middle of the clip...we actually create two
                   // NEW clips out of the left and right halves...
 
-                  WaveClip *left = new WaveClip(*clip, mDirManager);
+                  // left
-                  left->Clear(t0, clip->GetEndTime());
+                  clipsToAdd.push_back(make_movable<WaveClip>(*clip, mDirManager));
-                  clipsToAdd.push_back(left);
+                  clipsToAdd.back()->Clear(t0, clip->GetEndTime());
 
-                  WaveClip *right = new WaveClip(*clip, mDirManager);
+                  // right
+                  clipsToAdd.push_back(make_movable<WaveClip>(*clip, mDirManager));
+                  WaveClip *const right = clipsToAdd.back().get();
                   right->Clear(clip->GetStartTime(), t1);
-                  right->Offset(t1-clip->GetStartTime());
+                  right->Offset(t1 - clip->GetStartTime());
-                  clipsToAdd.push_back(right);
 
-                  clipsToDelete.push_back(clip);
+                  clipsToDelete.push_back(clip.get());
                }
             }
             else { // (We are not doing a split cut)
@@ -1140,14 +1137,13 @@ bool WaveTrack::HandleClear(double t0, double t1,
    {
       auto myIt = FindClip(mClips, clip);
       if (myIt != mClips.end())
-         mClips.erase(myIt);
+         mClips.erase(myIt); // deletes the clip!
       else
          wxASSERT(false);
-      delete clip;
    }
 
-   for (const auto &clip: clipsToAdd)
+   for (auto &clip: clipsToAdd)
-      mClips.push_back(clip);
+      mClips.push_back(std::move(clip)); // transfer ownership
 
    return true;
 }
@@ -1291,7 +1287,7 @@ bool WaveTrack::Paste(double t0, const Track *src)
             {
                //printf("t0=%.6f: inside clip is %.6f ... %.6f\n",
                //       t0, clip->GetStartTime(), clip->GetEndTime());
-               insideClip = clip;
+               insideClip = clip.get();
                break;
             }
          } else
@@ -1300,7 +1296,7 @@ bool WaveTrack::Paste(double t0, const Track *src)
             if (clip->WithinClip(t0) ||
                   TimeToLongSamples(t0) == clip->GetStartSample())
             {
-               insideClip = clip;
+               insideClip = clip.get();
                break;
             }
          }
@@ -1350,11 +1346,11 @@ bool WaveTrack::Paste(double t0, const Track *src)
       // AWD Oct. 2009: Don't actually paste in placeholder clips
       if (!clip->GetIsPlaceholder())
       {
-         WaveClip* newClip = new WaveClip(*clip, mDirManager);
+         auto newClip = make_movable<WaveClip>(*clip, mDirManager);
          newClip->Resample(mRate);
          newClip->Offset(t0);
          newClip->MarkChanged();
-         mClips.push_back(newClip);
+         mClips.push_back(std::move(newClip)); // transfer ownership
       }
    }
    return true;
@@ -1528,7 +1524,7 @@ bool WaveTrack::Join(double t0, double t1)
             if ((*it)->GetStartTime() > clip->GetStartTime())
                break;
          //printf("Insert clip %.6f at position %d\n", clip->GetStartTime(), i);
-         clipsToDelete.insert(it, clip);
+         clipsToDelete.insert(it, clip.get());
       }
    }
 
@@ -1560,8 +1556,7 @@ bool WaveTrack::Join(double t0, double t1)
       t = newClip->GetEndTime();
 
       auto it = FindClip(mClips, clip);
-      mClips.erase(it);
+      mClips.erase(it); // deletes the clip
-      delete clip;
    }
 
    return true;
@@ -2171,7 +2166,7 @@ WaveClip* WaveTrack::GetClipAtX(int xcoord)
       wxRect r;
       clip->GetDisplayRect(&r);
       if (xcoord >= r.x && xcoord < r.x+r.width)
-         return clip;
+         return clip.get();
    }
 
    return NULL;
@@ -2187,7 +2182,7 @@ WaveClip* WaveTrack::GetClipAtSample(sampleCount sample)
       len   = clip->GetNumSamples();
 
       if (sample >= start && sample < start + len)
-         return clip;
+         return clip.get();
    }
 
    return NULL;
@@ -2225,9 +2220,8 @@ Sequence* WaveTrack::GetSequenceAtX(int xcoord)
 
 WaveClip* WaveTrack::CreateClip()
 {
-   WaveClip* clip = new WaveClip(mDirManager, mFormat, mRate);
+   mClips.push_back(make_movable<WaveClip>(mDirManager, mFormat, mRate));
-   mClips.push_back(clip);
+   return mClips.back().get();
-   return clip;
 }
 
 WaveClip* WaveTrack::NewestOrNewClip()
@@ -2238,7 +2232,7 @@ WaveClip* WaveTrack::NewestOrNewClip()
       return clip;
    }
    else
-      return mClips.back();
+      return mClips.back().get();
 }
 
 WaveClip* WaveTrack::RightmostOrNewClip()
@@ -2251,11 +2245,11 @@ WaveClip* WaveTrack::RightmostOrNewClip()
    else
    {
       auto it = mClips.begin();
-      WaveClip *rightmost = *it++;
+      WaveClip *rightmost = (*it++).get();
       double maxOffset = rightmost->GetOffset();
       for (auto end = mClips.end(); it != end; ++it)
       {
-         WaveClip *clip = *it;
+         WaveClip *clip = it->get();
          double offset = clip->GetOffset();
          if (maxOffset < offset)
             maxOffset = offset, rightmost = clip;
@@ -2274,9 +2268,9 @@ int WaveTrack::GetClipIndex(const WaveClip* clip) const
 WaveClip* WaveTrack::GetClipByIndex(int index)
 {
    if(index < (int)mClips.size())
-      return mClips[index];
+      return mClips[index].get();
    else
-      return NULL;
+      return nullptr;
 }
 
 const WaveClip* WaveTrack::GetClipByIndex(int index) const
@@ -2289,29 +2283,6 @@ int WaveTrack::GetNumClips() const
    return mClips.size();
 }
 
-// unused
-//void WaveTrack::MoveClipToTrack(int clipIndex, WaveTrack* dest)
-//{
-//   WaveClipHolders::compatibility_iterator node = mClips.Item(clipIndex);
-//   WaveClip* clip = node->GetData();
-//   mClips.DeleteNode(node);
-//   dest->mClips.Append(clip);
-//}
-
-void WaveTrack::MoveClipToTrack(WaveClip *clip, WaveTrack* dest)
-{
-   for (auto it = mClips.begin(), end = mClips.end(); it != end; ++it) {
-      if (*it == clip) {
-         // This could invalidate the iterators for the loop!  But we return
-         // at once so it's okay
-         mClips.erase(it);
-         if (dest)
-            dest->mClips.push_back(clip);
-         return; // JKC iterator is now 'defunct' so better return straight away.
-      }
-   }
-}
-
 bool WaveTrack::CanOffsetClip(WaveClip* clip, double amount,
                               double *allowedAmount /* = NULL */)
 {
@@ -2320,7 +2291,7 @@ bool WaveTrack::CanOffsetClip(WaveClip* clip, double amount,
 
    for (const auto &c: mClips)
    {
-      if (c != clip && c->GetStartTime() < clip->GetEndTime()+amount &&
+      if (c.get() != clip && c->GetStartTime() < clip->GetEndTime()+amount &&
                        c->GetEndTime() > clip->GetStartTime()+amount)
       {
          if (!allowedAmount)
@@ -2392,25 +2363,22 @@ bool WaveTrack::SplitAt(double t)
          if(t - 1.0/c->GetRate() >= c->GetOffset())
             c->GetEnvelope()->Insert(t - c->GetOffset() - 1.0/c->GetRate(), val);  // frame end points
          c->GetEnvelope()->Insert(t - c->GetOffset(), val);
-         WaveClip* newClip = new WaveClip(*c, mDirManager);
+         auto newClip = make_movable<WaveClip>(*c, mDirManager);
          if (!c->Clear(t, c->GetEndTime()))
          {
-            delete newClip;
             return false;
          }
          if (!newClip->Clear(c->GetStartTime(), t))
          {
-            delete newClip;
             return false;
          }
 
          //offset the NEW clip by the splitpoint (noting that it is already offset to c->GetStartTime())
          sampleCount here = llrint(floor(((t - c->GetStartTime()) * mRate) + 0.5));
          newClip->Offset((double)here/(double)mRate);
-
          // This could invalidate the iterators for the loop!  But we return
          // at once so it's okay
-         mClips.push_back(newClip);
+         mClips.push_back(std::move(newClip)); // transfer ownership
          return true;
       }
    }
@@ -2564,7 +2532,6 @@ bool WaveTrack::MergeClips(int clipidx1, int clipidx2)
    // Delete second clip
    auto it = FindClip(mClips, clip2);
    mClips.erase(it);
-   delete clip2;
 
    return true;
 }
@@ -2590,7 +2557,8 @@ namespace {
    Cont1 FillSortedClipArray(const Cont2& mClips)
    {
       Cont1 clips;
-      std::copy(mClips.begin(), mClips.end(), std::back_inserter(clips));
+      for (const auto &clip : mClips)
+         clips.push_back(clip.get());
       std::sort(clips.begin(), clips.end(),
          [](const WaveClip *a, const WaveClip *b)
       { return a->GetStartTime() < b->GetStartTime(); });

src/WaveTrack.h

@@ -384,17 +384,12 @@ class AUDACITY_DLL_API WaveTrack final : public Track {
    // existing clips).
    bool CanInsertClip(WaveClip* clip);
 
-   // Move a clip into a NEW track. This will remove the clip
+   // Remove the clip from the track and return a SMART pointer to it.
-   // in this cliplist and add it to the cliplist of the
+   // You assume responsibility for its memory!
-   // other track (if that is not NULL). No fancy additional stuff is done.
+   movable_ptr<WaveClip> RemoveAndReturnClip(WaveClip* clip);
-   // unused   void MoveClipToTrack(int clipIndex, WaveTrack* dest);
-   void MoveClipToTrack(WaveClip *clip, WaveTrack* dest);
-
-   // Remove the clip from the track and return a pointer to it.
-   WaveClip* RemoveAndReturnClip(WaveClip* clip);
 
    // Append a clip to the track
-   void AddClip(WaveClip* clip);
+   void AddClip(movable_ptr<WaveClip> &&clip); // Call using std::move
 
    // Merge two clips, that is append data from clip2 to clip1,
    // then remove clip2 from track.

src/effects/Reverse.cpp

@@ -115,7 +115,7 @@ bool EffectReverse::ProcessOneWave(int count, WaveTrack * track, sampleCount sta
    const auto &clips = track->GetClips();
    // Beware, the array grows as we loop over it.  Use integer subscripts, not iterators.
    for (int ii = 0; ii < clips.size(); ++ii) {
-      const auto &clip = clips[ii];
+      const auto &clip = clips[ii].get();
       sampleCount clipStart = clip->GetStartSample();
       sampleCount clipEnd = clip->GetEndSample();
       if (clipStart < start && clipEnd > start && clipEnd <= end) { // the reverse selection begins at the inside of a clip
@@ -189,15 +189,12 @@ bool EffectReverse::ProcessOneWave(int count, WaveTrack * track, sampleCount sta
                currentEnd = (sampleCount)(currentEnd - (clipEnd - clipStart) - (nextClipStart - clipEnd));
             }
 
-            clip = track->RemoveAndReturnClip(clip); // detach the clip from track
+            revClips.push_back(track->RemoveAndReturnClip(clip)); // detach the clip from track
-            clip->SetOffset(track->LongSamplesToTime(track->TimeToLongSamples(offsetStartTime))); // align time to a sample and set offset
+            revClips.back()->SetOffset(track->LongSamplesToTime(track->TimeToLongSamples(offsetStartTime))); // align time to a sample and set offset
-            revClips.push_back(clip);
-
          }
       }
       else if (clipStart >= end) { // clip is after the selection region
-         clip = track->RemoveAndReturnClip(clip); // simply remove and append to otherClips
+         otherClips.push_back(track->RemoveAndReturnClip(clip)); // simply remove and append to otherClips
-         otherClips.push_back(clip);
       }
    }
 
@@ -207,10 +204,10 @@ bool EffectReverse::ProcessOneWave(int count, WaveTrack * track, sampleCount sta
    // PRL:  I don't think that matters, the sequence of storage of clips in the track
    // is not elsewhere assumed to be by time
    for (auto it = revClips.rbegin(), end = revClips.rend(); it != end; ++it)
-      track->AddClip(*it);
+      track->AddClip(std::move(*it));
 
-   for (const auto &clip : otherClips)
+   for (auto &clip : otherClips)
-      track->AddClip(clip);
+      track->AddClip(std::move(clip));
 
    return rValue;
 }