Skip to content

reduce memory usage for Echo effect #1302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Be-ing wants to merge 2 commits into from
Closed

reduce memory usage for Echo effect #1302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26cec56
Echo effect: reduce size of buffers
Be-ing Dec 8, 2017
7aa5ad6
Echo: reduce maximum delay time back down to 2 seconds
Be-ing Dec 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/effects/native/echoeffect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,24 @@
#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"
#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
EchoGroupState()
: delay_buf(mixxx::AudioSignal::SampleRate::max() * kMaxDelaySeconds *
kChannelCount) {
: delay_buf(kMaxDelaySeconds * kChannelCount *
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde Dec 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some static functions to SoundManager to avoid leaking internal storage and ordering and to make the code more readable:
static constexpr SINT minSupportedSampleRate()
static constexpr SINT maxSupportedSampleRate()
static bool isSupportedSampleRate(SINT sampleRate)

SoundManager::s_iSupportedSampleRates[
sizeof(SoundManager::s_iSupportedSampleRates) /
sizeof(SoundManager::s_iSupportedSampleRates[0]) - 1]) {
delay_buf.clear();
prev_send = 0.0f;
prev_feedback= 0.0f;
Expand Down
2 changes: 1 addition & 1 deletion src/preferences/dialog/dlgprefsound.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 3 additions & 23 deletions src/soundio/soundmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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<EngineNetworkStream>(
new EngineNetworkStream(2, 0));

Expand Down Expand Up @@ -242,23 +239,6 @@ void SoundManager::clearDeviceList(bool sleepAfterClosing) {
#endif
}

QList<unsigned int> 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<unsigned int> samplerates;
samplerates.append(m_jackSampleRate);
return samplerates;
}
#endif
return m_samplerates;
}

QList<unsigned int> SoundManager::getSampleRates() const {
return getSampleRates("");
}

void SoundManager::queryDevices() {
//qDebug() << "SoundManager::queryDevices()";
queryDevicesPortaudio();
Expand Down Expand Up @@ -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);
}
Expand Down
22 changes: 15 additions & 7 deletions src/soundio/soundmanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde Dec 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be private. Do you explicitly need to state the size here?

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.
Expand All @@ -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<unsigned int> getSampleRates(QString api) const;

// Convenience overload for SoundManager::getSampleRates(QString)
QList<unsigned int> getSampleRates() const;

// Get a list of host APIs supported by PortAudio.
QList<QString> getHostAPIList() const;
SoundManagerConfig getConfig() const;
Expand Down Expand Up @@ -150,7 +159,6 @@ class SoundManager : public QObject {
unsigned int m_jackSampleRate;
#endif
QList<SoundDevice*> m_devices;
QList<unsigned int> m_samplerates;
QList<CSAMPLE*> m_inputBuffers;

SoundManagerConfig m_config;
Expand Down
28 changes: 15 additions & 13 deletions src/soundio/soundmanagerconfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde Dec 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced by:
return SoundManager::isSupportedSampleRate(m_sampleRate);

for (const unsigned int& rate : SoundManager::s_iSupportedSampleRates) {
if (rate == m_sampleRate) {
return true;
}
}
return true;
return false;
}

unsigned int SoundManagerConfig::getDeckCount() const {
Expand Down Expand Up @@ -398,15 +400,15 @@ void SoundManagerConfig::loadDefaults(SoundManager *soundManager, unsigned int f
}
}
if (flags & SoundManagerConfig::OTHER) {
QList<unsigned int> 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;
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde Dec 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

if (SoundManager::isSupportedSampleRate(defaultSampleRate)) {
   m_sampleRate =  defaultSampleRate;
} else {
   DEBUG_ASSERT(SoundManager::isSupportedSampleRate(kFallbackSampleRate));
   m_sampleRate = kFallbackSampleRate;
}

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;
Expand Down
2 changes: 1 addition & 1 deletion src/soundio/soundmanagerconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down