From 9d7b053ba6d571a7a5e45be131a0486d1f4b4000 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:54:30 +0200 Subject: [PATCH 1/5] refactor: use higher-level `std::span` based logic --- .../backends/builtin/metronomeeffect.cpp | 125 ++++++++++-------- .../backends/builtin/metronomeeffect.h | 6 +- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/src/effects/backends/builtin/metronomeeffect.cpp b/src/effects/backends/builtin/metronomeeffect.cpp index 934bfc6be0c..2599fee60fc 100644 --- a/src/effects/backends/builtin/metronomeeffect.cpp +++ b/src/effects/backends/builtin/metronomeeffect.cpp @@ -1,22 +1,61 @@ #include "metronomeeffect.h" +#include +#include +#include +#include + +#include "audio/types.h" #include "effects/backends/effectmanifest.h" #include "engine/effects/engineeffectparameter.h" +#include "engine/engine.h" #include "metronomeclick.h" -#include "util/math.h" #include "util/sample.h" +#include "util/types.h" namespace { -void playMonoSamples(std::span monoSource, std::span output) { - const auto outputBufferFrames = output.size() / mixxx::kEngineChannelOutputCount; - SINT framesPlayed = std::min(monoSource.size(), outputBufferFrames); +std::size_t playMonoSamples(std::span monoSource, std::span output) { + const std::size_t outputBufferFrames = output.size() / mixxx::kEngineChannelOutputCount; + std::size_t framesPlayed = std::min(monoSource.size(), outputBufferFrames); SampleUtil::addMonoToStereo(output.data(), monoSource.data(), framesPlayed); + return framesPlayed; } -double framesPerBeat(mixxx::audio::SampleRate sampleRate, double bpm) { - double framesPerMinute = sampleRate * 60; - return framesPerMinute / bpm; +template +std::span subspan_clamped(std::span in, typename std::span::size_type offset) { + // TODO (Swiftb0y): should we instead create a wrapper type that implements + // UB-free "clamped" operations? + return in.subspan(std::min(offset, in.size())); +} + +constexpr std::size_t framesPerBeat( + mixxx::audio::SampleRate framesPerSecond, double beatsPerMinute) { + double framesPerMinute = framesPerSecond * 60; + return static_cast(framesPerMinute / beatsPerMinute); +} +// returns where in the output buffer to start playing the next click. +// If there the span is empty, no click should be played yet. +std::span syncedClickOutput(double beatFractionBufferEnd, + std::optional beatLengthAndScratch, + std::span output) { + if (!beatLengthAndScratch.has_value() || beatLengthAndScratch->scratch_rate == 0.0) { + return {}; + } + double beatLength = beatLengthAndScratch->frames / beatLengthAndScratch->scratch_rate; + + const bool needsPreviousBeat = beatLength < 0; + double beatToBufferEndFrames = std::abs(beatLength) * + (needsPreviousBeat ? (1 - beatFractionBufferEnd) + : beatFractionBufferEnd); + std::size_t beatToBufferEndSamples = + static_cast(beatToBufferEndFrames) * + mixxx::kEngineChannelOutputCount; + + if (beatToBufferEndSamples <= output.size()) { + return output.last(beatToBufferEndSamples); + } + return {}; } } // namespace @@ -83,73 +122,43 @@ void MetronomeEffect::processChannel( MetronomeGroupState* gs = pGroupState; const std::span click = clickForSampleRate(engineParameters.sampleRate()); - SINT clickSize = click.size(); if (pOutput != pInput) { SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer()); } const bool shouldSync = m_pSyncParameter->toBool(); + const bool hasBeatInfo = groupFeatures.beat_length.has_value() && + groupFeatures.beat_fraction_buffer_end.has_value(); if (enableState == EffectEnableState::Enabling) { - if (shouldSync && groupFeatures.beat_fraction_buffer_end.has_value()) { + if (shouldSync && hasBeatInfo) { // Skip first click and sync phase - gs->m_framesSinceClickStart = clickSize; + gs->framesSinceLastClick = click.size(); } else { - // click right away after enabling - gs->m_framesSinceClickStart = 0; + gs->framesSinceLastClick = 0; } } - if (gs->m_framesSinceClickStart < clickSize) { - // In click region, write remaining click frames. - playMonoSamples(click.subspan(gs->m_framesSinceClickStart), output); - } + playMonoSamples(subspan_clamped(click, gs->framesSinceLastClick), output); + gs->framesSinceLastClick += engineParameters.framesPerBuffer(); - double bufferEnd = gs->m_framesSinceClickStart + engineParameters.framesPerBuffer(); - - double nextClickStart = bufferEnd; // default to "no new click"; - if (shouldSync && groupFeatures.beat_fraction_buffer_end.has_value()) { - // Sync enabled and have a track with beats - if (groupFeatures.beat_length.has_value() && - groupFeatures.beat_length->scratch_rate != 0.0) { - double beatLength = groupFeatures.beat_length->frames / - groupFeatures.beat_length->scratch_rate; - double beatToBufferEnd; - if (beatLength > 0) { - beatToBufferEnd = - beatLength * - *groupFeatures.beat_fraction_buffer_end; - } else { - beatToBufferEnd = - beatLength * -1 * - (1 - *groupFeatures.beat_fraction_buffer_end); - } - - if (bufferEnd > beatToBufferEnd) { - // We have a new beat before the current buffer ends - nextClickStart = bufferEnd - beatToBufferEnd; - } + std::span outputBufferOffset = [&] { + if (shouldSync && hasBeatInfo) { + return syncedClickOutput(*groupFeatures.beat_fraction_buffer_end, + groupFeatures.beat_length, + output); } else { - // no transport, continue until the current click has been fully played - if (gs->m_framesSinceClickStart < clickSize) { - gs->m_framesSinceClickStart += engineParameters.framesPerBuffer(); - } - return; + // EngineParameters::sampleRate in reality returns the framerate. + mixxx::audio::SampleRate framesPerSecond = engineParameters.sampleRate(); + std::size_t offset = gs->framesSinceLastClick % + framesPerBeat(framesPerSecond, + m_pBpmParameter->value()); + return subspan_clamped(output, offset * mixxx::kEngineChannelOutputCount); } - } else { - nextClickStart = framesPerBeat(engineParameters.sampleRate(), m_pBpmParameter->value()); - } + }(); - if (bufferEnd > nextClickStart) { - // We need to start a new click - SINT outputOffset = static_cast(nextClickStart) - gs->m_framesSinceClickStart; - if (outputOffset > 0 && outputOffset < engineParameters.framesPerBuffer()) { - playMonoSamples(click, output.subspan(outputOffset * 2)); - } - // Due to seeking, we may have missed the start position of the click. - // We pretend that it has been played to stay in phase - gs->m_framesSinceClickStart = -outputOffset; + if (!outputBufferOffset.empty()) { + gs->framesSinceLastClick = playMonoSamples(click, outputBufferOffset); } - gs->m_framesSinceClickStart += engineParameters.framesPerBuffer(); } diff --git a/src/effects/backends/builtin/metronomeeffect.h b/src/effects/backends/builtin/metronomeeffect.h index e3c81c7bfff..68a5dee987f 100644 --- a/src/effects/backends/builtin/metronomeeffect.h +++ b/src/effects/backends/builtin/metronomeeffect.h @@ -9,12 +9,10 @@ class MetronomeGroupState final : public EffectState { public: MetronomeGroupState(const mixxx::EngineParameters& engineParameters) - : EffectState(engineParameters), - m_framesSinceClickStart(0) { - } + : EffectState(engineParameters){}; ~MetronomeGroupState() override = default; - SINT m_framesSinceClickStart; + std::size_t framesSinceLastClick = 0; }; class MetronomeEffect : public EffectProcessorImpl { From 736034cb35bef0b4f92e1f50d9985e1e20ab4f10 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 4 Oct 2024 17:55:01 +0200 Subject: [PATCH 2/5] feat: add SampleUtil::addMonoToStereoWithGain --- src/util/sample.cpp | 20 +++++++++++++++++--- src/util/sample.h | 9 +++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/util/sample.cpp b/src/util/sample.cpp index f11768a3196..692ba384937 100644 --- a/src/util/sample.cpp +++ b/src/util/sample.cpp @@ -842,17 +842,31 @@ void SampleUtil::copyMonoToDualMono(CSAMPLE* M_RESTRICT pDest, } // static -void SampleUtil::addMonoToStereo(CSAMPLE* M_RESTRICT pDest, - const CSAMPLE* M_RESTRICT pSrc, SINT numFrames) { +void SampleUtil::addMonoToStereoWithGain(CSAMPLE_GAIN gain, + CSAMPLE* M_RESTRICT pDest, + const CSAMPLE* M_RESTRICT pSrc, + SINT numFrames) { + if (gain == 0.0) { + // no need to add silence + return; + } // forward loop // note: LOOP VECTORIZED. for (SINT i = 0; i < numFrames; ++i) { - const CSAMPLE s = pSrc[i]; + const CSAMPLE s = pSrc[i] * gain; pDest[i * 2] += s; pDest[i * 2 + 1] += s; } } +// static +void SampleUtil::addMonoToStereo(CSAMPLE* M_RESTRICT pDest, + const CSAMPLE* M_RESTRICT pSrc, + SINT numFrames) { + // lets hope the compiler inlines here and optimizes the multiplication away + return addMonoToStereoWithGain(1, pDest, pSrc, numFrames); +} + // static void SampleUtil::stripMultiToStereo( CSAMPLE* pBuffer, diff --git a/src/util/sample.h b/src/util/sample.h index a4ca617a6c9..12fe8032e52 100644 --- a/src/util/sample.h +++ b/src/util/sample.h @@ -347,6 +347,15 @@ class SampleUtil { static void copyMonoToDualMono(CSAMPLE* pDest, const CSAMPLE* pSrc, SINT numFrames); + // Scales, adds and doubles the mono samples in pSrc to dual mono samples + // to pDest + // (numFrames) samples will be read from pSrc + // (numFrames * 2) samples will be added to pDest + static void addMonoToStereoWithGain(CSAMPLE_GAIN gain, + CSAMPLE* pDest, + const CSAMPLE* pSrc, + SINT numFrames); + // Adds and doubles the mono samples in pSrc to dual mono samples // to pDest. // (numFrames) samples will be read from pSrc From ce990e6f0a9d1bcebfc3fbdfc55e292147a9ba23 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 4 Oct 2024 17:57:45 +0200 Subject: [PATCH 3/5] refactor: wrap literals in QStringLiteral as appropriate --- src/effects/backends/builtin/metronomeeffect.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/effects/backends/builtin/metronomeeffect.cpp b/src/effects/backends/builtin/metronomeeffect.cpp index 2599fee60fc..c095f7be6df 100644 --- a/src/effects/backends/builtin/metronomeeffect.cpp +++ b/src/effects/backends/builtin/metronomeeffect.cpp @@ -62,16 +62,16 @@ std::span syncedClickOutput(double beatFractionBufferEnd, // static QString MetronomeEffect::getId() { - return "org.mixxx.effects.metronome"; + return QStringLiteral("org.mixxx.effects.metronome"); } // static EffectManifestPointer MetronomeEffect::getManifest() { - EffectManifestPointer pManifest(new EffectManifest()); + auto pManifest = EffectManifestPointer::create(); pManifest->setId(getId()); pManifest->setName(QObject::tr("Metronome")); - pManifest->setAuthor("The Mixxx Team"); - pManifest->setVersion("1.0"); + pManifest->setAuthor(QObject::tr("The Mixxx Team")); + pManifest->setVersion(QStringLiteral("1.0")); pManifest->setDescription(QObject::tr("Adds a metronome click sound to the stream")); pManifest->setEffectRampsFromDry(true); @@ -79,7 +79,7 @@ EffectManifestPointer MetronomeEffect::getManifest() { // The maximum is at 128 + 1 allowing 128 as max value and // enabling us to pause time when the parameter is above EffectManifestParameterPointer period = pManifest->addParameter(); - period->setId("bpm"); + period->setId(QStringLiteral("bpm")); period->setName(QObject::tr("BPM")); period->setDescription(QObject::tr("Set the beats per minute value of the click sound")); period->setValueScaler(EffectManifestParameter::ValueScaler::Logarithmic); @@ -88,7 +88,7 @@ EffectManifestPointer MetronomeEffect::getManifest() { // Period unit EffectManifestParameterPointer periodUnit = pManifest->addParameter(); - periodUnit->setId("sync"); + periodUnit->setId(QStringLiteral("sync")); periodUnit->setName(QObject::tr("Sync")); periodUnit->setDescription(QObject::tr( "Synchronizes the BPM with the track if it can be retrieved")); @@ -101,8 +101,8 @@ EffectManifestPointer MetronomeEffect::getManifest() { void MetronomeEffect::loadEngineEffectParameters( const QMap& parameters) { - m_pBpmParameter = parameters.value("bpm"); - m_pSyncParameter = parameters.value("sync"); + m_pBpmParameter = parameters.value(QStringLiteral("bpm")); + m_pSyncParameter = parameters.value(QStringLiteral("sync")); } void MetronomeEffect::processChannel( From ecababe538da925d10dc2e3f3d7965d0f24b185f Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 4 Oct 2024 18:03:32 +0200 Subject: [PATCH 4/5] feat: add linked gain knob to metronome effect --- .../backends/builtin/metronomeeffect.cpp | 28 ++++++++++++++++--- .../backends/builtin/metronomeeffect.h | 1 + 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/effects/backends/builtin/metronomeeffect.cpp b/src/effects/backends/builtin/metronomeeffect.cpp index c095f7be6df..7f73821ca92 100644 --- a/src/effects/backends/builtin/metronomeeffect.cpp +++ b/src/effects/backends/builtin/metronomeeffect.cpp @@ -7,18 +7,22 @@ #include "audio/types.h" #include "effects/backends/effectmanifest.h" +#include "effects/backends/effectmanifestparameter.h" #include "engine/effects/engineeffectparameter.h" #include "engine/engine.h" #include "metronomeclick.h" +#include "util/math.h" #include "util/sample.h" #include "util/types.h" namespace { -std::size_t playMonoSamples(std::span monoSource, std::span output) { +std::size_t playMonoSamplesWithGain(std::span monoSource, + std::span output, + CSAMPLE_GAIN gain) { const std::size_t outputBufferFrames = output.size() / mixxx::kEngineChannelOutputCount; std::size_t framesPlayed = std::min(monoSource.size(), outputBufferFrames); - SampleUtil::addMonoToStereo(output.data(), monoSource.data(), framesPlayed); + SampleUtil::addMonoToStereoWithGain(gain, output.data(), monoSource.data(), framesPlayed); return framesPlayed; } @@ -96,6 +100,19 @@ EffectManifestPointer MetronomeEffect::getManifest() { periodUnit->setUnitsHint(EffectManifestParameter::UnitsHint::Unknown); periodUnit->setRange(0, 1, 1); + EffectManifestParameterPointer gain = pManifest->addParameter(); + gain->setId(QStringLiteral("gain")); + gain->setName(QObject::tr("Gain")); + gain->setDescription(QObject::tr( + "Set the gain of metronome click sound")); + gain->setValueScaler(EffectManifestParameter::ValueScaler::Linear); + gain->setUnitsHint(EffectManifestParameter::UnitsHint::Decibel); + gain->setDefaultLinkType(EffectManifestParameter::LinkType::Linked); + gain->setRange(-24.0, 0.0, 3.0); // decibel + // 0db on the range above, assumes scale is linear default=(max-min)x+min (solve for x) + // TODO: move this generally to ControlPotmeterBehavior? + gain->setNeutralPointOnScale(24.0 / 27.0); + return pManifest; } @@ -103,6 +120,7 @@ void MetronomeEffect::loadEngineEffectParameters( const QMap& parameters) { m_pBpmParameter = parameters.value(QStringLiteral("bpm")); m_pSyncParameter = parameters.value(QStringLiteral("sync")); + m_pGainParameter = parameters.value(QStringLiteral("gain")); } void MetronomeEffect::processChannel( @@ -140,7 +158,9 @@ void MetronomeEffect::processChannel( } } - playMonoSamples(subspan_clamped(click, gs->framesSinceLastClick), output); + const CSAMPLE_GAIN gain = db2ratio(static_cast(m_pGainParameter->value())); + + playMonoSamplesWithGain(subspan_clamped(click, gs->framesSinceLastClick), output, gain); gs->framesSinceLastClick += engineParameters.framesPerBuffer(); std::span outputBufferOffset = [&] { @@ -159,6 +179,6 @@ void MetronomeEffect::processChannel( }(); if (!outputBufferOffset.empty()) { - gs->framesSinceLastClick = playMonoSamples(click, outputBufferOffset); + gs->framesSinceLastClick = playMonoSamplesWithGain(click, outputBufferOffset, gain); } } diff --git a/src/effects/backends/builtin/metronomeeffect.h b/src/effects/backends/builtin/metronomeeffect.h index 68a5dee987f..d89e1934035 100644 --- a/src/effects/backends/builtin/metronomeeffect.h +++ b/src/effects/backends/builtin/metronomeeffect.h @@ -37,6 +37,7 @@ class MetronomeEffect : public EffectProcessorImpl { private: EngineEffectParameterPointer m_pBpmParameter; EngineEffectParameterPointer m_pSyncParameter; + EngineEffectParameterPointer m_pGainParameter; DISALLOW_COPY_AND_ASSIGN(MetronomeEffect); }; From a092e8f6867d71b5af60ff40f2238385813a3d0d Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:29:01 +0100 Subject: [PATCH 5/5] temp: incorporate style feedback --- .../backends/builtin/metronomeeffect.cpp | 35 +++++++++++-------- .../backends/builtin/metronomeeffect.h | 2 +- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/effects/backends/builtin/metronomeeffect.cpp b/src/effects/backends/builtin/metronomeeffect.cpp index 7f73821ca92..63db6b2cb08 100644 --- a/src/effects/backends/builtin/metronomeeffect.cpp +++ b/src/effects/backends/builtin/metronomeeffect.cpp @@ -1,6 +1,7 @@ #include "metronomeeffect.h" #include +#include #include #include #include @@ -62,6 +63,15 @@ std::span syncedClickOutput(double beatFractionBufferEnd, return {}; } +std::span unsyncedClickOutput(mixxx::audio::SampleRate framesPerSecond, + std::size_t framesSinceLastClick, + double bpm, + std::span output) { + std::size_t offset = framesSinceLastClick % + framesPerBeat(framesPerSecond, bpm); + return subspan_clamped(output, offset * mixxx::kEngineChannelOutputCount); +} + } // namespace // static @@ -163,20 +173,17 @@ void MetronomeEffect::processChannel( playMonoSamplesWithGain(subspan_clamped(click, gs->framesSinceLastClick), output, gain); gs->framesSinceLastClick += engineParameters.framesPerBuffer(); - std::span outputBufferOffset = [&] { - if (shouldSync && hasBeatInfo) { - return syncedClickOutput(*groupFeatures.beat_fraction_buffer_end, - groupFeatures.beat_length, - output); - } else { - // EngineParameters::sampleRate in reality returns the framerate. - mixxx::audio::SampleRate framesPerSecond = engineParameters.sampleRate(); - std::size_t offset = gs->framesSinceLastClick % - framesPerBeat(framesPerSecond, - m_pBpmParameter->value()); - return subspan_clamped(output, offset * mixxx::kEngineChannelOutputCount); - } - }(); + std::span outputBufferOffset = shouldSync && hasBeatInfo + ? syncedClickOutput(*groupFeatures.beat_fraction_buffer_end, + groupFeatures.beat_length, + output) + : unsyncedClickOutput( + engineParameters + .sampleRate(), // engineParameters::sampleRate() + // in reality returns the frameRate + gs->framesSinceLastClick, + m_pBpmParameter->value(), + output); if (!outputBufferOffset.empty()) { gs->framesSinceLastClick = playMonoSamplesWithGain(click, outputBufferOffset, gain); diff --git a/src/effects/backends/builtin/metronomeeffect.h b/src/effects/backends/builtin/metronomeeffect.h index d89e1934035..b98476d39ec 100644 --- a/src/effects/backends/builtin/metronomeeffect.h +++ b/src/effects/backends/builtin/metronomeeffect.h @@ -9,7 +9,7 @@ class MetronomeGroupState final : public EffectState { public: MetronomeGroupState(const mixxx::EngineParameters& engineParameters) - : EffectState(engineParameters){}; + : EffectState(engineParameters) {}; ~MetronomeGroupState() override = default; std::size_t framesSinceLastClick = 0;