From 26cec5634091899654e44aaa4251e4f0d42f8a8e Mon Sep 17 00:00:00 2001 From: be_ Date: Thu, 7 Dec 2017 18:12:52 -0600 Subject: [PATCH 1/2] Echo effect: reduce size of buffers The Echo effect referred to the mixxx::AudioSignal::SampleRate::max() constant, equal to 192 kHz, to determine the size of the buffers it allocates. This resulted in a huge waste of memory. There is no reason Mixxx should support 192 kHz, even if a user's sound card does. It provides no benefit and is likely to introduce introduce intermodulation distortion when playing ultrasonic frequencies. Refer to http://xiph.org/~xiphmont/demo/neil-young.html for details. --- src/effects/native/echoeffect.h | 8 +++++-- src/preferences/dialog/dlgprefsound.cpp | 2 +- src/soundio/soundmanager.cpp | 26 +++-------------------- src/soundio/soundmanager.h | 22 ++++++++++++------- src/soundio/soundmanagerconfig.cpp | 28 +++++++++++++------------ src/soundio/soundmanagerconfig.h | 2 +- 6 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/effects/native/echoeffect.h b/src/effects/native/echoeffect.h index f2feda191cb2..f26d1a31f593 100644 --- a/src/effects/native/echoeffect.h +++ b/src/effects/native/echoeffect.h @@ -7,6 +7,7 @@ #include "engine/engine.h" #include "engine/effects/engineeffect.h" #include "engine/effects/engineeffectparameter.h" +#include "soundio/soundmanager.h" #include "util/class.h" #include "util/defs.h" #include "util/sample.h" @@ -18,9 +19,12 @@ struct EchoGroupState { static constexpr int kMaxDelaySeconds = 3; static constexpr auto kChannelCount = mixxx::kEngineChannelCount; + // TODO: allocate buffers of the appropriate size when the sample rate is configured EchoGroupState() - : delay_buf(mixxx::AudioSignal::SampleRate::max() * kMaxDelaySeconds * - kChannelCount) { + : delay_buf(kMaxDelaySeconds * kChannelCount * + SoundManager::s_iSupportedSampleRates[ + sizeof(SoundManager::s_iSupportedSampleRates) / + sizeof(SoundManager::s_iSupportedSampleRates[0]) - 1]) { delay_buf.clear(); prev_send = 0.0f; prev_feedback= 0.0f; diff --git a/src/preferences/dialog/dlgprefsound.cpp b/src/preferences/dialog/dlgprefsound.cpp index 527541ef753d..77ab765ebc56 100644 --- a/src/preferences/dialog/dlgprefsound.cpp +++ b/src/preferences/dialog/dlgprefsound.cpp @@ -52,7 +52,7 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent, SoundManager* pSoundManager, this, SLOT(apiChanged(int))); sampleRateComboBox->clear(); - foreach (unsigned int srate, m_pSoundManager->getSampleRates()) { + for (unsigned int srate : SoundManager::s_iSupportedSampleRates) { if (srate > 0) { // no ridiculous sample rate values. prohibiting zero means // avoiding a potential div-by-0 error in ::updateLatencies diff --git a/src/soundio/soundmanager.cpp b/src/soundio/soundmanager.cpp index 1bf64ae2a58c..525f309ed8a1 100644 --- a/src/soundio/soundmanager.cpp +++ b/src/soundio/soundmanager.cpp @@ -61,6 +61,8 @@ const unsigned int kSleepSecondsAfterClosingDevice = 5; #endif } // anonymous namespace +constexpr const unsigned int SoundManager::s_iSupportedSampleRates[3]; + SoundManager::SoundManager(UserSettingsPointer pConfig, EngineMaster *pMaster) : m_pMaster(pMaster), @@ -86,11 +88,6 @@ SoundManager::SoundManager(UserSettingsPointer pConfig, m_pMasterAudioLatencyOverload = new ControlProxy("[Master]", "audio_latency_overload"); - //Hack because PortAudio samplerate enumeration is slow as hell on Linux (ALSA dmix sucks, so we can't blame PortAudio) - m_samplerates.push_back(44100); - m_samplerates.push_back(48000); - m_samplerates.push_back(96000); - m_pNetworkStream = QSharedPointer( new EngineNetworkStream(2, 0)); @@ -242,23 +239,6 @@ void SoundManager::clearDeviceList(bool sleepAfterClosing) { #endif } -QList SoundManager::getSampleRates(QString api) const { -#ifdef __PORTAUDIO__ - if (api == MIXXX_PORTAUDIO_JACK_STRING) { - // queryDevices must have been called for this to work, but the - // ctor calls it -bkgood - QList samplerates; - samplerates.append(m_jackSampleRate); - return samplerates; - } -#endif - return m_samplerates; -} - -QList SoundManager::getSampleRates() const { - return getSampleRates(""); -} - void SoundManager::queryDevices() { //qDebug() << "SoundManager::queryDevices()"; queryDevicesPortaudio(); @@ -585,7 +565,7 @@ void SoundManager::checkConfig() { m_config.setAPI(SoundManagerConfig::kDefaultAPI); m_config.loadDefaults(this, SoundManagerConfig::API | SoundManagerConfig::DEVICES); } - if (!m_config.checkSampleRate(*this)) { + if (!m_config.checkSampleRate()) { m_config.setSampleRate(SoundManagerConfig::kFallbackSampleRate); m_config.loadDefaults(this, SoundManagerConfig::OTHER); } diff --git a/src/soundio/soundmanager.h b/src/soundio/soundmanager.h index ec7a5237a339..d914a0eb8642 100644 --- a/src/soundio/soundmanager.h +++ b/src/soundio/soundmanager.h @@ -59,6 +59,21 @@ class SoundManager : public QObject { SoundManager(UserSettingsPointer pConfig, EngineMaster *_master); virtual ~SoundManager(); + // PortAudio sample rate enumeration is very slow on Linux + // (ALSA dmix sucks, so we can't blame PortAudio), so use this static + // list of supported sample rates. + + // There is no reason Mixxx should support higher sample rates for playback, + // even if a user's sound card does. It provides no benefit and is likely to + // introduce introduce intermodulation distortion when playing ultrasonic + // frequencies. Refer to http://xiph.org/~xiphmont/demo/neil-young.html + // for details. + constexpr static const unsigned int s_iSupportedSampleRates[3] = { + 44100, + 48000, + 96000 + }; + // Returns a list of all devices we've enumerated that match the provided // filterApi, and have at least one output or input channel if the // bOutputDevices or bInputDevices are set, respectively. @@ -82,12 +97,6 @@ class SoundManager : public QObject { QString getErrorDeviceName() const; QString getLastErrorMessage(SoundDeviceError err) const; - // Returns a list of samplerates we will attempt to support for a given API. - QList getSampleRates(QString api) const; - - // Convenience overload for SoundManager::getSampleRates(QString) - QList getSampleRates() const; - // Get a list of host APIs supported by PortAudio. QList getHostAPIList() const; SoundManagerConfig getConfig() const; @@ -150,7 +159,6 @@ class SoundManager : public QObject { unsigned int m_jackSampleRate; #endif QList m_devices; - QList m_samplerates; QList m_inputBuffers; SoundManagerConfig m_config; diff --git a/src/soundio/soundmanagerconfig.cpp b/src/soundio/soundmanagerconfig.cpp index fc02df90925b..01e01e2fad55 100644 --- a/src/soundio/soundmanagerconfig.cpp +++ b/src/soundio/soundmanagerconfig.cpp @@ -216,11 +216,13 @@ void SoundManagerConfig::setForceNetworkClock(bool force) { * @returns false if the sample rate is not found in SoundManager's list, * otherwise true */ -bool SoundManagerConfig::checkSampleRate(const SoundManager &soundManager) { - if (!soundManager.getSampleRates(m_api).contains(m_sampleRate)) { - return false; +bool SoundManagerConfig::checkSampleRate() { + for (const unsigned int& rate : SoundManager::s_iSupportedSampleRates) { + if (rate == m_sampleRate) { + return true; + } } - return true; + return false; } unsigned int SoundManagerConfig::getDeckCount() const { @@ -398,15 +400,15 @@ void SoundManagerConfig::loadDefaults(SoundManager *soundManager, unsigned int f } } if (flags & SoundManagerConfig::OTHER) { - QList sampleRates = soundManager->getSampleRates(m_api); - if (sampleRates.contains(defaultSampleRate)) { - m_sampleRate = defaultSampleRate; - } else if (sampleRates.contains(kFallbackSampleRate)) { - m_sampleRate = kFallbackSampleRate; - } else if (!sampleRates.isEmpty()) { - m_sampleRate = sampleRates.first(); - } else { - qWarning() << "got empty sample rate list from SoundManager, this is a bug"; + m_sampleRate = defaultSampleRate; + bool bDefaultSampleRateIsSupported = false; + for (const unsigned int& rate : SoundManager::s_iSupportedSampleRates) { + if (rate == defaultSampleRate) { + bDefaultSampleRateIsSupported = true; + break; + } + } + if (!bDefaultSampleRateIsSupported) { m_sampleRate = kFallbackSampleRate; } m_audioBufferSizeIndex = kDefaultAudioBufferSizeIndex; diff --git a/src/soundio/soundmanagerconfig.h b/src/soundio/soundmanagerconfig.h index d8f6357cf6c5..875674698baf 100644 --- a/src/soundio/soundmanagerconfig.h +++ b/src/soundio/soundmanagerconfig.h @@ -54,7 +54,7 @@ class SoundManagerConfig { bool checkAPI(const SoundManager &soundManager); unsigned int getSampleRate() const; void setSampleRate(unsigned int sampleRate); - bool checkSampleRate(const SoundManager &soundManager); + bool checkSampleRate(); // Record the number of decks configured with this setup so they can // be created and configured. From 7aa5ad68620a4cf65f14c7e0b128ef734842a75d Mon Sep 17 00:00:00 2001 From: be_ Date: Thu, 7 Dec 2017 20:06:21 -0600 Subject: [PATCH 2/2] Echo: reduce maximum delay time back down to 2 seconds There is a tradeoff between the maximum delay time and the memory required by the effect. The higher the maximum delay time, the lower the minimum BPM that can use the full range of 2 beats. The cutoff point is arbitrary. The default minimum BPM for analysis is set at 70, so I think it is fairly unlikely that a user would use the Echo effect with the maximum delay time of 2 beats with a track less than 60 BPM. --- src/effects/native/echoeffect.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/effects/native/echoeffect.h b/src/effects/native/echoeffect.h index f26d1a31f593..931055c69c1f 100644 --- a/src/effects/native/echoeffect.h +++ b/src/effects/native/echoeffect.h @@ -14,9 +14,9 @@ #include "util/samplebuffer.h" struct EchoGroupState { - // 3 seconds max. This supports the full range of 2 beats for tempos down to - // 40 BPM. - static constexpr int kMaxDelaySeconds = 3; + // 2 seconds max. This supports the full range of 2 beats for tempos down to + // 60 BPM. + static constexpr int kMaxDelaySeconds = 2; static constexpr auto kChannelCount = mixxx::kEngineChannelCount; // TODO: allocate buffers of the appropriate size when the sample rate is configured