From a5ddb48a572baf5fc353425fdcc24fc6ae951d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 25 Apr 2023 07:45:39 +0200 Subject: [PATCH 1/9] Assert that we have no integer overflow in the AudioSource. This allows 13h31:35.8 length @ 44100 Hz on 32 bit targets and 3186904 years on 64 bit --- src/sources/audiosource.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sources/audiosource.cpp b/src/sources/audiosource.cpp index 211d39f1c57..c955bb1ac32 100644 --- a/src/sources/audiosource.cpp +++ b/src/sources/audiosource.cpp @@ -27,6 +27,7 @@ AudioSource::AudioSource( m_signalInfo(signalInfo), m_bitrate(inner.m_bitrate), m_frameIndexRange(inner.m_frameIndexRange) { + DEBUG_ASSERT(m_frameIndexRange.length() >= 0); } AudioSource::OpenResult AudioSource::open( From ae94ca7d8a4fcfecfa2fb1f6fbaeb9f4f1fd1bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 25 Apr 2023 07:47:50 +0200 Subject: [PATCH 2/9] Analyzer: Work with SINT frameLength and make sure it is not multiplied by the number of channels in the using code. --- src/analyzer/analyzer.h | 13 +++++---- src/analyzer/analyzerbeats.cpp | 30 ++++++++++----------- src/analyzer/analyzerbeats.h | 11 ++++---- src/analyzer/analyzerebur128.cpp | 19 +++++++------ src/analyzer/analyzerebur128.h | 6 +++-- src/analyzer/analyzergain.cpp | 37 +++++++++++++++----------- src/analyzer/analyzergain.h | 8 +++--- src/analyzer/analyzerkey.cpp | 36 +++++++++++++------------ src/analyzer/analyzerkey.h | 14 +++++----- src/analyzer/analyzersilence.cpp | 44 ++++++++++++++++--------------- src/analyzer/analyzersilence.h | 12 +++++---- src/analyzer/analyzerthread.cpp | 8 +++--- src/analyzer/analyzerwaveform.cpp | 35 +++++++++++++----------- src/analyzer/analyzerwaveform.h | 6 +++-- src/test/analyserwaveformtest.cpp | 7 +++-- src/test/analyzersilence_test.cpp | 4 ++- src/track/keyfactory.cpp | 5 ++-- src/track/keyfactory.h | 10 ++++--- src/track/keyutils.cpp | 9 +++---- src/track/keyutils.h | 5 ++-- src/waveform/waveform.cpp | 22 +++++++++++----- src/waveform/waveform.h | 19 +++++++------ 22 files changed, 205 insertions(+), 155 deletions(-) diff --git a/src/analyzer/analyzer.h b/src/analyzer/analyzer.h index 2704717aa50..93906af5946 100644 --- a/src/analyzer/analyzer.h +++ b/src/analyzer/analyzer.h @@ -1,5 +1,6 @@ #pragma once +#include "audio/signalinfo.h" #include "util/assert.h" #include "util/types.h" @@ -21,7 +22,9 @@ class Analyzer { // 1. Check if the track needs to be analyzed, otherwise return false. // 2. Perform the initialization and return true on success. // 3. If the initialization failed log the internal error and return false. - virtual bool initialize(TrackPointer tio, int sampleRate, int totalSamples) = 0; + virtual bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) = 0; ///////////////////////////////////////////////////////////////////////// // All following methods will only be invoked after initialize() @@ -32,12 +35,12 @@ class Analyzer { // If processing fails the analysis can be aborted early by returning // false. After aborting the analysis only cleanup() will be invoked, // but not finalize()! - virtual bool processSamples(const CSAMPLE* pIn, const int iLen) = 0; + virtual bool processSamples(const CSAMPLE* pIn, SINT count) = 0; // Update the track object with the analysis results after // processing finished successfully, i.e. all available audio // samples have been processed. - virtual void storeResults(TrackPointer tio) = 0; + virtual void storeResults(TrackPointer pTrack) = 0; // Discard any temporary results or free allocated memory. // This function will be invoked after the results have been @@ -66,9 +69,9 @@ class AnalyzerWithState final { return m_active; } - bool initialize(TrackPointer tio, int sampleRate, int totalSamples) { + bool initialize(TrackPointer pTrack, mixxx::audio::SampleRate sampleRate, SINT frameLength) { DEBUG_ASSERT(!m_active); - return m_active = m_analyzer->initialize(tio, sampleRate, totalSamples); + return m_active = m_analyzer->initialize(pTrack, sampleRate, frameLength); } void processSamples(const CSAMPLE* pIn, const int iLen) { diff --git a/src/analyzer/analyzerbeats.cpp b/src/analyzer/analyzerbeats.cpp index a06bc9806d0..42d1198c9ae 100644 --- a/src/analyzer/analyzerbeats.cpp +++ b/src/analyzer/analyzerbeats.cpp @@ -37,13 +37,14 @@ AnalyzerBeats::AnalyzerBeats(UserSettingsPointer pConfig, bool enforceBpmDetecti m_bPreferencesReanalyzeImported(false), m_bPreferencesFixedTempo(true), m_bPreferencesFastAnalysis(false), - m_totalSamples(0), - m_iMaxSamplesToProcess(0), - m_iCurrentSample(0) { + m_maxFramesToProcess(0), + m_currentFrame(0) { } -bool AnalyzerBeats::initialize(TrackPointer pTrack, int sampleRate, int totalSamples) { - if (totalSamples == 0) { +bool AnalyzerBeats::initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { + if (frameLength <= 0) { return false; } @@ -85,16 +86,15 @@ bool AnalyzerBeats::initialize(TrackPointer pTrack, int sampleRate, int totalSam << "\nFast analysis:" << m_bPreferencesFastAnalysis; m_sampleRate = sampleRate; - m_totalSamples = totalSamples; // In fast analysis mode, skip processing after // kFastAnalysisSecondsToAnalyze seconds are analyzed. if (m_bPreferencesFastAnalysis) { - m_iMaxSamplesToProcess = - mixxx::kFastAnalysisSecondsToAnalyze * m_sampleRate * mixxx::kAnalysisChannels; + m_maxFramesToProcess = + mixxx::kFastAnalysisSecondsToAnalyze * m_sampleRate; } else { - m_iMaxSamplesToProcess = m_totalSamples; + m_maxFramesToProcess = frameLength; } - m_iCurrentSample = 0; + m_currentFrame = 0; // if we can load a stored track don't reanalyze it bool bShouldAnalyze = shouldAnalyze(pTrack); @@ -112,7 +112,7 @@ bool AnalyzerBeats::initialize(TrackPointer pTrack, int sampleRate, int totalSam } if (m_pPlugin) { - if (m_pPlugin->initialize(sampleRate)) { + if (m_pPlugin->initialize(m_sampleRate)) { qDebug() << "Beat calculation started with plugin" << m_pluginId; } else { qDebug() << "Beat calculation will not start."; @@ -191,17 +191,17 @@ bool AnalyzerBeats::shouldAnalyze(TrackPointer pTrack) const { return true; } -bool AnalyzerBeats::processSamples(const CSAMPLE *pIn, const int iLen) { +bool AnalyzerBeats::processSamples(const CSAMPLE* pIn, SINT count) { VERIFY_OR_DEBUG_ASSERT(m_pPlugin) { return false; } - m_iCurrentSample += iLen; - if (m_iCurrentSample > m_iMaxSamplesToProcess) { + m_currentFrame += count / mixxx::kAnalysisChannels; + if (m_currentFrame > m_maxFramesToProcess) { return true; // silently ignore all remaining samples } - return m_pPlugin->processSamples(pIn, iLen); + return m_pPlugin->processSamples(pIn, count); } void AnalyzerBeats::cleanup() { diff --git a/src/analyzer/analyzerbeats.h b/src/analyzer/analyzerbeats.h index 0e4e44f74b0..3fa57a41cbd 100644 --- a/src/analyzer/analyzerbeats.h +++ b/src/analyzer/analyzerbeats.h @@ -26,8 +26,10 @@ class AnalyzerBeats : public Analyzer { static QList availablePlugins(); static mixxx::AnalyzerPluginInfo defaultPlugin(); - bool initialize(TrackPointer pTrack, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE *pIn, const int iLen) override; + bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* pIn, SINT count) override; void storeResults(TrackPointer tio) override; void cleanup() override; @@ -46,7 +48,6 @@ class AnalyzerBeats : public Analyzer { bool m_bPreferencesFastAnalysis; mixxx::audio::SampleRate m_sampleRate; - SINT m_totalSamples; - int m_iMaxSamplesToProcess; - int m_iCurrentSample; + SINT m_maxFramesToProcess; + SINT m_currentFrame; }; diff --git a/src/analyzer/analyzerebur128.cpp b/src/analyzer/analyzerebur128.cpp index c8fee49b7b7..9c111e619af 100644 --- a/src/analyzer/analyzerebur128.cpp +++ b/src/analyzer/analyzerebur128.cpp @@ -2,6 +2,7 @@ #include +#include "analyzer/constants.h" #include "track/track.h" #include "util/math.h" #include "util/sample.h" @@ -20,16 +21,18 @@ AnalyzerEbur128::~AnalyzerEbur128() { cleanup(); // ...to prevent memory leaks } -bool AnalyzerEbur128::initialize(TrackPointer tio, - int sampleRate, - int totalSamples) { - if (m_rgSettings.isAnalyzerDisabled(2, tio) || totalSamples == 0) { +bool AnalyzerEbur128::initialize( + TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { + if (m_rgSettings.isAnalyzerDisabled(2, pTrack) || frameLength <= 0) { qDebug() << "Skipping AnalyzerEbur128"; return false; } DEBUG_ASSERT(m_pState == nullptr); - m_pState = ebur128_init(2u, - static_cast(sampleRate), + m_pState = ebur128_init( + mixxx::kAnalysisChannels, + sampleRate, EBUR128_MODE_I); return m_pState != nullptr; } @@ -42,12 +45,12 @@ void AnalyzerEbur128::cleanup() { } } -bool AnalyzerEbur128::processSamples(const CSAMPLE *pIn, const int iLen) { +bool AnalyzerEbur128::processSamples(const CSAMPLE* pIn, SINT count) { VERIFY_OR_DEBUG_ASSERT(m_pState) { return false; } ScopedTimer t("AnalyzerEbur128::processSamples()"); - size_t frames = iLen / 2; + size_t frames = count / mixxx::kAnalysisChannels; int e = ebur128_add_frames_float(m_pState, pIn, frames); VERIFY_OR_DEBUG_ASSERT(e == EBUR128_SUCCESS) { qWarning() << "AnalyzerEbur128::processSamples() failed with" << e; diff --git a/src/analyzer/analyzerebur128.h b/src/analyzer/analyzerebur128.h index 5cc794425d1..3a4b1bed068 100644 --- a/src/analyzer/analyzerebur128.h +++ b/src/analyzer/analyzerebur128.h @@ -14,8 +14,10 @@ class AnalyzerEbur128 : public Analyzer { return rgSettings.isAnalyzerEnabled(2); } - bool initialize(TrackPointer tio, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE* pIn, const int iLen) override; + bool initialize(TrackPointer tio, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* pIn, SINT count) override; void storeResults(TrackPointer tio) override; void cleanup() override; diff --git a/src/analyzer/analyzergain.cpp b/src/analyzer/analyzergain.cpp index 9802e4d6e1c..05177a1693d 100644 --- a/src/analyzer/analyzergain.cpp +++ b/src/analyzer/analyzergain.cpp @@ -1,7 +1,10 @@ +#include "analyzer/analyzergain.h" + #include + #include -#include "analyzer/analyzergain.h" +#include "analyzer/constants.h" #include "track/track.h" #include "util/math.h" #include "util/sample.h" @@ -11,7 +14,7 @@ AnalyzerGain::AnalyzerGain(UserSettingsPointer pConfig) : m_rgSettings(pConfig), m_pLeftTempBuffer(nullptr), m_pRightTempBuffer(nullptr), - m_iBufferSize(0) { + m_bufferSize(0) { m_pReplayGain = new ReplayGain(); } @@ -21,33 +24,37 @@ AnalyzerGain::~AnalyzerGain() { delete m_pReplayGain; } -bool AnalyzerGain::initialize(TrackPointer tio, int sampleRate, int totalSamples) { - if (m_rgSettings.isAnalyzerDisabled(1, tio) || totalSamples == 0) { +bool AnalyzerGain::initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { + if (m_rgSettings.isAnalyzerDisabled(1, pTrack) || frameLength <= 0) { qDebug() << "Skipping AnalyzerGain"; return false; } - return m_pReplayGain->initialise((long)sampleRate, 2); + return m_pReplayGain->initialise( + sampleRate, + mixxx::kAnalysisChannels); } void AnalyzerGain::cleanup() { } -bool AnalyzerGain::processSamples(const CSAMPLE *pIn, const int iLen) { +bool AnalyzerGain::processSamples(const CSAMPLE* pIn, SINT count) { ScopedTimer t("AnalyzerGain::process()"); - int halfLength = static_cast(iLen / 2); - if (halfLength > m_iBufferSize) { + SINT numFrames = count / mixxx::kAnalysisChannels; + if (numFrames > m_bufferSize) { delete[] m_pLeftTempBuffer; delete[] m_pRightTempBuffer; - m_pLeftTempBuffer = new CSAMPLE[halfLength]; - m_pRightTempBuffer = new CSAMPLE[halfLength]; - m_iBufferSize = halfLength; + m_pLeftTempBuffer = new CSAMPLE[numFrames]; + m_pRightTempBuffer = new CSAMPLE[numFrames]; + m_bufferSize = numFrames; } - SampleUtil::deinterleaveBuffer(m_pLeftTempBuffer, m_pRightTempBuffer, pIn, halfLength); - SampleUtil::applyGain(m_pLeftTempBuffer, 32767, halfLength); - SampleUtil::applyGain(m_pRightTempBuffer, 32767, halfLength); - return m_pReplayGain->process(m_pLeftTempBuffer, m_pRightTempBuffer, halfLength); + SampleUtil::deinterleaveBuffer(m_pLeftTempBuffer, m_pRightTempBuffer, pIn, numFrames); + SampleUtil::applyGain(m_pLeftTempBuffer, 32767, numFrames); + SampleUtil::applyGain(m_pRightTempBuffer, 32767, numFrames); + return m_pReplayGain->process(m_pLeftTempBuffer, m_pRightTempBuffer, numFrames); } void AnalyzerGain::storeResults(TrackPointer tio) { diff --git a/src/analyzer/analyzergain.h b/src/analyzer/analyzergain.h index ddc2d6df578..5f80967d5cf 100644 --- a/src/analyzer/analyzergain.h +++ b/src/analyzer/analyzergain.h @@ -21,8 +21,10 @@ class AnalyzerGain : public Analyzer { return rgSettings.isAnalyzerEnabled(1); } - bool initialize(TrackPointer tio, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE* pIn, const int iLen) override; + bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* pIn, SINT count) override; void storeResults(TrackPointer tio) override; void cleanup() override; @@ -31,5 +33,5 @@ class AnalyzerGain : public Analyzer { CSAMPLE* m_pLeftTempBuffer; CSAMPLE* m_pRightTempBuffer; ReplayGain* m_pReplayGain; - int m_iBufferSize; + SINT m_bufferSize; }; diff --git a/src/analyzer/analyzerkey.cpp b/src/analyzer/analyzerkey.cpp index a11f711525e..a36b7207ba9 100644 --- a/src/analyzer/analyzerkey.cpp +++ b/src/analyzer/analyzerkey.cpp @@ -32,17 +32,19 @@ mixxx::AnalyzerPluginInfo AnalyzerKey::defaultPlugin() { AnalyzerKey::AnalyzerKey(const KeyDetectionSettings& keySettings) : m_keySettings(keySettings), - m_iSampleRate(0), - m_iTotalSamples(0), - m_iMaxSamplesToProcess(0), - m_iCurrentSample(0), + m_sampleRate(0), + m_totalFrames(0), + m_maxFramesToProcess(0), + m_currentFrame(0), m_bPreferencesKeyDetectionEnabled(true), m_bPreferencesFastAnalysisEnabled(false), m_bPreferencesReanalyzeEnabled(false) { } -bool AnalyzerKey::initialize(TrackPointer tio, int sampleRate, int totalSamples) { - if (totalSamples == 0) { +bool AnalyzerKey::initialize(TrackPointer tio, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { + if (frameLength <= 0) { return false; } @@ -72,16 +74,16 @@ bool AnalyzerKey::initialize(TrackPointer tio, int sampleRate, int totalSamples) << "\nRe-analyze when settings change:" << m_bPreferencesReanalyzeEnabled << "\nFast analysis:" << m_bPreferencesFastAnalysisEnabled; - m_iSampleRate = sampleRate; - m_iTotalSamples = totalSamples; + m_sampleRate = sampleRate; + m_totalFrames = frameLength; // In fast analysis mode, skip processing after // kFastAnalysisSecondsToAnalyze seconds are analyzed. if (m_bPreferencesFastAnalysisEnabled) { - m_iMaxSamplesToProcess = mixxx::kFastAnalysisSecondsToAnalyze * m_iSampleRate * mixxx::kAnalysisChannels; + m_maxFramesToProcess = mixxx::kFastAnalysisSecondsToAnalyze * m_sampleRate; } else { - m_iMaxSamplesToProcess = m_iTotalSamples; + m_maxFramesToProcess = frameLength; } - m_iCurrentSample = 0; + m_currentFrame = 0; // if we can't load a stored track reanalyze it bool bShouldAnalyze = shouldAnalyze(tio); @@ -101,7 +103,7 @@ bool AnalyzerKey::initialize(TrackPointer tio, int sampleRate, int totalSamples) } if (m_pPlugin) { - if (m_pPlugin->initialize(sampleRate)) { + if (m_pPlugin->initialize(m_sampleRate)) { qDebug() << "Key calculation started with plugin" << m_pluginId; } else { qDebug() << "Key calculation will not start."; @@ -148,17 +150,17 @@ bool AnalyzerKey::shouldAnalyze(TrackPointer tio) const { return true; } -bool AnalyzerKey::processSamples(const CSAMPLE *pIn, const int iLen) { +bool AnalyzerKey::processSamples(const CSAMPLE* pIn, SINT count) { VERIFY_OR_DEBUG_ASSERT(m_pPlugin) { return false; } - m_iCurrentSample += iLen; - if (m_iCurrentSample > m_iMaxSamplesToProcess) { + m_currentFrame += count / mixxx::kAnalysisChannels; + if (m_currentFrame > m_maxFramesToProcess) { return true; // silently ignore remaining samples } - return m_pPlugin->processSamples(pIn, iLen); + return m_pPlugin->processSamples(pIn, count); } void AnalyzerKey::cleanup() { @@ -179,7 +181,7 @@ void AnalyzerKey::storeResults(TrackPointer tio) { QHash extraVersionInfo = getExtraVersionInfo( m_pluginId, m_bPreferencesFastAnalysisEnabled); Keys track_keys = KeyFactory::makePreferredKeys( - key_changes, extraVersionInfo, m_iSampleRate, m_iTotalSamples); + key_changes, extraVersionInfo, m_sampleRate, m_totalFrames); tio->setKeys(track_keys); } diff --git a/src/analyzer/analyzerkey.h b/src/analyzer/analyzerkey.h index 504df21705a..f4fce626950 100644 --- a/src/analyzer/analyzerkey.h +++ b/src/analyzer/analyzerkey.h @@ -19,8 +19,10 @@ class AnalyzerKey : public Analyzer { static QList availablePlugins(); static mixxx::AnalyzerPluginInfo defaultPlugin(); - bool initialize(TrackPointer tio, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE *pIn, const int iLen) override; + bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* pIn, SINT count) override; void storeResults(TrackPointer tio) override; void cleanup() override; @@ -33,10 +35,10 @@ class AnalyzerKey : public Analyzer { KeyDetectionSettings m_keySettings; std::unique_ptr m_pPlugin; QString m_pluginId; - int m_iSampleRate; - int m_iTotalSamples; - int m_iMaxSamplesToProcess; - int m_iCurrentSample; + int m_sampleRate; + SINT m_totalFrames; + SINT m_maxFramesToProcess; + SINT m_currentFrame; bool m_bPreferencesKeyDetectionEnabled; bool m_bPreferencesFastAnalysisEnabled; diff --git a/src/analyzer/analyzersilence.cpp b/src/analyzer/analyzersilence.cpp index c07cde2e53f..c54534383b8 100644 --- a/src/analyzer/analyzersilence.cpp +++ b/src/analyzer/analyzersilence.cpp @@ -26,30 +26,32 @@ bool shouldAnalyze(TrackPointer pTrack) { AnalyzerSilence::AnalyzerSilence(UserSettingsPointer pConfig) : m_pConfig(pConfig), m_fThreshold(kSilenceThreshold), - m_iFramesProcessed(0), + m_framesProcessed(0), m_bPrevSilence(true), - m_iSignalStart(-1), - m_iSignalEnd(-1) { + m_signalStart(-1), + m_signalEnd(-1) { } -bool AnalyzerSilence::initialize(TrackPointer pTrack, int sampleRate, int totalSamples) { +bool AnalyzerSilence::initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { Q_UNUSED(sampleRate); - Q_UNUSED(totalSamples); + Q_UNUSED(frameLength); if (!shouldAnalyze(pTrack)) { return false; } - m_iFramesProcessed = 0; + m_framesProcessed = 0; m_bPrevSilence = true; - m_iSignalStart = -1; - m_iSignalEnd = -1; + m_signalStart = -1; + m_signalEnd = -1; return true; } -bool AnalyzerSilence::processSamples(const CSAMPLE* pIn, const int iLen) { - for (int i = 0; i < iLen; i += mixxx::kAnalysisChannels) { +bool AnalyzerSilence::processSamples(const CSAMPLE* pIn, SINT count) { + for (int i = 0; i < count; i += mixxx::kAnalysisChannels) { // Compute max of channels in this sample frame CSAMPLE fMax = CSAMPLE_ZERO; for (SINT ch = 0; ch < mixxx::kAnalysisChannels; ++ch) { @@ -60,16 +62,16 @@ bool AnalyzerSilence::processSamples(const CSAMPLE* pIn, const int iLen) { bool bSilence = fMax < m_fThreshold; if (m_bPrevSilence && !bSilence) { - if (m_iSignalStart < 0) { - m_iSignalStart = m_iFramesProcessed + i / mixxx::kAnalysisChannels; + if (m_signalStart < 0) { + m_signalStart = m_framesProcessed + i / mixxx::kAnalysisChannels; } } else if (!m_bPrevSilence && bSilence) { - m_iSignalEnd = m_iFramesProcessed + i / mixxx::kAnalysisChannels; + m_signalEnd = m_framesProcessed + i / mixxx::kAnalysisChannels; } m_bPrevSilence = bSilence; } - m_iFramesProcessed += iLen / mixxx::kAnalysisChannels; + m_framesProcessed += count / mixxx::kAnalysisChannels; return true; } @@ -77,21 +79,21 @@ void AnalyzerSilence::cleanup() { } void AnalyzerSilence::storeResults(TrackPointer pTrack) { - if (m_iSignalStart < 0) { - m_iSignalStart = 0; + if (m_signalStart < 0) { + m_signalStart = 0; } - if (m_iSignalEnd < 0) { - m_iSignalEnd = m_iFramesProcessed; + if (m_signalEnd < 0) { + m_signalEnd = m_framesProcessed; } // If track didn't end with silence, place signal end marker // on the end of the track. if (!m_bPrevSilence) { - m_iSignalEnd = m_iFramesProcessed; + m_signalEnd = m_framesProcessed; } - double firstSound = mixxx::kAnalysisChannels * m_iSignalStart; - double lastSound = mixxx::kAnalysisChannels * m_iSignalEnd; + double firstSound = mixxx::kAnalysisChannels * m_signalStart; + double lastSound = mixxx::kAnalysisChannels * m_signalEnd; CuePointer pAudibleSound = pTrack->findCueByType(mixxx::CueType::AudibleSound); if (pAudibleSound == nullptr) { diff --git a/src/analyzer/analyzersilence.h b/src/analyzer/analyzersilence.h index e9950fa0e76..412cd2204c1 100644 --- a/src/analyzer/analyzersilence.h +++ b/src/analyzer/analyzersilence.h @@ -10,16 +10,18 @@ class AnalyzerSilence : public Analyzer { explicit AnalyzerSilence(UserSettingsPointer pConfig); ~AnalyzerSilence() override = default; - bool initialize(TrackPointer pTrack, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE* pIn, const int iLen) override; + bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* pIn, SINT count) override; void storeResults(TrackPointer pTrack) override; void cleanup() override; private: UserSettingsPointer m_pConfig; CSAMPLE m_fThreshold; - int m_iFramesProcessed; + SINT m_framesProcessed; bool m_bPrevSilence; - int m_iSignalStart; - int m_iSignalEnd; + SINT m_signalStart; + SINT m_signalEnd; }; diff --git a/src/analyzer/analyzerthread.cpp b/src/analyzer/analyzerthread.cpp index 19427ad9df2..b4766efa2ee 100644 --- a/src/analyzer/analyzerthread.cpp +++ b/src/analyzer/analyzerthread.cpp @@ -130,7 +130,7 @@ void AnalyzerThread::doRun() { kLogger.debug() << "Analyzing" << m_currentTrack->getLocation(); // Get the audio - const auto audioSource = + const mixxx::AudioSourcePointer audioSource = SoundSourceProxy(m_currentTrack).openAudioSource(openParams); if (!audioSource) { kLogger.warning() @@ -146,7 +146,7 @@ void AnalyzerThread::doRun() { if (analyzer.initialize( m_currentTrack, audioSource->getSignalInfo().getSampleRate(), - audioSource->frameLength() * mixxx::kAnalysisChannels)) { + audioSource->frameLength())) { processTrack = true; } } @@ -311,8 +311,8 @@ AnalyzerThread::AnalysisResult AnalyzerThread::analyzeAudioSource( // 3rd step: Update & emit progress if (audioSource->frameLength() > 0) { const double frameProgress = - double(audioSource->frameLength() - remainingFrameRange.length()) / - double(audioSource->frameLength()); + static_cast(audioSource->frameLength() - remainingFrameRange.length()) / + audioSource->frameLength(); // math_min is required to compensate rounding errors const AnalyzerProgress progress = math_min(kAnalyzerProgressFinalizing, diff --git a/src/analyzer/analyzerwaveform.cpp b/src/analyzer/analyzerwaveform.cpp index f3ab634f86f..c77c4cac084 100644 --- a/src/analyzer/analyzerwaveform.cpp +++ b/src/analyzer/analyzerwaveform.cpp @@ -1,5 +1,6 @@ #include "analyzer/analyzerwaveform.h" +#include "analyzer/constants.h" #include "engine/engineobject.h" #include "engine/filters/enginefilterbessel4.h" #include "engine/filters/enginefilterbutterworth8.h" @@ -34,14 +35,16 @@ AnalyzerWaveform::~AnalyzerWaveform() { destroyFilters(); } -bool AnalyzerWaveform::initialize(TrackPointer tio, int sampleRate, int totalSamples) { - if (totalSamples == 0) { +bool AnalyzerWaveform::initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) { + if (frameLength <= 0) { qWarning() << "AnalyzerWaveform::initialize - no waveform/waveform summary"; return false; } // If we don't need to calculate the waveform/wavesummary, skip. - if (!shouldAnalyze(tio)) { + if (!shouldAnalyze(pTrack)) { return false; } @@ -57,15 +60,15 @@ bool AnalyzerWaveform::initialize(TrackPointer tio, int sampleRate, int totalSam const int summaryWaveformSamples = 2 * 1920; m_waveform = WaveformPointer(new Waveform( - sampleRate, totalSamples, mainWaveformSampleRate, -1)); + sampleRate, frameLength, mainWaveformSampleRate, -1)); m_waveformSummary = WaveformPointer(new Waveform( - sampleRate, totalSamples, mainWaveformSampleRate, summaryWaveformSamples)); + sampleRate, frameLength, mainWaveformSampleRate, summaryWaveformSamples)); // Now, that the Waveform memory is initialized, we can set set them to // the TIO. Be aware that other threads of Mixxx can touch them from // now. - tio->setWaveform(m_waveform); - tio->setWaveformSummary(m_waveformSummary); + pTrack->setWaveform(m_waveform); + pTrack->setWaveformSummary(m_waveformSummary); m_waveformData = m_waveform->data(); m_waveformSummaryData = m_waveformSummary->data(); @@ -167,7 +170,7 @@ void AnalyzerWaveform::destroyFilters() { } } -bool AnalyzerWaveform::processSamples(const CSAMPLE* buffer, const int bufferLength) { +bool AnalyzerWaveform::processSamples(const CSAMPLE* buffer, SINT count) { VERIFY_OR_DEBUG_ASSERT(m_waveform) { return false; } @@ -176,20 +179,20 @@ bool AnalyzerWaveform::processSamples(const CSAMPLE* buffer, const int bufferLen } //this should only append once if bufferLength is constant - if (bufferLength > (int)m_buffers[0].size()) { - m_buffers[Low].resize(bufferLength); - m_buffers[Mid].resize(bufferLength); - m_buffers[High].resize(bufferLength); + if (count > static_cast(m_buffers[0].size())) { + m_buffers[Low].resize(count); + m_buffers[Mid].resize(count); + m_buffers[High].resize(count); } - m_filter[Low]->process(buffer, &m_buffers[Low][0], bufferLength); - m_filter[Mid]->process(buffer, &m_buffers[Mid][0], bufferLength); - m_filter[High]->process(buffer, &m_buffers[High][0], bufferLength); + m_filter[Low]->process(buffer, &m_buffers[Low][0], count); + m_filter[Mid]->process(buffer, &m_buffers[Mid][0], count); + m_filter[High]->process(buffer, &m_buffers[High][0], count); m_waveform->setSaveState(Waveform::SaveState::NotSaved); m_waveformSummary->setSaveState(Waveform::SaveState::NotSaved); - for (int i = 0; i < bufferLength; i += 2) { + for (int i = 0; i < count; i += 2) { // Take max value, not average of data CSAMPLE cover[2] = {fabs(buffer[i]), fabs(buffer[i + 1])}; CSAMPLE clow[2] = {fabs(m_buffers[Low][i]), fabs(m_buffers[Low][i + 1])}; diff --git a/src/analyzer/analyzerwaveform.h b/src/analyzer/analyzerwaveform.h index 85e4e05bb5d..6180a97b30a 100644 --- a/src/analyzer/analyzerwaveform.h +++ b/src/analyzer/analyzerwaveform.h @@ -140,8 +140,10 @@ class AnalyzerWaveform : public Analyzer { const QSqlDatabase& dbConnection); ~AnalyzerWaveform() override; - bool initialize(TrackPointer tio, int sampleRate, int totalSamples) override; - bool processSamples(const CSAMPLE* buffer, const int bufferLength) override; + bool initialize(TrackPointer pTrack, + mixxx::audio::SampleRate sampleRate, + SINT frameLength) override; + bool processSamples(const CSAMPLE* buffer, SINT count) override; void storeResults(TrackPointer tio) override; void cleanup() override; diff --git a/src/test/analyserwaveformtest.cpp b/src/test/analyserwaveformtest.cpp index 9845c7035a6..16c18a220ca 100644 --- a/src/test/analyserwaveformtest.cpp +++ b/src/test/analyserwaveformtest.cpp @@ -14,6 +14,7 @@ constexpr std::size_t kBigBufSize = 1024 * 1024; // Megabyte constexpr std::size_t kCanarySize = 1024 * 4; constexpr float kMagicFloat = 1234.567890f; constexpr float kCanaryFloat = 0.0f; +constexpr int kChannelCount = 2; class AnalyzerWaveformTest : public MixxxTest { protected: @@ -25,7 +26,7 @@ class AnalyzerWaveformTest : public MixxxTest { void SetUp() override { m_pTrack = Track::newTemporary(); m_pTrack->setAudioProperties( - mixxx::audio::ChannelCount(2), + mixxx::audio::ChannelCount(kChannelCount), mixxx::audio::SampleRate(44100), mixxx::audio::Bitrate(), mixxx::Duration::fromMillis(1000)); @@ -57,7 +58,9 @@ class AnalyzerWaveformTest : public MixxxTest { // Basic test to make sure we don't alter the input buffer and don't step out of bounds. TEST_F(AnalyzerWaveformTest, canary) { - m_aw.initialize(m_pTrack, m_pTrack->getSampleRate(), kBigBufSize); + m_aw.initialize(m_pTrack, + mixxx::audio::SampleRate(44100), + kBigBufSize / kChannelCount); m_aw.processSamples(&m_canaryBigBuf[kCanarySize], kBigBufSize); m_aw.storeResults(m_pTrack); m_aw.cleanup(); diff --git a/src/test/analyzersilence_test.cpp b/src/test/analyzersilence_test.cpp index 346e1af0dd4..28a4e983686 100644 --- a/src/test/analyzersilence_test.cpp +++ b/src/test/analyzersilence_test.cpp @@ -37,7 +37,9 @@ class AnalyzerSilenceTest : public MixxxTest { } void analyzeTrack() { - analyzerSilence.initialize(pTrack, pTrack->getSampleRate(), nTrackSampleDataLength); + analyzerSilence.initialize(pTrack, + mixxx::audio::SampleRate(44100), + kTrackLengthFrames); analyzerSilence.processSamples(pTrackSampleData, nTrackSampleDataLength); analyzerSilence.storeResults(pTrack); analyzerSilence.cleanup(); diff --git a/src/track/keyfactory.cpp b/src/track/keyfactory.cpp index b6c24b5e2ef..d98ca694da1 100644 --- a/src/track/keyfactory.cpp +++ b/src/track/keyfactory.cpp @@ -81,7 +81,8 @@ QString KeyFactory::getPreferredSubVersion( Keys KeyFactory::makePreferredKeys( const KeyChangeList& key_changes, const QHash& extraVersionInfo, - const int iSampleRate, const int iTotalSamples) { + const int iSampleRate, + SINT totalFrames) { Q_UNUSED(iSampleRate); const QString version = getPreferredVersion(); @@ -99,7 +100,7 @@ Keys KeyFactory::makePreferredKeys( pChange->set_key(it->first); pChange->set_frame_position(static_cast(frame)); } - key_map.set_global_key(KeyUtils::calculateGlobalKey(key_changes, iTotalSamples, iSampleRate)); + key_map.set_global_key(KeyUtils::calculateGlobalKey(key_changes, totalFrames, iSampleRate)); key_map.set_source(mixxx::track::io::key::ANALYZER); Keys keys(key_map); keys.setSubVersion(subVersion); diff --git a/src/track/keyfactory.h b/src/track/keyfactory.h index 72ce8c873b3..a58aac8b5ae 100644 --- a/src/track/keyfactory.h +++ b/src/track/keyfactory.h @@ -6,6 +6,7 @@ #include "proto/keys.pb.h" #include "track/keys.h" +#include "util/types.h" class KeyFactory { public: @@ -22,10 +23,11 @@ class KeyFactory { static QString getPreferredVersion(); static QString getPreferredSubVersion( - const QHash& extraVersionInfo); + const QHash& extraVersionInfo); static Keys makePreferredKeys( - const KeyChangeList& key_changes, - const QHash& extraVersionInfo, - const int iSampleRate, const int iTotalSamples); + const KeyChangeList& key_changes, + const QHash& extraVersionInfo, + int iSampleRate, + SINT totalFrames); }; diff --git a/src/track/keyutils.cpp b/src/track/keyutils.cpp index 47083e625fe..8256767b455 100644 --- a/src/track/keyutils.cpp +++ b/src/track/keyutils.cpp @@ -418,20 +418,19 @@ ChromaticKey KeyUtils::scaleKeySteps(ChromaticKey key, int key_changes) { // static mixxx::track::io::key::ChromaticKey KeyUtils::calculateGlobalKey( - const KeyChangeList& key_changes, const int iTotalSamples, int iSampleRate) { + const KeyChangeList& key_changes, SINT totalFrames, int iSampleRate) { if (key_changes.size() == 1) { qDebug() << keyDebugName(key_changes[0].first); return key_changes[0].first; } - - const int iTotalFrames = iTotalSamples / 2; QMap key_histogram; for (int i = 0; i < key_changes.size(); ++i) { mixxx::track::io::key::ChromaticKey key = key_changes[i].first; const double start_frame = key_changes[i].second; - const double next_frame = (i == key_changes.size() - 1) ? - iTotalFrames : key_changes[i+1].second; + const double next_frame = (i == key_changes.size() - 1) + ? totalFrames + : key_changes[i + 1].second; key_histogram[key] += (next_frame - start_frame); } diff --git a/src/track/keyutils.h b/src/track/keyutils.h index 5ac8d19dc0a..4395ac289cf 100644 --- a/src/track/keyutils.h +++ b/src/track/keyutils.h @@ -1,13 +1,14 @@ #pragma once +#include #include #include -#include #include "control/controlproxy.h" #include "proto/keys.pb.h" #include "track/keys.h" #include "util/math.h" +#include "util/types.h" class KeyUtils { public: @@ -79,7 +80,7 @@ class KeyUtils { static mixxx::track::io::key::ChromaticKey guessKeyFromText(const QString& text); static mixxx::track::io::key::ChromaticKey calculateGlobalKey( - const KeyChangeList& key_changes, int iTotalSamples, int iSampleRate); + const KeyChangeList& key_changes, SINT totalFrames, int iSampleRate); static void setNotation( const QMap& notation); diff --git a/src/waveform/waveform.cpp b/src/waveform/waveform.cpp index f9731e9c377..57b80e34337 100644 --- a/src/waveform/waveform.cpp +++ b/src/waveform/waveform.cpp @@ -1,6 +1,8 @@ +#include "waveform/waveform.h" + #include -#include "waveform/waveform.h" +#include "analyzer/constants.h" #include "proto/waveform.pb.h" using namespace mixxx::track; @@ -28,8 +30,11 @@ Waveform::Waveform(const QByteArray& data) readByteArray(data); } -Waveform::Waveform(int audioSampleRate, int audioSamples, - int desiredVisualSampleRate, int maxVisualSamples) +Waveform::Waveform( + int audioSampleRate, + SINT frameLength, + int desiredVisualSampleRate, + int maxVisualSamples) : m_id(-1), m_saveState(SaveState::NotSaved), m_dataSize(0), @@ -49,15 +54,18 @@ Waveform::Waveform(int audioSampleRate, int audioSamples, } } else { // Waveform Summary (Overview) - if (audioSamples > maxVisualSamples) { - m_visualSampleRate = (double)maxVisualSamples * - (double)audioSampleRate / (double)audioSamples; + if (frameLength > maxVisualSamples / mixxx::kAnalysisChannels) { + m_visualSampleRate = static_cast(audioSampleRate) * + maxVisualSamples / mixxx::kAnalysisChannels / frameLength; } else { m_visualSampleRate = audioSampleRate; } } m_audioVisualRatio = (double)audioSampleRate / (double)m_visualSampleRate; - numberOfVisualSamples = static_cast(audioSamples / m_audioVisualRatio) + 1; + numberOfVisualSamples = + static_cast(frameLength / m_audioVisualRatio * + mixxx::kAnalysisChannels) + + 1; numberOfVisualSamples += numberOfVisualSamples%2; } assign(numberOfVisualSamples, 0); diff --git a/src/waveform/waveform.h b/src/waveform/waveform.h index eaa587b6558..5458938275c 100644 --- a/src/waveform/waveform.h +++ b/src/waveform/waveform.h @@ -1,14 +1,14 @@ #pragma once -#include - -#include -#include -#include #include -#include +#include +#include #include +#include +#include +#include +#include "audio/signalinfo.h" #include "util/class.h" #include "util/compatibility.h" @@ -37,8 +37,11 @@ class Waveform { }; explicit Waveform(const QByteArray& pData = QByteArray()); - Waveform(int audioSampleRate, int audioSamples, - int desiredVisualSampleRate, int maxVisualSamples); + Waveform( + int audioSampleRate, + SINT frameLength, + int desiredVisualSampleRate, + int maxVisualSamples); virtual ~Waveform(); From 749053526a8fbb0f9dd74f4e12ba3ae312a72a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 25 Apr 2023 21:00:27 +0200 Subject: [PATCH 3/9] Pass number of track samples as doubel via trackLoaded to work around an integer overflow. --- src/engine/cachingreader/cachingreader.h | 2 +- src/engine/cachingreader/cachingreaderchunk.h | 9 ++++++--- src/engine/cachingreader/cachingreaderworker.cpp | 4 ++-- src/engine/cachingreader/cachingreaderworker.h | 2 +- src/engine/enginebuffer.cpp | 13 +++++++------ src/engine/enginebuffer.h | 6 ++++-- 6 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/engine/cachingreader/cachingreader.h b/src/engine/cachingreader/cachingreader.h index c5f5a6ee32c..d5bc22af253 100644 --- a/src/engine/cachingreader/cachingreader.h +++ b/src/engine/cachingreader/cachingreader.h @@ -111,7 +111,7 @@ class CachingReader : public QObject { signals: // Emitted once a new track is loaded and ready to be read from. void trackLoading(); - void trackLoaded(TrackPointer pTrack, int iSampleRate, int iNumSamples); + void trackLoaded(TrackPointer pTrack, int trackSampleRate, double trackNumSamples); void trackLoadFailed(TrackPointer pTrack, const QString& reason); private: diff --git a/src/engine/cachingreader/cachingreaderchunk.h b/src/engine/cachingreader/cachingreaderchunk.h index b968a61dd5b..ccc75a23bed 100644 --- a/src/engine/cachingreader/cachingreaderchunk.h +++ b/src/engine/cachingreader/cachingreaderchunk.h @@ -20,17 +20,20 @@ class CachingReaderChunk { static const SINT kSamples; // Converts frames to samples - inline static SINT frames2samples(SINT frames) { + static SINT frames2samples(SINT frames) { return frames * kChannels; } + static double dFrames2samples(SINT frames) { + return static_cast(frames) * kChannels; + } // Converts samples to frames - inline static SINT samples2frames(SINT samples) { + static SINT samples2frames(SINT samples) { DEBUG_ASSERT(0 == (samples % kChannels)); return samples / kChannels; } // Returns the corresponding chunk index for a frame index - inline static SINT indexForFrame( + static SINT indexForFrame( /*const mixxx::AudioSourcePointer& pAudioSource,*/ SINT frameIndex) { //DEBUG_ASSERT(pAudioSource->frameIndexRange().contains(frameIndex)); diff --git a/src/engine/cachingreader/cachingreaderworker.cpp b/src/engine/cachingreader/cachingreaderworker.cpp index 0cec4cc93fa..0affb1d8ab8 100644 --- a/src/engine/cachingreader/cachingreaderworker.cpp +++ b/src/engine/cachingreader/cachingreaderworker.cpp @@ -215,8 +215,8 @@ void CachingReaderWorker::loadTrack(const TrackPointer& pTrack) { m_pReaderStatusFIFO->writeBlocking(&update, 1); // Emit that the track is loaded. - const SINT sampleCount = - CachingReaderChunk::frames2samples( + const double sampleCount = + CachingReaderChunk::dFrames2samples( m_pAudioSource->frameLength()); // The engine must not request any chunks before receiving the diff --git a/src/engine/cachingreader/cachingreaderworker.h b/src/engine/cachingreader/cachingreaderworker.h index 1eb4126c1f1..d6e8f8e888a 100644 --- a/src/engine/cachingreader/cachingreaderworker.h +++ b/src/engine/cachingreader/cachingreaderworker.h @@ -113,7 +113,7 @@ class CachingReaderWorker : public EngineWorker { signals: // Emitted once a new track is loaded and ready to be read from. void trackLoading(); - void trackLoaded(TrackPointer pTrack, int iSampleRate, int iNumSamples); + void trackLoaded(TrackPointer pTrack, int sampleRate, double numSamples); void trackLoadFailed(TrackPointer pTrack, const QString& reason); private: diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index b70e4e9bed7..494ed970e18 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -498,14 +498,15 @@ void EngineBuffer::loadFakeTrack(TrackPointer pTrack, bool bPlay) { if (bPlay) { m_playButton->set((double)bPlay); } - slotTrackLoaded(pTrack, pTrack->getSampleRate(), - pTrack->getSampleRate() * pTrack->getDurationInt()); + slotTrackLoaded(pTrack, + pTrack->getSampleRate(), + pTrack->getSampleRate() * pTrack->getDuration()); } // WARNING: Always called from the EngineWorker thread pool void EngineBuffer::slotTrackLoaded(TrackPointer pTrack, - int iTrackSampleRate, - int iTrackNumSamples) { + int trackSampleRate, + double trackNumSamples) { if (kLogger.traceEnabled()) { kLogger.trace() << getGroup() << "EngineBuffer::slotTrackLoaded"; } @@ -515,8 +516,8 @@ void EngineBuffer::slotTrackLoaded(TrackPointer pTrack, m_visualPlayPos->setInvalid(); m_filepos_play = kInitialSamplePosition; // for execute seeks to 0.0 m_pCurrentTrack = pTrack; - m_pTrackSamples->set(iTrackNumSamples); - m_pTrackSampleRate->set(iTrackSampleRate); + m_pTrackSamples->set(trackNumSamples); + m_pTrackSampleRate->set(trackSampleRate); // Reset slip mode m_pSlipButton->set(0); m_bSlipEnabledProcessing = false; diff --git a/src/engine/enginebuffer.h b/src/engine/enginebuffer.h index cfe65ffc093..32d5808c16b 100644 --- a/src/engine/enginebuffer.h +++ b/src/engine/enginebuffer.h @@ -182,8 +182,10 @@ class EngineBuffer : public EngineObject { private slots: void slotTrackLoading(); - void slotTrackLoaded(TrackPointer pTrack, - int iSampleRate, int iNumSamples); + void slotTrackLoaded( + TrackPointer pTrack, + int trackSampleRate, + double trackNumSamples); void slotTrackLoadFailed(TrackPointer pTrack, const QString& reason); // Fired when passthrough mode is enabled or disabled. From 504f5baeaa98a211064caf02fe182041e4ecf149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 25 Apr 2023 21:04:30 +0200 Subject: [PATCH 4/9] Fix int overflow in LoopingControl --- src/engine/controls/loopingcontrol.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/controls/loopingcontrol.cpp b/src/engine/controls/loopingcontrol.cpp index e055c0257c1..65af1fa5e77 100644 --- a/src/engine/controls/loopingcontrol.cpp +++ b/src/engine/controls/loopingcontrol.cpp @@ -247,7 +247,7 @@ void LoopingControl::slotLoopScale(double scaleFactor) { return; } const double loopLength = (loopSamples.end - loopSamples.start) * scaleFactor; - const int trackSamples = static_cast(m_pTrackSamples->get()); + const double trackSamples = m_pTrackSamples->get(); // Abandon loops that are too short of extend beyond the end of the file. if (loopLength < MINIMUM_AUDIBLE_LOOP_SIZE || @@ -1027,7 +1027,7 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable beats = minBeatSize; } - int samples = static_cast(m_pTrackSamples->get()); + double samples = m_pTrackSamples->get(); const mixxx::BeatsPointer pBeats = m_pBeats; if (samples == 0 || !pBeats) { clearActiveBeatLoop(); From c1a2f61541f94ea5ba14d39b934115dbf6428a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 25 Apr 2023 21:22:28 +0200 Subject: [PATCH 5/9] Fix integer overflow in waveform classes --- src/waveform/renderers/glslwaveformrenderersignal.cpp | 2 +- src/waveform/renderers/glvsynctestrenderer.cpp | 2 +- src/waveform/renderers/glwaveformrendererfilteredsignal.cpp | 2 +- src/waveform/renderers/glwaveformrendererrgb.cpp | 2 +- src/waveform/renderers/glwaveformrenderersimplesignal.cpp | 2 +- src/waveform/renderers/qtwaveformrendererfilteredsignal.cpp | 2 +- src/waveform/renderers/qtwaveformrenderersimplesignal.cpp | 2 +- src/waveform/renderers/waveformrenderbeat.cpp | 2 +- src/waveform/renderers/waveformrendererfilteredsignal.cpp | 2 +- src/waveform/renderers/waveformrendererhsv.cpp | 2 +- src/waveform/renderers/waveformrendererrgb.cpp | 2 +- src/waveform/renderers/waveformwidgetrenderer.cpp | 4 ++-- src/waveform/renderers/waveformwidgetrenderer.h | 4 ++-- 13 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/waveform/renderers/glslwaveformrenderersignal.cpp b/src/waveform/renderers/glslwaveformrenderersignal.cpp index 18561a8115a..2d9d9ac5a7b 100644 --- a/src/waveform/renderers/glslwaveformrenderersignal.cpp +++ b/src/waveform/renderers/glslwaveformrenderersignal.cpp @@ -272,7 +272,7 @@ void GLSLWaveformRendererSignal::draw(QPainter* painter, QPaintEvent* /*event*/) return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/glvsynctestrenderer.cpp b/src/waveform/renderers/glvsynctestrenderer.cpp index d39e36dbb28..e64e25dfa30 100644 --- a/src/waveform/renderers/glvsynctestrenderer.cpp +++ b/src/waveform/renderers/glvsynctestrenderer.cpp @@ -54,7 +54,7 @@ void GLVSyncTestRenderer::draw(QPainter* painter, QPaintEvent* /*event*/) { return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/glwaveformrendererfilteredsignal.cpp b/src/waveform/renderers/glwaveformrendererfilteredsignal.cpp index 75b3f39cf5f..dd6a7b353ee 100644 --- a/src/waveform/renderers/glwaveformrendererfilteredsignal.cpp +++ b/src/waveform/renderers/glwaveformrendererfilteredsignal.cpp @@ -45,7 +45,7 @@ void GLWaveformRendererFilteredSignal::draw(QPainter* painter, QPaintEvent* /*ev return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/glwaveformrendererrgb.cpp b/src/waveform/renderers/glwaveformrendererrgb.cpp index e6323a19c91..53995d14868 100644 --- a/src/waveform/renderers/glwaveformrendererrgb.cpp +++ b/src/waveform/renderers/glwaveformrendererrgb.cpp @@ -47,7 +47,7 @@ void GLWaveformRendererRGB::draw(QPainter* painter, QPaintEvent* /*event*/) { return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/glwaveformrenderersimplesignal.cpp b/src/waveform/renderers/glwaveformrenderersimplesignal.cpp index 24ed5dcaf23..b2a95698506 100644 --- a/src/waveform/renderers/glwaveformrenderersimplesignal.cpp +++ b/src/waveform/renderers/glwaveformrenderersimplesignal.cpp @@ -47,7 +47,7 @@ void GLWaveformRendererSimpleSignal::draw(QPainter* painter, QPaintEvent* /*even return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/qtwaveformrendererfilteredsignal.cpp b/src/waveform/renderers/qtwaveformrendererfilteredsignal.cpp index ee627b65c2c..f4f0cb4d3e6 100644 --- a/src/waveform/renderers/qtwaveformrendererfilteredsignal.cpp +++ b/src/waveform/renderers/qtwaveformrendererfilteredsignal.cpp @@ -117,7 +117,7 @@ int QtWaveformRendererFilteredSignal::buildPolygon() { return 0; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return 0; } diff --git a/src/waveform/renderers/qtwaveformrenderersimplesignal.cpp b/src/waveform/renderers/qtwaveformrenderersimplesignal.cpp index 5cdce479b3b..f8732b51284 100644 --- a/src/waveform/renderers/qtwaveformrenderersimplesignal.cpp +++ b/src/waveform/renderers/qtwaveformrenderersimplesignal.cpp @@ -58,7 +58,7 @@ void QtWaveformRendererSimpleSignal::draw(QPainter* painter, QPaintEvent* /*even return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/waveformrenderbeat.cpp b/src/waveform/renderers/waveformrenderbeat.cpp index 52b97de2864..408dbf500ef 100644 --- a/src/waveform/renderers/waveformrenderbeat.cpp +++ b/src/waveform/renderers/waveformrenderbeat.cpp @@ -42,7 +42,7 @@ void WaveformRenderBeat::draw(QPainter* painter, QPaintEvent* /*event*/) { } m_beatColor.setAlphaF(alpha/100.0); - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/waveformrendererfilteredsignal.cpp b/src/waveform/renderers/waveformrendererfilteredsignal.cpp index c0c4db9ccd2..b18c122575d 100644 --- a/src/waveform/renderers/waveformrendererfilteredsignal.cpp +++ b/src/waveform/renderers/waveformrendererfilteredsignal.cpp @@ -50,7 +50,7 @@ void WaveformRendererFilteredSignal::draw(QPainter* painter, return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/waveformrendererhsv.cpp b/src/waveform/renderers/waveformrendererhsv.cpp index f468b03e998..982c3bb63e8 100644 --- a/src/waveform/renderers/waveformrendererhsv.cpp +++ b/src/waveform/renderers/waveformrendererhsv.cpp @@ -44,7 +44,7 @@ void WaveformRendererHSV::draw( return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/waveformrendererrgb.cpp b/src/waveform/renderers/waveformrendererrgb.cpp index 3fe23ffc39a..08d4bb8fd75 100644 --- a/src/waveform/renderers/waveformrendererrgb.cpp +++ b/src/waveform/renderers/waveformrendererrgb.cpp @@ -43,7 +43,7 @@ void WaveformRendererRGB::draw( return; } - const int trackSamples = m_waveformRenderer->getTrackSamples(); + const double trackSamples = m_waveformRenderer->getTrackSamples(); if (trackSamples <= 0) { return; } diff --git a/src/waveform/renderers/waveformwidgetrenderer.cpp b/src/waveform/renderers/waveformwidgetrenderer.cpp index 95e6d394385..d8788bb1795 100644 --- a/src/waveform/renderers/waveformwidgetrenderer.cpp +++ b/src/waveform/renderers/waveformwidgetrenderer.cpp @@ -103,7 +103,7 @@ bool WaveformWidgetRenderer::init() { void WaveformWidgetRenderer::onPreRender(VSyncThread* vsyncThread) { // For a valid track to render we need - m_trackSamples = static_cast(m_pTrackSamplesControlObject->get()); + m_trackSamples = m_pTrackSamplesControlObject->get(); if (m_trackSamples <= 0) { return; } @@ -135,7 +135,7 @@ void WaveformWidgetRenderer::onPreRender(VSyncThread* vsyncThread) { if (m_audioSamplePerPixel > 0 && truePlayPos != -1) { // Track length in pixels. - m_trackPixelCount = static_cast(m_trackSamples) / 2.0 / m_audioSamplePerPixel; + m_trackPixelCount = m_trackSamples / 2.0 / m_audioSamplePerPixel; // Avoid pixel jitter in play position by rounding to the nearest track // pixel. diff --git a/src/waveform/renderers/waveformwidgetrenderer.h b/src/waveform/renderers/waveformwidgetrenderer.h index 23cc2ca98fb..3617f9c9592 100644 --- a/src/waveform/renderers/waveformwidgetrenderer.h +++ b/src/waveform/renderers/waveformwidgetrenderer.h @@ -100,7 +100,7 @@ class WaveformWidgetRenderer { double getGain() const { return m_gain; } - int getTrackSamples() const { + double getTrackSamples() const { return m_trackSamples; } @@ -187,7 +187,7 @@ class WaveformWidgetRenderer { ControlProxy* m_pGainControlObject; double m_gain; ControlProxy* m_pTrackSamplesControlObject; - int m_trackSamples; + double m_trackSamples; double m_scaleFactor; double m_playMarkerPosition; // 0.0 - left, 0.5 - center, 1.0 - right From 4d227d0a6479964949f8de050efd5bf56e3985f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 9 Aug 2023 23:17:01 +0200 Subject: [PATCH 6/9] Fix assertion against backward ranges --- src/sources/audiosource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sources/audiosource.cpp b/src/sources/audiosource.cpp index c955bb1ac32..0813bd79487 100644 --- a/src/sources/audiosource.cpp +++ b/src/sources/audiosource.cpp @@ -27,7 +27,7 @@ AudioSource::AudioSource( m_signalInfo(signalInfo), m_bitrate(inner.m_bitrate), m_frameIndexRange(inner.m_frameIndexRange) { - DEBUG_ASSERT(m_frameIndexRange.length() >= 0); + DEBUG_ASSERT(m_frameIndexRange.orientation() != IndexRange::Orientation::Backward); } AudioSource::OpenResult AudioSource::open( From 7cc5df078f63e7d729e0e9cfeae9b9eadf4e1f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 10 Aug 2023 13:49:46 +0200 Subject: [PATCH 7/9] Introduce kCanarySampleRate --- src/test/analyserwaveformtest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/analyserwaveformtest.cpp b/src/test/analyserwaveformtest.cpp index 8d14194673c..10ee20c2d17 100644 --- a/src/test/analyserwaveformtest.cpp +++ b/src/test/analyserwaveformtest.cpp @@ -12,6 +12,7 @@ namespace { constexpr std::size_t kBigBufSize = 2 * 1920; // Matches the WaveformSummary constexpr std::size_t kCanarySize = 1024 * 4; +constexpr auto kCanarySampleRate = mixxx::audio::SampleRate(44100); constexpr float kMagicFloat = 1234.567890f; constexpr float kCanaryFloat = 0.0f; constexpr int kChannelCount = 2; @@ -102,7 +103,7 @@ class AnalyzerWaveformTest : public MixxxTest { // Basic test to make sure we don't alter the input buffer and don't step out of bounds. TEST_F(AnalyzerWaveformTest, canary) { m_aw.initialize(m_pTrack, - mixxx::audio::SampleRate(44100), + kCanarySampleRate, kBigBufSize / kChannelCount); m_aw.processSamples(&m_canaryBigBuf[kCanarySize], kBigBufSize); m_aw.storeResults(m_pTrack); From f07fe40c065efa9e1c349ced409386cf2fd9bd99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 10 Aug 2023 14:10:52 +0200 Subject: [PATCH 8/9] Replace tio by pTrack --- src/analyzer/analyzer.h | 4 ++-- src/analyzer/analyzerebur128.cpp | 9 +++++---- src/analyzer/analyzerebur128.h | 4 ++-- src/analyzer/analyzergain.cpp | 9 +++++---- src/analyzer/analyzerkey.cpp | 4 ++-- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/analyzer/analyzer.h b/src/analyzer/analyzer.h index 93906af5946..c9ac651d13b 100644 --- a/src/analyzer/analyzer.h +++ b/src/analyzer/analyzer.h @@ -85,9 +85,9 @@ class AnalyzerWithState final { } } - void finish(TrackPointer tio) { + void finish(TrackPointer pTrack) { if (m_active) { - m_analyzer->storeResults(tio); + m_analyzer->storeResults(pTrack); m_analyzer->cleanup(); m_active = false; } diff --git a/src/analyzer/analyzerebur128.cpp b/src/analyzer/analyzerebur128.cpp index 9c111e619af..033dddcedc8 100644 --- a/src/analyzer/analyzerebur128.cpp +++ b/src/analyzer/analyzerebur128.cpp @@ -59,7 +59,7 @@ bool AnalyzerEbur128::processSamples(const CSAMPLE* pIn, SINT count) { return true; } -void AnalyzerEbur128::storeResults(TrackPointer tio) { +void AnalyzerEbur128::storeResults(TrackPointer pTrack) { VERIFY_OR_DEBUG_ASSERT(m_pState) { return; } @@ -76,8 +76,9 @@ void AnalyzerEbur128::storeResults(TrackPointer tio) { } const double fReplayGain2 = kReplayGain2ReferenceLUFS - averageLufs; - mixxx::ReplayGain replayGain(tio->getReplayGain()); + mixxx::ReplayGain replayGain(pTrack->getReplayGain()); replayGain.setRatio(db2ratio(fReplayGain2)); - tio->setReplayGain(replayGain); - qDebug() << "ReplayGain 2.0 (libebur128) result is" << fReplayGain2 << "dB for" << tio->getFileInfo(); + pTrack->setReplayGain(replayGain); + qDebug() << "ReplayGain 2.0 (libebur128) result is" << fReplayGain2 + << "dB for" << pTrack->getFileInfo(); } diff --git a/src/analyzer/analyzerebur128.h b/src/analyzer/analyzerebur128.h index 3a4b1bed068..1c987b2b656 100644 --- a/src/analyzer/analyzerebur128.h +++ b/src/analyzer/analyzerebur128.h @@ -14,11 +14,11 @@ class AnalyzerEbur128 : public Analyzer { return rgSettings.isAnalyzerEnabled(2); } - bool initialize(TrackPointer tio, + bool initialize(TrackPointer pTrack, mixxx::audio::SampleRate sampleRate, SINT frameLength) override; bool processSamples(const CSAMPLE* pIn, SINT count) override; - void storeResults(TrackPointer tio) override; + void storeResults(TrackPointer pTrack) override; void cleanup() override; private: diff --git a/src/analyzer/analyzergain.cpp b/src/analyzer/analyzergain.cpp index 05177a1693d..2053be32134 100644 --- a/src/analyzer/analyzergain.cpp +++ b/src/analyzer/analyzergain.cpp @@ -57,7 +57,7 @@ bool AnalyzerGain::processSamples(const CSAMPLE* pIn, SINT count) { return m_pReplayGain->process(m_pLeftTempBuffer, m_pRightTempBuffer, numFrames); } -void AnalyzerGain::storeResults(TrackPointer tio) { +void AnalyzerGain::storeResults(TrackPointer pTrack) { //TODO: We are going to store values as relative peaks so that "0" means that no replaygain has been evaluated. // This means that we are going to transform from dB to peaks and vice-versa. // One may think to digg into replay_gain code and modify it so that @@ -70,8 +70,9 @@ void AnalyzerGain::storeResults(TrackPointer tio) { return; } - mixxx::ReplayGain replayGain(tio->getReplayGain()); + mixxx::ReplayGain replayGain(pTrack->getReplayGain()); replayGain.setRatio(db2ratio(fReplayGainOutput)); - tio->setReplayGain(replayGain); - qDebug() << "ReplayGain 1.0 result is" << fReplayGainOutput << "dB for" << tio->getLocation(); + pTrack->setReplayGain(replayGain); + qDebug() << "ReplayGain 1.0 result is" << fReplayGainOutput << "dB for" + << pTrack->getLocation(); } diff --git a/src/analyzer/analyzerkey.cpp b/src/analyzer/analyzerkey.cpp index a36b7207ba9..b8815198088 100644 --- a/src/analyzer/analyzerkey.cpp +++ b/src/analyzer/analyzerkey.cpp @@ -41,7 +41,7 @@ AnalyzerKey::AnalyzerKey(const KeyDetectionSettings& keySettings) m_bPreferencesReanalyzeEnabled(false) { } -bool AnalyzerKey::initialize(TrackPointer tio, +bool AnalyzerKey::initialize(TrackPointer pTrack, mixxx::audio::SampleRate sampleRate, SINT frameLength) { if (frameLength <= 0) { @@ -86,7 +86,7 @@ bool AnalyzerKey::initialize(TrackPointer tio, m_currentFrame = 0; // if we can't load a stored track reanalyze it - bool bShouldAnalyze = shouldAnalyze(tio); + bool bShouldAnalyze = shouldAnalyze(pTrack); DEBUG_ASSERT(!m_pPlugin); if (bShouldAnalyze) { From 2a3436edff39b12debbc44ddb95d090dd268cb3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 10 Aug 2023 23:01:38 +0200 Subject: [PATCH 9/9] Replace kCanarySampleRate with m_pTrack->getSampleRate() --- src/test/analyserwaveformtest.cpp | 3 +-- src/test/analyzersilence_test.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/analyserwaveformtest.cpp b/src/test/analyserwaveformtest.cpp index 10ee20c2d17..5572f01003e 100644 --- a/src/test/analyserwaveformtest.cpp +++ b/src/test/analyserwaveformtest.cpp @@ -12,7 +12,6 @@ namespace { constexpr std::size_t kBigBufSize = 2 * 1920; // Matches the WaveformSummary constexpr std::size_t kCanarySize = 1024 * 4; -constexpr auto kCanarySampleRate = mixxx::audio::SampleRate(44100); constexpr float kMagicFloat = 1234.567890f; constexpr float kCanaryFloat = 0.0f; constexpr int kChannelCount = 2; @@ -103,7 +102,7 @@ class AnalyzerWaveformTest : public MixxxTest { // Basic test to make sure we don't alter the input buffer and don't step out of bounds. TEST_F(AnalyzerWaveformTest, canary) { m_aw.initialize(m_pTrack, - kCanarySampleRate, + m_pTrack->getSampleRate(), kBigBufSize / kChannelCount); m_aw.processSamples(&m_canaryBigBuf[kCanarySize], kBigBufSize); m_aw.storeResults(m_pTrack); diff --git a/src/test/analyzersilence_test.cpp b/src/test/analyzersilence_test.cpp index 28a4e983686..02c2caf904a 100644 --- a/src/test/analyzersilence_test.cpp +++ b/src/test/analyzersilence_test.cpp @@ -38,7 +38,7 @@ class AnalyzerSilenceTest : public MixxxTest { void analyzeTrack() { analyzerSilence.initialize(pTrack, - mixxx::audio::SampleRate(44100), + pTrack->getSampleRate(), kTrackLengthFrames); analyzerSilence.processSamples(pTrackSampleData, nTrackSampleDataLength); analyzerSilence.storeResults(pTrack);