From 6eb5f3ac5bb251c7524d72020100b232ebbb8537 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 27 Nov 2020 21:06:39 -0500 Subject: [PATCH 1/9] Eliminate CopySamples calls where destination is always float... .. Call the function SamplesToFloats instead, or in one place, where source is also always float, just do memcpy. Dithering never happened in these cases. --- src/AudioIO.cpp | 11 ++++------- src/SampleFormat.cpp | 9 +++++++++ src/SampleFormat.h | 13 +++++++++++++ src/SqliteSampleBlock.cpp | 7 ++----- src/WaveClip.cpp | 6 +++--- src/effects/nyquist/Nyquist.cpp | 6 +++--- 6 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/AudioIO.cpp b/src/AudioIO.cpp index 9ccc16667..994d9293f 100644 --- a/src/AudioIO.cpp +++ b/src/AudioIO.cpp @@ -3622,11 +3622,9 @@ static void DoSoftwarePlaythrough(const void *inputBuffer, { for (unsigned int i=0; i < inputChannels; i++) { samplePtr inputPtr = ((samplePtr)inputBuffer) + (i * SAMPLE_SIZE(inputFormat)); - samplePtr outputPtr = ((samplePtr)outputBuffer) + (i * SAMPLE_SIZE(floatSample)); - CopySamples(inputPtr, inputFormat, - (samplePtr)outputPtr, floatSample, - len, true, inputChannels, 2); + SamplesToFloats(inputPtr, inputFormat, + outputBuffer + i, len, inputChannels, 2); } // One mono input channel goes to both output channels... @@ -4411,9 +4409,8 @@ int AudioIoCallback::AudioCallback(const void *inputBuffer, void *outputBuffer, inputSamples = (float *) inputBuffer; } else { - CopySamples((samplePtr)inputBuffer, mCaptureFormat, - (samplePtr)tempFloats, floatSample, - framesPerBuffer * numCaptureChannels); + SamplesToFloats(reinterpret_cast(inputBuffer), + mCaptureFormat, tempFloats, framesPerBuffer * numCaptureChannels); inputSamples = tempFloats; } diff --git a/src/SampleFormat.cpp b/src/SampleFormat.cpp index de13bfbd6..58f06d568 100644 --- a/src/SampleFormat.cpp +++ b/src/SampleFormat.cpp @@ -99,6 +99,15 @@ void ReverseSamples(samplePtr dst, sampleFormat format, } } +void SamplesToFloats(constSamplePtr src, sampleFormat srcFormat, + float *dst, size_t len, size_t srcStride, size_t dstStride) +{ + CopySamples( src, srcFormat, + reinterpret_cast(dst), floatSample, len, + true, // doesn't matter, no dither happens when destination is float + srcStride, dstStride); +} + void CopySamples(constSamplePtr src, sampleFormat srcFormat, samplePtr dst, sampleFormat dstFormat, unsigned int len, diff --git a/src/SampleFormat.h b/src/SampleFormat.h index eec4009dc..344c8add8 100644 --- a/src/SampleFormat.h +++ b/src/SampleFormat.h @@ -124,6 +124,19 @@ private: // Copying, Converting and Clearing Samples // +AUDACITY_DLL_API +//! Copy samples from any format into the widest format, which is 32 bit float, with no dithering +/*! + @param src address of source samples + @param srcFormat format of source samples, determines sizeof each one + @param dst address of floating-point numbers + @param len count of samples to copy + @param srcStride how many samples to advance src after copying each one + @param dstString how many samples to advance dst after copying each one + */ +void SamplesToFloats(constSamplePtr src, sampleFormat srcFormat, + float *dst, size_t len, size_t srcStride = 1, size_t dstStride = 1); + AUDACITY_DLL_API void CopySamples(constSamplePtr src, sampleFormat srcFormat, samplePtr dst, sampleFormat dstFormat, diff --git a/src/SqliteSampleBlock.cpp b/src/SqliteSampleBlock.cpp index e647a4c78..645b4ef13 100644 --- a/src/SqliteSampleBlock.cpp +++ b/src/SqliteSampleBlock.cpp @@ -806,11 +806,8 @@ void SqliteSampleBlock::CalcSummary(Sizes sizes) else { samplebuffer.reinit((unsigned) mSampleCount); - CopySamples(mSamples.get(), - mSampleFormat, - (samplePtr) samplebuffer.get(), - floatSample, - mSampleCount); + SamplesToFloats(mSamples.get(), mSampleFormat, + samplebuffer.get(), mSampleCount); samples = samplebuffer.get(); } diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 38fb6577c..74f0eb53d 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -525,9 +525,9 @@ bool WaveClip::GetWaveDisplay(WaveDisplay &display, double t0, else { b.reinit(len); pb = b.get(); - CopySamples(mAppendBuffer.ptr() + sLeft * SAMPLE_SIZE(seqFormat), - seqFormat, - (samplePtr)pb, floatSample, len); + SamplesToFloats( + mAppendBuffer.ptr() + sLeft * SAMPLE_SIZE(seqFormat), + seqFormat, pb, len); } float theMax, theMin, sumsq; diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index 4e9c3faf7..d011e3243 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -29,6 +29,7 @@ effects from this one class. #include #include +#include #include @@ -2462,9 +2463,8 @@ int NyquistEffect::GetCallback(float *buffer, int ch, // We have guaranteed above that this is nonnegative and bounded by // mCurBufferLen[ch]: auto offset = ( mCurStart[ch] + start - mCurBufferStart[ch] ).as_size_t(); - CopySamples(mCurBuffer[ch].ptr() + offset*SAMPLE_SIZE(floatSample), floatSample, - (samplePtr)buffer, floatSample, - len); + const void *src = mCurBuffer[ch].ptr() + offset*SAMPLE_SIZE(floatSample); + std::memcpy(buffer, src, len * sizeof(float)); if (ch == 0) { double progress = mScale * From 50a26d9cafb0e904c7ebbe515b893ce85c17accd Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sat, 28 Nov 2020 12:53:53 -0500 Subject: [PATCH 2/9] CopySamples gives more than binary choice of dither algorithm... ... And so a separate function CopySamplesNoDither is not needed --- src/Mix.cpp | 4 ++-- src/SampleFormat.cpp | 29 ++++++++--------------------- src/SampleFormat.h | 28 +++++++++++++++------------- src/WaveClip.cpp | 2 +- src/export/ExportPCM.cpp | 7 ++++--- src/import/ImportAUP.cpp | 2 +- 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/Mix.cpp b/src/Mix.cpp index 158343c88..8f8eff3c1 100644 --- a/src/Mix.cpp +++ b/src/Mix.cpp @@ -691,7 +691,7 @@ size_t Mixer::Process(size_t maxToProcess) mBuffer[0].ptr() + (c * SAMPLE_SIZE(mFormat)), mFormat, maxOut, - mHighQuality, + mHighQuality ? gHighQualityDither : gLowQualityDither, mNumChannels, mNumChannels); } @@ -703,7 +703,7 @@ size_t Mixer::Process(size_t maxToProcess) mBuffer[c].ptr(), mFormat, maxOut, - mHighQuality); + mHighQuality ? gHighQualityDither : gLowQualityDither); } } // MB: this doesn't take warping into account, replaced with code based on mSamplePos diff --git a/src/SampleFormat.cpp b/src/SampleFormat.cpp index 58f06d568..1d4e7baee 100644 --- a/src/SampleFormat.cpp +++ b/src/SampleFormat.cpp @@ -43,11 +43,10 @@ #include #include "Prefs.h" -#include "Dither.h" #include "Internat.h" -static DitherType gLowQualityDither = DitherType::none; -static DitherType gHighQualityDither = DitherType::none; +DitherType gLowQualityDither = DitherType::none; +DitherType gHighQualityDither = DitherType::shaped; static Dither gDitherAlgorithm; void InitDitherers() @@ -104,29 +103,17 @@ void SamplesToFloats(constSamplePtr src, sampleFormat srcFormat, { CopySamples( src, srcFormat, reinterpret_cast(dst), floatSample, len, - true, // doesn't matter, no dither happens when destination is float + DitherType::none, srcStride, dstStride); } void CopySamples(constSamplePtr src, sampleFormat srcFormat, - samplePtr dst, sampleFormat dstFormat, - unsigned int len, - bool highQuality, /* = true */ - unsigned int srcStride /* = 1 */, - unsigned int dstStride /* = 1 */) + samplePtr dst, sampleFormat dstFormat, size_t len, + DitherType ditherType, /* = gHighQualityDither */ + unsigned int srcStride /* = 1 */, + unsigned int dstStride /* = 1 */) { gDitherAlgorithm.Apply( - highQuality ? gHighQualityDither : gLowQualityDither, - src, srcFormat, dst, dstFormat, len, srcStride, dstStride); -} - -void CopySamplesNoDither(samplePtr src, sampleFormat srcFormat, - samplePtr dst, sampleFormat dstFormat, - unsigned int len, - unsigned int srcStride /* = 1 */, - unsigned int dstStride /* = 1 */) -{ - gDitherAlgorithm.Apply( - DitherType::none, + ditherType, src, srcFormat, dst, dstFormat, len, srcStride, dstStride); } diff --git a/src/SampleFormat.h b/src/SampleFormat.h index 344c8add8..90e88ed1f 100644 --- a/src/SampleFormat.h +++ b/src/SampleFormat.h @@ -2,7 +2,7 @@ Audacity: A Digital Audio Editor - SampleFormat.h + @file SampleFormat.h Dominic Mazzoni @@ -17,11 +17,15 @@ #include #include "audacity/Types.h" +#include "Dither.h" // // Definitions / Meta-Information // +//! These global variables are assigned at application startup or after change of preferences. +extern AUDACITY_DLL_API DitherType gLowQualityDither, gHighQualityDither; + #if 0 // Moved to audacity/types.h typedef enum { @@ -138,18 +142,16 @@ void SamplesToFloats(constSamplePtr src, sampleFormat srcFormat, float *dst, size_t len, size_t srcStride = 1, size_t dstStride = 1); AUDACITY_DLL_API -void CopySamples(constSamplePtr src, sampleFormat srcFormat, - samplePtr dst, sampleFormat dstFormat, - unsigned int len, bool highQuality=true, - unsigned int srcStride=1, - unsigned int dstStride=1); - -AUDACITY_DLL_API -void CopySamplesNoDither(samplePtr src, sampleFormat srcFormat, - samplePtr dst, sampleFormat dstFormat, - unsigned int len, - unsigned int srcStride=1, - unsigned int dstStride=1); +//! Copy samples from any format to any other format; apply dithering only if narrowing the format +/*! + @copydetails SamplesToFloats() + @param dstFormat format of destination samples, determines sizeof each one + @param ditherType choice of dithering algorithm to use if narrowing the format + */ +void CopySamples(constSamplePtr src, sampleFormat srcFormat, + samplePtr dst, sampleFormat dstFormat, size_t len, + DitherType ditherType = gHighQualityDither, //!< default is loaded from a global variable + unsigned int srcStride=1, unsigned int dstStride=1); AUDACITY_DLL_API void ClearSamples(samplePtr buffer, sampleFormat format, diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 74f0eb53d..b8ba2576d 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1248,7 +1248,7 @@ bool WaveClip::Append(constSamplePtr buffer, sampleFormat format, mAppendBuffer.ptr() + mAppendBufferLen * SAMPLE_SIZE(seqFormat), seqFormat, toCopy, - true, // high quality + gHighQualityDither, stride); mAppendBufferLen += toCopy; diff --git a/src/export/ExportPCM.cpp b/src/export/ExportPCM.cpp index 529bf3245..2a0258b9b 100644 --- a/src/export/ExportPCM.cpp +++ b/src/export/ExportPCM.cpp @@ -643,12 +643,13 @@ ProgressResult ExportPCM::Export(AudacityProject *project, CopySamples( mixed + (c * SAMPLE_SIZE(format)), format, dither.data() + (c * SAMPLE_SIZE(int24Sample)), int24Sample, - numSamples, true, info.channels, info.channels + numSamples, gHighQualityDither, info.channels, info.channels ); - CopySamplesNoDither( + // Copy back without dither + CopySamples( dither.data() + (c * SAMPLE_SIZE(int24Sample)), int24Sample, mixed + (c * SAMPLE_SIZE(format)), format, - numSamples, info.channels, info.channels); + numSamples, DitherType::none, info.channels, info.channels); } } diff --git a/src/import/ImportAUP.cpp b/src/import/ImportAUP.cpp index 0484c7a6c..6228bd6f5 100644 --- a/src/import/ImportAUP.cpp +++ b/src/import/ImportAUP.cpp @@ -1640,7 +1640,7 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, bufptr, format, framesRead, - true /* high quality by default */, + gHighQualityDither /* high quality by default */, channels /* source stride */); } From 0053c61c0880c5cf2c6072716a9e3a91c26e4e3e Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 30 Nov 2020 19:58:00 -0500 Subject: [PATCH 3/9] Do not dither samples as they pass through RingBuffer... ... See allocation of RingBuffers in AudioIO. Playback buffers always used floatSample format so this change has no effect on them. But we also want no extra dithering applied during recording, where the capture format might be narrower than float. --- src/RingBuffer.cpp | 4 ++-- src/RingBuffer.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/RingBuffer.cpp b/src/RingBuffer.cpp index 2020cd521..a5d54dac6 100644 --- a/src/RingBuffer.cpp +++ b/src/RingBuffer.cpp @@ -85,7 +85,7 @@ size_t RingBuffer::Put(samplePtr buffer, sampleFormat format, CopySamples(src, format, mBuffer.ptr() + pos * SAMPLE_SIZE(mFormat), mFormat, - block); + block, DitherType::none); src += block * SAMPLE_SIZE(format); pos = (pos + block) % mBufferSize; @@ -167,7 +167,7 @@ size_t RingBuffer::Get(samplePtr buffer, sampleFormat format, CopySamples(mBuffer.ptr() + start * SAMPLE_SIZE(mFormat), mFormat, dest, format, - block); + block, DitherType::none); dest += block * SAMPLE_SIZE(format); start = (start + block) % mBufferSize; diff --git a/src/RingBuffer.h b/src/RingBuffer.h index dad54b709..e92a0fc1e 100644 --- a/src/RingBuffer.h +++ b/src/RingBuffer.h @@ -24,6 +24,7 @@ class RingBuffer { // size_t AvailForPut(); + //! Does not apply dithering size_t Put(samplePtr buffer, sampleFormat format, size_t samples, // optional number of trailing zeroes size_t padding = 0); @@ -34,6 +35,7 @@ class RingBuffer { // size_t AvailForGet(); + //! Does not apply dithering size_t Get(samplePtr buffer, sampleFormat format, size_t samples); size_t Discard(size_t samples); From 43310b6a1e6959ee1f92f55d40439b50d7966906 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sat, 22 May 2021 14:25:53 -0400 Subject: [PATCH 4/9] Review of ImportAup.cpp ... ... incidental to a review of CopySamples in it. This code is used only when importing an old-style .aup project file into the new project format. Fixed some RAII for file handle. Rewrote every call to IsGoodInt or IsGoodInt64 with a narrowly scoped temporary variable. Use IsGoodInt64 only for total track lengths and positions; but use IsGoodInt for block and blockfile sizes, which are not supposed to be huge but are supposed to fit in memory buffers. --- src/import/ImportAUP.cpp | 68 +++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/import/ImportAUP.cpp b/src/import/ImportAUP.cpp index 6228bd6f5..f26166cab 100644 --- a/src/import/ImportAUP.cpp +++ b/src/import/ImportAUP.cpp @@ -221,6 +221,10 @@ private: AUPImportPlugin::AUPImportPlugin() : ImportPlugin(FileExtensions(exts.begin(), exts.end())) { + static_assert( + sizeof(long long) >= sizeof(uint64_t) && + sizeof(long) >= sizeof(uint32_t), + "Assumptions about sizes in XMLValueChecker calls are invalid!"); } AUPImportPlugin::~AUPImportPlugin() @@ -643,8 +647,6 @@ bool AUPImportFileHandle::HandleProject(XMLTagHandler *&handler) { const wxChar *attr = *mAttrs++; const wxChar *value = *mAttrs++; - long lValue; - long long llValue; double dValue; if (!value) @@ -664,6 +666,7 @@ bool AUPImportFileHandle::HandleProject(XMLTagHandler *&handler) // ViewInfo if (!wxStrcmp(attr, wxT("vpos"))) { + long lValue; if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue < 0)) { return SetError(XO("Invalid project 'vpos' attribute.")); @@ -1115,14 +1118,13 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler) break; } - long long nValue = 0; - const wxString strValue = value; // promote string, we need this for all if (!wxStrcmp(attr, wxT("maxsamples"))) { // This attribute is a sample count, so can be 64bit - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0)) + long long llvalue; + if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0)) { return SetError(XO("Invalid sequence 'maxsamples' attribute.")); } @@ -1130,7 +1132,7 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler) // Dominic, 12/10/2006: // Let's check that maxsamples is >= 1024 and <= 64 * 1024 * 1024 // - that's a pretty wide range of reasonable values. - if ((nValue < 1024) || (nValue > 64 * 1024 * 1024)) + if ((llvalue < 1024) || (llvalue > 64 * 1024 * 1024)) { return SetError(XO("Invalid sequence 'maxsamples' attribute.")); } @@ -1150,7 +1152,8 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler) else if (!wxStrcmp(attr, wxT("numsamples"))) { // This attribute is a sample count, so can be 64bit - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0)) + long long llvalue; + if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0)) { return SetError(XO("Invalid sequence 'numsamples' attribute.")); } @@ -1169,8 +1172,6 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler) const wxChar *attr = *mAttrs++; const wxChar *value = *mAttrs++; - long long nValue = 0; - if (!value) { break; @@ -1181,7 +1182,8 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler) if (!wxStrcmp(attr, wxT("start"))) { // making sure that values > 2^31 are OK because long clips will need them. - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0)) + long long llvalue; + if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0)) { return SetError(XO("Unable to parse the waveblock 'start' attribute")); } @@ -1196,13 +1198,12 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler) bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler) { FilePath filename; - sampleCount len = 0; + size_t len = 0; while (*mAttrs) { const wxChar *attr = *mAttrs++; const wxChar *value = *mAttrs++; - long long nValue; if (!value) { @@ -1229,12 +1230,13 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler) } else if (!wxStrcmp(attr, wxT("len"))) { - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue <= 0)) + long lValue; + if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue <= 0)) { return SetError(XO("Missing or invalid simpleblockfile 'len' attribute.")); } - len = nValue; + len = lValue; } } @@ -1248,13 +1250,12 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler) bool AUPImportFileHandle::HandleSilentBlockFile(XMLTagHandler *&handler) { FilePath filename; - sampleCount len = 0; + size_t len = 0; while (*mAttrs) { const wxChar *attr = *mAttrs++; const wxChar *value = *mAttrs++; - long long nValue; if (!value) { @@ -1265,12 +1266,13 @@ bool AUPImportFileHandle::HandleSilentBlockFile(XMLTagHandler *&handler) if (!wxStrcmp(attr, wxT("len"))) { - if (!XMLValueChecker::IsGoodInt64(value) || !strValue.ToLongLong(&nValue) || !(nValue > 0)) + long lValue; + if (!XMLValueChecker::IsGoodInt(value) || !strValue.ToLong(&lValue) || !(lValue > 0)) { return SetError(XO("Missing or invalid silentblockfile 'len' attribute.")); } - len = nValue; + len = lValue; } } @@ -1286,7 +1288,7 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler) wxString summaryFilename; wxFileName filename; sampleCount start = 0; - sampleCount len = 0; + size_t len = 0; int channel = 0; wxString name; @@ -1294,7 +1296,6 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler) { const wxChar *attr = *mAttrs++; const wxChar *value = *mAttrs++; - long long nValue; if (!value) { @@ -1328,31 +1329,33 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler) } else if (!wxStricmp(attr, wxT("aliasstart"))) { - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0)) + long long llValue; + if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llValue) || (llValue < 0)) { return SetError(XO("Missing or invalid pcmaliasblockfile 'aliasstart' attribute.")); } - start = nValue; + start = llValue; } else if (!wxStricmp(attr, wxT("aliaslen"))) { - if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue <= 0)) + long lValue; + if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue <= 0)) { return SetError(XO("Missing or invalid pcmaliasblockfile 'aliaslen' attribute.")); } - len = nValue; + len = lValue; } else if (!wxStricmp(attr, wxT("aliaschannel"))) { - long nValue; - if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&nValue) || (nValue < 0)) + long lValue; + if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue < 0)) { return SetError(XO("Missing or invalid pcmaliasblockfile 'aliaslen' attribute.")); } - channel = nValue; + channel = lValue; } } @@ -1512,6 +1515,12 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, auto cleanup = finally([&] { + // Do this before any throwing might happen + if (sf) + { + SFCall(sf_close, sf); + } + if (!success) { SetWarning(XO("Error while processing %s\n\nInserting silence.").Format(audioFilename)); @@ -1522,11 +1531,6 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, // If this does throw, let that propagate, don't guard the call AddSilence(len); } - - if (sf) - { - SFCall(sf_close, sf); - } }); if (!f.Open(audioFilename)) From 528b57ff8e9c6cafc566978592c87c2717769308 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Tue, 1 Dec 2020 14:58:08 -0500 Subject: [PATCH 5/9] Comments about dithering in ImportAup --- src/import/ImportAUP.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/import/ImportAUP.cpp b/src/import/ImportAUP.cpp index f26166cab..1c471fae6 100644 --- a/src/import/ImportAUP.cpp +++ b/src/import/ImportAUP.cpp @@ -1573,6 +1573,8 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, size_t framesRead = 0; + // These cases preserve the logic formerly in BlockFile.cpp, + // which was deleted at commit 98d1468. if (channels == 1 && format == int16Sample && sf_subtype_is_integer(info.format)) { // If both the src and dest formats are integer formats, @@ -1624,6 +1626,18 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, } else { + /* + Therefore none of the three cases above: + !(channels == 1 && format == int16Sample && sf_subtype_is_integer(info.format)) + && + !(channels == 1 && format == int24Sample && sf_subtype_is_integer(info.format)) + && + !(format == int16Sample && !sf_subtype_more_than_16_bits(info.format)) + + So format is not 16 bits with wider file format (third conjunct), + but still maybe it is 24 bits with float file format (second conjunct). + */ + // Otherwise, let libsndfile handle the conversion and // scaling, and pass us normalized data as floats. We can // then convert to whatever format we want. @@ -1639,6 +1653,19 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename, return true; } + /* + Dithering will happen in CopySamples if format is 24 bits. + Should that be done? + + Either the file is an ordinary simple block file -- and presumably the + track was saved specifying a matching format, so format is float and + there is no dithering. + + Or else this is the very unusual case of an .auf file, importing PCM data + on demand. The destination format is narrower, requiring dither, only + if the user also specified a narrow format for the track. In such a + case, dithering is right. + */ CopySamples((samplePtr)(tmpptr + channel), floatSample, bufptr, From 4fcb8bffbf895bbf838f47fa4c4c5470365408e3 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Tue, 1 Dec 2020 14:58:08 -0500 Subject: [PATCH 6/9] Comments about dithering in SqliteSampleBlock --- src/SqliteSampleBlock.cpp | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/SqliteSampleBlock.cpp b/src/SqliteSampleBlock.cpp index 645b4ef13..4128be9e4 100644 --- a/src/SqliteSampleBlock.cpp +++ b/src/SqliteSampleBlock.cpp @@ -593,6 +593,42 @@ size_t SqliteSampleBlock::GetBlob(void *dest, srcoffset += 0; } + /* + Will dithering happen in CopySamples? Answering this as of 3.0.3 by + examining all uses. + + As this function is called from GetSummary, no, because destination format + is float. + + There is only one other call to this function, in DoGetSamples. At one + call to that function, in DoGetMinMaxRMS, again format is float always. + + There is only one other call to DoGetSamples, in SampleBlock::GetSamples(). + In one call to that function, in WaveformView.cpp, again format is float. + + That leaves two calls in Sequence.cpp. One of those can be proved to be + used only in copy and paste operations, always supplying the same sample + format as the samples were stored in, therefore no dither. + + That leaves uses of Sequence::Read(). There are uses of Read() in internal + operations also easily shown to use only the saved format, and + GetWaveDisplay() always reads as float. + + The remaining use of Sequence::Read() is in Sequence::Get(). That is used + by WaveClip::Resample(), always fetching float. It is also used in + WaveClip::GetSamples(). + + There is only one use of that function not always fetching float, in + WaveTrack::Get(). + + It can be shown that the only paths to WaveTrack::Get() not specifying + floatSample are in Benchmark, which is only a diagnostic test, and there + the sample format is the same as what the track was constructed with. + + Therefore, no dithering even there! + */ + wxASSERT(destformat == floatSample || destformat == srcformat); + CopySamples(src + srcoffset, srcformat, (samplePtr) dest, From 0aa8625cff21b2679295d17c4e6846acad59a2b5 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 23 May 2021 17:25:55 -0400 Subject: [PATCH 7/9] WaveTrackCache was only used with float; don't support other formats --- src/Mix.cpp | 9 +++++---- src/WaveClip.cpp | 4 ++-- src/WaveTrack.cpp | 33 ++++++++++++++++++++------------- src/WaveTrack.h | 18 ++++++++---------- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/Mix.cpp b/src/Mix.cpp index 8f8eff3c1..96dbc8b13 100644 --- a/src/Mix.cpp +++ b/src/Mix.cpp @@ -460,7 +460,8 @@ size_t Mixer::MixVariableRates(int *channelFlags, WaveTrackCache &cache, // Nothing to do if past end of play interval if (getLen > 0) { if (backwards) { - auto results = cache.Get(floatSample, *pos - (getLen - 1), getLen, mMayThrow); + auto results = + cache.GetFloats(*pos - (getLen - 1), getLen, mMayThrow); if (results) memcpy(&queue[*queueLen], results, sizeof(float) * getLen); else @@ -472,7 +473,7 @@ size_t Mixer::MixVariableRates(int *channelFlags, WaveTrackCache &cache, *pos -= getLen; } else { - auto results = cache.Get(floatSample, *pos, getLen, mMayThrow); + auto results = cache.GetFloats(*pos, getLen, mMayThrow); if (results) memcpy(&queue[*queueLen], results, sizeof(float) * getLen); else @@ -589,7 +590,7 @@ size_t Mixer::MixSameRate(int *channelFlags, WaveTrackCache &cache, ); if (backwards) { - auto results = cache.Get(floatSample, *pos - (slen - 1), slen, mMayThrow); + auto results = cache.GetFloats(*pos - (slen - 1), slen, mMayThrow); if (results) memcpy(mFloatBuffer.get(), results, sizeof(float) * slen); else @@ -602,7 +603,7 @@ size_t Mixer::MixSameRate(int *channelFlags, WaveTrackCache &cache, *pos -= slen; } else { - auto results = cache.Get(floatSample, *pos, slen, mMayThrow); + auto results = cache.GetFloats(*pos, slen, mMayThrow); if (results) memcpy(mFloatBuffer.get(), results, sizeof(float) * slen); else diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index b8ba2576d..98859980c 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -707,8 +707,8 @@ bool SpecCache::CalculateOneSpectrum } if (myLen > 0) { - useBuffer = (float*)(waveTrackCache.Get( - floatSample, sampleCount( + useBuffer = (float*)(waveTrackCache.GetFloats( + sampleCount( floor(0.5 + from.as_double() + offset * rate) ), myLen, diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index b66e1a51b..dda66e2dc 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -2542,9 +2542,10 @@ void WaveTrackCache::SetTrack(const std::shared_ptr &pTrack) } } -constSamplePtr WaveTrackCache::Get(sampleFormat format, +const float *WaveTrackCache::GetFloats( sampleCount start, size_t len, bool mayThrow) { + constexpr auto format = floatSample; if (format == floatSample && len > 0) { const auto end = start + len; @@ -2596,7 +2597,7 @@ constSamplePtr WaveTrackCache::Get(sampleFormat format, if (!mPTrack->Get( samplePtr(mBuffers[0].data.get()), floatSample, start0, len0, fillZero, mayThrow)) - return 0; + return nullptr; mBuffers[0].start = start0; mBuffers[0].len = len0; if (!fillSecond && @@ -2622,7 +2623,7 @@ constSamplePtr WaveTrackCache::Get(sampleFormat format, const auto len1 = mPTrack->GetBestBlockSize(start1); wxASSERT(len1 <= mBufferSize); if (!mPTrack->Get(samplePtr(mBuffers[1].data.get()), floatSample, start1, len1, fillZero, mayThrow)) - return 0; + return nullptr; mBuffers[1].start = start1; mBuffers[1].len = len1; mNValidBuffers = 2; @@ -2648,7 +2649,7 @@ constSamplePtr WaveTrackCache::Get(sampleFormat format, auto sinitLen = initLen.as_size_t(); if (!mPTrack->Get(mOverlapBuffer.ptr(), format, start, sinitLen, fillZero, mayThrow)) - return 0; + return nullptr; wxASSERT( sinitLen <= remaining ); remaining -= sinitLen; start += initLen; @@ -2669,7 +2670,7 @@ constSamplePtr WaveTrackCache::Get(sampleFormat format, // All is contiguous already. We can completely avoid copying // leni is nonnegative, therefore start falls within mBuffers[ii], // so starti is bounded between 0 and buffer length - return samplePtr(mBuffers[ii].data.get() + starti.as_size_t() ); + return mBuffers[ii].data.get() + starti.as_size_t() ; } else if (leni > 0) { // leni is nonnegative, therefore start falls within mBuffers[ii] @@ -2700,15 +2701,21 @@ constSamplePtr WaveTrackCache::Get(sampleFormat format, return 0; } - return mOverlapBuffer.ptr(); + // Overlap buffer was meant for the more general support of sample formats + // besides float, which explains the cast + return reinterpret_cast(mOverlapBuffer.ptr()); + } + else { +#if 0 + // Cache works only for float format. + mOverlapBuffer.Resize(len, format); + if (mPTrack->Get(mOverlapBuffer.ptr(), format, start, len, fillZero, mayThrow)) + return mOverlapBuffer.ptr(); +#else + // No longer handling other than float format. Therefore len is 0. +#endif + return nullptr; } - - // Cache works only for float format. - mOverlapBuffer.Resize(len, format); - if (mPTrack->Get(mOverlapBuffer.ptr(), format, start, len, fillZero, mayThrow)) - return mOverlapBuffer.ptr(); - else - return 0; } void WaveTrackCache::Free() diff --git a/src/WaveTrack.h b/src/WaveTrack.h index 33a2ccfe4..f9c4ea94e 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -592,10 +592,9 @@ private: std::unique_ptr mpWaveformSettings; }; -// This is meant to be a short-lived object, during whose lifetime, -// the contents of the WaveTrack are known not to change. It can replace -// repeated calls to WaveTrack::Get() (each of which opens and closes at least -// one block file). +//! A short-lived object, during whose lifetime, the contents of the WaveTrack are assumed not to change. +/*! It can replace repeated calls to WaveTrack::Get() (each of which opens and closes at least one block). + */ class AUDACITY_DLL_API WaveTrackCache { public: WaveTrackCache() @@ -617,12 +616,11 @@ public: const std::shared_ptr& GetTrack() const { return mPTrack; } void SetTrack(const std::shared_ptr &pTrack); - // Uses fillZero always - // Returns null on failure - // Returned pointer may be invalidated if Get is called again - // Do not DELETE[] the pointer - constSamplePtr Get( - sampleFormat format, sampleCount start, size_t len, bool mayThrow); + //! Retrieve samples as floats from the track or from the memory cache + /*! Uses fillZero always + @return null on failure; this object owns the memory; may be invalidated if GetFloats() is called again + */ + const float *GetFloats(sampleCount start, size_t len, bool mayThrow); private: void Free(); From f369b5109b33d2fffd9857dfbdbe97a975418c67 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 23 May 2021 17:43:38 -0400 Subject: [PATCH 8/9] Change almost all uses of WaveTrack::Get() to GetFloats() ... ... A call graph browser easily shows that the extra generality of fetching samples in some other format is only used in Benchmark -- where the format is always the same as what the track is constructed with. This makes re-verification of the claims in comments two commits ago easier. --- src/FreqWindow.cpp | 4 +-- src/MixerBoard.cpp | 8 +++--- src/VoiceKey.cpp | 14 +++++----- src/WaveTrack.cpp | 22 +++++++++------- src/WaveTrack.h | 26 +++++++++++++++++++ src/commands/CompareAudioCommand.cpp | 4 +-- src/effects/AutoDuck.cpp | 4 +-- src/effects/ChangePitch.cpp | 2 +- src/effects/ChangeSpeed.cpp | 2 +- src/effects/ClickRemoval.cpp | 2 +- src/effects/Effect.cpp | 4 +-- src/effects/Equalization.cpp | 2 +- src/effects/FindClipping.cpp | 2 +- src/effects/Loudness.cpp | 2 +- src/effects/NoiseReduction.cpp | 2 +- src/effects/Normalize.cpp | 4 +-- src/effects/Paulstretch.cpp | 6 ++--- src/effects/Repair.cpp | 2 +- src/effects/Reverse.cpp | 4 +-- src/effects/SBSMSEffect.cpp | 8 +++--- src/effects/SimpleMono.cpp | 2 +- src/effects/SoundTouchEffect.cpp | 6 ++--- src/effects/TruncSilence.cpp | 6 ++--- src/effects/TwoPassSimpleMono.cpp | 4 +-- src/effects/nyquist/Nyquist.cpp | 4 +-- src/effects/vamp/VampEffect.cpp | 4 +-- src/menus/SelectMenus.cpp | 2 +- src/menus/TransportMenus.cpp | 2 +- .../wavetrack/ui/SampleHandle.cpp | 4 +-- src/tracks/ui/SelectHandle.cpp | 6 ++--- 30 files changed, 97 insertions(+), 67 deletions(-) diff --git a/src/FreqWindow.cpp b/src/FreqWindow.cpp index 02a2ba7b1..3c81b1292 100644 --- a/src/FreqWindow.cpp +++ b/src/FreqWindow.cpp @@ -602,7 +602,7 @@ void FrequencyPlotDialog::GetAudio() mDataLen = dataLen.as_size_t(); mData = Floats{ mDataLen }; // Don't allow throw for bad reads - track->Get((samplePtr)mData.get(), floatSample, start, mDataLen, + track->GetFloats(mData.get(), start, mDataLen, fillZero, false); } else { @@ -617,7 +617,7 @@ void FrequencyPlotDialog::GetAudio() auto start = track->TimeToLongSamples(selectedRegion.t0()); Floats buffer2{ mDataLen }; // Again, stop exceptions - track->Get((samplePtr)buffer2.get(), floatSample, start, mDataLen, + track->GetFloats(buffer2.get(), start, mDataLen, fillZero, false); for (size_t i = 0; i < mDataLen; i++) mData[i] += buffer2[i]; diff --git a/src/MixerBoard.cpp b/src/MixerBoard.cpp index 4ea01f77f..a0ade17d0 100644 --- a/src/MixerBoard.cpp +++ b/src/MixerBoard.cpp @@ -610,8 +610,8 @@ void MixerTrackCluster::UpdateMeter(const double t0, const double t1) Floats tempFloatsArray{ nFrames }; decltype(tempFloatsArray) meterFloatsArray; // Don't throw on read error in this drawing update routine - bool bSuccess = pTrack->Get((samplePtr)tempFloatsArray.get(), - floatSample, startSample, nFrames, fillZero, false); + bool bSuccess = pTrack->GetFloats(tempFloatsArray.get(), + startSample, nFrames, fillZero, false); if (bSuccess) { // We always pass a stereo sample array to the meter, as it shows 2 channels. @@ -625,8 +625,8 @@ void MixerTrackCluster::UpdateMeter(const double t0, const double t1) if (GetRight()) // Again, don't throw - bSuccess = GetRight()->Get((samplePtr)tempFloatsArray.get(), - floatSample, startSample, nFrames, fillZero, false); + bSuccess = GetRight()->GetFloats(tempFloatsArray.get(), + startSample, nFrames, fillZero, false); if (bSuccess) // Interleave right channel, or duplicate same signal for "right" channel in mono case. diff --git a/src/VoiceKey.cpp b/src/VoiceKey.cpp index 17db53c69..f522e3da8 100644 --- a/src/VoiceKey.cpp +++ b/src/VoiceKey.cpp @@ -149,7 +149,7 @@ sampleCount VoiceKey::OnForward ( //To speed things up, create a local buffer to store things in, to avoid the costly t.Get(); //Only go through the first SignalWindowSizeInt samples, and choose the first that trips the key. Floats buffer{ remaining }; - t.Get((samplePtr)buffer.get(), floatSample, + t.GetFloats(buffer.get(), lastsubthresholdsample, remaining); @@ -299,7 +299,7 @@ sampleCount VoiceKey::OnBackward ( //To speed things up, create a local buffer to store things in, to avoid the costly t.Get(); //Only go through the first mSilentWindowSizeInt samples, and choose the first that trips the key. Floats buffer{ remaining }; - t.Get((samplePtr)buffer.get(), floatSample, + t.GetFloats(buffer.get(), lastsubthresholdsample - remaining, remaining); //Initialize these trend markers atrend and ztrend. They keep track of the @@ -442,7 +442,7 @@ sampleCount VoiceKey::OffForward ( //To speed things up, create a local buffer to store things in, to avoid the costly t.Get(); //Only go through the first SilentWindowSizeInt samples, and choose the first that trips the key. Floats buffer{ remaining }; - t.Get((samplePtr)buffer.get(), floatSample, + t.GetFloats(buffer.get(), lastsubthresholdsample, remaining); //Initialize these trend markers atrend and ztrend. They keep track of the @@ -579,7 +579,7 @@ sampleCount VoiceKey::OffBackward ( //To speed things up, create a local buffer to store things in, to avoid the costly t.Get(); //Only go through the first SilentWindowSizeInt samples, and choose the first that trips the key. Floats buffer{ remaining }; - t.Get((samplePtr)buffer.get(), floatSample, + t.GetFloats(buffer.get(), lastsubthresholdsample - remaining, remaining); //Initialize these trend markers atrend and ztrend. They keep track of the @@ -870,7 +870,7 @@ double VoiceKey::TestEnergy ( //Figure out how much to grab auto block = limitSampleBufferSize ( t.GetBestBlockSize(s), len ); - t.Get((samplePtr)buffer.get(), floatSample, s,block); //grab the block; + t.GetFloats(buffer.get(), s,block); //grab the block; //Now, go through the block and calculate energy for(decltype(block) i = 0; i< block; i++) @@ -913,7 +913,7 @@ double VoiceKey::TestSignChanges( //Figure out how much to grab auto block = limitSampleBufferSize ( t.GetBestBlockSize(s), len ); - t.Get((samplePtr)buffer.get(), floatSample, s, block); //grab the block; + t.GetFloats(buffer.get(), s, block); //grab the block; if (len == originalLen) { @@ -970,7 +970,7 @@ double VoiceKey::TestDirectionChanges( //Figure out how much to grab auto block = limitSampleBufferSize ( t.GetBestBlockSize(s), len ); - t.Get((samplePtr)buffer.get(), floatSample, s, block); //grab the block; + t.GetFloats(buffer.get(), s, block); //grab the block; if (len == originalLen) { //The first time through, set stuff up special. diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index dda66e2dc..7ee6f8a89 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -2594,8 +2594,8 @@ const float *WaveTrackCache::GetFloats( if (start0 >= 0) { const auto len0 = mPTrack->GetBestBlockSize(start0); wxASSERT(len0 <= mBufferSize); - if (!mPTrack->Get( - samplePtr(mBuffers[0].data.get()), floatSample, start0, len0, + if (!mPTrack->GetFloats( + mBuffers[0].data.get(), start0, len0, fillZero, mayThrow)) return nullptr; mBuffers[0].start = start0; @@ -2622,7 +2622,7 @@ const float *WaveTrackCache::GetFloats( if (start1 == end0) { const auto len1 = mPTrack->GetBestBlockSize(start1); wxASSERT(len1 <= mBufferSize); - if (!mPTrack->Get(samplePtr(mBuffers[1].data.get()), floatSample, start1, len1, fillZero, mayThrow)) + if (!mPTrack->GetFloats(mBuffers[1].data.get(), start1, len1, fillZero, mayThrow)) return nullptr; mBuffers[1].start = start1; mBuffers[1].len = len1; @@ -2632,7 +2632,7 @@ const float *WaveTrackCache::GetFloats( } wxASSERT(mNValidBuffers < 2 || mBuffers[0].end() == mBuffers[1].start); - samplePtr buffer = 0; + samplePtr buffer = nullptr; // will point into mOverlapBuffer auto remaining = len; // Possibly get an initial portion that is uncached @@ -2647,8 +2647,10 @@ const float *WaveTrackCache::GetFloats( mOverlapBuffer.Resize(len, format); // initLen is not more than len: auto sinitLen = initLen.as_size_t(); - if (!mPTrack->Get(mOverlapBuffer.ptr(), format, start, sinitLen, - fillZero, mayThrow)) + if (!mPTrack->GetFloats( + // See comment below about casting + reinterpret_cast(mOverlapBuffer.ptr()), + start, sinitLen, fillZero, mayThrow)) return nullptr; wxASSERT( sinitLen <= remaining ); remaining -= sinitLen; @@ -2675,7 +2677,7 @@ const float *WaveTrackCache::GetFloats( else if (leni > 0) { // leni is nonnegative, therefore start falls within mBuffers[ii] // But we can't satisfy all from one buffer, so copy - if (buffer == 0) { + if (!buffer) { mOverlapBuffer.Resize(len, format); buffer = mOverlapBuffer.ptr(); } @@ -2693,11 +2695,13 @@ const float *WaveTrackCache::GetFloats( if (remaining > 0) { // Very big request! // Fall back to direct fetch - if (buffer == 0) { + if (!buffer) { mOverlapBuffer.Resize(len, format); buffer = mOverlapBuffer.ptr(); } - if (!mPTrack->Get(buffer, format, start, remaining, fillZero, mayThrow)) + // See comment below about casting + if (!mPTrack->GetFloats( reinterpret_cast(buffer), + start, remaining, fillZero, mayThrow)) return 0; } diff --git a/src/WaveTrack.h b/src/WaveTrack.h index f9c4ea94e..89aad4dd3 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -246,6 +246,31 @@ private: /// same value for "start" in both calls to "Set" and "Get" it is /// guaranteed that the same samples are affected. /// + + //! Retrieve samples from a track in floating-point format, regardless of the storage format + /*! + @param buffer receives the samples + @param start starting sample, relative to absolute time zero (not to the track's offset value) + @param len how many samples to get. buffer is assumed sufficiently large + @param fill how to assign values for sample positions between clips + @param mayThrow if false, fill buffer with zeros when there is failure to retrieve samples; else throw + @param[out] pNumWithinClips Report how many samples were copied from within clips, rather + than filled according to fillFormat; but these were not necessarily one contiguous range. + */ + bool GetFloats(float *buffer, sampleCount start, size_t len, + fillFormat fill = fillZero, bool mayThrow = true, + sampleCount * pNumWithinClips = nullptr) const + { + //! Cast the pointer to pass it to Get() which handles multiple destination formats + return Get(reinterpret_cast(buffer), + floatSample, start, len, fill, mayThrow, pNumWithinClips); + } + + //! Retrieve samples from a track in a specified format + /*! + @copydetails WaveTrack::GetFloats() + @param format sample format of the destination buffer + */ bool Get(samplePtr buffer, sampleFormat format, sampleCount start, size_t len, fillFormat fill = fillZero, @@ -254,6 +279,7 @@ private: // filled according to fillFormat; but these were not necessarily one // contiguous range. sampleCount * pNumWithinClips = nullptr) const; + void Set(constSamplePtr buffer, sampleFormat format, sampleCount start, size_t len); diff --git a/src/commands/CompareAudioCommand.cpp b/src/commands/CompareAudioCommand.cpp index 1cd893975..b21c60ad8 100644 --- a/src/commands/CompareAudioCommand.cpp +++ b/src/commands/CompareAudioCommand.cpp @@ -138,8 +138,8 @@ bool CompareAudioCommand::Apply(const CommandContext & context) auto block = limitSampleBufferSize( mTrack0->GetBestBlockSize(position), s1 - position ); - mTrack0->Get((samplePtr)buff0.get(), floatSample, position, block); - mTrack1->Get((samplePtr)buff1.get(), floatSample, position, block); + mTrack0->GetFloats(buff0.get(), position, block); + mTrack1->GetFloats(buff1.get(), position, block); for (decltype(block) buffPos = 0; buffPos < block; ++buffPos) { diff --git a/src/effects/AutoDuck.cpp b/src/effects/AutoDuck.cpp index 572b3b230..104973a1f 100644 --- a/src/effects/AutoDuck.cpp +++ b/src/effects/AutoDuck.cpp @@ -321,7 +321,7 @@ bool EffectAutoDuck::Process() { const auto len = limitSampleBufferSize( kBufSize, end - pos ); - mControlTrack->Get((samplePtr)buf.get(), floatSample, pos, len); + mControlTrack->GetFloats(buf.get(), pos, len); for (auto i = pos; i < pos + len; i++) { @@ -559,7 +559,7 @@ bool EffectAutoDuck::ApplyDuckFade(int trackNum, WaveTrack* t, { const auto len = limitSampleBufferSize( kBufSize, end - pos ); - t->Get((samplePtr)buf.get(), floatSample, pos, len); + t->GetFloats(buf.get(), pos, len); for (auto i = pos; i < pos + len; i++) { diff --git a/src/effects/ChangePitch.cpp b/src/effects/ChangePitch.cpp index 802a94dd9..2eb022fb2 100644 --- a/src/effects/ChangePitch.cpp +++ b/src/effects/ChangePitch.cpp @@ -477,7 +477,7 @@ void EffectChangePitch::DeduceFrequencies() Floats freq{ windowSize / 2 }; Floats freqa{ windowSize / 2, true }; - track->Get((samplePtr) buffer.get(), floatSample, start, analyzeSize); + track->GetFloats(buffer.get(), start, analyzeSize); for(unsigned i = 0; i < numWindows; i++) { ComputeSpectrum(buffer.get() + i * windowSize, windowSize, windowSize, rate, freq.get(), true); diff --git a/src/effects/ChangeSpeed.cpp b/src/effects/ChangeSpeed.cpp index a7129f4c4..98252b17c 100644 --- a/src/effects/ChangeSpeed.cpp +++ b/src/effects/ChangeSpeed.cpp @@ -520,7 +520,7 @@ bool EffectChangeSpeed::ProcessOne(WaveTrack * track, ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr) inBuffer.get(), floatSample, samplePos, blockSize); + track->GetFloats(inBuffer.get(), samplePos, blockSize); const auto results = resample.Process(mFactor, inBuffer.get(), diff --git a/src/effects/ClickRemoval.cpp b/src/effects/ClickRemoval.cpp index 2f8daa13e..4c8b0a624 100644 --- a/src/effects/ClickRemoval.cpp +++ b/src/effects/ClickRemoval.cpp @@ -233,7 +233,7 @@ bool EffectClickRemoval::ProcessOne(int count, WaveTrack * track, sampleCount st { auto block = limitSampleBufferSize( idealBlockLen, len - s ); - track->Get((samplePtr) buffer.get(), floatSample, start + s, block); + track->GetFloats(buffer.get(), start + s, block); for (decltype(block) i = 0; i + windowSize / 2 < block; i += windowSize / 2) { diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index 4921ba5ae..1cfc2e802 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -1637,10 +1637,10 @@ bool Effect::ProcessTrack(int count, limitSampleBufferSize( mBufferSize, inputRemaining ); // Fill the input buffers - left->Get((samplePtr) inBuffer[0].get(), floatSample, inPos, inputBufferCnt); + left->GetFloats(inBuffer[0].get(), inPos, inputBufferCnt); if (right) { - right->Get((samplePtr) inBuffer[1].get(), floatSample, inPos, inputBufferCnt); + right->GetFloats(inBuffer[1].get(), inPos, inputBufferCnt); } // Reset the input buffer positions diff --git a/src/effects/Equalization.cpp b/src/effects/Equalization.cpp index 83082bc5b..ad1fbd0fa 100644 --- a/src/effects/Equalization.cpp +++ b/src/effects/Equalization.cpp @@ -1330,7 +1330,7 @@ bool EffectEqualization::ProcessOne(int count, WaveTrack * t, { auto block = limitSampleBufferSize( idealBlockLen, len ); - t->Get((samplePtr)buffer.get(), floatSample, s, block); + t->GetFloats(buffer.get(), s, block); for(size_t i = 0; i < block; i += L) //go through block in lumps of length L { diff --git a/src/effects/FindClipping.cpp b/src/effects/FindClipping.cpp index b3dab31fc..180bdc737 100644 --- a/src/effects/FindClipping.cpp +++ b/src/effects/FindClipping.cpp @@ -198,7 +198,7 @@ bool EffectFindClipping::ProcessOne(LabelTrack * lt, block = limitSampleBufferSize( blockSize, len - s ); - wt->Get((samplePtr)buffer.get(), floatSample, start + s, block); + wt->GetFloats(buffer.get(), start + s, block); ptr = buffer.get(); } diff --git a/src/effects/Loudness.cpp b/src/effects/Loudness.cpp index 02c947ecf..3c17d9c87 100644 --- a/src/effects/Loudness.cpp +++ b/src/effects/Loudness.cpp @@ -515,7 +515,7 @@ void EffectLoudness::LoadBufferBlock(TrackIterRange range, int idx = 0; for(auto channel : range) { - channel->Get((samplePtr) mTrackBuffer[idx].get(), floatSample, pos, len ); + channel->GetFloats(mTrackBuffer[idx].get(), pos, len ); ++idx; } mTrackBufferLen = len; diff --git a/src/effects/NoiseReduction.cpp b/src/effects/NoiseReduction.cpp index 64aa2ab7d..9215f9fc8 100644 --- a/src/effects/NoiseReduction.cpp +++ b/src/effects/NoiseReduction.cpp @@ -1330,7 +1330,7 @@ bool EffectNoiseReduction::Worker::ProcessOne ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr)&buffer[0], floatSample, samplePos, blockSize); + track->GetFloats(&buffer[0], samplePos, blockSize); samplePos += blockSize; mInSampleCount += blockSize; diff --git a/src/effects/Normalize.cpp b/src/effects/Normalize.cpp index 2bce11cae..30b50d0c2 100644 --- a/src/effects/Normalize.cpp +++ b/src/effects/Normalize.cpp @@ -435,7 +435,7 @@ bool EffectNormalize::AnalyseTrackData(const WaveTrack * track, const Translatab ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr) buffer.get(), floatSample, s, block, fillZero, true, &blockSamples); + track->GetFloats(buffer.get(), s, block, fillZero, true, &blockSamples); totalSamples += blockSamples; //Process the buffer. @@ -495,7 +495,7 @@ bool EffectNormalize::ProcessOne( ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr) buffer.get(), floatSample, s, block); + track->GetFloats(buffer.get(), s, block); //Process the buffer. ProcessData(buffer.get(), block, offset); diff --git a/src/effects/Paulstretch.cpp b/src/effects/Paulstretch.cpp index 28687fd78..6a504104e 100644 --- a/src/effects/Paulstretch.cpp +++ b/src/effects/Paulstretch.cpp @@ -359,7 +359,7 @@ bool EffectPaulstretch::ProcessOne(WaveTrack *track,double t0,double t1,int coun decltype(len) s=0; while (s < len) { - track->Get((samplePtr)bufferptr0, floatSample, start + s, nget); + track->GetFloats(bufferptr0, start + s, nget); stretch.process(buffer0.get(), nget); if (first_time) { @@ -369,7 +369,7 @@ bool EffectPaulstretch::ProcessOne(WaveTrack *track,double t0,double t1,int coun s += nget; if (first_time){//blend the start of the selection - track->Get((samplePtr)fade_track_smps.get(), floatSample, start, fade_len); + track->GetFloats(fade_track_smps.get(), start, fade_len); first_time = false; for (size_t i = 0; i < fade_len; i++){ float fi = (float)i / (float)fade_len; @@ -378,7 +378,7 @@ bool EffectPaulstretch::ProcessOne(WaveTrack *track,double t0,double t1,int coun } } if (s >= len){//blend the end of the selection - track->Get((samplePtr)fade_track_smps.get(), floatSample, end - fade_len, fade_len); + track->GetFloats(fade_track_smps.get(), end - fade_len, fade_len); for (size_t i = 0; i < fade_len; i++){ float fi = (float)i / (float)fade_len; auto i2 = bufsize / 2 - 1 - i; diff --git a/src/effects/Repair.cpp b/src/effects/Repair.cpp index e1c516fd6..57fff709f 100644 --- a/src/effects/Repair.cpp +++ b/src/effects/Repair.cpp @@ -146,7 +146,7 @@ bool EffectRepair::ProcessOne(int count, WaveTrack * track, size_t repairStart, size_t repairLen) { Floats buffer{ len }; - track->Get((samplePtr) buffer.get(), floatSample, start, len); + track->GetFloats(buffer.get(), start, len); InterpolateAudio(buffer.get(), len, repairStart, repairLen); track->Set((samplePtr)&buffer[repairStart], floatSample, start + repairStart, repairLen); diff --git a/src/effects/Reverse.cpp b/src/effects/Reverse.cpp index f97c5e998..1fd884d77 100644 --- a/src/effects/Reverse.cpp +++ b/src/effects/Reverse.cpp @@ -232,8 +232,8 @@ bool EffectReverse::ProcessOneClip(int count, WaveTrack *track, limitSampleBufferSize( track->GetBestBlockSize(first), len / 2 ); auto second = first + (len - block); - track->Get((samplePtr)buffer1.get(), floatSample, first, block); - track->Get((samplePtr)buffer2.get(), floatSample, second, block); + track->GetFloats(buffer1.get(), first, block); + track->GetFloats(buffer2.get(), second, block); for (decltype(block) i = 0; i < block; i++) { tmp = buffer1[i]; buffer1[i] = buffer2[block-i-1]; diff --git a/src/effects/SBSMSEffect.cpp b/src/effects/SBSMSEffect.cpp index 7cba9c5b2..6078c827f 100644 --- a/src/effects/SBSMSEffect.cpp +++ b/src/effects/SBSMSEffect.cpp @@ -99,10 +99,10 @@ long resampleCB(void *cb_data, SBSMSFrame *data) // does not seem to let us report error codes, so use this roundabout to // stop the effect early. try { - r->leftTrack->Get( - (samplePtr)(r->leftBuffer.get()), floatSample, r->offset, blockSize); - r->rightTrack->Get( - (samplePtr)(r->rightBuffer.get()), floatSample, r->offset, blockSize); + r->leftTrack->GetFloats( + (r->leftBuffer.get()), r->offset, blockSize); + r->rightTrack->GetFloats( + (r->rightBuffer.get()), r->offset, blockSize); } catch ( ... ) { // Save the exception object for re-throw when out of the library diff --git a/src/effects/SimpleMono.cpp b/src/effects/SimpleMono.cpp index 7faf3b721..dc95a1fff 100644 --- a/src/effects/SimpleMono.cpp +++ b/src/effects/SimpleMono.cpp @@ -96,7 +96,7 @@ bool EffectSimpleMono::ProcessOne(WaveTrack * track, limitSampleBufferSize( track->GetBestBlockSize(s), end - s ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr) buffer.get(), floatSample, s, block); + track->GetFloats(buffer.get(), s, block); //Process the buffer. If it fails, clean up and exit. if (!ProcessSimpleMono(buffer.get(), block)) diff --git a/src/effects/SoundTouchEffect.cpp b/src/effects/SoundTouchEffect.cpp index 7cc73a47a..96ffb869b 100644 --- a/src/effects/SoundTouchEffect.cpp +++ b/src/effects/SoundTouchEffect.cpp @@ -217,7 +217,7 @@ bool EffectSoundTouch::ProcessOne(WaveTrack *track, limitSampleBufferSize( track->GetBestBlockSize(s), end - s )); //Get the samples from the track and put them in the buffer - track->Get((samplePtr)buffer.get(), floatSample, s, block); + track->GetFloats(buffer.get(), s, block); //Add samples to SoundTouch mSoundTouch->putSamples(buffer.get(), block); @@ -298,8 +298,8 @@ bool EffectSoundTouch::ProcessStereo( ); // Get the samples from the tracks and put them in the buffers. - leftTrack->Get((samplePtr)(leftBuffer.get()), floatSample, sourceSampleCount, blockSize); - rightTrack->Get((samplePtr)(rightBuffer.get()), floatSample, sourceSampleCount, blockSize); + leftTrack->GetFloats((leftBuffer.get()), sourceSampleCount, blockSize); + rightTrack->GetFloats((rightBuffer.get()), sourceSampleCount, blockSize); // Interleave into soundTouchBuffer. for (decltype(blockSize) index = 0; index < blockSize; index++) { diff --git a/src/effects/TruncSilence.cpp b/src/effects/TruncSilence.cpp index b05a7ae97..d45742025 100644 --- a/src/effects/TruncSilence.cpp +++ b/src/effects/TruncSilence.cpp @@ -568,8 +568,8 @@ bool EffectTruncSilence::DoRemoval auto t1 = wt->TimeToLongSamples(cutStart) - blendFrames / 2; auto t2 = wt->TimeToLongSamples(cutEnd) - blendFrames / 2; - wt->Get((samplePtr)buf1.get(), floatSample, t1, blendFrames); - wt->Get((samplePtr)buf2.get(), floatSample, t2, blendFrames); + wt->GetFloats(buf1.get(), t1, blendFrames); + wt->GetFloats(buf2.get(), t2, blendFrames); for (decltype(blendFrames) i = 0; i < blendFrames; ++i) { @@ -689,7 +689,7 @@ bool EffectTruncSilence::Analyze(RegionList& silenceList, auto count = limitSampleBufferSize( blockLen, end - *index ); // Fill buffer - wt->Get((samplePtr)(buffer.get()), floatSample, *index, count); + wt->GetFloats((buffer.get()), *index, count); // Look for silenceList in current block for (decltype(count) i = 0; i < count; ++i) { diff --git a/src/effects/TwoPassSimpleMono.cpp b/src/effects/TwoPassSimpleMono.cpp index 97294850b..4f5ecce27 100644 --- a/src/effects/TwoPassSimpleMono.cpp +++ b/src/effects/TwoPassSimpleMono.cpp @@ -130,7 +130,7 @@ bool EffectTwoPassSimpleMono::ProcessOne(WaveTrack * track, WaveTrack * outTrack std::min( maxblock, track->GetBestBlockSize(start) ), end - start ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr) buffer1.get(), floatSample, start, samples1); + track->GetFloats(buffer1.get(), start, samples1); // Process the first buffer with a NULL previous buffer if (mPass == 0) @@ -152,7 +152,7 @@ bool EffectTwoPassSimpleMono::ProcessOne(WaveTrack * track, WaveTrack * outTrack ); //Get the samples from the track and put them in the buffer - track->Get((samplePtr)buffer2.get(), floatSample, s, samples2); + track->GetFloats(buffer2.get(), s, samples2); //Process the buffer. If it fails, clean up and exit. if (mPass == 0) diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index d011e3243..3780732f0 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -2449,8 +2449,8 @@ int NyquistEffect::GetCallback(float *buffer, int ch, mCurBuffer[ch].Allocate(mCurBufferLen[ch], floatSample); try { - mCurTrack[ch]->Get( - mCurBuffer[ch].ptr(), floatSample, + mCurTrack[ch]->GetFloats( + reinterpret_cast(mCurBuffer[ch].ptr()), mCurBufferStart[ch], mCurBufferLen[ch]); } catch ( ... ) { diff --git a/src/effects/vamp/VampEffect.cpp b/src/effects/vamp/VampEffect.cpp index 9125f5625..de7e298a2 100644 --- a/src/effects/vamp/VampEffect.cpp +++ b/src/effects/vamp/VampEffect.cpp @@ -453,12 +453,12 @@ bool VampEffect::Process() if (left) { - left->Get((samplePtr)data[0].get(), floatSample, pos, request); + left->GetFloats(data[0].get(), pos, request); } if (right) { - right->Get((samplePtr)data[1].get(), floatSample, pos, request); + right->GetFloats(data[1].get(), pos, request); } if (request < block) diff --git a/src/menus/SelectMenus.cpp b/src/menus/SelectMenus.cpp index 6e3d428cd..00c6ce68d 100644 --- a/src/menus/SelectMenus.cpp +++ b/src/menus/SelectMenus.cpp @@ -70,7 +70,7 @@ double NearestZeroCrossing auto s = one->TimeToLongSamples(t0); // fillTwo to ensure that missing values are treated as 2, and hence do // not get used as zero crossings. - one->Get((samplePtr)oneDist.get(), floatSample, + one->GetFloats(oneDist.get(), s - (int)oneWindowSize/2, oneWindowSize, fillTwo); diff --git a/src/menus/TransportMenus.cpp b/src/menus/TransportMenus.cpp index aa91ca852..cdabd31e8 100644 --- a/src/menus/TransportMenus.cpp +++ b/src/menus/TransportMenus.cpp @@ -593,7 +593,7 @@ void OnPunchAndRoll(const CommandContext &context) if (getLen > 0) { float *const samples = data.data(); const sampleCount pos = wt->TimeToLongSamples(t1); - wt->Get((samplePtr)samples, floatSample, pos, getLen); + wt->GetFloats(samples, pos, getLen); } crossfadeData.push_back(std::move(data)); } diff --git a/src/tracks/playabletrack/wavetrack/ui/SampleHandle.cpp b/src/tracks/playabletrack/wavetrack/ui/SampleHandle.cpp index 1c7e64dc8..8fb755735 100644 --- a/src/tracks/playabletrack/wavetrack/ui/SampleHandle.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/SampleHandle.cpp @@ -127,7 +127,7 @@ UIHandlePtr SampleHandle::HitTest float oneSample; const double rate = wavetrack->GetRate(); const auto s0 = (sampleCount)(tt * rate + 0.5); - if (! wavetrack->Get((samplePtr)&oneSample, floatSample, s0, 1, fillZero, + if (! wavetrack->GetFloats(&oneSample, s0, 1, fillZero, // Do not propagate exception but return a failure value false) ) return {}; @@ -238,7 +238,7 @@ UIHandle::Result SampleHandle::Click Floats newSampleRegion{ 1 + 2 * (size_t)SMOOTHING_BRUSH_RADIUS }; //Get a sample from the track to do some tricks on. - mClickedTrack->Get((samplePtr)sampleRegion.get(), floatSample, + mClickedTrack->GetFloats(sampleRegion.get(), mClickedStartSample - SMOOTHING_KERNEL_RADIUS - SMOOTHING_BRUSH_RADIUS, sampleRegionSize); diff --git a/src/tracks/ui/SelectHandle.cpp b/src/tracks/ui/SelectHandle.cpp index 705e0845e..9bce385cf 100644 --- a/src/tracks/ui/SelectHandle.cpp +++ b/src/tracks/ui/SelectHandle.cpp @@ -1364,9 +1364,9 @@ void SelectHandle::StartSnappingFreqSelection end - start)); const auto effectiveLength = std::max(minLength, length); frequencySnappingData.resize(effectiveLength, 0.0f); - pTrack->Get( - reinterpret_cast(&frequencySnappingData[0]), - floatSample, start, length, fillZero, + pTrack->GetFloats( + &frequencySnappingData[0], + start, length, fillZero, // Don't try to cope with exceptions, just read zeroes instead. false); From 9c70887c34a5d5c082bc7c072820daaa442df069 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 23 May 2021 17:54:49 -0400 Subject: [PATCH 9/9] A rewrite in Nyquist to make reinterpret_cast unnecessary --- src/effects/nyquist/Nyquist.cpp | 21 +++++++++++---------- src/effects/nyquist/Nyquist.h | 3 ++- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index 3780732f0..6e1ecf05b 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -1391,12 +1391,12 @@ bool NyquistEffect::ProcessOne() // Put the fetch buffers in a clean initial state for (size_t i = 0; i < mCurNumChannels; i++) - mCurBuffer[i].Free(); + mCurBuffer[i].reset(); // Guarantee release of memory when done auto cleanup = finally( [&] { for (size_t i = 0; i < mCurNumChannels; i++) - mCurBuffer[i].Free(); + mCurBuffer[i].reset(); } ); // Evaluate the expression, which may invoke the get callback, but often does @@ -1564,7 +1564,7 @@ bool NyquistEffect::ProcessOne() // Clean the initial buffer states again for the get callbacks // -- is this really needed? - mCurBuffer[i].Free(); + mCurBuffer[i].reset(); } // Now fully evaluate the sound @@ -2427,15 +2427,15 @@ int NyquistEffect::StaticGetCallback(float *buffer, int channel, int NyquistEffect::GetCallback(float *buffer, int ch, int64_t start, int64_t len, int64_t WXUNUSED(totlen)) { - if (mCurBuffer[ch].ptr()) { + if (mCurBuffer[ch]) { if ((mCurStart[ch] + start) < mCurBufferStart[ch] || (mCurStart[ch] + start)+len > mCurBufferStart[ch]+mCurBufferLen[ch]) { - mCurBuffer[ch].Free(); + mCurBuffer[ch].reset(); } } - if (!mCurBuffer[ch].ptr()) { + if (!mCurBuffer[ch]) { mCurBufferStart[ch] = (mCurStart[ch] + start); mCurBufferLen[ch] = mCurTrack[ch]->GetBestBlockSize(mCurBufferStart[ch]); @@ -2447,10 +2447,11 @@ int NyquistEffect::GetCallback(float *buffer, int ch, limitSampleBufferSize( mCurBufferLen[ch], mCurStart[ch] + mCurLen - mCurBufferStart[ch] ); - mCurBuffer[ch].Allocate(mCurBufferLen[ch], floatSample); + // C++20 + // mCurBuffer[ch] = std::make_unique_for_overwrite(mCurBufferLen[ch]); + mCurBuffer[ch] = Buffer{ safenew float[ mCurBufferLen[ch] ] }; try { - mCurTrack[ch]->GetFloats( - reinterpret_cast(mCurBuffer[ch].ptr()), + mCurTrack[ch]->GetFloats( mCurBuffer[ch].get(), mCurBufferStart[ch], mCurBufferLen[ch]); } catch ( ... ) { @@ -2463,7 +2464,7 @@ int NyquistEffect::GetCallback(float *buffer, int ch, // We have guaranteed above that this is nonnegative and bounded by // mCurBufferLen[ch]: auto offset = ( mCurStart[ch] + start - mCurBufferStart[ch] ).as_size_t(); - const void *src = mCurBuffer[ch].ptr() + offset*SAMPLE_SIZE(floatSample); + const void *src = &mCurBuffer[ch][offset]; std::memcpy(buffer, src, len * sizeof(float)); if (ch == 0) { diff --git a/src/effects/nyquist/Nyquist.h b/src/effects/nyquist/Nyquist.h index 15cc04a45..8acbd3174 100644 --- a/src/effects/nyquist/Nyquist.h +++ b/src/effects/nyquist/Nyquist.h @@ -267,7 +267,8 @@ private: double mProgressTot; double mScale; - SampleBuffer mCurBuffer[2]; + using Buffer = std::unique_ptr; + Buffer mCurBuffer[2]; sampleCount mCurBufferStart[2]; size_t mCurBufferLen[2];