From 5e67d9d3eac8425c3678dac184512a4bc3a37e3a Mon Sep 17 00:00:00 2001 From: Antoine C Date: Sat, 20 Apr 2024 13:50:32 +0100 Subject: [PATCH 1/9] feat: wrap rubberband implementation --- CMakeLists.txt | 1 + .../enginebufferscalerubberband.cpp | 92 +++++++------ .../enginebufferscalerubberband.h | 3 +- .../bufferscalers/rubberbandwrapper.cpp | 122 ++++++++++++++++++ src/engine/bufferscalers/rubberbandwrapper.h | 35 +++++ 5 files changed, 205 insertions(+), 48 deletions(-) create mode 100644 src/engine/bufferscalers/rubberbandwrapper.cpp create mode 100644 src/engine/bufferscalers/rubberbandwrapper.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 7fddb27134b..88cc99e7cd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3245,6 +3245,7 @@ if(RUBBERBAND) target_sources(mixxx-lib PRIVATE src/effects/backends/builtin/pitchshifteffect.cpp src/engine/bufferscalers/enginebufferscalerubberband.cpp + src/engine/bufferscalers/rubberbandwrapper.cpp ) endif() diff --git a/src/engine/bufferscalers/enginebufferscalerubberband.cpp b/src/engine/bufferscalers/enginebufferscalerubberband.cpp index 5e61b4ede74..c6116df184b 100644 --- a/src/engine/bufferscalers/enginebufferscalerubberband.cpp +++ b/src/engine/bufferscalers/enginebufferscalerubberband.cpp @@ -7,7 +7,9 @@ #include "util/counter.h" #include "util/defs.h" #include "util/math.h" +#include "util/mutex.h" #include "util/sample.h" +#include "util/timer.h" using RubberBand::RubberBandStretcher; @@ -55,7 +57,7 @@ void EngineBufferScaleRubberBand::setScaleParameters(double base_rate, if (pitchScale > 0) { //qDebug() << "EngineBufferScaleRubberBand setPitchScale" << *pitch << pitchScale; - m_pRubberBand->setPitchScale(pitchScale); + m_rubberBand.setPitchScale(pitchScale); } // RubberBand handles checking for whether the change in timeRatio is a @@ -65,20 +67,20 @@ void EngineBufferScaleRubberBand::setScaleParameters(double base_rate, double timeRatioInverse = base_rate * speed_abs; if (timeRatioInverse > 0) { //qDebug() << "EngineBufferScaleRubberBand setTimeRatio" << 1 / timeRatioInverse; - m_pRubberBand->setTimeRatio(1.0 / timeRatioInverse); + m_rubberBand.setTimeRatio(1.0 / timeRatioInverse); } if (runningEngineVersion() == 2) { - if (m_pRubberBand->getInputIncrement() == 0) { + if (m_rubberBand.getInputIncrement() == 0) { qWarning() << "EngineBufferScaleRubberBand inputIncrement is 0." << "On RubberBand <=1.8.1 a SIGFPE is imminent despite" << "our workaround. Taking evasive action." << "Please file an issue on https://github.com/mixxxdj/mixxx/issues"; // This is much slower than the minimum seek speed workaround above. - while (m_pRubberBand->getInputIncrement() == 0) { + while (m_rubberBand.getInputIncrement() == 0) { timeRatioInverse += 0.001; - m_pRubberBand->setTimeRatio(1.0 / timeRatioInverse); + m_rubberBand.setTimeRatio(1.0 / timeRatioInverse); } speed_abs = timeRatioInverse / base_rate; *pTempoRatio = m_bBackwards ? -speed_abs : speed_abs; @@ -94,8 +96,8 @@ void EngineBufferScaleRubberBand::onSampleRateChanged() { // TODO: Resetting the sample rate will cause internal // memory allocations that may block the real-time thread. // When is this function actually invoked?? + m_rubberBand.clear(); if (!getOutputSignal().isValid()) { - m_pRubberBand.reset(); return; } RubberBandStretcher::Options rubberbandOptions = @@ -110,19 +112,19 @@ void EngineBufferScaleRubberBand::onSampleRateChanged() { } #endif - m_pRubberBand = std::make_unique( + m_rubberBand.setup( getOutputSignal().getSampleRate(), getOutputSignal().getChannelCount(), rubberbandOptions); // Setting the time ratio to a very high value will cause RubberBand // to preallocate buffers large enough to (almost certainly) // avoid memory reallocations during playback. - m_pRubberBand->setTimeRatio(2.0); - m_pRubberBand->setTimeRatio(1.0); + m_rubberBand.setTimeRatio(2.0); + m_rubberBand.setTimeRatio(1.0); } void EngineBufferScaleRubberBand::clear() { - VERIFY_OR_DEBUG_ASSERT(m_pRubberBand) { + VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { return; } reset(); @@ -131,14 +133,21 @@ void EngineBufferScaleRubberBand::clear() { SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave( CSAMPLE* pBuffer, SINT frames) { - const SINT frames_available = m_pRubberBand->available(); + VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { + return 0; + } + const SINT frames_available = m_rubberBand.available(); // NOTE: If we still need to throw away padding, then we can also // immediately read those frames in addition to the frames we actually // need for the output const SINT frames_to_read = math_min(frames_available, frames + m_remainingPaddingInOutput); - DEBUG_ASSERT(frames_to_read <= m_buffers[0].size()); - SINT received_frames = static_cast(m_pRubberBand->retrieve( - m_bufferPtrs.data(), frames_to_read)); + DEBUG_ASSERT(frames_to_read <= MAX_BUFFER_LEN); + SINT received_frames; + { + ScopedTimer t(u"RubberBand::retrieve"); + received_frames = static_cast(m_rubberBand.retrieve( + m_bufferPtrs.data(), frames_to_read)); + } SINT frame_offset = 0; // As explained below in `reset()`, the first time this is called we need to @@ -164,6 +173,9 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave( void EngineBufferScaleRubberBand::deinterleaveAndProcess( const CSAMPLE* pBuffer, SINT frames) { + VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { + return; + } DEBUG_ASSERT(frames <= static_cast(m_buffers[0].size())); SampleUtil::deinterleaveBuffer( @@ -172,14 +184,21 @@ void EngineBufferScaleRubberBand::deinterleaveAndProcess( pBuffer, frames); - m_pRubberBand->process(m_bufferPtrs.data(), - frames, - false); + { + ScopedTimer t(u"RubberBand::process"); + m_rubberBand.process(m_bufferPtrs.data(), + frames, + false); + } } double EngineBufferScaleRubberBand::scaleBuffer( CSAMPLE* pOutputBuffer, SINT iOutputBufferSize) { + VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { + return 0.0; + } + ScopedTimer t(u"EngineBufferScaleRubberBand::scaleBuffer"); if (m_dBaseRate == 0.0 || m_dTempoRatio == 0.0) { SampleUtil::clear(pOutputBuffer, iOutputBufferSize); // No actual samples/frames have been read from the @@ -206,7 +225,7 @@ double EngineBufferScaleRubberBand::scaleBuffer( read += getOutputSignal().frames2samples(received_frames); const SINT next_block_frames_required = - static_cast(m_pRubberBand->getSamplesRequired()); + static_cast(m_rubberBand.getSamplesRequired()); if (remaining_frames > 0 && next_block_frames_required > 0) { // The requested setting becomes effective after all previous frames have been processed m_effectiveRate = m_dBaseRate * m_dTempoRatio; @@ -266,44 +285,20 @@ void EngineBufferScaleRubberBand::useEngineFiner(bool enable) { } } -// See -// https://github.com/breakfastquay/rubberband/commit/72654b04ea4f0707e214377515119e933efbdd6c -// for how these two functions were implemented within librubberband itself size_t EngineBufferScaleRubberBand::getPreferredStartPad() const { -#if RUBBERBANDV3 - return m_pRubberBand->getPreferredStartPad(); -#else - // `getPreferredStartPad()` returns `window_size / 2`, while with - // `getLatency()` both time stretching engines return `window_size / 2 / - // pitch_scale` - return static_cast(std::ceil( - m_pRubberBand->getLatency() * m_pRubberBand->getPitchScale())); -#endif + return m_rubberBand.getPreferredStartPad(); } size_t EngineBufferScaleRubberBand::getStartDelay() const { -#if RUBBERBANDV3 - return m_pRubberBand->getStartDelay(); -#else - // In newer Rubber Band versions `getLatency()` is a deprecated alias for - // `getStartDelay()`, so they should behave the same. In the commit linked - // above the behavior was different for the R3 stretcher, but that was only - // during the initial betas of Rubberband 3.0 so we shouldn't have to worry - // about that. - return m_pRubberBand->getLatency(); -#endif + return m_rubberBand.getStartDelay(); } int EngineBufferScaleRubberBand::runningEngineVersion() { -#if RUBBERBANDV3 - return m_pRubberBand->getEngineVersion(); -#else - return 2; -#endif + return m_rubberBand.getEngineVersion(); } void EngineBufferScaleRubberBand::reset() { - m_pRubberBand->reset(); + m_rubberBand.reset(); // As mentioned in the docs (https://breakfastquay.com/rubberband/code-doc/) // and FAQ (https://breakfastquay.com/rubberband/integration.html#faqs), you @@ -319,7 +314,10 @@ void EngineBufferScaleRubberBand::reset() { std::fill_n(m_buffers[1].span().begin(), block_size, 0.0f); while (remaining_padding > 0) { const size_t pad_samples = std::min(remaining_padding, block_size); - m_pRubberBand->process(m_bufferPtrs.data(), pad_samples, false); + { + ScopedTimer t(u"RubberBand::process"); + m_rubberBand.process(m_bufferPtrs.data(), pad_samples, false); + } remaining_padding -= pad_samples; } diff --git a/src/engine/bufferscalers/enginebufferscalerubberband.h b/src/engine/bufferscalers/enginebufferscalerubberband.h index 9e5bc7d2763..06c6b67d562 100644 --- a/src/engine/bufferscalers/enginebufferscalerubberband.h +++ b/src/engine/bufferscalers/enginebufferscalerubberband.h @@ -6,6 +6,7 @@ #include #include "engine/bufferscalers/enginebufferscale.h" +#include "engine/bufferscalers/rubberbandwrapper.h" #include "util/samplebuffer.h" class ReadAheadManager; @@ -62,7 +63,7 @@ class EngineBufferScaleRubberBand final : public EngineBufferScale { // The read-ahead manager that we use to fetch samples ReadAheadManager* m_pReadAheadManager; - std::unique_ptr m_pRubberBand; + RubberBandWrapper m_rubberBand; /// The audio buffers samples used to send audio to Rubber Band and to /// receive processed audio from Rubber Band. This is needed because Mixxx diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp new file mode 100644 index 00000000000..724ac75ca5b --- /dev/null +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -0,0 +1,122 @@ +#include "engine/bufferscalers/rubberbandwrapper.h" + +#include "util/assert.h" + +using RubberBand::RubberBandStretcher; + +#define RUBBERBANDV3 (RUBBERBAND_API_MAJOR_VERSION >= 2 && RUBBERBAND_API_MINOR_VERSION >= 7) + +int RubberBandWrapper::getEngineVersion() const { +#if RUBBERBANDV3 + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->getEngineVersion(); +#else + return 2; +#endif +} +void RubberBandWrapper::setTimeRatio(double ratio) { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return; + } + m_pInstance->setTimeRatio(ratio); +} +size_t RubberBandWrapper::getSamplesRequired() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->getSamplesRequired(); +} +int RubberBandWrapper::available() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->available(); +} +size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->retrieve(output, samples); +} +size_t RubberBandWrapper::getInputIncrement() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->getInputIncrement(); +} +size_t RubberBandWrapper::getLatency() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->getLatency(); +} +double RubberBandWrapper::getPitchScale() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } + return m_pInstance->getPitchScale(); +} +// See +// https://github.com/breakfastquay/rubberband/commit/72654b04ea4f0707e214377515119e933efbdd6c +// for how these two functions were implemented within librubberband itself +size_t RubberBandWrapper::getPreferredStartPad() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } +#if RUBBERBANDV3 + return m_pInstance->getPreferredStartPad(); +#else + // `getPreferredStartPad()` returns `window_size / 2`, while with + // `getLatency()` both time stretching engines return `window_size / 2 / + // pitch_scale` + return static_cast(std::ceil( + m_pInstance->getLatency() * m_pInstance->getPitchScale())); +#endif +} +size_t RubberBandWrapper::getStartDelay() const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return -1; + } +#if RUBBERBANDV3 + return m_pInstance->getStartDelay(); +#else + // In newer Rubber Band versions `getLatency()` is a deprecated alias for + // `getStartDelay()`, so they should behave the same. In the commit linked + // above the behavior was different for the R3 stretcher, but that was only + // during the initial betas of Rubberband 3.0 so we shouldn't have to worry + // about that. + return m_pInstance->getLatency(); +#endif +} +void RubberBandWrapper::process(const float* const* input, size_t samples, bool final) { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return; + } + return m_pInstance->process(input, samples, final); +} +void RubberBandWrapper::reset() { + m_pInstance->reset(); +} +void RubberBandWrapper::clear() { + m_pInstance.reset(); +} +void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, + mixxx::audio::ChannelCount chCount, + const RubberBandStretcher::Options& opt) { + // The instance should have been cleared, or not set before + VERIFY_OR_DEBUG_ASSERT(!m_pInstance) { + m_pInstance.reset(); + } + + m_pInstance = std::make_unique( + sampleRate, chCount, opt); +} +void RubberBandWrapper::setPitchScale(double scale) { + m_pInstance->setPitchScale(scale); +} + +bool RubberBandWrapper::isValid() const { + return !!m_pInstance; +} diff --git a/src/engine/bufferscalers/rubberbandwrapper.h b/src/engine/bufferscalers/rubberbandwrapper.h new file mode 100644 index 00000000000..95e0afd23dd --- /dev/null +++ b/src/engine/bufferscalers/rubberbandwrapper.h @@ -0,0 +1,35 @@ +#pragma once + +#include + +#include "audio/types.h" + +/// RubberBandWrapper is a wrapper around RubberBand::RubberBandStretcher which +/// allows to distribute signal stretching over multiple instance, but interface +/// with it like if it was a single instance +class RubberBandWrapper { + public: + int getEngineVersion() const; + void setTimeRatio(double ratio); + size_t getSamplesRequired() const; + int available() const; + size_t retrieve(float* const* output, size_t samples) const; + size_t getInputIncrement() const; + size_t getLatency() const; + double getPitchScale() const; + size_t getPreferredStartPad() const; + size_t getStartDelay() const; + void process(const float* const* input, size_t samples, bool final); + void setPitchScale(double scale); + void reset(); + + // The following method are helper function and do not wrap any RubberBand calls + void clear(); + void setup(mixxx::audio::SampleRate sampleRate, + mixxx::audio::ChannelCount chCount, + const RubberBand::RubberBandStretcher::Options& opt); + bool isValid() const; + + private: + std::unique_ptr m_pInstance; +}; From 18968a4d43e57b80a146f66f2801a2161394783e Mon Sep 17 00:00:00 2001 From: Antoine C Date: Sun, 19 May 2024 15:45:17 +0100 Subject: [PATCH 2/9] feat: add multithreading processing on multi channel tracks --- CMakeLists.txt | 2 + src/coreservices.cpp | 9 ++ src/engine/bufferscalers/rubberbandworker.cpp | 56 ++++++++++ src/engine/bufferscalers/rubberbandworker.h | 52 +++++++++ .../bufferscalers/rubberbandworkerpool.cpp | 44 ++++++++ .../bufferscalers/rubberbandworkerpool.h | 37 +++++++ .../bufferscalers/rubberbandwrapper.cpp | 102 ++++++++++++------ src/engine/bufferscalers/rubberbandwrapper.h | 2 +- src/test/enginebuffertest.cpp | 3 + 9 files changed, 274 insertions(+), 33 deletions(-) create mode 100644 src/engine/bufferscalers/rubberbandworker.cpp create mode 100644 src/engine/bufferscalers/rubberbandworker.h create mode 100644 src/engine/bufferscalers/rubberbandworkerpool.cpp create mode 100644 src/engine/bufferscalers/rubberbandworkerpool.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 88cc99e7cd3..6913fb3402a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3246,6 +3246,8 @@ if(RUBBERBAND) src/effects/backends/builtin/pitchshifteffect.cpp src/engine/bufferscalers/enginebufferscalerubberband.cpp src/engine/bufferscalers/rubberbandwrapper.cpp + src/engine/bufferscalers/rubberbandworker.cpp + src/engine/bufferscalers/rubberbandworkerpool.cpp ) endif() diff --git a/src/coreservices.cpp b/src/coreservices.cpp index e1b57089f90..86cd2f49d1b 100644 --- a/src/coreservices.cpp +++ b/src/coreservices.cpp @@ -14,6 +14,9 @@ #include "database/mixxxdb.h" #include "effects/effectsmanager.h" #include "engine/enginemixer.h" +#ifdef __RUBBERBAND__ +#include "engine/bufferscalers/rubberbandworkerpool.h" +#endif #include "library/coverartcache.h" #include "library/library.h" #include "library/library_prefs.h" @@ -267,6 +270,9 @@ void CoreServices::initialize(QApplication* pApp) { m_pEffectsManager.get(), pChannelHandleFactory, true); +#ifdef __RUBBERBAND__ + RubberBandWorkerPool::createInstance(); +#endif emit initializationProgressUpdate(30, tr("audio interface")); // Although m_pSoundManager is created here, m_pSoundManager->setupDevices() @@ -608,6 +614,9 @@ void CoreServices::finalize() { // EngineMixer depends on Config and m_pEffectsManager. qDebug() << t.elapsed(false).debugMillisWithUnit() << "deleting EngineMixer"; CLEAR_AND_CHECK_DELETED(m_pEngine); +#ifdef __RUBBERBAND__ + RubberBandWorkerPool::destroy(); +#endif // Destroy PlayerInfo explicitly to release the track // pointers of tracks that were still loaded in decks diff --git a/src/engine/bufferscalers/rubberbandworker.cpp b/src/engine/bufferscalers/rubberbandworker.cpp new file mode 100644 index 00000000000..8e6669d61d9 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandworker.cpp @@ -0,0 +1,56 @@ +#include "engine/bufferscalers/rubberbandworker.h" + +#include "engine/engine.h" +#include "moc_rubberbandworker.cpp" +#include "util/assert.h" +#include "util/compatibility/qmutex.h" + +using RubberBand::RubberBandStretcher; + +RubberBandWorker::RubberBandWorker() + : QThread(), + m_currentJob(Job{nullptr, nullptr, 0, false}), + m_completed(false) { +} + +void RubberBandWorker::schedule(RubberBand::RubberBandStretcher* stretcher, + const float* const* input, + size_t samples, + bool final) { + auto locker = lockMutex(&m_waitLock); + m_currentJob.instance = stretcher; + m_currentJob.input = input; + m_currentJob.samples = samples; + m_currentJob.final = final; + m_completed = false; + m_waitCondition.wakeOne(); +} + +void RubberBandWorker::waitReady() { + auto locker = lockMutex(&m_waitLock); + while (!m_completed) { + m_waitCondition.wait(&m_waitLock); + } +} + +void RubberBandWorker::stop() { + requestInterruption(); + m_waitCondition.wakeOne(); + wait(); +} +void RubberBandWorker::run() { + auto locker = lockMutex(&m_waitLock); + while (!isInterruptionRequested()) { + if (!m_completed && m_currentJob.instance && m_currentJob.input && m_currentJob.samples) { + m_currentJob.instance->process(m_currentJob.input, + m_currentJob.samples, + m_currentJob.final); + m_completed = true; + DEBUG_ASSERT(m_assigned.test(std::memory_order_relaxed)); + m_assigned.clear(std::memory_order_release); + m_waitCondition.wakeOne(); + } + m_waitCondition.wait(&m_waitLock); + } + quit(); +} diff --git a/src/engine/bufferscalers/rubberbandworker.h b/src/engine/bufferscalers/rubberbandworker.h new file mode 100644 index 00000000000..dbfaa864f80 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandworker.h @@ -0,0 +1,52 @@ +#pragma once + +#include + +#include +#include +#include +#include + +#include "audio/types.h" + +class RubberBandWorkerPool; +class RubberBandWorker : public QThread { + Q_OBJECT + public: + // Wait for the current job to complete. + void waitReady(); + + static constexpr mixxx::audio::ChannelCount kChannelPerWorker = + mixxx::audio::ChannelCount::mono(); + + protected: + RubberBandWorker(); + + void run() override; + void schedule(RubberBand::RubberBandStretcher* stretcher, + const float* const* input, + size_t samples, + bool final); + void stop(); + + private: + struct Job { + RubberBand::RubberBandStretcher* instance; + const float* const* input; + size_t samples; + bool final; + }; + /// Contains the scheduled job. May be dangling if completed=true + Job m_currentJob; + // Used to schedule the thread + QMutex m_waitLock; + QWaitCondition m_waitCondition; + // Whether or not the scheduled job as completed + bool m_completed; + + // Used by RubberBandWorkerPool to manage thread availability. + // RubberBandWorker only clears the flag, once the job is completed + std::atomic_flag m_assigned; + + friend RubberBandWorkerPool; +}; diff --git a/src/engine/bufferscalers/rubberbandworkerpool.cpp b/src/engine/bufferscalers/rubberbandworkerpool.cpp new file mode 100644 index 00000000000..02f3a73b525 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandworkerpool.cpp @@ -0,0 +1,44 @@ +#include "engine/bufferscalers/rubberbandworkerpool.h" + +#include + +#include "engine/bufferscalers/rubberbandworker.h" +#include "engine/engine.h" +#include "util/assert.h" + +RubberBandWorkerPool::RubberBandWorkerPool() { + DEBUG_ASSERT(0 == mixxx::kEngineChannelCount % RubberBandWorker::kChannelPerWorker); + + // We allocate one runner less than the total of maximum supported channel, + // so the engine thread will also perform a stretching operation, instead of + // waiting all workers to complete. During performance testing, this ahas + // show better results + for (int w = 1; w < mixxx::kEngineChannelCount / RubberBandWorker::kChannelPerWorker; w++) { + m_workers.emplace_back( + // We cannot use make_unique here because RubberBandWorker ctor + // is protected to prevent direct usage. + new RubberBandWorker); + m_workers.back()->start(QThread::HighPriority); + } +} + +RubberBandWorkerPool::~RubberBandWorkerPool() { + for (auto& member : m_workers) { + member->stop(); + } + m_workers.clear(); +} + +RubberBandWorker* RubberBandWorkerPool::submit( + RubberBand::RubberBandStretcher* stretcher, + const float* const* input, + size_t samples, + bool final) { + for (auto& member : m_workers) { + if (!member->m_assigned.test_and_set(std::memory_order_acquire)) { + member->schedule(stretcher, input, samples, final); + return member.get(); + } + } + return nullptr; +} diff --git a/src/engine/bufferscalers/rubberbandworkerpool.h b/src/engine/bufferscalers/rubberbandworkerpool.h new file mode 100644 index 00000000000..3f3716089f9 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandworkerpool.h @@ -0,0 +1,37 @@ +#pragma once + +#include "util/singleton.h" + +class RubberBandWorker; +namespace RubberBand { +class RubberBandStretcher; +} + +// RubberBandWorkerPool is a global pool manager for RubberBandWorkerPool. It +// allows a the Engine thread to use a pool of agnostic RubberBandWorker which +// can be distributed stretching job +class RubberBandWorkerPool : public Singleton { + public: + ~RubberBandWorkerPool(); + + /// @brief Submit a new stretching job + /// @param stretcher the RubberBand::RubberBandStretcher instance to use. + /// Must remain valid till waitReady() as returned + /// @param input The samples buffer + /// @param samples the samples count + /// @param final whether or not this is the final buffer + /// @return the worker on which the job as been schedule one, or null if + /// none available + RubberBandWorker* submit(RubberBand::RubberBandStretcher* stretcher, + const float* const* input, + size_t samples, + bool final); + + protected: + RubberBandWorkerPool(); + + private: + std::vector> m_workers; + + friend class Singleton; +}; diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp index 724ac75ca5b..e5cefaf4684 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.cpp +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -1,5 +1,7 @@ #include "engine/bufferscalers/rubberbandwrapper.h" +#include "engine/bufferscalers/rubberbandworker.h" +#include "engine/bufferscalers/rubberbandworkerpool.h" #include "util/assert.h" using RubberBand::RubberBandStretcher; @@ -7,56 +9,65 @@ using RubberBand::RubberBandStretcher; #define RUBBERBANDV3 (RUBBERBAND_API_MAJOR_VERSION >= 2 && RUBBERBAND_API_MINOR_VERSION >= 7) int RubberBandWrapper::getEngineVersion() const { -#if RUBBERBANDV3 VERIFY_OR_DEBUG_ASSERT(isValid()) { return -1; } - return m_pInstance->getEngineVersion(); +#if RUBBERBANDV3 + return m_pInstances[0]->getEngineVersion(); #else return 2; #endif } void RubberBandWrapper::setTimeRatio(double ratio) { - VERIFY_OR_DEBUG_ASSERT(isValid()) { - return; + for (auto& stretcher : m_pInstances) { + stretcher->setTimeRatio(ratio); } - m_pInstance->setTimeRatio(ratio); } size_t RubberBandWrapper::getSamplesRequired() const { - VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + size_t require = 0; + for (auto& stretcher : m_pInstances) { + require = qMax(require, stretcher->getSamplesRequired()); } - return m_pInstance->getSamplesRequired(); + return require; } int RubberBandWrapper::available() const { - VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + int available = std::numeric_limits::max(); + for (auto& stretcher : m_pInstances) { + available = qMin(available, stretcher->available()); } - return m_pInstance->available(); + return available; } size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { - VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + if (m_pInstances.size() == 1) { + return m_pInstances[0]->retrieve(output, samples); + } else { + size_t ret = 0; + for (const auto& stretcher : m_pInstances) { + size_t thisRet = stretcher->retrieve(output, samples); + DEBUG_ASSERT(!ret || thisRet == ret); + ret = qMax(thisRet, ret); + output += RubberBandWorker::kChannelPerWorker; + } + return ret; } - return m_pInstance->retrieve(output, samples); } size_t RubberBandWrapper::getInputIncrement() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { return -1; } - return m_pInstance->getInputIncrement(); + return m_pInstances[0]->getInputIncrement(); } size_t RubberBandWrapper::getLatency() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { return -1; } - return m_pInstance->getLatency(); + return m_pInstances[0]->getLatency(); } double RubberBandWrapper::getPitchScale() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { return -1; } - return m_pInstance->getPitchScale(); + return m_pInstances[0]->getPitchScale(); } // See // https://github.com/breakfastquay/rubberband/commit/72654b04ea4f0707e214377515119e933efbdd6c @@ -66,13 +77,13 @@ size_t RubberBandWrapper::getPreferredStartPad() const { return -1; } #if RUBBERBANDV3 - return m_pInstance->getPreferredStartPad(); + return m_pInstances[0]->getPreferredStartPad(); #else // `getPreferredStartPad()` returns `window_size / 2`, while with // `getLatency()` both time stretching engines return `window_size / 2 / // pitch_scale` return static_cast(std::ceil( - m_pInstance->getLatency() * m_pInstance->getPitchScale())); + m_pInstances[0]->getLatency() * m_pInstances[0]->getPitchScale())); #endif } size_t RubberBandWrapper::getStartDelay() const { @@ -80,43 +91,70 @@ size_t RubberBandWrapper::getStartDelay() const { return -1; } #if RUBBERBANDV3 - return m_pInstance->getStartDelay(); + return m_pInstances[0]->getStartDelay(); #else // In newer Rubber Band versions `getLatency()` is a deprecated alias for // `getStartDelay()`, so they should behave the same. In the commit linked // above the behavior was different for the R3 stretcher, but that was only // during the initial betas of Rubberband 3.0 so we shouldn't have to worry // about that. - return m_pInstance->getLatency(); + return m_pInstances[0]->getLatency(); #endif } void RubberBandWrapper::process(const float* const* input, size_t samples, bool final) { - VERIFY_OR_DEBUG_ASSERT(isValid()) { - return; + if (m_pInstances.size() == 1) { + return m_pInstances[0]->process(input, samples, final); + } else { + RubberBandWorkerPool* pPool = RubberBandWorkerPool::instance(); + QSet workers; + for (auto& instance : m_pInstances) { + auto pWorker = pPool->submit(instance.get(), input, samples, final); + if (!pWorker) { + instance->process(input, samples, final); + } else { + workers.insert(pWorker); + } + input += RubberBandWorker::kChannelPerWorker; + } + for (auto& worker : workers) { + worker->waitReady(); + } } - return m_pInstance->process(input, samples, final); } void RubberBandWrapper::reset() { - m_pInstance->reset(); + for (auto& stretcher : m_pInstances) { + stretcher->reset(); + } } void RubberBandWrapper::clear() { - m_pInstance.reset(); + m_pInstances.clear(); } void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, mixxx::audio::ChannelCount chCount, const RubberBandStretcher::Options& opt) { // The instance should have been cleared, or not set before - VERIFY_OR_DEBUG_ASSERT(!m_pInstance) { - m_pInstance.reset(); + VERIFY_OR_DEBUG_ASSERT(m_pInstances.size() == 0) { + m_pInstances.clear(); + } + VERIFY_OR_DEBUG_ASSERT(0 == chCount % RubberBandWorker::kChannelPerWorker) { + m_pInstances.emplace_back( + std::make_unique( + sampleRate, chCount, opt)); + return; } - m_pInstance = std::make_unique( - sampleRate, chCount, opt); + for (int c = 0; c < chCount; c += RubberBandWorker::kChannelPerWorker) { + m_pInstances.emplace_back( + std::make_unique( + sampleRate, RubberBandWorker::kChannelPerWorker, opt)); + } } void RubberBandWrapper::setPitchScale(double scale) { - m_pInstance->setPitchScale(scale); + for (auto& stretcher : m_pInstances) { + stretcher->setPitchScale(scale); + } } bool RubberBandWrapper::isValid() const { - return !!m_pInstance; + return m_pInstances.size(); } diff --git a/src/engine/bufferscalers/rubberbandwrapper.h b/src/engine/bufferscalers/rubberbandwrapper.h index 95e0afd23dd..080f04b8681 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.h +++ b/src/engine/bufferscalers/rubberbandwrapper.h @@ -31,5 +31,5 @@ class RubberBandWrapper { bool isValid() const; private: - std::unique_ptr m_pInstance; + std::vector> m_pInstances; }; diff --git a/src/test/enginebuffertest.cpp b/src/test/enginebuffertest.cpp index 274c035adfc..6a0330de656 100644 --- a/src/test/enginebuffertest.cpp +++ b/src/test/enginebuffertest.cpp @@ -8,6 +8,7 @@ #include #include "control/controlobject.h" +#include "engine/bufferscalers/rubberbandworkerpool.h" #include "engine/controls/ratecontrol.h" #include "mixer/basetrackplayer.h" #include "preferences/usersettings.h" @@ -423,6 +424,7 @@ TEST_F(EngineBufferE2ETest, SoundTouchReverseTest) { #ifdef __RUBBERBAND__ TEST_F(EngineBufferE2ETest, RubberbandReverseTest) { + RubberBandWorkerPool::createInstance(); // This test must not crash when changing to reverse while pitch is tweaked // Testing issue #8061 ControlObject::set(ConfigKey(kAppGroup, QStringLiteral("keylock_engine")), @@ -434,6 +436,7 @@ TEST_F(EngineBufferE2ETest, RubberbandReverseTest) { ProcessBuffer(); // Note: we cannot compare a golden buffer here, because the result depends // on the uses library version + RubberBandWorkerPool::destroy(); } #endif From 75923e9fd8bbcf3c5a5903a4f81bbd59a8b61e72 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Wed, 22 May 2024 16:03:39 +0100 Subject: [PATCH 3/9] Add a setting in the UI --- src/coreservices.cpp | 2 +- src/engine/bufferscalers/rubberbandworker.h | 3 - .../bufferscalers/rubberbandworkerpool.cpp | 13 ++- .../bufferscalers/rubberbandworkerpool.h | 9 +- .../bufferscalers/rubberbandwrapper.cpp | 24 +++-- src/preferences/dialog/dlgprefsound.cpp | 93 ++++++++++++++++++- src/preferences/dialog/dlgprefsound.h | 3 + src/preferences/dialog/dlgprefsounddlg.ui | 27 +++++- src/test/enginebuffertest.cpp | 3 - src/test/playermanagertest.cpp | 7 ++ src/test/signalpathtest.h | 22 ++++- src/util/singleton.h | 5 +- 12 files changed, 186 insertions(+), 25 deletions(-) diff --git a/src/coreservices.cpp b/src/coreservices.cpp index 86cd2f49d1b..af0f6a73963 100644 --- a/src/coreservices.cpp +++ b/src/coreservices.cpp @@ -271,7 +271,7 @@ void CoreServices::initialize(QApplication* pApp) { pChannelHandleFactory, true); #ifdef __RUBBERBAND__ - RubberBandWorkerPool::createInstance(); + RubberBandWorkerPool::createInstance(pConfig); #endif emit initializationProgressUpdate(30, tr("audio interface")); diff --git a/src/engine/bufferscalers/rubberbandworker.h b/src/engine/bufferscalers/rubberbandworker.h index dbfaa864f80..392e9b70c60 100644 --- a/src/engine/bufferscalers/rubberbandworker.h +++ b/src/engine/bufferscalers/rubberbandworker.h @@ -16,9 +16,6 @@ class RubberBandWorker : public QThread { // Wait for the current job to complete. void waitReady(); - static constexpr mixxx::audio::ChannelCount kChannelPerWorker = - mixxx::audio::ChannelCount::mono(); - protected: RubberBandWorker(); diff --git a/src/engine/bufferscalers/rubberbandworkerpool.cpp b/src/engine/bufferscalers/rubberbandworkerpool.cpp index 02f3a73b525..8dea16a7600 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.cpp +++ b/src/engine/bufferscalers/rubberbandworkerpool.cpp @@ -6,14 +6,21 @@ #include "engine/engine.h" #include "util/assert.h" -RubberBandWorkerPool::RubberBandWorkerPool() { - DEBUG_ASSERT(0 == mixxx::kEngineChannelCount % RubberBandWorker::kChannelPerWorker); +RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) { + bool multiThreadedOnStereo = pConfig && + pConfig->getValue(ConfigKey(QStringLiteral("[App]"), + QStringLiteral("keylock_multithreading")), + false); + m_channelPerWorker = multiThreadedOnStereo + ? mixxx::audio::ChannelCount::mono() + : mixxx::audio::ChannelCount::stereo(); + DEBUG_ASSERT(0 == mixxx::kEngineChannelCount % m_channelPerWorker); // We allocate one runner less than the total of maximum supported channel, // so the engine thread will also perform a stretching operation, instead of // waiting all workers to complete. During performance testing, this ahas // show better results - for (int w = 1; w < mixxx::kEngineChannelCount / RubberBandWorker::kChannelPerWorker; w++) { + for (int w = 1; w < mixxx::kEngineChannelCount / m_channelPerWorker; w++) { m_workers.emplace_back( // We cannot use make_unique here because RubberBandWorker ctor // is protected to prevent direct usage. diff --git a/src/engine/bufferscalers/rubberbandworkerpool.h b/src/engine/bufferscalers/rubberbandworkerpool.h index 3f3716089f9..087be5eca3b 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.h +++ b/src/engine/bufferscalers/rubberbandworkerpool.h @@ -1,5 +1,7 @@ #pragma once +#include "audio/types.h" +#include "preferences/usersettings.h" #include "util/singleton.h" class RubberBandWorker; @@ -27,11 +29,16 @@ class RubberBandWorkerPool : public Singleton { size_t samples, bool final); + const mixxx::audio::ChannelCount& channelPerWorker() const { + return m_channelPerWorker; + } + protected: - RubberBandWorkerPool(); + RubberBandWorkerPool(UserSettingsPointer pConfig = nullptr); private: std::vector> m_workers; + mixxx::audio::ChannelCount m_channelPerWorker; friend class Singleton; }; diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp index e5cefaf4684..c5a1a5cbe2b 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.cpp +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -2,12 +2,21 @@ #include "engine/bufferscalers/rubberbandworker.h" #include "engine/bufferscalers/rubberbandworkerpool.h" +#include "engine/engine.h" #include "util/assert.h" using RubberBand::RubberBandStretcher; #define RUBBERBANDV3 (RUBBERBAND_API_MAJOR_VERSION >= 2 && RUBBERBAND_API_MINOR_VERSION >= 7) +namespace { + +mixxx::audio::ChannelCount getChannelPerWorker() { + RubberBandWorkerPool* pPool = RubberBandWorkerPool::instance(); + return pPool ? pPool->channelPerWorker() : mixxx::kEngineChannelCount; +} +} // namespace + int RubberBandWrapper::getEngineVersion() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { return -1; @@ -46,7 +55,7 @@ size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { size_t thisRet = stretcher->retrieve(output, samples); DEBUG_ASSERT(!ret || thisRet == ret); ret = qMax(thisRet, ret); - output += RubberBandWorker::kChannelPerWorker; + output += getChannelPerWorker(); } return ret; } @@ -114,7 +123,7 @@ void RubberBandWrapper::process(const float* const* input, size_t samples, bool } else { workers.insert(pWorker); } - input += RubberBandWorker::kChannelPerWorker; + input += pPool->channelPerWorker(); } for (auto& worker : workers) { worker->waitReady(); @@ -135,18 +144,21 @@ void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, // The instance should have been cleared, or not set before VERIFY_OR_DEBUG_ASSERT(m_pInstances.size() == 0) { m_pInstances.clear(); - } - VERIFY_OR_DEBUG_ASSERT(0 == chCount % RubberBandWorker::kChannelPerWorker) { + }; + + auto channelPerWorker = getChannelPerWorker(); + qDebug() << "RubberBandWrapper::setup" << channelPerWorker; + VERIFY_OR_DEBUG_ASSERT(0 == chCount % channelPerWorker) { m_pInstances.emplace_back( std::make_unique( sampleRate, chCount, opt)); return; } - for (int c = 0; c < chCount; c += RubberBandWorker::kChannelPerWorker) { + for (int c = 0; c < chCount; c += channelPerWorker) { m_pInstances.emplace_back( std::make_unique( - sampleRate, RubberBandWorker::kChannelPerWorker, opt)); + sampleRate, channelPerWorker, opt)); } } void RubberBandWrapper::setPitchScale(double scale) { diff --git a/src/preferences/dialog/dlgprefsound.cpp b/src/preferences/dialog/dlgprefsound.cpp index 003d92374d6..8b452aec01a 100644 --- a/src/preferences/dialog/dlgprefsound.cpp +++ b/src/preferences/dialog/dlgprefsound.cpp @@ -15,6 +15,10 @@ #include "util/rlimit.h" #include "util/scopedoverridecursor.h" +#ifdef __RUBBERBAND__ +#include "engine/bufferscalers/rubberbandworkerpool.h" +#endif + namespace { const QString kAppGroup = QStringLiteral("[App]"); @@ -32,6 +36,16 @@ bool soundItemAlreadyExists(const AudioPath& output, const QWidget& widget) { return false; } +#ifdef __RUBBERBAND__ +const QString kKeylockMultiThreadedAvailable = QObject::tr( + "

Warning!

Using multi " + "threading may result in pitch and tone imperfection, and this is " + "mono-incompatible, due to third party limitations.

"); +const QString kKeylockMultiThreadedUnavailableMono = QObject::tr( + "Multi threading mode is incompatible with mono main mix."); +const QString kKeylockMultiThreadedUnavailableRubberband = QObject::tr( + "Multi threading mode is only available with RubberBand."); +#endif } // namespace /// Construct a new sound preferences pane. Initializes and populates @@ -182,6 +196,14 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent, QOverload::of(&QComboBox::currentIndexChanged), this, &DlgPrefSound::settingChanged); +#ifdef __RUBBERBAND__ + connect(keylockMultithreadedComboBox, + &QCheckBox::clicked, + this, + &DlgPrefSound::updateKeylockMultithreading); +#else + keylockMultithreadedComboBox->hide(); +#endif connect(queryButton, &QAbstractButton::clicked, this, &DlgPrefSound::queryClicked); @@ -298,6 +320,21 @@ void DlgPrefSound::slotApply() { m_pSettings->set(ConfigKey("[Master]", "keylock_engine"), ConfigValue(static_cast(keylockEngine))); +#ifdef __RUBBERBAND__ + bool keylockMultithreading = m_pSettings->getValue( + ConfigKey(kAppGroup, "keylock_multithreading"), false); + m_pSettings->setValue(ConfigKey(kAppGroup, "keylock_multithreading"), + keylockMultithreadedComboBox->isChecked() && + keylockMultithreadedComboBox->isEnabled()); + if (keylockMultithreading != + (keylockMultithreadedComboBox->isChecked() && + keylockMultithreadedComboBox->isEnabled())) { + QMessageBox::information(this, + tr("Information"), + tr("Mixxx must be restarted before the multi-threaded " + "RubberBand settings change will take effect.")); + } +#endif status = m_pSoundManager->setConfig(m_config); } if (status != SoundDeviceStatus::Ok) { @@ -488,6 +525,13 @@ void DlgPrefSound::loadSettings(const SoundManagerConfig& config) { keylockComboBox->setCurrentIndex(keylockComboBox->count() - 1); } +#ifdef __RUBBERBAND__ + // Default is no multi threading on keylock + keylockMultithreadedComboBox->setChecked(m_pSettings->getValue( + ConfigKey(kAppGroup, QStringLiteral("keylock_multithreading")), + false)); +#endif + // Collect selected I/O channel indices for all non-empty device comboboxes // in order to allow auto-selecting free channels when different devices are // selected later on, when a different device is selected for any I/O. @@ -682,6 +726,41 @@ void DlgPrefSound::settingChanged() { return; // doesn't count if we're just loading prefs } m_settingsModified = true; + +#ifdef __RUBBERBAND__ + bool supportedScaler = keylockComboBox->currentData() + .value() != + EngineBuffer::KeylockEngine::SoundTouch; + bool monoMix = mainOutputModeComboBox->currentIndex() == 1; + keylockMultithreadedComboBox->setEnabled(!monoMix && supportedScaler); + keylockMultithreadedComboBox->setToolTip(monoMix + ? kKeylockMultiThreadedUnavailableMono + : (supportedScaler + ? kKeylockMultiThreadedAvailable + : kKeylockMultiThreadedUnavailableRubberband)); +} + +void DlgPrefSound::updateKeylockMultithreading(bool enabled) { + m_settingsModified = true; + if (!enabled) { + return; + } + QMessageBox msg; + msg.setIcon(QMessageBox::Warning); + msg.setWindowTitle(tr("Are you sure?")); + msg.setText( + tr("

Using multi threading may result in pitch and tone imperfection " + "depending of the platform, leading to mono-incompatibility, " + "due to third party limitations.

Are you sure you wish " + "to proceed?

")); + QPushButton* pNoBtn = msg.addButton(tr("No"), QMessageBox::AcceptRole); + QPushButton* pYesBtn = msg.addButton( + tr("Yes, I know what I am doing"), QMessageBox::RejectRole); + msg.setDefaultButton(pNoBtn); + msg.exec(); + keylockMultithreadedComboBox->setChecked(msg.clickedButton() == pYesBtn); +#endif } void DlgPrefSound::deviceChanged() { @@ -828,7 +907,19 @@ void DlgPrefSound::mainEnabledChanged(double value) { } void DlgPrefSound::mainOutputModeComboBoxChanged(int value) { - m_pMainMonoMixdown->set((double)value); + m_pMainMonoMixdown->set(static_cast(value)); + +#ifdef __RUBBERBAND__ + bool supportedScaler = keylockComboBox->currentData() + .value() != + EngineBuffer::KeylockEngine::SoundTouch; + keylockMultithreadedComboBox->setEnabled(!value && supportedScaler); + keylockMultithreadedComboBox->setToolTip( + value ? kKeylockMultiThreadedUnavailableMono + : (supportedScaler + ? kKeylockMultiThreadedAvailable + : kKeylockMultiThreadedUnavailableRubberband)); +#endif } void DlgPrefSound::mainMonoMixdownChanged(double value) { diff --git a/src/preferences/dialog/dlgprefsound.h b/src/preferences/dialog/dlgprefsound.h index 0170c02dfb2..746e4af4581 100644 --- a/src/preferences/dialog/dlgprefsound.h +++ b/src/preferences/dialog/dlgprefsound.h @@ -71,6 +71,9 @@ class DlgPrefSound : public DlgPreferencePage, public Ui::DlgPrefSoundDlg { void deviceChanged(); void deviceChannelsChanged(); void queryClicked(); +#ifdef __RUBBERBAND__ + void updateKeylockMultithreading(bool enabled); +#endif private: void initializePaths(); diff --git a/src/preferences/dialog/dlgprefsounddlg.ui b/src/preferences/dialog/dlgprefsounddlg.ui index 117d1339532..93a95111074 100644 --- a/src/preferences/dialog/dlgprefsounddlg.ui +++ b/src/preferences/dialog/dlgprefsounddlg.ui @@ -89,9 +89,6 @@ - - - @@ -221,6 +218,29 @@ + + + + + + + + + + 0 + 0 + + + + <html><head/><body><p><span style=" font-weight:600;">Warning!</span></p><p>Using multi threading may result in pitch and tone imperfaction depending of the platform, leading to mono-incompatibiltiy, due to third party limitations. </p><p><br/></p></body></html> + + + Multi-threaded + + + + + @@ -412,7 +432,6 @@ audioBufferComboBox deviceSyncComboBox engineClockComboBox - keylockComboBox mainMixComboBox mainOutputModeComboBox micMonitorModeComboBox diff --git a/src/test/enginebuffertest.cpp b/src/test/enginebuffertest.cpp index 6a0330de656..274c035adfc 100644 --- a/src/test/enginebuffertest.cpp +++ b/src/test/enginebuffertest.cpp @@ -8,7 +8,6 @@ #include #include "control/controlobject.h" -#include "engine/bufferscalers/rubberbandworkerpool.h" #include "engine/controls/ratecontrol.h" #include "mixer/basetrackplayer.h" #include "preferences/usersettings.h" @@ -424,7 +423,6 @@ TEST_F(EngineBufferE2ETest, SoundTouchReverseTest) { #ifdef __RUBBERBAND__ TEST_F(EngineBufferE2ETest, RubberbandReverseTest) { - RubberBandWorkerPool::createInstance(); // This test must not crash when changing to reverse while pitch is tweaked // Testing issue #8061 ControlObject::set(ConfigKey(kAppGroup, QStringLiteral("keylock_engine")), @@ -436,7 +434,6 @@ TEST_F(EngineBufferE2ETest, RubberbandReverseTest) { ProcessBuffer(); // Note: we cannot compare a golden buffer here, because the result depends // on the uses library version - RubberBandWorkerPool::destroy(); } #endif diff --git a/src/test/playermanagertest.cpp b/src/test/playermanagertest.cpp index 111b6fcd1df..a0f8ac2d36d 100644 --- a/src/test/playermanagertest.cpp +++ b/src/test/playermanagertest.cpp @@ -20,6 +20,9 @@ #include "test/soundsourceproviderregistration.h" #include "track/track.h" #include "util/cmdlineargs.h" +#ifdef __RUBBERBAND__ +#include "engine/bufferscalers/rubberbandworkerpool.h" +#endif namespace { @@ -95,10 +98,14 @@ class PlayerManagerTest : public MixxxDbTest, SoundSourceProviderRegistration { m_pRecordingManager.get()); m_pPlayerManager->bindToLibrary(m_pLibrary.get()); + RubberBandWorkerPool::createInstance(); } void TearDown() override { CoverArtCache::destroy(); +#ifdef __RUBBERBAND__ + RubberBandWorkerPool::destroy(); +#endif } ~PlayerManagerTest() { diff --git a/src/test/signalpathtest.h b/src/test/signalpathtest.h index 30f5a342f93..aef1e098d80 100644 --- a/src/test/signalpathtest.h +++ b/src/test/signalpathtest.h @@ -28,6 +28,9 @@ #include "util/defs.h" #include "util/sample.h" #include "util/types.h" +#ifdef __RUBBERBAND__ +#include "engine/bufferscalers/rubberbandworkerpool.h" +#endif using ::testing::Return; using ::testing::_; @@ -142,6 +145,18 @@ class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration { PlayerInfo::destroy(); } +#ifdef __RUBBERBAND__ + void SetUp() override { + RubberBandWorkerPool::createInstance(); + } +#endif + +#ifdef __RUBBERBAND__ + void TearDown() override { + RubberBandWorkerPool::destroy(); + } +#endif + void addDeck(EngineDeck* pDeck) { ControlObject::set(ConfigKey(pDeck->getGroup(), "main_mix"), 1.0); ControlObject::set(ConfigKey(pDeck->getGroup(), "rate_dir"), kDefaultRateDir); @@ -265,7 +280,8 @@ class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration { class SignalPathTest : public BaseSignalPathTest { protected: - SignalPathTest() { + void SetUp() override { + BaseSignalPathTest::SetUp(); const QString kTrackLocationTest = getTestDir().filePath(QStringLiteral("sine-30.wav")); TrackPointer pTrack(Track::newTemporary(kTrackLocationTest)); @@ -273,4 +289,8 @@ class SignalPathTest : public BaseSignalPathTest { loadTrack(m_pMixerDeck2, pTrack); loadTrack(m_pMixerDeck3, pTrack); } + + void TearDown() override { + BaseSignalPathTest::TearDown(); + } }; diff --git a/src/util/singleton.h b/src/util/singleton.h index ae4a0531003..d46628567a1 100644 --- a/src/util/singleton.h +++ b/src/util/singleton.h @@ -7,13 +7,14 @@ template class Singleton { public: - static T* createInstance() { + template + static T* createInstance(Args&&... args) { VERIFY_OR_DEBUG_ASSERT(!m_instance) { qWarning() << "Singleton class has already been created!"; return m_instance; } - m_instance = new T(); + m_instance = new T(std::forward(args)...); return m_instance; } From 05df96d3ca385dade90caf88187355178870ce71 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Fri, 24 May 2024 11:12:26 +0100 Subject: [PATCH 4/9] Use QStringLiteral on ScopedTimer --- src/engine/bufferscalers/enginebufferscalerubberband.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/engine/bufferscalers/enginebufferscalerubberband.cpp b/src/engine/bufferscalers/enginebufferscalerubberband.cpp index c6116df184b..7a93d199871 100644 --- a/src/engine/bufferscalers/enginebufferscalerubberband.cpp +++ b/src/engine/bufferscalers/enginebufferscalerubberband.cpp @@ -144,7 +144,7 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave( DEBUG_ASSERT(frames_to_read <= MAX_BUFFER_LEN); SINT received_frames; { - ScopedTimer t(u"RubberBand::retrieve"); + ScopedTimer t(QStringLiteral("RubberBand::retrieve")); received_frames = static_cast(m_rubberBand.retrieve( m_bufferPtrs.data(), frames_to_read)); } @@ -185,7 +185,7 @@ void EngineBufferScaleRubberBand::deinterleaveAndProcess( frames); { - ScopedTimer t(u"RubberBand::process"); + ScopedTimer t(QStringLiteral("RubberBand::process")); m_rubberBand.process(m_bufferPtrs.data(), frames, false); @@ -198,7 +198,7 @@ double EngineBufferScaleRubberBand::scaleBuffer( VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { return 0.0; } - ScopedTimer t(u"EngineBufferScaleRubberBand::scaleBuffer"); + ScopedTimer t(QStringLiteral("EngineBufferScaleRubberBand::scaleBuffer")); if (m_dBaseRate == 0.0 || m_dTempoRatio == 0.0) { SampleUtil::clear(pOutputBuffer, iOutputBufferSize); // No actual samples/frames have been read from the @@ -315,7 +315,7 @@ void EngineBufferScaleRubberBand::reset() { while (remaining_padding > 0) { const size_t pad_samples = std::min(remaining_padding, block_size); { - ScopedTimer t(u"RubberBand::process"); + ScopedTimer t(QStringLiteral("RubberBand::process")); m_rubberBand.process(m_bufferPtrs.data(), pad_samples, false); } From 406fc3aacbd0d5a4c7949f2b78ecd35956ed57df Mon Sep 17 00:00:00 2001 From: Antoine C Date: Fri, 31 May 2024 20:02:24 +0100 Subject: [PATCH 5/9] Few nits and using QThreadPool/QRunnable --- CMakeLists.txt | 2 +- src/engine/bufferscalers/rubberbandtask.cpp | 48 ++++++++++++++++ src/engine/bufferscalers/rubberbandtask.h | 45 +++++++++++++++ src/engine/bufferscalers/rubberbandworker.cpp | 56 ------------------- src/engine/bufferscalers/rubberbandworker.h | 49 ---------------- .../bufferscalers/rubberbandworkerpool.cpp | 34 +++-------- .../bufferscalers/rubberbandworkerpool.h | 24 ++------ .../bufferscalers/rubberbandwrapper.cpp | 28 ++++------ src/engine/bufferscalers/rubberbandwrapper.h | 8 +-- src/test/signalpathtest.h | 8 +-- 10 files changed, 126 insertions(+), 176 deletions(-) create mode 100644 src/engine/bufferscalers/rubberbandtask.cpp create mode 100644 src/engine/bufferscalers/rubberbandtask.h delete mode 100644 src/engine/bufferscalers/rubberbandworker.cpp delete mode 100644 src/engine/bufferscalers/rubberbandworker.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 6913fb3402a..bb5582838c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3246,7 +3246,7 @@ if(RUBBERBAND) src/effects/backends/builtin/pitchshifteffect.cpp src/engine/bufferscalers/enginebufferscalerubberband.cpp src/engine/bufferscalers/rubberbandwrapper.cpp - src/engine/bufferscalers/rubberbandworker.cpp + src/engine/bufferscalers/rubberbandtask.cpp src/engine/bufferscalers/rubberbandworkerpool.cpp ) endif() diff --git a/src/engine/bufferscalers/rubberbandtask.cpp b/src/engine/bufferscalers/rubberbandtask.cpp new file mode 100644 index 00000000000..76badc0a7c7 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandtask.cpp @@ -0,0 +1,48 @@ +#include "engine/bufferscalers/rubberbandtask.h" + +#include "engine/engine.h" +#include "util/assert.h" +#include "util/compatibility/qmutex.h" + +RubberBandTask::RubberBandTask( + size_t sampleRate, size_t channels, Options options) + : RubberBand::RubberBandStretcher(sampleRate, channels, options), + QRunnable(), + m_completed(false), + m_input(nullptr), + m_samples(0), + m_isFinal(false) { + setAutoDelete(false); +} + +void RubberBandTask::set(const float* const* input, + size_t samples, + bool isFinal) { + auto locker = lockMutex(&m_waitLock); + m_completed = false; + m_input = input; + m_samples = samples; + m_isFinal = isFinal; +} + +void RubberBandTask::waitReady() { + auto locker = lockMutex(&m_waitLock); + VERIFY_OR_DEBUG_ASSERT(m_input && m_samples) { + return; + }; + while (!m_completed) { + m_waitCondition.wait(&m_waitLock); + } +} + +void RubberBandTask::run() { + auto locker = lockMutex(&m_waitLock); + VERIFY_OR_DEBUG_ASSERT(!m_completed && m_input && m_samples) { + return; + }; + process(m_input, + m_samples, + m_isFinal); + m_completed = true; + m_waitCondition.wakeOne(); +} diff --git a/src/engine/bufferscalers/rubberbandtask.h b/src/engine/bufferscalers/rubberbandtask.h new file mode 100644 index 00000000000..a127fb99251 --- /dev/null +++ b/src/engine/bufferscalers/rubberbandtask.h @@ -0,0 +1,45 @@ +#pragma once + +#include + +#include +#include +#include +#include + +#include "audio/types.h" + +using RubberBand::RubberBandStretcher; + +class RubberBandTask : public RubberBandStretcher, public QRunnable { + public: + RubberBandTask(size_t sampleRate, + size_t channels, + Options options = DefaultOptions); + + /// @brief Submit a new stretching task + /// @param stretcher the RubberBand::RubberBandStretcher instance to use. + /// Must remain valid till waitReady() as returned + /// @param input The samples buffer + /// @param samples the samples count + /// @param final whether or not this is the final buffer + void set(const float* const* input, + size_t samples, + bool isFinal); + + // Wait for the current task to complete. + void waitReady(); + + void run(); + + private: + // Used to schedule the thread + QMutex m_waitLock; + QWaitCondition m_waitCondition; + // Whether or not the scheduled job as completed + bool m_completed; + + const float* const* m_input; + size_t m_samples; + bool m_isFinal; +}; diff --git a/src/engine/bufferscalers/rubberbandworker.cpp b/src/engine/bufferscalers/rubberbandworker.cpp deleted file mode 100644 index 8e6669d61d9..00000000000 --- a/src/engine/bufferscalers/rubberbandworker.cpp +++ /dev/null @@ -1,56 +0,0 @@ -#include "engine/bufferscalers/rubberbandworker.h" - -#include "engine/engine.h" -#include "moc_rubberbandworker.cpp" -#include "util/assert.h" -#include "util/compatibility/qmutex.h" - -using RubberBand::RubberBandStretcher; - -RubberBandWorker::RubberBandWorker() - : QThread(), - m_currentJob(Job{nullptr, nullptr, 0, false}), - m_completed(false) { -} - -void RubberBandWorker::schedule(RubberBand::RubberBandStretcher* stretcher, - const float* const* input, - size_t samples, - bool final) { - auto locker = lockMutex(&m_waitLock); - m_currentJob.instance = stretcher; - m_currentJob.input = input; - m_currentJob.samples = samples; - m_currentJob.final = final; - m_completed = false; - m_waitCondition.wakeOne(); -} - -void RubberBandWorker::waitReady() { - auto locker = lockMutex(&m_waitLock); - while (!m_completed) { - m_waitCondition.wait(&m_waitLock); - } -} - -void RubberBandWorker::stop() { - requestInterruption(); - m_waitCondition.wakeOne(); - wait(); -} -void RubberBandWorker::run() { - auto locker = lockMutex(&m_waitLock); - while (!isInterruptionRequested()) { - if (!m_completed && m_currentJob.instance && m_currentJob.input && m_currentJob.samples) { - m_currentJob.instance->process(m_currentJob.input, - m_currentJob.samples, - m_currentJob.final); - m_completed = true; - DEBUG_ASSERT(m_assigned.test(std::memory_order_relaxed)); - m_assigned.clear(std::memory_order_release); - m_waitCondition.wakeOne(); - } - m_waitCondition.wait(&m_waitLock); - } - quit(); -} diff --git a/src/engine/bufferscalers/rubberbandworker.h b/src/engine/bufferscalers/rubberbandworker.h deleted file mode 100644 index 392e9b70c60..00000000000 --- a/src/engine/bufferscalers/rubberbandworker.h +++ /dev/null @@ -1,49 +0,0 @@ -#pragma once - -#include - -#include -#include -#include -#include - -#include "audio/types.h" - -class RubberBandWorkerPool; -class RubberBandWorker : public QThread { - Q_OBJECT - public: - // Wait for the current job to complete. - void waitReady(); - - protected: - RubberBandWorker(); - - void run() override; - void schedule(RubberBand::RubberBandStretcher* stretcher, - const float* const* input, - size_t samples, - bool final); - void stop(); - - private: - struct Job { - RubberBand::RubberBandStretcher* instance; - const float* const* input; - size_t samples; - bool final; - }; - /// Contains the scheduled job. May be dangling if completed=true - Job m_currentJob; - // Used to schedule the thread - QMutex m_waitLock; - QWaitCondition m_waitCondition; - // Whether or not the scheduled job as completed - bool m_completed; - - // Used by RubberBandWorkerPool to manage thread availability. - // RubberBandWorker only clears the flag, once the job is completed - std::atomic_flag m_assigned; - - friend RubberBandWorkerPool; -}; diff --git a/src/engine/bufferscalers/rubberbandworkerpool.cpp b/src/engine/bufferscalers/rubberbandworkerpool.cpp index 8dea16a7600..077d4877317 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.cpp +++ b/src/engine/bufferscalers/rubberbandworkerpool.cpp @@ -2,11 +2,11 @@ #include -#include "engine/bufferscalers/rubberbandworker.h" #include "engine/engine.h" #include "util/assert.h" -RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) { +RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) + : QThreadPool() { bool multiThreadedOnStereo = pConfig && pConfig->getValue(ConfigKey(QStringLiteral("[App]"), QStringLiteral("keylock_multithreading")), @@ -16,36 +16,18 @@ RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) { : mixxx::audio::ChannelCount::stereo(); DEBUG_ASSERT(0 == mixxx::kEngineChannelCount % m_channelPerWorker); + setThreadPriority(QThread::HighPriority); + setMaxThreadCount(mixxx::kEngineChannelCount / m_channelPerWorker); + // We allocate one runner less than the total of maximum supported channel, // so the engine thread will also perform a stretching operation, instead of // waiting all workers to complete. During performance testing, this ahas // show better results - for (int w = 1; w < mixxx::kEngineChannelCount / m_channelPerWorker; w++) { - m_workers.emplace_back( - // We cannot use make_unique here because RubberBandWorker ctor - // is protected to prevent direct usage. - new RubberBandWorker); - m_workers.back()->start(QThread::HighPriority); + for (int w = 0; w < maxThreadCount(); w++) { + reserveThread(); } } RubberBandWorkerPool::~RubberBandWorkerPool() { - for (auto& member : m_workers) { - member->stop(); - } - m_workers.clear(); -} - -RubberBandWorker* RubberBandWorkerPool::submit( - RubberBand::RubberBandStretcher* stretcher, - const float* const* input, - size_t samples, - bool final) { - for (auto& member : m_workers) { - if (!member->m_assigned.test_and_set(std::memory_order_acquire)) { - member->schedule(stretcher, input, samples, final); - return member.get(); - } - } - return nullptr; + waitForDone(); } diff --git a/src/engine/bufferscalers/rubberbandworkerpool.h b/src/engine/bufferscalers/rubberbandworkerpool.h index 087be5eca3b..d6887058c91 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.h +++ b/src/engine/bufferscalers/rubberbandworkerpool.h @@ -1,34 +1,18 @@ #pragma once +#include + #include "audio/types.h" #include "preferences/usersettings.h" #include "util/singleton.h" -class RubberBandWorker; -namespace RubberBand { -class RubberBandStretcher; -} - // RubberBandWorkerPool is a global pool manager for RubberBandWorkerPool. It // allows a the Engine thread to use a pool of agnostic RubberBandWorker which // can be distributed stretching job -class RubberBandWorkerPool : public Singleton { +class RubberBandWorkerPool : public QThreadPool, public Singleton { public: ~RubberBandWorkerPool(); - /// @brief Submit a new stretching job - /// @param stretcher the RubberBand::RubberBandStretcher instance to use. - /// Must remain valid till waitReady() as returned - /// @param input The samples buffer - /// @param samples the samples count - /// @param final whether or not this is the final buffer - /// @return the worker on which the job as been schedule one, or null if - /// none available - RubberBandWorker* submit(RubberBand::RubberBandStretcher* stretcher, - const float* const* input, - size_t samples, - bool final); - const mixxx::audio::ChannelCount& channelPerWorker() const { return m_channelPerWorker; } @@ -37,7 +21,7 @@ class RubberBandWorkerPool : public Singleton { RubberBandWorkerPool(UserSettingsPointer pConfig = nullptr); private: - std::vector> m_workers; + ; mixxx::audio::ChannelCount m_channelPerWorker; friend class Singleton; diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp index c5a1a5cbe2b..f22a4e1deae 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.cpp +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -1,6 +1,5 @@ #include "engine/bufferscalers/rubberbandwrapper.h" -#include "engine/bufferscalers/rubberbandworker.h" #include "engine/bufferscalers/rubberbandworkerpool.h" #include "engine/engine.h" #include "util/assert.h" @@ -44,7 +43,7 @@ int RubberBandWrapper::available() const { for (auto& stretcher : m_pInstances) { available = qMin(available, stretcher->available()); } - return available; + return available == std::numeric_limits::max() ? 0 : available; } size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { if (m_pInstances.size() == 1) { @@ -110,23 +109,20 @@ size_t RubberBandWrapper::getStartDelay() const { return m_pInstances[0]->getLatency(); #endif } -void RubberBandWrapper::process(const float* const* input, size_t samples, bool final) { +void RubberBandWrapper::process(const float* const* input, size_t samples, bool isFinal) { if (m_pInstances.size() == 1) { - return m_pInstances[0]->process(input, samples, final); + return m_pInstances[0]->process(input, samples, isFinal); } else { RubberBandWorkerPool* pPool = RubberBandWorkerPool::instance(); - QSet workers; - for (auto& instance : m_pInstances) { - auto pWorker = pPool->submit(instance.get(), input, samples, final); - if (!pWorker) { - instance->process(input, samples, final); - } else { - workers.insert(pWorker); + for (auto& pInstance : m_pInstances) { + pInstance->set(input, samples, isFinal); + if (!pPool->tryStart(pInstance.get())) { + pInstance->run(); } input += pPool->channelPerWorker(); } - for (auto& worker : workers) { - worker->waitReady(); + for (auto& pInstance : m_pInstances) { + pInstance->waitReady(); } } } @@ -150,14 +146,14 @@ void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, qDebug() << "RubberBandWrapper::setup" << channelPerWorker; VERIFY_OR_DEBUG_ASSERT(0 == chCount % channelPerWorker) { m_pInstances.emplace_back( - std::make_unique( + std::make_unique( sampleRate, chCount, opt)); return; } for (int c = 0; c < chCount; c += channelPerWorker) { m_pInstances.emplace_back( - std::make_unique( + std::make_unique( sampleRate, channelPerWorker, opt)); } } @@ -168,5 +164,5 @@ void RubberBandWrapper::setPitchScale(double scale) { } bool RubberBandWrapper::isValid() const { - return m_pInstances.size(); + return !m_pInstances.empty(); } diff --git a/src/engine/bufferscalers/rubberbandwrapper.h b/src/engine/bufferscalers/rubberbandwrapper.h index 080f04b8681..2a2224ba459 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.h +++ b/src/engine/bufferscalers/rubberbandwrapper.h @@ -1,8 +1,7 @@ #pragma once -#include - #include "audio/types.h" +#include "engine/bufferscalers/rubberbandtask.h" /// RubberBandWrapper is a wrapper around RubberBand::RubberBandStretcher which /// allows to distribute signal stretching over multiple instance, but interface @@ -11,7 +10,7 @@ class RubberBandWrapper { public: int getEngineVersion() const; void setTimeRatio(double ratio); - size_t getSamplesRequired() const; + std::size_t getSamplesRequired() const; int available() const; size_t retrieve(float* const* output, size_t samples) const; size_t getInputIncrement() const; @@ -31,5 +30,6 @@ class RubberBandWrapper { bool isValid() const; private: - std::vector> m_pInstances; + // copy constructor of RubberBand::RubberBandStretcher is implicitly deleted. + std::vector> m_pInstances; }; diff --git a/src/test/signalpathtest.h b/src/test/signalpathtest.h index aef1e098d80..a1d4926bfdf 100644 --- a/src/test/signalpathtest.h +++ b/src/test/signalpathtest.h @@ -145,17 +145,17 @@ class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration { PlayerInfo::destroy(); } -#ifdef __RUBBERBAND__ void SetUp() override { +#ifdef __RUBBERBAND__ RubberBandWorkerPool::createInstance(); - } #endif + } -#ifdef __RUBBERBAND__ void TearDown() override { +#ifdef __RUBBERBAND__ RubberBandWorkerPool::destroy(); - } #endif + } void addDeck(EngineDeck* pDeck) { ControlObject::set(ConfigKey(pDeck->getGroup(), "main_mix"), 1.0); From cf2c5d02d17cd5eb745bbd385af4e9b51d065b31 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Mon, 3 Jun 2024 11:08:25 +0100 Subject: [PATCH 6/9] Add missing const qualifier and add checks on RB retrieve --- .../bufferscalers/rubberbandworkerpool.cpp | 8 ++------ .../bufferscalers/rubberbandworkerpool.h | 2 -- .../bufferscalers/rubberbandwrapper.cpp | 20 ++++++++++++------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/engine/bufferscalers/rubberbandworkerpool.cpp b/src/engine/bufferscalers/rubberbandworkerpool.cpp index 077d4877317..3fec48ba528 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.cpp +++ b/src/engine/bufferscalers/rubberbandworkerpool.cpp @@ -14,10 +14,10 @@ RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) m_channelPerWorker = multiThreadedOnStereo ? mixxx::audio::ChannelCount::mono() : mixxx::audio::ChannelCount::stereo(); - DEBUG_ASSERT(0 == mixxx::kEngineChannelCount % m_channelPerWorker); + DEBUG_ASSERT(mixxx::kEngineChannelCount % m_channelPerWorker == 0); setThreadPriority(QThread::HighPriority); - setMaxThreadCount(mixxx::kEngineChannelCount / m_channelPerWorker); + setMaxThreadCount(mixxx::kEngineChannelCount / m_channelPerWorker - 1); // We allocate one runner less than the total of maximum supported channel, // so the engine thread will also perform a stretching operation, instead of @@ -27,7 +27,3 @@ RubberBandWorkerPool::RubberBandWorkerPool(UserSettingsPointer pConfig) reserveThread(); } } - -RubberBandWorkerPool::~RubberBandWorkerPool() { - waitForDone(); -} diff --git a/src/engine/bufferscalers/rubberbandworkerpool.h b/src/engine/bufferscalers/rubberbandworkerpool.h index d6887058c91..46914316ad9 100644 --- a/src/engine/bufferscalers/rubberbandworkerpool.h +++ b/src/engine/bufferscalers/rubberbandworkerpool.h @@ -11,8 +11,6 @@ // can be distributed stretching job class RubberBandWorkerPool : public QThreadPool, public Singleton { public: - ~RubberBandWorkerPool(); - const mixxx::audio::ChannelCount& channelPerWorker() const { return m_channelPerWorker; } diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp index f22a4e1deae..72ac375e894 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.cpp +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -33,14 +33,14 @@ void RubberBandWrapper::setTimeRatio(double ratio) { } size_t RubberBandWrapper::getSamplesRequired() const { size_t require = 0; - for (auto& stretcher : m_pInstances) { + for (const auto& stretcher : m_pInstances) { require = qMax(require, stretcher->getSamplesRequired()); } return require; } int RubberBandWrapper::available() const { int available = std::numeric_limits::max(); - for (auto& stretcher : m_pInstances) { + for (const auto& stretcher : m_pInstances) { available = qMin(available, stretcher->available()); } return available == std::numeric_limits::max() ? 0 : available; @@ -49,14 +49,19 @@ size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { if (m_pInstances.size() == 1) { return m_pInstances[0]->retrieve(output, samples); } else { - size_t ret = 0; + // ensure we don't fetch more samples than we really have available. + samples = std::min(static_cast(available()), samples); for (const auto& stretcher : m_pInstances) { - size_t thisRet = stretcher->retrieve(output, samples); - DEBUG_ASSERT(!ret || thisRet == ret); - ret = qMax(thisRet, ret); +#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED + size_t numSamplesRequested = +#endif + stretcher->retrieve(output, samples); + // there is something very wrong if we got a different of amount + // of samples than we requested + DEBUG_ASSERT(numSamplesRequested == samples); output += getChannelPerWorker(); } - return ret; + return samples; } } size_t RubberBandWrapper::getInputIncrement() const { @@ -151,6 +156,7 @@ void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, return; } + m_pInstances.reserve(chCount / channelPerWorker); for (int c = 0; c < chCount; c += channelPerWorker) { m_pInstances.emplace_back( std::make_unique( From 5996927e72adfc9b4adb2a832413c686eb20df00 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Tue, 4 Jun 2024 09:39:51 +0100 Subject: [PATCH 7/9] Replace the waitcond and mutex with sema on RBTask --- src/engine/bufferscalers/rubberbandtask.cpp | 16 +++++----------- src/engine/bufferscalers/rubberbandtask.h | 8 ++------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/engine/bufferscalers/rubberbandtask.cpp b/src/engine/bufferscalers/rubberbandtask.cpp index 76badc0a7c7..f76bf720b70 100644 --- a/src/engine/bufferscalers/rubberbandtask.cpp +++ b/src/engine/bufferscalers/rubberbandtask.cpp @@ -8,7 +8,7 @@ RubberBandTask::RubberBandTask( size_t sampleRate, size_t channels, Options options) : RubberBand::RubberBandStretcher(sampleRate, channels, options), QRunnable(), - m_completed(false), + m_completedSema(0), m_input(nullptr), m_samples(0), m_isFinal(false) { @@ -18,31 +18,25 @@ RubberBandTask::RubberBandTask( void RubberBandTask::set(const float* const* input, size_t samples, bool isFinal) { - auto locker = lockMutex(&m_waitLock); - m_completed = false; + DEBUG_ASSERT(m_completedSema.available() == 0); m_input = input; m_samples = samples; m_isFinal = isFinal; } void RubberBandTask::waitReady() { - auto locker = lockMutex(&m_waitLock); VERIFY_OR_DEBUG_ASSERT(m_input && m_samples) { return; }; - while (!m_completed) { - m_waitCondition.wait(&m_waitLock); - } + m_completedSema.acquire(); } void RubberBandTask::run() { - auto locker = lockMutex(&m_waitLock); - VERIFY_OR_DEBUG_ASSERT(!m_completed && m_input && m_samples) { + VERIFY_OR_DEBUG_ASSERT(m_completedSema.available() == 0 && m_input && m_samples) { return; }; process(m_input, m_samples, m_isFinal); - m_completed = true; - m_waitCondition.wakeOne(); + m_completedSema.release(); } diff --git a/src/engine/bufferscalers/rubberbandtask.h b/src/engine/bufferscalers/rubberbandtask.h index a127fb99251..2b146794dcf 100644 --- a/src/engine/bufferscalers/rubberbandtask.h +++ b/src/engine/bufferscalers/rubberbandtask.h @@ -2,9 +2,8 @@ #include -#include #include -#include +#include #include #include "audio/types.h" @@ -33,11 +32,8 @@ class RubberBandTask : public RubberBandStretcher, public QRunnable { void run(); private: - // Used to schedule the thread - QMutex m_waitLock; - QWaitCondition m_waitCondition; // Whether or not the scheduled job as completed - bool m_completed; + QSemaphore m_completedSema; const float* const* m_input; size_t m_samples; From b49544bc1ef60d6b62c017cf7e55acf771b93290 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Tue, 4 Jun 2024 10:00:26 +0100 Subject: [PATCH 8/9] Add better comments and various nits --- .../enginebufferscalerubberband.cpp | 5 +-- .../bufferscalers/rubberbandwrapper.cpp | 45 ++++++++++++++----- src/engine/bufferscalers/rubberbandwrapper.h | 2 +- src/preferences/dialog/dlgprefsound.cpp | 25 +++++++---- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/src/engine/bufferscalers/enginebufferscalerubberband.cpp b/src/engine/bufferscalers/enginebufferscalerubberband.cpp index 7a93d199871..ef8c758df04 100644 --- a/src/engine/bufferscalers/enginebufferscalerubberband.cpp +++ b/src/engine/bufferscalers/enginebufferscalerubberband.cpp @@ -136,17 +136,14 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave( VERIFY_OR_DEBUG_ASSERT(m_rubberBand.isValid()) { return 0; } - const SINT frames_available = m_rubberBand.available(); // NOTE: If we still need to throw away padding, then we can also // immediately read those frames in addition to the frames we actually // need for the output - const SINT frames_to_read = math_min(frames_available, frames + m_remainingPaddingInOutput); - DEBUG_ASSERT(frames_to_read <= MAX_BUFFER_LEN); SINT received_frames; { ScopedTimer t(QStringLiteral("RubberBand::retrieve")); received_frames = static_cast(m_rubberBand.retrieve( - m_bufferPtrs.data(), frames_to_read)); + m_bufferPtrs.data(), frames + m_remainingPaddingInOutput, m_buffers[0].size())); } SINT frame_offset = 0; diff --git a/src/engine/bufferscalers/rubberbandwrapper.cpp b/src/engine/bufferscalers/rubberbandwrapper.cpp index 72ac375e894..ed7f7bc562f 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.cpp +++ b/src/engine/bufferscalers/rubberbandwrapper.cpp @@ -3,6 +3,7 @@ #include "engine/bufferscalers/rubberbandworkerpool.h" #include "engine/engine.h" #include "util/assert.h" +#include "util/sample.h" using RubberBand::RubberBandStretcher; @@ -45,20 +46,31 @@ int RubberBandWrapper::available() const { } return available == std::numeric_limits::max() ? 0 : available; } -size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { +size_t RubberBandWrapper::retrieve( + float* const* output, size_t samples, SINT channelBufferSize) const { + // ensure we don't fetch more samples than we really have available. + samples = std::min(static_cast(available()), samples); + VERIFY_OR_DEBUG_ASSERT(samples <= static_cast(channelBufferSize)) { + samples = channelBufferSize; + } if (m_pInstances.size() == 1) { return m_pInstances[0]->retrieve(output, samples); } else { - // ensure we don't fetch more samples than we really have available. - samples = std::min(static_cast(available()), samples); for (const auto& stretcher : m_pInstances) { -#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED - size_t numSamplesRequested = -#endif + size_t numSamplesRetrieved = stretcher->retrieve(output, samples); // there is something very wrong if we got a different of amount // of samples than we requested - DEBUG_ASSERT(numSamplesRequested == samples); + // We clear the buffer to limit the damage, but the signal + // interruption will still create undesirable audio artefacts. + VERIFY_OR_DEBUG_ASSERT(numSamplesRetrieved == samples) { + if (samples > numSamplesRetrieved) { + for (int ch = 0; ch < getChannelPerWorker(); ch++) { + SampleUtil::clear(output[ch] + numSamplesRetrieved, + samples - numSamplesRetrieved); + } + } + } output += getChannelPerWorker(); } return samples; @@ -66,19 +78,19 @@ size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const { } size_t RubberBandWrapper::getInputIncrement() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + return {}; } return m_pInstances[0]->getInputIncrement(); } size_t RubberBandWrapper::getLatency() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + return {}; } return m_pInstances[0]->getLatency(); } double RubberBandWrapper::getPitchScale() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + return {}; } return m_pInstances[0]->getPitchScale(); } @@ -87,7 +99,7 @@ double RubberBandWrapper::getPitchScale() const { // for how these two functions were implemented within librubberband itself size_t RubberBandWrapper::getPreferredStartPad() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + return {}; } #if RUBBERBANDV3 return m_pInstances[0]->getPreferredStartPad(); @@ -101,7 +113,7 @@ size_t RubberBandWrapper::getPreferredStartPad() const { } size_t RubberBandWrapper::getStartDelay() const { VERIFY_OR_DEBUG_ASSERT(isValid()) { - return -1; + return {}; } #if RUBBERBANDV3 return m_pInstances[0]->getStartDelay(); @@ -121,11 +133,16 @@ void RubberBandWrapper::process(const float* const* input, size_t samples, bool RubberBandWorkerPool* pPool = RubberBandWorkerPool::instance(); for (auto& pInstance : m_pInstances) { pInstance->set(input, samples, isFinal); + // We try to get the stretching job ran by the RBPool if there is a + // worker slot available if (!pPool->tryStart(pInstance.get())) { + // Otherwise, it means the main thread should take care of the stretching pInstance->run(); } input += pPool->channelPerWorker(); } + // We always perform a wait, even for task that were ran in the main + // thread, so it resets the semaphore for (auto& pInstance : m_pInstances) { pInstance->waitReady(); } @@ -150,6 +167,10 @@ void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate, auto channelPerWorker = getChannelPerWorker(); qDebug() << "RubberBandWrapper::setup" << channelPerWorker; VERIFY_OR_DEBUG_ASSERT(0 == chCount % channelPerWorker) { + // If we have an uneven number of channel, which we can't evenly + // distribute across the RubberBandPool workers, we fallback to using a + // single instance to limit the audio impefection that may come from + // using RB withg different parameters. m_pInstances.emplace_back( std::make_unique( sampleRate, chCount, opt)); diff --git a/src/engine/bufferscalers/rubberbandwrapper.h b/src/engine/bufferscalers/rubberbandwrapper.h index 2a2224ba459..610c1d2caa3 100644 --- a/src/engine/bufferscalers/rubberbandwrapper.h +++ b/src/engine/bufferscalers/rubberbandwrapper.h @@ -12,7 +12,7 @@ class RubberBandWrapper { void setTimeRatio(double ratio); std::size_t getSamplesRequired() const; int available() const; - size_t retrieve(float* const* output, size_t samples) const; + size_t retrieve(float* const* output, size_t samples, SINT channelBufferSize) const; size_t getInputIncrement() const; size_t getLatency() const; double getPitchScale() const; diff --git a/src/preferences/dialog/dlgprefsound.cpp b/src/preferences/dialog/dlgprefsound.cpp index 8b452aec01a..55ba9149045 100644 --- a/src/preferences/dialog/dlgprefsound.cpp +++ b/src/preferences/dialog/dlgprefsound.cpp @@ -37,14 +37,23 @@ bool soundItemAlreadyExists(const AudioPath& output, const QWidget& widget) { } #ifdef __RUBBERBAND__ -const QString kKeylockMultiThreadedAvailable = QObject::tr( - "

Warning!

Using multi " - "threading may result in pitch and tone imperfection, and this is " - "mono-incompatible, due to third party limitations.

"); -const QString kKeylockMultiThreadedUnavailableMono = QObject::tr( - "Multi threading mode is incompatible with mono main mix."); -const QString kKeylockMultiThreadedUnavailableRubberband = QObject::tr( - "Multi threading mode is only available with RubberBand."); +const QString kKeylockMultiThreadedAvailable = + QStringLiteral("

") + + QObject::tr("Warning!") + QStringLiteral("

") + + QObject::tr( + "Using multi " + "threading may result in pitch and tone imperfection, and this " + "is " + "mono-incompatible, due to third party limitations.") + + QStringLiteral("

"); +const QString kKeylockMultiThreadedUnavailableMono = QStringLiteral("") + + QObject::tr( + "Multi threading mode is incompatible with mono main mix.") + + QStringLiteral(""); +const QString kKeylockMultiThreadedUnavailableRubberband = + QStringLiteral("") + + QObject::tr("Multi threading mode is only available with RubberBand.") + + QStringLiteral(""); #endif } // namespace From 0fc61701a0900e4f87a5e988849e9e76723d682d Mon Sep 17 00:00:00 2001 From: Antoine C Date: Thu, 6 Jun 2024 10:11:03 +0100 Subject: [PATCH 9/9] Fix incorrect checkbox name and reword warning message --- src/preferences/dialog/dlgprefsound.cpp | 38 +++++++++++------------ src/preferences/dialog/dlgprefsounddlg.ui | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/preferences/dialog/dlgprefsound.cpp b/src/preferences/dialog/dlgprefsound.cpp index 55ba9149045..b199f63bc7f 100644 --- a/src/preferences/dialog/dlgprefsound.cpp +++ b/src/preferences/dialog/dlgprefsound.cpp @@ -206,12 +206,12 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent, this, &DlgPrefSound::settingChanged); #ifdef __RUBBERBAND__ - connect(keylockMultithreadedComboBox, + connect(keylockMultithreadedCheckBox, &QCheckBox::clicked, this, &DlgPrefSound::updateKeylockMultithreading); #else - keylockMultithreadedComboBox->hide(); + keylockMultithreadedCheckBox->hide(); #endif connect(queryButton, &QAbstractButton::clicked, this, &DlgPrefSound::queryClicked); @@ -333,15 +333,15 @@ void DlgPrefSound::slotApply() { bool keylockMultithreading = m_pSettings->getValue( ConfigKey(kAppGroup, "keylock_multithreading"), false); m_pSettings->setValue(ConfigKey(kAppGroup, "keylock_multithreading"), - keylockMultithreadedComboBox->isChecked() && - keylockMultithreadedComboBox->isEnabled()); + keylockMultithreadedCheckBox->isChecked() && + keylockMultithreadedCheckBox->isEnabled()); if (keylockMultithreading != - (keylockMultithreadedComboBox->isChecked() && - keylockMultithreadedComboBox->isEnabled())) { + (keylockMultithreadedCheckBox->isChecked() && + keylockMultithreadedCheckBox->isEnabled())) { QMessageBox::information(this, tr("Information"), tr("Mixxx must be restarted before the multi-threaded " - "RubberBand settings change will take effect.")); + "RubberBand setting change will take effect.")); } #endif status = m_pSoundManager->setConfig(m_config); @@ -536,7 +536,7 @@ void DlgPrefSound::loadSettings(const SoundManagerConfig& config) { #ifdef __RUBBERBAND__ // Default is no multi threading on keylock - keylockMultithreadedComboBox->setChecked(m_pSettings->getValue( + keylockMultithreadedCheckBox->setChecked(m_pSettings->getValue( ConfigKey(kAppGroup, QStringLiteral("keylock_multithreading")), false)); #endif @@ -741,8 +741,8 @@ void DlgPrefSound::settingChanged() { .value() != EngineBuffer::KeylockEngine::SoundTouch; bool monoMix = mainOutputModeComboBox->currentIndex() == 1; - keylockMultithreadedComboBox->setEnabled(!monoMix && supportedScaler); - keylockMultithreadedComboBox->setToolTip(monoMix + keylockMultithreadedCheckBox->setEnabled(!monoMix && supportedScaler); + keylockMultithreadedCheckBox->setToolTip(monoMix ? kKeylockMultiThreadedUnavailableMono : (supportedScaler ? kKeylockMultiThreadedAvailable @@ -757,18 +757,18 @@ void DlgPrefSound::updateKeylockMultithreading(bool enabled) { QMessageBox msg; msg.setIcon(QMessageBox::Warning); msg.setWindowTitle(tr("Are you sure?")); - msg.setText( - tr("

Using multi threading may result in pitch and tone imperfection " - "depending of the platform, leading to mono-incompatibility, " - "due to third party limitations.

Are you sure you wish " - "to proceed?

")); + msg.setText(QStringLiteral("

%1

%2

") + .arg(tr("Using multi threading result in a loss of " + "mono compatibility and a diffuse stereo " + "image. It is not recommended during " + "broadcasting or recording."), + tr("Are you sure you wish to proceed?"))); QPushButton* pNoBtn = msg.addButton(tr("No"), QMessageBox::AcceptRole); QPushButton* pYesBtn = msg.addButton( tr("Yes, I know what I am doing"), QMessageBox::RejectRole); msg.setDefaultButton(pNoBtn); msg.exec(); - keylockMultithreadedComboBox->setChecked(msg.clickedButton() == pYesBtn); + keylockMultithreadedCheckBox->setChecked(msg.clickedButton() == pYesBtn); #endif } @@ -922,8 +922,8 @@ void DlgPrefSound::mainOutputModeComboBoxChanged(int value) { bool supportedScaler = keylockComboBox->currentData() .value() != EngineBuffer::KeylockEngine::SoundTouch; - keylockMultithreadedComboBox->setEnabled(!value && supportedScaler); - keylockMultithreadedComboBox->setToolTip( + keylockMultithreadedCheckBox->setEnabled(!value && supportedScaler); + keylockMultithreadedCheckBox->setToolTip( value ? kKeylockMultiThreadedUnavailableMono : (supportedScaler ? kKeylockMultiThreadedAvailable diff --git a/src/preferences/dialog/dlgprefsounddlg.ui b/src/preferences/dialog/dlgprefsounddlg.ui index 93a95111074..ca0bebbd380 100644 --- a/src/preferences/dialog/dlgprefsounddlg.ui +++ b/src/preferences/dialog/dlgprefsounddlg.ui @@ -224,7 +224,7 @@
- + 0