From 7294f7a8e0ec9885416570fda5ebefc7b69f86d8 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 30 Aug 2020 18:18:18 -0400 Subject: [PATCH] Bug2532: should preserve sharing of sample blocks when importing AUP (#651) --- src/Sequence.cpp | 51 ++++++++++++++++++++++++++++++++++++++-- src/Sequence.h | 8 +++++++ src/WaveClip.cpp | 13 ++++++++++ src/WaveClip.h | 8 +++++++ src/import/ImportAUP.cpp | 28 ++++++++++++---------- 5 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 5e323a5d8..4da6e583f 100644 --- a/src/Sequence.cpp +++ b/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, diff --git a/src/Sequence.h b/src/Sequence.h index b59e93e5f..160b419d9 100644 --- a/src/Sequence.h +++ b/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, diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index b6d78dd7e..4ed524874 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1202,6 +1202,19 @@ void WaveClip::GetDisplayRect(wxRect* r) *r = mDisplayRect; } +/*! @excsafety{Strong} */ +std::shared_ptr WaveClip::AppendNewBlock( + samplePtr buffer, sampleFormat format, size_t len) +{ + return mSequence->AppendNewBlock( buffer, format, len ); +} + +/*! @excsafety{Strong} */ +void WaveClip::AppendSharedBlock(const std::shared_ptr &pBlock) +{ + mSequence->AppendSharedBlock( pBlock ); +} + /*! @excsafety{Partial} -- Some prefix (maybe none) of the buffer is appended, and no content already flushed to disk is lost. */ diff --git a/src/WaveClip.h b/src/WaveClip.h index a29f8dbda..6a939cce0 100644 --- a/src/WaveClip.h +++ b/src/WaveClip.h @@ -26,6 +26,7 @@ class BlockArray; class Envelope; class ProgressDialog; +class SampleBlock; class SampleBlockFactory; using SampleBlockFactoryPtr = std::shared_ptr; 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 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 &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, diff --git a/src/import/ImportAUP.cpp b/src/import/ImportAUP.cpp index 2e8cf913f..67ae09c82 100644 --- a/src/import/ImportAUP.cpp +++ b/src/import/ImportAUP.cpp @@ -196,7 +196,8 @@ private: const wxChar **mAttrs; wxFileName mProjDir; - using BlockFileMap = std::map; + using BlockFileMap = + std::map>>; 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); - mClip->Flush(); - } - else if (mWaveTrack) - { - mWaveTrack->Append(bufptr, format, cnt); - mWaveTrack->Flush(); + pBlock = pClip->AppendNewBlock(bufptr, format, cnt); } // Let the finally block know everything is good