Bug2532: should preserve sharing of sample blocks when importing AUP (#651)

src/Sequence.cpp

@@ -1470,11 +1470,51 @@ size_t Sequence::GetIdealAppendLen() const
       return max - lastBlockLen;
 }
 
+/*! @excsafety{Strong} */
+SeqBlock::SampleBlockPtr Sequence::AppendNewBlock(
+   samplePtr buffer, sampleFormat format, size_t len)
+{
+   return DoAppend( buffer, format, len, false );
+}
+
+/*! @excsafety{Strong} */
+void Sequence::AppendSharedBlock(const SeqBlock::SampleBlockPtr &pBlock)
+{
+   auto len = pBlock->GetSampleCount();
+
+   // Quick check to make sure that it doesn't overflow
+   if (Overflows(mNumSamples.as_double() + ((double)len)))
+      THROW_INCONSISTENCY_EXCEPTION;
+
+   BlockArray newBlock;
+   newBlock.emplace_back( pBlock, mNumSamples );
+   auto newNumSamples = mNumSamples + len;
+
+   AppendBlocksIfConsistent(newBlock, false,
+                            newNumSamples, wxT("Append"));
+
+// JKC: During generate we use Append again and again.
+// If generating a long sequence this test would give O(n^2)
+// performance - not good!
+#ifdef VERY_SLOW_CHECKING
+   ConsistencyCheck(wxT("Append"));
+#endif
+}
+
 /*! @excsafety{Strong} */
 void Sequence::Append(samplePtr buffer, sampleFormat format, size_t len)
 {
+   DoAppend(buffer, format, len, true);
+}
+
+/*! @excsafety{Strong} */
+SeqBlock::SampleBlockPtr Sequence::DoAppend(
+   samplePtr buffer, sampleFormat format, size_t len, bool coalesce)
+{
+   SeqBlock::SampleBlockPtr result;
+
    if (len == 0)
-      return;
+      return result;
 
    auto &factory = *mpFactory;
 
@@ -1492,7 +1532,8 @@ void Sequence::Append(samplePtr buffer, sampleFormat format, size_t len)
    size_t bufferSize = mMaxSamples;
    SampleBuffer buffer2(bufferSize, mSampleFormat);
    bool replaceLast = false;
-   if (numBlocks > 0 &&
+   if (coalesce &&
+       numBlocks > 0 &&
        (length =
         (pLastBlock = &mBlock.back())->sb->GetSampleCount()) < mMinSamples) {
       // Enlarge a sub-minimum block at the end
@@ -1529,6 +1570,10 @@ void Sequence::Append(samplePtr buffer, sampleFormat format, size_t len)
       SampleBlockPtr pBlock;
       if (format == mSampleFormat) {
          pBlock = factory.Create(buffer, addedLen, mSampleFormat);
+         // It's expected that when not requesting coalescence, the
+         // data should fit in one block
+         wxASSERT( coalesce || !result );
+         result = pBlock;
       }
       else {
          CopySamples(buffer, format, buffer2.ptr(), mSampleFormat, addedLen);
@@ -1551,6 +1596,8 @@ void Sequence::Append(samplePtr buffer, sampleFormat format, size_t len)
 #ifdef VERY_SLOW_CHECKING
    ConsistencyCheck(wxT("Append"));
 #endif
+
+   return result;
 }
 
 void Sequence::Blockify(SampleBlockFactory &factory,

src/Sequence.h

@@ -103,6 +103,11 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{
 
    size_t GetIdealAppendLen() const;
    void Append(samplePtr buffer, sampleFormat format, size_t len);
+
+   //! Append data, not coalescing blocks, returning a pointer to the new block.
+   SeqBlock::SampleBlockPtr AppendNewBlock(samplePtr buffer, sampleFormat format, size_t len);
+   //! Append a complete block, not coalescing
+   void AppendSharedBlock(const SeqBlock::SampleBlockPtr &pBlock);
    void Delete(sampleCount start, sampleCount len);
 
    void SetSilence(sampleCount s0, sampleCount len);
@@ -196,6 +201,9 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{
 
    int FindBlock(sampleCount pos) const;
 
+   SeqBlock::SampleBlockPtr DoAppend(
+      samplePtr buffer, sampleFormat format, size_t len, bool coalesce);
+
    static void AppendBlock(SampleBlockFactory *pFactory, sampleFormat format,
                            BlockArray &blocks,
                            sampleCount &numSamples,

src/WaveClip.cpp

@@ -1202,6 +1202,19 @@ void WaveClip::GetDisplayRect(wxRect* r)
    *r = mDisplayRect;
 }
 
+/*! @excsafety{Strong} */
+std::shared_ptr<SampleBlock> WaveClip::AppendNewBlock(
+   samplePtr buffer, sampleFormat format, size_t len)
+{
+   return mSequence->AppendNewBlock( buffer, format, len );
+}
+
+/*! @excsafety{Strong} */
+void WaveClip::AppendSharedBlock(const std::shared_ptr<SampleBlock> &pBlock)
+{
+   mSequence->AppendSharedBlock( pBlock );
+}
+
 /*! @excsafety{Partial}
  -- Some prefix (maybe none) of the buffer is appended,
 and no content already flushed to disk is lost. */

src/WaveClip.h

@@ -26,6 +26,7 @@
 class BlockArray;
 class Envelope;
 class ProgressDialog;
+class SampleBlock;
 class SampleBlockFactory;
 using SampleBlockFactoryPtr = std::shared_ptr<SampleBlockFactory>;
 class Sequence;
@@ -280,6 +281,13 @@ public:
     * function to tell the envelope about it. */
    void UpdateEnvelopeTrackLen();
 
+   //! For use in importing pre-version-3 projects to preserve sharing of blocks
+   std::shared_ptr<SampleBlock> AppendNewBlock(
+      samplePtr buffer, sampleFormat format, size_t len);
+
+   //! For use in importing pre-version-3 projects to preserve sharing of blocks
+   void AppendSharedBlock(const std::shared_ptr<SampleBlock> &pBlock);
+
    /// You must call Flush after the last Append
    /// @return true if at least one complete block was created
    bool Append(samplePtr buffer, sampleFormat format,

src/import/ImportAUP.cpp

@@ -196,7 +196,8 @@ private:
    const wxChar **mAttrs;
 
    wxFileName mProjDir;
-   using BlockFileMap = std::map<wxString, FilePath>;
+   using BlockFileMap =
+      std::map<wxString, std::pair<FilePath, std::shared_ptr<SampleBlock>>>;
    BlockFileMap mFileMap;
 
    ListOfTracks mTracks;
@@ -774,10 +775,9 @@ bool AUPImportFileHandle::HandleProject(XMLTagHandler *&handler)
                                          &files,
                                          "*.*");
 
-         for (size_t i = 0; i < cnt; ++i)
+         for (const auto &fn : files)
          {
-            FilePath fn = files[i];
+            mFileMap[wxFileNameFromPath(fn)] = {fn, {}};
-            mFileMap[wxFileNameFromPath(fn)] = fn;
          }
       }
       else if (!wxStrcmp(attr, wxT("rate")))
@@ -1199,7 +1199,7 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler)
          {
             if (mFileMap.find(strValue) != mFileMap.end())
             {
-               filename = mFileMap[strValue];
+               filename = mFileMap[strValue].first;
             }
             else
             {
@@ -1387,6 +1387,14 @@ bool AUPImportFileHandle::AddSamples(const FilePath &filename,
                                      sampleCount origin /* = 0 */,
                                      int channel /* = 0 */)
 {
+   auto pClip = mClip ? mClip : mWaveTrack->RightmostOrNewClip();
+   auto &pBlock = mFileMap[wxFileNameFromPath(filename)].second;
+   if (pBlock) {
+      // Replicate the sharing of blocks
+      pClip->AppendSharedBlock( pBlock );
+      return true;
+   }
+
    // Third party library has its own type alias, check it before
    // adding origin + size_t
    static_assert(sizeof(sampleCount::type) <= sizeof(sf_count_t),
@@ -1533,15 +1541,9 @@ bool AUPImportFileHandle::AddSamples(const FilePath &filename,
    wxASSERT(mClip || mWaveTrack);
 
    // Add the samples to the clip/track
-   if (mClip)
+   if (pClip)
    {
-      mClip->Append(bufptr, format, cnt);
+      pBlock = pClip->AppendNewBlock(bufptr, format, cnt);
-      mClip->Flush();
-   }
-   else if (mWaveTrack)
-   {
-      mWaveTrack->Append(bufptr, format, cnt);
-      mWaveTrack->Flush();
    }
 
    // Let the finally block know everything is good