From ef2b747c16a80bc4243e9d512c169f6a69160e04 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Wed, 14 Dec 2016 10:29:23 -0500 Subject: [PATCH 1/4] Define ValueRestorer for a frequent kind of RAII action --- src/MemoryX.h | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/MemoryX.h b/src/MemoryX.h index d36e095d7..2aa73abea 100644 --- a/src/MemoryX.h +++ b/src/MemoryX.h @@ -758,6 +758,46 @@ Final_action finally (F f) return Final_action(f); } +/* + * Set a variable temporarily in a scope + */ +template< typename T > +struct RestoreValue { + T oldValue; + void operator () ( T *p ) const { if (p) *p = oldValue; } +}; + +template< typename T > +class ValueRestorer : public std::unique_ptr< T, RestoreValue > +{ + using std::unique_ptr< T, RestoreValue >::reset; // make private + // But release() remains public and can be useful to commit a changed value +public: + explicit ValueRestorer( T &var ) + : std::unique_ptr< T, RestoreValue >( &var, { var } ) + {} + explicit ValueRestorer( T &var, const T& newValue ) + : std::unique_ptr< T, RestoreValue >( &var, { var } ) + { var = newValue; } + ValueRestorer(ValueRestorer &&that) + : std::unique_ptr < T, RestoreValue > ( std::move(that) ) {}; + ValueRestorer & operator= (ValueRestorer &&that) + { + if (this != &that) + std::unique_ptr < T, RestoreValue >::operator=(std::move(that)); + return *this; + } +}; + +// inline functions provide convenient parameter type deduction +template< typename T > +ValueRestorer< T > valueRestorer( T& var ) +{ return ValueRestorer< T >{ var }; } + +template< typename T > +ValueRestorer< T > valueRestorer( T& var, const T& newValue ) +{ return ValueRestorer< T >{ var, newValue }; } + /* * A convenience for use with range-for */ From aebaaf46a0b56010523e362148d14421c26342aa Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 17 Apr 2016 00:30:18 -0400 Subject: [PATCH 2/4] require unsigned arguments for Array(s)Of::reinit --- src/BlockFile.cpp | 6 ++--- src/BlockFile.h | 2 +- src/MemoryX.h | 39 ++++++++++++++++++++------- src/blockfile/LegacyBlockFile.cpp | 6 ++--- src/blockfile/ODDecodeBlockFile.cpp | 2 +- src/blockfile/ODPCMAliasBlockFile.cpp | 8 +++--- src/blockfile/SilentBlockFile.cpp | 2 +- src/blockfile/SimpleBlockFile.cpp | 11 ++++---- src/effects/Effect.cpp | 2 +- src/effects/Effect.h | 4 +-- src/import/ImportFFmpeg.cpp | 2 +- 11 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/BlockFile.cpp b/src/BlockFile.cpp index c4b8bcea6..636862344 100644 --- a/src/BlockFile.cpp +++ b/src/BlockFile.cpp @@ -349,7 +349,7 @@ void BlockFile::FixSummary(void *data) if (min != summary64K[0] || max != summary64K[1] || bad > 0) { unsigned int *buffer = (unsigned int *)data; - int len = mSummaryInfo.totalSummaryBytes / 4; + auto len = mSummaryInfo.totalSummaryBytes / 4; for(i=0; i struct default_delete { @@ -160,7 +161,7 @@ namespace std { // Skip the self-assignment test -- self-assignment should go to the non-template overload get_deleter()(p); p = that.release(); - ((D&)*this) = move(that.get_deleter()); + get_deleter() = move(that.get_deleter()); return *this; } @@ -439,11 +440,15 @@ class ArrayOf : public std::unique_ptr { public: ArrayOf() {} - explicit ArrayOf(size_t count, bool initialize = false) + + template + explicit ArrayOf(Integral count, bool initialize = false) { + static_assert(std::is_unsigned::value, "Unsigned arguments only"); reinit(count, initialize); } - ArrayOf(const ArrayOf&) = delete; + + ArrayOf(const ArrayOf&) PROHIBITED; ArrayOf(ArrayOf&& that) : std::unique_ptr < X[] > (std::move((std::unique_ptr < X[] >&)(that))) @@ -460,11 +465,16 @@ public: return *this; } - void reinit(size_t count, bool initialize = false) + template< typename Integral > + void reinit(Integral count, + bool initialize = false) { + static_assert(std::is_unsigned::value, "Unsigned arguments only"); if (initialize) + // Initialize elements (usually, to zero for a numerical type) std::unique_ptr::reset(safenew X[count]{}); else + // Avoid the slight initialization overhead std::unique_ptr::reset(safenew X[count]); } }; @@ -480,16 +490,23 @@ class ArraysOf : public ArrayOf> { public: ArraysOf() {} - explicit ArraysOf(size_t N) + + template + explicit ArraysOf(Integral N) : ArrayOf>( N ) {} - ArraysOf(size_t N, size_t M, bool initialize = false) - : ArrayOf>( N ) + + template + ArraysOf(Integral1 N, Integral2 M, bool initialize = false) + : ArrayOf>( N ) { + static_assert(std::is_unsigned::value, "Unsigned arguments only"); + static_assert(std::is_unsigned::value, "Unsigned arguments only"); for (size_t ii = 0; ii < N; ++ii) (*this)[ii] = ArrayOf{ M, initialize }; } - ArraysOf(const ArraysOf&) = delete; + + ArraysOf(const ArraysOf&) PROHIBITED; ArraysOf& operator= (ArraysOf&& that) { ArrayOf>::operator=(std::move(that)); @@ -497,8 +514,12 @@ public: } using ArrayOf>::reinit; - void reinit(size_t countN, size_t countM, bool initialize = false) + + template + void reinit(Integral1 countN, Integral2 countM, bool initialize = false) { + static_assert(std::is_unsigned::value, "Unsigned arguments only"); + static_assert(std::is_unsigned::value, "Unsigned arguments only"); reinit(countN, false); for (size_t ii = 0; ii < countN; ++ii) (*this)[ii].reinit(countM, initialize); diff --git a/src/blockfile/LegacyBlockFile.cpp b/src/blockfile/LegacyBlockFile.cpp index df10fa6e5..e4d8f49f2 100644 --- a/src/blockfile/LegacyBlockFile.cpp +++ b/src/blockfile/LegacyBlockFile.cpp @@ -159,7 +159,7 @@ LegacyBlockFile::~LegacyBlockFile() bool LegacyBlockFile::ReadSummary(void *data) { wxFFile summaryFile(mFileName.GetFullPath(), wxT("rb")); - int read; + size_t read; { Maybe silence{}; if (mSilentLog) @@ -167,14 +167,14 @@ bool LegacyBlockFile::ReadSummary(void *data) if (!summaryFile.IsOpened()){ - memset(data, 0, (size_t)mSummaryInfo.totalSummaryBytes); + memset(data, 0, mSummaryInfo.totalSummaryBytes); mSilentLog = TRUE; return true; } - read = summaryFile.Read(data, (size_t)mSummaryInfo.totalSummaryBytes); + read = summaryFile.Read(data, mSummaryInfo.totalSummaryBytes); } mSilentLog=FALSE; diff --git a/src/blockfile/ODDecodeBlockFile.cpp b/src/blockfile/ODDecodeBlockFile.cpp index 1a65d55ce..946d60f05 100644 --- a/src/blockfile/ODDecodeBlockFile.cpp +++ b/src/blockfile/ODDecodeBlockFile.cpp @@ -472,7 +472,7 @@ bool ODDecodeBlockFile::ReadSummary(void *data) if(IsSummaryAvailable()) return SimpleBlockFile::ReadSummary(data); - memset(data, 0, (size_t)mSummaryInfo.totalSummaryBytes); + memset(data, 0, mSummaryInfo.totalSummaryBytes); return true; } diff --git a/src/blockfile/ODPCMAliasBlockFile.cpp b/src/blockfile/ODPCMAliasBlockFile.cpp index 11107144a..d91539b92 100644 --- a/src/blockfile/ODPCMAliasBlockFile.cpp +++ b/src/blockfile/ODPCMAliasBlockFile.cpp @@ -518,7 +518,7 @@ bool ODPCMAliasBlockFile::ReadSummary(void *data) if( !summaryFile.IsOpened() ){ // NEW model; we need to return valid data - memset(data,0,(size_t)mSummaryInfo.totalSummaryBytes); + memset(data, 0, mSummaryInfo.totalSummaryBytes); // we silence the logging for this operation in this object // after first occurrence of error; it's already reported and @@ -529,9 +529,11 @@ bool ODPCMAliasBlockFile::ReadSummary(void *data) mFileNameMutex.Unlock(); return true; - }else mSilentLog=FALSE; // worked properly, any future error is NEW + } + else + mSilentLog=FALSE; // worked properly, any future error is NEW - int read = summaryFile.Read(data, (size_t)mSummaryInfo.totalSummaryBytes); + auto read = summaryFile.Read(data, mSummaryInfo.totalSummaryBytes); FixSummary(data); diff --git a/src/blockfile/SilentBlockFile.cpp b/src/blockfile/SilentBlockFile.cpp index 59323d424..fb7a61811 100644 --- a/src/blockfile/SilentBlockFile.cpp +++ b/src/blockfile/SilentBlockFile.cpp @@ -26,7 +26,7 @@ SilentBlockFile::~SilentBlockFile() bool SilentBlockFile::ReadSummary(void *data) { - memset(data, 0, (size_t)mSummaryInfo.totalSummaryBytes); + memset(data, 0, mSummaryInfo.totalSummaryBytes); return true; } diff --git a/src/blockfile/SimpleBlockFile.cpp b/src/blockfile/SimpleBlockFile.cpp index 36f93cb4e..69d4b0a74 100644 --- a/src/blockfile/SimpleBlockFile.cpp +++ b/src/blockfile/SimpleBlockFile.cpp @@ -131,7 +131,7 @@ SimpleBlockFile::SimpleBlockFile(wxFileNameWrapper &&baseFileName, format, cleanup); mCache.summaryData = new char[mSummaryInfo.totalSummaryBytes]; memcpy(mCache.summaryData, summaryData, - (size_t)mSummaryInfo.totalSummaryBytes); + mSummaryInfo.totalSummaryBytes); } } @@ -346,9 +346,10 @@ bool SimpleBlockFile::ReadSummary(void *data) if (mCache.active) { //wxLogDebug("SimpleBlockFile::ReadSummary(): Summary is already in cache."); - memcpy(data, mCache.summaryData, (size_t)mSummaryInfo.totalSummaryBytes); + memcpy(data, mCache.summaryData, mSummaryInfo.totalSummaryBytes); return true; - } else + } + else { //wxLogDebug("SimpleBlockFile::ReadSummary(): Reading summary from disk."); @@ -361,7 +362,7 @@ bool SimpleBlockFile::ReadSummary(void *data) // FIXME: TRAP_ERR no report to user of absent summary files? // filled with zero instead. if (!file.IsOpened()){ - memset(data, 0, (size_t)mSummaryInfo.totalSummaryBytes); + memset(data, 0, mSummaryInfo.totalSummaryBytes); mSilentLog = TRUE; return true; } @@ -373,7 +374,7 @@ bool SimpleBlockFile::ReadSummary(void *data) if( !file.Seek(sizeof(auHeader)) ) return false; - int read = (int)file.Read(data, (size_t)mSummaryInfo.totalSummaryBytes); + auto read = file.Read(data, mSummaryInfo.totalSummaryBytes); FixSummary(data); diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index 6eeac0e1e..34728b9a9 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -1555,7 +1555,7 @@ bool Effect::ProcessTrack(int count, decltype(mBufferSize) outputBufferCnt = 0; bool cleared = false; - auto chans = std::min(mNumAudioOut, mNumChannels); + auto chans = std::min(mNumAudioOut, mNumChannels); std::unique_ptr genLeft, genRight; decltype(len) genLength = 0; diff --git a/src/effects/Effect.h b/src/effects/Effect.h index a3d268568..403fccef6 100644 --- a/src/effects/Effect.h +++ b/src/effects/Effect.h @@ -501,8 +501,8 @@ private: // For client driver EffectClientInterface *mClient; - unsigned mNumAudioIn; - unsigned mNumAudioOut; + size_t mNumAudioIn; + size_t mNumAudioOut; float **mInBuffer; float **mOutBuffer; diff --git a/src/import/ImportFFmpeg.cpp b/src/import/ImportFFmpeg.cpp index 939c7234d..ad0581eb7 100644 --- a/src/import/ImportFFmpeg.cpp +++ b/src/import/ImportFFmpeg.cpp @@ -392,7 +392,7 @@ bool FFmpegImportFileHandle::InitCodecs() { // Allocate the array of pointers to hold stream contexts pointers // Some of the allocated space may be unused (corresponds to video, subtitle, or undecodeable audio streams) - mScs = std::make_shared(mFormatContext->nb_streams); + mScs = std::make_shared(size_t{mFormatContext->nb_streams}); // Fill the stream contexts for (unsigned int i = 0; i < mFormatContext->nb_streams; i++) { From 1eff721f091f37777f3c8c645bf2b62cd2518c80 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Thu, 14 Apr 2016 00:46:48 -0400 Subject: [PATCH 3/4] Define some useful type aliases for arrays of floats and doubles --- src/MemoryX.h | 12 +++++++++++- src/SampleFormat.h | 7 +++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/MemoryX.h b/src/MemoryX.h index f60eea480..205555cf8 100644 --- a/src/MemoryX.h +++ b/src/MemoryX.h @@ -513,7 +513,17 @@ public: return *this; } - using ArrayOf>::reinit; + template< typename Integral > + void reinit(Integral count) + { + ArrayOf>::reinit( count ); + } + + template< typename Integral > + void reinit(Integral count, bool initialize) + { + ArrayOf>::reinit( count, initialize ); + } template void reinit(Integral1 countN, Integral2 countM, bool initialize = false) diff --git a/src/SampleFormat.h b/src/SampleFormat.h index 67f312fec..f41d881d0 100644 --- a/src/SampleFormat.h +++ b/src/SampleFormat.h @@ -12,6 +12,7 @@ #define __AUDACITY_SAMPLE_FORMAT__ #include "Audacity.h" +#include "MemoryX.h" #include #include "audacity/Types.h" @@ -151,4 +152,10 @@ void ReverseSamples(samplePtr buffer, sampleFormat format, void InitDitherers(); +// These are so commonly done for processing samples in floating point form in memory, +// let's have abbeviations. +using Floats = ArrayOf; +using FloatBuffers = ArraysOf; +using Doubles = ArrayOf; + #endif From 5ca8766c5262eda28eb383d041e7c2fd50124bd1 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sat, 13 Aug 2016 11:22:03 -0400 Subject: [PATCH 4/4] define freer --- src/MemoryX.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/MemoryX.h b/src/MemoryX.h index 205555cf8..1eb296e30 100644 --- a/src/MemoryX.h +++ b/src/MemoryX.h @@ -750,6 +750,23 @@ make_movable_with_deleter(const Deleter &d, Args&&... args) return movable_ptr_with_deleter(safenew T(std::forward(args)...), d); } +/* + * A deleter for pointers obtained with malloc + */ +struct freer { void operator() (void *p) const { free(p); } }; + +/* + * A useful alias for holding the result of malloc + */ +template< typename T > +using MallocPtr = std::unique_ptr< T, freer >; + +/* + * A useful alias for holding the result of strup and similar + */ +template +using MallocString = std::unique_ptr< Character[], freer >; + /* * A deleter class to supply the second template parameter of unique_ptr for * classes like wxWindow that should be sent a message called Destroy rather